From 470dba8fc1890938a65bbf4293a4759a9b9615a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Wed, 8 Nov 2023 11:22:08 +0200 Subject: [PATCH] core/vm: set basefee to 0 internally on eth_call (#28470) * core/vm: set basefee to 0 internally on eth_call * core: nicer 0-basefee, make it work for blob fees too * internal/ethapi: make tests a bit more complex * core: fix blob fee checker * core: make code a bit more readable * core: fix some test error strings * core/vm: Get rid of weird comment * core: dict wrong typo --- core/evm.go | 6 +++++- core/state_processor_test.go | 9 ++++---- core/state_transition.go | 22 +++++++++++-------- core/vm/evm.go | 26 +++++++++++++---------- core/vm/runtime/env.go | 1 + core/vm/runtime/runtime.go | 3 ++- internal/ethapi/api_test.go | 41 ++++++++++++++++++++++++++++++++++++ 7 files changed, 82 insertions(+), 26 deletions(-) diff --git a/core/evm.go b/core/evm.go index 46dcb3146..c4801dc79 100644 --- a/core/evm.go +++ b/core/evm.go @@ -77,11 +77,15 @@ func NewEVMBlockContext(header *types.Header, chain ChainContext, author *common // NewEVMTxContext creates a new transaction context for a single transaction. func NewEVMTxContext(msg *Message) vm.TxContext { - return vm.TxContext{ + ctx := vm.TxContext{ Origin: msg.From, GasPrice: new(big.Int).Set(msg.GasPrice), BlobHashes: msg.BlobHashes, } + if msg.BlobGasFeeCap != nil { + ctx.BlobFeeCap = new(big.Int).Set(msg.BlobGasFeeCap) + } + return ctx } // GetHashFn returns a GetHashFunc which retrieves header hashes by number diff --git a/core/state_processor_test.go b/core/state_processor_test.go index e80bc288c..5ff9353bd 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -95,7 +95,7 @@ func TestStateProcessorErrors(t *testing.T) { }), signer, key1) return tx } - var mkBlobTx = func(nonce uint64, to common.Address, gasLimit uint64, gasTipCap, gasFeeCap *big.Int, hashes []common.Hash) *types.Transaction { + var mkBlobTx = func(nonce uint64, to common.Address, gasLimit uint64, gasTipCap, gasFeeCap, blobGasFeeCap *big.Int, hashes []common.Hash) *types.Transaction { tx, err := types.SignTx(types.NewTx(&types.BlobTx{ Nonce: nonce, GasTipCap: uint256.MustFromBig(gasTipCap), @@ -103,6 +103,7 @@ func TestStateProcessorErrors(t *testing.T) { Gas: gasLimit, To: to, BlobHashes: hashes, + BlobFeeCap: uint256.MustFromBig(blobGasFeeCap), Value: new(uint256.Int), }), signer, key1) if err != nil { @@ -196,7 +197,7 @@ func TestStateProcessorErrors(t *testing.T) { txs: []*types.Transaction{ mkDynamicTx(0, common.Address{}, params.TxGas, big.NewInt(0), big.NewInt(0)), }, - want: "could not apply tx 0 [0xc4ab868fef0c82ae0387b742aee87907f2d0fc528fc6ea0a021459fb0fc4a4a8]: max fee per gas less than block base fee: address 0x71562b71999873DB5b286dF957af199Ec94617F7, maxFeePerGas: 0 baseFee: 875000000", + want: "could not apply tx 0 [0xc4ab868fef0c82ae0387b742aee87907f2d0fc528fc6ea0a021459fb0fc4a4a8]: max fee per gas less than block base fee: address 0x71562b71999873DB5b286dF957af199Ec94617F7, maxFeePerGas: 0, baseFee: 875000000", }, { // ErrTipVeryHigh txs: []*types.Transaction{ @@ -247,9 +248,9 @@ func TestStateProcessorErrors(t *testing.T) { }, { // ErrBlobFeeCapTooLow txs: []*types.Transaction{ - mkBlobTx(0, common.Address{}, params.TxGas, big.NewInt(1), big.NewInt(1), []common.Hash{(common.Hash{1})}), + mkBlobTx(0, common.Address{}, params.TxGas, big.NewInt(1), big.NewInt(1), big.NewInt(0), []common.Hash{(common.Hash{1})}), }, - want: "could not apply tx 0 [0x6c11015985ce82db691d7b2d017acda296db88b811c3c60dc71449c76256c716]: max fee per gas less than block base fee: address 0x71562b71999873DB5b286dF957af199Ec94617F7, maxFeePerGas: 1 baseFee: 875000000", + want: "could not apply tx 0 [0x6c11015985ce82db691d7b2d017acda296db88b811c3c60dc71449c76256c716]: max fee per gas less than block base fee: address 0x71562b71999873DB5b286dF957af199Ec94617F7, maxFeePerGas: 1, baseFee: 875000000", }, } { block := GenerateBadBlock(gspec.ToBlock(), beacon.New(ethash.NewFaker()), tt.txs, gspec.Config) diff --git a/core/state_transition.go b/core/state_transition.go index fb03c48aa..612fdd781 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -287,11 +287,11 @@ func (st *StateTransition) preCheck() error { msg.From.Hex(), codeHash) } } - // Make sure that transaction gasFeeCap is greater than the baseFee (post london) if st.evm.ChainConfig().IsLondon(st.evm.Context.BlockNumber) { // Skip the checks if gas fields are zero and baseFee was explicitly disabled (eth_call) - if !st.evm.Config.NoBaseFee || msg.GasFeeCap.BitLen() > 0 || msg.GasTipCap.BitLen() > 0 { + skipCheck := st.evm.Config.NoBaseFee && msg.GasFeeCap.BitLen() == 0 && msg.GasTipCap.BitLen() == 0 + if !skipCheck { if l := msg.GasFeeCap.BitLen(); l > 256 { return fmt.Errorf("%w: address %v, maxFeePerGas bit length: %d", ErrFeeCapVeryHigh, msg.From.Hex(), l) @@ -307,7 +307,7 @@ func (st *StateTransition) preCheck() error { // This will panic if baseFee is nil, but basefee presence is verified // as part of header validation. if msg.GasFeeCap.Cmp(st.evm.Context.BaseFee) < 0 { - return fmt.Errorf("%w: address %v, maxFeePerGas: %s baseFee: %s", ErrFeeCapTooLow, + return fmt.Errorf("%w: address %v, maxFeePerGas: %s, baseFee: %s", ErrFeeCapTooLow, msg.From.Hex(), msg.GasFeeCap, st.evm.Context.BaseFee) } } @@ -324,17 +324,21 @@ func (st *StateTransition) preCheck() error { } } } - + // Check that the user is paying at least the current blob fee if st.evm.ChainConfig().IsCancun(st.evm.Context.BlockNumber, st.evm.Context.Time) { if st.blobGasUsed() > 0 { - // Check that the user is paying at least the current blob fee - blobFee := st.evm.Context.BlobBaseFee - if st.msg.BlobGasFeeCap.Cmp(blobFee) < 0 { - return fmt.Errorf("%w: address %v have %v want %v", ErrBlobFeeCapTooLow, st.msg.From.Hex(), st.msg.BlobGasFeeCap, blobFee) + // Skip the checks if gas fields are zero and blobBaseFee was explicitly disabled (eth_call) + skipCheck := st.evm.Config.NoBaseFee && msg.BlobGasFeeCap.BitLen() == 0 + if !skipCheck { + // This will panic if blobBaseFee is nil, but blobBaseFee presence + // is verified as part of header validation. + if msg.BlobGasFeeCap.Cmp(st.evm.Context.BlobBaseFee) < 0 { + return fmt.Errorf("%w: address %v blobGasFeeCap: %v, blobBaseFee: %v", ErrBlobFeeCapTooLow, + msg.From.Hex(), msg.BlobGasFeeCap, st.evm.Context.BlobBaseFee) + } } } } - return st.buyGas() } diff --git a/core/vm/evm.go b/core/vm/evm.go index 2c6cc7d48..088b18aaa 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -72,8 +72,8 @@ type BlockContext struct { BlockNumber *big.Int // Provides information for NUMBER Time uint64 // Provides information for TIME Difficulty *big.Int // Provides information for DIFFICULTY - BaseFee *big.Int // Provides information for BASEFEE - BlobBaseFee *big.Int // Provides information for BLOBBASEFEE + BaseFee *big.Int // Provides information for BASEFEE (0 if vm runs with NoBaseFee flag and 0 gas price) + BlobBaseFee *big.Int // Provides information for BLOBBASEFEE (0 if vm runs with NoBaseFee flag and 0 blob gas price) Random *common.Hash // Provides information for PREVRANDAO } @@ -82,8 +82,9 @@ type BlockContext struct { type TxContext struct { // Message information Origin common.Address // Provides information for ORIGIN - GasPrice *big.Int // Provides information for GASPRICE + GasPrice *big.Int // Provides information for GASPRICE (and is used to zero the basefee if NoBaseFee is set) BlobHashes []common.Hash // Provides information for BLOBHASH + BlobFeeCap *big.Int // Is used to zero the blobbasefee if NoBaseFee is set } // EVM is the Ethereum Virtual Machine base object and provides @@ -125,6 +126,17 @@ type EVM struct { // NewEVM returns a new EVM. The returned EVM is not thread safe and should // only ever be used *once*. func NewEVM(blockCtx BlockContext, txCtx TxContext, statedb StateDB, chainConfig *params.ChainConfig, config Config) *EVM { + // If basefee tracking is disabled (eth_call, eth_estimateGas, etc), and no + // gas prices were specified, lower the basefee to 0 to avoid breaking EVM + // invariants (basefee < feecap) + if config.NoBaseFee { + if txCtx.GasPrice.BitLen() == 0 { + blockCtx.BaseFee = new(big.Int) + } + if txCtx.BlobFeeCap != nil && txCtx.BlobFeeCap.BitLen() == 0 { + blockCtx.BlobBaseFee = new(big.Int) + } + } evm := &EVM{ Context: blockCtx, TxContext: txCtx, @@ -160,14 +172,6 @@ func (evm *EVM) Interpreter() *EVMInterpreter { return evm.interpreter } -// SetBlockContext updates the block context of the EVM. -func (evm *EVM) SetBlockContext(blockCtx BlockContext) { - evm.Context = blockCtx - num := blockCtx.BlockNumber - timestamp := blockCtx.Time - evm.chainRules = evm.chainConfig.Rules(num, blockCtx.Random != nil, timestamp) -} - // Call executes the contract associated with the addr with the given input as // parameters. It also handles any necessary value transfer required and takes // the necessary steps to create accounts and reverses the state in case of an diff --git a/core/vm/runtime/env.go b/core/vm/runtime/env.go index 64aa550a2..34335b8e9 100644 --- a/core/vm/runtime/env.go +++ b/core/vm/runtime/env.go @@ -26,6 +26,7 @@ func NewEnv(cfg *Config) *vm.EVM { Origin: cfg.Origin, GasPrice: cfg.GasPrice, BlobHashes: cfg.BlobHashes, + BlobFeeCap: cfg.BlobFeeCap, } blockContext := vm.BlockContext{ CanTransfer: core.CanTransfer, diff --git a/core/vm/runtime/runtime.go b/core/vm/runtime/runtime.go index cfd7e4dbc..d10457e7f 100644 --- a/core/vm/runtime/runtime.go +++ b/core/vm/runtime/runtime.go @@ -46,6 +46,7 @@ type Config struct { BaseFee *big.Int BlobBaseFee *big.Int BlobHashes []common.Hash + BlobFeeCap *big.Int Random *common.Hash State *state.StateDB @@ -97,7 +98,7 @@ func setDefaults(cfg *Config) { cfg.BaseFee = big.NewInt(params.InitialBaseFee) } if cfg.BlobBaseFee == nil { - cfg.BlobBaseFee = new(big.Int) + cfg.BlobBaseFee = big.NewInt(params.BlobTxMinBlobGasprice) } } diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go index 59882cd6b..a67bd1203 100644 --- a/internal/ethapi/api_test.go +++ b/internal/ethapi/api_test.go @@ -678,6 +678,47 @@ func TestEstimateGas(t *testing.T) { }, expectErr: core.ErrInsufficientFunds, }, + // Test for a bug where the gas price was set to zero but the basefee non-zero + // + // contract BasefeeChecker { + // constructor() { + // require(tx.gasprice >= block.basefee); + // if (tx.gasprice > 0) { + // require(block.basefee > 0); + // } + // } + //} + { + blockNumber: rpc.LatestBlockNumber, + call: TransactionArgs{ + From: &accounts[0].addr, + Input: hex2Bytes("6080604052348015600f57600080fd5b50483a1015601c57600080fd5b60003a111560315760004811603057600080fd5b5b603f80603e6000396000f3fe6080604052600080fdfea264697066735822122060729c2cee02b10748fae5200f1c9da4661963354973d9154c13a8e9ce9dee1564736f6c63430008130033"), + GasPrice: (*hexutil.Big)(big.NewInt(1_000_000_000)), // Legacy as pricing + }, + expectErr: nil, + want: 67617, + }, + { + blockNumber: rpc.LatestBlockNumber, + call: TransactionArgs{ + From: &accounts[0].addr, + Input: hex2Bytes("6080604052348015600f57600080fd5b50483a1015601c57600080fd5b60003a111560315760004811603057600080fd5b5b603f80603e6000396000f3fe6080604052600080fdfea264697066735822122060729c2cee02b10748fae5200f1c9da4661963354973d9154c13a8e9ce9dee1564736f6c63430008130033"), + MaxFeePerGas: (*hexutil.Big)(big.NewInt(1_000_000_000)), // 1559 gas pricing + }, + expectErr: nil, + want: 67617, + }, + { + blockNumber: rpc.LatestBlockNumber, + call: TransactionArgs{ + From: &accounts[0].addr, + Input: hex2Bytes("6080604052348015600f57600080fd5b50483a1015601c57600080fd5b60003a111560315760004811603057600080fd5b5b603f80603e6000396000f3fe6080604052600080fdfea264697066735822122060729c2cee02b10748fae5200f1c9da4661963354973d9154c13a8e9ce9dee1564736f6c63430008130033"), + GasPrice: nil, // No legacy gas pricing + MaxFeePerGas: nil, // No 1559 gas pricing + }, + expectErr: nil, + want: 67595, + }, } for i, tc := range testSuite { result, err := api.EstimateGas(context.Background(), tc.call, &rpc.BlockNumberOrHash{BlockNumber: &tc.blockNumber}, &tc.overrides)