From b3996e4719b32e05be1f01046742ecd8416f1d1b Mon Sep 17 00:00:00 2001 From: Sina Mahmoodi Date: Wed, 20 Sep 2023 19:25:36 +0200 Subject: [PATCH] dont set defaults for calls --- internal/ethapi/api.go | 7 +-- internal/ethapi/transaction_args.go | 59 ++++++++++++++++-------- internal/ethapi/transaction_args_test.go | 14 ++++-- 3 files changed, 55 insertions(+), 25 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 47687fb242..a1d29ec523 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1312,10 +1312,11 @@ func (s *BlockChainAPI) MulticallV1(ctx context.Context, opts multicallOpts, blo call.Gas = (*hexutil.Uint64)(&remaining) } if call.GasPrice == nil && call.MaxFeePerGas == nil && call.MaxPriorityFeePerGas == nil { - call.GasPrice = (*hexutil.Big)(big.NewInt(0)) + call.MaxFeePerGas = (*hexutil.Big)(big.NewInt(0)) + call.MaxPriorityFeePerGas = (*hexutil.Big)(big.NewInt(0)) } - // TODO: Tx and message used for executing will probably have different values. - if err := call.setDefaults(ctx, s.b); err != nil { + // TODO: check chainID and against current header for london fees + if err := call.validateAll(); err != nil { return nil, err } tx := call.ToTransaction(true) diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index 95983d797c..7bad196f02 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -74,8 +74,46 @@ func (args *TransactionArgs) data() []byte { return nil } +func (args *TransactionArgs) validateAll() error { + if err := args.validate(); err != nil { + return err + } + return args.validateFees() +} + +func (args *TransactionArgs) validate() error { + if args.Data != nil && args.Input != nil && !bytes.Equal(*args.Data, *args.Input) { + return errors.New(`both "data" and "input" are set and not equal. Please use "input" to pass transaction call data`) + } + if args.To == nil && len(args.data()) == 0 { + return errors.New(`contract creation without any data provided`) + } + return nil +} + +func (args *TransactionArgs) validateFees() error { + // If both gasPrice and at least one of the EIP-1559 fee parameters are specified, error. + 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. + eip1559ParamsSet := args.MaxFeePerGas != nil && args.MaxPriorityFeePerGas != nil + if (args.GasPrice != nil && !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) + } + } + return nil +} + // setDefaults fills in default values for unspecified tx fields. func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error { + if err := args.validate(); err != nil { + return err + } if err := args.setFeeDefaults(ctx, b); err != nil { return err } @@ -89,12 +127,6 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error { } args.Nonce = (*hexutil.Uint64)(&nonce) } - if args.Data != nil && args.Input != nil && !bytes.Equal(*args.Data, *args.Input) { - return errors.New(`both "data" and "input" are set and not equal. Please use "input" to pass transaction call data`) - } - if args.To == nil && len(args.data()) == 0 { - return errors.New(`contract creation without any data provided`) - } // Estimate the gas usage if necessary. if args.Gas == nil { // These fields are immutable during the estimation, safe to @@ -133,19 +165,10 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error { // setFeeDefaults fills in default fee values for unspecified tx fields. func (args *TransactionArgs) setFeeDefaults(ctx context.Context, b Backend) error { - // If both gasPrice and at least one of the EIP-1559 fee parameters are specified, error. - if args.GasPrice != nil && (args.MaxFeePerGas != nil || args.MaxPriorityFeePerGas != nil) { - return errors.New("both gasPrice and (maxFeePerGas or maxPriorityFeePerGas) specified") + if err := args.validateFees(); err != nil { + return err } - // 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 - if (args.GasPrice != nil && !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) - } + if args.GasPrice != nil && args.MaxFeePerGas == nil && args.MaxPriorityFeePerGas == nil { return nil } // Now attempt to fill in default value depending on whether London is active or not. diff --git a/internal/ethapi/transaction_args_test.go b/internal/ethapi/transaction_args_test.go index 9161d5e681..d4b7d83b41 100644 --- a/internal/ethapi/transaction_args_test.go +++ b/internal/ethapi/transaction_args_test.go @@ -19,6 +19,7 @@ package ethapi import ( "context" "errors" + "fmt" "math/big" "reflect" "testing" @@ -195,10 +196,15 @@ func TestSetFeeDefaults(t *testing.T) { } got := test.in err := got.setFeeDefaults(ctx, b) - if err != nil && err.Error() == test.err.Error() { - // Test threw expected error. - continue - } else if err != nil { + fmt.Printf("err: %v\n", err) + if err != nil { + if test.err == nil { + t.Fatalf("test %d (%s): unexpected error: %s", i, test.name, err) + } + if err.Error() == test.err.Error() { + // Test threw expected error. + continue + } t.Fatalf("test %d (%s): unexpected error: %s", i, test.name, err) } if !reflect.DeepEqual(got, test.want) {