fix(mempool): do not allow duplicate nonce if fee escalation if off (#…

…2064)

crates/starknet_mempool/src/mempool.rs

@@ -258,31 +258,37 @@ impl Mempool {
 
     #[tracing::instrument(level = "debug", skip(self, incoming_tx), err)]
     fn handle_fee_escalation(&mut self, incoming_tx: &AccountTransaction) -> MempoolResult<()> {
+        let incoming_tx_reference = TransactionReference::new(incoming_tx);
+        let TransactionReference { address, nonce, .. } = incoming_tx_reference;
+
         if !self.config.enable_fee_escalation {
+            if self.tx_pool.get_by_address_and_nonce(address, nonce).is_some() {
+                return Err(MempoolError::DuplicateNonce { address, nonce });
+            };
+
             return Ok(());
         }
 
-        let incoming_tx_ref = TransactionReference::new(incoming_tx);
-        let TransactionReference { address, nonce, .. } = incoming_tx_ref;
-        let Some(existing_tx_ref) = self.tx_pool.get_by_address_and_nonce(address, nonce) else {
+        let Some(existing_tx_reference) = self.tx_pool.get_by_address_and_nonce(address, nonce)
+        else {
             // Replacement irrelevant: no existing transaction with the same nonce for address.
             return Ok(());
         };
 
-        if !self.should_replace_tx(&existing_tx_ref, &incoming_tx_ref) {
+        if !self.should_replace_tx(&existing_tx_reference, &incoming_tx_reference) {
             tracing::debug!(
-                "{existing_tx_ref} was not replaced by {incoming_tx_ref} due to insufficient
-                fee escalation."
+                "{existing_tx_reference} was not replaced by {incoming_tx_reference} due to
+                insufficient fee escalation."
             );
             // TODO(Elin): consider adding a more specific error type / message.
             return Err(MempoolError::DuplicateNonce { address, nonce });
         }
 
-        tracing::debug!("{existing_tx_ref} will be replaced by {incoming_tx_ref}.");
+        tracing::debug!("{existing_tx_reference} will be replaced by {incoming_tx_reference}.");
 
         self.tx_queue.remove(address);
         self.tx_pool
-            .remove(existing_tx_ref.tx_hash)
+            .remove(existing_tx_reference.tx_hash)
             .expect("Transaction hash from pool must exist.");
 
         Ok(())

crates/starknet_mempool/src/mempool_test.rs

@@ -372,18 +372,19 @@ fn test_add_tx_lower_than_queued_nonce(mut mempool: Mempool) {
     add_tx(&mut mempool, &input);
 
     // Test and assert: original transaction remains.
-    for tx_nonce in [0, 1] {
-        let invalid_input =
-            add_tx_input!(tx_hash: 2, address: "0x0", tx_nonce: tx_nonce, account_nonce: 0);
-        add_tx_expect_error(
-            &mut mempool,
-            &invalid_input,
-            MempoolError::DuplicateNonce {
-                address: contract_address!("0x0"),
-                nonce: nonce!(tx_nonce),
-            },
-        );
-    }
+    let invalid_input = add_tx_input!(tx_hash: 2, address: "0x0", tx_nonce: 0, account_nonce: 0);
+    add_tx_expect_error(
+        &mut mempool,
+        &invalid_input,
+        MempoolError::DuplicateNonce { address: contract_address!("0x0"), nonce: nonce!(0) },
+    );
+
+    let invalid_input = add_tx_input!(tx_hash: 2, address: "0x0", tx_nonce: 1, account_nonce: 0);
+    add_tx_expect_error(
+        &mut mempool,
+        &invalid_input,
+        MempoolError::DuplicateNonce { address: contract_address!("0x0"), nonce: nonce!(1) },
+    );
 }
 
 #[rstest]

crates/starknet_mempool/src/transaction_pool.rs

@@ -41,8 +41,9 @@ impl TransactionPool {
         let unexpected_existing_tx = self.txs_by_account.insert(tx_reference);
         if unexpected_existing_tx.is_some() {
             panic!(
-                "Transaction pool consistency error: transaction with hash {tx_hash} does not \
-                 appear in main mapping, but it appears in the account mapping",
+                "Transaction pool consistency error: transaction with hash {tx_hash} does not
+                appear in main mapping, but transaction with same nonce appears in the account
+                mapping",
             )
         };
 
@@ -59,8 +60,8 @@ impl TransactionPool {
         // Remove from account mapping.
         self.txs_by_account.remove(TransactionReference::new(&tx)).unwrap_or_else(|| {
             panic!(
-                "Transaction pool consistency error: transaction with hash {tx_hash} appears in \
-                 main mapping, but does not appear in the account mapping"
+                "Transaction pool consistency error: transaction with hash {tx_hash} appears in
+                main mapping, but does not appear in the account mapping"
             )
         });
 
@@ -75,8 +76,8 @@ impl TransactionPool {
         for TransactionReference { tx_hash, .. } in removed_txs {
             self.tx_pool.remove(&tx_hash).unwrap_or_else(|| {
                 panic!(
-                    "Transaction pool consistency error: transaction with hash {tx_hash} appears \
-                     in account mapping, but does not appear in the main mapping"
+                    "Transaction pool consistency error: transaction with hash {tx_hash} appears
+                    in account mapping, but does not appear in the main mapping"
                 );
             });