soft commit

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!(

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<const MAX_TX_COUNT: usize> 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::<MAX_PARKED_TXS_PER_ACCOUNT>::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);