Skip to content

Estimate gas for ETH transfers (fix Arbitrum "intrinsic gas too low")#64

Merged
Praz314159 merged 1 commit into
mainfrom
fix/arbitrum-eth-transfer-gas
Jul 3, 2026
Merged

Estimate gas for ETH transfers (fix Arbitrum "intrinsic gas too low")#64
Praz314159 merged 1 commit into
mainfrom
fix/arbitrum-eth-transfer-gas

Conversation

@Praz314159

@Praz314159 Praz314159 commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

transfer_eth hardcoded GasLimits::ETH_TRANSFER (21,000), the Ethereum L1 intrinsic. Arbitrum folds an L1 data component into intrinsic gas, so the node rejected these transfers as "intrinsic gas too low", which broke cohort child-wallet funding on Arbitrum.

Transfers now estimate gas via eth_estimateGas like every other operation. simulate() previously errored on calldata shorter than 4 bytes (no function selector to cache on); it now estimates such transactions directly — plain value transfers can't revert from contract logic — and applies the same 20% buffer the estimate cache uses. The eth_estimateGas call is factored into a helper shared by both paths. GasLimits::ETH_TRANSFER stays as a reference value.

Summary by CodeRabbit

  • Bug Fixes
    • Improved ETH transfer reliability on Arbitrum by using node gas estimation instead of a fixed gas amount, reducing “intrinsic gas too low” failures.
    • Updated transaction handling so short calldata is gas-estimated correctly rather than rejected.
    • Made gas estimation fall back more gracefully, with clearer handling when a simulation or estimation fails.

transfer_eth hardcoded GasLimits::ETH_TRANSFER (21,000), the Ethereum L1
intrinsic. Arbitrum folds an L1 data component into intrinsic gas, so the
node rejected these transfers as "intrinsic gas too low", which broke
cohort child-wallet funding on Arbitrum.

Transfers now estimate gas via eth_estimateGas like every other operation.
simulate() previously errored on calldata shorter than 4 bytes (no function
selector to cache on); it now estimates such transactions directly — plain
value transfers can't revert from contract logic — and applies the same 20%
buffer the estimate cache uses. The eth_estimateGas call is factored into a
helper shared by both paths. GasLimits::ETH_TRANSFER stays as a reference
value.
@Praz314159
Praz314159 merged commit d963b61 into main Jul 3, 2026
6 checks passed
@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 76f7c895-8d88-46fb-b86b-3919b087eacf

📥 Commits

Reviewing files that changed from the base of the PR and between b1741a4 and 23ce711.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • src/client/trades.rs
  • src/client/transactions.rs
  • src/hft/gas.rs

📝 Walkthrough

Walkthrough

This PR replaces the hardcoded Ethereum L1 intrinsic gas constant (21,000) used for ETH transfers with dynamic eth_estimateGas-based estimation. It introduces a centralized estimate_gas helper in transactions.rs, updates transfer_eth to rely on estimation, and revises documentation and changelog accordingly.

Changes

Gas Estimation Refactor

Layer / File(s) Summary
Centralized gas estimation helper and simulate() integration
src/client/transactions.rs
New private estimate_gas method builds a TransactionRequest, calls provider.estimate_gas, and maps errors to TransactionError::SimulationReverted or TransactionError::GasUnavailable; simulate() uses it for both the short-calldata path (replacing a prior validation error) and the cache-miss path.
transfer_eth relies on estimation instead of fixed gas limit
src/client/trades.rs
Removes GasLimits import and with_gas_limit(GasLimits::ETH_TRANSFER) call; builds the transfer with empty calldata and value, letting gas be estimated, while preserving urgency handling.
Documentation and changelog updates
src/hft/gas.rs, CHANGELOG.md
Updates the ETH_TRANSFER doc comment to note it's now a reference value due to node-based estimation and Arbitrum's L1 data gas component; updates changelog to describe the fix.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

  • StrobeLabs/perpcity-rust-sdk#38: Both PRs modify PerpClient's gas-limit resolution for transfer_eth, including GasLimits::ETH_TRANSFER usage and calldata-based estimation behavior.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/arbitrum-eth-transfer-gas

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant