diff --git a/database/batch.go b/database/batch.go index f3187a1fa954..8a37ee723693 100644 --- a/database/batch.go +++ b/database/batch.go @@ -75,6 +75,7 @@ func (b *BatchOps) Size() int { } func (b *BatchOps) Reset() { + clear(b.Ops) b.Ops = b.Ops[:0] b.size = 0 } diff --git a/database/encdb/db.go b/database/encdb/db.go index 2bdacb465ffa..ea82b16a3659 100644 --- a/database/encdb/db.go +++ b/database/encdb/db.go @@ -204,6 +204,7 @@ func (b *batch) Reset() { if cap(b.ops) > len(b.ops)*database.MaxExcessCapacityFactor { b.ops = make([]database.BatchOp, 0, cap(b.ops)/database.CapacityReductionFactor) } else { + clear(b.ops) b.ops = b.ops[:0] } b.Batch.Reset() diff --git a/database/prefixdb/db.go b/database/prefixdb/db.go index b3082d9e986e..e1ec8da5007e 100644 --- a/database/prefixdb/db.go +++ b/database/prefixdb/db.go @@ -313,6 +313,7 @@ func (b *batch) Reset() { if cap(b.ops) > len(b.ops)*database.MaxExcessCapacityFactor { b.ops = make([]batchOp, 0, cap(b.ops)/database.CapacityReductionFactor) } else { + clear(b.ops) b.ops = b.ops[:0] } b.Batch.Reset() diff --git a/utils/set/sampleable_set.go b/utils/set/sampleable_set.go index efb63c2d777c..a70b332a9a68 100644 --- a/utils/set/sampleable_set.go +++ b/utils/set/sampleable_set.go @@ -108,9 +108,7 @@ func (s *SampleableSet[T]) Remove(elements ...T) { // Clear empties this set func (s *SampleableSet[T]) Clear() { clear(s.indices) - for i := range s.elements { - s.elements[i] = utils.Zero[T]() - } + clear(s.elements) s.elements = s.elements[:0] } diff --git a/utils/zero.go b/utils/zero.go index c691ed2e653c..bb31de1c8d58 100644 --- a/utils/zero.go +++ b/utils/zero.go @@ -4,16 +4,6 @@ package utils // Returns a new instance of a T. -func Zero[T any]() T { - return *new(T) -} - -// ZeroSlice sets all values of the provided slice to the type's zero value. -// -// This can be useful to ensure that the garbage collector doesn't hold -// references to values that are no longer desired. -func ZeroSlice[T any](s []T) { - for i := range s { - s[i] = *new(T) - } +func Zero[T any]() (_ T) { + return } diff --git a/vms/platformvm/block/builder/helpers_test.go b/vms/platformvm/block/builder/helpers_test.go index 02f7102a562c..756c58090a2e 100644 --- a/vms/platformvm/block/builder/helpers_test.go +++ b/vms/platformvm/block/builder/helpers_test.go @@ -46,12 +46,12 @@ import ( "github.com/ava-labs/avalanchego/vms/platformvm/txs/mempool" "github.com/ava-labs/avalanchego/vms/platformvm/txs/txstest" "github.com/ava-labs/avalanchego/vms/platformvm/utxo" + "github.com/ava-labs/avalanchego/vms/platformvm/validators/validatorstest" "github.com/ava-labs/avalanchego/vms/secp256k1fx" "github.com/ava-labs/avalanchego/wallet/chain/p/wallet" blockexecutor "github.com/ava-labs/avalanchego/vms/platformvm/block/executor" txexecutor "github.com/ava-labs/avalanchego/vms/platformvm/txs/executor" - pvalidators "github.com/ava-labs/avalanchego/vms/platformvm/validators" ) const ( @@ -153,7 +153,7 @@ func newEnvironment(t *testing.T, f upgradetest.Fork) *environment { //nolint:un metrics, res.state, &res.backend, - pvalidators.TestManager, + validatorstest.Manager, ) txVerifier := network.NewLockedTxVerifier(&res.ctx.Lock, res.blkManager) diff --git a/vms/platformvm/block/executor/acceptor_test.go b/vms/platformvm/block/executor/acceptor_test.go index 6f23961b7452..b3f1482c1f82 100644 --- a/vms/platformvm/block/executor/acceptor_test.go +++ b/vms/platformvm/block/executor/acceptor_test.go @@ -23,7 +23,7 @@ import ( "github.com/ava-labs/avalanchego/vms/platformvm/metrics" "github.com/ava-labs/avalanchego/vms/platformvm/state" "github.com/ava-labs/avalanchego/vms/platformvm/txs" - "github.com/ava-labs/avalanchego/vms/platformvm/validators" + "github.com/ava-labs/avalanchego/vms/platformvm/validators/validatorstest" "github.com/ava-labs/avalanchego/vms/secp256k1fx" ) @@ -62,7 +62,7 @@ func TestAcceptorVisitProposalBlock(t *testing.T) { state: s, }, metrics: metrics.Noop, - validators: validators.TestManager, + validators: validatorstest.Manager, } require.NoError(acceptor.ApricotProposalBlock(blk)) @@ -97,7 +97,7 @@ func TestAcceptorVisitAtomicBlock(t *testing.T) { }, }, metrics: metrics.Noop, - validators: validators.TestManager, + validators: validatorstest.Manager, } blk, err := block.NewApricotAtomicBlock( @@ -177,7 +177,7 @@ func TestAcceptorVisitStandardBlock(t *testing.T) { }, }, metrics: metrics.Noop, - validators: validators.TestManager, + validators: validatorstest.Manager, } blk, err := block.NewBanffStandardBlock( @@ -266,7 +266,7 @@ func TestAcceptorVisitCommitBlock(t *testing.T) { }, }, metrics: metrics.Noop, - validators: validators.TestManager, + validators: validatorstest.Manager, bootstrapped: &utils.Atomic[bool]{}, } @@ -376,7 +376,7 @@ func TestAcceptorVisitAbortBlock(t *testing.T) { }, }, metrics: metrics.Noop, - validators: validators.TestManager, + validators: validatorstest.Manager, bootstrapped: &utils.Atomic[bool]{}, } diff --git a/vms/platformvm/block/executor/helpers_test.go b/vms/platformvm/block/executor/helpers_test.go index b11e4714c885..2fa53be65b10 100644 --- a/vms/platformvm/block/executor/helpers_test.go +++ b/vms/platformvm/block/executor/helpers_test.go @@ -46,10 +46,9 @@ import ( "github.com/ava-labs/avalanchego/vms/platformvm/txs/mempool" "github.com/ava-labs/avalanchego/vms/platformvm/txs/txstest" "github.com/ava-labs/avalanchego/vms/platformvm/utxo" + "github.com/ava-labs/avalanchego/vms/platformvm/validators/validatorstest" "github.com/ava-labs/avalanchego/vms/secp256k1fx" "github.com/ava-labs/avalanchego/wallet/chain/p/wallet" - - pvalidators "github.com/ava-labs/avalanchego/vms/platformvm/validators" ) const ( @@ -166,7 +165,7 @@ func newEnvironment(t *testing.T, ctrl *gomock.Controller, f upgradetest.Fork) * metrics, res.state, res.backend, - pvalidators.TestManager, + validatorstest.Manager, ) addSubnet(t, res) } else { @@ -175,7 +174,7 @@ func newEnvironment(t *testing.T, ctrl *gomock.Controller, f upgradetest.Fork) * metrics, res.mockedState, res.backend, - pvalidators.TestManager, + validatorstest.Manager, ) // we do not add any subnet to state, since we can mock // whatever we need diff --git a/vms/platformvm/validators/test_manager.go b/vms/platformvm/validators/test_manager.go deleted file mode 100644 index e04742f265c7..000000000000 --- a/vms/platformvm/validators/test_manager.go +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package validators - -import ( - "context" - - "github.com/ava-labs/avalanchego/ids" - "github.com/ava-labs/avalanchego/snow/validators" -) - -var TestManager Manager = testManager{} - -type testManager struct{} - -func (testManager) GetMinimumHeight(context.Context) (uint64, error) { - return 0, nil -} - -func (testManager) GetCurrentHeight(context.Context) (uint64, error) { - return 0, nil -} - -func (testManager) GetSubnetID(context.Context, ids.ID) (ids.ID, error) { - return ids.Empty, nil -} - -func (testManager) GetValidatorSet(context.Context, uint64, ids.ID) (map[ids.NodeID]*validators.GetValidatorOutput, error) { - return nil, nil -} - -func (testManager) OnAcceptedBlockID(ids.ID) {} diff --git a/vms/platformvm/validators/validatorstest/manager.go b/vms/platformvm/validators/validatorstest/manager.go new file mode 100644 index 000000000000..068073f4af6b --- /dev/null +++ b/vms/platformvm/validators/validatorstest/manager.go @@ -0,0 +1,35 @@ +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package validatorstest + +import ( + "context" + + "github.com/ava-labs/avalanchego/ids" + + snowvalidators "github.com/ava-labs/avalanchego/snow/validators" + vmvalidators "github.com/ava-labs/avalanchego/vms/platformvm/validators" +) + +var Manager vmvalidators.Manager = manager{} + +type manager struct{} + +func (manager) GetMinimumHeight(context.Context) (uint64, error) { + return 0, nil +} + +func (manager) GetCurrentHeight(context.Context) (uint64, error) { + return 0, nil +} + +func (manager) GetSubnetID(context.Context, ids.ID) (ids.ID, error) { + return ids.Empty, nil +} + +func (manager) GetValidatorSet(context.Context, uint64, ids.ID) (map[ids.NodeID]*snowvalidators.GetValidatorOutput, error) { + return nil, nil +} + +func (manager) OnAcceptedBlockID(ids.ID) {} diff --git a/vms/platformvm/vm_regression_test.go b/vms/platformvm/vm_regression_test.go index 608ebaa956fc..4dc69a07990e 100644 --- a/vms/platformvm/vm_regression_test.go +++ b/vms/platformvm/vm_regression_test.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "errors" + "strconv" "testing" "time" @@ -1437,9 +1438,19 @@ func TestAddValidatorDuringRemoval(t *testing.T) { // GetValidatorSet must return the BLS keys for a given validator correctly when // queried at a previous height, even in case it has currently expired +// +// 1. Add primary network validator +// 2. Advance chain time for the primary network validator to be moved to the +// current validator set +// 3. Add permissioned subnet validator +// 4. Advance chain time for the subnet validator to be moved to the current +// validator set +// 5. Advance chain time for the subnet validator to be removed +// 6. Advance chain time for the primary network validator to be removed +// 7. Re-add the primary network validator with a different BLS key +// 8. Advance chain time for the primary network validator to be moved to the +// current validator set func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { - // setup - require := require.New(t) vm, _, _ := defaultVM(t, upgradetest.Cortina) vm.ctx.Lock.Lock() defer vm.ctx.Lock.Unlock() @@ -1449,7 +1460,6 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { subnetIDs: []ids.ID{subnetID}, }) - // A subnet validator stakes and then stops; also its primary network counterpart stops staking var ( primaryStartTime = genesistest.DefaultValidatorStartTime.Add(executor.SyncBound) subnetStartTime = primaryStartTime.Add(executor.SyncBound) @@ -1468,7 +1478,7 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { } ) sk1, err := bls.NewSecretKey() - require.NoError(err) + require.NoError(t, err) pk1 := bls.PublicFromSecretKey(sk1) // build primary network validator with BLS key @@ -1488,22 +1498,23 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { rewardsOwner, reward.PercentDenominator, ) - require.NoError(err) + require.NoError(t, err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTxFromRPC(primaryTx)) + require.NoError(t, vm.issueTxFromRPC(primaryTx)) vm.ctx.Lock.Lock() - require.NoError(buildAndAcceptStandardBlock(vm)) + require.NoError(t, buildAndAcceptStandardBlock(vm)) // move time ahead, promoting primary validator to current vm.clock.Set(primaryStartTime) - require.NoError(buildAndAcceptStandardBlock(vm)) + require.NoError(t, buildAndAcceptStandardBlock(vm)) _, err = vm.state.GetCurrentValidator(constants.PrimaryNetworkID, nodeID) - require.NoError(err) + require.NoError(t, err) primaryStartHeight, err := vm.GetCurrentHeight(context.Background()) - require.NoError(err) + require.NoError(t, err) + t.Logf("primaryStartHeight: %d", primaryStartHeight) // insert the subnet validator subnetTx, err := wallet.IssueAddSubnetValidatorTx( @@ -1517,60 +1528,63 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { Subnet: subnetID, }, ) - require.NoError(err) + require.NoError(t, err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTxFromRPC(subnetTx)) + require.NoError(t, vm.issueTxFromRPC(subnetTx)) vm.ctx.Lock.Lock() - require.NoError(buildAndAcceptStandardBlock(vm)) + require.NoError(t, buildAndAcceptStandardBlock(vm)) // move time ahead, promoting the subnet validator to current vm.clock.Set(subnetStartTime) - require.NoError(buildAndAcceptStandardBlock(vm)) + require.NoError(t, buildAndAcceptStandardBlock(vm)) _, err = vm.state.GetCurrentValidator(subnetID, nodeID) - require.NoError(err) + require.NoError(t, err) subnetStartHeight, err := vm.GetCurrentHeight(context.Background()) - require.NoError(err) + require.NoError(t, err) + t.Logf("subnetStartHeight: %d", subnetStartHeight) // move time ahead, terminating the subnet validator vm.clock.Set(subnetEndTime) - require.NoError(buildAndAcceptStandardBlock(vm)) + require.NoError(t, buildAndAcceptStandardBlock(vm)) _, err = vm.state.GetCurrentValidator(subnetID, nodeID) - require.ErrorIs(err, database.ErrNotFound) + require.ErrorIs(t, err, database.ErrNotFound) subnetEndHeight, err := vm.GetCurrentHeight(context.Background()) - require.NoError(err) + require.NoError(t, err) + t.Logf("subnetEndHeight: %d", subnetEndHeight) // move time ahead, terminating primary network validator vm.clock.Set(primaryEndTime) blk, err := vm.Builder.BuildBlock(context.Background()) // must be a proposal block rewarding the primary validator - require.NoError(err) - require.NoError(blk.Verify(context.Background())) + require.NoError(t, err) + require.NoError(t, blk.Verify(context.Background())) proposalBlk := blk.(snowman.OracleBlock) options, err := proposalBlk.Options(context.Background()) - require.NoError(err) + require.NoError(t, err) commit := options[0].(*blockexecutor.Block) - require.IsType(&block.BanffCommitBlock{}, commit.Block) + require.IsType(t, &block.BanffCommitBlock{}, commit.Block) - require.NoError(blk.Accept(context.Background())) - require.NoError(commit.Verify(context.Background())) - require.NoError(commit.Accept(context.Background())) - require.NoError(vm.SetPreference(context.Background(), vm.manager.LastAccepted())) + require.NoError(t, blk.Accept(context.Background())) + require.NoError(t, commit.Verify(context.Background())) + require.NoError(t, commit.Accept(context.Background())) + require.NoError(t, vm.SetPreference(context.Background(), vm.manager.LastAccepted())) _, err = vm.state.GetCurrentValidator(constants.PrimaryNetworkID, nodeID) - require.ErrorIs(err, database.ErrNotFound) + require.ErrorIs(t, err, database.ErrNotFound) primaryEndHeight, err := vm.GetCurrentHeight(context.Background()) - require.NoError(err) + require.NoError(t, err) + t.Logf("primaryEndHeight: %d", primaryEndHeight) // reinsert primary validator with a different BLS key sk2, err := bls.NewSecretKey() - require.NoError(err) + require.NoError(t, err) pk2 := bls.PublicFromSecretKey(sk2) primaryRestartTx, err := wallet.IssueAddPermissionlessValidatorTx( @@ -1589,70 +1603,69 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { rewardsOwner, reward.PercentDenominator, ) - require.NoError(err) + require.NoError(t, err) vm.ctx.Lock.Unlock() - require.NoError(vm.issueTxFromRPC(primaryRestartTx)) + require.NoError(t, vm.issueTxFromRPC(primaryRestartTx)) vm.ctx.Lock.Lock() - require.NoError(buildAndAcceptStandardBlock(vm)) + require.NoError(t, buildAndAcceptStandardBlock(vm)) // move time ahead, promoting restarted primary validator to current vm.clock.Set(primaryReStartTime) - require.NoError(buildAndAcceptStandardBlock(vm)) + require.NoError(t, buildAndAcceptStandardBlock(vm)) _, err = vm.state.GetCurrentValidator(constants.PrimaryNetworkID, nodeID) - require.NoError(err) + require.NoError(t, err) primaryRestartHeight, err := vm.GetCurrentHeight(context.Background()) - require.NoError(err) + require.NoError(t, err) + t.Logf("primaryRestartHeight: %d", primaryRestartHeight) - // Show that validators are rebuilt with the right BLS key - for height := primaryStartHeight; height < primaryEndHeight; height++ { - require.NoError(checkValidatorBlsKeyIsSet( - vm.State, - nodeID, - constants.PrimaryNetworkID, - height, - pk1, - )) - } - for height := primaryEndHeight; height < primaryRestartHeight; height++ { - err := checkValidatorBlsKeyIsSet( - vm.State, - nodeID, - constants.PrimaryNetworkID, - primaryEndHeight, - pk1, - ) - require.ErrorIs(err, database.ErrNotFound) - } - require.NoError(checkValidatorBlsKeyIsSet( - vm.State, - nodeID, - constants.PrimaryNetworkID, - primaryRestartHeight, - pk2, - )) + for height := uint64(0); height <= primaryRestartHeight; height++ { + t.Run(strconv.Itoa(int(height)), func(t *testing.T) { + require := require.New(t) - for height := subnetStartHeight; height < subnetEndHeight; height++ { - require.NoError(checkValidatorBlsKeyIsSet( - vm.State, - nodeID, - subnetID, - height, - pk1, - )) - } + // The primary network validator doesn't exist for heights + // [0, primaryStartHeight) and [primaryEndHeight, primaryRestartHeight). + var expectedPrimaryNetworkErr error + if height < primaryStartHeight || (height >= primaryEndHeight && height < primaryRestartHeight) { + expectedPrimaryNetworkErr = database.ErrNotFound + } - for height := subnetEndHeight; height <= primaryRestartHeight; height++ { - err := checkValidatorBlsKeyIsSet( - vm.State, - nodeID, - subnetID, - primaryEndHeight, - pk1, - ) - require.ErrorIs(err, database.ErrNotFound) + // The primary network validator's BLS key is pk1 for the first + // validation period and pk2 for the second validation period. + var expectedPrimaryNetworkBLSKey *bls.PublicKey + if height >= primaryStartHeight && height < primaryEndHeight { + expectedPrimaryNetworkBLSKey = pk1 + } else if height >= primaryRestartHeight { + expectedPrimaryNetworkBLSKey = pk2 + } + + err := checkValidatorBlsKeyIsSet( + vm.State, + nodeID, + constants.PrimaryNetworkID, + height, + expectedPrimaryNetworkBLSKey, + ) + require.ErrorIs(err, expectedPrimaryNetworkErr) + + // The subnet validator doesn't exist for heights + // [0, subnetStartHeight) and [subnetEndHeight, primaryRestartHeight). + var expectedSubnetErr error + if height < subnetStartHeight || height >= subnetEndHeight { + expectedSubnetErr = database.ErrNotFound + } + + err = checkValidatorBlsKeyIsSet( + vm.State, + nodeID, + subnetID, + height, + pk1, + ) + require.ErrorIs(err, expectedSubnetErr) + }) } }