Skip to content

Commit 85abda8

Browse files
kariyclaude
andcommitted
fix(pool): clear pool_nonces on validator update to prevent nonce drift
pool_nonces tracks the expected next nonce per account based on validated transactions. When update() is called after a block is mined, it replaced the state and block env but never cleared pool_nonces. This caused stale nonce values to persist across block boundaries, allowing transactions with nonces far ahead of the actual state to pass validation. The executor then rejected them with "Invalid transaction nonce" errors. Also adds an early rejection check for tx_nonce < current_nonce to fail fast before reaching the blockifier. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 63f5688 commit 85abda8

File tree

2 files changed

+154
-3
lines changed

2 files changed

+154
-3
lines changed

crates/pool/pool/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,4 @@ tracing.workspace = true
2222
futures-util.workspace = true
2323
rand.workspace = true
2424
starknet.workspace = true
25+
tokio = { workspace = true, features = ["macros", "rt"] }

crates/pool/pool/src/validation/stateful.rs

Lines changed: 153 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ impl TxValidator {
7272
let mut this = self.inner.lock();
7373
this.block_env = block_env;
7474
this.state = Arc::new(new_state);
75+
this.pool_nonces.clear();
7576
}
7677
}
7778

@@ -116,6 +117,7 @@ impl Validator for TxValidator {
116117
let permit = self.permit.clone();
117118
let tx_hash = tx.hash();
118119

120+
let span = tracing::trace_span!(target: "pool", "pool_validate", tx_hash = format!("{:#x}", tx_hash));
119121
async move {
120122
let _permit = permit.lock();
121123
let mut this = inner.lock();
@@ -149,6 +151,22 @@ impl Validator for TxValidator {
149151
if tx_nonce > current_nonce {
150152
return Ok(ValidationOutcome::Dependent { current_nonce, tx_nonce, tx });
151153
}
154+
// this nonce validation is also handled in this function:
155+
// blockifier::transaction::account_transaction::AccountTransaction::perform_pre_validation_stage
156+
// |
157+
// -- blockifier::transaction::account_transaction::AccountTransaction::handle_nonce
158+
//
159+
// but we're handle this here to fail early
160+
if tx_nonce < current_nonce {
161+
return Ok(ValidationOutcome::Invalid {
162+
tx,
163+
error: InvalidTransactionError::InvalidNonce {
164+
address,
165+
current_nonce,
166+
tx_nonce,
167+
},
168+
});
169+
}
152170

153171
// Check if validation of an invoke transaction should be skipped due to deploy_account
154172
// not being proccessed yet. This feature is used to improve UX for users
@@ -179,9 +197,7 @@ impl Validator for TxValidator {
179197
_ => result,
180198
}
181199
}
182-
.instrument(
183-
tracing::trace_span!(target: "pool", "pool_validate", tx_hash = format!("{:#x}", tx_hash))
184-
)
200+
.instrument(span)
185201
}
186202
}
187203

