Confirmed tx should not be marked as conflicted

mintlayer · Jan 21, 2025 · 3a2fd08 · 3a2fd08
1 parent 7281875
commit 3a2fd08
Show file tree

Hide file tree

Showing 3 changed files with 393 additions and 38 deletions.
diff --git a/test/functional/wallet_conflict.py b/test/functional/wallet_conflict.py
@@ -198,7 +198,7 @@ async def async_test(self):
 
             # check we cannot abandon an already confirmed transaction
             assert_in("Success", await wallet.select_account(1))
-            assert_in("Cannot abandon a transaction in Confirmed at height 6", await wallet.abandon_transaction(new_transfer_tx_id))
+            assert_in("Cannot change a transaction's state from Confirmed", await wallet.abandon_transaction(new_transfer_tx_id))
 
 
 

diff --git a/wallet/src/account/output_cache/mod.rs b/wallet/src/account/output_cache/mod.rs
@@ -689,9 +689,6 @@ impl OutputCache {
         confirmed_tx: &Transaction,
         block_id: Id<GenBlock>,
     ) -> WalletResult<Vec<Id<Transaction>>> {
-        // Collect all conflicting txs
-        let mut conflicting_txs = vec![];
-
         let mut frozen_token_id: Option<TokenId> = None;
         let mut confirmed_account_nonce: Option<(AccountType, AccountNonce)> = None;
 
@@ -722,14 +719,15 @@ impl OutputCache {
             }
         }
 
-        if frozen_token_id.is_some() | confirmed_account_nonce.is_some() {
+        // Collect all conflicting txs
+        let mut conflicting_txs = vec![];
+
+        if frozen_token_id.is_some() || confirmed_account_nonce.is_some() {
             for unconfirmed in self.unconfirmed_descendants.keys() {
                 if let Some(frozen_token_id) = frozen_token_id {
                     let unconfirmed_tx = self.txs.get(unconfirmed).expect("must be present");
                     if self.uses_token(unconfirmed_tx, &frozen_token_id) {
-                        let unconfirmed_tx =
-                            self.txs.get_mut(unconfirmed).expect("must be present");
-                        if let WalletTx::Tx(ref mut tx) = unconfirmed_tx {
+                        if let WalletTx::Tx(tx) = unconfirmed_tx {
                             conflicting_txs.push(tx.get_transaction().get_id());
                         }
                     }
@@ -743,9 +741,7 @@ impl OutputCache {
                         confirmed_account,
                         confirmed_account_nonce,
                     ) {
-                        let unconfirmed_tx =
-                            self.txs.get_mut(unconfirmed).expect("must be present");
-                        if let WalletTx::Tx(ref mut tx) = unconfirmed_tx {
+                        if let WalletTx::Tx(tx) = unconfirmed_tx {
                             conflicting_txs.push(tx.get_transaction().get_id());
                         }
                     }
@@ -755,11 +751,16 @@ impl OutputCache {
 
         // Remove all descendants of conflicting txs
         let mut conflicting_txs_with_descendants = vec![];
+
         for conflicting_tx in conflicting_txs {
-            let descendants =
-                self.remove_descendants_and_mark_as(conflicting_tx, TxState::Conflicted(block_id))?;
+            if conflicting_tx != confirmed_tx.get_id() {
+                let descendants = self.remove_descendants_and_mark_as(
+                    conflicting_tx,
+                    TxState::Conflicted(block_id),
+                )?;
 
-            conflicting_txs_with_descendants.extend(descendants.into_iter());
+                conflicting_txs_with_descendants.extend(descendants.into_iter());
+            }
         }
 
         Ok(conflicting_txs_with_descendants)
@@ -828,8 +829,11 @@ impl OutputCache {
             | TxState::Abandoned => true,
             TxState::Confirmed(_, _, _) => false,
         };
+
         if is_unconfirmed && !already_present {
             self.unconfirmed_descendants.insert(tx_id.clone(), BTreeSet::new());
+        } else {
+            self.unconfirmed_descendants.remove(&tx_id);
         }
 
         self.update_inputs(&tx, is_unconfirmed, &tx_id, already_present)?;
@@ -944,13 +948,14 @@ impl OutputCache {
             match input {
                 TxInput::Utxo(outpoint) => {
                     self.consumed.insert(outpoint.clone(), tx.state());
-                    if is_unconfirmed {
-                        self.unconfirmed_descendants
-                            .get_mut(&outpoint.source_id())
-                            .as_mut()
-                            .map(|descendants| descendants.insert(tx_id.clone()));
-                    } else {
-                        self.unconfirmed_descendants.remove(tx_id);
+                    if let Some(descendants) =
+                        self.unconfirmed_descendants.get_mut(&outpoint.source_id())
+                    {
+                        if is_unconfirmed {
+                            descendants.insert(tx_id.clone());
+                        } else {
+                            descendants.remove(tx_id);
+                        }
                     }
                 }
                 TxInput::Account(outpoint) => match outpoint.account() {
@@ -1753,8 +1758,8 @@ fn uses_conflicting_nonce(
     confirmed_account_type: AccountType,
     confirmed_nonce: AccountNonce,
 ) -> bool {
-    unconfirmed_tx.inputs().iter().all(|inp| match inp {
-        TxInput::Utxo(_) => true,
+    unconfirmed_tx.inputs().iter().any(|inp| match inp {
+        TxInput::Utxo(_) => false,
         TxInput::AccountCommand(nonce, cmd) => {
             confirmed_account_type == cmd.into() && *nonce <= confirmed_nonce
         }