diff --git a/vms/platformvm/block/builder/builder_test.go b/vms/platformvm/block/builder/builder_test.go index e691c6f6ce2..1a056fb4b90 100644 --- a/vms/platformvm/block/builder/builder_test.go +++ b/vms/platformvm/block/builder/builder_test.go @@ -221,10 +221,10 @@ func TestBuildBlockAdvanceTime(t *testing.T) { ) // Add a staker to [env.state] - env.state.PutCurrentValidator(&state.Staker{ + require.NoError(env.state.PutCurrentValidator(&state.Staker{ NextTime: nextTime, Priority: txs.PrimaryNetworkValidatorCurrentPriority, - }) + })) // Advance wall clock to [nextTime] env.backend.Clk.Set(nextTime) @@ -278,10 +278,10 @@ func TestBuildBlockForceAdvanceTime(t *testing.T) { ) // Add a staker to [env.state] - env.state.PutCurrentValidator(&state.Staker{ + require.NoError(env.state.PutCurrentValidator(&state.Staker{ NextTime: nextTime, Priority: txs.PrimaryNetworkValidatorCurrentPriority, - }) + })) // Advance wall clock to [nextTime] + [txexecutor.SyncBound] env.backend.Clk.Set(nextTime.Add(txexecutor.SyncBound)) diff --git a/vms/platformvm/block/executor/helpers_test.go b/vms/platformvm/block/executor/helpers_test.go index f066b1fc13c..b11e4714c88 100644 --- a/vms/platformvm/block/executor/helpers_test.go +++ b/vms/platformvm/block/executor/helpers_test.go @@ -378,7 +378,7 @@ func addPendingValidator( ) require.NoError(err) - env.state.PutPendingValidator(staker) + require.NoError(env.state.PutPendingValidator(staker)) env.state.AddTx(addValidatorTx, status.Committed) env.state.SetHeight(1) require.NoError(env.state.Commit()) diff --git a/vms/platformvm/block/executor/proposal_block_test.go b/vms/platformvm/block/executor/proposal_block_test.go index 397802cf0fa..a1c5151044d 100644 --- a/vms/platformvm/block/executor/proposal_block_test.go +++ b/vms/platformvm/block/executor/proposal_block_test.go @@ -543,7 +543,7 @@ func TestBanffProposalBlockUpdateStakers(t *testing.T) { ) require.NoError(err) - env.state.PutPendingValidator(staker) + require.NoError(env.state.PutPendingValidator(staker)) env.state.AddTx(tx, status.Committed) require.NoError(env.state.Commit()) } @@ -572,7 +572,7 @@ func TestBanffProposalBlockUpdateStakers(t *testing.T) { ) require.NoError(err) - env.state.PutPendingValidator(subnetStaker) + require.NoError(env.state.PutPendingValidator(subnetStaker)) env.state.AddTx(tx, status.Committed) require.NoError(env.state.Commit()) } @@ -611,7 +611,7 @@ func TestBanffProposalBlockUpdateStakers(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker0) + require.NoError(env.state.PutCurrentValidator(staker0)) env.state.AddTx(addStaker0, status.Committed) require.NoError(env.state.Commit()) @@ -714,7 +714,7 @@ func TestBanffProposalBlockRemoveSubnetValidator(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(tx, status.Committed) require.NoError(env.state.Commit()) @@ -741,7 +741,7 @@ func TestBanffProposalBlockRemoveSubnetValidator(t *testing.T) { ) require.NoError(err) - env.state.PutPendingValidator(staker) + require.NoError(env.state.PutPendingValidator(staker)) env.state.AddTx(tx, status.Committed) require.NoError(env.state.Commit()) @@ -782,7 +782,7 @@ func TestBanffProposalBlockRemoveSubnetValidator(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(addStaker0, status.Committed) require.NoError(env.state.Commit()) @@ -867,7 +867,7 @@ func TestBanffProposalBlockTrackedSubnet(t *testing.T) { ) require.NoError(err) - env.state.PutPendingValidator(staker) + require.NoError(env.state.PutPendingValidator(staker)) env.state.AddTx(tx, status.Committed) require.NoError(env.state.Commit()) @@ -904,7 +904,7 @@ func TestBanffProposalBlockTrackedSubnet(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(addStaker0, status.Committed) require.NoError(env.state.Commit()) @@ -996,7 +996,7 @@ func TestBanffProposalBlockDelegatorStakerWeight(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(addStaker0, status.Committed) require.NoError(env.state.Commit()) @@ -1085,7 +1085,7 @@ func TestBanffProposalBlockDelegatorStakerWeight(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(addStaker0, status.Committed) require.NoError(env.state.Commit()) @@ -1181,7 +1181,7 @@ func TestBanffProposalBlockDelegatorStakers(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(addStaker0, status.Committed) require.NoError(env.state.Commit()) @@ -1270,7 +1270,7 @@ func TestBanffProposalBlockDelegatorStakers(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(addStaker0, status.Committed) require.NoError(env.state.Commit()) diff --git a/vms/platformvm/block/executor/standard_block_test.go b/vms/platformvm/block/executor/standard_block_test.go index 15cc43cdad9..7c31ccb7f25 100644 --- a/vms/platformvm/block/executor/standard_block_test.go +++ b/vms/platformvm/block/executor/standard_block_test.go @@ -534,7 +534,7 @@ func TestBanffStandardBlockUpdateStakers(t *testing.T) { ) require.NoError(err) - env.state.PutPendingValidator(staker) + require.NoError(env.state.PutPendingValidator(staker)) env.state.AddTx(tx, status.Committed) } env.state.SetHeight( /*dummyHeight*/ 1) @@ -631,7 +631,7 @@ func TestBanffStandardBlockRemoveSubnetValidator(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(tx, status.Committed) require.NoError(env.state.Commit()) @@ -658,7 +658,7 @@ func TestBanffStandardBlockRemoveSubnetValidator(t *testing.T) { ) require.NoError(err) - env.state.PutPendingValidator(staker) + require.NoError(env.state.PutPendingValidator(staker)) env.state.AddTx(tx, status.Committed) require.NoError(env.state.Commit()) @@ -733,7 +733,7 @@ func TestBanffStandardBlockTrackedSubnet(t *testing.T) { ) require.NoError(err) - env.state.PutPendingValidator(staker) + require.NoError(env.state.PutPendingValidator(staker)) env.state.AddTx(tx, status.Committed) require.NoError(env.state.Commit()) diff --git a/vms/platformvm/service_test.go b/vms/platformvm/service_test.go index 7b3252c140e..9454de53455 100644 --- a/vms/platformvm/service_test.go +++ b/vms/platformvm/service_test.go @@ -582,7 +582,7 @@ func TestGetStake(t *testing.T) { ) require.NoError(err) - service.vm.state.PutPendingValidator(staker) + require.NoError(service.vm.state.PutPendingValidator(staker)) service.vm.state.AddTx(tx, status.Committed) require.NoError(service.vm.state.Commit()) diff --git a/vms/platformvm/state/diff.go b/vms/platformvm/state/diff.go index 66477823673..16f7edf4435 100644 --- a/vms/platformvm/state/diff.go +++ b/vms/platformvm/state/diff.go @@ -180,8 +180,8 @@ func (d *diff) GetDelegateeReward(subnetID ids.ID, nodeID ids.NodeID) (uint64, e return parentState.GetDelegateeReward(subnetID, nodeID) } -func (d *diff) PutCurrentValidator(staker *Staker) { - d.currentStakerDiffs.PutValidator(staker) +func (d *diff) PutCurrentValidator(staker *Staker) error { + return d.currentStakerDiffs.PutValidator(staker) } func (d *diff) DeleteCurrentValidator(staker *Staker) { @@ -243,8 +243,8 @@ func (d *diff) GetPendingValidator(subnetID ids.ID, nodeID ids.NodeID) (*Staker, } } -func (d *diff) PutPendingValidator(staker *Staker) { - d.pendingStakerDiffs.PutValidator(staker) +func (d *diff) PutPendingValidator(staker *Staker) error { + return d.pendingStakerDiffs.PutValidator(staker) } func (d *diff) DeletePendingValidator(staker *Staker) { @@ -444,7 +444,9 @@ func (d *diff) Apply(baseState Chain) error { for _, validatorDiff := range subnetValidatorDiffs { switch validatorDiff.validatorStatus { case added: - baseState.PutCurrentValidator(validatorDiff.validator) + if err := baseState.PutCurrentValidator(validatorDiff.validator); err != nil { + return err + } case deleted: baseState.DeleteCurrentValidator(validatorDiff.validator) } @@ -471,7 +473,9 @@ func (d *diff) Apply(baseState Chain) error { for _, validatorDiff := range subnetValidatorDiffs { switch validatorDiff.validatorStatus { case added: - baseState.PutPendingValidator(validatorDiff.validator) + if err := baseState.PutPendingValidator(validatorDiff.validator); err != nil { + return err + } case deleted: baseState.DeletePendingValidator(validatorDiff.validator) } diff --git a/vms/platformvm/state/diff_test.go b/vms/platformvm/state/diff_test.go index 82525e08759..56ff3bc8ac8 100644 --- a/vms/platformvm/state/diff_test.go +++ b/vms/platformvm/state/diff_test.go @@ -111,7 +111,7 @@ func TestDiffCurrentValidator(t *testing.T) { SubnetID: ids.GenerateTestID(), NodeID: ids.GenerateTestNodeID(), } - d.PutCurrentValidator(currentValidator) + require.NoError(d.PutCurrentValidator(currentValidator)) // Assert that we get the current validator back gotCurrentValidator, err := d.GetCurrentValidator(currentValidator.SubnetID, currentValidator.NodeID) @@ -145,7 +145,7 @@ func TestDiffPendingValidator(t *testing.T) { SubnetID: ids.GenerateTestID(), NodeID: ids.GenerateTestNodeID(), } - d.PutPendingValidator(pendingValidator) + require.NoError(d.PutPendingValidator(pendingValidator)) // Assert that we get the pending validator back gotPendingValidator, err := d.GetPendingValidator(pendingValidator.SubnetID, pendingValidator.NodeID) diff --git a/vms/platformvm/state/mock_chain.go b/vms/platformvm/state/mock_chain.go index 13adc2d2cc8..727847a7c07 100644 --- a/vms/platformvm/state/mock_chain.go +++ b/vms/platformvm/state/mock_chain.go @@ -416,9 +416,11 @@ func (mr *MockChainMockRecorder) PutCurrentDelegator(staker any) *gomock.Call { } // PutCurrentValidator mocks base method. -func (m *MockChain) PutCurrentValidator(staker *Staker) { +func (m *MockChain) PutCurrentValidator(staker *Staker) error { m.ctrl.T.Helper() - m.ctrl.Call(m, "PutCurrentValidator", staker) + ret := m.ctrl.Call(m, "PutCurrentValidator", staker) + ret0, _ := ret[0].(error) + return ret0 } // PutCurrentValidator indicates an expected call of PutCurrentValidator. @@ -440,9 +442,11 @@ func (mr *MockChainMockRecorder) PutPendingDelegator(staker any) *gomock.Call { } // PutPendingValidator mocks base method. -func (m *MockChain) PutPendingValidator(staker *Staker) { +func (m *MockChain) PutPendingValidator(staker *Staker) error { m.ctrl.T.Helper() - m.ctrl.Call(m, "PutPendingValidator", staker) + ret := m.ctrl.Call(m, "PutPendingValidator", staker) + ret0, _ := ret[0].(error) + return ret0 } // PutPendingValidator indicates an expected call of PutPendingValidator. diff --git a/vms/platformvm/state/mock_diff.go b/vms/platformvm/state/mock_diff.go index 8bfd09c44d9..ccf6619b5b2 100644 --- a/vms/platformvm/state/mock_diff.go +++ b/vms/platformvm/state/mock_diff.go @@ -430,9 +430,11 @@ func (mr *MockDiffMockRecorder) PutCurrentDelegator(staker any) *gomock.Call { } // PutCurrentValidator mocks base method. -func (m *MockDiff) PutCurrentValidator(staker *Staker) { +func (m *MockDiff) PutCurrentValidator(staker *Staker) error { m.ctrl.T.Helper() - m.ctrl.Call(m, "PutCurrentValidator", staker) + ret := m.ctrl.Call(m, "PutCurrentValidator", staker) + ret0, _ := ret[0].(error) + return ret0 } // PutCurrentValidator indicates an expected call of PutCurrentValidator. @@ -454,9 +456,11 @@ func (mr *MockDiffMockRecorder) PutPendingDelegator(staker any) *gomock.Call { } // PutPendingValidator mocks base method. -func (m *MockDiff) PutPendingValidator(staker *Staker) { +func (m *MockDiff) PutPendingValidator(staker *Staker) error { m.ctrl.T.Helper() - m.ctrl.Call(m, "PutPendingValidator", staker) + ret := m.ctrl.Call(m, "PutPendingValidator", staker) + ret0, _ := ret[0].(error) + return ret0 } // PutPendingValidator indicates an expected call of PutPendingValidator. diff --git a/vms/platformvm/state/mock_state.go b/vms/platformvm/state/mock_state.go index 9bc07c2333f..527db5cf8a5 100644 --- a/vms/platformvm/state/mock_state.go +++ b/vms/platformvm/state/mock_state.go @@ -651,9 +651,11 @@ func (mr *MockStateMockRecorder) PutCurrentDelegator(staker any) *gomock.Call { } // PutCurrentValidator mocks base method. -func (m *MockState) PutCurrentValidator(staker *Staker) { +func (m *MockState) PutCurrentValidator(staker *Staker) error { m.ctrl.T.Helper() - m.ctrl.Call(m, "PutCurrentValidator", staker) + ret := m.ctrl.Call(m, "PutCurrentValidator", staker) + ret0, _ := ret[0].(error) + return ret0 } // PutCurrentValidator indicates an expected call of PutCurrentValidator. @@ -675,9 +677,11 @@ func (mr *MockStateMockRecorder) PutPendingDelegator(staker any) *gomock.Call { } // PutPendingValidator mocks base method. -func (m *MockState) PutPendingValidator(staker *Staker) { +func (m *MockState) PutPendingValidator(staker *Staker) error { m.ctrl.T.Helper() - m.ctrl.Call(m, "PutPendingValidator", staker) + ret := m.ctrl.Call(m, "PutPendingValidator", staker) + ret0, _ := ret[0].(error) + return ret0 } // PutPendingValidator indicates an expected call of PutPendingValidator. diff --git a/vms/platformvm/state/stakers.go b/vms/platformvm/state/stakers.go index c39d84d48d0..65879685595 100644 --- a/vms/platformvm/state/stakers.go +++ b/vms/platformvm/state/stakers.go @@ -4,6 +4,8 @@ package state import ( + "errors" + "github.com/google/btree" "github.com/ava-labs/avalanchego/database" @@ -11,6 +13,8 @@ import ( "github.com/ava-labs/avalanchego/utils/iterator" ) +var ErrAddingStakerAfterDeletion = errors.New("attempted to add a staker after deleting it") + type Stakers interface { CurrentStakers PendingStakers @@ -26,7 +30,7 @@ type CurrentStakers interface { // staker set. // // Invariant: [staker] is not currently a CurrentValidator - PutCurrentValidator(staker *Staker) + PutCurrentValidator(staker *Staker) error // DeleteCurrentValidator removes the [staker] describing a validator from // the staker set. @@ -72,7 +76,7 @@ type PendingStakers interface { // PutPendingValidator adds the [staker] describing a validator to the // staker set. - PutPendingValidator(staker *Staker) + PutPendingValidator(staker *Staker) error // DeletePendingValidator removes the [staker] describing a validator from // the staker set. @@ -289,8 +293,14 @@ func (s *diffStakers) GetValidator(subnetID ids.ID, nodeID ids.NodeID) (*Staker, return nil, validatorDiff.validatorStatus } -func (s *diffStakers) PutValidator(staker *Staker) { +func (s *diffStakers) PutValidator(staker *Staker) error { validatorDiff := s.getOrCreateDiff(staker.SubnetID, staker.NodeID) + if validatorDiff.validatorStatus == deleted { + // Enforce the invariant that a validator cannot be added after being + // deleted. + return ErrAddingStakerAfterDeletion + } + validatorDiff.validatorStatus = added validatorDiff.validator = staker @@ -298,6 +308,7 @@ func (s *diffStakers) PutValidator(staker *Staker) { s.addedStakers = btree.NewG(defaultTreeDegree, (*Staker).Less) } s.addedStakers.ReplaceOrInsert(staker) + return nil } func (s *diffStakers) DeleteValidator(staker *Staker) { diff --git a/vms/platformvm/state/stakers_test.go b/vms/platformvm/state/stakers_test.go index e4ae2aa840c..d536b2a719d 100644 --- a/vms/platformvm/state/stakers_test.go +++ b/vms/platformvm/state/stakers_test.go @@ -162,7 +162,7 @@ func TestDiffStakersValidator(t *testing.T) { stakerIterator := v.GetStakerIterator(iterator.Empty[*Staker]{}) assertIteratorsEqual(t, iterator.FromSlice(delegator), stakerIterator) - v.PutValidator(staker) + require.NoError(v.PutValidator(staker)) returnedStaker, status := v.GetValidator(staker.SubnetID, staker.NodeID) require.Equal(added, status) @@ -203,7 +203,7 @@ func TestDiffStakersDelegator(t *testing.T) { v := diffStakers{} - v.PutValidator(staker) + require.NoError(t, v.PutValidator(staker)) delegatorIterator := v.GetDelegatorIterator(iterator.Empty[*Staker]{}, ids.GenerateTestID(), delegator.NodeID) assertIteratorsEqual(t, iterator.Empty[*Staker]{}, delegatorIterator) diff --git a/vms/platformvm/state/state.go b/vms/platformvm/state/state.go index 17a5e83bd28..b5288df7514 100644 --- a/vms/platformvm/state/state.go +++ b/vms/platformvm/state/state.go @@ -682,8 +682,9 @@ func (s *state) GetCurrentValidator(subnetID ids.ID, nodeID ids.NodeID) (*Staker return s.currentStakers.GetValidator(subnetID, nodeID) } -func (s *state) PutCurrentValidator(staker *Staker) { +func (s *state) PutCurrentValidator(staker *Staker) error { s.currentStakers.PutValidator(staker) + return nil } func (s *state) DeleteCurrentValidator(staker *Staker) { @@ -710,8 +711,9 @@ func (s *state) GetPendingValidator(subnetID ids.ID, nodeID ids.NodeID) (*Staker return s.pendingStakers.GetValidator(subnetID, nodeID) } -func (s *state) PutPendingValidator(staker *Staker) { +func (s *state) PutPendingValidator(staker *Staker) error { s.pendingStakers.PutValidator(staker) + return nil } func (s *state) DeletePendingValidator(staker *Staker) { @@ -1287,7 +1289,9 @@ func (s *state) syncGenesis(genesisBlk block.Block, genesis *genesis.Genesis) er return err } - s.PutCurrentValidator(staker) + if err := s.PutCurrentValidator(staker); err != nil { + return err + } s.AddTx(vdrTx, status.Committed) s.SetCurrentSupply(constants.PrimaryNetworkID, newCurrentSupply) } diff --git a/vms/platformvm/state/state_test.go b/vms/platformvm/state/state_test.go index de2bab544c0..08b53eeb4fe 100644 --- a/vms/platformvm/state/state_test.go +++ b/vms/platformvm/state/state_test.go @@ -148,7 +148,7 @@ func TestPersistStakers(t *testing.T) { ) r.NoError(err) - s.PutCurrentValidator(staker) + r.NoError(s.PutCurrentValidator(staker)) s.AddTx(addPermValTx, status.Committed) // this is currently needed to reload the staker r.NoError(s.Commit()) return staker @@ -243,7 +243,7 @@ func TestPersistStakers(t *testing.T) { ) r.NoError(err) - s.PutCurrentValidator(val) + r.NoError(s.PutCurrentValidator(val)) s.AddTx(addPermValTx, status.Committed) // this is currently needed to reload the staker r.NoError(s.Commit()) @@ -308,7 +308,7 @@ func TestPersistStakers(t *testing.T) { ) r.NoError(err) - s.PutPendingValidator(staker) + r.NoError(s.PutPendingValidator(staker)) s.AddTx(addPermValTx, status.Committed) // this is currently needed to reload the staker r.NoError(s.Commit()) return staker @@ -375,7 +375,7 @@ func TestPersistStakers(t *testing.T) { del, err := NewPendingStaker(addPermDelTx.ID(), utxDel) r.NoError(err) - s.PutPendingValidator(val) + r.NoError(s.PutPendingValidator(val)) s.AddTx(addPermValTx, status.Committed) // this is currently needed to reload the staker r.NoError(s.Commit()) @@ -428,7 +428,7 @@ func TestPersistStakers(t *testing.T) { ) r.NoError(err) - s.PutCurrentValidator(staker) + r.NoError(s.PutCurrentValidator(staker)) s.AddTx(addPermValTx, status.Committed) // this is currently needed to reload the staker r.NoError(s.Commit()) @@ -517,7 +517,7 @@ func TestPersistStakers(t *testing.T) { ) r.NoError(err) - s.PutCurrentValidator(val) + r.NoError(s.PutCurrentValidator(val)) s.AddTx(addPermValTx, status.Committed) // this is currently needed to reload the staker s.PutCurrentDelegator(del) @@ -582,7 +582,7 @@ func TestPersistStakers(t *testing.T) { ) r.NoError(err) - s.PutPendingValidator(staker) + r.NoError(s.PutPendingValidator(staker)) s.AddTx(addPermValTx, status.Committed) // this is currently needed to reload the staker r.NoError(s.Commit()) @@ -649,7 +649,7 @@ func TestPersistStakers(t *testing.T) { del, err := NewPendingStaker(addPermDelTx.ID(), utxDel) r.NoError(err) - s.PutPendingValidator(val) + r.NoError(s.PutPendingValidator(val)) s.AddTx(addPermValTx, status.Committed) // this is currently needed to reload the staker s.PutPendingDelegator(del) @@ -1099,7 +1099,7 @@ func TestStateAddRemoveValidator(t *testing.T) { for currentIndex, diff := range diffs { for _, added := range diff.addedValidators { added := added - state.PutCurrentValidator(&added) + require.NoError(state.PutCurrentValidator(&added)) } for _, added := range diff.addedDelegators { added := added diff --git a/vms/platformvm/txs/executor/advance_time_test.go b/vms/platformvm/txs/executor/advance_time_test.go index aadf8413661..c301b8bde8b 100644 --- a/vms/platformvm/txs/executor/advance_time_test.go +++ b/vms/platformvm/txs/executor/advance_time_test.go @@ -408,7 +408,7 @@ func TestAdvanceTimeTxUpdateStakers(t *testing.T) { ) require.NoError(err) - env.state.PutPendingValidator(staker) + require.NoError(env.state.PutPendingValidator(staker)) env.state.AddTx(tx, status.Committed) } env.state.SetHeight(dummyHeight) @@ -513,7 +513,7 @@ func TestAdvanceTimeTxRemoveSubnetValidator(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(tx, status.Committed) env.state.SetHeight(dummyHeight) require.NoError(env.state.Commit()) @@ -541,7 +541,7 @@ func TestAdvanceTimeTxRemoveSubnetValidator(t *testing.T) { ) require.NoError(err) - env.state.PutPendingValidator(staker) + require.NoError(env.state.PutPendingValidator(staker)) env.state.AddTx(tx, status.Committed) env.state.SetHeight(dummyHeight) require.NoError(env.state.Commit()) @@ -625,7 +625,7 @@ func TestTrackedSubnet(t *testing.T) { ) require.NoError(err) - env.state.PutPendingValidator(staker) + require.NoError(env.state.PutPendingValidator(staker)) env.state.AddTx(tx, status.Committed) env.state.SetHeight(dummyHeight) require.NoError(env.state.Commit()) @@ -962,7 +962,7 @@ func addPendingValidator( ) require.NoError(err) - env.state.PutPendingValidator(staker) + require.NoError(env.state.PutPendingValidator(staker)) env.state.AddTx(addPendingValidatorTx, status.Committed) dummyHeight := uint64(1) env.state.SetHeight(dummyHeight) diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index d7b76d10c61..f5a995772a7 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -138,7 +138,9 @@ func (e *ProposalTxExecutor) AddValidatorTx(tx *txs.AddValidatorTx) error { return err } - e.OnCommitState.PutPendingValidator(newStaker) + if err := e.OnCommitState.PutPendingValidator(newStaker); err != nil { + return err + } // Set up the state if this tx is aborted // Consume the UTXOs @@ -185,7 +187,9 @@ func (e *ProposalTxExecutor) AddSubnetValidatorTx(tx *txs.AddSubnetValidatorTx) return err } - e.OnCommitState.PutPendingValidator(newStaker) + if err := e.OnCommitState.PutPendingValidator(newStaker); err != nil { + return err + } // Set up the state if this tx is aborted // Consume the UTXOs diff --git a/vms/platformvm/txs/executor/proposal_tx_executor_test.go b/vms/platformvm/txs/executor/proposal_tx_executor_test.go index 70687a96583..eed2e6a6dde 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor_test.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor_test.go @@ -65,7 +65,7 @@ func TestProposalTxExecuteAddDelegator(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(tx, status.Committed) env.state.SetHeight(dummyHeight) require.NoError(env.state.Commit()) @@ -101,7 +101,7 @@ func TestProposalTxExecuteAddDelegator(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(tx, status.Committed) env.state.SetHeight(dummyHeight) require.NoError(env.state.Commit()) @@ -434,7 +434,7 @@ func TestProposalTxExecuteAddSubnetValidator(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(addDSTx, status.Committed) dummyHeight := uint64(1) env.state.SetHeight(dummyHeight) @@ -622,7 +622,7 @@ func TestProposalTxExecuteAddSubnetValidator(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(subnetTx, status.Committed) env.state.SetHeight(dummyHeight) require.NoError(env.state.Commit()) @@ -779,7 +779,7 @@ func TestProposalTxExecuteAddSubnetValidator(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(tx, status.Committed) env.state.SetHeight(dummyHeight) require.NoError(env.state.Commit()) @@ -909,7 +909,7 @@ func TestProposalTxExecuteAddValidator(t *testing.T) { ) require.NoError(err) - env.state.PutPendingValidator(staker) + require.NoError(env.state.PutPendingValidator(staker)) env.state.AddTx(tx, status.Committed) dummyHeight := uint64(1) env.state.SetHeight(dummyHeight) diff --git a/vms/platformvm/txs/executor/reward_validator_test.go b/vms/platformvm/txs/executor/reward_validator_test.go index 55f210ebc95..86fb83f994e 100644 --- a/vms/platformvm/txs/executor/reward_validator_test.go +++ b/vms/platformvm/txs/executor/reward_validator_test.go @@ -300,7 +300,7 @@ func TestRewardDelegatorTxExecuteOnCommitPreDelegateeDeferral(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(vdrStaker) + require.NoError(env.state.PutCurrentValidator(vdrStaker)) env.state.AddTx(vdrTx, status.Committed) env.state.PutCurrentDelegator(delStaker) env.state.AddTx(delTx, status.Committed) @@ -433,7 +433,7 @@ func TestRewardDelegatorTxExecuteOnCommitPostDelegateeDeferral(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(vdrStaker) + require.NoError(env.state.PutCurrentValidator(vdrStaker)) env.state.AddTx(vdrTx, status.Committed) env.state.PutCurrentDelegator(delStaker) env.state.AddTx(delTx, status.Committed) @@ -660,7 +660,7 @@ func TestRewardDelegatorTxAndValidatorTxExecuteOnCommitPostDelegateeDeferral(t * ) require.NoError(err) - env.state.PutCurrentValidator(vdrStaker) + require.NoError(env.state.PutCurrentValidator(vdrStaker)) env.state.AddTx(vdrTx, status.Committed) env.state.PutCurrentDelegator(delStaker) env.state.AddTx(delTx, status.Committed) @@ -831,7 +831,7 @@ func TestRewardDelegatorTxExecuteOnAbort(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(vdrStaker) + require.NoError(env.state.PutCurrentValidator(vdrStaker)) env.state.AddTx(vdrTx, status.Committed) env.state.PutCurrentDelegator(delStaker) env.state.AddTx(delTx, status.Committed) diff --git a/vms/platformvm/txs/executor/standard_tx_executor.go b/vms/platformvm/txs/executor/standard_tx_executor.go index 1204b7d8250..16e520e0af6 100644 --- a/vms/platformvm/txs/executor/standard_tx_executor.go +++ b/vms/platformvm/txs/executor/standard_tx_executor.go @@ -671,11 +671,15 @@ func (e *StandardTxExecutor) putStaker(stakerTx txs.Staker) error { switch priority := staker.Priority; { case priority.IsCurrentValidator(): - e.State.PutCurrentValidator(staker) + if err := e.State.PutCurrentValidator(staker); err != nil { + return err + } case priority.IsCurrentDelegator(): e.State.PutCurrentDelegator(staker) case priority.IsPendingValidator(): - e.State.PutPendingValidator(staker) + if err := e.State.PutPendingValidator(staker); err != nil { + return err + } case priority.IsPendingDelegator(): e.State.PutPendingDelegator(staker) default: diff --git a/vms/platformvm/txs/executor/standard_tx_executor_test.go b/vms/platformvm/txs/executor/standard_tx_executor_test.go index 2b01148a7f7..459c96c08b9 100644 --- a/vms/platformvm/txs/executor/standard_tx_executor_test.go +++ b/vms/platformvm/txs/executor/standard_tx_executor_test.go @@ -143,7 +143,7 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(tx, status.Committed) env.state.SetHeight(dummyHeight) require.NoError(env.state.Commit()) @@ -178,7 +178,7 @@ func TestStandardTxExecutorAddDelegator(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(tx, status.Committed) env.state.SetHeight(dummyHeight) require.NoError(env.state.Commit()) @@ -497,7 +497,7 @@ func TestApricotStandardTxExecutorAddSubnetValidator(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(addDSTx, status.Committed) dummyHeight := uint64(1) env.state.SetHeight(dummyHeight) @@ -669,7 +669,7 @@ func TestApricotStandardTxExecutorAddSubnetValidator(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(subnetTx, status.Committed) env.state.SetHeight(dummyHeight) require.NoError(env.state.Commit()) @@ -858,7 +858,7 @@ func TestApricotStandardTxExecutorAddSubnetValidator(t *testing.T) { ) require.NoError(err) - env.state.PutCurrentValidator(staker) + require.NoError(env.state.PutCurrentValidator(staker)) env.state.AddTx(tx, status.Committed) env.state.SetHeight(dummyHeight) require.NoError(env.state.Commit()) @@ -987,7 +987,7 @@ func TestBanffStandardTxExecutorAddValidator(t *testing.T) { onAcceptState, err := state.NewDiff(lastAcceptedID, env) require.NoError(err) - onAcceptState.PutCurrentValidator(staker) + require.NoError(onAcceptState.PutCurrentValidator(staker)) onAcceptState.AddTx(tx, status.Committed) feeCalculator := state.PickFeeCalculator(env.config, onAcceptState) @@ -1027,7 +1027,7 @@ func TestBanffStandardTxExecutorAddValidator(t *testing.T) { onAcceptState, err := state.NewDiff(lastAcceptedID, env) require.NoError(err) - onAcceptState.PutPendingValidator(staker) + require.NoError(onAcceptState.PutPendingValidator(staker)) onAcceptState.AddTx(tx, status.Committed) feeCalculator := state.PickFeeCalculator(env.config, onAcceptState) diff --git a/vms/platformvm/txs/executor/state_changes.go b/vms/platformvm/txs/executor/state_changes.go index 244be246f8a..55f940fca50 100644 --- a/vms/platformvm/txs/executor/state_changes.go +++ b/vms/platformvm/txs/executor/state_changes.go @@ -97,7 +97,9 @@ func AdvanceTimeTo( stakerToAdd.Priority = txs.PendingToCurrentPriorities[stakerToRemove.Priority] if stakerToRemove.Priority == txs.SubnetPermissionedValidatorPendingPriority { - changes.PutCurrentValidator(&stakerToAdd) + if err := changes.PutCurrentValidator(&stakerToAdd); err != nil { + return false, err + } changes.DeletePendingValidator(stakerToRemove) changed = true continue @@ -126,7 +128,9 @@ func AdvanceTimeTo( switch stakerToRemove.Priority { case txs.PrimaryNetworkValidatorPendingPriority, txs.SubnetPermissionlessValidatorPendingPriority: - changes.PutCurrentValidator(&stakerToAdd) + if err := changes.PutCurrentValidator(&stakerToAdd); err != nil { + return false, err + } changes.DeletePendingValidator(stakerToRemove) case txs.PrimaryNetworkDelegatorApricotPendingPriority, txs.PrimaryNetworkDelegatorBanffPendingPriority, txs.SubnetPermissionlessDelegatorPendingPriority: diff --git a/vms/platformvm/validators/manager_benchmark_test.go b/vms/platformvm/validators/manager_benchmark_test.go index 98227e09913..0b2222f45f1 100644 --- a/vms/platformvm/validators/manager_benchmark_test.go +++ b/vms/platformvm/validators/manager_benchmark_test.go @@ -115,7 +115,7 @@ func addPrimaryValidator( } nodeID := ids.GenerateTestNodeID() - s.PutCurrentValidator(&state.Staker{ + if err := s.PutCurrentValidator(&state.Staker{ TxID: ids.GenerateTestID(), NodeID: nodeID, PublicKey: bls.PublicFromSecretKey(sk), @@ -126,7 +126,9 @@ func addPrimaryValidator( PotentialReward: 0, NextTime: endTime, Priority: txs.PrimaryNetworkValidatorCurrentPriority, - }) + }); err != nil { + return ids.EmptyNodeID, err + } blk, err := block.NewBanffStandardBlock(startTime, ids.GenerateTestID(), height, nil) if err != nil { @@ -146,7 +148,7 @@ func addSubnetValidator( nodeID ids.NodeID, height uint64, ) error { - s.PutCurrentValidator(&state.Staker{ + if err := s.PutCurrentValidator(&state.Staker{ TxID: ids.GenerateTestID(), NodeID: nodeID, SubnetID: subnetID, @@ -156,7 +158,9 @@ func addSubnetValidator( PotentialReward: 0, NextTime: endTime, Priority: txs.SubnetPermissionlessValidatorCurrentPriority, - }) + }); err != nil { + return err + } blk, err := block.NewBanffStandardBlock(startTime, ids.GenerateTestID(), height, nil) if err != nil { diff --git a/vms/platformvm/vm_regression_test.go b/vms/platformvm/vm_regression_test.go index 811dc2dfb91..608ebaa956f 100644 --- a/vms/platformvm/vm_regression_test.go +++ b/vms/platformvm/vm_regression_test.go @@ -39,6 +39,7 @@ import ( "github.com/ava-labs/avalanchego/vms/platformvm/genesis/genesistest" "github.com/ava-labs/avalanchego/vms/platformvm/reward" "github.com/ava-labs/avalanchego/vms/platformvm/signer" + "github.com/ava-labs/avalanchego/vms/platformvm/state" "github.com/ava-labs/avalanchego/vms/platformvm/state/statetest" "github.com/ava-labs/avalanchego/vms/platformvm/txs" "github.com/ava-labs/avalanchego/vms/platformvm/txs/executor" @@ -1366,6 +1367,74 @@ func TestRemovePermissionedValidatorDuringPendingToCurrentTransitionTracked(t *t require.NoError(buildAndAcceptStandardBlock(vm)) } +func TestAddValidatorDuringRemoval(t *testing.T) { + require := require.New(t) + + vm, _, _ := defaultVM(t, upgradetest.Durango) + vm.ctx.Lock.Lock() + defer vm.ctx.Lock.Unlock() + + var ( + nodeID = genesistest.DefaultNodeIDs[0] + subnetID = testSubnet1.ID() + wallet = newWallet(t, vm, walletConfig{ + subnetIDs: []ids.ID{subnetID}, + }) + + duration = defaultMinStakingDuration + firstEndTime = latestForkTime.Add(duration) + ) + + firstAddSubnetValidatorTx, err := wallet.IssueAddSubnetValidatorTx(&txs.SubnetValidator{ + Validator: txs.Validator{ + NodeID: nodeID, + End: uint64(firstEndTime.Unix()), + Wght: 1, + }, + Subnet: subnetID, + }) + require.NoError(err) + + vm.ctx.Lock.Unlock() + require.NoError(vm.issueTxFromRPC(firstAddSubnetValidatorTx)) + vm.ctx.Lock.Lock() + + // Accept firstAddSubnetValidatorTx + require.NoError(buildAndAcceptStandardBlock(vm)) + + // Verify that the validator was added + _, err = vm.state.GetCurrentValidator(subnetID, nodeID) + require.NoError(err) + + secondEndTime := firstEndTime.Add(duration) + secondSubnetValidatorTx, err := wallet.IssueAddSubnetValidatorTx(&txs.SubnetValidator{ + Validator: txs.Validator{ + NodeID: nodeID, + End: uint64(secondEndTime.Unix()), + Wght: 1, + }, + Subnet: subnetID, + }) + require.NoError(err) + + vm.clock.Set(firstEndTime) + vm.ctx.Lock.Unlock() + err = vm.issueTxFromRPC(secondSubnetValidatorTx) + vm.ctx.Lock.Lock() + require.ErrorIs(err, state.ErrAddingStakerAfterDeletion) + + // Remove the first subnet validator + require.NoError(buildAndAcceptStandardBlock(vm)) + + // Verify that the validator does not exist + _, err = vm.state.GetCurrentValidator(subnetID, nodeID) + require.ErrorIs(err, database.ErrNotFound) + + // Verify that the invalid transaction was not executed + _, _, err = vm.state.GetTx(secondSubnetValidatorTx.ID()) + require.ErrorIs(err, database.ErrNotFound) +} + // GetValidatorSet must return the BLS keys for a given validator correctly when // queried at a previous height, even in case it has currently expired func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) {