diff --git a/consensus/dummy/consensus.go b/consensus/dummy/consensus.go index 1022dc873b..78272ab964 100644 --- a/consensus/dummy/consensus.go +++ b/consensus/dummy/consensus.go @@ -7,17 +7,13 @@ import ( "errors" "fmt" "math/big" - "time" "github.com/ava-labs/avalanchego/utils/timer/mockable" "github.com/ava-labs/avalanchego/vms/components/gas" "github.com/ava-labs/coreth/consensus" - "github.com/ava-labs/coreth/consensus/misc/eip4844" "github.com/ava-labs/coreth/core/state" "github.com/ava-labs/coreth/params" - "github.com/ava-labs/coreth/params/extras" "github.com/ava-labs/coreth/plugin/evm/customtypes" - "github.com/ava-labs/coreth/utils" "github.com/ava-labs/libevm/common" "github.com/ava-labs/libevm/core/types" "github.com/ava-labs/libevm/trie" @@ -26,14 +22,9 @@ import ( ) var ( - allowedFutureBlockTime = 10 * time.Second // Max time from current time allowed for blocks, before they're considered future blocks - ErrInsufficientBlockGas = errors.New("insufficient gas to cover the block cost") - errInvalidBlockTime = errors.New("timestamp less than parent's") - errUnclesUnsupported = errors.New("uncles unsupported") - errExtDataGasUsedNil = errors.New("extDataGasUsed is nil") - errExtDataGasUsedTooLarge = errors.New("extDataGasUsed is not uint64") + errUnclesUnsupported = errors.New("uncles unsupported") ) type Mode struct { @@ -51,7 +42,7 @@ type ( ) ( extraData []byte, blockFeeContribution *big.Int, - extDataGasUsed *big.Int, + extDataGasUsed uint64, err error, ) @@ -61,7 +52,6 @@ type ( statedb *state.StateDB, ) ( blockFeeContribution *big.Int, - extDataGasUsed *big.Int, err error, ) @@ -148,138 +138,12 @@ func NewFullFaker() *DummyEngine { } } -func verifyHeaderGasFields(config *extras.ChainConfig, header *types.Header, parent *types.Header) error { - if err := customheader.VerifyGasUsed(config, parent, header); err != nil { - return err - } - if err := customheader.VerifyGasLimit(config, parent, header); err != nil { - return err - } - if err := customheader.VerifyExtraPrefix(config, parent, header); err != nil { - return err - } - - // Verify header.BaseFee matches the expected value. - expectedBaseFee, err := customheader.BaseFee(config, parent, header.Time) - if err != nil { - return fmt.Errorf("failed to calculate base fee: %w", err) - } - if !utils.BigEqual(header.BaseFee, expectedBaseFee) { - return fmt.Errorf("expected base fee %d, found %d", expectedBaseFee, header.BaseFee) - } - - headerExtra := customtypes.GetHeaderExtra(header) - - // Enforce BlockGasCost constraints - expectedBlockGasCost := customheader.BlockGasCost( - config, - parent, - header.Time, - ) - if !utils.BigEqual(headerExtra.BlockGasCost, expectedBlockGasCost) { - return fmt.Errorf("invalid block gas cost: have %d, want %d", headerExtra.BlockGasCost, expectedBlockGasCost) - } - - // Verify ExtDataGasUsed not present before AP4 - if !config.IsApricotPhase4(header.Time) { - if headerExtra.ExtDataGasUsed != nil { - return fmt.Errorf("invalid extDataGasUsed before fork: have %d, want ", headerExtra.ExtDataGasUsed) - } - return nil - } - - // ExtDataGasUsed correctness is checked during block validation - // (when the validator has access to the block contents) - if headerExtra.ExtDataGasUsed == nil { - return errExtDataGasUsedNil - } - if !headerExtra.ExtDataGasUsed.IsUint64() { - return errExtDataGasUsedTooLarge - } - return nil -} - -// modified from consensus.go -func (eng *DummyEngine) verifyHeader(chain consensus.ChainHeaderReader, header *types.Header, parent *types.Header, uncle bool) error { - // Ensure that we do not verify an uncle - if uncle { - return errUnclesUnsupported - } - - // Verify the extra data is well-formed. - config := chain.Config() - configExtra := params.GetExtra(config) - rules := configExtra.GetAvalancheRules(header.Time) - if err := customheader.VerifyExtra(rules, header.Extra); err != nil { - return err - } - - // Ensure gas-related header fields are correct - if err := verifyHeaderGasFields(configExtra, header, parent); err != nil { - return err - } - - // Verify the header's timestamp - if header.Time > uint64(eng.clock.Time().Add(allowedFutureBlockTime).Unix()) { - return consensus.ErrFutureBlock - } - // Verify the header's timestamp is not earlier than parent's - // it does include equality(==), so multiple blocks per second is ok - if header.Time < parent.Time { - return errInvalidBlockTime - } - // Verify that the block number is parent's +1 - if diff := new(big.Int).Sub(header.Number, parent.Number); diff.Cmp(big.NewInt(1)) != 0 { - return consensus.ErrInvalidNumber - } - // Verify the existence / non-existence of excessBlobGas - cancun := config.IsCancun(header.Number, header.Time) - if !cancun { - switch { - case header.ExcessBlobGas != nil: - return fmt.Errorf("invalid excessBlobGas: have %d, expected nil", *header.ExcessBlobGas) - case header.BlobGasUsed != nil: - return fmt.Errorf("invalid blobGasUsed: have %d, expected nil", *header.BlobGasUsed) - case header.ParentBeaconRoot != nil: - return fmt.Errorf("invalid parentBeaconRoot, have %#x, expected nil", *header.ParentBeaconRoot) - } - } else { - if header.ParentBeaconRoot == nil { - return errors.New("header is missing beaconRoot") - } - if *header.ParentBeaconRoot != (common.Hash{}) { - return fmt.Errorf("invalid parentBeaconRoot, have %#x, expected empty", *header.ParentBeaconRoot) - } - if err := eip4844.VerifyEIP4844Header(parent, header); err != nil { - return err - } - if *header.BlobGasUsed > 0 { // VerifyEIP4844Header ensures BlobGasUsed is non-nil - return fmt.Errorf("blobs not enabled on avalanche networks: used %d blob gas, expected 0", *header.BlobGasUsed) - } - } - return nil -} - func (*DummyEngine) Author(header *types.Header) (common.Address, error) { return header.Coinbase, nil } func (eng *DummyEngine) VerifyHeader(chain consensus.ChainHeaderReader, header *types.Header) error { - // If we're running a full engine faking, accept any input as valid - if eng.consensusMode.ModeSkipHeader { - return nil - } - // Short circuit if the header is known, or it's parent not - number := header.Number.Uint64() - if chain.GetHeader(header.Hash(), number) != nil { - return nil - } - parent := chain.GetHeader(header.ParentHash, number-1) - if parent == nil { - return consensus.ErrUnknownAncestor - } - // Sanity checks passed, do a proper verification - return eng.verifyHeader(chain, header, parent, false) + return nil } func (*DummyEngine) VerifyUncles(chain consensus.ChainReader, block *types.Block) error { @@ -367,11 +231,11 @@ func (eng *DummyEngine) verifyBlockFee( func (eng *DummyEngine) Finalize(chain consensus.ChainHeaderReader, block *types.Block, parent *types.Header, state *state.StateDB, receipts []*types.Receipt) error { // Perform extra state change while finalizing the block var ( - contribution, extDataGasUsed *big.Int - err error + contribution *big.Int + err error ) if eng.cb.OnExtraStateChange != nil { - contribution, extDataGasUsed, err = eng.cb.OnExtraStateChange(block, parent, state) + contribution, err = eng.cb.OnExtraStateChange(block, parent, state) if err != nil { return err } @@ -379,29 +243,9 @@ func (eng *DummyEngine) Finalize(chain consensus.ChainHeaderReader, block *types config := params.GetExtra(chain.Config()) timestamp := block.Time() - // Verify the BlockGasCost set in the header matches the expected value. - blockGasCost := customtypes.BlockGasCost(block) - expectedBlockGasCost := customheader.BlockGasCost( - config, - parent, - timestamp, - ) - if !utils.BigEqual(blockGasCost, expectedBlockGasCost) { - return fmt.Errorf("invalid blockGasCost: have %d, want %d", blockGasCost, expectedBlockGasCost) - } if config.IsApricotPhase4(timestamp) { - // Validate extDataGasUsed and BlockGasCost match expectations - // - // NOTE: This is a duplicate check of what is already performed in - // blockValidator but is done here for symmetry with FinalizeAndAssemble. - if extDataGasUsed == nil { - extDataGasUsed = new(big.Int).Set(common.Big0) - } - if blockExtDataGasUsed := customtypes.BlockExtDataGasUsed(block); blockExtDataGasUsed == nil || !blockExtDataGasUsed.IsUint64() || blockExtDataGasUsed.Cmp(extDataGasUsed) != 0 { - return fmt.Errorf("invalid extDataGasUsed: have %d, want %d", blockExtDataGasUsed, extDataGasUsed) - } - // Verify the block fee was paid. + blockGasCost := customtypes.BlockGasCost(block) if err := eng.verifyBlockFee( block.BaseFee(), blockGasCost, @@ -420,9 +264,10 @@ func (eng *DummyEngine) FinalizeAndAssemble(chain consensus.ChainHeaderReader, h uncles []*types.Header, receipts []*types.Receipt, ) (*types.Block, error) { var ( - contribution, extDataGasUsed *big.Int - extraData []byte - err error + contribution *big.Int + extDataGasUsed uint64 + extraData []byte + err error ) if eng.cb.OnFinalizeAndAssemble != nil { extraData, contribution, extDataGasUsed, err = eng.cb.OnFinalizeAndAssemble(header, parent, state, txs) @@ -440,10 +285,7 @@ func (eng *DummyEngine) FinalizeAndAssemble(chain consensus.ChainHeaderReader, h header.Time, ) if configExtra.IsApricotPhase4(header.Time) { - headerExtra.ExtDataGasUsed = extDataGasUsed - if headerExtra.ExtDataGasUsed == nil { - headerExtra.ExtDataGasUsed = new(big.Int) - } + headerExtra.ExtDataGasUsed = new(big.Int).SetUint64(extDataGasUsed) // Verify that this block covers the block fee. if err := eng.verifyBlockFee( diff --git a/core/block_validator.go b/core/block_validator.go index f1c9922834..c1dd777fdb 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -28,7 +28,6 @@ package core import ( - "errors" "fmt" "github.com/ava-labs/coreth/consensus" @@ -63,57 +62,6 @@ func NewBlockValidator(config *params.ChainConfig, blockchain *BlockChain, engin // header's transaction and uncle roots. The headers are assumed to be already // validated at this point. func (v *BlockValidator) ValidateBody(block *types.Block) error { - // Check whether the block is already imported. - if v.bc.HasBlockAndState(block.Hash(), block.NumberU64()) { - return ErrKnownBlock - } - - // Header validity is known at this point. Here we verify that uncle and transactions - // given in the block body match the header. - header := block.Header() - if err := v.engine.VerifyUncles(v.bc, block); err != nil { - return err - } - if hash := types.CalcUncleHash(block.Uncles()); hash != header.UncleHash { - return fmt.Errorf("uncle root hash mismatch (header value %x, calculated %x)", header.UncleHash, hash) - } - if hash := types.DeriveSha(block.Transactions(), trie.NewStackTrie(nil)); hash != header.TxHash { - return fmt.Errorf("transaction root hash mismatch (header value %x, calculated %x)", header.TxHash, hash) - } - - // Blob transactions may be present after the Cancun fork. - var blobs int - for i, tx := range block.Transactions() { - // Count the number of blobs to validate against the header's blobGasUsed - blobs += len(tx.BlobHashes()) - - // If the tx is a blob tx, it must NOT have a sidecar attached to be valid in a block. - if tx.BlobTxSidecar() != nil { - return fmt.Errorf("unexpected blob sidecar in transaction at index %d", i) - } - - // The individual checks for blob validity (version-check + not empty) - // happens in StateTransition. - } - - // Check blob gas usage. - if header.BlobGasUsed != nil { - if want := *header.BlobGasUsed / params.BlobTxBlobGasPerBlob; uint64(blobs) != want { // div because the header is surely good vs the body might be bloated - return fmt.Errorf("blob gas used mismatch (header %v, calculated %v)", *header.BlobGasUsed, blobs*params.BlobTxBlobGasPerBlob) - } - } else { - if blobs > 0 { - return errors.New("data blobs present in block body") - } - } - - // Ancestor block must be known. - if !v.bc.HasBlockAndState(block.ParentHash(), block.NumberU64()-1) { - if !v.bc.HasBlock(block.ParentHash(), block.NumberU64()-1) { - return consensus.ErrUnknownAncestor - } - return consensus.ErrPrunedAncestor - } return nil } diff --git a/core/blockchain.go b/core/blockchain.go index 364cff6cab..60e0bf3466 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -124,7 +124,6 @@ var ( ErrRefuseToCorruptArchiver = errors.New("node has operated with pruning disabled, shutting down to prevent missing tries") - errFutureBlockUnsupported = errors.New("future block insertion not supported") errCacheConfigNotSpecified = errors.New("must specify cache config") errInvalidOldChain = errors.New("invalid old chain") errInvalidNewChain = errors.New("invalid new chain") @@ -1284,40 +1283,11 @@ func (bc *BlockChain) InsertBlockManual(block *types.Block, writes bool) error { func (bc *BlockChain) insertBlock(block *types.Block, writes bool) error { start := time.Now() - bc.senderCacher.Recover(types.MakeSigner(bc.chainConfig, block.Number(), block.Time()), block.Transactions()) + signer := types.MakeSigner(bc.chainConfig, block.Number(), block.Time()) + bc.senderCacher.Recover(signer, block.Transactions()) substart := time.Now() - err := bc.engine.VerifyHeader(bc, block.Header()) - if err == nil { - err = bc.validator.ValidateBody(block) - } - - switch { - case errors.Is(err, ErrKnownBlock): - // even if the block is already known, we still need to generate the - // snapshot layer and add a reference to the triedb, so we re-execute - // the block. Note that insertBlock should only be called on a block - // once if it returns nil - if bc.newTip(block) { - log.Debug("Setting head to be known block", "number", block.Number(), "hash", block.Hash()) - } else { - log.Debug("Reprocessing already known block", "number", block.Number(), "hash", block.Hash()) - } - - // If an ancestor has been pruned, then this block cannot be acceptable. - case errors.Is(err, consensus.ErrPrunedAncestor): - return errors.New("side chain insertion is not supported") - - // Future blocks are not supported, but should not be reported, so we return an error - // early here - case errors.Is(err, consensus.ErrFutureBlock): - return errFutureBlockUnsupported - - // Some other error occurred, abort - case err != nil: - bc.reportBlock(block, nil, err) - return err - } + blockContentValidationTimer.Inc(time.Since(substart).Milliseconds()) // No validation errors for the block @@ -1402,11 +1372,17 @@ func (bc *BlockChain) insertBlock(block *types.Block, writes bool) error { blockWriteTimer.Inc((time.Since(wstart) - statedb.AccountCommits - statedb.StorageCommits - statedb.SnapshotCommits - statedb.TrieDBCommits).Milliseconds()) blockInsertTimer.Inc(time.Since(start).Milliseconds()) - log.Debug("Inserted new block", "number", block.Number(), "hash", block.Hash(), + log.Debug("Inserted new block", + "number", block.Number(), + "hash", block.Hash(), "parentHash", block.ParentHash(), - "uncles", len(block.Uncles()), "txs", len(block.Transactions()), "gas", block.GasUsed(), + "uncles", len(block.Uncles()), + "txs", len(block.Transactions()), + "gas", block.GasUsed(), "elapsed", common.PrettyDuration(time.Since(start)), - "root", block.Root(), "baseFeePerGas", block.BaseFee(), "blockGasCost", customtypes.BlockGasCost(block), + "root", block.Root(), + "baseFeePerGas", block.BaseFee(), + "blockGasCost", customtypes.BlockGasCost(block), ) processedBlockGasUsedCounter.Inc(int64(block.GasUsed())) diff --git a/core/blockchain_ext_test.go b/core/blockchain_ext_test.go index f03917d5f5..c0e6bf823a 100644 --- a/core/blockchain_ext_test.go +++ b/core/blockchain_ext_test.go @@ -22,18 +22,18 @@ import ( ) var TestCallbacks = dummy.ConsensusCallbacks{ - OnExtraStateChange: func(block *types.Block, _ *types.Header, sdb *state.StateDB) (*big.Int, *big.Int, error) { + OnExtraStateChange: func(block *types.Block, _ *types.Header, sdb *state.StateDB) (*big.Int, error) { sdb.AddBalanceMultiCoin(common.HexToAddress("0xdeadbeef"), common.HexToHash("0xdeadbeef"), big.NewInt(block.Number().Int64())) - return nil, nil, nil + return nil, nil }, OnFinalizeAndAssemble: func( header *types.Header, _ *types.Header, sdb *state.StateDB, _ []*types.Transaction, - ) ([]byte, *big.Int, *big.Int, error) { + ) ([]byte, *big.Int, uint64, error) { sdb.AddBalanceMultiCoin(common.HexToAddress("0xdeadbeef"), common.HexToHash("0xdeadbeef"), big.NewInt(header.Number.Int64())) - return nil, nil, nil, nil + return nil, nil, 0, nil }, } diff --git a/core/state_processor.go b/core/state_processor.go index 2a4e8d05c4..ba4857bbb9 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -164,7 +164,7 @@ func applyTransaction(msg *Message, config *params.ChainConfig, gp *GasPool, sta receipt.BlockHash = blockHash receipt.BlockNumber = blockNumber receipt.TransactionIndex = uint(statedb.TxIndex()) - return receipt, err + return receipt, nil } // ApplyTransaction attempts to apply a transaction to the given state database diff --git a/eth/gasprice/fee_info_provider_test.go b/eth/gasprice/fee_info_provider_test.go index 9d5260d95f..15510d404a 100644 --- a/eth/gasprice/fee_info_provider_test.go +++ b/eth/gasprice/fee_info_provider_test.go @@ -11,13 +11,12 @@ import ( "github.com/ava-labs/coreth/core" "github.com/ava-labs/coreth/params" - "github.com/ava-labs/libevm/common" "github.com/ava-labs/libevm/core/types" "github.com/stretchr/testify/require" ) func TestFeeInfoProvider(t *testing.T) { - backend := newTestBackend(t, params.TestChainConfig, 2, common.Big0, testGenBlock(t, 55, 80)) + backend := newTestBackend(t, params.TestChainConfig, 2, 0, testGenBlock(t, 55, 80)) f, err := newFeeInfoProvider(backend, 1, 2) require.NoError(t, err) @@ -45,7 +44,7 @@ func TestFeeInfoProvider(t *testing.T) { func TestFeeInfoProviderCacheSize(t *testing.T) { size := 5 overflow := 3 - backend := newTestBackend(t, params.TestChainConfig, 0, common.Big0, testGenBlock(t, 55, 370)) + backend := newTestBackend(t, params.TestChainConfig, 0, 0, testGenBlock(t, 55, 370)) f, err := newFeeInfoProvider(backend, 1, size) require.NoError(t, err) diff --git a/eth/gasprice/gasprice_test.go b/eth/gasprice/gasprice_test.go index 90de7177cd..40927c6bb4 100644 --- a/eth/gasprice/gasprice_test.go +++ b/eth/gasprice/gasprice_test.go @@ -124,18 +124,18 @@ func newTestBackendFakerEngine(t *testing.T, config *params.ChainConfig, numBloc // newTestBackend creates a test backend. OBS: don't forget to invoke tearDown // after use, otherwise the blockchain instance will mem-leak via goroutines. -func newTestBackend(t *testing.T, config *params.ChainConfig, numBlocks int, extDataGasUsage *big.Int, genBlocks func(i int, b *core.BlockGen)) *testBackend { +func newTestBackend(t *testing.T, config *params.ChainConfig, numBlocks int, extDataGasUsage uint64, genBlocks func(i int, b *core.BlockGen)) *testBackend { var gspec = &core.Genesis{ Config: config, Alloc: types.GenesisAlloc{addr: {Balance: bal}}, } engine := dummy.NewFakerWithCallbacks(dummy.ConsensusCallbacks{ - OnFinalizeAndAssemble: func(*types.Header, *types.Header, *state.StateDB, []*types.Transaction) ([]byte, *big.Int, *big.Int, error) { + OnFinalizeAndAssemble: func(*types.Header, *types.Header, *state.StateDB, []*types.Transaction) ([]byte, *big.Int, uint64, error) { return nil, common.Big0, extDataGasUsage, nil }, - OnExtraStateChange: func(*types.Block, *types.Header, *state.StateDB) (*big.Int, *big.Int, error) { - return common.Big0, extDataGasUsage, nil + OnExtraStateChange: func(*types.Block, *types.Header, *state.StateDB) (*big.Int, error) { + return common.Big0, nil }, }) @@ -179,7 +179,7 @@ func (b *testBackend) GetBlockByNumber(number uint64) *types.Block { type suggestTipCapTest struct { chainConfig *params.ChainConfig numBlocks int - extDataGasUsage *big.Int + extDataGasUsage uint64 genBlock func(i int, b *core.BlockGen) expectedTip *big.Int } @@ -252,7 +252,7 @@ func TestSuggestTipCapEmptyExtDataGasUsage(t *testing.T) { applyGasPriceTest(t, suggestTipCapTest{ chainConfig: params.TestChainConfig, numBlocks: 3, - extDataGasUsage: nil, + extDataGasUsage: 0, genBlock: testGenBlock(t, 55, 80), expectedTip: big.NewInt(1), }, defaultOracleConfig()) @@ -262,7 +262,7 @@ func TestSuggestTipCapSimple(t *testing.T) { applyGasPriceTest(t, suggestTipCapTest{ chainConfig: params.TestChainConfig, numBlocks: 3, - extDataGasUsage: common.Big0, + extDataGasUsage: 0, genBlock: testGenBlock(t, 55, 80), expectedTip: big.NewInt(1), }, defaultOracleConfig()) @@ -272,7 +272,7 @@ func TestSuggestTipCapSimpleFloor(t *testing.T) { applyGasPriceTest(t, suggestTipCapTest{ chainConfig: params.TestChainConfig, numBlocks: 1, - extDataGasUsage: common.Big0, + extDataGasUsage: 0, genBlock: testGenBlock(t, 55, 80), expectedTip: big.NewInt(1), }, defaultOracleConfig()) @@ -283,7 +283,7 @@ func TestSuggestTipCapSmallTips(t *testing.T) { applyGasPriceTest(t, suggestTipCapTest{ chainConfig: params.TestChainConfig, numBlocks: 3, - extDataGasUsage: common.Big0, + extDataGasUsage: 0, genBlock: func(i int, b *core.BlockGen) { b.SetCoinbase(common.Address{1}) @@ -328,7 +328,7 @@ func TestSuggestTipCapExtDataUsage(t *testing.T) { applyGasPriceTest(t, suggestTipCapTest{ chainConfig: params.TestChainConfig, numBlocks: 3, - extDataGasUsage: big.NewInt(10_000), + extDataGasUsage: 10_000, genBlock: testGenBlock(t, 55, 80), expectedTip: big.NewInt(1), }, defaultOracleConfig()) @@ -338,7 +338,7 @@ func TestSuggestTipCapMinGas(t *testing.T) { applyGasPriceTest(t, suggestTipCapTest{ chainConfig: params.TestChainConfig, numBlocks: 3, - extDataGasUsage: common.Big0, + extDataGasUsage: 0, genBlock: testGenBlock(t, 500, 50), expectedTip: big.NewInt(1), }, defaultOracleConfig()) @@ -353,7 +353,7 @@ func TestSuggestGasPricePreAP3(t *testing.T) { Percentile: 60, } - backend := newTestBackend(t, params.TestApricotPhase2Config, 3, nil, func(i int, b *core.BlockGen) { + backend := newTestBackend(t, params.TestApricotPhase2Config, 3, 0, func(i int, b *core.BlockGen) { b.SetCoinbase(common.Address{1}) signer := types.LatestSigner(params.TestApricotPhase2Config) @@ -384,7 +384,7 @@ func TestSuggestTipCapMaxBlocksLookback(t *testing.T) { applyGasPriceTest(t, suggestTipCapTest{ chainConfig: params.TestChainConfig, numBlocks: 200, - extDataGasUsage: common.Big0, + extDataGasUsage: 0, genBlock: testGenBlock(t, 550, 80), expectedTip: big.NewInt(3), }, defaultOracleConfig()) @@ -394,7 +394,7 @@ func TestSuggestTipCapMaxBlocksSecondsLookback(t *testing.T) { applyGasPriceTest(t, suggestTipCapTest{ chainConfig: params.TestChainConfig, numBlocks: 20, - extDataGasUsage: big.NewInt(1), + extDataGasUsage: 1, genBlock: testGenBlock(t, 550, 80), expectedTip: big.NewInt(1), }, timeCrunchOracleConfig()) @@ -404,7 +404,7 @@ func TestSuggestTipCapIncludesExtraDataGas(t *testing.T) { applyGasPriceTest(t, suggestTipCapTest{ chainConfig: params.TestChainConfig, numBlocks: 1000, - extDataGasUsage: big.NewInt(acp176.MinMaxPerSecond - int64(params.TxGas)), + extDataGasUsage: acp176.MinMaxPerSecond - params.TxGas, // The tip on the transaction is very large to pay the block gas cost. genBlock: testGenBlock(t, 100_000, 1), // The actual tip doesn't matter, we just want to ensure that the tip is diff --git a/plugin/evm/atomic/export_tx.go b/plugin/evm/atomic/export_tx.go index 911e0b8bc2..46c6a9cbf9 100644 --- a/plugin/evm/atomic/export_tx.go +++ b/plugin/evm/atomic/export_tx.go @@ -311,7 +311,7 @@ func NewExportTx( // EVMStateTransfer executes the state update from the atomic export transaction func (utx *UnsignedExportTx) EVMStateTransfer(ctx *snow.Context, state StateDB) error { - addrs := map[[20]byte]uint64{} + addrs := map[common.Address]uint64{} for _, from := range utx.Ins { if from.AssetID == ctx.AVAXAssetID { log.Debug("export_tx", "dest", utx.DestinationChain, "addr", from.Address, "amount", from.Amount, "assetID", "AVAX") diff --git a/plugin/evm/atomic/tx.go b/plugin/evm/atomic/tx.go index ec92e61822..94be151022 100644 --- a/plugin/evm/atomic/tx.go +++ b/plugin/evm/atomic/tx.go @@ -230,34 +230,34 @@ func (tx *Tx) Sign(c codec.Manager, signers [][]*secp256k1.PrivateKey) error { // for via this transaction denominated in [avaxAssetID] with [baseFee] used to calculate the // cost of this transaction. This function also returns the [gasUsed] by the // transaction for inclusion in the [baseFee] algorithm. -func (tx *Tx) BlockFeeContribution(fixedFee bool, avaxAssetID ids.ID, baseFee *big.Int) (*big.Int, *big.Int, error) { +func (tx *Tx) BlockFeeContribution(fixedFee bool, avaxAssetID ids.ID, baseFee *big.Int) (*big.Int, uint64, error) { if baseFee == nil { - return nil, nil, errNilBaseFee + return nil, 0, errNilBaseFee } if baseFee.Cmp(common.Big0) <= 0 { - return nil, nil, fmt.Errorf("cannot calculate tip with base fee %d <= 0", baseFee) + return nil, 0, fmt.Errorf("cannot calculate tip with base fee %d <= 0", baseFee) } gasUsed, err := tx.GasUsed(fixedFee) if err != nil { - return nil, nil, err + return nil, 0, err } txFee, err := CalculateDynamicFee(gasUsed, baseFee) if err != nil { - return nil, nil, err + return nil, 0, err } burned, err := tx.Burned(avaxAssetID) if err != nil { - return nil, nil, err + return nil, 0, err } if txFee > burned { - return nil, nil, fmt.Errorf("insufficient AVAX burned (%d) to cover import tx fee (%d)", burned, txFee) + return nil, 0, fmt.Errorf("insufficient AVAX burned (%d) to cover import tx fee (%d)", burned, txFee) } excessBurned := burned - txFee // Calculate the amount of AVAX that has been burned above the required fee denominated // in C-Chain native 18 decimal places blockFeeContribution := new(big.Int).Mul(new(big.Int).SetUint64(excessBurned), X2CRate.ToBig()) - return blockFeeContribution, new(big.Int).SetUint64(gasUsed), nil + return blockFeeContribution, gasUsed, nil } func (tx *Tx) GossipID() ids.ID { diff --git a/plugin/evm/atomic/tx_semantic_verifier.go b/plugin/evm/atomic/tx_semantic_verifier.go index 2acd4ad93f..8bb47df753 100644 --- a/plugin/evm/atomic/tx_semantic_verifier.go +++ b/plugin/evm/atomic/tx_semantic_verifier.go @@ -21,7 +21,7 @@ import ( "github.com/ava-labs/coreth/plugin/evm/upgrade/ap0" ) -var _ Visitor = (*SemanticVerifier)(nil) +var _ Visitor = (*semanticVerifier)(nil) var ( ErrAssetIDMismatch = errors.New("asset IDs in the input don't match the utxo") @@ -44,20 +44,30 @@ type VerifierBackend struct { SecpCache *secp256k1.RecoverCache } -// SemanticVerifier is a visitor that checks the semantic validity of atomic transactions. -type SemanticVerifier struct { - Backend *VerifierBackend - Tx *Tx - Parent AtomicBlockContext - BaseFee *big.Int +// SemanticVerify checks the semantic validity of atomic transactions. +func (b *VerifierBackend) SemanticVerify(tx *Tx, parent AtomicBlockContext, baseFee *big.Int) error { + return tx.UnsignedAtomicTx.Visit(&semanticVerifier{ + backend: b, + tx: tx, + parent: parent, + baseFee: baseFee, + }) } -// ImportTx verifies this transaction is valid. -func (s *SemanticVerifier) ImportTx(utx *UnsignedImportTx) error { - backend := s.Backend +// semanticVerifier is a visitor that checks the semantic validity of atomic +// transactions. +type semanticVerifier struct { + backend *VerifierBackend + tx *Tx + parent AtomicBlockContext + baseFee *big.Int +} + +func (s *semanticVerifier) ImportTx(utx *UnsignedImportTx) error { + backend := s.backend ctx := backend.Ctx rules := backend.Rules - stx := s.Tx + stx := s.tx if err := utx.Verify(ctx, rules); err != nil { return err } @@ -71,7 +81,7 @@ func (s *SemanticVerifier) ImportTx(utx *UnsignedImportTx) error { if err != nil { return err } - txFee, err := CalculateDynamicFee(gasUsed, s.BaseFee) + txFee, err := CalculateDynamicFee(gasUsed, s.baseFee) if err != nil { return err } @@ -133,7 +143,7 @@ func (s *SemanticVerifier) ImportTx(utx *UnsignedImportTx) error { } } - return conflicts(backend, utx.InputUTXOs(), s.Parent) + return conflicts(backend, utx.InputUTXOs(), s.parent) } // conflicts returns an error if [inputs] conflicts with any of the atomic inputs contained in [ancestor] @@ -176,12 +186,11 @@ func conflicts(backend *VerifierBackend, inputs set.Set[ids.ID], ancestor Atomic return nil } -// ExportTx verifies this transaction is valid. -func (s *SemanticVerifier) ExportTx(utx *UnsignedExportTx) error { - backend := s.Backend +func (s *semanticVerifier) ExportTx(utx *UnsignedExportTx) error { + backend := s.backend ctx := backend.Ctx rules := backend.Rules - stx := s.Tx + stx := s.tx if err := utx.Verify(ctx, rules); err != nil { return err } @@ -195,7 +204,7 @@ func (s *SemanticVerifier) ExportTx(utx *UnsignedExportTx) error { if err != nil { return err } - txFee, err := CalculateDynamicFee(gasUsed, s.BaseFee) + txFee, err := CalculateDynamicFee(gasUsed, s.baseFee) if err != nil { return err } @@ -231,7 +240,7 @@ func (s *SemanticVerifier) ExportTx(utx *UnsignedExportTx) error { if len(cred.Sigs) != 1 { return fmt.Errorf("expected one signature for EVM Input Credential, but found: %d", len(cred.Sigs)) } - pubKey, err := s.Backend.SecpCache.RecoverPublicKey(utx.Bytes(), cred.Sigs[0][:]) + pubKey, err := s.backend.SecpCache.RecoverPublicKey(utx.Bytes(), cred.Sigs[0][:]) if err != nil { return err } diff --git a/plugin/evm/block.go b/plugin/evm/block.go index ffa182c881..cadb63854a 100644 --- a/plugin/evm/block.go +++ b/plugin/evm/block.go @@ -14,6 +14,7 @@ import ( "github.com/ava-labs/libevm/log" "github.com/ava-labs/libevm/rlp" + "github.com/ava-labs/coreth/consensus" "github.com/ava-labs/coreth/core" "github.com/ava-labs/coreth/params" "github.com/ava-labs/coreth/params/extras" @@ -22,12 +23,15 @@ import ( "github.com/ava-labs/coreth/plugin/evm/header" "github.com/ava-labs/coreth/precompile/precompileconfig" "github.com/ava-labs/coreth/predicate" + "github.com/ava-labs/coreth/utils" "github.com/ava-labs/libevm/core/rawdb" "github.com/ava-labs/libevm/core/types" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/consensus/snowman" "github.com/ava-labs/avalanchego/snow/engine/snowman/block" + + customheader "github.com/ava-labs/coreth/plugin/evm/header" ) var ( @@ -113,7 +117,6 @@ func readMainnetBonusBlocks() (map[uint64]ids.ID, error) { // Block implements the snowman.Block interface type Block struct { - id ids.ID ethBlock *types.Block vm *VM atomicTxs []*atomic.Tx @@ -128,7 +131,6 @@ func (vm *VM) newBlock(ethBlock *types.Block) (*Block, error) { } return &Block{ - id: ids.ID(ethBlock.Hash()), ethBlock: ethBlock, vm: vm, atomicTxs: atomicTxs, @@ -136,7 +138,7 @@ func (vm *VM) newBlock(ethBlock *types.Block) (*Block, error) { } // ID implements the snowman.Block interface -func (b *Block) ID() ids.ID { return b.id } +func (b *Block) ID() ids.ID { return ids.ID(b.ethBlock.Hash()) } func (b *Block) AtomicTxs() []*atomic.Tx { return b.atomicTxs } @@ -271,17 +273,6 @@ func (b *Block) Timestamp() time.Time { return time.Unix(int64(b.ethBlock.Time()), 0) } -// syntacticVerify verifies that a *Block is well-formed. -func (b *Block) syntacticVerify() error { - if b == nil || b.ethBlock == nil { - return errInvalidBlock - } - - header := b.ethBlock.Header() - rules := b.vm.chainConfig.Rules(header.Number, params.IsMergeTODO, header.Time) - return b.vm.syntacticBlockValidator.SyntacticVerify(b, rules) -} - // Verify implements the snowman.Block interface func (b *Block) Verify(context.Context) error { return b.verify(&precompileconfig.PredicateContext{ @@ -325,54 +316,168 @@ func (b *Block) VerifyWithContext(ctx context.Context, proposerVMBlockCtx *block // Verify the block is valid. // Enforces that the predicates are valid within [predicateContext]. // Writes the block details to disk and the state to the trie manager iff writes=true. -func (b *Block) verify(predicateContext *precompileconfig.PredicateContext, writes bool) error { - if predicateContext.ProposerVMBlockCtx != nil { - log.Debug("Verifying block with context", "block", b.ID(), "height", b.Height()) - } else { - log.Debug("Verifying block without context", "block", b.ID(), "height", b.Height()) +func (b *Block) verify(context *precompileconfig.PredicateContext, writes bool) error { + hash := b.ethBlock.Hash() + log.Debug("verifying block", + "hash", hash, + "height", b.ethBlock.NumberU64(), + "hasContext", context.ProposerVMBlockCtx != nil, + "writes", writes, + ) + + if err := b.verifyWithoutParent(); err != nil { + return fmt.Errorf("verifyWithoutParent: %w", err) } - if err := b.syntacticVerify(); err != nil { - return fmt.Errorf("syntactic block verification failed: %w", err) + if err := b.verifyContext(context); err != nil { + return fmt.Errorf("verifyCanExecute: %w", err) } + // The engine may call VerifyWithContext multiple times on the same block + // with different contexts. Since the engine will only call Accept/Reject + // once, we should only call InsertBlockManual once. Additionally, if a + // block is already in processing, then it has already passed verification + // and at this point we have checked the predicates are still valid in the + // different context so we can return nil. + if b.vm.State.IsProcessing(ids.ID(hash)) { + return nil + } + + if err := b.verifyCanExecute(); err != nil { + return fmt.Errorf("verifyCanExecute: %w", err) + } + if err := b.execute(context, writes); err != nil { + return fmt.Errorf("execute: %w", err) + } + return nil +} + +func (b *Block) verifyWithoutParent() error { + return verifyBlockStandalone( + b.vm.syntacticBlockValidator.extDataHashes, + &b.vm.clock, + b.vm.chainConfig, + b.ethBlock, + b.atomicTxs, + ) +} + +// Assumes [Block.verifyWithoutParent] has returned nil. +func (b *Block) verifyContext(context *precompileconfig.PredicateContext) error { // If the VM is not marked as bootstrapped the other chains may also be // bootstrapping and not have populated the required indices. Since // bootstrapping only verifies blocks that have been canonically accepted by // the network, these checks would be guaranteed to pass on a synced node. - if b.vm.bootstrapped.Get() { - // Verify that the UTXOs named in import txs are present in shared - // memory. - // - // This does not fully verify that this block can spend these UTXOs. - // However, it guarantees that any block that fails the later checks was - // built by an incorrect block proposer. This ensures that we only mark - // blocks as BAD BLOCKs if they were incorrectly generated. - if err := b.verifyUTXOsPresent(); err != nil { - return err - } + if !b.vm.bootstrapped.Get() { + return nil + } - // Verify that all the ICM messages are correctly marked as either valid - // or invalid. - if err := b.verifyPredicates(predicateContext); err != nil { - return fmt.Errorf("failed to verify predicates: %w", err) - } + // Verify that the atomic txs in this block are valid to be executed. + if err := b.verifyAtomicTxs(); err != nil { + return err } - // The engine may call VerifyWithContext multiple times on the same block with different contexts. - // Since the engine will only call Accept/Reject once, we should only call InsertBlockManual once. - // Additionally, if a block is already in processing, then it has already passed verification and - // at this point we have checked the predicates are still valid in the different context so we - // can return nil. - if b.vm.State.IsProcessing(b.id) { - return nil + // Verify that all the ICM messages are correctly marked as either valid + // or invalid. + if err := b.verifyPredicates(context); err != nil { + return fmt.Errorf("failed to verify predicates: %w", err) + } + return nil +} + +// Assumes [Block.verifyContext] has returned nil. +func (b *Block) verifyCanExecute() error { + header := b.ethBlock.Header() + number := header.Number.Uint64() + parentHeader := b.vm.blockChain.GetHeader(header.ParentHash, number-1) + if parentHeader == nil { + return consensus.ErrUnknownAncestor + } + + parentNumber := parentHeader.Number.Uint64() + if expectedNumber := parentNumber + 1; number != expectedNumber { + return fmt.Errorf("expected number %d, found %d", expectedNumber, number) + } + + // Verify the header's timestamp is not earlier than parent's. It allows + // equality(==), multiple blocks can have the same timestamp. + if header.Time < parentHeader.Time { + return fmt.Errorf("invalid timestamp %d < parent timestamp %d", header.Time, parentHeader.Time) + } + + config := params.GetExtra(b.vm.chainConfig) + if err := customheader.VerifyGasLimit(config, parentHeader, header); err != nil { + return err + } + + expectedBaseFee, err := customheader.BaseFee(config, parentHeader, header.Time) + if err != nil { + return fmt.Errorf("failed to calculate base fee: %w", err) + } + if !utils.BigEqual(header.BaseFee, expectedBaseFee) { + return fmt.Errorf("expected base fee %d, found %d", expectedBaseFee, header.BaseFee) + } + + // Verify the BlockGasCost set in the header matches the expected value. + expectedBlockGasCost := customheader.BlockGasCost( + config, + parentHeader, + header.Time, + ) + headerExtra := customtypes.GetHeaderExtra(header) + if !utils.BigEqual(headerExtra.BlockGasCost, expectedBlockGasCost) { + return fmt.Errorf("invalid blockGasCost: have %d, want %d", headerExtra.BlockGasCost, expectedBlockGasCost) + } + + // These checks assume that GasUsed is set correctly. GasUsed still needs to + // be verified after execution. + if err := customheader.VerifyGasUsed(config, parentHeader, header); err != nil { + return err + } + if err := customheader.VerifyExtraPrefix(config, parentHeader, header); err != nil { + return err + } + + // Ancestor block must be known. + if !b.vm.blockChain.HasBlockAndState(header.ParentHash, parentNumber) { + return fmt.Errorf("parent block or state not available: hash=%x, number=%d", header.ParentHash, parentNumber) + } + + // Things still left to be verified: + // - Root hash + // - ReceiptHash + // - Bloom + // - GasUsed + // - BlockGasCost is paid for + // - Normal eth tx checks: + // - Sender can be recovered correctly (verifies chainID) + // - Tx nonces are correct (matches what is in state and won't cause an overflow) + // - Tx can afford the gas based on the gas limit and effective gas price + // - Tx doesn't specify a GasLimit which exceeds the gas remaining allowed to process in the block + // - Tx is able to send the value of the transaction from the sender after purchasing the gas + return nil +} + +// Assumes [Block.verifyCanExecute] has returned nil. +func (b *Block) execute(context *precompileconfig.PredicateContext, writes bool) error { + hash := b.ethBlock.Hash() + if writes { + // Update the atomic backend with [txs] from this block. + // + // Note: The atomic trie canonically contains the duplicate operations from + // any bonus blocks. + var ( + number = b.ethBlock.NumberU64() + parentHash = b.ethBlock.ParentHash() + ) + if _, err := b.vm.atomicBackend.InsertTxs(hash, number, parentHash, b.atomicTxs); err != nil { + return err + } } err := b.vm.blockChain.InsertBlockManual(b.ethBlock, writes) - if err != nil || !writes { - // if an error occurred inserting the block into the chain - // or if we are not pinning to memory, unpin the atomic trie - // changes from memory (if they were pinned). - if atomicState, err := b.vm.atomicBackend.GetVerifiedAtomicState(b.ethBlock.Hash()); err == nil { + if err != nil && writes { + // Unpin the atomic trie if it is pinned and verification failed. + if atomicState, err := b.vm.atomicBackend.GetVerifiedAtomicState(hash); err == nil { _ = atomicState.Reject() // ignore this error so we can return the original error instead. } } @@ -413,26 +518,27 @@ func (b *Block) verifyPredicates(predicateContext *precompileconfig.PredicateCon return nil } -// verifyUTXOsPresent verifies all atomic UTXOs consumed by the block are -// present in shared memory. -func (b *Block) verifyUTXOsPresent() error { - blockHash := common.Hash(b.ID()) - if b.vm.atomicBackend.IsBonus(b.Height(), blockHash) { - log.Info("skipping atomic tx verification on bonus block", "block", blockHash) +// verifyAtomicTxs verifies that the atomic txs consumed by the block are valid. +func (b *Block) verifyAtomicTxs() error { + hash := b.ethBlock.Hash() + if b.vm.atomicBackend.IsBonus(b.Height(), hash) { + log.Info("skipping atomic tx verification on bonus block", + "block", hash, + ) return nil } - for _, atomicTx := range b.atomicTxs { - utx := atomicTx.UnsignedAtomicTx - chainID, requests, err := utx.AtomicOps() - if err != nil { - return err - } - if _, err := b.vm.ctx.SharedMemory.Get(chainID, requests.RemoveRequests); err != nil { - return fmt.Errorf("%w: %s", errMissingUTXOs, err) - } - } - return nil + // Verify [txs] do not conflict with themselves or ancestor blocks. + var ( + parentHash = b.ethBlock.ParentHash() + baseFee = b.ethBlock.BaseFee() + number = b.ethBlock.Number() + time = b.ethBlock.Time() + rules = b.vm.chainConfig.Rules(number, params.IsMergeTODO, time) + rulesExtra = params.GetRulesExtra(rules) + ) + // TODO: Pass rulesExtra as a pointer + return b.vm.verifyTxs(b.atomicTxs, parentHash, baseFee, number.Uint64(), *rulesExtra) } // Bytes implements the snowman.Block interface diff --git a/plugin/evm/block_verification.go b/plugin/evm/block_verification.go index e439c2db16..067064ae76 100644 --- a/plugin/evm/block_verification.go +++ b/plugin/evm/block_verification.go @@ -6,250 +6,202 @@ package evm import ( "errors" "fmt" - "math/big" - - "github.com/ava-labs/libevm/common" - "github.com/ava-labs/libevm/trie" - - safemath "github.com/ava-labs/avalanchego/utils/math" + "github.com/ava-labs/avalanchego/utils/math" + "github.com/ava-labs/avalanchego/utils/timer/mockable" "github.com/ava-labs/coreth/constants" + "github.com/ava-labs/coreth/core" "github.com/ava-labs/coreth/params" + "github.com/ava-labs/coreth/plugin/evm/atomic" "github.com/ava-labs/coreth/plugin/evm/customtypes" "github.com/ava-labs/coreth/plugin/evm/header" - "github.com/ava-labs/coreth/plugin/evm/upgrade/ap0" - "github.com/ava-labs/coreth/plugin/evm/upgrade/ap1" - "github.com/ava-labs/coreth/plugin/evm/upgrade/ap5" "github.com/ava-labs/coreth/utils" + "github.com/ava-labs/libevm/common" "github.com/ava-labs/libevm/core/types" + "github.com/ava-labs/libevm/trie" ) var ( - ap0MinGasPrice = big.NewInt(ap0.MinGasPrice) - ap1MinGasPrice = big.NewInt(ap1.MinGasPrice) + errUnclesUnsupported = errors.New("uncles unsupported") + errInvalidUncleHash = errors.New("invalid uncle hash") + errInvalidCoinbase = errors.New("invalid coinbase") + errInvalidTxHash = errors.New("invalid tx hash") + errInvalidDifficulty = errors.New("invalid difficulty") + errInvalidNumber = errors.New("invalid number") ) -type BlockValidator interface { - SyntacticVerify(b *Block, rules params.Rules) error -} - type blockValidator struct { extDataHashes map[common.Hash]common.Hash } -func NewBlockValidator(extDataHashes map[common.Hash]common.Hash) BlockValidator { +func newBlockValidator(extDataHashes map[common.Hash]common.Hash) *blockValidator { return &blockValidator{ extDataHashes: extDataHashes, } } -func (v blockValidator) SyntacticVerify(b *Block, rules params.Rules) error { - rulesExtra := params.GetRulesExtra(rules) - if b == nil || b.ethBlock == nil { - return errInvalidBlock +func verifyBlockStandalone( + extDataHashes map[common.Hash]common.Hash, + clock *mockable.Clock, + config *params.ChainConfig, + block *types.Block, + atomicTxs []*atomic.Tx, +) error { + var ( + ethHeader = block.Header() + txs = block.Transactions() + txsHash = types.DeriveSha(txs, trie.NewStackTrie(nil)) + maxBlockTime = clock.Unix() + maxFutureBlockTime + rules = config.Rules(ethHeader.Number, params.IsMergeTODO, ethHeader.Time) + rulesExtra = params.GetRulesExtra(rules) + errExtraValidity = header.VerifyExtra(rulesExtra.AvalancheRules, ethHeader.Extra) + version = customtypes.BlockVersion(block) + ) + switch { + case len(block.Uncles()) != 0: // Block must not have any uncles + return errUnclesUnsupported + case ethHeader.UncleHash != types.EmptyUncleHash: + return fmt.Errorf("%w: %x", errInvalidUncleHash, ethHeader.UncleHash) + case ethHeader.Coinbase != constants.BlackholeAddr: + return fmt.Errorf("%w: %x", errInvalidCoinbase, ethHeader.Coinbase) + case txsHash != ethHeader.TxHash: + return fmt.Errorf("%w: %x does not match expected %x", errInvalidTxHash, ethHeader.TxHash, txsHash) + case !utils.BigEqualUint64(ethHeader.Difficulty, 1): + return fmt.Errorf("%w: %d", errInvalidDifficulty, ethHeader.Difficulty) + case ethHeader.Number == nil || !ethHeader.Number.IsUint64(): + return fmt.Errorf("%w: %v", errInvalidNumber, ethHeader.Number) + case ethHeader.Time > maxBlockTime: // Make sure the block isn't too far in the future + return fmt.Errorf("block timestamp is too far in the future: %d > allowed %d", ethHeader.Time, maxBlockTime) + case errExtraValidity != nil: + return errExtraValidity + case ethHeader.MixDigest != (common.Hash{}): + return fmt.Errorf("invalid mix digest: %v", ethHeader.MixDigest) + case ethHeader.Nonce.Uint64() != 0: + return fmt.Errorf("%w: %d", errInvalidNonce, ethHeader.Nonce.Uint64()) + case ethHeader.WithdrawalsHash != nil: + return fmt.Errorf("unexpected withdrawalsHash: %x", ethHeader.WithdrawalsHash) + case version != 0: + return fmt.Errorf("invalid version: %d", version) + case len(txs) == 0 && len(atomicTxs) == 0: // Block must not be empty + return errEmptyBlock } - ethHeader := b.ethBlock.Header() - blockHash := b.ethBlock.Hash() + var cumulativeIntrinsicGas uint64 + for i, tx := range txs { + if tx.Type() == types.BlobTxType { + return fmt.Errorf("unexpected blobTx at index %d", i) + } - if !rulesExtra.IsApricotPhase1 { - if v.extDataHashes != nil { - extData := customtypes.BlockExtData(b.ethBlock) - extDataHash := customtypes.CalcExtDataHash(extData) - // If there is no extra data, check that there is no extra data in the hash map either to ensure we do not - // have a block that is unexpectedly missing extra data. - expectedExtDataHash, ok := v.extDataHashes[blockHash] - if len(extData) == 0 { - if ok { - return fmt.Errorf("found block with unexpected missing extra data (%s, %d), expected extra data hash: %s", blockHash, b.Height(), expectedExtDataHash) - } - } else { - // If there is extra data, check to make sure that the extra data hash matches the expected extra data hash for this - // block - if extDataHash != expectedExtDataHash { - return fmt.Errorf("extra data hash in block (%s, %d): %s, did not match the expected extra data hash: %s", blockHash, b.Height(), extDataHash, expectedExtDataHash) - } - } + var ( + contractCreation = tx.To() == nil + txData = tx.Data() + ) + // Check whether the init code size has been exceeded. + if rulesExtra.IsDurango && contractCreation && len(txData) > params.MaxInitCodeSize { + return fmt.Errorf("code size %d exceeds limit %d at index %d", len(txData), params.MaxInitCodeSize, i) + } + + intrinsicGas, err := core.IntrinsicGas( + txData, + tx.AccessList(), + contractCreation, + rules, + ) + if err != nil { + return fmt.Errorf("could not calculate intrinsic gas with %w at index %d", err, i) + } + if tx.Gas() < intrinsicGas { + return fmt.Errorf("insufficient intrinsic gas at index %d", i) } - } - // Skip verification of the genesis block since it should already be marked as accepted. - if blockHash == b.vm.genesisHash { - return nil + cumulativeIntrinsicGas, err = math.Add(cumulativeIntrinsicGas, intrinsicGas) + if err != nil { + return fmt.Errorf("calculating cumulative intrinsic gas with %w at index %d", err, i) + } + } + // Ensures that there aren't an unreasonable number of transactions in the + // block. + if cumulativeIntrinsicGas > ethHeader.GasLimit { + return fmt.Errorf("cumulative intrinsic gas %d exceeds block gas limit %d", cumulativeIntrinsicGas, ethHeader.GasLimit) } // Verify the ExtDataHash field headerExtra := customtypes.GetHeaderExtra(ethHeader) if rulesExtra.IsApricotPhase1 { - extraData := customtypes.BlockExtData(b.ethBlock) + extraData := customtypes.BlockExtData(block) hash := customtypes.CalcExtDataHash(extraData) if headerExtra.ExtDataHash != hash { return fmt.Errorf("extra data hash mismatch: have %x, want %x", headerExtra.ExtDataHash, hash) } } else { - if headerExtra.ExtDataHash != (common.Hash{}) { - return fmt.Errorf( - "expected ExtDataHash to be empty but got %x", - headerExtra.ExtDataHash, - ) - } - } - - // Perform block and header sanity checks - if !ethHeader.Number.IsUint64() { - return fmt.Errorf("invalid block number: %v", ethHeader.Number) - } - if !ethHeader.Difficulty.IsUint64() || ethHeader.Difficulty.Cmp(common.Big1) != 0 { - return fmt.Errorf("invalid difficulty: %d", ethHeader.Difficulty) - } - if ethHeader.Nonce.Uint64() != 0 { - return fmt.Errorf( - "expected nonce to be 0 but got %d: %w", - ethHeader.Nonce.Uint64(), errInvalidNonce, + var ( + blockHash = block.Hash() + extData = customtypes.BlockExtData(block) ) - } - - if ethHeader.MixDigest != (common.Hash{}) { - return fmt.Errorf("invalid mix digest: %v", ethHeader.MixDigest) - } - - // Verify the extra data is well-formed. - if err := header.VerifyExtra(rulesExtra.AvalancheRules, ethHeader.Extra); err != nil { - return err - } - - if version := customtypes.BlockVersion(b.ethBlock); version != 0 { - return fmt.Errorf("invalid version: %d", version) - } - - // Check that the tx hash in the header matches the body - txsHash := types.DeriveSha(b.ethBlock.Transactions(), trie.NewStackTrie(nil)) - if txsHash != ethHeader.TxHash { - return fmt.Errorf("invalid txs hash %v does not match calculated txs hash %v", ethHeader.TxHash, txsHash) - } - // Check that the uncle hash in the header matches the body - uncleHash := types.CalcUncleHash(b.ethBlock.Uncles()) - if uncleHash != ethHeader.UncleHash { - return fmt.Errorf("invalid uncle hash %v does not match calculated uncle hash %v", ethHeader.UncleHash, uncleHash) - } - // Coinbase must match the BlackholeAddr on C-Chain - if ethHeader.Coinbase != constants.BlackholeAddr { - return fmt.Errorf("invalid coinbase %v does not match required blackhole address %v", ethHeader.Coinbase, constants.BlackholeAddr) - } - // Block must not have any uncles - if len(b.ethBlock.Uncles()) > 0 { - return errUnclesUnsupported - } - - // Block must not be empty - txs := b.ethBlock.Transactions() - if len(txs) == 0 && len(b.atomicTxs) == 0 { - return errEmptyBlock - } - - // Enforce minimum gas prices here prior to dynamic fees going into effect. - switch { - case !rulesExtra.IsApricotPhase1: - // If we are in ApricotPhase0, enforce each transaction has a minimum gas price of at least the LaunchMinGasPrice - for _, tx := range b.ethBlock.Transactions() { - if tx.GasPrice().Cmp(ap0MinGasPrice) < 0 { - return fmt.Errorf("block contains tx %s with gas price too low (%d < %d)", tx.Hash(), tx.GasPrice(), ap0.MinGasPrice) + expectedExtDataHash, ok := extDataHashes[blockHash] + if len(extData) == 0 { + // If there is no extra data, check that there is no extra data in + // the hash map either to ensure we do not have a block that is + // unexpectedly missing extra data. + if ok { + return fmt.Errorf("found block with unexpected missing extra data (%s, %d), expected extra data hash: %s", blockHash, block.NumberU64(), expectedExtDataHash) } - } - case !rulesExtra.IsApricotPhase3: - // If we are prior to ApricotPhase3, enforce each transaction has a minimum gas price of at least the ApricotPhase1MinGasPrice - for _, tx := range b.ethBlock.Transactions() { - if tx.GasPrice().Cmp(ap1MinGasPrice) < 0 { - return fmt.Errorf("block contains tx %s with gas price too low (%d < %d)", tx.Hash(), tx.GasPrice(), ap1.MinGasPrice) + } else { + // If there is extra data, check to make sure that the extra data + // hash matches the expected extra data hash for this block. + extDataHash := customtypes.CalcExtDataHash(extData) + if extDataHash != expectedExtDataHash { + return fmt.Errorf("extra data hash in block (%s, %d): %s, did not match the expected extra data hash: %s", blockHash, block.NumberU64(), extDataHash, expectedExtDataHash) } } } - // Make sure the block isn't too far in the future - // TODO: move this to only be part of semantic verification. - blockTimestamp := b.ethBlock.Time() - if maxBlockTime := uint64(b.vm.clock.Time().Add(maxFutureBlockTime).Unix()); blockTimestamp > maxBlockTime { - return fmt.Errorf("block timestamp is too far in the future: %d > allowed %d", blockTimestamp, maxBlockTime) - } - - // Ensure BaseFee is non-nil as of ApricotPhase3. if rulesExtra.IsApricotPhase3 { if ethHeader.BaseFee == nil { return errNilBaseFeeApricotPhase3 } - // TODO: this should be removed as 256 is the maximum possible bit length of a big int - if bfLen := ethHeader.BaseFee.BitLen(); bfLen > 256 { - return fmt.Errorf("too large base fee: bitlen %d", bfLen) + if bitLen := ethHeader.BaseFee.BitLen(); bitLen > 256 { + return fmt.Errorf("baseFee too large: bitLen %d", bitLen) } - } - // If we are in ApricotPhase4, ensure that ExtDataGasUsed is populated correctly. - if rulesExtra.IsApricotPhase4 { - // After the F upgrade, the extDataGasUsed field is validated by - // [header.VerifyGasUsed]. - if !rulesExtra.IsFortuna && rulesExtra.IsApricotPhase5 { - if !utils.BigLessOrEqualUint64(headerExtra.ExtDataGasUsed, ap5.AtomicGasLimit) { - return fmt.Errorf("too large extDataGasUsed: %d", headerExtra.ExtDataGasUsed) + for i, tx := range txs { + if gasFeeCap := tx.GasFeeCap(); gasFeeCap.Cmp(ethHeader.BaseFee) < 0 { + return fmt.Errorf("insufficient gasFeeCap %d must be at least %d at index %d", gasFeeCap, ethHeader.BaseFee, i) } } + } + + // If we are in ApricotPhase4, ensure that ExtDataGasUsed is populated + // correctly. + if rulesExtra.IsApricotPhase4 { var totalGasUsed uint64 - for _, atomicTx := range b.atomicTxs { - // We perform this check manually here to avoid the overhead of having to - // reparse the atomicTx in `CalcExtDataGasUsed`. + for _, atomicTx := range atomicTxs { + // We perform this check manually here to avoid the overhead of + // having to reparse the atomicTx in `CalcExtDataGasUsed`. fixedFee := rulesExtra.IsApricotPhase5 // Charge the atomic tx fixed fee as of ApricotPhase5 gasUsed, err := atomicTx.GasUsed(fixedFee) if err != nil { return err } - totalGasUsed, err = safemath.Add(totalGasUsed, gasUsed) + totalGasUsed, err = math.Add(totalGasUsed, gasUsed) if err != nil { return err } } - switch { - case !utils.BigEqualUint64(headerExtra.ExtDataGasUsed, totalGasUsed): + if !utils.BigEqualUint64(headerExtra.ExtDataGasUsed, totalGasUsed) { return fmt.Errorf("invalid extDataGasUsed: have %d, want %d", headerExtra.ExtDataGasUsed, totalGasUsed) - - // Make sure BlockGasCost is not nil - // NOTE: ethHeader.BlockGasCost correctness is checked in header verification - case headerExtra.BlockGasCost == nil: - return errNilBlockGasCostApricotPhase4 - case !headerExtra.BlockGasCost.IsUint64(): - return fmt.Errorf("too large blockGasCost: %d", headerExtra.BlockGasCost) } } - // Verify the existence / non-existence of excessBlobGas - cancun := rules.IsCancun - if !cancun && ethHeader.ExcessBlobGas != nil { - return fmt.Errorf("invalid excessBlobGas: have %d, expected nil", *ethHeader.ExcessBlobGas) - } - if !cancun && ethHeader.BlobGasUsed != nil { - return fmt.Errorf("invalid blobGasUsed: have %d, expected nil", *ethHeader.BlobGasUsed) - } - if cancun && ethHeader.ExcessBlobGas == nil { - return errors.New("header is missing excessBlobGas") - } - if cancun && ethHeader.BlobGasUsed == nil { - return errors.New("header is missing blobGasUsed") - } - if !cancun && ethHeader.ParentBeaconRoot != nil { - return fmt.Errorf("invalid parentBeaconRoot: have %x, expected nil", *ethHeader.ParentBeaconRoot) - } - // TODO: decide what to do after Cancun - // currently we are enforcing it to be empty hash - if cancun { + if rules.IsCancun { switch { - case ethHeader.ParentBeaconRoot == nil: - return errors.New("header is missing parentBeaconRoot") - case *ethHeader.ParentBeaconRoot != (common.Hash{}): - return fmt.Errorf("invalid parentBeaconRoot: have %x, expected empty hash", ethHeader.ParentBeaconRoot) - } - if ethHeader.BlobGasUsed == nil { - return fmt.Errorf("blob gas used must not be nil in Cancun") - } else if *ethHeader.BlobGasUsed > 0 { - return fmt.Errorf("blobs not enabled on avalanche networks: used %d blob gas, expected 0", *ethHeader.BlobGasUsed) + case !utils.PointerEqualsValue(ethHeader.BlobGasUsed, 0): + return fmt.Errorf("invalid BlobGasUsed: expected 0 got %v -> %d", ethHeader.BlobGasUsed, ethHeader.BlobGasUsed) + case !utils.PointerEqualsValue(ethHeader.ExcessBlobGas, 0): + return fmt.Errorf("invalid ExcessBlobGas: expected 0 got %v -> %d", ethHeader.ExcessBlobGas, ethHeader.ExcessBlobGas) + case !utils.PointerEqualsValue(ethHeader.ParentBeaconRoot, common.Hash{}): + return fmt.Errorf("invalid ParentBeaconRoot: expected empty hash got %x", ethHeader.ParentBeaconRoot) } } return nil diff --git a/plugin/evm/block_verification_test.go b/plugin/evm/block_verification_test.go new file mode 100644 index 0000000000..4969579676 --- /dev/null +++ b/plugin/evm/block_verification_test.go @@ -0,0 +1,141 @@ +// (c) 2019-2020, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package evm + +import ( + "math/big" + "testing" + + "github.com/ava-labs/avalanchego/upgrade" + "github.com/ava-labs/avalanchego/upgrade/upgradetest" + "github.com/ava-labs/avalanchego/utils/timer/mockable" + "github.com/ava-labs/coreth/constants" + "github.com/ava-labs/coreth/plugin/evm/atomic" + "github.com/ava-labs/coreth/plugin/evm/customtypes" + "github.com/ava-labs/coreth/plugin/evm/upgrade/acp176" + "github.com/ava-labs/coreth/utils" + "github.com/ava-labs/libevm/common" + "github.com/ava-labs/libevm/core/types" + "github.com/ava-labs/libevm/crypto" + "github.com/ava-labs/libevm/trie" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestVerifyBlockStandalone(t *testing.T) { + latestConfig := forkToChainConfig[upgradetest.Latest] + signer := types.LatestSigner(latestConfig) + + key, err := crypto.GenerateKey() + require.NoError(t, err) + + tx := types.NewTx(&types.DynamicFeeTx{}) + tx, err = types.SignTx(tx, signer, key) + require.NoError(t, err) + + var ( + validHeader = customtypes.WithHeaderExtra( + &types.Header{ + Coinbase: constants.BlackholeAddr, + Difficulty: common.Big1, + Number: common.Big1, + Time: uint64(upgrade.InitiallyActiveTime.Unix()), + Extra: make([]byte, acp176.StateSize), + BaseFee: common.Big1, + BlobGasUsed: utils.NewUint64(0), + ExcessBlobGas: utils.NewUint64(0), + ParentBeaconRoot: &common.Hash{}, + }, + &customtypes.HeaderExtra{ + ExtDataHash: customtypes.CalcExtDataHash(nil), + ExtDataGasUsed: common.Big0, + BlockGasCost: common.Big0, + }, + ) + txs = []*types.Transaction{ + tx, + } + ) + + tests := []struct { + name string + extDataHashes map[common.Hash]common.Hash + clock mockable.Clock + fork upgradetest.Fork + header *types.Header + txs []*types.Transaction + uncles []*types.Header + override func(*types.Block) + atomicTxs []*atomic.Tx + want error + }{ + { + name: "valid_latest", + fork: upgradetest.Latest, + header: validHeader, + txs: txs, + }, + { + name: "has_uncles", + fork: upgradetest.Latest, + header: validHeader, + txs: txs, + uncles: []*types.Header{validHeader}, + want: errUnclesUnsupported, + }, + { + name: "invalid_coinbase", + fork: upgradetest.Latest, + header: func() *types.Header { + h := types.CopyHeader(validHeader) + h.Coinbase[0]++ + return h + }(), + txs: txs, + want: errInvalidCoinbase, + }, + { + name: "invalid_difficulty", + fork: upgradetest.Latest, + header: func() *types.Header { + h := types.CopyHeader(validHeader) + h.Difficulty = common.Big0 + return h + }(), + txs: txs, + want: errInvalidDifficulty, + }, + { + name: "invalid_number_overflow", + fork: upgradetest.Latest, + header: func() *types.Header { + h := types.CopyHeader(validHeader) + h.Number = new(big.Int).Lsh(common.Big1, 64) + return h + }(), + txs: txs, + want: errInvalidNumber, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + config := forkToChainConfig[test.fork] + block := types.NewBlock( + test.header, + test.txs, + test.uncles, + nil, + trie.NewStackTrie(nil), + ) + got := verifyBlockStandalone( + test.extDataHashes, + &test.clock, + config, + block, + test.atomicTxs, + ) + assert.ErrorIs(t, got, test.want) + }) + } +} diff --git a/plugin/evm/export_tx_test.go b/plugin/evm/export_tx_test.go index d0a3aed252..59bde7cb0b 100644 --- a/plugin/evm/export_tx_test.go +++ b/plugin/evm/export_tx_test.go @@ -932,15 +932,7 @@ func TestExportTxSemanticVerify(t *testing.T) { } t.Run(test.name, func(t *testing.T) { - tx := test.tx - exportTx := tx.UnsignedAtomicTx - - err := exportTx.Visit(&atomic.SemanticVerifier{ - Backend: backend, - Tx: tx, - Parent: parent, - BaseFee: test.baseFee, - }) + err := backend.SemanticVerify(test.tx, parent, test.baseFee) if test.shouldErr && err == nil { t.Fatalf("should have errored but returned valid") } @@ -1791,13 +1783,7 @@ func TestNewExportTx(t *testing.T) { BlockFetcher: tvm.vm, SecpCache: tvm.vm.secpCache, } - - if err := exportTx.Visit(&atomic.SemanticVerifier{ - Backend: backend, - Tx: tx, - Parent: parent, - BaseFee: parent.ethBlock.BaseFee(), - }); err != nil { + if err := backend.SemanticVerify(tx, parent, parent.ethBlock.BaseFee()); err != nil { t.Fatal("newExportTx created an invalid transaction", err) } @@ -1995,13 +1981,7 @@ func TestNewExportTxMulticoin(t *testing.T) { BlockFetcher: tvm.vm, SecpCache: tvm.vm.secpCache, } - - if err := exportTx.Visit(&atomic.SemanticVerifier{ - Backend: backend, - Tx: tx, - Parent: parent, - BaseFee: parent.ethBlock.BaseFee(), - }); err != nil { + if err := backend.SemanticVerify(tx, parent, parent.ethBlock.BaseFee()); err != nil { t.Fatal("newExportTx created an invalid transaction", err) } diff --git a/plugin/evm/tx_test.go b/plugin/evm/tx_test.go index 6017f69205..ea96466c89 100644 --- a/plugin/evm/tx_test.go +++ b/plugin/evm/tx_test.go @@ -122,12 +122,7 @@ func executeTxTest(t *testing.T, test atomicTxTest) { BlockFetcher: tvm.vm, SecpCache: tvm.vm.secpCache, } - if err := tx.UnsignedAtomicTx.Visit(&atomic.SemanticVerifier{ - Backend: backend, - Tx: tx, - Parent: lastAcceptedBlock, - BaseFee: baseFee, - }); len(test.semanticVerifyErr) == 0 && err != nil { + if err := backend.SemanticVerify(tx, lastAcceptedBlock, baseFee); len(test.semanticVerifyErr) == 0 && err != nil { t.Fatalf("SemanticVerify failed unexpectedly due to: %s", err) } else if len(test.semanticVerifyErr) != 0 { if err == nil { diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index d1bdd21996..fd2fc02c2f 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -24,6 +24,7 @@ import ( avalanchegoConstants "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/coreth/network" "github.com/ava-labs/coreth/plugin/evm/customtypes" + "github.com/ava-labs/coreth/utils" "github.com/prometheus/client_golang/prometheus" "github.com/ava-labs/coreth/consensus/dummy" @@ -48,7 +49,6 @@ import ( "github.com/ava-labs/coreth/plugin/evm/upgrade/acp176" "github.com/ava-labs/coreth/plugin/evm/upgrade/ap5" "github.com/ava-labs/coreth/triedb/hashdb" - "github.com/ava-labs/coreth/utils" "github.com/ava-labs/libevm/core/rawdb" "github.com/ava-labs/libevm/core/types" @@ -120,7 +120,7 @@ var ( const ( // Max time from current time allowed for blocks, before they're considered future blocks // and fail verification - maxFutureBlockTime = 10 * time.Second + maxFutureBlockTime = 10 // seconds maxUTXOsToFetch = 1024 defaultMempoolSize = 4096 @@ -173,9 +173,7 @@ var ( var ( errEmptyBlock = errors.New("empty block") - errInvalidBlock = errors.New("invalid block") errInvalidNonce = errors.New("invalid nonce") - errUnclesUnsupported = errors.New("uncles unsupported") errRejectedParent = errors.New("rejected parent") errNilBaseFeeApricotPhase3 = errors.New("nil base fee is invalid after apricotPhase3") errNilBlockGasCostApricotPhase4 = errors.New("nil blockGasCost is invalid after apricotPhase4") @@ -255,7 +253,7 @@ type VM struct { toEngine chan<- commonEng.Message - syntacticBlockValidator BlockValidator + syntacticBlockValidator *blockValidator // [atomicTxRepository] maintains two indexes on accepted atomic txs. // - txID to accepted atomic tx @@ -405,7 +403,7 @@ func (vm *VM) Initialize( case avalanchegoConstants.FujiID: extDataHashes = fujiExtDataHashes } - vm.syntacticBlockValidator = NewBlockValidator(extDataHashes) + vm.syntacticBlockValidator = newBlockValidator(extDataHashes) // Free the memory of the extDataHash map that is not used (i.e. if mainnet // config, free fuji) @@ -764,71 +762,24 @@ func (vm *VM) createConsensusCallbacks() dummy.ConsensusCallbacks { } } -func (vm *VM) preBatchOnFinalizeAndAssemble(header *types.Header, state *state.StateDB, txs []*types.Transaction) ([]byte, *big.Int, *big.Int, error) { - for { - tx, exists := vm.mempool.NextTx() - if !exists { - break - } - // Take a snapshot of [state] before calling verifyTx so that if the transaction fails verification - // we can revert to [snapshot]. - // Note: snapshot is taken inside the loop because you cannot revert to the same snapshot more than - // once. - snapshot := state.Snapshot() - rules := vm.rules(header.Number, header.Time) - if err := vm.verifyTx(tx, header.ParentHash, header.BaseFee, state, rules); err != nil { - // Discard the transaction from the mempool on failed verification. - log.Debug("discarding tx from mempool on failed verification", "txID", tx.ID(), "err", err) - vm.mempool.DiscardCurrentTx(tx.ID()) - state.RevertToSnapshot(snapshot) - continue - } - - atomicTxBytes, err := atomic.Codec.Marshal(atomic.CodecVersion, tx) - if err != nil { - // Discard the transaction from the mempool and error if the transaction - // cannot be marshalled. This should never happen. - log.Debug("discarding tx due to unmarshal err", "txID", tx.ID(), "err", err) - vm.mempool.DiscardCurrentTx(tx.ID()) - return nil, nil, nil, fmt.Errorf("failed to marshal atomic transaction %s due to %w", tx.ID(), err) - } - var contribution, gasUsed *big.Int - if rules.IsApricotPhase4 { - contribution, gasUsed, err = tx.BlockFeeContribution(rules.IsApricotPhase5, vm.ctx.AVAXAssetID, header.BaseFee) - if err != nil { - return nil, nil, nil, err - } - } - return atomicTxBytes, contribution, gasUsed, nil - } - - if len(txs) == 0 { - // this could happen due to the async logic of geth tx pool - return nil, nil, nil, errEmptyBlock - } - - return nil, nil, nil, nil -} - -// assumes that we are in at least Apricot Phase 5. -func (vm *VM) postBatchOnFinalizeAndAssemble( +func (vm *VM) onFinalizeAndAssemble( header *types.Header, parent *types.Header, state *state.StateDB, txs []*types.Transaction, -) ([]byte, *big.Int, *big.Int, error) { +) ([]byte, *big.Int, uint64, error) { var ( batchAtomicTxs []*atomic.Tx batchAtomicUTXOs set.Set[ids.ID] batchContribution *big.Int = new(big.Int).Set(common.Big0) - batchGasUsed *big.Int = new(big.Int).Set(common.Big0) - rules = vm.rules(header.Number, header.Time) + batchGasUsed uint64 + rules = vm.rules(header.Number, header.Time) size int ) - atomicGasLimit, err := customheader.RemainingAtomicGasCapacity(vm.chainConfigExtra(), parent, header) + remainingCapacity, err := customheader.RemainingAtomicGasCapacity(vm.chainConfigExtra(), parent, header) if err != nil { - return nil, nil, nil, err + return nil, nil, 0, err } for { @@ -844,20 +795,14 @@ func (vm *VM) postBatchOnFinalizeAndAssemble( break } - var ( - txGasUsed, txContribution *big.Int - err error - ) - // Note: we do not need to check if we are in at least ApricotPhase4 here because // we assume that this function will only be called when the block is in at least // ApricotPhase5. - txContribution, txGasUsed, err = tx.BlockFeeContribution(true, vm.ctx.AVAXAssetID, header.BaseFee) + txContribution, txGasUsed, err := tx.BlockFeeContribution(true, vm.ctx.AVAXAssetID, header.BaseFee) if err != nil { - return nil, nil, nil, err + return nil, nil, 0, err } - // ensure `gasUsed + batchGasUsed` doesn't exceed `atomicGasLimit` - if totalGasUsed := new(big.Int).Add(batchGasUsed, txGasUsed); !utils.BigLessOrEqualUint64(totalGasUsed, atomicGasLimit) { + if txGasUsed > remainingCapacity { // Send [tx] back to the mempool's tx heap. vm.mempool.CancelCurrentTx(tx.ID()) break @@ -889,10 +834,11 @@ func (vm *VM) postBatchOnFinalizeAndAssemble( batchAtomicTxs = append(batchAtomicTxs, tx) batchAtomicUTXOs.Union(tx.InputUTXOs()) - // Add the [txGasUsed] to the [batchGasUsed] when the [tx] has passed verification - batchGasUsed.Add(batchGasUsed, txGasUsed) batchContribution.Add(batchContribution, txContribution) + batchGasUsed += txGasUsed size += txSize + + remainingCapacity -= txGasUsed } // If there is a non-zero number of transactions, marshal them and return the byte slice @@ -904,7 +850,7 @@ func (vm *VM) postBatchOnFinalizeAndAssemble( // discard the entire set of current transactions. log.Debug("discarding txs due to error marshaling atomic transactions", "err", err) vm.mempool.DiscardCurrentTxs() - return nil, nil, nil, fmt.Errorf("failed to marshal batch of atomic transactions due to %w", err) + return nil, nil, 0, fmt.Errorf("failed to marshal batch of atomic transactions due to %w", err) } return atomicTxBytes, batchContribution, batchGasUsed, nil } @@ -913,93 +859,59 @@ func (vm *VM) postBatchOnFinalizeAndAssemble( // then the block is empty and should be considered invalid. if len(txs) == 0 { // this could happen due to the async logic of geth tx pool - return nil, nil, nil, errEmptyBlock + return nil, nil, 0, errEmptyBlock } // If there are no atomic transactions, but there is a non-zero number of regular transactions, then // we return a nil slice with no contribution from the atomic transactions and a nil error. - return nil, nil, nil, nil -} - -func (vm *VM) onFinalizeAndAssemble( - header *types.Header, - parent *types.Header, - state *state.StateDB, - txs []*types.Transaction, -) ([]byte, *big.Int, *big.Int, error) { - if !vm.chainConfigExtra().IsApricotPhase5(header.Time) { - return vm.preBatchOnFinalizeAndAssemble(header, state, txs) - } - return vm.postBatchOnFinalizeAndAssemble(header, parent, state, txs) + return nil, nil, 0, nil } -func (vm *VM) onExtraStateChange(block *types.Block, parent *types.Header, state *state.StateDB) (*big.Int, *big.Int, error) { +func (vm *VM) onExtraStateChange(block *types.Block, parent *types.Header, state *state.StateDB) (*big.Int, error) { var ( - batchContribution *big.Int = big.NewInt(0) - batchGasUsed *big.Int = big.NewInt(0) - header = block.Header() - rules = vm.rules(header.Number, header.Time) + header = block.Header() + rules = vm.rules(header.Number, header.Time) ) - txs, err := atomic.ExtractAtomicTxs(customtypes.BlockExtData(block), rules.IsApricotPhase5, atomic.Codec) if err != nil { - return nil, nil, err - } - - // If [atomicBackend] is nil, the VM is still initializing and is reprocessing accepted blocks. - if vm.atomicBackend != nil { - if vm.atomicBackend.IsBonus(block.NumberU64(), block.Hash()) { - log.Info("skipping atomic tx verification on bonus block", "block", block.Hash()) - } else { - // Verify [txs] do not conflict with themselves or ancestor blocks. - if err := vm.verifyTxs(txs, block.ParentHash(), block.BaseFee(), block.NumberU64(), rules); err != nil { - return nil, nil, err - } - } - // Update the atomic backend with [txs] from this block. - // - // Note: The atomic trie canonically contains the duplicate operations - // from any bonus blocks. - _, err := vm.atomicBackend.InsertTxs(block.Hash(), block.NumberU64(), block.ParentHash(), txs) - if err != nil { - return nil, nil, err - } + return nil, err } // If there are no transactions, we can return early. if len(txs) == 0 { - return nil, nil, nil + return nil, nil } + batchContribution := big.NewInt(0) for _, tx := range txs { if err := tx.UnsignedAtomicTx.EVMStateTransfer(vm.ctx, state); err != nil { - return nil, nil, err + return nil, err } // If ApricotPhase4 is enabled, calculate the block fee contribution if rules.IsApricotPhase4 { - contribution, gasUsed, err := tx.BlockFeeContribution(rules.IsApricotPhase5, vm.ctx.AVAXAssetID, block.BaseFee()) + contribution, _, err := tx.BlockFeeContribution(rules.IsApricotPhase5, vm.ctx.AVAXAssetID, block.BaseFee()) if err != nil { - return nil, nil, err + return nil, err } batchContribution.Add(batchContribution, contribution) - batchGasUsed.Add(batchGasUsed, gasUsed) } } // If ApricotPhase5 is enabled, enforce that the atomic gas used does not exceed the // atomic gas limit. if rules.IsApricotPhase5 { - atomicGasLimit, err := customheader.RemainingAtomicGasCapacity(vm.chainConfigExtra(), parent, header) + remainingCapacity, err := customheader.RemainingAtomicGasCapacity(vm.chainConfigExtra(), parent, header) if err != nil { - return nil, nil, err + return nil, err } - if !utils.BigLessOrEqualUint64(batchGasUsed, atomicGasLimit) { - return nil, nil, fmt.Errorf("atomic gas used (%d) by block (%s), exceeds atomic gas limit (%d)", batchGasUsed, block.Hash().Hex(), atomicGasLimit) + headerExtra := customtypes.GetHeaderExtra(header) + if !utils.BigLessOrEqualUint64(headerExtra.ExtDataGasUsed, remainingCapacity) { + return nil, fmt.Errorf("atomic gas used (%d) by block (%s), exceeds atomic gas limit (%d)", headerExtra.ExtDataGasUsed, block.Hash().Hex(), remainingCapacity) } } - return batchContribution, batchGasUsed, nil + return batchContribution, nil } func (vm *VM) SetState(_ context.Context, state snow.State) error { @@ -1338,7 +1250,7 @@ func (vm *VM) parseBlock(_ context.Context, b []byte) (snowman.Block, error) { } // Performing syntactic verification in ParseBlock allows for // short-circuiting bad blocks before they are processed by the VM. - if err := block.syntacticVerify(); err != nil { + if err := block.verifyWithoutParent(); err != nil { return nil, fmt.Errorf("syntactic block verification failed: %w", err) } return block, nil @@ -1602,12 +1514,7 @@ func (vm *VM) verifyTx(tx *atomic.Tx, parentHash common.Hash, baseFee *big.Int, BlockFetcher: vm, SecpCache: vm.secpCache, } - if err := tx.UnsignedAtomicTx.Visit(&atomic.SemanticVerifier{ - Backend: atomicBackend, - Tx: tx, - Parent: parent, - BaseFee: baseFee, - }); err != nil { + if atomicBackend.SemanticVerify(tx, parent, baseFee); err != nil { return err } return tx.UnsignedAtomicTx.EVMStateTransfer(vm.ctx, state) @@ -1621,42 +1528,38 @@ func (vm *VM) verifyTxs(txs []*atomic.Tx, parentHash common.Hash, baseFee *big.I return errRejectedParent } - ancestorID := ids.ID(parentHash) + parentID := ids.ID(parentHash) // If the ancestor is unknown, then the parent failed verification when // it was called. // If the ancestor is rejected, then this block shouldn't be inserted // into the canonical chain because the parent will be missing. - ancestorInf, err := vm.GetBlockInternal(context.TODO(), ancestorID) + parentInf, err := vm.GetBlockInternal(context.TODO(), parentID) if err != nil { return errRejectedParent } - ancestor, ok := ancestorInf.(*Block) + parent, ok := parentInf.(*Block) if !ok { - return fmt.Errorf("expected parent block %s, to be *Block but is %T", ancestor.ID(), ancestorInf) + return fmt.Errorf("expected parent block %s, to be *Block but is %T", parent.ID(), parentInf) } // Ensure each tx in [txs] doesn't conflict with any other atomic tx in // a processing ancestor block. - inputs := set.Set[ids.ID]{} - atomicBackend := &atomic.VerifierBackend{ - Ctx: vm.ctx, - Fx: &vm.fx, - Rules: rules, - Bootstrapped: vm.bootstrapped.Get(), - BlockFetcher: vm, - SecpCache: vm.secpCache, - } - for _, atomicTx := range txs { - utx := atomicTx.UnsignedAtomicTx - if err := utx.Visit(&atomic.SemanticVerifier{ - Backend: atomicBackend, - Tx: atomicTx, - Parent: ancestor, - BaseFee: baseFee, - }); err != nil { - return fmt.Errorf("invalid block due to failed semanatic verify: %w at height %d", err, height) + var ( + atomicBackend = &atomic.VerifierBackend{ + Ctx: vm.ctx, + Fx: &vm.fx, + Rules: rules, + Bootstrapped: vm.bootstrapped.Get(), + BlockFetcher: vm, + SecpCache: vm.secpCache, + } + inputs set.Set[ids.ID] + ) + for _, tx := range txs { + if atomicBackend.SemanticVerify(tx, parent, baseFee); err != nil { + return fmt.Errorf("invalid block due to failed semantic verify: %w at height %d", err, height) } - txInputs := utx.InputUTXOs() + txInputs := tx.UnsignedAtomicTx.InputUTXOs() if inputs.Overlaps(txInputs) { return atomic.ErrConflictingAtomicInputs } diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index bd144dfc71..d68c5fcb96 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -2645,8 +2645,7 @@ func TestFutureBlock(t *testing.T) { // Set the VM's clock to the time of the produced block tvm.vm.clock.Set(time.Unix(int64(modifiedHeader.Time), 0)) // Set the modified time to exceed the allowed future time - modifiedTime := modifiedHeader.Time + uint64(maxFutureBlockTime.Seconds()+1) - modifiedHeader.Time = modifiedTime + modifiedHeader.Time += maxFutureBlockTime + 1 modifiedBlock := customtypes.NewBlockWithExtData( modifiedHeader, nil, @@ -3661,7 +3660,7 @@ func TestExtraStateChangeAtomicGasLimitExceeded(t *testing.T) { } // Hack: test [onExtraStateChange] directly to ensure it catches the atomic gas limit error correctly. - if _, _, err := tvm2.vm.onExtraStateChange(ethBlk2, nil, state); err == nil || !strings.Contains(err.Error(), "exceeds atomic gas limit") { + if _, err := tvm2.vm.onExtraStateChange(ethBlk2, nil, state); err == nil || !strings.Contains(err.Error(), "exceeds atomic gas limit") { t.Fatalf("Expected block to fail verification due to exceeded atomic gas limit, but found error: %v", err) } } @@ -3728,39 +3727,39 @@ func TestParentBeaconRootBlock(t *testing.T) { errString string }{ { - name: "non-empty parent beacon root in Durango", + name: "non_empty_parent_beacon_root_in_Durango", fork: upgradetest.Durango, beaconRoot: &common.Hash{0x01}, expectedError: true, // err string wont work because it will also fail with blob gas is non-empty (zeroed) }, { - name: "empty parent beacon root in Durango", + name: "empty_parent_beacon_root_in_Durango", fork: upgradetest.Durango, beaconRoot: &common.Hash{}, expectedError: true, }, { - name: "nil parent beacon root in Durango", + name: "nil_parent_beacon_root_in_Durango", fork: upgradetest.Durango, beaconRoot: nil, expectedError: false, }, { - name: "non-empty parent beacon root in E-Upgrade (Cancun)", + name: "non_empty_parent_beacon_root_in_Etna_Cancun", fork: upgradetest.Etna, beaconRoot: &common.Hash{0x01}, expectedError: true, errString: "expected empty hash", }, { - name: "empty parent beacon root in E-Upgrade (Cancun)", + name: "empty_parent_beacon_root_in_Etna_Cancun", fork: upgradetest.Etna, beaconRoot: &common.Hash{}, expectedError: false, }, { - name: "nil parent beacon root in E-Upgrade (Cancun)", + name: "nil_parent_beacon_root_in_Etna_Cancun", fork: upgradetest.Etna, beaconRoot: nil, expectedError: true, diff --git a/precompile/precompileconfig/upgradeable.go b/precompile/precompileconfig/upgradeable.go index a639e12587..04d386e844 100644 --- a/precompile/precompileconfig/upgradeable.go +++ b/precompile/precompileconfig/upgradeable.go @@ -29,5 +29,5 @@ func (u *Upgrade) Equal(other *Upgrade) bool { if other == nil { return false } - return u.Disable == other.Disable && utils.Uint64PtrEqual(u.BlockTimestamp, other.BlockTimestamp) + return u.Disable == other.Disable && utils.PointerEqual(u.BlockTimestamp, other.BlockTimestamp) } diff --git a/utils/numbers.go b/utils/numbers.go index 5d983a25fe..ecbfb2fd28 100644 --- a/utils/numbers.go +++ b/utils/numbers.go @@ -20,15 +20,20 @@ func Uint64ToTime(val *uint64) time.Time { return time.Unix(timestamp, 0) } -// Uint64PtrEqual returns true if x and y pointers are equivalent ie. both nil or both -// contain the same value. -func Uint64PtrEqual(x, y *uint64) bool { +// PointerEqual returns true if x and y point to the same values, or are both +// nil. +func PointerEqual[T comparable](x, y *T) bool { if x == nil || y == nil { return x == y } return *x == *y } +// PointerEqualsValue returns true if p points to a value equal to v. +func PointerEqualsValue[T comparable](p *T, v T) bool { + return p != nil && *p == v +} + // BigEqual returns true if a is equal to b. If a and b are nil, it returns // true. func BigEqual(a, b *big.Int) bool { diff --git a/utils/numbers_test.go b/utils/numbers_test.go index 074a3d45bd..f4965bada3 100644 --- a/utils/numbers_test.go +++ b/utils/numbers_test.go @@ -10,6 +10,99 @@ import ( "github.com/stretchr/testify/assert" ) +func TestPointerEqual(t *testing.T) { + tests := []struct { + name string + x *uint64 + y *uint64 + want bool + }{ + { + name: "nil_nil", + x: nil, + y: nil, + want: true, + }, + { + name: "0_nil", + x: NewUint64(0), + y: nil, + want: false, + }, + { + name: "0_1", + x: NewUint64(0), + y: NewUint64(1), + want: false, + }, + { + name: "0_0", + x: NewUint64(0), + y: NewUint64(0), + want: true, + }, + { + name: "1_1", + x: NewUint64(1), + y: NewUint64(1), + want: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert := assert.New(t) + + assert.Equal(test.want, PointerEqual(test.x, test.y)) + assert.Equal(test.want, PointerEqual(test.y, test.x)) + }) + } +} + +func TestPointerEqualsValue(t *testing.T) { + tests := []struct { + name string + p *uint64 + v uint64 + want bool + }{ + { + name: "nil_0", + p: nil, + v: 0, + want: false, + }, + { + name: "nil_1", + p: nil, + v: 1, + want: false, + }, + { + name: "0_0", + p: NewUint64(0), + v: 0, + want: true, + }, + { + name: "1_0", + p: NewUint64(1), + v: 0, + want: false, + }, + { + name: "1_1", + p: NewUint64(1), + v: 1, + want: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.want, PointerEqualsValue(test.p, test.v)) + }) + } +} + func TestBigEqual(t *testing.T) { tests := []struct { name string