fix(txpool): include operator fee in post-Isthmus affordability check#2645
fix(txpool): include operator fee in post-Isthmus affordability check#26450x00101010 wants to merge 2 commits into
Conversation
After Isthmus, BaseHandler deducts L1 data fee + operator fee from the sender, but the txpool admission check was only adding L1 data fee. A sender funded for tx.cost + l1_data_fee but not the operator fee was admitted by the pool and then permanently rejected by execution, since mark_invalid in the payload builder is iterator-local and does not evict the underlying transaction. Mirror the execution-side cost in apply_base_checks by calling L1BlockInfo::tx_cost, which adds the operator fee when post-Isthmus. Adds a regression test asserting InsufficientFunds at admission for an operator-fee-underfunded EIP-1559 transaction.
🟡 Heimdall Review Status
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| let cost_addition = l1_block_info.tx_cost( | ||
| &encoded, | ||
| false, | ||
| ) { | ||
| Ok(cost) => cost, | ||
| Err(err) => { | ||
| return TransactionValidationOutcome::Error(*valid_tx.hash(), Box::new(err)); | ||
| } | ||
| }; | ||
| U256::from(valid_tx.transaction().gas_limit()), | ||
| spec_id, | ||
| ); |
There was a problem hiding this comment.
Potential panic on Isthmus+ chains before the first L1 info deposit is processed.
tx_cost → operator_fee_charge → operator_fee_charge_inner calls .expect() on operator_fee_scalar / operator_fee_constant. At genesis (block 0), the validator initializes L1BlockInfo with Default (both fields are None), and only the timestamp is set — update_l1_block_info is skipped because genesis has no transactions (see new() above, lines 130–131).
If the chain activates Isthmus at genesis (or before the first update_l1_block_info call populates the fields), BaseSpecId::from_timestamp will return an Isthmus+ spec, and the .expect() will panic on the first transaction validation.
The execution-side handler (BaseHandler::validate_against_state_and_deduct_caller) has the same latent issue, so this PR isn't making things worse — it's aligning txpool with execution. But it's worth noting that this code path is now reachable from the txpool for the first time.
Consider either:
- Falling back to zero operator fee when the scalars are
None(matching pre-Isthmus behavior), or - Returning
TransactionValidationOutcome::Errorif the scalars are missing.
Review SummaryThe fix correctly addresses the HackerOne #74725 vulnerability by switching from FindingLatent panic on Isthmus+ chains at genesis (low severity): What looks good
|
|
As Base doesn't have an operator fee and probably won't, should we just come up with a plan to remove this in a future hard fork too? |
3 participants
Summary
Mirror the execution-side cost in the txpool affordability check.
After Isthmus,
BaseHandlerdeductstx.cost + l1_data_fee + operator_fee, but the txpool only addedl1_data_fee. A sender funded fortx.cost + l1_data_fee(but not the operator fee) was admitted by the pool and rejected by execution. The payload builder'smark_invalidis iterator-local, so the tx persists across builds.apply_base_checksnow callsL1BlockInfo::tx_cost, the same function execution uses.Adds a regression test asserting
Invalid(InsufficientFunds)at admission for an operator-fee-underfunded EIP-1559 tx.Reported via HackerOne #74725.