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

Jump to bottom

Fixed wrong multikey backup machine step-in #5840

Merged
merged 7 commits into from
Jan 23, 2024
Merged

Fixed wrong multikey backup machine step-in #5840

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e8bccc0
- fixed wrong backup machine step-in by correctly resetting counters …
iulianpascalau Jan 16, 2024
7cf5d48
Merge branch 'rc/sirius-patch-2' into fix-wrong-backup-step-in
iulianpascalau Jan 16, 2024
5042328
- fix after merge
iulianpascalau Jan 16, 2024
e9ad64c
- fixes after review: renaming + code optimization
iulianpascalau Jan 17, 2024
8ed7c58
Merge branch 'rc/sirius-patch-2' into fix-wrong-backup-step-in
iulianpascalau Jan 17, 2024
2bc1c58
- fix after review
iulianpascalau Jan 18, 2024
7cab217
Merge branch 'rc/sirius-patch-2' into fix-wrong-backup-step-in
iulianpascalau Jan 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion consensus/spos/bls/blsSubroundsFactory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/multiversx/mx-chain-core-go/core"
"github.com/multiversx/mx-chain-core-go/core/check"
"github.com/multiversx/mx-chain-go/consensus/spos"
"github.com/multiversx/mx-chain-go/errors"
"github.com/multiversx/mx-chain-go/outport"
)

