Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change AddProof #6712

Merged
merged 8 commits into from
Jan 20, 2025
Merged

Change AddProof #6712

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6cd1ce6
change AddProof to return bool if was added of not
ssd04 Jan 16, 2025
ae23f87
added trace log for AddProof
ssd04 Jan 16, 2025
b1fe372
update proof log
ssd04 Jan 16, 2025
ddbb707
Merge branch 'feat/equivalent-messages' into add-proof-update
ssd04 Jan 20, 2025
eb04577
fixes after merge
ssd04 Jan 20, 2025
275e516
Update consensus/spos/bls/v2/subroundBlock.go
ssd04 Jan 20, 2025
9e359e4
fix import
ssd04 Jan 20, 2025
e509e47
fixes after review
ssd04 Jan 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions common/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
2 changes: 1 addition & 1 deletion consensus/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions consensus/spos/bls/v2/subroundBlock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,9 +367,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", proof.GetHeaderHash())
ssd04 marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
6 changes: 3 additions & 3 deletions consensus/spos/bls/v2/subroundEndRound.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
4 changes: 2 additions & 2 deletions consensus/spos/bls/v2/subroundEndRound_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
})

Expand Down
3 changes: 0 additions & 3 deletions dataRetriever/dataPool/proofsCache/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
9 changes: 4 additions & 5 deletions dataRetriever/dataPool/proofsCache/proofsPool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package proofscache

import (
"encoding/hex"
"fmt"
"sync"

Expand Down Expand Up @@ -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()
Expand All @@ -65,7 +64,7 @@ func (pp *proofsPool) AddProof(

pp.callAddedProofSubscribers(headerProof)

return nil
return true
}

func (pp *proofsPool) callAddedProofSubscribers(headerProof data.HeaderProofHandler) {
Expand Down
4 changes: 2 additions & 2 deletions dataRetriever/dataPool/proofsCache/proofsPool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion dataRetriever/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions integrationTests/testProcessorNode.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2833,9 +2833,9 @@ 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not return an error here

}
Expand Down
3 changes: 1 addition & 2 deletions process/block/interceptedBlocks/interceptedEquivalentProof.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"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"
"github.com/multiversx/mx-chain-go/storage"
Expand Down Expand Up @@ -146,7 +145,7 @@ func (iep *interceptedEquivalentProof) CheckValidity() error {

ok := iep.proofsPool.HasProof(iep.proof.GetHeaderShardId(), iep.proof.GetHeaderHash())
if ok {
return proofscache.ErrAlreadyExistingEquivalentProof
return common.ErrAlreadyExistingEquivalentProof
}

err = iep.checkHeaderParamsFromProof()
Expand Down
8 changes: 7 additions & 1 deletion process/interceptors/processor/equivalentProofsInterceptorProcessor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions process/interceptors/processor/equivalentProofsInterceptorProcessor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,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)
Expand Down
6 changes: 1 addition & 5 deletions process/interceptors/processor/hdrInterceptorProcessor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package processor

import (
"reflect"
"sync"

"github.com/multiversx/mx-chain-core-go/core"
Expand Down Expand Up @@ -82,10 +81,7 @@ 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()))
}
_ = hip.proofs.AddProof(interceptedHdr.HeaderHandler().GetPreviousProof())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a log.Trace here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

}

return nil
Expand Down
4 changes: 2 additions & 2 deletions process/interceptors/processor/hdrInterceptorProcessor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
}

Expand Down
2 changes: 1 addition & 1 deletion process/interceptors/processor/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions testscommon/dataRetriever/proofsPoolMock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, changed

}

// CleanupProofsBehindNonce -
Expand Down
Loading