From ec993734342d99fa2ca2ac5a37801392203f0504 Mon Sep 17 00:00:00 2001 From: Ljubisa Gacevic Date: Tue, 3 Feb 2026 11:41:49 +0100 Subject: [PATCH 1/3] fix(transaction): improve gas estimation and prevent intrinsic gas too low errors --- pkg/transaction/backend/backend.go | 2 +- pkg/transaction/backendmock/backend.go | 42 ++--- pkg/transaction/backendnoop/backend.go | 2 +- pkg/transaction/backendsimulation/backend.go | 26 +-- pkg/transaction/transaction.go | 163 ++++++++++++++++--- pkg/transaction/transaction_test.go | 12 +- pkg/transaction/wrapped/wrapped.go | 4 +- 7 files changed, 188 insertions(+), 63 deletions(-) diff --git a/pkg/transaction/backend/backend.go b/pkg/transaction/backend/backend.go index 827e7f8176a..67323d14e45 100644 --- a/pkg/transaction/backend/backend.go +++ b/pkg/transaction/backend/backend.go @@ -20,7 +20,7 @@ type Geth interface { CallContract(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) ([]byte, error) ChainID(ctx context.Context) (*big.Int, error) Close() - EstimateGasAtBlock(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) (uint64, error) + EstimateGas(ctx context.Context, msg ethereum.CallMsg) (uint64, error) FilterLogs(ctx context.Context, q ethereum.FilterQuery) ([]types.Log, error) HeaderByNumber(ctx context.Context, number *big.Int) (*types.Header, error) NonceAt(ctx context.Context, account common.Address, blockNumber *big.Int) (uint64, error) diff --git a/pkg/transaction/backendmock/backend.go b/pkg/transaction/backendmock/backend.go index e07095fd352..364493b0341 100644 --- a/pkg/transaction/backendmock/backend.go +++ b/pkg/transaction/backendmock/backend.go @@ -15,12 +15,14 @@ import ( "github.com/ethersphere/bee/v2/pkg/transaction" ) +var ErrNotImplemented = errors.New("not implemented") + type backendMock struct { callContract func(ctx context.Context, call ethereum.CallMsg, blockNumber *big.Int) ([]byte, error) sendTransaction func(ctx context.Context, tx *types.Transaction) error suggestedFeeAndTip func(ctx context.Context, gasPrice *big.Int, boostPercent int) (*big.Int, *big.Int, error) suggestGasTipCap func(ctx context.Context) (*big.Int, error) - estimateGasAtBlock func(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) (gas uint64, err error) + estimateGas func(ctx context.Context, msg ethereum.CallMsg) (gas uint64, err error) transactionReceipt func(ctx context.Context, txHash common.Hash) (*types.Receipt, error) pendingNonceAt func(ctx context.Context, account common.Address) (uint64, error) transactionByHash func(ctx context.Context, hash common.Hash) (tx *types.Transaction, isPending bool, err error) @@ -34,92 +36,92 @@ func (m *backendMock) CallContract(ctx context.Context, call ethereum.CallMsg, b if m.callContract != nil { return m.callContract(ctx, call, blockNumber) } - return nil, errors.New("not implemented") + return nil, ErrNotImplemented } func (m *backendMock) PendingNonceAt(ctx context.Context, account common.Address) (uint64, error) { if m.pendingNonceAt != nil { return m.pendingNonceAt(ctx, account) } - return 0, errors.New("not implemented") + return 0, ErrNotImplemented } func (m *backendMock) SuggestedFeeAndTip(ctx context.Context, gasPrice *big.Int, boostPercent int) (*big.Int, *big.Int, error) { if m.suggestedFeeAndTip != nil { return m.suggestedFeeAndTip(ctx, gasPrice, boostPercent) } - return nil, nil, errors.New("not implemented") + return nil, nil, ErrNotImplemented } -func (m *backendMock) EstimateGasAtBlock(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) (uint64, error) { - if m.estimateGasAtBlock != nil { - return m.estimateGasAtBlock(ctx, msg, blockNumber) +func (m *backendMock) EstimateGas(ctx context.Context, msg ethereum.CallMsg) (uint64, error) { + if m.estimateGas != nil { + return m.estimateGas(ctx, msg) } - return 0, errors.New("not implemented") + return 0, ErrNotImplemented } func (m *backendMock) SendTransaction(ctx context.Context, tx *types.Transaction) error { if m.sendTransaction != nil { return m.sendTransaction(ctx, tx) } - return errors.New("not implemented") + return ErrNotImplemented } func (*backendMock) FilterLogs(ctx context.Context, query ethereum.FilterQuery) ([]types.Log, error) { - return nil, errors.New("not implemented") + return nil, ErrNotImplemented } func (m *backendMock) TransactionReceipt(ctx context.Context, txHash common.Hash) (*types.Receipt, error) { if m.transactionReceipt != nil { return m.transactionReceipt(ctx, txHash) } - return nil, errors.New("not implemented") + return nil, ErrNotImplemented } func (m *backendMock) TransactionByHash(ctx context.Context, hash common.Hash) (tx *types.Transaction, isPending bool, err error) { if m.transactionByHash != nil { return m.transactionByHash(ctx, hash) } - return nil, false, errors.New("not implemented") + return nil, false, ErrNotImplemented } func (m *backendMock) BlockNumber(ctx context.Context) (uint64, error) { if m.blockNumber != nil { return m.blockNumber(ctx) } - return 0, errors.New("not implemented") + return 0, ErrNotImplemented } func (m *backendMock) HeaderByNumber(ctx context.Context, number *big.Int) (*types.Header, error) { if m.headerByNumber != nil { return m.headerByNumber(ctx, number) } - return nil, errors.New("not implemented") + return nil, ErrNotImplemented } func (m *backendMock) BalanceAt(ctx context.Context, address common.Address, block *big.Int) (*big.Int, error) { if m.balanceAt != nil { return m.balanceAt(ctx, address, block) } - return nil, errors.New("not implemented") + return nil, ErrNotImplemented } func (m *backendMock) NonceAt(ctx context.Context, account common.Address, blockNumber *big.Int) (uint64, error) { if m.nonceAt != nil { return m.nonceAt(ctx, account, blockNumber) } - return 0, errors.New("not implemented") + return 0, ErrNotImplemented } func (m *backendMock) SuggestGasTipCap(ctx context.Context) (*big.Int, error) { if m.suggestGasTipCap != nil { return m.suggestGasTipCap(ctx) } - return nil, errors.New("not implemented") + return nil, ErrNotImplemented } func (m *backendMock) ChainID(ctx context.Context) (*big.Int, error) { - return nil, errors.New("not implemented") + return nil, ErrNotImplemented } func (m *backendMock) Close() {} @@ -171,9 +173,9 @@ func WithSuggestGasTipCapFunc(f func(ctx context.Context) (*big.Int, error)) Opt }) } -func WithEstimateGasAtBlockFunc(f func(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) (gas uint64, err error)) Option { +func WithEstimateGasFunc(f func(ctx context.Context, msg ethereum.CallMsg) (gas uint64, err error)) Option { return optionFunc(func(s *backendMock) { - s.estimateGasAtBlock = f + s.estimateGas = f }) } diff --git a/pkg/transaction/backendnoop/backend.go b/pkg/transaction/backendnoop/backend.go index cc09b459bd2..4b149369981 100644 --- a/pkg/transaction/backendnoop/backend.go +++ b/pkg/transaction/backendnoop/backend.go @@ -55,7 +55,7 @@ func (b *Backend) SuggestGasTipCap(context.Context) (*big.Int, error) { return nil, postagecontract.ErrChainDisabled } -func (b *Backend) EstimateGasAtBlock(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) (uint64, error) { +func (b *Backend) EstimateGas(ctx context.Context, msg ethereum.CallMsg) (uint64, error) { return 0, postagecontract.ErrChainDisabled } diff --git a/pkg/transaction/backendsimulation/backend.go b/pkg/transaction/backendsimulation/backend.go index eacddc44f0f..9c647b6404a 100644 --- a/pkg/transaction/backendsimulation/backend.go +++ b/pkg/transaction/backendsimulation/backend.go @@ -16,6 +16,8 @@ import ( "github.com/ethersphere/bee/v2/pkg/transaction" ) +var ErrNotImplemented = errors.New("not implemented") + type AccountAtKey struct { BlockNumber uint64 Account common.Address @@ -84,27 +86,27 @@ func (m *simulatedBackend) advanceBlock() { } func (*simulatedBackend) CallContract(ctx context.Context, call ethereum.CallMsg, blockNumber *big.Int) ([]byte, error) { - return nil, errors.New("not implemented") + return nil, ErrNotImplemented } func (m *simulatedBackend) PendingNonceAt(ctx context.Context, account common.Address) (uint64, error) { - return 0, errors.New("not implemented") + return 0, ErrNotImplemented } func (m *simulatedBackend) SuggestedFeeAndTip(ctx context.Context, gasPrice *big.Int, boostPercent int) (*big.Int, *big.Int, error) { - return nil, nil, errors.New("not implemented") + return nil, nil, ErrNotImplemented } -func (m *simulatedBackend) EstimateGasAtBlock(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) (uint64, error) { - return 0, errors.New("not implemented") +func (m *simulatedBackend) EstimateGas(ctx context.Context, msg ethereum.CallMsg) (uint64, error) { + return 0, ErrNotImplemented } func (m *simulatedBackend) SendTransaction(ctx context.Context, tx *types.Transaction) error { - return errors.New("not implemented") + return ErrNotImplemented } func (*simulatedBackend) FilterLogs(ctx context.Context, query ethereum.FilterQuery) ([]types.Log, error) { - return nil, errors.New("not implemented") + return nil, ErrNotImplemented } func (m *simulatedBackend) TransactionReceipt(ctx context.Context, txHash common.Hash) (*types.Receipt, error) { @@ -117,7 +119,7 @@ func (m *simulatedBackend) TransactionReceipt(ctx context.Context, txHash common } func (m *simulatedBackend) TransactionByHash(ctx context.Context, hash common.Hash) (tx *types.Transaction, isPending bool, err error) { - return nil, false, errors.New("not implemented") + return nil, false, ErrNotImplemented } func (m *simulatedBackend) BlockNumber(ctx context.Context) (uint64, error) { @@ -126,11 +128,11 @@ func (m *simulatedBackend) BlockNumber(ctx context.Context) (uint64, error) { } func (m *simulatedBackend) HeaderByNumber(ctx context.Context, number *big.Int) (*types.Header, error) { - return nil, errors.New("not implemented") + return nil, ErrNotImplemented } func (m *simulatedBackend) BalanceAt(ctx context.Context, address common.Address, block *big.Int) (*big.Int, error) { - return nil, errors.New("not implemented") + return nil, ErrNotImplemented } func (m *simulatedBackend) NonceAt(ctx context.Context, account common.Address, blockNumber *big.Int) (uint64, error) { @@ -143,11 +145,11 @@ func (m *simulatedBackend) NonceAt(ctx context.Context, account common.Address, } func (m *simulatedBackend) SuggestGasTipCap(ctx context.Context) (*big.Int, error) { - return nil, errors.New("not implemented") + return nil, ErrNotImplemented } func (m *simulatedBackend) ChainID(ctx context.Context) (*big.Int, error) { - return nil, errors.New("not implemented") + return nil, ErrNotImplemented } func (m *simulatedBackend) Close() {} diff --git a/pkg/transaction/transaction.go b/pkg/transaction/transaction.go index cc88e7a3ab4..6a8a22dc8aa 100644 --- a/pkg/transaction/transaction.go +++ b/pkg/transaction/transaction.go @@ -44,8 +44,12 @@ var ( ) const ( - DefaultGasLimit = 1_000_000 + DefaultGasLimit = 1_000_000 // Used for contract operations when setGasLimit flag is enabled DefaultTipBoostPercent = 25 + MaxGasLimit = 10_000_000 // Maximum allowed gas limit to prevent excessive values + MinGasLimit = 21_000 // Minimum gas for any transaction + GasBufferPercent = 33 // Add 33% buffer to estimated gas + FallbackGasLimit = 500_000 // Fallback when estimation fails and no minimum is set ) // TxRequest describes a request for a transaction that can be executed. @@ -271,26 +275,6 @@ func (t *transactionService) StoredTransaction(txHash common.Hash) (*StoredTrans // prepareTransaction creates a signable transaction based on a request. func (t *transactionService) prepareTransaction(ctx context.Context, request *TxRequest, nonce uint64, boostPercent int) (tx *types.Transaction, err error) { - var gasLimit uint64 - if request.GasLimit == 0 { - gasLimit, err = t.backend.EstimateGasAtBlock(ctx, ethereum.CallMsg{ - From: t.sender, - To: request.To, - Data: request.Data, - }, nil) // nil for latest block - if err != nil { - t.logger.Debug("estimate gas failed", "error", err) - gasLimit = request.MinEstimatedGasLimit - } - - gasLimit += gasLimit / 2 // add 50% buffer to the estimated gas limit - if gasLimit < request.MinEstimatedGasLimit { - gasLimit = request.MinEstimatedGasLimit - } - } else { - gasLimit = request.GasLimit - } - /* Transactions are EIP 1559 dynamic transactions where there are three fee related fields: 1. base fee is the price that will be burned as part of the transaction. @@ -303,11 +287,148 @@ func (t *transactionService) prepareTransaction(ctx context.Context, request *Tx notice that gas price does not exceed 20 as defined by max fee. */ + // Calculate gas fees first so we can use them in gas estimation for more accurate simulation gasFeeCap, gasTipCap, err := t.backend.SuggestedFeeAndTip(ctx, request.GasPrice, boostPercent) if err != nil { return nil, err } + var gasLimit uint64 + if request.GasLimit == 0 { + // Estimate gas using pending state to account for pending transactions + // This is consistent with using PendingNonceAt for nonce selection + gasLimit, err = t.backend.EstimateGas(ctx, ethereum.CallMsg{ + From: t.sender, + To: request.To, + Data: request.Data, + Value: request.Value, + GasFeeCap: gasFeeCap, + GasTipCap: gasTipCap, + }) + + if err != nil { + // Gas estimation failed - analyze error to provide better diagnostics + errStr := err.Error() + + // Try to get revert reason for better error reporting + var revertReason string + if strings.Contains(errStr, "execution reverted") || + strings.Contains(errStr, "Transaction execution fails") || + strings.Contains(errStr, "always failing transaction") { + // Attempt to call contract to get revert reason + if output, callErr := t.backend.CallContract(ctx, ethereum.CallMsg{ + From: t.sender, + To: request.To, + Data: request.Data, + Value: request.Value, + GasFeeCap: gasFeeCap, + GasTipCap: gasTipCap, + }, nil); callErr == nil && len(output) > 0 { + revertReason = fmt.Sprintf("revert_data=0x%x", output) + } else if callErr != nil { + revertReason = fmt.Sprintf("call_error=%v", callErr) + } + } + + switch { + case strings.Contains(errStr, "execution reverted") || + strings.Contains(errStr, "Transaction execution fails"): + t.logger.Error(err, "transaction would revert if sent - contract requirements not met", + "description", request.Description, + "to", request.To, + "revert_info", revertReason, + ) + + case strings.Contains(errStr, "always failing transaction"): + t.logger.Error(err, "transaction always fails - invalid parameters or contract state", + "description", request.Description, + "to", request.To, + "revert_info", revertReason, + ) + + case strings.Contains(errStr, "insufficient funds") || + strings.Contains(errStr, "exceeds balance"): + t.logger.Error(err, "insufficient balance to execute transaction", + "description", request.Description, + "value", request.Value, + ) + + case strings.Contains(errStr, "gas required exceeds allowance") || + strings.Contains(errStr, "out of gas"): + t.logger.Error(err, "transaction requires too much gas", + "description", request.Description, + ) + + case strings.Contains(errStr, "nonce too low"): + t.logger.Error(err, "nonce conflict detected", + "description", request.Description, + ) + + default: + t.logger.Warning("gas estimation failed, using fallback", + "error", err, + "description", request.Description, + ) + } + + // Use fallback gas limit with priority order + if request.MinEstimatedGasLimit > 0 { + gasLimit = request.MinEstimatedGasLimit + } else if len(request.Data) > 0 { + // Contract interaction - use reasonable fallback + gasLimit = FallbackGasLimit + } else { + // Simple transfer - use minimum + gasLimit = MinGasLimit + } + } else { + // Gas estimation succeeded - add buffer to handle state changes + buffer := gasLimit * GasBufferPercent / 100 + gasLimit += buffer + + // Ensure minimum requirement is met + if gasLimit < request.MinEstimatedGasLimit { + gasLimit = request.MinEstimatedGasLimit + } + + // Cap at maximum to prevent excessive gas limits + if gasLimit > MaxGasLimit { + t.logger.Warning("estimated gas exceeds maximum, capping", + "estimated", gasLimit, + "max", MaxGasLimit, + "description", request.Description, + ) + gasLimit = MaxGasLimit + } + } + + // Final safety check - ensure absolute minimum + if gasLimit < MinGasLimit { + gasLimit = MinGasLimit + } + } else { + // User provided explicit gas limit - use it but validate + gasLimit = request.GasLimit + if gasLimit < MinGasLimit { + t.logger.Warning("provided gas limit too low, using minimum", + "provided", gasLimit, + "minimum", MinGasLimit, + ) + gasLimit = MinGasLimit + } + if gasLimit > MaxGasLimit { + t.logger.Warning("provided gas limit too high, capping", + "provided", gasLimit, + "maximum", MaxGasLimit, + ) + gasLimit = MaxGasLimit + } + } + + if gasLimit == 0 { + return nil, errors.New("gas limit cannot be zero") + } + return types.NewTx(&types.DynamicFeeTx{ Nonce: nonce, ChainID: t.chainID, diff --git a/pkg/transaction/transaction_test.go b/pkg/transaction/transaction_test.go index e9f4e385c32..b84d7031a76 100644 --- a/pkg/transaction/transaction_test.go +++ b/pkg/transaction/transaction_test.go @@ -151,7 +151,7 @@ func TestTransactionSend(t *testing.T) { } return nil }), - backendmock.WithEstimateGasAtBlockFunc(func(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) (gas uint64, err error) { + backendmock.WithEstimateGasFunc(func(ctx context.Context, msg ethereum.CallMsg) (gas uint64, err error) { if !bytes.Equal(msg.To.Bytes(), recipient.Bytes()) { t.Fatalf("estimating with wrong recipient. wanted %x, got %x", recipient, msg.To) } @@ -234,7 +234,7 @@ func TestTransactionSend(t *testing.T) { } return nil }), - backendmock.WithEstimateGasAtBlockFunc(func(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) (gas uint64, err error) { + backendmock.WithEstimateGasFunc(func(ctx context.Context, msg ethereum.CallMsg) (gas uint64, err error) { return 0, errors.New("estimate failure") }), backendmock.WithPendingNonceAtFunc(func(ctx context.Context, account common.Address) (uint64, error) { @@ -314,7 +314,7 @@ func TestTransactionSend(t *testing.T) { } return nil }), - backendmock.WithEstimateGasAtBlockFunc(func(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) (gas uint64, err error) { + backendmock.WithEstimateGasFunc(func(ctx context.Context, msg ethereum.CallMsg) (gas uint64, err error) { if !bytes.Equal(msg.To.Bytes(), recipient.Bytes()) { t.Fatalf("estimating with wrong recipient. wanted %x, got %x", recipient, msg.To) } @@ -396,7 +396,7 @@ func TestTransactionSend(t *testing.T) { } return nil }), - backendmock.WithEstimateGasAtBlockFunc(func(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) (gas uint64, err error) { + backendmock.WithEstimateGasFunc(func(ctx context.Context, msg ethereum.CallMsg) (gas uint64, err error) { if !bytes.Equal(msg.To.Bytes(), recipient.Bytes()) { t.Fatalf("estimating with wrong recipient. wanted %x, got %x", recipient, msg.To) } @@ -461,7 +461,7 @@ func TestTransactionSend(t *testing.T) { } return nil }), - backendmock.WithEstimateGasAtBlockFunc(func(ctx context.Context, call ethereum.CallMsg, blockNumber *big.Int) (gas uint64, err error) { + backendmock.WithEstimateGasFunc(func(ctx context.Context, call ethereum.CallMsg) (gas uint64, err error) { if !bytes.Equal(call.To.Bytes(), recipient.Bytes()) { t.Fatalf("estimating with wrong recipient. wanted %x, got %x", recipient, call.To) } @@ -527,7 +527,7 @@ func TestTransactionSend(t *testing.T) { } return nil }), - backendmock.WithEstimateGasAtBlockFunc(func(ctx context.Context, call ethereum.CallMsg, blockNumber *big.Int) (gas uint64, err error) { + backendmock.WithEstimateGasFunc(func(ctx context.Context, call ethereum.CallMsg) (gas uint64, err error) { if !bytes.Equal(call.To.Bytes(), recipient.Bytes()) { t.Fatalf("estimating with wrong recipient. wanted %x, got %x", recipient, call.To) } diff --git a/pkg/transaction/wrapped/wrapped.go b/pkg/transaction/wrapped/wrapped.go index 1d4e452aef3..f573934a7d3 100644 --- a/pkg/transaction/wrapped/wrapped.go +++ b/pkg/transaction/wrapped/wrapped.go @@ -138,10 +138,10 @@ func (b *wrappedBackend) SuggestGasTipCap(ctx context.Context) (*big.Int, error) } return gasTipCap, nil } -func (b *wrappedBackend) EstimateGasAtBlock(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) (uint64, error) { +func (b *wrappedBackend) EstimateGas(ctx context.Context, msg ethereum.CallMsg) (uint64, error) { b.metrics.TotalRPCCalls.Inc() b.metrics.EstimateGasCalls.Inc() - gas, err := b.backend.EstimateGasAtBlock(ctx, msg, blockNumber) + gas, err := b.backend.EstimateGas(ctx, msg) if err != nil { b.metrics.TotalRPCErrors.Inc() return 0, err From e6b17072f9810eb5b3ae91510b72a5e58e5c04dc Mon Sep 17 00:00:00 2001 From: Ljubisa Gacevic Date: Tue, 3 Feb 2026 13:29:53 +0100 Subject: [PATCH 2/3] fix(transaction): improve gas estimation reliability and error handling --- pkg/transaction/transaction.go | 162 +++++++++------------------------ 1 file changed, 44 insertions(+), 118 deletions(-) diff --git a/pkg/transaction/transaction.go b/pkg/transaction/transaction.go index 6a8a22dc8aa..d3494fd866c 100644 --- a/pkg/transaction/transaction.go +++ b/pkg/transaction/transaction.go @@ -275,160 +275,86 @@ func (t *transactionService) StoredTransaction(txHash common.Hash) (*StoredTrans // prepareTransaction creates a signable transaction based on a request. func (t *transactionService) prepareTransaction(ctx context.Context, request *TxRequest, nonce uint64, boostPercent int) (tx *types.Transaction, err error) { - /* - Transactions are EIP 1559 dynamic transactions where there are three fee related fields: - 1. base fee is the price that will be burned as part of the transaction. - 2. max fee is the max price we are willing to spend as gas price. - 3. max priority fee is max price want to give to the miner to prioritize the transaction. - as an example: - if base fee is 15, max fee is 20, and max priority is 3, gas price will be 15 + 3 = 18 - if base is 15, max fee is 20, and max priority fee is 10, - gas price will be 15 + 10 = 25, but since 25 > 20, gas price is 20. - notice that gas price does not exceed 20 as defined by max fee. - */ - - // Calculate gas fees first so we can use them in gas estimation for more accurate simulation - gasFeeCap, gasTipCap, err := t.backend.SuggestedFeeAndTip(ctx, request.GasPrice, boostPercent) - if err != nil { - return nil, err - } - var gasLimit uint64 if request.GasLimit == 0 { - // Estimate gas using pending state to account for pending transactions - // This is consistent with using PendingNonceAt for nonce selection + // Estimate gas using pending state for consistency with PendingNonceAt gasLimit, err = t.backend.EstimateGas(ctx, ethereum.CallMsg{ - From: t.sender, - To: request.To, - Data: request.Data, - Value: request.Value, - GasFeeCap: gasFeeCap, - GasTipCap: gasTipCap, + From: t.sender, + To: request.To, + Data: request.Data, + Value: request.Value, }) if err != nil { - // Gas estimation failed - analyze error to provide better diagnostics - errStr := err.Error() - - // Try to get revert reason for better error reporting - var revertReason string - if strings.Contains(errStr, "execution reverted") || - strings.Contains(errStr, "Transaction execution fails") || - strings.Contains(errStr, "always failing transaction") { - // Attempt to call contract to get revert reason - if output, callErr := t.backend.CallContract(ctx, ethereum.CallMsg{ - From: t.sender, - To: request.To, - Data: request.Data, - Value: request.Value, - GasFeeCap: gasFeeCap, - GasTipCap: gasTipCap, - }, nil); callErr == nil && len(output) > 0 { - revertReason = fmt.Sprintf("revert_data=0x%x", output) - } else if callErr != nil { - revertReason = fmt.Sprintf("call_error=%v", callErr) - } - } - - switch { - case strings.Contains(errStr, "execution reverted") || - strings.Contains(errStr, "Transaction execution fails"): - t.logger.Error(err, "transaction would revert if sent - contract requirements not met", - "description", request.Description, - "to", request.To, - "revert_info", revertReason, - ) - - case strings.Contains(errStr, "always failing transaction"): - t.logger.Error(err, "transaction always fails - invalid parameters or contract state", - "description", request.Description, - "to", request.To, - "revert_info", revertReason, - ) - - case strings.Contains(errStr, "insufficient funds") || - strings.Contains(errStr, "exceeds balance"): - t.logger.Error(err, "insufficient balance to execute transaction", - "description", request.Description, - "value", request.Value, - ) - - case strings.Contains(errStr, "gas required exceeds allowance") || - strings.Contains(errStr, "out of gas"): - t.logger.Error(err, "transaction requires too much gas", - "description", request.Description, - ) - - case strings.Contains(errStr, "nonce too low"): - t.logger.Error(err, "nonce conflict detected", - "description", request.Description, - ) - - default: - t.logger.Warning("gas estimation failed, using fallback", - "error", err, - "description", request.Description, - ) - } + t.logger.Warning("gas estimation failed, using fallback", + "error", err, + "description", request.Description, + ) - // Use fallback gas limit with priority order if request.MinEstimatedGasLimit > 0 { gasLimit = request.MinEstimatedGasLimit } else if len(request.Data) > 0 { - // Contract interaction - use reasonable fallback + // Contract call - use reasonable fallback gasLimit = FallbackGasLimit } else { // Simple transfer - use minimum gasLimit = MinGasLimit } } else { - // Gas estimation succeeded - add buffer to handle state changes - buffer := gasLimit * GasBufferPercent / 100 - gasLimit += buffer + // Estimation succeeded - add buffer for state changes + gasLimit += gasLimit * GasBufferPercent / 100 - // Ensure minimum requirement is met + // Apply minimum if specified if gasLimit < request.MinEstimatedGasLimit { gasLimit = request.MinEstimatedGasLimit } - // Cap at maximum to prevent excessive gas limits + // Cap at maximum if gasLimit > MaxGasLimit { - t.logger.Warning("estimated gas exceeds maximum, capping", - "estimated", gasLimit, - "max", MaxGasLimit, - "description", request.Description, - ) gasLimit = MaxGasLimit } } - // Final safety check - ensure absolute minimum + // Ensure absolute minimum if gasLimit < MinGasLimit { gasLimit = MinGasLimit } } else { - // User provided explicit gas limit - use it but validate - gasLimit = request.GasLimit - if gasLimit < MinGasLimit { - t.logger.Warning("provided gas limit too low, using minimum", - "provided", gasLimit, - "minimum", MinGasLimit, - ) - gasLimit = MinGasLimit - } - if gasLimit > MaxGasLimit { - t.logger.Warning("provided gas limit too high, capping", - "provided", gasLimit, - "maximum", MaxGasLimit, - ) - gasLimit = MaxGasLimit - } + // Use provided gas limit with bounds validation + gasLimit = min(max(request.GasLimit, MinGasLimit), MaxGasLimit) } if gasLimit == 0 { return nil, errors.New("gas limit cannot be zero") } + /* + Transactions are EIP 1559 dynamic transactions where there are three fee related fields: + 1. base fee is the price that will be burned as part of the transaction. + 2. max fee is the max price we are willing to spend as gas price. + 3. max priority fee is max price want to give to the miner to prioritize the transaction. + as an example: + if base fee is 15, max fee is 20, and max priority is 3, gas price will be 15 + 3 = 18 + if base is 15, max fee is 20, and max priority fee is 10, + gas price will be 15 + 10 = 25, but since 25 > 20, gas price is 20. + notice that gas price does not exceed 20 as defined by max fee. + */ + + gasFeeCap, gasTipCap, err := t.backend.SuggestedFeeAndTip(ctx, request.GasPrice, boostPercent) + if err != nil { + return nil, err + } + + t.logger.Debug("prepared transaction", + "to", request.To, + "value", request.Value, + "gas_limit", gasLimit, + "gas_fee_cap", gasFeeCap, + "gas_tip_cap", gasTipCap, + "nonce", nonce, + "description", request.Description, + ) + return types.NewTx(&types.DynamicFeeTx{ Nonce: nonce, ChainID: t.chainID, From 6178e7830b4ac4e6a8ff971c8fab2d75329aadd8 Mon Sep 17 00:00:00 2001 From: Ljubisa Gacevic Date: Tue, 3 Feb 2026 14:03:50 +0100 Subject: [PATCH 3/3] test(transaction): add gas limit handling tests --- pkg/transaction/transaction_test.go | 501 +++++++++++++++++++++++++++- 1 file changed, 497 insertions(+), 4 deletions(-) diff --git a/pkg/transaction/transaction_test.go b/pkg/transaction/transaction_test.go index b84d7031a76..615962ca513 100644 --- a/pkg/transaction/transaction_test.go +++ b/pkg/transaction/transaction_test.go @@ -117,8 +117,8 @@ func TestTransactionSend(t *testing.T) { txData := common.Hex2Bytes("0xabcdee") value := big.NewInt(1) suggestedGasTip := minimumTip - estimatedGasLimit := uint64(3) - gasLimit := estimatedGasLimit + estimatedGasLimit/2 // added 50% buffer + estimatedGasLimit := uint64(30000) + gasLimit := estimatedGasLimit + estimatedGasLimit*transaction.GasBufferPercent/100 // added 33% buffer nonce := uint64(2) chainID := big.NewInt(5) gasFeeCap := new(big.Int).Add(new(big.Int).Mul(baseFee, big.NewInt(2)), suggestedGasTip) @@ -208,12 +208,15 @@ func TestTransactionSend(t *testing.T) { t.Run("send with estimate error", func(t *testing.T) { t.Parallel() + // When estimation fails, use MinEstimatedGasLimit without buffer + gasLimitFallback := estimatedGasLimit + signedTx := types.NewTx(&types.DynamicFeeTx{ ChainID: chainID, Nonce: nonce, To: &recipient, Value: value, - Gas: gasLimit, + Gas: gasLimitFallback, GasFeeCap: gasFeeCap, GasTipCap: suggestedGasTip, Data: txData, @@ -267,7 +270,7 @@ func TestTransactionSend(t *testing.T) { t.Fatal("returning wrong transaction hash") } - checkStoredTransaction(t, transactionService, txHash, request, recipient, gasLimit, gasFeeCap, nonce) + checkStoredTransaction(t, transactionService, txHash, request, recipient, gasLimitFallback, gasFeeCap, nonce) pending, err := transactionService.PendingTransactions() if err != nil { @@ -570,6 +573,496 @@ func TestTransactionSend(t *testing.T) { t.Fatalf("got wrong gas tip in stored transaction. wanted %d, got %d", customGasFeeCap, storedTransaction.GasTipCap) } }) + + t.Run("send with contract fallback", func(t *testing.T) { + t.Parallel() + + // When estimation fails for contract call (has data), use FallbackGasLimit (500k) + contractData := []byte{0xab, 0xcd, 0xef} // Explicit non-empty data for contract call + signedTx := types.NewTx(&types.DynamicFeeTx{ + ChainID: chainID, + Nonce: nonce, + To: &recipient, + Value: value, + Gas: transaction.FallbackGasLimit, + GasFeeCap: gasFeeCap, + GasTipCap: suggestedGasTip, + Data: contractData, + }) + request := &transaction.TxRequest{ + To: &recipient, + Data: contractData, + Value: value, + } + store := storemock.NewStateStore() + + transactionService, err := transaction.NewService(logger, sender, + backendmock.New( + backendmock.WithSendTransactionFunc(func(ctx context.Context, tx *types.Transaction) error { + if tx != signedTx { + t.Fatal("not sending signed transaction") + } + return nil + }), + backendmock.WithEstimateGasFunc(func(ctx context.Context, msg ethereum.CallMsg) (gas uint64, err error) { + return 0, errors.New("estimation failed") + }), + backendmock.WithPendingNonceAtFunc(func(ctx context.Context, account common.Address) (uint64, error) { + return nonce, nil + }), + backendmock.WithSuggestedFeeAndTipFunc(func(ctx context.Context, gasPrice *big.Int, boostPercent int) (*big.Int, *big.Int, error) { + return gasFeeCap, suggestedGasTip, nil + }), + ), + signerMockForTransaction(t, signedTx, sender, chainID), + store, + chainID, + monitormock.New(), + ) + if err != nil { + t.Fatal(err) + } + testutil.CleanupCloser(t, transactionService) + + txHash, err := transactionService.Send(context.Background(), request, 0) + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal(txHash.Bytes(), signedTx.Hash().Bytes()) { + t.Fatal("returning wrong transaction hash") + } + + storedTransaction, err := transactionService.StoredTransaction(txHash) + if err != nil { + t.Fatal(err) + } + + if storedTransaction.GasLimit != transaction.FallbackGasLimit { + t.Fatalf("expected fallback gas limit %d, got %d", transaction.FallbackGasLimit, storedTransaction.GasLimit) + } + }) + + t.Run("send with simple transfer fallback", func(t *testing.T) { + t.Parallel() + + // When estimation fails for simple transfer (no data), use MinGasLimit (21k) + signedTx := types.NewTx(&types.DynamicFeeTx{ + ChainID: chainID, + Nonce: nonce, + To: &recipient, + Value: value, + Gas: transaction.MinGasLimit, + GasFeeCap: gasFeeCap, + GasTipCap: suggestedGasTip, + Data: nil, + }) + request := &transaction.TxRequest{ + To: &recipient, + Data: nil, + Value: value, + } + store := storemock.NewStateStore() + + transactionService, err := transaction.NewService(logger, sender, + backendmock.New( + backendmock.WithSendTransactionFunc(func(ctx context.Context, tx *types.Transaction) error { + if tx != signedTx { + t.Fatal("not sending signed transaction") + } + return nil + }), + backendmock.WithEstimateGasFunc(func(ctx context.Context, msg ethereum.CallMsg) (gas uint64, err error) { + return 0, errors.New("estimation failed") + }), + backendmock.WithPendingNonceAtFunc(func(ctx context.Context, account common.Address) (uint64, error) { + return nonce, nil + }), + backendmock.WithSuggestedFeeAndTipFunc(func(ctx context.Context, gasPrice *big.Int, boostPercent int) (*big.Int, *big.Int, error) { + return gasFeeCap, suggestedGasTip, nil + }), + ), + signerMockForTransaction(t, signedTx, sender, chainID), + store, + chainID, + monitormock.New(), + ) + if err != nil { + t.Fatal(err) + } + testutil.CleanupCloser(t, transactionService) + + txHash, err := transactionService.Send(context.Background(), request, 0) + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal(txHash.Bytes(), signedTx.Hash().Bytes()) { + t.Fatal("returning wrong transaction hash") + } + + storedTransaction, err := transactionService.StoredTransaction(txHash) + if err != nil { + t.Fatal(err) + } + + if storedTransaction.GasLimit != transaction.MinGasLimit { + t.Fatalf("expected min gas limit %d, got %d", transaction.MinGasLimit, storedTransaction.GasLimit) + } + }) + + t.Run("send with max gas limit cap", func(t *testing.T) { + t.Parallel() + + // When estimation returns value that exceeds MaxGasLimit, cap it + highEstimate := uint64(15_000_000) // Above MaxGasLimit of 10M + expectedGasLimit := uint64(transaction.MaxGasLimit) + + signedTx := types.NewTx(&types.DynamicFeeTx{ + ChainID: chainID, + Nonce: nonce, + To: &recipient, + Value: value, + Gas: expectedGasLimit, + GasFeeCap: gasFeeCap, + GasTipCap: suggestedGasTip, + Data: txData, + }) + request := &transaction.TxRequest{ + To: &recipient, + Data: txData, + Value: value, + } + store := storemock.NewStateStore() + + transactionService, err := transaction.NewService(logger, sender, + backendmock.New( + backendmock.WithSendTransactionFunc(func(ctx context.Context, tx *types.Transaction) error { + if tx != signedTx { + t.Fatal("not sending signed transaction") + } + return nil + }), + backendmock.WithEstimateGasFunc(func(ctx context.Context, msg ethereum.CallMsg) (gas uint64, err error) { + return highEstimate, nil + }), + backendmock.WithPendingNonceAtFunc(func(ctx context.Context, account common.Address) (uint64, error) { + return nonce, nil + }), + backendmock.WithSuggestedFeeAndTipFunc(func(ctx context.Context, gasPrice *big.Int, boostPercent int) (*big.Int, *big.Int, error) { + return gasFeeCap, suggestedGasTip, nil + }), + ), + signerMockForTransaction(t, signedTx, sender, chainID), + store, + chainID, + monitormock.New(), + ) + if err != nil { + t.Fatal(err) + } + testutil.CleanupCloser(t, transactionService) + + txHash, err := transactionService.Send(context.Background(), request, 0) + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal(txHash.Bytes(), signedTx.Hash().Bytes()) { + t.Fatal("returning wrong transaction hash") + } + + storedTransaction, err := transactionService.StoredTransaction(txHash) + if err != nil { + t.Fatal(err) + } + + if storedTransaction.GasLimit != transaction.MaxGasLimit { + t.Fatalf("expected max gas limit %d, got %d", transaction.MaxGasLimit, storedTransaction.GasLimit) + } + }) + + t.Run("send with provided gas limit", func(t *testing.T) { + t.Parallel() + + // When GasLimit is explicitly provided, use it with bounds validation + providedGasLimit := uint64(100_000) + + signedTx := types.NewTx(&types.DynamicFeeTx{ + ChainID: chainID, + Nonce: nonce, + To: &recipient, + Value: value, + Gas: providedGasLimit, + GasFeeCap: gasFeeCap, + GasTipCap: suggestedGasTip, + Data: txData, + }) + request := &transaction.TxRequest{ + To: &recipient, + Data: txData, + Value: value, + GasLimit: providedGasLimit, + } + store := storemock.NewStateStore() + + transactionService, err := transaction.NewService(logger, sender, + backendmock.New( + backendmock.WithSendTransactionFunc(func(ctx context.Context, tx *types.Transaction) error { + if tx != signedTx { + t.Fatal("not sending signed transaction") + } + return nil + }), + // EstimateGas should not be called when GasLimit is provided + backendmock.WithEstimateGasFunc(func(ctx context.Context, msg ethereum.CallMsg) (gas uint64, err error) { + t.Fatal("EstimateGas should not be called when GasLimit is provided") + return 0, nil + }), + backendmock.WithPendingNonceAtFunc(func(ctx context.Context, account common.Address) (uint64, error) { + return nonce, nil + }), + backendmock.WithSuggestedFeeAndTipFunc(func(ctx context.Context, gasPrice *big.Int, boostPercent int) (*big.Int, *big.Int, error) { + return gasFeeCap, suggestedGasTip, nil + }), + ), + signerMockForTransaction(t, signedTx, sender, chainID), + store, + chainID, + monitormock.New(), + ) + if err != nil { + t.Fatal(err) + } + testutil.CleanupCloser(t, transactionService) + + txHash, err := transactionService.Send(context.Background(), request, 0) + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal(txHash.Bytes(), signedTx.Hash().Bytes()) { + t.Fatal("returning wrong transaction hash") + } + + storedTransaction, err := transactionService.StoredTransaction(txHash) + if err != nil { + t.Fatal(err) + } + + if storedTransaction.GasLimit != providedGasLimit { + t.Fatalf("expected provided gas limit %d, got %d", providedGasLimit, storedTransaction.GasLimit) + } + }) + + t.Run("send with MinEstimatedGasLimit enforced after buffer", func(t *testing.T) { + t.Parallel() + + // When estimated gas + buffer is below MinEstimatedGasLimit, enforce the minimum + lowEstimate := uint64(50_000) + minGas := uint64(100_000) + // lowEstimate + 33% = 66,500, which is < minGas, so minGas should be used + + signedTx := types.NewTx(&types.DynamicFeeTx{ + ChainID: chainID, + Nonce: nonce, + To: &recipient, + Value: value, + Gas: minGas, + GasFeeCap: gasFeeCap, + GasTipCap: suggestedGasTip, + Data: txData, + }) + request := &transaction.TxRequest{ + To: &recipient, + Data: txData, + Value: value, + MinEstimatedGasLimit: minGas, + } + store := storemock.NewStateStore() + + transactionService, err := transaction.NewService(logger, sender, + backendmock.New( + backendmock.WithSendTransactionFunc(func(ctx context.Context, tx *types.Transaction) error { + if tx != signedTx { + t.Fatal("not sending signed transaction") + } + return nil + }), + backendmock.WithEstimateGasFunc(func(ctx context.Context, msg ethereum.CallMsg) (gas uint64, err error) { + return lowEstimate, nil + }), + backendmock.WithPendingNonceAtFunc(func(ctx context.Context, account common.Address) (uint64, error) { + return nonce, nil + }), + backendmock.WithSuggestedFeeAndTipFunc(func(ctx context.Context, gasPrice *big.Int, boostPercent int) (*big.Int, *big.Int, error) { + return gasFeeCap, suggestedGasTip, nil + }), + ), + signerMockForTransaction(t, signedTx, sender, chainID), + store, + chainID, + monitormock.New(), + ) + if err != nil { + t.Fatal(err) + } + testutil.CleanupCloser(t, transactionService) + + txHash, err := transactionService.Send(context.Background(), request, 0) + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal(txHash.Bytes(), signedTx.Hash().Bytes()) { + t.Fatal("returning wrong transaction hash") + } + + storedTransaction, err := transactionService.StoredTransaction(txHash) + if err != nil { + t.Fatal(err) + } + + if storedTransaction.GasLimit != minGas { + t.Fatalf("expected MinEstimatedGasLimit %d, got %d", minGas, storedTransaction.GasLimit) + } + }) + + t.Run("send with provided gas limit below minimum", func(t *testing.T) { + t.Parallel() + + // When provided GasLimit is below MinGasLimit, enforce MinGasLimit + lowGasLimit := uint64(10_000) // Below MinGasLimit of 21k + + signedTx := types.NewTx(&types.DynamicFeeTx{ + ChainID: chainID, + Nonce: nonce, + To: &recipient, + Value: value, + Gas: transaction.MinGasLimit, + GasFeeCap: gasFeeCap, + GasTipCap: suggestedGasTip, + Data: txData, + }) + request := &transaction.TxRequest{ + To: &recipient, + Data: txData, + Value: value, + GasLimit: lowGasLimit, + } + store := storemock.NewStateStore() + + transactionService, err := transaction.NewService(logger, sender, + backendmock.New( + backendmock.WithSendTransactionFunc(func(ctx context.Context, tx *types.Transaction) error { + if tx != signedTx { + t.Fatal("not sending signed transaction") + } + return nil + }), + backendmock.WithPendingNonceAtFunc(func(ctx context.Context, account common.Address) (uint64, error) { + return nonce, nil + }), + backendmock.WithSuggestedFeeAndTipFunc(func(ctx context.Context, gasPrice *big.Int, boostPercent int) (*big.Int, *big.Int, error) { + return gasFeeCap, suggestedGasTip, nil + }), + ), + signerMockForTransaction(t, signedTx, sender, chainID), + store, + chainID, + monitormock.New(), + ) + if err != nil { + t.Fatal(err) + } + testutil.CleanupCloser(t, transactionService) + + txHash, err := transactionService.Send(context.Background(), request, 0) + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal(txHash.Bytes(), signedTx.Hash().Bytes()) { + t.Fatal("returning wrong transaction hash") + } + + storedTransaction, err := transactionService.StoredTransaction(txHash) + if err != nil { + t.Fatal(err) + } + + if storedTransaction.GasLimit != transaction.MinGasLimit { + t.Fatalf("expected min gas limit enforced %d, got %d", transaction.MinGasLimit, storedTransaction.GasLimit) + } + }) + + t.Run("send with provided gas limit above maximum", func(t *testing.T) { + t.Parallel() + + // When provided GasLimit is above MaxGasLimit, cap at MaxGasLimit + highGasLimit := uint64(15_000_000) // Above MaxGasLimit of 10M + + signedTx := types.NewTx(&types.DynamicFeeTx{ + ChainID: chainID, + Nonce: nonce, + To: &recipient, + Value: value, + Gas: transaction.MaxGasLimit, + GasFeeCap: gasFeeCap, + GasTipCap: suggestedGasTip, + Data: txData, + }) + request := &transaction.TxRequest{ + To: &recipient, + Data: txData, + Value: value, + GasLimit: highGasLimit, + } + store := storemock.NewStateStore() + + transactionService, err := transaction.NewService(logger, sender, + backendmock.New( + backendmock.WithSendTransactionFunc(func(ctx context.Context, tx *types.Transaction) error { + if tx != signedTx { + t.Fatal("not sending signed transaction") + } + return nil + }), + backendmock.WithPendingNonceAtFunc(func(ctx context.Context, account common.Address) (uint64, error) { + return nonce, nil + }), + backendmock.WithSuggestedFeeAndTipFunc(func(ctx context.Context, gasPrice *big.Int, boostPercent int) (*big.Int, *big.Int, error) { + return gasFeeCap, suggestedGasTip, nil + }), + ), + signerMockForTransaction(t, signedTx, sender, chainID), + store, + chainID, + monitormock.New(), + ) + if err != nil { + t.Fatal(err) + } + testutil.CleanupCloser(t, transactionService) + + txHash, err := transactionService.Send(context.Background(), request, 0) + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal(txHash.Bytes(), signedTx.Hash().Bytes()) { + t.Fatal("returning wrong transaction hash") + } + + storedTransaction, err := transactionService.StoredTransaction(txHash) + if err != nil { + t.Fatal(err) + } + + if storedTransaction.GasLimit != transaction.MaxGasLimit { + t.Fatalf("expected max gas limit enforced %d, got %d", transaction.MaxGasLimit, storedTransaction.GasLimit) + } + }) } func TestTransactionWaitForReceipt(t *testing.T) {