From eeaaeb27744179c03654348bc9dba51b4b96eb9a Mon Sep 17 00:00:00 2001 From: jsvisa Date: Tue, 1 Aug 2023 21:27:36 +0800 Subject: [PATCH 01/16] internal/ethapi: distinguish Call and Transaction nobasefee check Signed-off-by: jsvisa --- internal/ethapi/api.go | 12 ++++++------ internal/ethapi/transaction_args.go | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index e130d9c5070f..7f9be23ff949 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1024,7 +1024,7 @@ func (context *ChainContext) GetHeader(hash common.Hash, number uint64) *types.H return header } -func doCall(ctx context.Context, b Backend, args TransactionArgs, state *state.StateDB, header *types.Header, overrides *StateOverride, blockOverrides *BlockOverrides, timeout time.Duration, globalGasCap uint64) (*core.ExecutionResult, error) { +func doCall(ctx context.Context, b Backend, args TransactionArgs, state *state.StateDB, header *types.Header, overrides *StateOverride, blockOverrides *BlockOverrides, timeout time.Duration, globalGasCap uint64, noBaseFee bool) (*core.ExecutionResult, error) { if err := overrides.Apply(state); err != nil { return nil, err } @@ -1049,7 +1049,7 @@ func doCall(ctx context.Context, b Backend, args TransactionArgs, state *state.S if blockOverrides != nil { blockOverrides.Apply(&blockCtx) } - evm, vmError := b.GetEVM(ctx, msg, state, header, &vm.Config{NoBaseFee: true}, &blockCtx) + evm, vmError := b.GetEVM(ctx, msg, state, header, &vm.Config{NoBaseFee: noBaseFee}, &blockCtx) // Wait for the context to be done and cancel the evm. Even if the // EVM has finished, cancelling may be done (repeatedly) @@ -1083,7 +1083,7 @@ func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash return nil, err } - return doCall(ctx, b, args, state, header, overrides, blockOverrides, timeout, globalGasCap) + return doCall(ctx, b, args, state, header, overrides, blockOverrides, timeout, globalGasCap, true) } func newRevertError(result *core.ExecutionResult) *revertError { @@ -1134,7 +1134,7 @@ func (s *BlockChainAPI) Call(ctx context.Context, args TransactionArgs, blockNrO return result.Return(), result.Err } -func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap uint64) (hexutil.Uint64, error) { +func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap uint64, noBaseFee bool) (hexutil.Uint64, error) { // Binary search the gas requirement, as it may be higher than the amount used var ( lo uint64 = params.TxGas - 1 @@ -1208,7 +1208,7 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr executable := func(gas uint64, state *state.StateDB, header *types.Header) (bool, *core.ExecutionResult, error) { args.Gas = (*hexutil.Uint64)(&gas) - result, err := doCall(ctx, b, args, state, header, nil, nil, 0, gasCap) + result, err := doCall(ctx, b, args, state, header, nil, nil, 0, gasCap, noBaseFee) if err != nil { if errors.Is(err, core.ErrIntrinsicGas) { return true, nil, nil // Special case, raise gas limit @@ -1266,7 +1266,7 @@ func (s *BlockChainAPI) EstimateGas(ctx context.Context, args TransactionArgs, b if blockNrOrHash != nil { bNrOrHash = *blockNrOrHash } - return DoEstimateGas(ctx, s.b, args, bNrOrHash, s.b.RPCGasCap()) + return DoEstimateGas(ctx, s.b, args, bNrOrHash, s.b.RPCGasCap(), false) } // RPCMarshalHeader converts the given header to the RPC output . diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index 6e2690e095d9..2c490037861c 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -111,7 +111,7 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error { AccessList: args.AccessList, } pendingBlockNr := rpc.BlockNumberOrHashWithNumber(rpc.PendingBlockNumber) - estimated, err := DoEstimateGas(ctx, b, callArgs, pendingBlockNr, b.RPCGasCap()) + estimated, err := DoEstimateGas(ctx, b, callArgs, pendingBlockNr, b.RPCGasCap(), false) if err != nil { return err } From f06765649a269a7bd2971d8238c4f74b1d3cd137 Mon Sep 17 00:00:00 2001 From: jsvisa Date: Tue, 1 Aug 2023 21:28:18 +0800 Subject: [PATCH 02/16] graphql: estimateGas nobasefee Signed-off-by: jsvisa --- graphql/graphql.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/graphql/graphql.go b/graphql/graphql.go index aa042862bd36..eac85ab96a44 100644 --- a/graphql/graphql.go +++ b/graphql/graphql.go @@ -1130,7 +1130,7 @@ func (b *Block) Call(ctx context.Context, args struct { func (b *Block) EstimateGas(ctx context.Context, args struct { Data ethapi.TransactionArgs }) (hexutil.Uint64, error) { - return ethapi.DoEstimateGas(ctx, b.r.backend, args.Data, *b.numberOrHash, b.r.backend.RPCGasCap()) + return ethapi.DoEstimateGas(ctx, b.r.backend, args.Data, *b.numberOrHash, b.r.backend.RPCGasCap(), true) } type Pending struct { @@ -1194,7 +1194,7 @@ func (p *Pending) EstimateGas(ctx context.Context, args struct { Data ethapi.TransactionArgs }) (hexutil.Uint64, error) { latestBlockNr := rpc.BlockNumberOrHashWithNumber(rpc.LatestBlockNumber) - return ethapi.DoEstimateGas(ctx, p.r.backend, args.Data, latestBlockNr, p.r.backend.RPCGasCap()) + return ethapi.DoEstimateGas(ctx, p.r.backend, args.Data, latestBlockNr, p.r.backend.RPCGasCap(), true) } // Resolver is the top-level object in the GraphQL hierarchy. From 21dffcb7694c710c1507c78a93b1af23eebf4486 Mon Sep 17 00:00:00 2001 From: jsvisa Date: Tue, 1 Aug 2023 23:08:31 +0800 Subject: [PATCH 03/16] internal/ethapi: api EstimateGas noBaseFee Signed-off-by: jsvisa --- internal/ethapi/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 7f9be23ff949..58031776f712 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1266,7 +1266,7 @@ func (s *BlockChainAPI) EstimateGas(ctx context.Context, args TransactionArgs, b if blockNrOrHash != nil { bNrOrHash = *blockNrOrHash } - return DoEstimateGas(ctx, s.b, args, bNrOrHash, s.b.RPCGasCap(), false) + return DoEstimateGas(ctx, s.b, args, bNrOrHash, s.b.RPCGasCap(), true) } // RPCMarshalHeader converts the given header to the RPC output . From aa47d0197a3b0928de255a86413a0cc6f212bbaa Mon Sep 17 00:00:00 2001 From: jsvisa Date: Tue, 1 Aug 2023 23:14:38 +0800 Subject: [PATCH 04/16] internal/ethapi: add document for DoEstimateGas Signed-off-by: jsvisa --- internal/ethapi/api.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 58031776f712..480eaae9cd69 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1134,6 +1134,8 @@ func (s *BlockChainAPI) Call(ctx context.Context, args TransactionArgs, blockNrO return result.Return(), result.Err } +// DoEstimateGas estimates the gas needed to execute a specific transaction. +// Note, the flag noBaseFee indicating whether to skip the base fee check func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap uint64, noBaseFee bool) (hexutil.Uint64, error) { // Binary search the gas requirement, as it may be higher than the amount used var ( From a237e35cf37312374a9fbddab3df065c369eb28b Mon Sep 17 00:00:00 2001 From: jsvisa Date: Thu, 3 Aug 2023 14:05:07 +0800 Subject: [PATCH 05/16] Revert "internal/ethapi: add document for DoEstimateGas" This reverts commit aa47d0197a3b0928de255a86413a0cc6f212bbaa. Signed-off-by: jsvisa --- internal/ethapi/api.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 480eaae9cd69..58031776f712 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1134,8 +1134,6 @@ func (s *BlockChainAPI) Call(ctx context.Context, args TransactionArgs, blockNrO return result.Return(), result.Err } -// DoEstimateGas estimates the gas needed to execute a specific transaction. -// Note, the flag noBaseFee indicating whether to skip the base fee check func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap uint64, noBaseFee bool) (hexutil.Uint64, error) { // Binary search the gas requirement, as it may be higher than the amount used var ( From 70ab5085820a138ccb51e69a7cecf7b284e5691b Mon Sep 17 00:00:00 2001 From: jsvisa Date: Thu, 3 Aug 2023 14:05:17 +0800 Subject: [PATCH 06/16] Revert "internal/ethapi: api EstimateGas noBaseFee" This reverts commit 21dffcb7694c710c1507c78a93b1af23eebf4486. Signed-off-by: jsvisa --- internal/ethapi/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 58031776f712..7f9be23ff949 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1266,7 +1266,7 @@ func (s *BlockChainAPI) EstimateGas(ctx context.Context, args TransactionArgs, b if blockNrOrHash != nil { bNrOrHash = *blockNrOrHash } - return DoEstimateGas(ctx, s.b, args, bNrOrHash, s.b.RPCGasCap(), true) + return DoEstimateGas(ctx, s.b, args, bNrOrHash, s.b.RPCGasCap(), false) } // RPCMarshalHeader converts the given header to the RPC output . From e460555f232fb9d696e0c9af141ef147152bddd0 Mon Sep 17 00:00:00 2001 From: jsvisa Date: Thu, 3 Aug 2023 14:05:25 +0800 Subject: [PATCH 07/16] Revert "graphql: estimateGas nobasefee" This reverts commit f06765649a269a7bd2971d8238c4f74b1d3cd137. Signed-off-by: jsvisa --- graphql/graphql.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/graphql/graphql.go b/graphql/graphql.go index eac85ab96a44..aa042862bd36 100644 --- a/graphql/graphql.go +++ b/graphql/graphql.go @@ -1130,7 +1130,7 @@ func (b *Block) Call(ctx context.Context, args struct { func (b *Block) EstimateGas(ctx context.Context, args struct { Data ethapi.TransactionArgs }) (hexutil.Uint64, error) { - return ethapi.DoEstimateGas(ctx, b.r.backend, args.Data, *b.numberOrHash, b.r.backend.RPCGasCap(), true) + return ethapi.DoEstimateGas(ctx, b.r.backend, args.Data, *b.numberOrHash, b.r.backend.RPCGasCap()) } type Pending struct { @@ -1194,7 +1194,7 @@ func (p *Pending) EstimateGas(ctx context.Context, args struct { Data ethapi.TransactionArgs }) (hexutil.Uint64, error) { latestBlockNr := rpc.BlockNumberOrHashWithNumber(rpc.LatestBlockNumber) - return ethapi.DoEstimateGas(ctx, p.r.backend, args.Data, latestBlockNr, p.r.backend.RPCGasCap(), true) + return ethapi.DoEstimateGas(ctx, p.r.backend, args.Data, latestBlockNr, p.r.backend.RPCGasCap()) } // Resolver is the top-level object in the GraphQL hierarchy. From 124665eb6e61dc05f18b1da8f1667dc2bf152730 Mon Sep 17 00:00:00 2001 From: jsvisa Date: Thu, 3 Aug 2023 14:05:33 +0800 Subject: [PATCH 08/16] Revert "internal/ethapi: distinguish Call and Transaction nobasefee check" This reverts commit eeaaeb27744179c03654348bc9dba51b4b96eb9a. Signed-off-by: jsvisa --- internal/ethapi/api.go | 12 ++++++------ internal/ethapi/transaction_args.go | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 7f9be23ff949..e130d9c5070f 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1024,7 +1024,7 @@ func (context *ChainContext) GetHeader(hash common.Hash, number uint64) *types.H return header } -func doCall(ctx context.Context, b Backend, args TransactionArgs, state *state.StateDB, header *types.Header, overrides *StateOverride, blockOverrides *BlockOverrides, timeout time.Duration, globalGasCap uint64, noBaseFee bool) (*core.ExecutionResult, error) { +func doCall(ctx context.Context, b Backend, args TransactionArgs, state *state.StateDB, header *types.Header, overrides *StateOverride, blockOverrides *BlockOverrides, timeout time.Duration, globalGasCap uint64) (*core.ExecutionResult, error) { if err := overrides.Apply(state); err != nil { return nil, err } @@ -1049,7 +1049,7 @@ func doCall(ctx context.Context, b Backend, args TransactionArgs, state *state.S if blockOverrides != nil { blockOverrides.Apply(&blockCtx) } - evm, vmError := b.GetEVM(ctx, msg, state, header, &vm.Config{NoBaseFee: noBaseFee}, &blockCtx) + evm, vmError := b.GetEVM(ctx, msg, state, header, &vm.Config{NoBaseFee: true}, &blockCtx) // Wait for the context to be done and cancel the evm. Even if the // EVM has finished, cancelling may be done (repeatedly) @@ -1083,7 +1083,7 @@ func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash return nil, err } - return doCall(ctx, b, args, state, header, overrides, blockOverrides, timeout, globalGasCap, true) + return doCall(ctx, b, args, state, header, overrides, blockOverrides, timeout, globalGasCap) } func newRevertError(result *core.ExecutionResult) *revertError { @@ -1134,7 +1134,7 @@ func (s *BlockChainAPI) Call(ctx context.Context, args TransactionArgs, blockNrO return result.Return(), result.Err } -func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap uint64, noBaseFee bool) (hexutil.Uint64, error) { +func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap uint64) (hexutil.Uint64, error) { // Binary search the gas requirement, as it may be higher than the amount used var ( lo uint64 = params.TxGas - 1 @@ -1208,7 +1208,7 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr executable := func(gas uint64, state *state.StateDB, header *types.Header) (bool, *core.ExecutionResult, error) { args.Gas = (*hexutil.Uint64)(&gas) - result, err := doCall(ctx, b, args, state, header, nil, nil, 0, gasCap, noBaseFee) + result, err := doCall(ctx, b, args, state, header, nil, nil, 0, gasCap) if err != nil { if errors.Is(err, core.ErrIntrinsicGas) { return true, nil, nil // Special case, raise gas limit @@ -1266,7 +1266,7 @@ func (s *BlockChainAPI) EstimateGas(ctx context.Context, args TransactionArgs, b if blockNrOrHash != nil { bNrOrHash = *blockNrOrHash } - return DoEstimateGas(ctx, s.b, args, bNrOrHash, s.b.RPCGasCap(), false) + return DoEstimateGas(ctx, s.b, args, bNrOrHash, s.b.RPCGasCap()) } // RPCMarshalHeader converts the given header to the RPC output . diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index 2c490037861c..6e2690e095d9 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -111,7 +111,7 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error { AccessList: args.AccessList, } pendingBlockNr := rpc.BlockNumberOrHashWithNumber(rpc.PendingBlockNumber) - estimated, err := DoEstimateGas(ctx, b, callArgs, pendingBlockNr, b.RPCGasCap(), false) + estimated, err := DoEstimateGas(ctx, b, callArgs, pendingBlockNr, b.RPCGasCap()) if err != nil { return err } From cd92b3fe6aa9d6af588f0ec4527d83ef298f3525 Mon Sep 17 00:00:00 2001 From: jsvisa Date: Thu, 3 Aug 2023 14:11:48 +0800 Subject: [PATCH 09/16] internal/ethapi: check zero gasPrice after London Signed-off-by: jsvisa --- internal/ethapi/transaction_args.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index 6e2690e095d9..294207d2cc35 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -171,6 +171,10 @@ func (args *TransactionArgs) setFeeDefaults(ctx context.Context, b Backend) erro // setLondonFeeDefaults fills in reasonable default fee values for unspecified fields. func (args *TransactionArgs) setLondonFeeDefaults(ctx context.Context, head *types.Header, b Backend) error { + // After London was activated, a gas price of zero is forbidden. + if args.GasPrice != nil && args.GasPrice.ToInt().Sign() == 0 { + return errors.New("gasPrice must be positive") + } // Set maxPriorityFeePerGas if it is missing. if args.MaxPriorityFeePerGas == nil { tip, err := b.SuggestGasTipCap(ctx) From 4f9cbd86abacaa21f3d7ec14c5fb9117af281eb0 Mon Sep 17 00:00:00 2001 From: jsvisa Date: Thu, 3 Aug 2023 14:26:18 +0800 Subject: [PATCH 10/16] internal/ethapi: add testcase for after London zero price Signed-off-by: jsvisa --- internal/ethapi/transaction_args_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/ethapi/transaction_args_test.go b/internal/ethapi/transaction_args_test.go index 9161d5e681f2..a855b3d6c962 100644 --- a/internal/ethapi/transaction_args_test.go +++ b/internal/ethapi/transaction_args_test.go @@ -52,6 +52,7 @@ func TestSetFeeDefaults(t *testing.T) { var ( b = newBackendMock() + zero = (*hexutil.Big)(big.NewInt(0)) fortytwo = (*hexutil.Big)(big.NewInt(42)) maxFee = (*hexutil.Big)(new(big.Int).Add(new(big.Int).Mul(b.current.BaseFee, big.NewInt(2)), fortytwo.ToInt())) al = &types.AccessList{types.AccessTuple{Address: common.Address{0xaa}, StorageKeys: []common.Hash{{0x01}}}} @@ -184,6 +185,13 @@ func TestSetFeeDefaults(t *testing.T) { nil, errors.New("both gasPrice and (maxFeePerGas or maxPriorityFeePerGas) specified"), }, + { + "set gas price to zero", + true, + &TransactionArgs{GasPrice: zero}, + nil, + errors.New("gasPrice must be positive"), + }, } ctx := context.Background() From 1ae784545ec16f530c186c598dc27e5d7daa8c7b Mon Sep 17 00:00:00 2001 From: jsvisa Date: Thu, 3 Aug 2023 14:35:27 +0800 Subject: [PATCH 11/16] internal/ethapi: check gasPrice >0 Signed-off-by: jsvisa --- internal/ethapi/transaction_args.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index 294207d2cc35..c4427a8d0d78 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -141,7 +141,7 @@ func (args *TransactionArgs) setFeeDefaults(ctx context.Context, b Backend) erro // who are not yet synced past London to get defaults for other tx values. See // https://github.com/ethereum/go-ethereum/pull/23274 for more information. eip1559ParamsSet := args.MaxFeePerGas != nil && args.MaxPriorityFeePerGas != nil - if (args.GasPrice != nil && !eip1559ParamsSet) || (args.GasPrice == nil && eip1559ParamsSet) { + if (args.GasPrice != nil && args.GasPrice.ToInt().Sign() > 0 && !eip1559ParamsSet) || (args.GasPrice == nil && eip1559ParamsSet) { // Sanity check the EIP-1559 fee parameters if present. if args.GasPrice == nil && args.MaxFeePerGas.ToInt().Cmp(args.MaxPriorityFeePerGas.ToInt()) < 0 { return fmt.Errorf("maxFeePerGas (%v) < maxPriorityFeePerGas (%v)", args.MaxFeePerGas, args.MaxPriorityFeePerGas) From afd421766c132fb0440d94f6978418b7a3709944 Mon Sep 17 00:00:00 2001 From: jsvisa Date: Thu, 3 Aug 2023 14:35:50 +0800 Subject: [PATCH 12/16] internal/ethapi: add testcase of pre-london Signed-off-by: jsvisa --- internal/ethapi/transaction_args_test.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/internal/ethapi/transaction_args_test.go b/internal/ethapi/transaction_args_test.go index a855b3d6c962..6862b228274d 100644 --- a/internal/ethapi/transaction_args_test.go +++ b/internal/ethapi/transaction_args_test.go @@ -67,6 +67,13 @@ func TestSetFeeDefaults(t *testing.T) { &TransactionArgs{GasPrice: fortytwo}, nil, }, + { + "legacy tx pre-London with zero price", + false, + &TransactionArgs{GasPrice: zero}, + &TransactionArgs{GasPrice: fortytwo}, + nil, + }, { "legacy tx post-London, explicit gas price", true, @@ -74,6 +81,13 @@ func TestSetFeeDefaults(t *testing.T) { &TransactionArgs{GasPrice: fortytwo}, nil, }, + { + "legacy tx post-London with zero price", + true, + &TransactionArgs{GasPrice: zero}, + nil, + errors.New("gasPrice must be positive"), + }, // Access list txs { @@ -185,13 +199,6 @@ func TestSetFeeDefaults(t *testing.T) { nil, errors.New("both gasPrice and (maxFeePerGas or maxPriorityFeePerGas) specified"), }, - { - "set gas price to zero", - true, - &TransactionArgs{GasPrice: zero}, - nil, - errors.New("gasPrice must be positive"), - }, } ctx := context.Background() From 9120fa212060d583a509a64ca14e16468bfa8962 Mon Sep 17 00:00:00 2001 From: jsvisa Date: Thu, 3 Aug 2023 16:47:54 +0800 Subject: [PATCH 13/16] internal/ethapi: review fix Signed-off-by: jsvisa --- internal/ethapi/transaction_args.go | 22 ++++++++++++++-------- internal/ethapi/transaction_args_test.go | 4 ++-- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index c4427a8d0d78..c17650f96db5 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -141,16 +141,26 @@ func (args *TransactionArgs) setFeeDefaults(ctx context.Context, b Backend) erro // who are not yet synced past London to get defaults for other tx values. See // https://github.com/ethereum/go-ethereum/pull/23274 for more information. eip1559ParamsSet := args.MaxFeePerGas != nil && args.MaxPriorityFeePerGas != nil - if (args.GasPrice != nil && args.GasPrice.ToInt().Sign() > 0 && !eip1559ParamsSet) || (args.GasPrice == nil && eip1559ParamsSet) { + if args.GasPrice == nil && eip1559ParamsSet { // Sanity check the EIP-1559 fee parameters if present. - if args.GasPrice == nil && args.MaxFeePerGas.ToInt().Cmp(args.MaxPriorityFeePerGas.ToInt()) < 0 { + if args.MaxFeePerGas.ToInt().Cmp(args.MaxPriorityFeePerGas.ToInt()) < 0 { return fmt.Errorf("maxFeePerGas (%v) < maxPriorityFeePerGas (%v)", args.MaxFeePerGas, args.MaxPriorityFeePerGas) } - return nil + return nil // No need to set anything, user already set MaxFeePerGas and MaxPriorityFeePerGas } + // Now attempt to fill in default value depending on whether London is active or not. head := b.CurrentHeader() - if b.ChainConfig().IsLondon(head.Number) { + isLondon := b.ChainConfig().IsLondon(head.Number) + if args.GasPrice != nil && !eip1559ParamsSet { + // Sanity-check that zero gasprice is not used post London + if args.GasPrice.ToInt().Sign() == 0 && isLondon { + return errors.New("gasPrice must be non-zero") + } + return nil // No need to set anything, user already set GasPrice + } + + if isLondon { // London is active, set maxPriorityFeePerGas and maxFeePerGas. if err := args.setLondonFeeDefaults(ctx, head, b); err != nil { return err @@ -171,10 +181,6 @@ func (args *TransactionArgs) setFeeDefaults(ctx context.Context, b Backend) erro // setLondonFeeDefaults fills in reasonable default fee values for unspecified fields. func (args *TransactionArgs) setLondonFeeDefaults(ctx context.Context, head *types.Header, b Backend) error { - // After London was activated, a gas price of zero is forbidden. - if args.GasPrice != nil && args.GasPrice.ToInt().Sign() == 0 { - return errors.New("gasPrice must be positive") - } // Set maxPriorityFeePerGas if it is missing. if args.MaxPriorityFeePerGas == nil { tip, err := b.SuggestGasTipCap(ctx) diff --git a/internal/ethapi/transaction_args_test.go b/internal/ethapi/transaction_args_test.go index 6862b228274d..39feb4c08c8b 100644 --- a/internal/ethapi/transaction_args_test.go +++ b/internal/ethapi/transaction_args_test.go @@ -71,7 +71,7 @@ func TestSetFeeDefaults(t *testing.T) { "legacy tx pre-London with zero price", false, &TransactionArgs{GasPrice: zero}, - &TransactionArgs{GasPrice: fortytwo}, + &TransactionArgs{GasPrice: zero}, nil, }, { @@ -86,7 +86,7 @@ func TestSetFeeDefaults(t *testing.T) { true, &TransactionArgs{GasPrice: zero}, nil, - errors.New("gasPrice must be positive"), + errors.New("gasPrice must be non-zero"), }, // Access list txs From d59298fd7aa4e0172269a13f1048a97f5f59fd8b Mon Sep 17 00:00:00 2001 From: jsvisa Date: Wed, 16 Aug 2023 01:44:27 +0800 Subject: [PATCH 14/16] add eip1559 zero check too Signed-off-by: jsvisa --- internal/ethapi/transaction_args.go | 3 +++ internal/ethapi/transaction_args_test.go | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index c17650f96db5..a155cabd1bcd 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -143,6 +143,9 @@ func (args *TransactionArgs) setFeeDefaults(ctx context.Context, b Backend) erro eip1559ParamsSet := args.MaxFeePerGas != nil && args.MaxPriorityFeePerGas != nil if args.GasPrice == nil && eip1559ParamsSet { // Sanity check the EIP-1559 fee parameters if present. + if args.MaxFeePerGas.ToInt().Sign() == 0 { + return errors.New("maxFeePerGas must be non-zero") + } if args.MaxFeePerGas.ToInt().Cmp(args.MaxPriorityFeePerGas.ToInt()) < 0 { return fmt.Errorf("maxFeePerGas (%v) < maxPriorityFeePerGas (%v)", args.MaxFeePerGas, args.MaxPriorityFeePerGas) } diff --git a/internal/ethapi/transaction_args_test.go b/internal/ethapi/transaction_args_test.go index 39feb4c08c8b..3740d655d743 100644 --- a/internal/ethapi/transaction_args_test.go +++ b/internal/ethapi/transaction_args_test.go @@ -176,6 +176,13 @@ func TestSetFeeDefaults(t *testing.T) { nil, errors.New("maxFeePerGas (0x7) < maxPriorityFeePerGas (0x2a)"), }, + { + "dynamic fee tx post-London, explicit gas price", + true, + &TransactionArgs{MaxFeePerGas: zero, MaxPriorityFeePerGas: zero}, + nil, + errors.New("maxFeePerGas must be non-zero"), + }, // Misc { From b65f8200001033c4c4083003a8fb766bdb3ec464 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Mon, 21 Aug 2023 14:07:20 +0800 Subject: [PATCH 15/16] internal/ethapi: polish code --- internal/ethapi/transaction_args.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index a155cabd1bcd..cddd301b60c6 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -137,12 +137,14 @@ func (args *TransactionArgs) setFeeDefaults(ctx context.Context, b Backend) erro if args.GasPrice != nil && (args.MaxFeePerGas != nil || args.MaxPriorityFeePerGas != nil) { return errors.New("both gasPrice and (maxFeePerGas or maxPriorityFeePerGas) specified") } - // If the tx has completely specified a fee mechanism, no default is needed. This allows users - // who are not yet synced past London to get defaults for other tx values. See - // https://github.com/ethereum/go-ethereum/pull/23274 for more information. + // If the tx has completely specified a fee mechanism, no default is needed. + // This allows users who are not yet synced past London to get defaults for + // other tx values. See https://github.com/ethereum/go-ethereum/pull/23274 + // for more information. eip1559ParamsSet := args.MaxFeePerGas != nil && args.MaxPriorityFeePerGas != nil + + // Sanity check the EIP-1559 fee parameters if present. if args.GasPrice == nil && eip1559ParamsSet { - // Sanity check the EIP-1559 fee parameters if present. if args.MaxFeePerGas.ToInt().Sign() == 0 { return errors.New("maxFeePerGas must be non-zero") } @@ -151,18 +153,18 @@ func (args *TransactionArgs) setFeeDefaults(ctx context.Context, b Backend) erro } return nil // No need to set anything, user already set MaxFeePerGas and MaxPriorityFeePerGas } - - // Now attempt to fill in default value depending on whether London is active or not. + // Sanity check the non-EIP-1559 fee parameters. head := b.CurrentHeader() isLondon := b.ChainConfig().IsLondon(head.Number) if args.GasPrice != nil && !eip1559ParamsSet { - // Sanity-check that zero gasprice is not used post London + // Zero gas-price is not allowed after London fork if args.GasPrice.ToInt().Sign() == 0 && isLondon { - return errors.New("gasPrice must be non-zero") + return errors.New("gasPrice must be non-zero after london fork") } return nil // No need to set anything, user already set GasPrice } + // Now attempt to fill in default value depending on whether London is active or not. if isLondon { // London is active, set maxPriorityFeePerGas and maxFeePerGas. if err := args.setLondonFeeDefaults(ctx, head, b); err != nil { From ef96ce1ef4ade4e0a08852dc5a415be632099636 Mon Sep 17 00:00:00 2001 From: jsvisa Date: Mon, 21 Aug 2023 21:32:25 +0800 Subject: [PATCH 16/16] internal/ethapi: test case error msg Signed-off-by: jsvisa --- internal/ethapi/transaction_args_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ethapi/transaction_args_test.go b/internal/ethapi/transaction_args_test.go index 3740d655d743..d69ce41184a3 100644 --- a/internal/ethapi/transaction_args_test.go +++ b/internal/ethapi/transaction_args_test.go @@ -86,7 +86,7 @@ func TestSetFeeDefaults(t *testing.T) { true, &TransactionArgs{GasPrice: zero}, nil, - errors.New("gasPrice must be non-zero"), + errors.New("gasPrice must be non-zero after london fork"), }, // Access list txs