fix(driver): fall back to another EOA when a submission account is unusable#4547
fix(driver): fall back to another EOA when a submission account is unusable#4547frdrckj wants to merge 1 commit into
Conversation
…usable EIP-7702 parallel submission picks an account from a FIFO pool but never falls back when the chosen account can't broadcast (no gas for the tx, stale nonce, or a pending tx that can't be replaced). A valid settlement then fails even when another funded account is available. Classify pre-broadcast node rejections as account-specific (`mempools::Error::SubmitterUnusable`) and, on such a failure, retry the settlement from another account while the submission deadline still holds. Spent accounts are held for the duration of the request so each retry picks a different one, and `try_acquire` keeps the fallback non-blocking to avoid deadlocking against concurrent settlements. Retrying is safe against double-submission: a settlement that actually lands always returns `Ok`, so only failures (where nothing was broadcast) are ever retried. `already known` is deliberately not treated as account-specific, since it means our exact tx is already in the mempool and may still be mined. Towards cowprotocol#4541.
|
All contributors have signed the CLA ✍️ ✅ |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a fallback mechanism to retry transaction submissions using alternative accounts when encountering account-specific failures (such as insufficient funds, nonce issues, or underpriced transactions). Feedback focuses on improving the robustness of the error classification: first, by formatting the full anyhow::Error chain using format!("{err:#}") instead of err.to_string() to prevent underlying node errors from being hidden; and second, by loosening the substring matching logic to support non-Geth clients like Nethermind which format error messages differently.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // A rejection at broadcast time means nothing was sent, so an | ||
| // account-specific failure (no gas, stale nonce, ...) can be | ||
| // retried from a different submission account. | ||
| match mempools::classify_submission_failure(&err.to_string()) { |
There was a problem hiding this comment.
Using err.to_string() on an anyhow::Error only returns the outermost error message. If context is added to the error in the future (e.g., via .context()), the underlying node error message containing 'insufficient funds' or 'nonce too low' will be hidden, silently breaking the classification and fallback mechanism. Use format!("{err:#}") to format the entire error chain, ensuring robustness against future refactoring.
| match mempools::classify_submission_failure(&err.to_string()) { | |
| match mempools::classify_submission_failure(&format!("{err:#}")) { |
| pub fn classify_submission_failure(message: &str) -> Option<AccountFailure> { | ||
| let message = message.to_lowercase(); | ||
| if message.contains("insufficient funds") { | ||
| Some(AccountFailure::InsufficientFunds) | ||
| } else if message.contains("nonce too low") { | ||
| Some(AccountFailure::Nonce) | ||
| } else if message.contains("underpriced") { | ||
| Some(AccountFailure::Underpriced) | ||
| } else { | ||
| None | ||
| } | ||
| } |
There was a problem hiding this comment.
Matching exact substrings like "insufficient funds" or "nonce too low" is fragile and can fail on non-Geth clients (such as Nethermind) which may return error variants like "SenderInsufficientFunds" or "NonceTooLow" without spaces. Checking for the presence of both keywords (e.g., "insufficient" and "funds", or "nonce" and "low") makes the classification significantly more robust across different Ethereum clients.
pub fn classify_submission_failure(message: &str) -> Option<AccountFailure> {
let message = message.to_lowercase();
if message.contains("insufficient") && message.contains("funds") {
Some(AccountFailure::InsufficientFunds)
} else if message.contains("nonce") && message.contains("low") {
Some(AccountFailure::Nonce)
} else if message.contains("underpriced") {
Some(AccountFailure::Underpriced)
} else {
None
}
}
|
I have read the CLA Document and I hereby sign the CLA |
1 participant
Description
EIP-7702 parallel submission selects a submission account from a FIFO pool. When the selected account cannot broadcast the transaction for an account-specific reason (no balance for gas, a stale nonce, or a pending tx that cannot be replaced), the settlement currently fails even though another funded account may be available. This is the missing fallback described in #4541.
This PR adds the reactive fallback: on an account-specific submission failure, the settlement is retried from another eligible account while the submission deadline still holds.
Changes
mempools::Error::SubmitterUnusable(AccountFailure)and a smallclassify_submission_failurehelper that recognises pre-broadcast node rejections (insufficient funds,nonce too low,... underpriced).already knownis intentionally excluded.Mempool::submitnow returns this variant for those broadcast-time rejections instead of the genericOther.Competition::process_settle_requestretries the settlement from another account onSubmitterUnusable. Spent accounts are held until the request finishes so each retry uses a different one, and a new non-blockingSubmitterPool::try_acquireavoids deadlocking against concurrent settlements.Retrying is safe against double-submission: a settlement that actually lands always returns
Ok, so only failures (where nothing was broadcast) are ever retried. That is also whyalready knownis excluded, since it means our exact transaction is already in the mempool and may still be mined.This implements the reactive fallback and failure classification. Proactive balance/nonce pre-checks before account selection, and a persistent cooldown for repeatedly-failing accounts, are left as follow-ups since they involve tuning choices worth agreeing on separately.
How to test
New unit tests cover the classifier in both directions:
classifies_account_specific_submission_failuresanddoes_not_classify_non_account_failures_as_account_specific(the latter guards against retrying reverts,already known, rate limits, and transport errors).Towards #4541.