From 53f787293602e2a74713a66a08425ba2a97a85f3 Mon Sep 17 00:00:00 2001 From: colinlyguo Date: Sun, 21 Jan 2024 05:40:29 +0800 Subject: [PATCH 1/4] internal/ethapi: set block gas limit as msg gas limit when rpc gas cap is set 0 (infinite) --- internal/ethapi/api.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index ee479d7139ab..80e32880c547 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1513,8 +1513,13 @@ func AccessList(ctx context.Context, b Backend, blockNrOrHash rpc.BlockNumberOrH } // If the gas amount is not set, default to RPC gas cap. if args.Gas == nil { - tmp := hexutil.Uint64(b.RPCGasCap()) - args.Gas = &tmp + if b.RPCGasCap() != 0 { + tmp := hexutil.Uint64(b.RPCGasCap()) + args.Gas = &tmp + } else { + tmp := hexutil.Uint64(b.CurrentBlock().GasLimit) + args.Gas = &tmp + } } // Ensure any missing fields are filled, extract the recipient and input data From 76cad9bae5ddc964ce8915e006a27b11843313c3 Mon Sep 17 00:00:00 2001 From: colinlyguo Date: Tue, 23 Jan 2024 00:21:19 +0800 Subject: [PATCH 2/4] change default gas limit --- 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 80e32880c547..68b2ccf8ecad 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1517,7 +1517,7 @@ func AccessList(ctx context.Context, b Backend, blockNrOrHash rpc.BlockNumberOrH tmp := hexutil.Uint64(b.RPCGasCap()) args.Gas = &tmp } else { - tmp := hexutil.Uint64(b.CurrentBlock().GasLimit) + tmp := hexutil.Uint64(math.MaxUint64 / 2) args.Gas = &tmp } } From 942453152e0646d8fed8b17fa86cfee3a30a8a34 Mon Sep 17 00:00:00 2001 From: colinlyguo Date: Mon, 29 Jan 2024 11:52:15 +0800 Subject: [PATCH 3/4] add a bool flag to bypass gas estimation in setDefaults --- internal/ethapi/api.go | 22 ++++++---------------- internal/ethapi/transaction_args.go | 8 +++++++- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 68b2ccf8ecad..898e70ea6385 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -451,7 +451,7 @@ func (s *PersonalAccountAPI) signTransaction(ctx context.Context, args *Transact return nil, err } // Set some sanity defaults and terminate on failure - if err := args.setDefaults(ctx, s.b); err != nil { + if err := args.setDefaults(ctx, s.b, false); err != nil { return nil, err } // Assemble the transaction and sign with the wallet @@ -1511,19 +1511,9 @@ func AccessList(ctx context.Context, b Backend, blockNrOrHash rpc.BlockNumberOrH if db == nil || err != nil { return nil, 0, nil, err } - // If the gas amount is not set, default to RPC gas cap. - if args.Gas == nil { - if b.RPCGasCap() != 0 { - tmp := hexutil.Uint64(b.RPCGasCap()) - args.Gas = &tmp - } else { - tmp := hexutil.Uint64(math.MaxUint64 / 2) - args.Gas = &tmp - } - } // Ensure any missing fields are filled, extract the recipient and input data - if err := args.setDefaults(ctx, b); err != nil { + if err := args.setDefaults(ctx, b, true); err != nil { return nil, 0, nil, err } var to common.Address @@ -1828,7 +1818,7 @@ func (s *TransactionAPI) SendTransaction(ctx context.Context, args TransactionAr } // Set some sanity defaults and terminate on failure - if err := args.setDefaults(ctx, s.b); err != nil { + if err := args.setDefaults(ctx, s.b, false); err != nil { return common.Hash{}, err } // Assemble the transaction and sign with the wallet @@ -1846,7 +1836,7 @@ func (s *TransactionAPI) SendTransaction(ctx context.Context, args TransactionAr // processing (signing + broadcast). func (s *TransactionAPI) FillTransaction(ctx context.Context, args TransactionArgs) (*SignTransactionResult, error) { // Set some sanity defaults and terminate on failure - if err := args.setDefaults(ctx, s.b); err != nil { + if err := args.setDefaults(ctx, s.b, false); err != nil { return nil, err } // Assemble the transaction and obtain rlp @@ -1916,7 +1906,7 @@ func (s *TransactionAPI) SignTransaction(ctx context.Context, args TransactionAr if args.Nonce == nil { return nil, errors.New("nonce not specified") } - if err := args.setDefaults(ctx, s.b); err != nil { + if err := args.setDefaults(ctx, s.b, false); err != nil { return nil, err } // Before actually sign the transaction, ensure the transaction fee is reasonable. @@ -1965,7 +1955,7 @@ func (s *TransactionAPI) Resend(ctx context.Context, sendArgs TransactionArgs, g if sendArgs.Nonce == nil { return common.Hash{}, errors.New("missing transaction nonce in transaction spec") } - if err := sendArgs.setDefaults(ctx, s.b); err != nil { + if err := sendArgs.setDefaults(ctx, s.b, false); err != nil { return common.Hash{}, err } matchTx := sendArgs.toTransaction() diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index 75dbe38a59e8..d4b70922324e 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -81,7 +81,7 @@ func (args *TransactionArgs) data() []byte { } // setDefaults fills in default values for unspecified tx fields. -func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error { +func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend, infiniteGas bool) error { if err := args.setFeeDefaults(ctx, b); err != nil { return err } @@ -107,6 +107,12 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error { if args.To == nil && len(args.data()) == 0 { return errors.New(`contract creation without any data provided`) } + // Assign a very high gas limit for cases where an accurate gas limit is not critical, + // but need to ensure that gas is sufficient. + if infiniteGas { + tmp := hexutil.Uint64(math.MaxUint64 / 2) + args.Gas = &tmp + } // Estimate the gas usage if necessary. if args.Gas == nil { // These fields are immutable during the estimation, safe to From ee59f15a1069b6eff58eff54754fd3a59ea67620 Mon Sep 17 00:00:00 2001 From: colinlyguo Date: Thu, 15 Feb 2024 02:18:44 +0800 Subject: [PATCH 4/4] address comments --- internal/ethapi/transaction_args.go | 57 ++++++++++++++--------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index 57ce4cdf5b60..e86173489364 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -96,7 +96,7 @@ func (args *TransactionArgs) data() []byte { } // setDefaults fills in default values for unspecified tx fields. -func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend, infiniteGas bool) error { +func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend, skipGasEstimation bool) error { if err := args.setBlobTxSidecar(ctx, b); err != nil { return err } @@ -135,37 +135,36 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend, infinit return errors.New(`contract creation without any data provided`) } } - // Assign a very high gas limit for cases where an accurate gas limit is not critical, - // but need to ensure that gas is sufficient. - if infiniteGas { - tmp := hexutil.Uint64(math.MaxUint64 / 2) - args.Gas = &tmp - } - // Estimate the gas usage if necessary. if args.Gas == nil { - // These fields are immutable during the estimation, safe to - // pass the pointer directly. - data := args.data() - callArgs := TransactionArgs{ - From: args.From, - To: args.To, - GasPrice: args.GasPrice, - MaxFeePerGas: args.MaxFeePerGas, - MaxPriorityFeePerGas: args.MaxPriorityFeePerGas, - Value: args.Value, - Data: (*hexutil.Bytes)(&data), - AccessList: args.AccessList, - BlobFeeCap: args.BlobFeeCap, - BlobHashes: args.BlobHashes, - } - latestBlockNr := rpc.BlockNumberOrHashWithNumber(rpc.LatestBlockNumber) - estimated, err := DoEstimateGas(ctx, b, callArgs, latestBlockNr, nil, b.RPCGasCap()) - if err != nil { - return err + if skipGasEstimation { // Skip gas usage estimation if a precise gas limit is not critical, e.g., in non-transaction calls. + gas := hexutil.Uint64(b.RPCGasCap()) + if gas == 0 { + gas = hexutil.Uint64(math.MaxUint64 / 2) + } + args.Gas = &gas + } else { // Estimate the gas usage otherwise. + // These fields are immutable during the estimation, safe to + // pass the pointer directly. + data := args.data() + callArgs := TransactionArgs{ + From: args.From, + To: args.To, + GasPrice: args.GasPrice, + MaxFeePerGas: args.MaxFeePerGas, + MaxPriorityFeePerGas: args.MaxPriorityFeePerGas, + Value: args.Value, + Data: (*hexutil.Bytes)(&data), + AccessList: args.AccessList, + } + latestBlockNr := rpc.BlockNumberOrHashWithNumber(rpc.LatestBlockNumber) + estimated, err := DoEstimateGas(ctx, b, callArgs, latestBlockNr, nil, b.RPCGasCap()) + if err != nil { + return err + } + args.Gas = &estimated + log.Trace("Estimate gas usage automatically", "gas", args.Gas) } - args.Gas = &estimated - log.Trace("Estimate gas usage automatically", "gas", args.Gas) } // If chain id is provided, ensure it matches the local chain id. Otherwise, set the local