Repository navigation
feat: pass the consumed nonce to NonceManager.reset - #5373
Conversation
Port of #5353 to main. Add an optional nonce to reset's parameters, set to what consume returned and left out when consume threw, in sendTransaction and sendTransactionSync. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rewrite the reset-nonce tests to use custom nonce managers instead of `vi.spyOn`, asserting the broadcast nonces: a failed send's nonce is the only one released and the next send reuses it. Document that a failed send does not prove its nonce is unused, and shorten the changeset to one sentence. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D3nmWi1KSPEvFxpccK5EDN
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: a960a47 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a960a477df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| transport: custom({ | ||
| async request({ method, params }) { |
There was a problem hiding this comment.
The four newly added tests replace RPC behavior with custom({ request(...) }) handlers, including synthesizing broadcast failures and successful hashes, so they validate a hand-built fake client rather than the configured chain behavior. Rework these cases to trigger the failure and nonce reuse through the configured Anvil chain or a real ephemeral server, as required by the repository’s explicit prohibition on mocks, stubs, and fake clients.
AGENTS.md reference: AGENTS.md:L153-L157
Useful? React with 👍 / 👎.
| get: (chainId: FunctionParameters & { client: Client }) => Promise<number> | ||
| /** Reset a nonce. */ | ||
| reset: (chainId: FunctionParameters) => void | ||
| reset: (chainId: ResetParameters) => void |
There was a problem hiding this comment.
Changing the exported NonceManager.reset parameter is a public type-surface change, but the added runtime tests only consume that type internally and do not verify through a package entrypoint that nonce is exposed or that existing reset implementations remain assignable. Add a colocated .test-d.ts assertion and public type snapshot for this signature.
AGENTS.md reference: AGENTS.md:L72-L73
Useful? React with 👍 / 👎.
| type NonceManager, | ||
| nonceManager, | ||
| } from './nonceManager.js' | ||
| import { parseTransaction } from './transaction/parseTransaction.js' |
There was a problem hiding this comment.
parseTransaction is a public root export, so importing its implementation by a relative path makes this test depend on internal file layout instead of exercising the supported consumer surface. Import it from the viem test alias as required for public APIs.
AGENTS.md reference: AGENTS.md:L186-L190
Useful? React with 👍 / 👎.
Closes #5352. Supersedes #5369, keeping its commit by @aelmanaa (rebased onto
main) with review fixes on top.When
sendTransactionorsendTransactionSyncfails after the account's nonce manager consumed a nonce,resetnow receives that nonce as an optionalnoncefield. It is omitted whenconsumethrew. A manager that reserves a nonce per send can use it to release only the failed send's nonce when sends overlap. Existing managers keep working:createNonceManagerignores the field, and aresettyped with only{ address, chainId }is still assignable toNonceManager.Changes on top of #5369:
vi.spyOnor call-count assertions. They use custom nonce managers that keep released nonces in a free list, and snapshot the nonces each broadcast signs. With three concurrent sends where the nonce-6 broadcast fails, only 6 is released and the next send reuses it ([5, 6, 7, 6]). The two release tests fail without the source change.resetTSDoc and on thecreateNonceManagerpage.Verification: the 4 new tests pass. I ran them with the Anvil
beforeAlldisabled, since Anvil isn't available in my environment and these tests only use acustomtransport.tsc -breports no errors in changed files; its only errors come fromcontracts/generated.jsmissing without Foundry.Not changed: the reset parameter type stays unexported (as in #5369); implementers can use
Parameters<NonceManager['reset']>[0]. The two existing "does not reset an unconsumed nonce" tests still usevi.spyOn.Generated by Claude Code