@@ -342,3 +358,137 @@ fn map_pre_validation_err(
342358
}
343359
}
344360
}
361+
362+
#[cfg(test)]
363+
mod tests {
364+
use std::sync::Arc;
365+
366+
use katana_chain_spec::ChainSpec;
367+
use katana_executor::blockifier::cache::ClassCacheBuilder;
368+
use katana_executor::ExecutionFlags;
369+
use katana_pool_api::validation::{ValidationOutcome, Validator};
370+
use katana_primitives::block::{Block, FinalityStatus, SealedBlockWithStatus};
371+
use katana_primitives::chain::ChainId;
372+
use katana_primitives::contract::{ContractAddress, Nonce};
373+
use katana_primitives::env::BlockEnv;
374+
use katana_primitives::transaction::{ExecutableTx, ExecutableTxWithHash, InvokeTx, InvokeTxV1};
375+
use katana_primitives::Felt;
376+
use katana_provider::api::block::BlockWriter;
377+
use katana_provider::api::state::{StateFactoryProvider, StateProvider};
378+
use katana_provider::{DbProviderFactory, MutableProvider, ProviderFactory};
379+
use parking_lot::Mutex;
380+
381+
use super::TxValidator;
382+
383+
fn create_test_state(chain_spec: &ChainSpec) -> Box<dyn StateProvider> {
384+
let ChainSpec::Dev(chain) = chain_spec else { panic!("should be dev chain spec") };
385+
let states = chain.state_updates();
386+
let provider_factory = DbProviderFactory::new_in_memory();
387+
let provider_mut = provider_factory.provider_mut();
388+
let block = SealedBlockWithStatus {
389+
status: FinalityStatus::AcceptedOnL2,
390+
block: Block::default().seal_with_hash(Felt::ZERO),
391+
};
392+
provider_mut
393+
.insert_block_with_states_and_receipts(block, states, vec![], vec![])
394+
.unwrap();
395+
provider_mut.commit().unwrap();
396+
provider_factory.provider().latest().unwrap()
397+
}
398+
399+
fn create_invoke_tx(
400+
sender: ContractAddress,
401+
chain_id: ChainId,
402+
nonce: Nonce,
403+
) -> ExecutableTxWithHash {
404+
ExecutableTxWithHash::new(ExecutableTx::Invoke(InvokeTx::V1(InvokeTxV1 {
405+
chain_id,
406+
sender_address: sender,
407+
nonce,
408+
calldata: vec![],
409+
signature: vec![],
410+
max_fee: 1_000_000_000_000_000,
411+
})))
412+
}
413+
414+
/// Reproduces the pool_nonces drift bug after `validator.update()`.
415+
///
416+
/// `pool_nonces` tracks the expected next nonce per account based on validated
417+
/// transactions. When `update()` is called after a block is mined, it replaces
418+
/// the state and block env but does NOT clear `pool_nonces`. If none of the
419+
/// validated transactions were actually committed (e.g. they were dropped or
420+
/// the block was empty), pool_nonces retains stale values that are far ahead
421+
/// of the actual state nonce.
422+
///
423+
/// This causes the validator to skip the `tx_nonce > current_nonce` check
424+
/// (which would correctly flag the tx as Dependent) and fall through to the
425+
/// blockifier, which — with `strict_nonce_check = false` — allows any
426+
/// `account_nonce <= tx_nonce`. Since the real state nonce is 0, a tx with
427+
/// nonce 3 passes validation despite the massive nonce gap.
428+
///
429+
/// In production, this manifests as:
430+
/// "Invalid transaction nonce of contract at address ... Account nonce: 0xN; got: 0xM."
431+
/// where M >> N, because the executor sees the real state nonce.
432+
///
433+
/// This test FAILS with the current buggy code and PASSES once the fix
434+
/// (clearing pool_nonces in update()) is applied.
435+
#[tokio::test]
436+
async fn pool_nonces_must_be_cleared_after_validator_update() {
437+
// Initialize the global class cache (required by the blockifier)
438+
let _ = ClassCacheBuilder::new().build_global();
439+
440+
let chain_spec = Arc::new(ChainSpec::dev());
441+
let chain_id = chain_spec.id();
442+
let sender = *chain_spec.genesis().accounts().next().unwrap().0;
443+
444+
let state = create_test_state(&chain_spec);
445+
let execution_flags =
446+
ExecutionFlags::new().with_account_validation(false).with_fee(false);
447+
let block_env = BlockEnv::default();
448+
let permit = Arc::new(Mutex::new(()));
449+
450+
let validator = TxValidator::new(
451+
state,
452+
execution_flags,
453+
None,
454+
block_env.clone(),
455+
permit,
456+
chain_spec.clone(),
457+
);
458+
459+
// Validate 3 txs with nonces 0, 1, 2 — all should pass as Valid.
460+
// This advances pool_nonces[sender] to 3.
461+
for nonce in 0..3u64 {
462+
let tx = create_invoke_tx(sender, chain_id, Felt::from(nonce));
463+
let result = validator.validate(tx).await;
464+
assert!(
465+
matches!(result, Ok(ValidationOutcome::Valid(_))),
466+
"tx with nonce {nonce} should be Valid"
467+
);
468+
}
469+
470+
// Simulate block production where NONE of the txs were committed
471+
// (e.g. they were dropped, reverted, or the block was empty).
472+
// update() replaces state (nonce = 0) but pool_nonces stays at 3.
473+
let fresh_state = create_test_state(&chain_spec);
474+
validator.update(fresh_state, block_env);
475+
476+
// Now validate a tx with nonce 3. With the bug:
477+
// - current_nonce = pool_nonces[sender] = 3 (stale, not cleared)
478+
// - tx_nonce(3) == current_nonce(3) → falls through to blockifier
479+
// - blockifier (non-strict): state_nonce(0) <= tx_nonce(3) → passes
480+
// - Result: Valid ← WRONG! nonce gap of 3 from actual state
481+
//
482+
// After fix (pool_nonces cleared in update()):
483+
// - current_nonce = state.nonce(sender) = 0
484+
// - tx_nonce(3) > current_nonce(0) → Dependent ← CORRECT
485+
let tx = create_invoke_tx(sender, chain_id, Felt::THREE);
486+
let result = validator.validate(tx).await;
487+
488+
assert!(
489+
matches!(result, Ok(ValidationOutcome::Dependent { .. })),
490+
"After update(), tx with nonce 3 should be Dependent (state nonce is 0), \
491+
but stale pool_nonces caused it to be accepted as Valid. Got: {result:?}"
492+
);
493+
}
494+
}

0 commit comments

Comments
 (0)