diff --git a/upgrade/upgradetest/fork.go b/upgrade/upgradetest/fork.go index ce945498cc81..f92242191911 100644 --- a/upgrade/upgradetest/fork.go +++ b/upgrade/upgradetest/fork.go @@ -19,7 +19,7 @@ const ( Etna FUpgrade - Latest = Etna + Latest = FUpgrade ) // Fork is an enum of all the major network upgrades. diff --git a/vms/platformvm/block/executor/verifier.go b/vms/platformvm/block/executor/verifier.go index f374c9067e4b..0a613e811a18 100644 --- a/vms/platformvm/block/executor/verifier.go +++ b/vms/platformvm/block/executor/verifier.go @@ -74,7 +74,7 @@ func (v *verifier) BanffProposalBlock(b *block.BanffProposalBlock) error { } feeCalculator := state.PickFeeCalculator(v.txExecutorBackend.Config, onDecisionState) - inputs, atomicRequests, onAcceptFunc, gasConsumed, err := v.processStandardTxs( + inputs, atomicRequests, onAcceptFunc, gasConsumed, _, err := v.processStandardTxs( b.Transactions, feeCalculator, onDecisionState, @@ -129,19 +129,28 @@ func (v *verifier) BanffStandardBlock(b *block.BanffStandardBlock) error { return err } - // If this block doesn't perform any changes, then it should never have been - // issued. - if !changed && len(b.Transactions) == 0 { - return errBanffStandardBlockWithoutChanges - } - feeCalculator := state.PickFeeCalculator(v.txExecutorBackend.Config, onAcceptState) - return v.standardBlock( + lowBalanceL1ValidatorsEvicted, err := v.standardBlock( b, b.Transactions, feeCalculator, onAcceptState, ) + if err != nil { + return err + } + + // Verify that the block performs changes. If it does not, it never should + // have been issued. Prior to the F upgrade, evicting L1 validators that + // don't have enough balance for the next second is not considered a change. + // After the F upgrade, it is. + if !changed && + len(b.Transactions) == 0 && + (!v.txExecutorBackend.Config.UpgradeConfig.IsFUpgradeActivated(b.Timestamp()) || !lowBalanceL1ValidatorsEvicted) { + return errBanffStandardBlockWithoutChanges + } + + return nil } func (v *verifier) ApricotAbortBlock(b *block.ApricotAbortBlock) error { @@ -200,12 +209,13 @@ func (v *verifier) ApricotStandardBlock(b *block.ApricotStandardBlock) error { } feeCalculator := txfee.NewSimpleCalculator(0) - return v.standardBlock( + _, err = v.standardBlock( b, b.Transactions, feeCalculator, onAcceptState, ) + return err } func (v *verifier) ApricotAtomicBlock(b *block.ApricotAtomicBlock) error { @@ -464,15 +474,15 @@ func (v *verifier) standardBlock( txs []*txs.Tx, feeCalculator txfee.Calculator, onAcceptState state.Diff, -) error { - inputs, atomicRequests, onAcceptFunc, gasConsumed, err := v.processStandardTxs( +) (bool, error) { + inputs, atomicRequests, onAcceptFunc, gasConsumed, lowBalanceL1ValidatorsEvicted, err := v.processStandardTxs( txs, feeCalculator, onAcceptState, b.Parent(), ) if err != nil { - return err + return false, err } v.Mempool.Remove(txs...) @@ -495,7 +505,7 @@ func (v *verifier) standardBlock( gasConsumed, ), } - return nil + return lowBalanceL1ValidatorsEvicted, nil } func (v *verifier) processStandardTxs(txs []*txs.Tx, feeCalculator txfee.Calculator, diff state.Diff, parentID ids.ID) ( @@ -503,6 +513,7 @@ func (v *verifier) processStandardTxs(txs []*txs.Tx, feeCalculator txfee.Calcula map[ids.ID]*atomic.Requests, func(), gas.Gas, + bool, error, ) { // Complexity is limited first to avoid processing too large of a block. @@ -514,19 +525,19 @@ func (v *verifier) processStandardTxs(txs []*txs.Tx, feeCalculator txfee.Calcula if err != nil { txID := tx.ID() v.MarkDropped(txID, err) - return nil, nil, nil, 0, err + return nil, nil, nil, 0, false, err } blockComplexity, err = blockComplexity.Add(&txComplexity) if err != nil { - return nil, nil, nil, 0, fmt.Errorf("block complexity overflow: %w", err) + return nil, nil, nil, 0, false, fmt.Errorf("block complexity overflow: %w", err) } } var err error gasConsumed, err = blockComplexity.ToGas(v.txExecutorBackend.Config.DynamicFeeConfig.Weights) if err != nil { - return nil, nil, nil, 0, fmt.Errorf("block gas overflow: %w", err) + return nil, nil, nil, 0, false, fmt.Errorf("block gas overflow: %w", err) } // If this block exceeds the available capacity, ConsumeGas will return @@ -534,7 +545,7 @@ func (v *verifier) processStandardTxs(txs []*txs.Tx, feeCalculator txfee.Calcula feeState := diff.GetFeeState() feeState, err = feeState.ConsumeGas(gasConsumed) if err != nil { - return nil, nil, nil, 0, err + return nil, nil, nil, 0, false, err } // Updating the fee state prior to executing the transactions is fine @@ -558,11 +569,11 @@ func (v *verifier) processStandardTxs(txs []*txs.Tx, feeCalculator txfee.Calcula if err != nil { txID := tx.ID() v.MarkDropped(txID, err) // cache tx as dropped - return nil, nil, nil, 0, err + return nil, nil, nil, 0, false, err } // ensure it doesn't overlap with current input batch if inputs.Overlaps(txInputs) { - return nil, nil, nil, 0, ErrConflictingBlockTxs + return nil, nil, nil, 0, false, ErrConflictingBlockTxs } // Add UTXOs to batch inputs.Union(txInputs) @@ -586,7 +597,7 @@ func (v *verifier) processStandardTxs(txs []*txs.Tx, feeCalculator txfee.Calcula } if err := v.verifyUniqueInputs(parentID, inputs); err != nil { - return nil, nil, nil, 0, err + return nil, nil, nil, 0, false, err } if numFuncs := len(funcs); numFuncs == 1 { @@ -603,15 +614,15 @@ func (v *verifier) processStandardTxs(txs []*txs.Tx, feeCalculator txfee.Calcula // might not have sufficient fee to pay for the next second. // // This ensures that L1 validators are not undercharged for the next second. - err := deactivateLowBalanceL1Validators( + lowBalanceL1ValidatorsEvicted, err := deactivateLowBalanceL1Validators( v.txExecutorBackend.Config.ValidatorFeeConfig, diff, ) if err != nil { - return nil, nil, nil, 0, fmt.Errorf("failed to deactivate low balance L1 validators: %w", err) + return nil, nil, nil, 0, false, fmt.Errorf("failed to deactivate low balance L1 validators: %w", err) } - return inputs, atomicRequests, onAcceptFunc, gasConsumed, nil + return inputs, atomicRequests, onAcceptFunc, gasConsumed, lowBalanceL1ValidatorsEvicted, nil } func calculateBlockMetrics( @@ -647,11 +658,12 @@ func calculateBlockMetrics( } // deactivateLowBalanceL1Validators deactivates any L1 validators that might not -// have sufficient fees to pay for the next second. +// have sufficient fees to pay for the next second. The returned bool will be +// true if at least one L1 validator was deactivated. func deactivateLowBalanceL1Validators( config validatorfee.Config, diff state.Diff, -) error { +) (bool, error) { var ( accruedFees = diff.GetAccruedFees() validatorFeeState = validatorfee.State{ @@ -665,13 +677,13 @@ func deactivateLowBalanceL1Validators( ) potentialAccruedFees, err := math.Add(accruedFees, potentialCost) if err != nil { - return fmt.Errorf("could not calculate potentially accrued fees: %w", err) + return false, fmt.Errorf("could not calculate potentially accrued fees: %w", err) } // Invariant: Proposal transactions do not impact L1 validator state. l1ValidatorIterator, err := diff.GetActiveL1ValidatorsIterator() if err != nil { - return fmt.Errorf("could not iterate over active L1 validators: %w", err) + return false, fmt.Errorf("could not iterate over active L1 validators: %w", err) } var l1ValidatorsToDeactivate []state.L1Validator @@ -696,8 +708,8 @@ func deactivateLowBalanceL1Validators( for _, l1Validator := range l1ValidatorsToDeactivate { l1Validator.EndAccumulatedFee = 0 if err := diff.PutL1Validator(l1Validator); err != nil { - return fmt.Errorf("could not deactivate L1 validator %s: %w", l1Validator.ValidationID, err) + return false, fmt.Errorf("could not deactivate L1 validator %s: %w", l1Validator.ValidationID, err) } } - return nil + return len(l1ValidatorsToDeactivate) > 0, nil } diff --git a/vms/platformvm/block/executor/verifier_test.go b/vms/platformvm/block/executor/verifier_test.go index 94ef45cce28e..5f923f532d8c 100644 --- a/vms/platformvm/block/executor/verifier_test.go +++ b/vms/platformvm/block/executor/verifier_test.go @@ -53,9 +53,10 @@ import ( ) type testVerifierConfig struct { - DB database.Database - Upgrades upgrade.Config - Context *snow.Context + DB database.Database + Upgrades upgrade.Config + Context *snow.Context + ValidatorFeeConfig validatorfee.Config } func newTestVerifier(t testing.TB, c testVerifierConfig) *verifier { @@ -70,6 +71,9 @@ func newTestVerifier(t testing.TB, c testVerifierConfig) *verifier { if c.Context == nil { c.Context = snowtest.Context(t, constants.PlatformChainID) } + if c.ValidatorFeeConfig == (validatorfee.Config{}) { + c.ValidatorFeeConfig = genesis.LocalParams.ValidatorFeeConfig + } mempool, err := mempool.New("", prometheus.NewRegistry(), nil) require.NoError(err) @@ -99,7 +103,7 @@ func newTestVerifier(t testing.TB, c testVerifierConfig) *verifier { txExecutorBackend: &executor.Backend{ Config: &config.Internal{ DynamicFeeConfig: genesis.LocalParams.DynamicFeeConfig, - ValidatorFeeConfig: genesis.LocalParams.ValidatorFeeConfig, + ValidatorFeeConfig: c.ValidatorFeeConfig, SybilProtectionEnabled: true, UpgradeConfig: c.Upgrades, }, @@ -1246,9 +1250,10 @@ func TestDeactivateLowBalanceL1Validators(t *testing.T) { ) tests := []struct { - name string - initialL1Validators []state.L1Validator - expectedL1Validators []state.L1Validator + name string + initialL1Validators []state.L1Validator + expectedL1Validators []state.L1Validator + expectedLowBalanceL1ValidatorsEvicted bool }{ { name: "no L1 validators", @@ -1258,6 +1263,7 @@ func TestDeactivateLowBalanceL1Validators(t *testing.T) { initialL1Validators: []state.L1Validator{ fractionalTimeL1Validator0, }, + expectedLowBalanceL1ValidatorsEvicted: true, }, { name: "fractional L1 validators are not undercharged", @@ -1265,6 +1271,7 @@ func TestDeactivateLowBalanceL1Validators(t *testing.T) { fractionalTimeL1Validator0, fractionalTimeL1Validator1, }, + expectedLowBalanceL1ValidatorsEvicted: true, }, { name: "whole L1 validators are not overcharged", @@ -1284,6 +1291,7 @@ func TestDeactivateLowBalanceL1Validators(t *testing.T) { expectedL1Validators: []state.L1Validator{ wholeTimeL1Validator, }, + expectedLowBalanceL1ValidatorsEvicted: true, }, } for _, test := range tests { @@ -1304,7 +1312,9 @@ func TestDeactivateLowBalanceL1Validators(t *testing.T) { MinPrice: gas.Price(2 * units.NanoAvax), // Min price is increased to allow fractional fees ExcessConversionConstant: genesis.LocalParams.ValidatorFeeConfig.ExcessConversionConstant, } - require.NoError(deactivateLowBalanceL1Validators(config, diff)) + lowBalanceL1ValidatorsEvicted, err := deactivateLowBalanceL1Validators(config, diff) + require.NoError(err) + require.Equal(test.expectedLowBalanceL1ValidatorsEvicted, lowBalanceL1ValidatorsEvicted) l1Validators, err := diff.GetActiveL1ValidatorsIterator() require.NoError(err) @@ -1315,3 +1325,76 @@ func TestDeactivateLowBalanceL1Validators(t *testing.T) { }) } } + +func TestDeactivateLowBalanceL1ValidatorBlockChanges(t *testing.T) { + signer, err := localsigner.New() + require.NoError(t, err) + + fractionalTimeL1Validator := state.L1Validator{ + ValidationID: ids.GenerateTestID(), + SubnetID: ids.GenerateTestID(), + NodeID: ids.GenerateTestNodeID(), + PublicKey: bls.PublicKeyToUncompressedBytes(signer.PublicKey()), + Weight: 1, + EndAccumulatedFee: 3 * units.NanoAvax, // lasts 1.5 seconds + } + + tests := []struct { + name string + currentFork upgradetest.Fork + durationToAdvance time.Duration + expectedErr error + }{ + { + name: "Before F Upgrade - no L1 validators evicted", + currentFork: upgradetest.Etna, + durationToAdvance: 0, + expectedErr: errBanffStandardBlockWithoutChanges, + }, + { + name: "After F Upgrade - no L1 validators evicted", + currentFork: upgradetest.FUpgrade, + durationToAdvance: 0, + expectedErr: errBanffStandardBlockWithoutChanges, + }, + { + name: "Before F Upgrade - L1 validators evicted", + currentFork: upgradetest.Etna, + durationToAdvance: time.Second, + expectedErr: errBanffStandardBlockWithoutChanges, + }, + { + name: "After F Upgrade - L1 validators evicted", + currentFork: upgradetest.FUpgrade, + durationToAdvance: time.Second, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require := require.New(t) + + verifier := newTestVerifier(t, testVerifierConfig{ + Upgrades: upgradetest.GetConfig(test.currentFork), + ValidatorFeeConfig: validatorfee.Config{ + Capacity: genesis.LocalParams.ValidatorFeeConfig.Capacity, + Target: genesis.LocalParams.ValidatorFeeConfig.Target, + MinPrice: gas.Price(2 * units.NanoAvax), // Min price is increased to allow fractional fees + ExcessConversionConstant: genesis.LocalParams.ValidatorFeeConfig.ExcessConversionConstant, + }, + }) + + require.NoError(verifier.state.PutL1Validator(fractionalTimeL1Validator)) + + blk, err := block.NewBanffStandardBlock( + genesistest.DefaultValidatorStartTime.Add(test.durationToAdvance), + verifier.state.GetLastAccepted(), + 1, // This block is built on top of the genesis + nil, // There are no transactions in the block + ) + require.NoError(err) + + err = verifier.BanffStandardBlock(blk) + require.ErrorIs(err, test.expectedErr) + }) + } +}