-
Notifications
You must be signed in to change notification settings - Fork 105
Handle 'incorrect account sequence' in nonce error check #910
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Added support for detecting errors with the message 'incorrect account sequence' in the isNonceAlreadyUsedError utility. This improves error handling for cases where the nonce is invalid due to account sequence issues.
WalkthroughUpdated isNonceAlreadyUsedError in src/shared/utils/error.ts to recognize "incorrect account sequence" as an additional indicator of a nonce-already-used error, alongside existing checks for "nonce too low" and "already known", while retaining the fallback to ethers.errors.NONCE_EXPIRED. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes ✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/shared/utils/error.ts (2)
21-23: Refactor to a centralized pattern list and use.some()for maintainabilityThis logic will be easier to extend and read if we move known substrings into a constant and use
.some().Apply this diff within the selected range:
- return ( - message.includes("nonce too low") || message.includes("already known") || message.includes("incorrect account sequence") - ); + return NONCE_ALREADY_USED_PATTERNS.some((p) => message.includes(p));Add this helper (outside the selected lines, e.g., near the top of the file):
const NONCE_ALREADY_USED_PATTERNS = [ "nonce too low", "already known", "incorrect account sequence", // Consider adding, if seen in the wild: // "nonce too small", "nonce already used", "account sequence mismatch" ] as const;Optionally, consider applying the same pattern-array approach to isReplacementGasFeeTooLow and isInsufficientFundsError for consistency.
17-17: Nit: Add explicit return type for clarityAdd an explicit boolean return type to align with stricter TS settings and improve readability. Consider doing the same for the other helpers.
-export const isNonceAlreadyUsedError = (error: unknown) => { +export const isNonceAlreadyUsedError = (error: unknown): boolean => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/shared/utils/error.ts(1 hunks)
🔇 Additional comments (2)
src/shared/utils/error.ts (2)
21-23: LGTM: Added support for "incorrect account sequence" is appropriateGood call including this message. Cosmos/Ethermint-based EVM nodes (e.g., Cronos/Evmos) often surface stale nonce issues with that phrasing. The lowercase normalization in _parseMessage ensures this matches reliably, and the ethers NONCE_EXPIRED fallback remains intact.
17-27: Confirm handling of “incorrect account sequence” across all callersThe term “incorrect account sequence” can indicate either a stale/low nonce (nonce already used) or a gap/high nonce. We currently lump it into
isNonceAlreadyUsedError, but each caller reacts differently:• src/worker/tasks/send-transaction-worker.ts
– Lines 557–560: onisNonceAlreadyUsedError→ resync nonce from chain and retry
– Lines 656–661: onisNonceAlreadyUsedError→ assume tx already mined, stop and return null• src/worker/tasks/cancel-recycled-nonces-worker.ts
– Lines 63–66: onisNonceAlreadyUsedError→ skip recycling this noncePlease verify that treating all “incorrect account sequence” errors as “nonce already used” is correct in each of these contexts. If some callers should only react to “too low” cases, consider refining the string‐matching logic or handling downstream.
Added support for detecting errors with the message 'incorrect account sequence' in the isNonceAlreadyUsedError utility. This improves error handling for cases where the nonce is invalid due to account sequence issues.
PR-Codex overview
This PR focuses on expanding the error message handling in the
src/shared/utils/error.tsfile to include an additional condition for error messages.Detailed summary
message.includes("incorrect account sequence").trueif the message contains "nonce too low", "already known", or "incorrect account sequence".Summary by CodeRabbit