fix(rpc): align eth_estimateGas with geth on fee tokens and L1 data fee

[panos-xyz]

Closes #181
Closes #184
Closes #189

Summary

Align eth_estimateGas / eth_call with morph-geth on Morph fee-token handling and simulated L1 data fee sizing. Real execution is unchanged: signed transactions still size L1 fees from their own encoded_2718() bytes, and eth_sendTransaction / eth_signTransaction still follow geth's toTransaction (legacy gasPrice forces a standard ETH tx — see #190).

What was wrong

Several independent simulation bugs stacked:

1. Unregistered / inactive feeTokenID (fix(revm): eth_estimateGas incorrectly succeeds for unregistered feeTokenID #181). validate_and_deduct_token_fee short-circuited on disable_fee_charge before the L2 Token Registry check. Upstream's is_basic_transfer path then returned 0x5208 (21000) instead of rejecting.
2. feeTokenID dropped when gasPrice was also set (fix(rpc): eth_estimateGas and eth_call strip feeTokenID when gasPrice is present #184). try_into_tx_env treated gas_price.is_some() as "not a MorphTx", so the same request was priced as a plain ETH tx. geth's ToMessage (the call/estimate path) ignores GasPrice and keys MorphTx off isMorphTxArgs() alone.
3. MorphTx L1 fee under-sized a legacy gasPrice. alloy leaves gas_priority_fee = None; the MorphTx encoder fell back to a zero tip, four non-zero bytes short of geth (ToMessage maps gasPrice onto both caps).
4. Plain ETH L1 fee under-sized Legacy / EIP-2930 envelopes (eth_estimateGas under-sizes the L1 data fee: simulation rebuilds a Legacy/EIP-2930 envelope where geth always uses EIP-1559 #189). The sibling of (3): build_ethereum_envelope_for_l1_fee dispatched on tx_type from minimal_tx_type(), so a gasPrice-only estimate was sized as Legacy while geth's post-London asUnsignedTx always uses DynamicFeeTx.

(3) and (4) both feed caller_gas_allowance, so an under-sized l1_fee can return a gas estimate the caller cannot actually pay.

Changes

1. Run L2 Token Registry existence / active checks before the simulation short-circuit; do not deduct token fees or touch balanceOf on eth_call / eth_estimateGas.
2. Drop the gas_price.is_none() gate from try_into_tx_env so simulation keeps Morph fields. try_build_morph_tx_from_request (the toTransaction analogue) is unchanged.
3. When a MorphTx simulation carries a legacy gasPrice, map it onto both EIP-1559 caps in the TxEnv so L1-fee encoding matches geth.
4. Stop reconstructing Legacy / EIP-2930 envelopes for simulation L1 fees. Post-London, encode EIP-1559 (mapping gasPrice onto both caps) unless the request is a MorphTx or an executable EIP-7702. The statetest harness no longer needs its own "untyped + baseFee → 1559" override.

Test plan

• Unit: unregistered / inactive fee token rejected in simulation
• Unit: Morph fields kept on try_into_tx_env when gasPrice is set; try_build_morph_tx_from_request still drops them
• Unit: MorphTx and plain-ETH encode_for_l1_fee match geth DynamicFee byte lengths for gasPrice-only shapes
• Unit: executable EIP-7702 still carries the authorization list; empty auth list / create fall through to EIP-1559
• e2e: estimate_gas_rejects_unregistered_fee_token
• e2e: simulation_rpcs_keep_fee_token_with_legacy_gas_price

Summary by CodeRabbit

• Bug Fixes

  • Gas estimation and contract simulations now reject unregistered, inactive, or incorrectly configured fee tokens with clear validation errors.
  • Fee-token validation occurs before simulations proceed, while fee deduction and unnecessary fee-token contract queries remain skipped.
  • Morph-specific transaction fields retain their fee-token settings when a legacy gas price is supplied.
  • Simulation fee encoding now matches post-London transaction behavior.

• Tests

  • Added coverage for invalid fee-token requests and legacy gas-price handling across gas estimation and contract-call simulations.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 20941cec-e294-4f79-be16-63351a52d544

📥 Commits

Reviewing files that changed from the base of the PR and between 4645667 and c8b81a8.

📒 Files selected for processing (3)

• bin/morph-statetest/src/schema.rs
• crates/revm/src/tx.rs
• crates/rpc/src/eth/transaction.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Simulation paths now validate that fee tokens are registered and active before skipping fee deduction. Unit and RPC integration tests verify rejection of unregistered and inactive tokens.

Changes

Fee token validation

Layer / File(s)	Summary
Simulation token validation crates/revm/src/handler.rs	The fee-charge-disabled return now occurs after token registration and activity checks.
Invalid token test coverage crates/revm/src/handler.rs, crates/node/tests/it/rpc.rs	Tests verify rejection of unregistered token 65535 and inactive token 42, including through eth_estimateGas.

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

Merge Risk: ⚪ Minimal · up to c8b81

The simulation token-validation, legacy gas-price conversion, and L1 fee envelope changes have no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 65.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 46 functions across 7 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The implementation satisfies issue #181 by validating fee tokens before simulation bypass, including unregistered and inactive tokens. It satisfies issue #184 by preserving feeTokenID when gasPrice is…
Out of Scope Changes check	✅ Passed	The changes remain within the stated objectives. The L1 fee encoding updates support the required gasPrice simulation behavior, and the added tests cover the linked issue requirements.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main changes: aligning RPC simulation behavior with geth for fee-token validation and L1 data-fee calculation. It is concise and specific, although it does not mention…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/estimate-gas-unregistered-token

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

_{Tip: disable this comment in your organization's Code Review settings.}

[panos-xyz]

Thanks for addressing the fee token validation in simulation paths! Decoupling TokenRegistryEntry to avoid querying token balance or triggering nested balanceOf calls in simulation is a great improvement.

Comparing this with origin/main and the reference implementation in morph-geth (core/state_transition.go), here are two observations and suggestions:

---

1. Also validate non-zero price_ratio and scale in simulation

In validate_and_deduct_token_fee:

let token_registry_entry =
    TokenRegistryEntry::load(evm.ctx_mut().journal_mut().db_mut(), token_id)?
        .ok_or(MorphInvalidTransaction::TokenNotRegistered(token_id))?;
if !token_registry_entry.is_active() {
    return Err(MorphInvalidTransaction::TokenNotActive(token_id).into());
}

Currently, only !token_registry_entry.is_active() is checked before the simulation early return. However:

• In morph-geth (core/state_transition.go:445 & core/types/token_fee.go:52-58), preCheck() calls TokenRate() for every alt-token tx (including in eth_call). If priceRatio == 0 or scale == 0, geth rejects with invalid rate / invalid token scale.
• In morph-reth's RPC layer (crates/rpc/src/eth/call.rs:223-228), token_gas_allowance already requires:

  if !token_fee_info.is_active
      || token_fee_info.price_ratio.is_zero()
      || token_fee_info.scale.is_zero()
  {
      return Err(MorphEthApiError::InvalidFeeToken);
  }

• In real execution, eth_to_token_amount returns U256::MAX when price_ratio == 0 || scale == 0, rejecting the tx with InsufficientTokenBalance.

Since TokenRegistryEntry::load already reads price_ratio and scale from storage, checking them here ensures eth_call consistently rejects inactive or unpriced tokens:

if !token_registry_entry.is_active()
    || token_registry_entry.price_ratio.is_zero()
    || token_registry_entry.scale.is_zero()
{
    return Err(MorphInvalidTransaction::TokenNotActive(token_id).into());
}

---

2. Validation order: place caller_eth_balance < tx_value after token metadata validation

In PR 182:

// 1. Value check (real execution only)
if !is_fee_charge_disabled {
    let tx_value = evm.ctx_ref().tx().value();
    if !tx_value.is_zero() {
        ...
        if caller_eth_balance < tx_value {
            return Err(LackOfFundForMaxFee...);
        }
    }
}

// 2. Token registry check
let token_registry_entry = TokenRegistryEntry::load(...)?;
if !token_registry_entry.is_active() {
    return Err(TokenNotActive...);
}

In morph-geth (core/state_transition.go:437-451), the order of checks is:

1. fees.IsTokenActive(...) & fees.TokenRate(...) (Token existence & rate)
2. buyAltTokenGas(): st.state.GetBalance(from) < st.value (ETH value check)
3. Alt-token balance check & transfer

With PR 182's current ordering, if a caller has insufficient ETH for value AND specifies an invalid/unregistered fee_token_id:

• Real execution fails early at step 1 with LackOfFundForMaxFee (never validating the token).
• Simulation skips step 1 and fails at step 2 with TokenNotRegistered.
• In morph-geth, both simulation and execution reject with token inactive/invalid first.

Suggestion:
Move the tx_value check to right after if is_fee_charge_disabled { ... return Ok(()); }, before token_registry_entry.load_for_caller:

• This aligns error precedence with morph-geth.
• It eliminates the need for the if !is_fee_charge_disabled wrapper around the value check (since simulations have already returned Ok(()) above it).
• Both simulation and real execution will evaluate Token Registry eligibility first.

[panos-xyz]

Regarding observation 2, I’m going to keep the current validation order and not make this change.

Without treating geth’s error precedence as a compatibility requirement, checking the caller’s ETH value balance first is preferable for real transactions: the caller account has already been loaded for nonce/code validation, so the balance check is effectively free and avoids unnecessary Token Registry storage reads for transactions that cannot transfer their requested value anyway. It also preserves the real-execution ordering that existed on origin/main.

Simulation intentionally disables fee/balance charging checks, so it can validate token eligibility first without requiring identical error precedence to real execution. Both paths still reject invalid transactions, and the ordering has no consensus-state impact.

Observation 1 is valid and will be addressed separately with an explicit invalid-token-configuration error rather than reporting a zero price_ratio/scale as TokenNotActive.

[claude]

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

_{Tip: disable this comment in your organization's Code Review settings.}

[panos-xyz]

Observation: asymmetry in priority fee fallback between MorphTx and Ethereum L1-fee envelopes

In crates/revm/src/tx.rs:

In build_ethereum_envelope_for_l1_fee:

// geth maps a legacy `gasPrice` onto both EIP-1559 caps (`ToMessage`).
let max_priority_fee_per_gas = self.max_priority_fee_per_gas().unwrap_or(self.gas_price());

However, in build_morph_tx_for_l1_fee (line 135):

Some(TxMorph {
    chain_id: self.chain_id().unwrap_or(fallback_chain_id),
    nonce: self.inner.nonce,
    gas_limit: self.gas_limit(),
    max_fee_per_gas: self.max_fee_per_gas(),
    max_priority_fee_per_gas: self.max_priority_fee_per_gas().unwrap_or_default(),
    ...

While commit 4645667 backfills tx_env.inner.gas_priority_fee = Some(gas_price) during RPC try_into_tx_env conversion, build_morph_tx_for_l1_fee itself still falls back to unwrap_or_default().

If a MorphTxEnv is constructed outside of the RPC request conversion path (e.g. in test suites, statetest harnesses, or other callers) with is_morph_tx = true, a legacy gas_price, and gas_priority_fee == None, build_morph_tx_for_l1_fee will fallback to 0 (encoding a zero tip of 0x80 instead of gas_price), whereas build_ethereum_envelope_for_l1_fee defensively falls back to self.gas_price().

Suggestion:
Align build_morph_tx_for_l1_fee to use .unwrap_or(self.gas_price()) as well:

max_priority_fee_per_gas: self.max_priority_fee_per_gas().unwrap_or(self.gas_price()),

This ensures both envelope builders are consistent and self-contained without relying on upstream callers to populate gas_priority_fee.