From 4f88b45f4fb9a90a6c51cd351784feba9c6c57e0 Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Thu, 2 Nov 2023 12:51:34 +0100 Subject: [PATCH 1/7] Use safemath in accounts --- components/debugapi/node.go | 7 +++- components/restapi/core/accounts.go | 19 +++++++---- components/restapi/core/component.go | 5 ++- pkg/core/account/accounts.go | 34 ++++++++++++++----- pkg/core/account/accounts_test.go | 6 ++-- pkg/core/account/seated_accounts.go | 9 +++-- pkg/core/account/seated_accounts_test.go | 9 ++--- pkg/protocol/commitment_verifier.go | 7 +++- .../slotattestation/testframework_test.go | 4 ++- .../engine/filter/blockfilter/filter_test.go | 4 ++- .../conflictdag/tests/accounts_framework.go | 4 ++- .../seatmanager/mock/mockseatmanager.go | 18 ++++++---- .../sybilprotection/seatmanager/poa/poa.go | 18 +++++++--- .../seatmanager/topstakers/topstakers.go | 27 +++++++++++---- .../seatmanager/topstakers/topstakers_test.go | 29 +++++++++------- .../performance/testsuite_test.go | 6 ++-- .../sybilprotectionv1/sybilprotection.go | 16 +++++++-- pkg/storage/testframework_test.go | 4 ++- pkg/testsuite/sybilprotection.go | 14 +++++--- 19 files changed, 169 insertions(+), 71 deletions(-) diff --git a/components/debugapi/node.go b/components/debugapi/node.go index 280311b76..e625cfdc6 100644 --- a/components/debugapi/node.go +++ b/components/debugapi/node.go @@ -17,7 +17,12 @@ func validatorsSummary() (*ValidatorsSummaryResponse, error) { } var validatorSeats []*Validator - latestCommittee.Accounts().ForEach(func(id iotago.AccountID, pool *account.Pool) bool { + accounts, err := latestCommittee.Accounts() + if err != nil { + return nil, ierrors.Wrapf(err, "failed to get accounts from committee for slot %d", latestSlotIndex) + } + + accounts.ForEach(func(id iotago.AccountID, pool *account.Pool) bool { validatorSeats = append(validatorSeats, &Validator{ AccountID: id, SeatIndex: uint8(lo.Return1(latestCommittee.GetSeat(id))), diff --git a/components/restapi/core/accounts.go b/components/restapi/core/accounts.go index 91c6211c3..e6080f473 100644 --- a/components/restapi/core/accounts.go +++ b/components/restapi/core/accounts.go @@ -188,7 +188,7 @@ func rewardsByOutputID(c echo.Context) (*apimodels.ManaRewardsResponse, error) { }, nil } -func selectedCommittee(c echo.Context) *apimodels.CommitteeResponse { +func selectedCommittee(c echo.Context) (*apimodels.CommitteeResponse, error) { timeProvider := deps.Protocol.CommittedAPI().TimeProvider() var slot iotago.SlotIndex @@ -206,11 +206,16 @@ func selectedCommittee(c echo.Context) *apimodels.CommitteeResponse { if !exists { return &apimodels.CommitteeResponse{ Epoch: epoch, - } + }, nil + } + + accounts, err := seatedAccounts.Accounts() + if err != nil { + return nil, ierrors.Wrapf(err, "failed to get accounts from committee for slot %d", slot) } - committee := make([]*apimodels.CommitteeMemberResponse, 0, seatedAccounts.Accounts().Size()) - seatedAccounts.Accounts().ForEach(func(accountID iotago.AccountID, seat *account.Pool) bool { + committee := make([]*apimodels.CommitteeMemberResponse, 0, accounts.Size()) + accounts.ForEach(func(accountID iotago.AccountID, seat *account.Pool) bool { committee = append(committee, &apimodels.CommitteeMemberResponse{ AccountID: accountID, PoolStake: seat.PoolStake, @@ -224,7 +229,7 @@ func selectedCommittee(c echo.Context) *apimodels.CommitteeResponse { return &apimodels.CommitteeResponse{ Epoch: epoch, Committee: committee, - TotalStake: seatedAccounts.Accounts().TotalStake(), - TotalValidatorStake: seatedAccounts.Accounts().TotalValidatorStake(), - } + TotalStake: accounts.TotalStake(), + TotalValidatorStake: accounts.TotalValidatorStake(), + }, nil } diff --git a/components/restapi/core/component.go b/components/restapi/core/component.go index 1d1784943..8361b8d8f 100644 --- a/components/restapi/core/component.go +++ b/components/restapi/core/component.go @@ -357,7 +357,10 @@ func configure() error { }, checkNodeSynced()) routeGroup.GET(RouteCommittee, func(c echo.Context) error { - resp := selectedCommittee(c) + resp, err := selectedCommittee(c) + if err != nil { + return err + } return responseByHeader(c, resp) }, checkNodeSynced()) diff --git a/pkg/core/account/accounts.go b/pkg/core/account/accounts.go index 45fdc7f3c..770a72de1 100644 --- a/pkg/core/account/accounts.go +++ b/pkg/core/account/accounts.go @@ -6,6 +6,7 @@ import ( "io" "sync/atomic" + "github.com/iotaledger/hive.go/core/safemath" "github.com/iotaledger/hive.go/ds/shrinkingmap" "github.com/iotaledger/hive.go/ierrors" "github.com/iotaledger/hive.go/runtime/syncutils" @@ -71,31 +72,44 @@ func (a *Accounts) Get(id iotago.AccountID) (pool *Pool, exists bool) { } // setWithoutLocking sets the weight of the given identity. -func (a *Accounts) setWithoutLocking(id iotago.AccountID, pool *Pool) { +func (a *Accounts) setWithoutLocking(id iotago.AccountID, pool *Pool) error { value, created := a.accountPools.GetOrCreate(id, func() *Pool { return pool }) + var safeMathErr error + if !created { // if there was already an entry, we need to subtract the former // stake first and set the new value - // TODO: use safemath - a.totalStake -= value.PoolStake - a.totalValidatorStake -= value.ValidatorStake + if a.totalStake, safeMathErr = safemath.SafeSub(a.totalStake, value.PoolStake); safeMathErr != nil { + return ierrors.Wrapf(safeMathErr, "failed to subtract pool stake from total stake for account %s", id.String()) + } + + if a.totalValidatorStake, safeMathErr = safemath.SafeSub(a.totalValidatorStake, value.ValidatorStake); safeMathErr != nil { + return ierrors.Wrapf(safeMathErr, "failed to subtract validator stake from total validator stake for account %s", id.String()) + } a.accountPools.Set(id, pool) } - a.totalStake += pool.PoolStake - a.totalValidatorStake += pool.ValidatorStake + if a.totalStake, safeMathErr = safemath.SafeAdd(a.totalStake, pool.PoolStake); safeMathErr != nil { + return ierrors.Wrapf(safeMathErr, "failed to add pool stake to total stake for account %s", id.String()) + } + + if a.totalValidatorStake, safeMathErr = safemath.SafeAdd(a.totalValidatorStake, pool.ValidatorStake); safeMathErr != nil { + return ierrors.Wrapf(safeMathErr, "failed to add validator stake to total validator stake for account %s", id.String()) + } + + return nil } // Set sets the weight of the given identity. -func (a *Accounts) Set(id iotago.AccountID, pool *Pool) { +func (a *Accounts) Set(id iotago.AccountID, pool *Pool) error { a.mutex.Lock() defer a.mutex.Unlock() - a.setWithoutLocking(id, pool) + return a.setWithoutLocking(id, pool) } func (a *Accounts) TotalStake() iotago.BaseToken { @@ -168,7 +182,9 @@ func (a *Accounts) readFromReadSeeker(reader io.ReadSeeker) (n int, err error) { return 0, ierrors.Wrap(err, "invalid pool bytes length") } - a.setWithoutLocking(accountID, pool) + if err := a.setWithoutLocking(accountID, pool); err != nil { + return 0, ierrors.Wrapf(err, "failed to set pool for account %s", accountID.String()) + } } var reused bool diff --git a/pkg/core/account/accounts_test.go b/pkg/core/account/accounts_test.go index 7838ddf6b..ffc7dbe8e 100644 --- a/pkg/core/account/accounts_test.go +++ b/pkg/core/account/accounts_test.go @@ -20,11 +20,13 @@ func TestAccounts(t *testing.T) { // check "Set" for id, stake := range issuers { - accounts.Set(id, &account.Pool{ + if err := accounts.Set(id, &account.Pool{ PoolStake: iotago.BaseToken(stake), ValidatorStake: iotago.BaseToken(stake) * 2, FixedCost: iotago.Mana(stake) * 3, - }) + }); err != nil { + t.Fatal(err) + } } // check "Size" diff --git a/pkg/core/account/seated_accounts.go b/pkg/core/account/seated_accounts.go index 4e8abffb1..070eda6e4 100644 --- a/pkg/core/account/seated_accounts.go +++ b/pkg/core/account/seated_accounts.go @@ -78,19 +78,22 @@ func (s *SeatedAccounts) SeatCount() int { return s.seatsByAccount.Size() } -func (s *SeatedAccounts) Accounts() *Accounts { +func (s *SeatedAccounts) Accounts() (*Accounts, error) { accounts := NewAccounts() + var err error s.seatsByAccount.ForEachKey(func(id iotago.AccountID) bool { pool, exists := s.accounts.Get(id) if !exists { panic("account not found") } - accounts.Set(id, pool) + if err = accounts.Set(id, pool); err != nil { + return false + } return true }) - return accounts + return accounts, err } func (s *SeatedAccounts) String() string { diff --git a/pkg/core/account/seated_accounts_test.go b/pkg/core/account/seated_accounts_test.go index 2158d34d6..c87b8abda 100644 --- a/pkg/core/account/seated_accounts_test.go +++ b/pkg/core/account/seated_accounts_test.go @@ -19,9 +19,9 @@ func TestSelectedAccounts(t *testing.T) { account3 := iotago.AccountID([32]byte{3}) account4 := iotago.AccountID([32]byte{4}) - accounts.Set(account1, &account.Pool{}) - accounts.Set(account2, &account.Pool{}) - accounts.Set(account3, &account.Pool{}) + require.NoError(t, accounts.Set(account1, &account.Pool{})) + require.NoError(t, accounts.Set(account2, &account.Pool{})) + require.NoError(t, accounts.Set(account3, &account.Pool{})) // Create a new set of selected accounts seatedAccounts := account.NewSeatedAccounts(accounts, account1, account3) @@ -65,7 +65,8 @@ func TestSelectedAccounts(t *testing.T) { require.True(t, has) // Test the "Members" method - members := seatedAccounts.Accounts() + members, err := seatedAccounts.Accounts() + require.NoError(t, err) require.Equal(t, 2, members.Size()) require.True(t, members.Has(account2)) require.True(t, members.Has(account3)) diff --git a/pkg/protocol/commitment_verifier.go b/pkg/protocol/commitment_verifier.go index 1315071e3..7289fd86d 100644 --- a/pkg/protocol/commitment_verifier.go +++ b/pkg/protocol/commitment_verifier.go @@ -25,10 +25,15 @@ func NewCommitmentVerifier(mainEngine *engine.Engine, lastCommonCommitmentBefore return nil, ierrors.Errorf("committee in slot %d does not exist", lastCommonCommitmentBeforeFork.Slot()) } + accountsAtForkingPoint, err := committeeAtForkingPoint.Accounts() + if err != nil { + return nil, ierrors.Wrapf(err, "failed to get accounts from committee for slot %d", lastCommonCommitmentBeforeFork.Slot()) + } + return &CommitmentVerifier{ engine: mainEngine, cumulativeWeight: lastCommonCommitmentBeforeFork.CumulativeWeight(), - validatorAccountsAtFork: lo.PanicOnErr(mainEngine.Ledger.PastAccounts(committeeAtForkingPoint.Accounts().IDs(), lastCommonCommitmentBeforeFork.Slot())), + validatorAccountsAtFork: lo.PanicOnErr(mainEngine.Ledger.PastAccounts(accountsAtForkingPoint.IDs(), lastCommonCommitmentBeforeFork.Slot())), // TODO: what happens if the committee rotated after the fork? }, nil } diff --git a/pkg/protocol/engine/attestation/slotattestation/testframework_test.go b/pkg/protocol/engine/attestation/slotattestation/testframework_test.go index ad77b48fe..1214dd6b3 100644 --- a/pkg/protocol/engine/attestation/slotattestation/testframework_test.go +++ b/pkg/protocol/engine/attestation/slotattestation/testframework_test.go @@ -61,7 +61,9 @@ func NewTestFramework(test *testing.T) *TestFramework { accounts := account.NewAccounts() var members []iotago.AccountID t.issuerByAlias.ForEach(func(alias string, issuer *issuer) bool { - accounts.Set(issuer.accountID, &account.Pool{}) // we don't care about pools with PoA + if err := accounts.Set(issuer.accountID, &account.Pool{}); err != nil { // we don't care about pools with PoA + test.Fatal(err) + } members = append(members, issuer.accountID) return true }) diff --git a/pkg/protocol/engine/filter/blockfilter/filter_test.go b/pkg/protocol/engine/filter/blockfilter/filter_test.go index 6f1b4bdc4..2ab3aac3f 100644 --- a/pkg/protocol/engine/filter/blockfilter/filter_test.go +++ b/pkg/protocol/engine/filter/blockfilter/filter_test.go @@ -98,7 +98,9 @@ func (t *TestFramework) IssueBlockAtSlotWithVersion(alias string, slot iotago.Sl func mockedCommitteeFunc(validatorAccountID iotago.AccountID) func(iotago.SlotIndex) (*account.SeatedAccounts, bool) { mockedAccounts := account.NewAccounts() - mockedAccounts.Set(validatorAccountID, new(account.Pool)) + if err := mockedAccounts.Set(validatorAccountID, new(account.Pool)); err != nil { + panic(err) + } seatedAccounts := account.NewSeatedAccounts(mockedAccounts) seatedAccounts.Set(account.SeatIndex(0), validatorAccountID) diff --git a/pkg/protocol/engine/mempool/conflictdag/tests/accounts_framework.go b/pkg/protocol/engine/mempool/conflictdag/tests/accounts_framework.go index bc54c0fc8..6a32efefb 100644 --- a/pkg/protocol/engine/mempool/conflictdag/tests/accounts_framework.go +++ b/pkg/protocol/engine/mempool/conflictdag/tests/accounts_framework.go @@ -74,7 +74,9 @@ func (f *AccountsTestFramework) CreateID(alias string) iotago.AccountID { validatorID := iotago.AccountIDFromData(hashedAlias[:]) validatorID.RegisterAlias(alias) - f.Instance.Set(validatorID, &account.Pool{}) // we don't care about pools when doing PoA + if err := f.Instance.Set(validatorID, &account.Pool{}); err != nil { // we don't care about pools when doing PoA + f.test.Fatal(err) + } f.Committee.Set(account.SeatIndex(f.Committee.SeatCount()), validatorID) f.identitiesByAlias[alias] = validatorID diff --git a/pkg/protocol/sybilprotection/seatmanager/mock/mockseatmanager.go b/pkg/protocol/sybilprotection/seatmanager/mock/mockseatmanager.go index d8b53b4a1..7e90c5bc4 100644 --- a/pkg/protocol/sybilprotection/seatmanager/mock/mockseatmanager.go +++ b/pkg/protocol/sybilprotection/seatmanager/mock/mockseatmanager.go @@ -61,11 +61,13 @@ func NewManualPOAProvider() module.Provider[*engine.Engine, seatmanager.SeatMana func (m *ManualPOA) AddRandomAccount(alias string) iotago.AccountID { id := iotago.AccountID(tpkg.Rand32ByteArray()) id.RegisterAlias(alias) - m.accounts.Set(id, &account.Pool{ + if err := m.accounts.Set(id, &account.Pool{ // We don't care about pools with PoA, but need to set something to avoid division by zero errors. PoolStake: 1, ValidatorStake: 1, FixedCost: 1, - }) // We don't care about pools with PoA, but need to set something to avoid division by zero errors. + }); err != nil { + panic(err) + } m.aliases.Set(alias, id) @@ -79,11 +81,13 @@ func (m *ManualPOA) AddRandomAccount(alias string) iotago.AccountID { } func (m *ManualPOA) AddAccount(id iotago.AccountID, alias string) iotago.AccountID { - m.accounts.Set(id, &account.Pool{ + if err := m.accounts.Set(id, &account.Pool{ // We don't care about pools with PoA, but need to set something to avoid division by zero errors. PoolStake: 1, ValidatorStake: 1, FixedCost: 1, - }) // We don't care about pools with PoA, but need to set something to avoid division by zero errors. + }); err != nil { + panic(err) + } m.aliases.Set(alias, id) m.committee = m.accounts.SelectCommittee(m.accounts.IDs()...) @@ -164,11 +168,13 @@ func (m *ManualPOA) RotateCommittee(epoch iotago.EpochIndex, validators accounts m.accounts = account.NewAccounts() for _, validatorData := range validators { - m.accounts.Set(validatorData.ID, &account.Pool{ + if err := m.accounts.Set(validatorData.ID, &account.Pool{ PoolStake: validatorData.ValidatorStake + validatorData.DelegationStake, ValidatorStake: validatorData.ValidatorStake, FixedCost: validatorData.FixedCost, - }) + }); err != nil { + return nil, ierrors.Wrapf(err, "error while setting pool for epoch %d for validator %s", epoch, validatorData.ID.String()) + } } m.committee = m.accounts.SelectCommittee(m.accounts.IDs()...) } diff --git a/pkg/protocol/sybilprotection/seatmanager/poa/poa.go b/pkg/protocol/sybilprotection/seatmanager/poa/poa.go index d73ce7191..f5c113c76 100644 --- a/pkg/protocol/sybilprotection/seatmanager/poa/poa.go +++ b/pkg/protocol/sybilprotection/seatmanager/poa/poa.go @@ -89,17 +89,23 @@ func (s *SeatManager) RotateCommittee(epoch iotago.EpochIndex, validators accoun committeeAccounts := account.NewAccounts() for _, validatorData := range validators { - committeeAccounts.Set(validatorData.ID, &account.Pool{ + if err := committeeAccounts.Set(validatorData.ID, &account.Pool{ PoolStake: validatorData.ValidatorStake + validatorData.DelegationStake, ValidatorStake: validatorData.ValidatorStake, FixedCost: validatorData.FixedCost, - }) + }); err != nil { + return nil, ierrors.Wrapf(err, "error while setting committee for epoch %d for validator %s", epoch, validatorData.ID.String()) + } } s.committee = committeeAccounts.SelectCommittee(committeeAccounts.IDs()...) } - err := s.committeeStore.Store(epoch, s.committee.Accounts()) + accounts, err := s.committee.Accounts() if err != nil { + return nil, ierrors.Wrapf(err, "error while getting accounts from committee for epoch %d", epoch) + } + + if err := s.committeeStore.Store(epoch, accounts); err != nil { return nil, ierrors.Wrapf(err, "error while storing committee for epoch %d", epoch) } @@ -186,8 +192,12 @@ func (s *SeatManager) SetCommittee(epoch iotago.EpochIndex, validators *account. s.committee = validators.SelectCommittee(validators.IDs()...) - err := s.committeeStore.Store(epoch, s.committee.Accounts()) + accounts, err := s.committee.Accounts() if err != nil { + return ierrors.Wrapf(err, "failed to get accounts from committee for epoch %d", epoch) + } + + if err := s.committeeStore.Store(epoch, accounts); err != nil { return ierrors.Wrapf(err, "failed to set committee for epoch %d", epoch) } diff --git a/pkg/protocol/sybilprotection/seatmanager/topstakers/topstakers.go b/pkg/protocol/sybilprotection/seatmanager/topstakers/topstakers.go index 4d572c888..464b83c9c 100644 --- a/pkg/protocol/sybilprotection/seatmanager/topstakers/topstakers.go +++ b/pkg/protocol/sybilprotection/seatmanager/topstakers/topstakers.go @@ -98,18 +98,29 @@ func (s *SeatManager) RotateCommittee(epoch iotago.EpochIndex, candidates accoun return nil, ierrors.Errorf("cannot re-use previous committee from epoch %d as it does not exist", epoch-1) } - err := s.committeeStore.Store(epoch, committee.Accounts()) + accounts, err := committee.Accounts() if err != nil { + return nil, ierrors.Wrapf(err, "error while getting accounts from committee for epoch %d", epoch-1) + } + + if err := s.committeeStore.Store(epoch, accounts); err != nil { return nil, ierrors.Wrapf(err, "error while storing committee for epoch %d", epoch) } return committee, nil } - committee := s.selectNewCommittee(candidates) + committee, err := s.selectNewCommittee(candidates) + if err != nil { + return nil, ierrors.Wrap(err, "error while selecting new committee") + } - err := s.committeeStore.Store(epoch, committee.Accounts()) + accounts, err := committee.Accounts() if err != nil { + return nil, ierrors.Wrapf(err, "error while getting accounts for newly selected committee for epoch %d", epoch) + } + + if err := s.committeeStore.Store(epoch, accounts); err != nil { return nil, ierrors.Wrapf(err, "error while storing committee for epoch %d", epoch) } @@ -203,7 +214,7 @@ func (s *SeatManager) SetCommittee(epoch iotago.EpochIndex, validators *account. return nil } -func (s *SeatManager) selectNewCommittee(candidates accounts.AccountsData) *account.SeatedAccounts { +func (s *SeatManager) selectNewCommittee(candidates accounts.AccountsData) (*account.SeatedAccounts, error) { sort.Slice(candidates, func(i, j int) bool { // Prioritize the candidate that has a larger pool stake. if candidates[i].ValidatorStake+candidates[i].DelegationStake != candidates[j].ValidatorStake+candidates[j].DelegationStake { @@ -233,13 +244,15 @@ func (s *SeatManager) selectNewCommittee(candidates accounts.AccountsData) *acco newCommitteeAccounts := account.NewAccounts() for _, candidateData := range candidates[:s.optsSeatCount] { - newCommitteeAccounts.Set(candidateData.ID, &account.Pool{ + if err := newCommitteeAccounts.Set(candidateData.ID, &account.Pool{ PoolStake: candidateData.ValidatorStake + candidateData.DelegationStake, ValidatorStake: candidateData.ValidatorStake, FixedCost: candidateData.FixedCost, - }) + }); err != nil { + return nil, ierrors.Wrapf(err, "error while setting pool for committee candidate %s", candidateData.ID.String()) + } } committee := newCommitteeAccounts.SelectCommittee(newCommitteeAccounts.IDs()...) - return committee + return committee, nil } diff --git a/pkg/protocol/sybilprotection/seatmanager/topstakers/topstakers_test.go b/pkg/protocol/sybilprotection/seatmanager/topstakers/topstakers_test.go index 4b539e72f..54790e263 100644 --- a/pkg/protocol/sybilprotection/seatmanager/topstakers/topstakers_test.go +++ b/pkg/protocol/sybilprotection/seatmanager/topstakers/topstakers_test.go @@ -35,11 +35,13 @@ func TestTopStakers_InitializeCommittee(t *testing.T) { // Create committee for epoch 0 initialCommittee := account.NewAccounts() for i := 0; i < 3; i++ { - initialCommittee.Set(tpkg.RandAccountID(), &account.Pool{ + if err := initialCommittee.Set(tpkg.RandAccountID(), &account.Pool{ PoolStake: 1900, ValidatorStake: 900, FixedCost: 11, - }) + }); err != nil { + t.Fatal(err) + } } // Try setting committee that is too small - should return an error. err := topStakersSeatManager.SetCommittee(0, initialCommittee) @@ -53,7 +55,6 @@ func TestTopStakers_InitializeCommittee(t *testing.T) { require.NoError(t, topStakersSeatManager.InitializeCommittee(0, time.Time{})) assertOnlineCommittee(t, topStakersSeatManager.OnlineCommittee(), lo.Return1(weightedSeats.GetSeat(initialCommitteeAccountIDs[0])), lo.Return1(weightedSeats.GetSeat(initialCommitteeAccountIDs[2])), lo.Return1(weightedSeats.GetSeat(initialCommitteeAccountIDs[2]))) - } func TestTopStakers_RotateCommittee(t *testing.T) { @@ -77,27 +78,27 @@ func TestTopStakers_RotateCommittee(t *testing.T) { // Create committee for epoch 0 initialCommittee := account.NewAccounts() - initialCommittee.Set(tpkg.RandAccountID(), &account.Pool{ + require.NoError(t, initialCommittee.Set(tpkg.RandAccountID(), &account.Pool{ PoolStake: 1900, ValidatorStake: 900, FixedCost: 11, - }) + })) - initialCommittee.Set(tpkg.RandAccountID(), &account.Pool{ + require.NoError(t, initialCommittee.Set(tpkg.RandAccountID(), &account.Pool{ PoolStake: 1900, ValidatorStake: 900, FixedCost: 11, - }) + })) // Try setting committee that is too small - should return an error. err := topStakersSeatManager.SetCommittee(0, initialCommittee) require.Error(t, err) - initialCommittee.Set(tpkg.RandAccountID(), &account.Pool{ + require.NoError(t, initialCommittee.Set(tpkg.RandAccountID(), &account.Pool{ PoolStake: 1900, ValidatorStake: 900, FixedCost: 11, - }) + })) // Set committee with the correct size err = topStakersSeatManager.SetCommittee(0, initialCommittee) @@ -187,7 +188,9 @@ func TestTopStakers_RotateCommittee(t *testing.T) { // Make sure that after committee rotation, the online committee is not changed. assertOnlineCommittee(t, topStakersSeatManager.OnlineCommittee(), lo.Return1(weightedSeats.GetSeat(initialCommitteeAccountIDs[2]))) - newCommitteeMemberIDs := newCommittee.Accounts().IDs() + accounts, err := newCommittee.Accounts() + require.NoError(t, err) + newCommitteeMemberIDs := accounts.IDs() // A new committee member appears online and makes the previously active committee seat inactive. topStakersSeatManager.activityTracker.MarkSeatActive(lo.Return1(weightedSeats.GetSeat(newCommitteeMemberIDs[0])), newCommitteeMemberIDs[0], tpkg.TestAPI.TimeProvider().SlotEndTime(14)) @@ -213,9 +216,11 @@ func TestTopStakers_RotateCommittee(t *testing.T) { } func assertCommittee(t *testing.T, expectedCommittee *account.Accounts, actualCommittee *account.SeatedAccounts) { - require.Equal(t, actualCommittee.Accounts().Size(), expectedCommittee.Size()) + actualAccounts, err := actualCommittee.Accounts() + require.NoError(t, err) + require.Equal(t, actualAccounts.Size(), expectedCommittee.Size()) for _, memberID := range expectedCommittee.IDs() { - require.Truef(t, actualCommittee.Accounts().Has(memberID), "expected account %s to be part of committee, but it is not, actual committee members: %s", memberID, actualCommittee.Accounts().IDs()) + require.Truef(t, actualAccounts.Has(memberID), "expected account %s to be part of committee, but it is not, actual committee members: %s", memberID, actualAccounts.IDs()) } } diff --git a/pkg/protocol/sybilprotection/sybilprotectionv1/performance/testsuite_test.go b/pkg/protocol/sybilprotection/sybilprotectionv1/performance/testsuite_test.go index 684e21916..62c5c1e48 100644 --- a/pkg/protocol/sybilprotection/sybilprotectionv1/performance/testsuite_test.go +++ b/pkg/protocol/sybilprotection/sybilprotectionv1/performance/testsuite_test.go @@ -106,11 +106,13 @@ func (t *TestSuite) ApplyEpochActions(epoch iotago.EpochIndex, actions map[strin action.validate(t.T, t.api) accountID := t.Account(alias, true) - committee.Set(accountID, &account.Pool{ + if err := committee.Set(accountID, &account.Pool{ PoolStake: action.PoolStake, ValidatorStake: action.ValidatorStake, FixedCost: action.FixedCost, - }) + }); err != nil { + t.T.Fatal(err) + } } // Store directly on the committee store, because in actual code the SeatManager is responsible for adding the storage entry. diff --git a/pkg/protocol/sybilprotection/sybilprotectionv1/sybilprotection.go b/pkg/protocol/sybilprotection/sybilprotectionv1/sybilprotection.go index 2ebd75b0c..6389a2608 100644 --- a/pkg/protocol/sybilprotection/sybilprotectionv1/sybilprotection.go +++ b/pkg/protocol/sybilprotection/sybilprotectionv1/sybilprotection.go @@ -161,7 +161,11 @@ func (o *SybilProtection) CommitSlot(slot iotago.SlotIndex) (committeeRoot, rewa panic(fmt.Sprintf("committee for current epoch %d not found", currentEpoch)) } - committeeAccounts := committee.Accounts() + committeeAccounts, err := committee.Accounts() + if err != nil { + return iotago.Identifier{}, iotago.Identifier{}, ierrors.Wrapf(err, "failed to get accounts from committee for epoch %d", currentEpoch) + } + committeeAccounts.SetReused() if err = o.seatManager.SetCommittee(nextEpoch, committeeAccounts); err != nil { return iotago.Identifier{}, iotago.Identifier{}, ierrors.Wrapf(err, "failed to set committee for epoch %d", nextEpoch) @@ -276,7 +280,13 @@ func (o *SybilProtection) slotFinalized(slot iotago.SlotIndex) { epochEndSlot := timeProvider.EpochEnd(epoch) if slot+apiForSlot.ProtocolParameters().EpochNearingThreshold() == epochEndSlot && epochEndSlot > o.lastCommittedSlot+apiForSlot.ProtocolParameters().MaxCommittableAge() { - newCommittee := o.selectNewCommittee(slot) + newCommittee, err := o.selectNewCommittee(slot) + if err != nil { + // TODO: should we fail "harder" here? + o.errHandler(ierrors.Wrap(err, "error while selecting new committee")) + + return + } o.events.CommitteeSelected.Trigger(newCommittee, epoch+1) } } @@ -371,7 +381,7 @@ func (o *SybilProtection) OrderedRegisteredCandidateValidatorsList(epoch iotago. return validatorResp, nil } -func (o *SybilProtection) selectNewCommittee(slot iotago.SlotIndex) *account.Accounts { +func (o *SybilProtection) selectNewCommittee(slot iotago.SlotIndex) (*account.Accounts, error) { timeProvider := o.apiProvider.APIForSlot(slot).TimeProvider() currentEpoch := timeProvider.EpochFromSlot(slot) nextEpoch := currentEpoch + 1 diff --git a/pkg/storage/testframework_test.go b/pkg/storage/testframework_test.go index 290f180dd..847cc0ef4 100644 --- a/pkg/storage/testframework_test.go +++ b/pkg/storage/testframework_test.go @@ -152,7 +152,9 @@ func (t *TestFramework) GenerateSemiPermanentData(epoch iotago.EpochIndex) { createdBytes += int64(len(lo.PanicOnErr(versionAndHash.Bytes()))) + 8 // for epoch key accounts := account.NewAccounts() - accounts.Set(tpkg.RandAccountID(), &account.Pool{}) + if err := accounts.Set(tpkg.RandAccountID(), &account.Pool{}); err != nil { + t.t.Fatal(err) + } err = committeeStore.Store(epoch, accounts) require.NoError(t.t, err) createdBytes += int64(len(lo.PanicOnErr(accounts.Bytes()))) + 8 // for epoch key diff --git a/pkg/testsuite/sybilprotection.go b/pkg/testsuite/sybilprotection.go index 3eac2c524..39673cb29 100644 --- a/pkg/testsuite/sybilprotection.go +++ b/pkg/testsuite/sybilprotection.go @@ -17,13 +17,17 @@ func (t *TestSuite) AssertSybilProtectionCommittee(epoch iotago.EpochIndex, expe for _, node := range nodes { t.Eventually(func() error { - accounts := lo.Return1(node.Protocol.MainEngineInstance().SybilProtection.SeatManager().CommitteeInEpoch(epoch)).Accounts().IDs() - if !assert.ElementsMatch(t.fakeTesting, expectedAccounts, accounts) { - return ierrors.Errorf("AssertSybilProtectionCommittee: %s: expected %s, got %s", node.Name, expectedAccounts, accounts) + accounts, err := lo.Return1(node.Protocol.MainEngineInstance().SybilProtection.SeatManager().CommitteeInEpoch(epoch)).Accounts() + if err != nil { + t.Testing.Fatal(err) + } + accountIDs := accounts.IDs() + if !assert.ElementsMatch(t.fakeTesting, expectedAccounts, accountIDs) { + return ierrors.Errorf("AssertSybilProtectionCommittee: %s: expected %s, got %s", node.Name, expectedAccounts, accountIDs) } - if len(expectedAccounts) != len(accounts) { - return ierrors.Errorf("AssertSybilProtectionCommittee: %s: expected %v, got %v", node.Name, len(expectedAccounts), len(accounts)) + if len(expectedAccounts) != len(accountIDs) { + return ierrors.Errorf("AssertSybilProtectionCommittee: %s: expected %v, got %v", node.Name, len(expectedAccounts), len(accountIDs)) } return nil From cf40c206d35267aef097f341695c8700e63d1ec8 Mon Sep 17 00:00:00 2001 From: Piotr Macek <4007944+piotrm50@users.noreply.github.com> Date: Thu, 2 Nov 2023 14:54:53 +0100 Subject: [PATCH 2/7] Generate a snapshot with current timestamp. --- tools/genesis-snapshot/presets/presets.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/genesis-snapshot/presets/presets.go b/tools/genesis-snapshot/presets/presets.go index 29babde9f..315aebac9 100644 --- a/tools/genesis-snapshot/presets/presets.go +++ b/tools/genesis-snapshot/presets/presets.go @@ -244,7 +244,7 @@ var Feature = []options.Option[snapshotcreator.Options]{ iotago.NewV3ProtocolParameters( iotago.WithNetworkOptions("feature", "rms"), iotago.WithSupplyOptions(4_600_000_000_000_000, 100, 1, 10, 100, 100, 100), - iotago.WithTimeProviderOptions(1697631694, 10, 13), + iotago.WithTimeProviderOptions(time.Now().Unix(), 10, 13), iotago.WithLivenessOptions(30, 30, 10, 20, 30), // increase/decrease threshold = fraction * slotDurationInSeconds * schedulerRate iotago.WithCongestionControlOptions(500, 500, 500, 800000, 500000, 100000, 1000, 100), From 763e7681b6790f93fe9b841c553adb9582318e86 Mon Sep 17 00:00:00 2001 From: Piotr Macek <4007944+piotrm50@users.noreply.github.com> Date: Thu, 2 Nov 2023 14:56:13 +0100 Subject: [PATCH 3/7] Fix node shutdown. --- components/metrics/collector/metric.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/metrics/collector/metric.go b/components/metrics/collector/metric.go index 4ea8e0adc..1696b8d80 100644 --- a/components/metrics/collector/metric.go +++ b/components/metrics/collector/metric.go @@ -198,7 +198,7 @@ func (m *Metric) schedulePruning(labelValues []string) { func (m *Metric) shutdown() { if m.pruningExecutor != nil { - m.pruningExecutor.Shutdown() + m.pruningExecutor.Shutdown(timed.CancelPendingElements) } } From f56c9d1f1c82daa3242183ea23abde1841ccefd6 Mon Sep 17 00:00:00 2001 From: Piotr Macek <4007944+piotrm50@users.noreply.github.com> Date: Thu, 2 Nov 2023 14:57:03 +0100 Subject: [PATCH 4/7] Cleanup run.sh script --- tools/docker-network/run.sh | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/docker-network/run.sh b/tools/docker-network/run.sh index 6cac8241e..a47388ec1 100755 --- a/tools/docker-network/run.sh +++ b/tools/docker-network/run.sh @@ -1,17 +1,16 @@ #!/bin/bash -set -e + # Create a function to join an array of strings by a given character function join { local IFS="$1"; shift; echo "$*"; } # All parameters can be optional now, just make sure we don't have too many if [[ $# -gt 4 ]] ; then - echo 'Call with ./run [replicas=1|2|3|...] [monitoring=0|1] [feature=0|1]' + echo 'Call with ./run [monitoring=0|1]' exit 0 fi -REPLICAS=${1:-1} -MONITORING=${2:-0} +MONITORING=${1:-0} export DOCKER_BUILDKIT=1 export COMPOSE_DOCKER_CLI_BUILD=1 From d9c93abf7e78896faa44ec41ec8680b3ada86c08 Mon Sep 17 00:00:00 2001 From: Piotr Macek <4007944+piotrm50@users.noreply.github.com> Date: Thu, 2 Nov 2023 15:13:12 +0100 Subject: [PATCH 5/7] Fix run.sh after review. --- tools/docker-network/run.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/docker-network/run.sh b/tools/docker-network/run.sh index a47388ec1..323a6bdbb 100755 --- a/tools/docker-network/run.sh +++ b/tools/docker-network/run.sh @@ -5,8 +5,8 @@ function join { local IFS="$1"; shift; echo "$*"; } # All parameters can be optional now, just make sure we don't have too many -if [[ $# -gt 4 ]] ; then - echo 'Call with ./run [monitoring=0|1]' +if [[ $# -gt 2 ]] ; then + echo 'Call with ./run.sh [monitoring=0|1]' exit 0 fi From 6937f94cde18819fbbf2cad358d228e840cca191 Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Thu, 2 Nov 2023 15:33:07 +0100 Subject: [PATCH 6/7] Panic when failing to select new committee --- .../sybilprotectionv1/sybilprotection.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pkg/protocol/sybilprotection/sybilprotectionv1/sybilprotection.go b/pkg/protocol/sybilprotection/sybilprotectionv1/sybilprotection.go index 6389a2608..fd3ff4c70 100644 --- a/pkg/protocol/sybilprotection/sybilprotectionv1/sybilprotection.go +++ b/pkg/protocol/sybilprotection/sybilprotectionv1/sybilprotection.go @@ -282,10 +282,7 @@ func (o *SybilProtection) slotFinalized(slot iotago.SlotIndex) { epochEndSlot > o.lastCommittedSlot+apiForSlot.ProtocolParameters().MaxCommittableAge() { newCommittee, err := o.selectNewCommittee(slot) if err != nil { - // TODO: should we fail "harder" here? - o.errHandler(ierrors.Wrap(err, "error while selecting new committee")) - - return + panic(ierrors.Wrap(err, "error while selecting new committee")) } o.events.CommitteeSelected.Trigger(newCommittee, epoch+1) } @@ -387,7 +384,7 @@ func (o *SybilProtection) selectNewCommittee(slot iotago.SlotIndex) (*account.Ac nextEpoch := currentEpoch + 1 candidates, err := o.performanceTracker.EligibleValidatorCandidates(nextEpoch) if err != nil { - panic(ierrors.Wrapf(err, "failed to retrieve candidates for epoch %d", nextEpoch)) + return nil, ierrors.Wrapf(err, "failed to retrieve candidates for epoch %d", nextEpoch) } candidateAccounts := make(accounts.AccountsData, 0) @@ -404,12 +401,12 @@ func (o *SybilProtection) selectNewCommittee(slot iotago.SlotIndex) (*account.Ac return nil }); err != nil { - panic(ierrors.Wrap(err, "failed to iterate through candidates")) + return nil, ierrors.Wrap(err, "failed to iterate through candidates") } newCommittee, err := o.seatManager.RotateCommittee(nextEpoch, candidateAccounts) if err != nil { - panic(ierrors.Wrap(err, "failed to rotate committee")) + return nil, ierrors.Wrap(err, "failed to rotate committee") } o.performanceTracker.ClearCandidates() From 8226e8a6536c58e22e387eb4b6c9ab875d715481 Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Fri, 3 Nov 2023 10:34:36 +0100 Subject: [PATCH 7/7] Configure default genesis seed and fix fatal error --- tools/genesis-snapshot/main.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tools/genesis-snapshot/main.go b/tools/genesis-snapshot/main.go index 63d303a83..05728937e 100644 --- a/tools/genesis-snapshot/main.go +++ b/tools/genesis-snapshot/main.go @@ -37,7 +37,7 @@ func main() { func parseFlags() (opt []options.Option[snapshotcreator.Options], conf string) { filename := flag.String("filename", "", "the name of the generated snapshot file") config := flag.String("config", "", "use ready config: devnet, feature, docker") - genesisSeedStr := flag.String("seed", "", "the genesis seed provided in base58 format.") + genesisSeedStr := flag.String("seed", "7R1itJx5hVuo9w9hjg5cwKFmek4HMSoBDgJZN8hKGxih", "the genesis seed provided in base58 format.") flag.Parse() opt = []options.Option[snapshotcreator.Options]{} @@ -45,14 +45,12 @@ func parseFlags() (opt []options.Option[snapshotcreator.Options], conf string) { opt = append(opt, snapshotcreator.WithFilePath(*filename)) } - if *genesisSeedStr != "" { - genesisSeed, err := base58.Decode(*genesisSeedStr) - if err != nil { - log.Fatal(ierrors.Errorf("failed to decode base58 seed, using the default one: %w", err)) - } - keyManager := mock.NewKeyManager(genesisSeed[:], 0) - opt = append(opt, snapshotcreator.WithGenesisKeyManager(keyManager)) + genesisSeed, err := base58.Decode(*genesisSeedStr) + if err != nil { + log.Fatal(ierrors.Wrap(err, "failed to decode base58 seed")) } + keyManager := mock.NewKeyManager(genesisSeed[:], 0) + opt = append(opt, snapshotcreator.WithGenesisKeyManager(keyManager)) return opt, *config }