fix(node): get_next_nonce ignored the transaction pool - #17
Merged
Merged
Conversation
It answered confirmed_nonce + 1. A transaction sitting in the pool has not moved the confirmed nonce, so a caller submitting twice in quick succession was told the same number both times and signed two transactions with one nonce. The first to be mined wins; the second can never be valid. That is ordinary usage, not abuse -- it is exactly what the treasury's outbox does when it has several mints to send, and it is why stage halted at block 84 on 2026-09-14 with a stale mint wedged in the pool. The observed state matched: one mint landed, three credits recorded, one poisoned transaction left behind. Now walks upward from the confirmed nonce over what the pool already holds for that sender. Walking rather than taking the maximum fills gaps: with 1 and 3 queued the answer is 2, the transaction that lets both of the others become valid, rather than 4, which would strand 3 for ever. A pool read failure is an error rather than a silent fall back to confirmed + 1, since that is precisely the colliding answer this exists to stop giving. Three tests: a queued nonce is not handed out twice, a gap is filled, another sender's queue is ignored. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The three new nonce tests failed with "r and s must each be 32 bytes". `TransactionPool::add_transaction` validates the signature first, and `tf` builds unsigned transactions -- every other test here passes them around in memory, so none of them had hit that path before. What is under test is the nonce arithmetic over whatever the pool holds, not the signature check, so the tests now write the same key and bytes `add_transaction` would have written, exactly as `seed_nonce` already does for account state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It answered
confirmed_nonce + 1. A transaction sitting in the pool has not moved the confirmed nonce, so a caller submitting twice in quick succession was told the same number both times and signed two transactions with one nonce. The first to be mined wins; the second can never be valid.That is ordinary usage, not abuse. It is exactly what the treasury's outbox does when it has several mints to send — it fetches a nonce per row and submits them in one pass — and it is why stage halted at block 84 on 2026-09-14 with a stale mint wedged in the pool.
The observed state matched precisely: supply $10 (one mint landed), ledger liability $30 (three credits recorded), one poisoned transaction left behind.
The fix
Walk upward from the confirmed nonce over what the pool already holds for that sender.
Walking rather than taking the maximum fills gaps. With 1 and 3 queued the answer is 2 — the transaction that lets both of the others become valid — rather than 4, which would strand 3 for ever.
A pool read failure is an error rather than a silent fall back to
confirmed + 1, since that is precisely the colliding answer this exists to stop giving.Three tests: a queued nonce is not handed out twice, a gap is filled, another sender's queue is ignored.
Why this and not a treasury-side fix
Any client submitting two transactions quickly hits this — the SDK, the demo app, anything. Fixing only the outbox would leave the trap set for every other caller.
Together with #16 this closes both halves: that one stops an invalid transaction from halting the chain, this one stops it being created.
🤖 Generated with Claude Code