Enhance error handling in EoaExecutorWorker for unsupported EIP-1559 errors #39
Conversation
…errors - Updated the error handling logic to include a new function `is_unsupported_eip1559_error`, which checks for unsupported feature errors and specific error messages related to method availability. - Modified the transaction processing logic to utilize this new error handling function, improving the robustness of the EoaExecutorWorker's response to unsupported EIP-1559 features.
|
Caution Review failedThe pull request is closed. WalkthroughAdds a helper to detect unsupported EIP‑1559 RPC errors, updates the transaction worker to use it when deciding to fall back to legacy gas pricing, repositions a public re-export for Changes
Sequence Diagram(s)sequenceDiagram
participant TW as TransactionWorker
participant RPC as Node RPC
TW->>RPC: Submit EIP‑1559 transaction
RPC-->>TW: Success or Error
alt Error
TW->>TW: call is_unsupported_eip1559_error(error)
alt Unsupported EIP‑1559
note right of TW #DDDDFF: Fallback path (legacy gas)
TW->>RPC: Submit legacy (pre‑1559) transaction
RPC-->>TW: Result
else Other error
TW-->>TW: Propagate error
end
else Success
TW-->>TW: Continue normal flow
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
executors/src/eoa/worker/transaction.rs (1)
199-219: Fallback to legacy on EIP-1559 unsupported: ensure fee fields are mutually exclusiveSwitching to the helper is a solid maintainability win. One correctness concern: if the incoming request had one EIP-1559 fee set (e.g., user provided max_fee_per_gas but not max_priority), and 1559 estimation fails, we set gas_price but leave any previously set 1559 fields intact. Depending on Alloy’s TypedTransaction inference, this can lead to ambiguous or unintended type selection.
Mitigate by clearing 1559 fields when falling back to legacy, or reconstructing a legacy-only request prior to setting gas_price. Also consider making the EIP-7702 error message explicit that the failure derives from 1559 fee estimation being unsupported.
Suggested change (adjust field accessors if direct fields aren’t public; alternatively, rebuild the request):
- Ok(gas_price) => { - tracing::debug!("Using legacy gas price: {}", gas_price); - Ok(tx.with_gas_price(gas_price)) - } + Ok(gas_price) => { + tracing::debug!("Using legacy gas price: {}", gas_price); + // Ensure we don't carry partial 1559 fields into a legacy tx + let mut tx_legacy = tx; + // If these fields are not publicly settable, rebuild the request instead. + tx_legacy.max_fee_per_gas = None; + tx_legacy.max_priority_fee_per_gas = None; + Ok(tx_legacy.with_gas_price(gas_price)) + }Optionally refine the EIP-7702 message:
- Err(EoaExecutorWorkerError::TransactionBuildFailed { - message: "EIP7702 transactions not supported on chain".to_string(), - }) + Err(EoaExecutorWorkerError::TransactionBuildFailed { + message: "EIP-7702 requires EIP-1559 fee support, which is not available on this chain/provider" + .to_string(), + })If helpful, I can provide a quick helper like
fn clear_eip1559_fees(tx: AlloyTransactionRequest) -> AlloyTransactionRequestto encapsulate this.
🧹 Nitpick comments (5)
executors/src/eoa/worker/error.rs (3)
241-242: "invalid opcode" classified as non-retryable — consider expanding deterministic EVM error patternsGood call marking "invalid opcode" as deterministic/non-retryable. To reduce noisy retries further, consider also capturing other well-known deterministic EVM execution errors often surfaced in RPC messages (client-dependent phrasing), e.g., "invalid op", "bad instruction", "invalid jump". This keeps the retry loop focused on genuine transients.
Apply this diff to extend the filter:
- // if the error message contains "invalid chain", it's not retryable - !(message.contains("invalid chain") || message.contains("invalid opcode")) + // if the error message indicates an invalid chain or deterministic EVM fault, it's not retryable + !(message.contains("invalid chain") + || message.contains("invalid opcode") + || message.contains("invalid op") + || message.contains("bad instruction") + || message.contains("invalid jump"))
327-338: Broaden EIP-1559 unsupported detection beyond just “method not found”Relying solely on "method not found" risks false negatives. Clients/providers vary: common variants include "method is not available", "the method does not exist", "unknown/unsupported method", or explicit "EIP-1559 not supported"/"London not activated". Also, many return JSON-RPC code -32601. Consider matching a small set of phrases and, if accessible, the error code.
Additionally, prefer to_ascii_lowercase over to_lowercase to avoid locale pitfalls.
Proposed improvement:
pub fn is_unsupported_eip1559_error(error: &RpcError<TransportErrorKind>) -> bool { if let RpcError::UnsupportedFeature(_) = error { return true; } if let RpcError::ErrorResp(resp) = error { - let message = resp.message.to_lowercase(); - return message.contains("method not found"); + let message = resp.message.to_ascii_lowercase(); + // Common JSON-RPC phrasings from various clients/providers + let method_unavailable = [ + "method not found", + "method is not available", + "the method does not exist", + "unknown method", + "unsupported method", + ] + .iter() + .any(|p| message.contains(p)); + + let eip1559_unavailable = message.contains("eip-1559") + && (message.contains("not supported") + || message.contains("unsupported") + || message.contains("not available")); + + let fee_methods_unavailable = (message.contains("eth_feehistory") + || message.contains("eth_maxpriorityfeepergas")) + && (message.contains("not found") + || message.contains("not available") + || message.contains("does not exist") + || message.contains("unsupported")); + + // If resp.code is available, consider also: resp.code == -32601 + return method_unavailable || eip1559_unavailable || fee_methods_unavailable; } false }If you’d like, I can add a small unit-test module covering:
- UnsupportedFeature arm
- ErrorResp with "method is not available"
- ErrorResp with "unknown method"
- ErrorResp with explicit "EIP-1559 not supported"
- A negative case (unrelated RPC error)
154-205: Nit: share phrase lists between classifiers to keep semantics consistentYou now have several places matching error phrases (classify_send_error, is_retryable_rpc_error, and is_unsupported_eip1559_error). Consider centralizing phrase lists and helpers to avoid drift.
executors/src/eoa/worker/transaction.rs (2)
176-196: Trace more context when 1559 estimation succeedsMinor improvement: include chain_id and provider URL (if available) in the success log to simplify multi-chain debugging.
160-175: Pre-check could short-circuit 1559 path for known non-1559 chainsIf
Chainexposes London/EIP-1559 activation metadata, you can short-circuit to legacy for those chains, skipping the RPC round-trip. Not blocking; the current approach safely handles unknowns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
executors/src/eoa/worker/error.rs(2 hunks)executors/src/eoa/worker/transaction.rs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
executors/src/eoa/worker/error.rs (1)
executors/src/eoa/error_classifier.rs (1)
message(224-236)
executors/src/eoa/worker/transaction.rs (1)
executors/src/eoa/worker/error.rs (2)
is_retryable_preparation_error(247-278)is_unsupported_eip1559_error(327-338)
🔇 Additional comments (5)
executors/src/eoa/worker/transaction.rs (5)
25-27: Import updates — looks goodBringing
is_unsupported_eip1559_errorinto this scope keeps the estimation path readable and avoids duplicating pattern logic.
304-354: Gas estimation revert handling — solid coverageThe revert/oversized branches are thorough and correctly scoped to TransactionSimulationFailed vs RPCError. No action needed.
30-55: Backoff arithmetic — safe and readableThe retry loop with exponential backoff is reasonable for preparation errors. No action needed.
171-175: Guardrails align with fee-estimation contractsSkipping EIP-1559 estimation when gas_price is present and returning early when both 1559 fees are present is correct and avoids unnecessary RPCs.
197-205: The script will dump lines 150–220 oftransaction.rsso we can see the full context around the EIP-1559 fallback branch.
…action length instead of transactions directly.
is_unsupported_eip1559_error, which checks for unsupported feature errors and specific error messages related to method availability.Summary by CodeRabbit
New Features
Bug Fixes
Refactor