From cbad744953e289106a9df151a2c63ff83539884c Mon Sep 17 00:00:00 2001 From: Mehran Mazhar Date: Mon, 14 Sep 2026 18:37:16 +0400 Subject: [PATCH 1/2] fix(node): get_next_nonce ignored the transaction pool 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 --- src/node/blockchain.rs | 89 +++++++++++++++++++++++++++++++++++++++ src/node/wss/websocket.rs | 8 ++-- 2 files changed, 94 insertions(+), 3 deletions(-) diff --git a/src/node/blockchain.rs b/src/node/blockchain.rs index f393e89..b4d8abc 100644 --- a/src/node/blockchain.rs +++ b/src/node/blockchain.rs @@ -240,6 +240,51 @@ impl Blockchain { AccountState::get_current_nonce(public_key, &self.db) } + /// The nonce a caller should put on its NEXT transaction. + /// + /// Not simply `confirmed + 1`. A transaction sitting in the pool has not moved the confirmed + /// nonce yet, so a caller that submits twice in quick succession — which is ordinary, and is + /// exactly what the treasury's outbox does when it has several mints to send — got the same + /// answer both times and signed two transactions with one nonce. The first to be mined wins and + /// the second becomes permanently invalid. + /// + /// That is not a lost transaction, it is a stopped chain: an invalid transaction cannot leave + /// the pool, and every candidate block carries the whole pool. Stage halted at block 84 on + /// 2026-09-14 this way. + /// + /// Walking upward from the confirmed nonce also fills gaps rather than skipping past them: if + /// the pool holds N+1 and N+3, the answer is N+2, which is the transaction that would let both + /// of the others become valid. + pub fn get_next_nonce(&self, public_key: &String) -> Result { + Self::next_nonce_for(&self.db, public_key) + } + + /// Split from the method so it can be tested against a scratch database, like the pool filters + /// above, rather than needing a whole Blockchain. + fn next_nonce_for(db: &Database, public_key: &String) -> Result { + use crate::node::transactions::address::canonical_account_address; + + let confirmed = AccountState::get_current_nonce(public_key, db)?; + let sender = canonical_account_address(public_key); + + // A pool read failure must not silently downgrade this to `confirmed + 1`: that is the + // colliding answer this function exists to stop giving. + let pooled = TransactionPool::get_transactions(db) + .map_err(|e| format!("could not read the transaction pool for {}: {}", public_key, e))?; + + let taken: std::collections::HashSet = pooled + .iter() + .filter(|tx| canonical_account_address(&tx.from) == sender) + .map(|tx| tx.nonce) + .collect(); + + let mut next = confirmed + 1; + while taken.contains(&next) { + next += 1; + } + Ok(next) + } + pub fn shutdown_blockchain(&mut self) { if !self.developer_mode { return; @@ -629,6 +674,50 @@ mod tests { db.delete_database(name).ok(); } + #[test] + fn next_nonce_skips_what_is_already_queued() { + // The outbox case that halted stage: two mints sent in one pass. Before this, both were + // told nonce 1, both were signed with it, and the second could never become valid. + let name = "clutch-node-test-nonce-queued"; + let db = scratch_db(name); + seed_nonce(&db, "0xA", 0); + + let first = Blockchain::next_nonce_for(&db, &"0xA".to_string()).unwrap(); + TransactionPool::add_transaction(&db, &tf("0xA", first, "0xC")).unwrap(); + let second = Blockchain::next_nonce_for(&db, &"0xA".to_string()).unwrap(); + + drop_scratch(db, name); + assert_eq!(first, 1); + assert_eq!(second, 2, "a queued nonce must not be handed out twice"); + } + + #[test] + fn next_nonce_fills_a_gap_rather_than_stepping_over_it() { + // Pool holds 1 and 3. The useful answer is 2 -- the transaction that lets both of the + // others become valid -- not 4, which would leave 3 stranded for ever. + let name = "clutch-node-test-nonce-gap"; + let db = scratch_db(name); + seed_nonce(&db, "0xA", 0); + TransactionPool::add_transaction(&db, &tf("0xA", 1, "0xC")).unwrap(); + TransactionPool::add_transaction(&db, &tf("0xA", 3, "0xD")).unwrap(); + + let next = Blockchain::next_nonce_for(&db, &"0xA".to_string()).unwrap(); + drop_scratch(db, name); + assert_eq!(next, 2); + } + + #[test] + fn next_nonce_ignores_other_senders() { + let name = "clutch-node-test-nonce-other"; + let db = scratch_db(name); + seed_nonce(&db, "0xA", 4); + TransactionPool::add_transaction(&db, &tf("0xB", 5, "0xC")).unwrap(); + + let next = Blockchain::next_nonce_for(&db, &"0xA".to_string()).unwrap(); + drop_scratch(db, name); + assert_eq!(next, 5, "another account's queue says nothing about this one"); + } + #[test] fn evicts_a_nonce_the_chain_has_already_consumed() { // The stage halt of 2026-09-14 in miniature: a transaction whose nonce the account has diff --git a/src/node/wss/websocket.rs b/src/node/wss/websocket.rs index 9895fa7..c979c80 100644 --- a/src/node/wss/websocket.rs +++ b/src/node/wss/websocket.rs @@ -256,9 +256,11 @@ impl WebSocket { // Get the blockchain lock let blockchain = blockchain.lock().await; - match blockchain.get_current_nonce(¶ms.address) { - Ok(nonce) => { - let next_nonce = nonce + 1; + // The pool-aware answer: `get_current_nonce() + 1` ignored anything already queued, so two + // submissions in quick succession received the same nonce and the second could never be + // valid. See Blockchain::get_next_nonce. + match blockchain.get_next_nonce(¶ms.address) { + Ok(next_nonce) => { Some(json_rpc_success_response(serde_json::json!({ "nonce": next_nonce }), id)) } Err(e) => { From dbcd0bffeca3781802f76280d38c76aa615c5323 Mon Sep 17 00:00:00 2001 From: Mehran Mazhar Date: Mon, 14 Sep 2026 18:44:06 +0400 Subject: [PATCH 2/2] test: seed the pool directly instead of through add_transaction 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 --- src/node/blockchain.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/node/blockchain.rs b/src/node/blockchain.rs index b4d8abc..e726732 100644 --- a/src/node/blockchain.rs +++ b/src/node/blockchain.rs @@ -664,6 +664,16 @@ mod tests { .expect("seed nonce"); } + /// Put a transaction in the pool WITHOUT `TransactionPool::add_transaction`, which first + /// validates the signature. `tf` builds unsigned transactions, and what these tests exercise + /// is the nonce arithmetic over whatever the pool holds, not the signature check -- so the + /// test writes the same key and bytes `add_transaction` would have written. + fn pool(db: &Database, transaction: &Transaction) { + let key = TransactionPool::construct_tx_pool_key(&transaction.hash); + let value = serde_json::to_string(transaction).unwrap().into_bytes(); + db.put("tx_pool", &key, &value).expect("pool transaction"); + } + fn scratch_db(name: &str) -> Database { let _ = std::fs::remove_dir_all(format!("{}.db", name)); Database::new_db(name) @@ -683,7 +693,7 @@ mod tests { seed_nonce(&db, "0xA", 0); let first = Blockchain::next_nonce_for(&db, &"0xA".to_string()).unwrap(); - TransactionPool::add_transaction(&db, &tf("0xA", first, "0xC")).unwrap(); + pool(&db, &tf("0xA", first, "0xC")); let second = Blockchain::next_nonce_for(&db, &"0xA".to_string()).unwrap(); drop_scratch(db, name); @@ -698,8 +708,8 @@ mod tests { let name = "clutch-node-test-nonce-gap"; let db = scratch_db(name); seed_nonce(&db, "0xA", 0); - TransactionPool::add_transaction(&db, &tf("0xA", 1, "0xC")).unwrap(); - TransactionPool::add_transaction(&db, &tf("0xA", 3, "0xD")).unwrap(); + pool(&db, &tf("0xA", 1, "0xC")); + pool(&db, &tf("0xA", 3, "0xD")); let next = Blockchain::next_nonce_for(&db, &"0xA".to_string()).unwrap(); drop_scratch(db, name); @@ -711,7 +721,7 @@ mod tests { let name = "clutch-node-test-nonce-other"; let db = scratch_db(name); seed_nonce(&db, "0xA", 4); - TransactionPool::add_transaction(&db, &tf("0xB", 5, "0xC")).unwrap(); + pool(&db, &tf("0xB", 5, "0xC")); let next = Blockchain::next_nonce_for(&db, &"0xA".to_string()).unwrap(); drop_scratch(db, name);