From a48864479a1591a1a1bb4dfeae4bc7e6dcc4d23d Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Sun, 22 Sep 2024 12:25:55 -0400 Subject: [PATCH 1/4] Fix test to properly iterate heights --- vms/platformvm/vm_regression_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vms/platformvm/vm_regression_test.go b/vms/platformvm/vm_regression_test.go index 608ebaa956fc..b653961cda34 100644 --- a/vms/platformvm/vm_regression_test.go +++ b/vms/platformvm/vm_regression_test.go @@ -1621,7 +1621,7 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { vm.State, nodeID, constants.PrimaryNetworkID, - primaryEndHeight, + height, pk1, ) require.ErrorIs(err, database.ErrNotFound) @@ -1649,7 +1649,7 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { vm.State, nodeID, subnetID, - primaryEndHeight, + height, pk1, ) require.ErrorIs(err, database.ErrNotFound) From e92e9f5877f7beb93e19a0b61387cabd8c5d6d9e Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Sun, 22 Sep 2024 13:02:52 -0400 Subject: [PATCH 2/4] cleanup test --- vms/platformvm/vm_regression_test.go | 72 +++++++++++++++------------- 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/vms/platformvm/vm_regression_test.go b/vms/platformvm/vm_regression_test.go index b653961cda34..1c013dd816e2 100644 --- a/vms/platformvm/vm_regression_test.go +++ b/vms/platformvm/vm_regression_test.go @@ -1437,6 +1437,18 @@ 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) @@ -1606,53 +1618,47 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { primaryRestartHeight, err := vm.GetCurrentHeight(context.Background()) require.NoError(err) - // 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++ { + for height := uint64(0); height <= primaryRestartHeight; height++ { + // 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 + } + + // 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, - pk1, + expectedPrimaryNetworkBLSKey, ) - require.ErrorIs(err, database.ErrNotFound) - } - require.NoError(checkValidatorBlsKeyIsSet( - vm.State, - nodeID, - constants.PrimaryNetworkID, - primaryRestartHeight, - pk2, - )) + require.ErrorIs(err, expectedPrimaryNetworkErr) - for height := subnetStartHeight; height < subnetEndHeight; height++ { - require.NoError(checkValidatorBlsKeyIsSet( - vm.State, - nodeID, - subnetID, - height, - pk1, - )) - } + // The subnet validator doesn't exist for heights + // [0, subnetStartHeight) and [subnetEndHeight, primaryRestartHeight). + var expectedSubnetErr error + if height < subnetStartHeight || height >= subnetEndHeight { + expectedSubnetErr = database.ErrNotFound + } - for height := subnetEndHeight; height <= primaryRestartHeight; height++ { - err := checkValidatorBlsKeyIsSet( + err = checkValidatorBlsKeyIsSet( vm.State, nodeID, subnetID, height, pk1, ) - require.ErrorIs(err, database.ErrNotFound) + require.ErrorIs(err, expectedSubnetErr) } } From d2a4498be33d467ff50172afea45c10f538bead7 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Sun, 22 Sep 2024 13:07:32 -0400 Subject: [PATCH 3/4] cleanup test --- vms/platformvm/vm_regression_test.go | 146 ++++++++++++++------------- 1 file changed, 74 insertions(+), 72 deletions(-) diff --git a/vms/platformvm/vm_regression_test.go b/vms/platformvm/vm_regression_test.go index 1c013dd816e2..31ccdec8b7fb 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" @@ -1450,8 +1451,6 @@ func TestAddValidatorDuringRemoval(t *testing.T) { // 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() @@ -1461,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) @@ -1480,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 @@ -1500,22 +1498,22 @@ 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) // insert the subnet validator subnetTx, err := wallet.IssueAddSubnetValidatorTx( @@ -1529,60 +1527,60 @@ 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) // 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) // 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) // 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( @@ -1601,64 +1599,68 @@ 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) for height := uint64(0); height <= primaryRestartHeight; height++ { - // 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 - } + t.Run(strconv.Itoa(int(height)), func(t *testing.T) { + require := require.New(t) - // 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 - } + // 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 + } - err := checkValidatorBlsKeyIsSet( - vm.State, - nodeID, - constants.PrimaryNetworkID, - height, - expectedPrimaryNetworkBLSKey, - ) - require.ErrorIs(err, expectedPrimaryNetworkErr) + // 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 + } - // 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, + constants.PrimaryNetworkID, + height, + expectedPrimaryNetworkBLSKey, + ) + require.ErrorIs(err, expectedPrimaryNetworkErr) - err = checkValidatorBlsKeyIsSet( - vm.State, - nodeID, - subnetID, - height, - pk1, - ) - require.ErrorIs(err, expectedSubnetErr) + // 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) + }) } } From 61452b382bdfb366e5d03ef42daabdb8070252bf Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Sun, 22 Sep 2024 13:30:08 -0400 Subject: [PATCH 4/4] Add logs --- vms/platformvm/vm_regression_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/vms/platformvm/vm_regression_test.go b/vms/platformvm/vm_regression_test.go index 31ccdec8b7fb..4dc69a07990e 100644 --- a/vms/platformvm/vm_regression_test.go +++ b/vms/platformvm/vm_regression_test.go @@ -1514,6 +1514,7 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { primaryStartHeight, err := vm.GetCurrentHeight(context.Background()) require.NoError(t, err) + t.Logf("primaryStartHeight: %d", primaryStartHeight) // insert the subnet validator subnetTx, err := wallet.IssueAddSubnetValidatorTx( @@ -1543,6 +1544,7 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { subnetStartHeight, err := vm.GetCurrentHeight(context.Background()) require.NoError(t, err) + t.Logf("subnetStartHeight: %d", subnetStartHeight) // move time ahead, terminating the subnet validator vm.clock.Set(subnetEndTime) @@ -1553,6 +1555,7 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { subnetEndHeight, err := vm.GetCurrentHeight(context.Background()) require.NoError(t, err) + t.Logf("subnetEndHeight: %d", subnetEndHeight) // move time ahead, terminating primary network validator vm.clock.Set(primaryEndTime) @@ -1577,6 +1580,7 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { primaryEndHeight, err := vm.GetCurrentHeight(context.Background()) require.NoError(t, err) + t.Logf("primaryEndHeight: %d", primaryEndHeight) // reinsert primary validator with a different BLS key sk2, err := bls.NewSecretKey() @@ -1615,6 +1619,7 @@ func TestSubnetValidatorBLSKeyDiffAfterExpiry(t *testing.T) { primaryRestartHeight, err := vm.GetCurrentHeight(context.Background()) require.NoError(t, err) + t.Logf("primaryRestartHeight: %d", primaryRestartHeight) for height := uint64(0); height <= primaryRestartHeight; height++ { t.Run(strconv.Itoa(int(height)), func(t *testing.T) {