From cd83e06762088b074f51f829242296d52dc7678c Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Wed, 1 Jul 2026 21:37:05 +0800 Subject: [PATCH] fix(pool): don't gate L1-handler txs on account nonce MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pool validated the settlement chain's global L1->L2 message nonce as if it were a per-target-contract sequential account nonce. Since the pool has no future/queued tier, the first non-contiguous per-target nonce — a second target contract, a node restart wiping the in-memory `pool_nonces`, or a single dropped message — was rejected as InvalidNonce and dropped, and so was every later message, permanently. The L1-handler nonce is not an account nonce: it is global/monotonic across all messages, the executor never increments the target's nonce, and blockifier treats it as metadata (matching the official sequencer, which keeps L1-handlers out of the account-nonce mempool). Short-circuit L1-handler txs to Valid before the nonce-dependency gate, leaving `pool_nonces` untouched. Adds pool unit tests (gapped / interleaved targets / restart, plus an account-tx nonce-gap regression) and extends the messaging e2e with a second target contract (verified to fail without this fix). Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/messaging/tests/e2e.rs | 67 +++++++++++ crates/pool/pool/src/validation/stateful.rs | 116 ++++++++++++++++++++ 2 files changed, 183 insertions(+) diff --git a/crates/messaging/tests/e2e.rs b/crates/messaging/tests/e2e.rs index e74229060..72f87d504 100644 --- a/crates/messaging/tests/e2e.rs +++ b/crates/messaging/tests/e2e.rs @@ -229,6 +229,73 @@ async fn test_messaging() { } } + // Regression for the global-message-nonce vs per-target-account-nonce bug: the L1 core + // contract assigns a single monotonic nonce across ALL messages, so a message to a *second* + // target contract carries a nonce that is non-contiguous for that target (its L2 account + // nonce is still 0). The pool must not treat the L1-handler nonce as a per-target account + // nonce and reject it — every subsequent L1->L2 message would then stall permanently. + { + // Deploy a second instance of the already-declared L2 contract (different salt => new + // address) to act as a distinct message target. + let class_hash = rpc_client + .get_class_hash_at(BlockIdOrTag::Latest, l2_test_contract.into()) + .await + .expect("get class hash of first target"); + + let target_b_address = get_contract_address(Felt::ONE, class_hash, &[], Felt::ZERO); + let deploy = ContractFactory::new_with_udc(class_hash, &katana_account, UdcSelector::New) + .deploy_v3(Vec::new(), Felt::ONE, false) + .send() + .await + .expect("deploy second target contract"); + TxWaiter::new(deploy.transaction_hash, &rpc_client).await.expect("deploy B tx failed"); + + let sender = l1_test_contract.address(); + let recipient = ContractAddress::from(target_b_address); + let selector = selector!("msg_handler_value"); + // `msg_handler_value` only accepts this specific value; the point of this case is the + // distinct `to` target, not the payload. + let calldata = [123u8]; + // Global nonce has advanced past the first message, so this is > 0 while target B's + // account nonce is still 0 — the exact gap that used to be rejected as InvalidNonce. + let nonce = core_contract.l1ToL2MessageNonce().call().await.expect("get nonce"); + + let receipt = l1_test_contract + .sendMessage( + recipient.into(), + U256::from_be_bytes(selector.to_bytes_be()), + calldata.iter().map(|x| U256::from(*x)).collect::>(), + ) + .gas(12000000) + .value(Uint::from(1)) + .send() + .await + .expect("failed to send tx") + .get_receipt() + .await + .expect("error getting transaction receipt"); + assert!(receipt.status(), "failed to send L1 -> L2 message to second target"); + + let mut l1_tx_calldata = vec![Felt::from_bytes_be_slice(sender.as_slice())]; + l1_tx_calldata.extend(calldata.iter().map(|x| Felt::from(*x))); + + let tx_hash = compute_l1_handler_tx_hash( + Felt::ZERO, + recipient, + selector, + &l1_tx_calldata, + sequencer.starknet_provider().chain_id().await.unwrap(), + nonce.to::().into(), + ); + + // The message to the second target must be mined on L2. Before the fix this L1-handler + // was rejected with InvalidNonce (nonce ahead of target B's account nonce) and this wait + // would time out. + TxWaiter::new(tx_hash, &rpc_client) + .await + .expect("second-target L1 handler must be mined despite the global-nonce gap"); + } + // Send message from L2 to L1 testing must be done using Saya or part of // it to ensure the settlement contract is test on piltover and its `update_state` method. } diff --git a/crates/pool/pool/src/validation/stateful.rs b/crates/pool/pool/src/validation/stateful.rs index f52762cae..fc66c56b7 100644 --- a/crates/pool/pool/src/validation/stateful.rs +++ b/crates/pool/pool/src/validation/stateful.rs @@ -140,6 +140,20 @@ impl Validator for TxValidator { } } + // L1-handler txs carry the settlement chain's GLOBAL message nonce, which is not an + // L2 account nonce: it is not sequential per target contract, the executor never + // increments it, and it has no bearing on execution (blockifier treats it as metadata). + // + // Gating it through `pool_nonces` (account-nonce machinery) permanently rejects messages + // whenever the per-target sequence isn't contiguous — multiple target contracts, a + // restart (`pool_nonces` is wiped), or a single dropped message. Accept them + // unconditionally and leave `pool_nonces` untouched. + // + // This mirrors the official sequencer, which keeps L1-handler txs out of the account-nonce mempool entirely. + if matches!(tx.transaction, ExecutableTx::L1Handler(_)) { + return Ok(ValidationOutcome::Valid(tx)); + } + // Get the current nonce of the account from the pool or the state let current_nonce = if let Some(nonce) = this.pool_nonces.get(&address) { *nonce @@ -345,3 +359,105 @@ fn map_pre_validation_err( } } } + +#[cfg(test)] +mod tests { + use katana_primitives::transaction::{InvokeTx, InvokeTxV1, L1HandlerTx}; + use katana_provider::providers::EmptyStateProvider; + + use super::*; + + /// A validator over an empty state (every account nonce reads as 0) with default + /// envs. The assertions here all resolve before `prepare()` / blockifier, so no + /// real chain state is needed. + fn test_validator() -> TxValidator { + let state: Box = Box::new(EmptyStateProvider); + TxValidator::new( + state, + ExecutionFlags::new(), + None, + BlockEnv::default(), + Arc::new(Mutex::new(())), + Arc::new(ChainSpec::dev()), + ClassCache::new().expect("class cache"), + ) + } + + fn l1_handler(target: ContractAddress, nonce: u64) -> ExecutableTxWithHash { + ExecutableTxWithHash::new(ExecutableTx::L1Handler(L1HandlerTx { + nonce: Felt::from(nonce), + contract_address: target, + entry_point_selector: Felt::from(0x1234_u32), + ..Default::default() + })) + } + + fn invoke(sender: ContractAddress, nonce: u64) -> ExecutableTxWithHash { + ExecutableTxWithHash::new(ExecutableTx::Invoke(InvokeTx::V1(InvokeTxV1 { + sender_address: sender, + nonce: Felt::from(nonce), + ..Default::default() + }))) + } + + // An L1-handler carries the settlement chain's global message nonce, not an account + // nonce. A non-contiguous sequence to the same target (a gap) must NOT be rejected — + // otherwise a single dropped message permanently stalls all later ones. + #[tokio::test] + async fn l1_handler_nonce_gap_is_accepted() { + let validator = test_validator(); + let target = ContractAddress::from(Felt::from(0xbeef_u64)); + + for nonce in [0u64, 2, 5] { + let outcome = validator.validate(l1_handler(target, nonce)).await.unwrap(); + assert!( + matches!(outcome, ValidationOutcome::Valid(_)), + "gapped L1-handler nonce {nonce} must be accepted, got {outcome:?}", + ); + } + } + + // Global message nonce interleaved across two target contracts (0 -> A, 1 -> B, 2 -> A) + // leaves each target with a gapped subsequence; all must still be accepted. + #[tokio::test] + async fn l1_handler_interleaved_targets_all_accepted() { + let validator = test_validator(); + let a = ContractAddress::from(Felt::from(0xa_u64)); + let b = ContractAddress::from(Felt::from(0xb_u64)); + + for (target, nonce) in [(a, 0u64), (b, 1), (a, 2), (b, 3), (a, 6)] { + let outcome = validator.validate(l1_handler(target, nonce)).await.unwrap(); + assert!( + matches!(outcome, ValidationOutcome::Valid(_)), + "interleaved L1-handler (target {target:?}, nonce {nonce}) must be accepted", + ); + } + } + + // Simulates a restart: a fresh validator (empty `pool_nonces`) receives a message whose + // nonce is already high because the settlement chain's counter kept climbing. It must be + // accepted, not rejected as `InvalidNonce`. + #[tokio::test] + async fn l1_handler_high_first_nonce_accepted_after_restart() { + let validator = test_validator(); + let target = ContractAddress::from(Felt::from(0xbeef_u64)); + + let outcome = validator.validate(l1_handler(target, 30)).await.unwrap(); + assert!(matches!(outcome, ValidationOutcome::Valid(_)), "got {outcome:?}"); + } + + // Regression: ungating L1-handlers must NOT weaken account-tx nonce gating. An invoke + // whose nonce is ahead of the account nonce is still tagged `Dependent`. + #[tokio::test] + async fn account_tx_nonce_gap_still_gated() { + let validator = test_validator(); + let sender = ContractAddress::from(Felt::from(0xacc_u64)); + + // account nonce in the empty state is 0; nonce 3 is a gap. + let outcome = validator.validate(invoke(sender, 3)).await.unwrap(); + assert!( + matches!(outcome, ValidationOutcome::Dependent { .. }), + "account tx with a nonce gap must be gated as Dependent, got {outcome:?}", + ); + } +}