Merge pull request #6712 from multiversx/add-proof-update

Change AddProof
multiversx · Jan 20, 2025 · 2a1a875 · 2a1a875
2 parents fd92b5a + e509e47
commit 2a1a875
Show file tree

Hide file tree

Showing 18 changed files with 44 additions and 42 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1,7 +1,6 @@
 package proofscache
 
 import (
-	"encoding/hex"
 	"fmt"
 	"sync"
 
@@ -31,17 +30,17 @@ 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()
 	headerHash := headerProof.GetHeaderHash()
 
 	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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -7,20 +7,20 @@ 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
 	RegisterHandlerCalled          func(handler func(headerProof data.HeaderProofHandler))
 }
 
 // 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 -