Expand Down Expand Up @@ -80,7 +81,7 @@ func checkNewFactoryParams(
return spos.ErrNilAppStatusHandler
}
if check.IfNil(sentSignaturesTracker) {
return spos.ErrNilSentSignatureTracker
return errors.ErrNilSentSignatureTracker
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason to use the error in errors package?
it adds a new package dependency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

}
if len(chainID) == 0 {
return spos.ErrInvalidChainID
Expand Down
38 changes: 20 additions & 18 deletions consensus/spos/bls/blsSubroundsFactory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import (
"github.com/multiversx/mx-chain-go/consensus/mock"
"github.com/multiversx/mx-chain-go/consensus/spos"
"github.com/multiversx/mx-chain-go/consensus/spos/bls"
"github.com/multiversx/mx-chain-go/errors"
"github.com/multiversx/mx-chain-go/outport"
"github.com/multiversx/mx-chain-go/testscommon"
testscommonOutport "github.com/multiversx/mx-chain-go/testscommon/outport"
"github.com/multiversx/mx-chain-go/testscommon/statusHandler"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -76,7 +78,7 @@ func initFactoryWithContainer(container *mock.ConsensusCoreMock) bls.Factory {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

return fct
Expand Down Expand Up @@ -125,7 +127,7 @@ func TestFactory_NewFactoryNilContainerShouldFail(t *testing.T) {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -145,7 +147,7 @@ func TestFactory_NewFactoryNilConsensusStateShouldFail(t *testing.T) {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -167,7 +169,7 @@ func TestFactory_NewFactoryNilBlockchainShouldFail(t *testing.T) {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -189,7 +191,7 @@ func TestFactory_NewFactoryNilBlockProcessorShouldFail(t *testing.T) {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -211,7 +213,7 @@ func TestFactory_NewFactoryNilBootstrapperShouldFail(t *testing.T) {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -233,7 +235,7 @@ func TestFactory_NewFactoryNilChronologyHandlerShouldFail(t *testing.T) {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -255,7 +257,7 @@ func TestFactory_NewFactoryNilHasherShouldFail(t *testing.T) {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -277,7 +279,7 @@ func TestFactory_NewFactoryNilMarshalizerShouldFail(t *testing.T) {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -299,7 +301,7 @@ func TestFactory_NewFactoryNilMultiSignerContainerShouldFail(t *testing.T) {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -321,7 +323,7 @@ func TestFactory_NewFactoryNilRoundHandlerShouldFail(t *testing.T) {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -343,7 +345,7 @@ func TestFactory_NewFactoryNilShardCoordinatorShouldFail(t *testing.T) {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -365,7 +367,7 @@ func TestFactory_NewFactoryNilSyncTimerShouldFail(t *testing.T) {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -387,7 +389,7 @@ func TestFactory_NewFactoryNilValidatorGroupSelectorShouldFail(t *testing.T) {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -407,7 +409,7 @@ func TestFactory_NewFactoryNilWorkerShouldFail(t *testing.T) {
chainID,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -428,7 +430,7 @@ func TestFactory_NewFactoryNilAppStatusHandlerShouldFail(t *testing.T) {
chainID,
currentPid,
nil,
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand All @@ -453,7 +455,7 @@ func TestFactory_NewFactoryNilSignaturesTrackerShouldFail(t *testing.T) {
)

assert.Nil(t, fct)
assert.Equal(t, spos.ErrNilSentSignatureTracker, err)
assert.Equal(t, errors.ErrNilSentSignatureTracker, err)
}

func TestFactory_NewFactoryShouldWork(t *testing.T) {
Expand All @@ -478,7 +480,7 @@ func TestFactory_NewFactoryEmptyChainIDShouldFail(t *testing.T) {
nil,
currentPid,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, fct)
Expand Down
6 changes: 2 additions & 4 deletions consensus/spos/bls/subroundEndRound.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/multiversx/mx-chain-go/common"
"github.com/multiversx/mx-chain-go/consensus"
"github.com/multiversx/mx-chain-go/consensus/spos"
"github.com/multiversx/mx-chain-go/errors"
"github.com/multiversx/mx-chain-go/p2p"
)

Expand Down Expand Up @@ -48,7 +49,7 @@ func NewSubroundEndRound(
return nil, spos.ErrNilAppStatusHandler
}
if check.IfNil(sentSignatureTracker) {
return nil, spos.ErrNilSentSignatureTracker
return nil, errors.ErrNilSentSignatureTracker
}

srEndRound := subroundEndRound{
Expand Down Expand Up @@ -120,9 +121,6 @@ func (sr *subroundEndRound) receivedBlockHeaderFinalInfo(_ context.Context, cnsD
"AggregateSignature", cnsDta.AggregateSignature,
"LeaderSignature", cnsDta.LeaderSignature)

signers := computeSignersPublicKeys(sr.ConsensusGroup(), cnsDta.PubKeysBitmap)
sr.sentSignatureTracker.ReceivedActualSigners(signers)

sr.PeerHonestyHandler().ChangeScore(
node,
spos.GetConsensusTopicID(sr.ShardCoordinator()),
Expand Down
37 changes: 15 additions & 22 deletions consensus/spos/bls/subroundEndRound_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/multiversx/mx-chain-go/consensus/spos"
"github.com/multiversx/mx-chain-go/consensus/spos/bls"
"github.com/multiversx/mx-chain-go/dataRetriever/blockchain"
mxErrors "github.com/multiversx/mx-chain-go/errors"
"github.com/multiversx/mx-chain-go/p2p"
"github.com/multiversx/mx-chain-go/p2p/factory"
"github.com/multiversx/mx-chain-go/testscommon"
Expand Down Expand Up @@ -55,7 +56,7 @@ func initSubroundEndRoundWithContainer(
bls.ProcessingThresholdPercent,
displayStatistics,
appStatusHandler,
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

return srEndRound
Expand Down Expand Up @@ -97,7 +98,7 @@ func TestNewSubroundEndRound(t *testing.T) {
bls.ProcessingThresholdPercent,
displayStatistics,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, srEndRound)
Expand All @@ -112,7 +113,7 @@ func TestNewSubroundEndRound(t *testing.T) {
bls.ProcessingThresholdPercent,
displayStatistics,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, srEndRound)
Expand All @@ -127,7 +128,7 @@ func TestNewSubroundEndRound(t *testing.T) {
bls.ProcessingThresholdPercent,
displayStatistics,
nil,
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.Nil(t, srEndRound)
Expand All @@ -146,7 +147,7 @@ func TestNewSubroundEndRound(t *testing.T) {
)

assert.Nil(t, srEndRound)
assert.Equal(t, spos.ErrNilSentSignatureTracker, err)
assert.Equal(t, mxErrors.ErrNilSentSignatureTracker, err)
})
}

Expand Down Expand Up @@ -179,7 +180,7 @@ func TestSubroundEndRound_NewSubroundEndRoundNilBlockChainShouldFail(t *testing.
bls.ProcessingThresholdPercent,
displayStatistics,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.True(t, check.IfNil(srEndRound))
Expand Down Expand Up @@ -215,7 +216,7 @@ func TestSubroundEndRound_NewSubroundEndRoundNilBlockProcessorShouldFail(t *test
bls.ProcessingThresholdPercent,
displayStatistics,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.True(t, check.IfNil(srEndRound))
Expand Down Expand Up @@ -252,7 +253,7 @@ func TestSubroundEndRound_NewSubroundEndRoundNilConsensusStateShouldFail(t *test
bls.ProcessingThresholdPercent,
displayStatistics,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.True(t, check.IfNil(srEndRound))
Expand Down Expand Up @@ -288,7 +289,7 @@ func TestSubroundEndRound_NewSubroundEndRoundNilMultiSignerContainerShouldFail(t
bls.ProcessingThresholdPercent,
displayStatistics,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.True(t, check.IfNil(srEndRound))
Expand Down Expand Up @@ -324,7 +325,7 @@ func TestSubroundEndRound_NewSubroundEndRoundNilRoundHandlerShouldFail(t *testin
bls.ProcessingThresholdPercent,
displayStatistics,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.True(t, check.IfNil(srEndRound))
Expand Down Expand Up @@ -360,7 +361,7 @@ func TestSubroundEndRound_NewSubroundEndRoundNilSyncTimerShouldFail(t *testing.T
bls.ProcessingThresholdPercent,
displayStatistics,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.True(t, check.IfNil(srEndRound))
Expand Down Expand Up @@ -396,7 +397,7 @@ func TestSubroundEndRound_NewSubroundEndRoundShouldWork(t *testing.T) {
bls.ProcessingThresholdPercent,
displayStatistics,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

assert.False(t, check.IfNil(srEndRound))
Expand Down Expand Up @@ -902,16 +903,8 @@ func TestSubroundEndRound_ReceivedBlockHeaderFinalInfoShouldWork(t *testing.T) {
PubKey: []byte("A"),
}

sentTrackerInterface := sr.GetSentSignatureTracker()
sentTracker := sentTrackerInterface.(*mock.SentSignatureTrackerStub)
receivedActualSignersCalled := false
sentTracker.ReceivedActualSignersCalled = func(signersPks []string) {
receivedActualSignersCalled = true
}

res := sr.ReceivedBlockHeaderFinalInfo(&cnsData)
assert.True(t, res)
assert.True(t, receivedActualSignersCalled)
}

func TestSubroundEndRound_ReceivedBlockHeaderFinalInfoShouldReturnFalseWhenFinalInfoIsNotValid(t *testing.T) {
Expand Down Expand Up @@ -1372,7 +1365,7 @@ func TestSubroundEndRound_ReceivedInvalidSignersInfo(t *testing.T) {
bls.ProcessingThresholdPercent,
displayStatistics,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

srEndRound.SetSelfPubKey("A")
Expand Down Expand Up @@ -1743,7 +1736,7 @@ func TestSubroundEndRound_getMinConsensusGroupIndexOfManagedKeys(t *testing.T) {
bls.ProcessingThresholdPercent,
displayStatistics,
&statusHandler.AppStatusHandlerStub{},
&mock.SentSignatureTrackerStub{},
&testscommon.SentSignatureTrackerStub{},
)

t.Run("no managed keys from consensus group", func(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion consensus/spos/bls/subroundSignature.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/multiversx/mx-chain-go/common"
"github.com/multiversx/mx-chain-go/consensus"
"github.com/multiversx/mx-chain-go/consensus/spos"
"github.com/multiversx/mx-chain-go/errors"
)

type subroundSignature struct {
Expand Down Expand Up @@ -39,7 +40,7 @@ func NewSubroundSignature(
return nil, spos.ErrNilAppStatusHandler
}
if check.IfNil(sentSignatureTracker) {
return nil, spos.ErrNilSentSignatureTracker
return nil, errors.ErrNilSentSignatureTracker
}

srSignature := subroundSignature{
Expand Down