diff --git a/common/errors.go b/common/errors.go index eeeaf94c80..261b830808 100644 --- a/common/errors.go +++ b/common/errors.go @@ -13,3 +13,6 @@ var ErrNilStateSyncNotifierSubscriber = errors.New("nil state sync notifier subs // ErrInvalidHeaderProof signals that an invalid equivalent proof has been provided var ErrInvalidHeaderProof = errors.New("invalid equivalent proof") + +// ErrAlreadyExistingEquivalentProof signals that the provided proof was already exiting in the pool +var ErrAlreadyExistingEquivalentProof = errors.New("already existing equivalent proof") diff --git a/consensus/interface.go b/consensus/interface.go index 8dfc101817..057206d4b0 100644 --- a/consensus/interface.go +++ b/consensus/interface.go @@ -203,7 +203,7 @@ type KeysHandler interface { // EquivalentProofsPool defines the behaviour of a proofs pool components type EquivalentProofsPool interface { - AddProof(headerProof data.HeaderProofHandler) error + AddProof(headerProof data.HeaderProofHandler) bool GetProof(shardID uint32, headerHash []byte) (data.HeaderProofHandler, error) HasProof(shardID uint32, headerHash []byte) bool IsInterfaceNil() bool diff --git a/consensus/spos/bls/v2/subroundBlock.go b/consensus/spos/bls/v2/subroundBlock.go index 27bf49a7af..3de20efe41 100644 --- a/consensus/spos/bls/v2/subroundBlock.go +++ b/consensus/spos/bls/v2/subroundBlock.go @@ -3,6 +3,7 @@ package v2 import ( "bytes" "context" + "encoding/hex" "time" "github.com/multiversx/mx-chain-core-go/core" @@ -367,9 +368,9 @@ func (sr *subroundBlock) saveProofForPreviousHeaderIfNeeded(header data.HeaderHa return } - err = sr.EquivalentProofsPool().AddProof(proof) - if err != nil { - log.Debug("saveProofForPreviousHeaderIfNeeded: failed to add proof, %w", err) + ok := sr.EquivalentProofsPool().AddProof(proof) + if !ok { + log.Debug("saveProofForPreviousHeaderIfNeeded: proof not added", "headerHash", hex.EncodeToString(proof.GetHeaderHash())) } } diff --git a/consensus/spos/bls/v2/subroundEndRound.go b/consensus/spos/bls/v2/subroundEndRound.go index b91001bb2a..414862bbeb 100644 --- a/consensus/spos/bls/v2/subroundEndRound.go +++ b/consensus/spos/bls/v2/subroundEndRound.go @@ -261,9 +261,9 @@ func (sr *subroundEndRound) doEndRoundJobByNode() bool { // if proof not nil, it was created and broadcasted so it has to be added to the pool if proof != nil { - err = sr.EquivalentProofsPool().AddProof(proof) - if err != nil { - log.Debug("doEndRoundJobByNode.AddProof", "error", err) + ok := sr.EquivalentProofsPool().AddProof(proof) + if !ok { + log.Trace("doEndRoundJobByNode.AddProof", "added", ok) } } diff --git a/consensus/spos/bls/v2/subroundEndRound_test.go b/consensus/spos/bls/v2/subroundEndRound_test.go index 1db112cfff..394df2b8d2 100644 --- a/consensus/spos/bls/v2/subroundEndRound_test.go +++ b/consensus/spos/bls/v2/subroundEndRound_test.go @@ -1244,11 +1244,11 @@ func TestSubroundEndRound_DoEndRoundJobByNode(t *testing.T) { wasSetCurrentHeaderProofCalled := false container.SetEquivalentProofsPool(&dataRetriever.ProofsPoolMock{ - AddProofCalled: func(headerProof data.HeaderProofHandler) error { + AddProofCalled: func(headerProof data.HeaderProofHandler) bool { wasSetCurrentHeaderProofCalled = true require.NotEqual(t, providedPrevSig, headerProof.GetAggregatedSignature()) require.NotEqual(t, providedPrevBitmap, headerProof.GetPubKeysBitmap()) - return nil + return true }, }) diff --git a/dataRetriever/dataPool/proofsCache/errors.go b/dataRetriever/dataPool/proofsCache/errors.go index 630dd8cc39..63376ef0a9 100644 --- a/dataRetriever/dataPool/proofsCache/errors.go +++ b/dataRetriever/dataPool/proofsCache/errors.go @@ -7,6 +7,3 @@ var ErrMissingProof = errors.New("missing proof") // ErrNilProof signals that a nil proof has been provided var ErrNilProof = errors.New("nil proof provided") - -// ErrAlreadyExistingEquivalentProof signals that the provided proof was already exiting in the pool -var ErrAlreadyExistingEquivalentProof = errors.New("already existing equivalent proof") diff --git a/dataRetriever/dataPool/proofsCache/proofsPool.go b/dataRetriever/dataPool/proofsCache/proofsPool.go index a412794a6d..472cc0876b 100644 --- a/dataRetriever/dataPool/proofsCache/proofsPool.go +++ b/dataRetriever/dataPool/proofsCache/proofsPool.go @@ -1,7 +1,6 @@ package proofscache import ( - "encoding/hex" "fmt" "sync" @@ -31,9 +30,9 @@ func NewProofsPool() *proofsPool { // AddProof will add the provided proof to the pool func (pp *proofsPool) AddProof( headerProof data.HeaderProofHandler, -) error { +) bool { if check.IfNilReflect(headerProof) { - return ErrNilProof + return false } shardID := headerProof.GetHeaderShardId() @@ -41,7 +40,7 @@ func (pp *proofsPool) AddProof( hasProof := pp.HasProof(shardID, headerHash) if hasProof { - return fmt.Errorf("%w, headerHash: %s", ErrAlreadyExistingEquivalentProof, hex.EncodeToString(headerHash)) + return false } pp.mutCache.Lock() @@ -65,7 +64,7 @@ func (pp *proofsPool) AddProof( pp.callAddedProofSubscribers(headerProof) - return nil + return true } func (pp *proofsPool) callAddedProofSubscribers(headerProof data.HeaderProofHandler) { diff --git a/dataRetriever/dataPool/proofsCache/proofsPool_test.go b/dataRetriever/dataPool/proofsCache/proofsPool_test.go index c4e373eeba..3073526ef5 100644 --- a/dataRetriever/dataPool/proofsCache/proofsPool_test.go +++ b/dataRetriever/dataPool/proofsCache/proofsPool_test.go @@ -66,8 +66,8 @@ func TestProofsPool_ShouldWork(t *testing.T) { _ = pp.AddProof(proof3) _ = pp.AddProof(proof4) - err := pp.AddProof(proof4) - require.True(t, errors.Is(err, proofscache.ErrAlreadyExistingEquivalentProof)) + ok := pp.AddProof(proof4) + require.False(t, ok) proof, err := pp.GetProof(shardID, []byte("hash3")) require.Nil(t, err) diff --git a/dataRetriever/interface.go b/dataRetriever/interface.go index c28843491d..886ed42a18 100644 --- a/dataRetriever/interface.go +++ b/dataRetriever/interface.go @@ -362,7 +362,7 @@ type PeerAuthenticationPayloadValidator interface { // ProofsPool defines the behaviour of a proofs pool components type ProofsPool interface { - AddProof(headerProof data.HeaderProofHandler) error + AddProof(headerProof data.HeaderProofHandler) bool RegisterHandler(handler func(headerProof data.HeaderProofHandler)) CleanupProofsBehindNonce(shardID uint32, nonce uint64) error GetProof(shardID uint32, headerHash []byte) (data.HeaderProofHandler, error) diff --git a/integrationTests/testProcessorNode.go b/integrationTests/testProcessorNode.go index ae812a195c..8533c24530 100644 --- a/integrationTests/testProcessorNode.go +++ b/integrationTests/testProcessorNode.go @@ -2833,11 +2833,10 @@ func (tpn *TestProcessorNode) setBlockSignatures(blockHeader data.HeaderHandler) } blockHeader.SetPreviousProof(previousProof) - err = tpn.ProofsPool.AddProof(previousProof) - if err != nil { - log.Warn("ProofsPool.AddProof", "currHdrHash", currHdrHash, "node", tpn.OwnAccount.Address, "err", err.Error()) + wasAdded := tpn.ProofsPool.AddProof(previousProof) + if !wasAdded { + log.Warn("ProofsPool.AddProof not added", "currHdrHash", currHdrHash, "node", tpn.OwnAccount.Address) } - return err } err = blockHeader.SetPubKeysBitmap(pubKeysBitmap) diff --git a/process/block/interceptedBlocks/interceptedEquivalentProof.go b/process/block/interceptedBlocks/interceptedEquivalentProof.go index 2ba09e30a0..49bc7cad79 100644 --- a/process/block/interceptedBlocks/interceptedEquivalentProof.go +++ b/process/block/interceptedBlocks/interceptedEquivalentProof.go @@ -12,9 +12,9 @@ import ( logger "github.com/multiversx/mx-chain-logger-go" "github.com/multiversx/mx-chain-vm-v1_2-go/ipc/marshaling" + "github.com/multiversx/mx-chain-go/common" "github.com/multiversx/mx-chain-go/consensus" "github.com/multiversx/mx-chain-go/dataRetriever" - proofscache "github.com/multiversx/mx-chain-go/dataRetriever/dataPool/proofsCache" "github.com/multiversx/mx-chain-go/process" "github.com/multiversx/mx-chain-go/sharding" ) @@ -137,7 +137,7 @@ func (iep *interceptedEquivalentProof) CheckValidity() error { ok := iep.proofsPool.HasProof(iep.proof.GetHeaderShardId(), iep.proof.GetHeaderHash()) if ok { - return proofscache.ErrAlreadyExistingEquivalentProof + return common.ErrAlreadyExistingEquivalentProof } // TODO: make sure proof fields (besides ones used to verify signature) should be checked on processing. diff --git a/process/block/interceptedBlocks/interceptedEquivalentProof_test.go b/process/block/interceptedBlocks/interceptedEquivalentProof_test.go index 85262c297b..8687934409 100644 --- a/process/block/interceptedBlocks/interceptedEquivalentProof_test.go +++ b/process/block/interceptedBlocks/interceptedEquivalentProof_test.go @@ -11,8 +11,8 @@ import ( logger "github.com/multiversx/mx-chain-logger-go" "github.com/stretchr/testify/require" + "github.com/multiversx/mx-chain-go/common" "github.com/multiversx/mx-chain-go/consensus/mock" - proofscache "github.com/multiversx/mx-chain-go/dataRetriever/dataPool/proofsCache" "github.com/multiversx/mx-chain-go/process" "github.com/multiversx/mx-chain-go/testscommon" "github.com/multiversx/mx-chain-go/testscommon/consensus" @@ -202,7 +202,7 @@ func TestInterceptedEquivalentProof_CheckValidity(t *testing.T) { require.NoError(t, err) err = iep.CheckValidity() - require.Equal(t, proofscache.ErrAlreadyExistingEquivalentProof, err) + require.Equal(t, common.ErrAlreadyExistingEquivalentProof, err) }) t.Run("should work", func(t *testing.T) { t.Parallel() diff --git a/process/interceptors/processor/equivalentProofsInterceptorProcessor.go b/process/interceptors/processor/equivalentProofsInterceptorProcessor.go index ef8beff12a..be111820d6 100644 --- a/process/interceptors/processor/equivalentProofsInterceptorProcessor.go +++ b/process/interceptors/processor/equivalentProofsInterceptorProcessor.go @@ -4,6 +4,7 @@ import ( "github.com/multiversx/mx-chain-core-go/core" "github.com/multiversx/mx-chain-core-go/core/check" "github.com/multiversx/mx-chain-core-go/marshal" + "github.com/multiversx/mx-chain-go/common" "github.com/multiversx/mx-chain-go/process" ) @@ -56,7 +57,12 @@ func (epip *equivalentProofsInterceptorProcessor) Save(data process.InterceptedD return process.ErrWrongTypeAssertion } - return epip.equivalentProofsPool.AddProof(interceptedProof.GetProof()) + wasAdded := epip.equivalentProofsPool.AddProof(interceptedProof.GetProof()) + if !wasAdded { + return common.ErrAlreadyExistingEquivalentProof + } + + return nil } // RegisterHandler registers a callback function to be notified of incoming equivalent proofs diff --git a/process/interceptors/processor/equivalentProofsInterceptorProcessor_test.go b/process/interceptors/processor/equivalentProofsInterceptorProcessor_test.go index 94c22b0054..2e9849c80f 100644 --- a/process/interceptors/processor/equivalentProofsInterceptorProcessor_test.go +++ b/process/interceptors/processor/equivalentProofsInterceptorProcessor_test.go @@ -95,9 +95,9 @@ func TestEquivalentProofsInterceptorProcessor_Save(t *testing.T) { wasCalled := false args := createMockArgEquivalentProofsInterceptorProcessor() args.EquivalentProofsPool = &dataRetriever.ProofsPoolMock{ - AddProofCalled: func(notarizedProof data.HeaderProofHandler) error { + AddProofCalled: func(notarizedProof data.HeaderProofHandler) bool { wasCalled = true - return nil + return true }, } epip, err := NewEquivalentProofsInterceptorProcessor(args) diff --git a/process/interceptors/processor/hdrInterceptorProcessor.go b/process/interceptors/processor/hdrInterceptorProcessor.go index c49f2bb970..22950e9178 100644 --- a/process/interceptors/processor/hdrInterceptorProcessor.go +++ b/process/interceptors/processor/hdrInterceptorProcessor.go @@ -1,7 +1,6 @@ package processor import ( - "reflect" "sync" "github.com/multiversx/mx-chain-core-go/core" @@ -82,10 +81,8 @@ func (hip *HdrInterceptorProcessor) Save(data process.InterceptedData, _ core.Pe hip.headers.AddHeader(interceptedHdr.Hash(), interceptedHdr.HeaderHandler()) if common.ShouldBlockHavePrevProof(interceptedHdr.HeaderHandler(), hip.enableEpochsHandler, common.EquivalentMessagesFlag) { - err := hip.proofs.AddProof(interceptedHdr.HeaderHandler().GetPreviousProof()) - if err != nil { - log.Error("failed to add proof", "error", err, "intercepted header hash", interceptedHdr.Hash(), "header type", reflect.TypeOf(interceptedHdr.HeaderHandler())) - } + ok = hip.proofs.AddProof(interceptedHdr.HeaderHandler().GetPreviousProof()) + log.Trace("HdrInterceptorProcessor.AddProof: add previous proof", "intercepted header hash", interceptedHdr.Hash(), "added", ok) } return nil diff --git a/process/interceptors/processor/hdrInterceptorProcessor_test.go b/process/interceptors/processor/hdrInterceptorProcessor_test.go index 6b611f3a1c..1dd904b38f 100644 --- a/process/interceptors/processor/hdrInterceptorProcessor_test.go +++ b/process/interceptors/processor/hdrInterceptorProcessor_test.go @@ -207,9 +207,9 @@ func TestHdrInterceptorProcessor_SaveShouldWork(t *testing.T) { wasAddedProofs := false arg.Proofs = &dataRetriever.ProofsPoolMock{ - AddProofCalled: func(headerProof data.HeaderProofHandler) error { + AddProofCalled: func(headerProof data.HeaderProofHandler) bool { wasAddedProofs = true - return nil + return true }, } diff --git a/process/interceptors/processor/interface.go b/process/interceptors/processor/interface.go index 14c0ae73bd..58d142a09a 100644 --- a/process/interceptors/processor/interface.go +++ b/process/interceptors/processor/interface.go @@ -33,7 +33,7 @@ type interceptedEquivalentProof interface { // EquivalentProofsPool defines the behaviour of a proofs pool components type EquivalentProofsPool interface { - AddProof(headerProof data.HeaderProofHandler) error + AddProof(headerProof data.HeaderProofHandler) bool CleanupProofsBehindNonce(shardID uint32, nonce uint64) error GetProof(shardID uint32, headerHash []byte) (data.HeaderProofHandler, error) HasProof(shardID uint32, headerHash []byte) bool diff --git a/testscommon/dataRetriever/proofsPoolMock.go b/testscommon/dataRetriever/proofsPoolMock.go index 09a5d4646b..7461f28abe 100644 --- a/testscommon/dataRetriever/proofsPoolMock.go +++ b/testscommon/dataRetriever/proofsPoolMock.go @@ -7,7 +7,7 @@ import ( // ProofsPoolMock - type ProofsPoolMock struct { - AddProofCalled func(headerProof data.HeaderProofHandler) error + AddProofCalled func(headerProof data.HeaderProofHandler) bool CleanupProofsBehindNonceCalled func(shardID uint32, nonce uint64) error GetProofCalled func(shardID uint32, headerHash []byte) (data.HeaderProofHandler, error) HasProofCalled func(shardID uint32, headerHash []byte) bool @@ -15,12 +15,12 @@ type ProofsPoolMock struct { } // AddProof - -func (p *ProofsPoolMock) AddProof(headerProof data.HeaderProofHandler) error { +func (p *ProofsPoolMock) AddProof(headerProof data.HeaderProofHandler) bool { if p.AddProofCalled != nil { return p.AddProofCalled(headerProof) } - return nil + return true } // CleanupProofsBehindNonce -