diff --git a/crates/astria-sequencer/src/mempool/mod.rs b/crates/astria-sequencer/src/mempool/mod.rs index 68579620f4..6ffbf95f69 100644 --- a/crates/astria-sequencer/src/mempool/mod.rs +++ b/crates/astria-sequencer/src/mempool/mod.rs @@ -552,25 +552,6 @@ mod tests { "already present" ); - // try to replace nonce - let tx1_replacement = MockTxBuilder::new() - .nonce(1) - .chain_id("test-chain-id") - .build(); - assert_eq!( - mempool - .insert( - tx1_replacement.clone(), - 0, - account_balances.clone(), - tx_cost.clone(), - ) - .await - .unwrap_err(), - InsertionError::NonceTaken, - "nonce replace not allowed" - ); - // add too low nonce let tx0 = MockTxBuilder::new().nonce(0).build(); assert_eq!( diff --git a/crates/astria-sequencer/src/mempool/transactions_container.rs b/crates/astria-sequencer/src/mempool/transactions_container.rs index 95d17b00c1..f20e3e153d 100644 --- a/crates/astria-sequencer/src/mempool/transactions_container.rs +++ b/crates/astria-sequencer/src/mempool/transactions_container.rs @@ -301,6 +301,10 @@ impl TransactionsForAccount for PendingTransactionsForAccount { self.txs().contains_key(&previous_nonce) || ttx.signed_tx.nonce() == current_account_nonce } + fn nonce_replacement_enabled(&self) -> bool { + false + } + fn has_balance_to_cover( &self, ttx: &TimemarkedTransaction, @@ -377,6 +381,10 @@ impl TransactionsForAccount true } + fn nonce_replacement_enabled(&self) -> bool { + true + } + fn has_balance_to_cover( &self, _: &TimemarkedTransaction, @@ -410,6 +418,9 @@ pub(super) trait TransactionsForAccount: Default { current_account_nonce: u32, ) -> bool; + /// Returns true if the container allows for nonce replacement. + fn nonce_replacement_enabled(&self) -> bool; + /// Returns `Ok` if adding `ttx` would not break the balance precondition, i.e. enough /// balance to cover all transactions. /// Note: some implementations may clone the `current_account_balance` hashmap. @@ -444,11 +455,16 @@ pub(super) trait TransactionsForAccount: Default { } if let Some(existing_ttx) = self.txs().get(&ttx.signed_tx.nonce()) { - return Err(if existing_ttx.tx_hash == ttx.tx_hash { - InsertionError::AlreadyPresent + return if existing_ttx.tx_hash == ttx.tx_hash { + Err(InsertionError::AlreadyPresent) } else { - InsertionError::NonceTaken - }); + if self.nonce_replacement_enabled() { + self.txs_mut().insert(ttx.signed_tx.nonce(), ttx); + Ok(()) + } else { + Err(InsertionError::NonceTaken) + } + }; } if !self.is_sequential_nonce_precondition_met(&ttx, current_account_nonce) { @@ -1536,15 +1552,6 @@ mod tests { "re-adding same transaction should fail" ); - // nonce replacement fails - assert_eq!( - pending_txs - .add(ttx_s0_0_1, 0, &account_balances) - .unwrap_err(), - InsertionError::NonceTaken, - "nonce replacement not supported" - ); - // nonce gaps not supported assert_eq!( pending_txs @@ -1586,6 +1593,55 @@ mod tests { ); } + #[test] + fn transactions_container_nonce_replacement_parked() { + let mut parked_txs = ParkedTransactions::::new(TX_TTL, 10); + let account_balances = mock_balances(1, 1); + + // create two transactions with same nonce but different hash + let tx_0 = MockTTXBuilder::new().nonce(0).group(Group::UnbundleableSudo).build(); + let tx_1 = MockTTXBuilder::new().nonce(0).group(Group::UnbundleableGeneral).build(); + + // add first transaction + parked_txs.add(tx_0.clone(), 0, &account_balances).unwrap(); + + // replacing transaction should be ok + let res = parked_txs.add(tx_1.clone(), 0, &account_balances); + assert!(res.is_ok(), "nonce replacement for parked should be ok"); + + // a single transaction should be in the parked txs + assert_eq!( + parked_txs.len(), + 1, + "one transaction should be in the parked txs" + ); + + // the transaction should have been replaced + assert_eq!(parked_txs.contains_tx(&tx_1.tx_hash), true); + assert_eq!(parked_txs.contains_tx(&tx_0.tx_hash), false); + } + + #[test] + fn transactions_container_nonce_replacement_pending() { + let mut pending_txs = PendingTransactions::new(TX_TTL); + let account_balances = mock_balances(1, 1); + + // create two transactions with same nonce but different hash + let tx_0 = MockTTXBuilder::new().nonce(0).group(Group::UnbundleableSudo).build(); + let tx_1 = MockTTXBuilder::new().nonce(0).group(Group::UnbundleableGeneral).build(); + + // add first transaction + pending_txs.add(tx_0.clone(), 0, &account_balances).unwrap(); + + // replacing transaction should fail + let res = pending_txs.add(tx_1.clone(), 0, &account_balances); + assert_eq!(res, Err(InsertionError::NonceTaken), "nonce replacement for parked should return false"); + + // the transaction should not have been replaced + assert_eq!(pending_txs.contains_tx(&tx_0.tx_hash), true); + assert_eq!(pending_txs.contains_tx(&tx_1.tx_hash), false); + } + #[test] fn transactions_container_remove() { let mut pending_txs = PendingTransactions::new(TX_TTL);