Fix comments

Author: azarovh

wallet/src/account/output_cache/mod.rs

@@ -698,6 +698,7 @@ impl OutputCache {
             match input {
                 TxInput::Utxo(_) => {
                     //TODO: check conflicting utxo spends
+                    // See https://github.com/mintlayer/mintlayer-core/issues/1875
                     None
                 }
                 TxInput::Account(outpoint) => Some(Conflict {
@@ -731,26 +732,24 @@ impl OutputCache {
             for unconfirmed in self.unconfirmed_descendants.keys() {
                 let unconfirmed_tx = self.txs.get(unconfirmed).expect("must be present");
 
-                if let Some(frozen_token_id) = conflict.frozen_token_id {
-                    if self.uses_token(unconfirmed_tx, &frozen_token_id) {
-                        if let WalletTx::Tx(tx) = unconfirmed_tx {
+                if let WalletTx::Tx(tx) = unconfirmed_tx {
+                    if let Some(frozen_token_id) = conflict.frozen_token_id {
+                        if self.uses_token(unconfirmed_tx, &frozen_token_id) {
                             conflicting_txs.insert(tx.get_transaction().get_id());
                         }
                     }
-                }
 
-                if let Some((confirmed_account, confirmed_account_nonce)) =
-                    conflict.confirmed_account_nonce
-                {
-                    if uses_conflicting_nonce(
-                        unconfirmed_tx,
-                        confirmed_account,
-                        confirmed_account_nonce,
-                    ) {
-                        if let WalletTx::Tx(tx) = unconfirmed_tx {
-                            if confirmed_tx.get_id() != tx.get_transaction().get_id() {
-                                conflicting_txs.insert(tx.get_transaction().get_id());
-                            }
+                    if let Some((confirmed_account, confirmed_account_nonce)) =
+                        conflict.confirmed_account_nonce
+                    {
+                        if confirmed_tx.get_id() != tx.get_transaction().get_id()
+                            && uses_conflicting_nonce(
+                                unconfirmed_tx,
+                                confirmed_account,
+                                confirmed_account_nonce,
+                            )
+                        {
+                            conflicting_txs.insert(tx.get_transaction().get_id());
                         }
                     }
                 }
@@ -761,15 +760,17 @@ impl OutputCache {
         let mut conflicting_txs_with_descendants = vec![];
 
         for conflicting_tx in conflicting_txs {
-            let descendants = self.remove_descendants(conflicting_tx);
+            let txs_to_rollback = self.remove_from_unconfirmed_descendants(conflicting_tx);
 
             // Mark conflicting tx and its descendants as Conflicting and update OutputCache data accordingly
-            for tx_id in descendants.iter().rev().copied() {
+            for tx_id in txs_to_rollback.iter().rev().copied() {
                 match self.txs.entry(tx_id.into()) {
                     Entry::Occupied(mut entry) => match entry.get_mut() {
-                        WalletTx::Block(_) => Err(WalletError::CannotFindTransactionWithId(tx_id)),
+                        WalletTx::Block(_) => {
+                            Err(WalletError::TransactionIdCannotMapToBlock(tx_id))
+                        }
                         WalletTx::Tx(tx) => match tx.state() {
-                            TxState::Inactive(_) => {
+                            TxState::Inactive(_) | TxState::InMempool(_) => {
                                 tx.set_state(TxState::Conflicted(block_id));
                                 OutputCache::rollback_tx_data(
                                     tx,
@@ -783,20 +784,18 @@ impl OutputCache {
                             }
                             TxState::Abandoned
                             | TxState::Confirmed(..)
-                            | TxState::InMempool(..)
                             | TxState::Conflicted(..) => {
-                                Err(WalletError::CannotChangeTransactionState(
-                                    *tx.state(),
-                                    TxState::Conflicted(block_id),
-                                ))
+                                Err(WalletError::CannotMarkTxAsConflictedIfInState(*tx.state()))
                             }
                         },
                     },
-                    Entry::Vacant(_) => Err(WalletError::CannotFindTransactionWithId(tx_id)),
+                    Entry::Vacant(_) => {
+                        Err(WalletError::CannotFindDescendantTransactionWithId(tx_id))
+                    }
                 }?;
             }
 
-            conflicting_txs_with_descendants.extend(descendants.into_iter());
+            conflicting_txs_with_descendants.extend(txs_to_rollback.into_iter());
         }
 
         Ok(conflicting_txs_with_descendants)
@@ -1441,7 +1440,12 @@ impl OutputCache {
         })
     }
 
-    fn remove_descendants(&mut self, tx_id: Id<Transaction>) -> Vec<Id<Transaction>> {
+    // Removes a tx from unconfirmed descendant.
+    // Returns provided tx and all the descendants.
+    fn remove_from_unconfirmed_descendants(
+        &mut self,
+        tx_id: Id<Transaction>,
+    ) -> Vec<Id<Transaction>> {
         let mut all_txs = Vec::new();
         let mut to_update = BTreeSet::from_iter([OutPointSourceId::from(tx_id)]);
 
@@ -1511,7 +1515,7 @@ impl OutputCache {
         &mut self,
         tx_id: Id<Transaction>,
     ) -> WalletResult<Vec<Id<Transaction>>> {
-        let all_abandoned = self.remove_descendants(tx_id);
+        let all_abandoned = self.remove_from_unconfirmed_descendants(tx_id);
 
         for tx_id in all_abandoned.iter().rev().copied() {
             match self.txs.entry(tx_id.into()) {

wallet/src/wallet/mod.rs

@@ -177,8 +177,14 @@ pub enum WalletError {
     CoinSelectionError(#[from] UtxoSelectorError),
     #[error("Cannot change a transaction's state from {0} to {1}")]
     CannotChangeTransactionState(TxState, TxState),
+    #[error("Cannot mark as conflicted transaction in {0} state")]
+    CannotMarkTxAsConflictedIfInState(TxState),
     #[error("Transaction with Id {0} not found")]
     CannotFindTransactionWithId(Id<Transaction>),
+    #[error("Transaction with Id {0} cannot map to a block")]
+    TransactionIdCannotMapToBlock(Id<Transaction>),
+    #[error("Descendant transaction with Id {0} not found")]
+    CannotFindDescendantTransactionWithId(Id<Transaction>),
     #[error("Address error: {0}")]
     AddressError(#[from] AddressError),
     #[error("Unknown pool id {0}")]

wallet/src/wallet/tests.rs

@@ -3481,17 +3481,17 @@ fn lock_then_transfer(#[case] seed: Seed) {
     scan_wallet(&mut wallet, BlockHeight::new(1), vec![block2]);
 
     // check balance
-    let balance_without_locked_transer =
+    let balance_without_locked_transfer =
         ((block1_amount * 2).unwrap() - amount_to_lock_then_transfer).unwrap();
 
     let coin_balance = get_coin_balance(&wallet);
-    assert_eq!(coin_balance, balance_without_locked_transer);
+    assert_eq!(coin_balance, balance_without_locked_transfer);
 
     // check that for block_count_lock, the amount is not included
     for idx in 0..block_count_lock {
         let coin_balance = get_coin_balance(&wallet);
         // check that the amount is still not unlocked
-        assert_eq!(coin_balance, balance_without_locked_transer);
+        assert_eq!(coin_balance, balance_without_locked_transfer);
 
         let currency_balances = wallet
             .get_balance(
@@ -3524,7 +3524,7 @@ fn lock_then_transfer(#[case] seed: Seed) {
     let coin_balance = get_coin_balance(&wallet);
     assert_eq!(
         coin_balance,
-        (balance_without_locked_transer + amount_to_lock_then_transfer).unwrap()
+        (balance_without_locked_transfer + amount_to_lock_then_transfer).unwrap()
     );
 }
 
@@ -3882,15 +3882,15 @@ fn wallet_abandone_transactions(#[case] seed: Seed) {
 
     assert_eq!(abandonable_transactions.len(), transactions.len());
 
-    let txs_to_abandone = rng.gen_range(0..total_num_transactions) as usize;
-    let (txs_to_keep, txs_to_abandone) = transactions.split_at(txs_to_abandone);
+    let txs_to_abandon = rng.gen_range(0..total_num_transactions) as usize;
+    let (txs_to_keep, txs_to_abandon) = transactions.split_at(txs_to_abandon);
 
-    assert!(!txs_to_abandone.is_empty());
+    assert!(!txs_to_abandon.is_empty());
 
-    let transaction_id = txs_to_abandone.first().unwrap().0.transaction().get_id();
+    let transaction_id = txs_to_abandon.first().unwrap().0.transaction().get_id();
     wallet.abandon_transaction(DEFAULT_ACCOUNT_INDEX, transaction_id).unwrap();
 
-    let coins_after_abandon = txs_to_abandone.first().unwrap().1;
+    let coins_after_abandon = txs_to_abandon.first().unwrap().1;
 
     let coin_balance = get_coin_balance_with_inactive(&wallet);
     assert_eq!(coin_balance, coins_after_abandon);
@@ -3900,7 +3900,7 @@ fn wallet_abandone_transactions(#[case] seed: Seed) {
     let coin_balance = get_coin_balance_with_inactive(&wallet);
     assert_eq!(coin_balance, coins_after_abandon);
 
-    // Abandone the same tx again
+    // Abandon the same tx again
     let result = wallet.abandon_transaction(DEFAULT_ACCOUNT_INDEX, transaction_id);
     assert_eq!(
         result.unwrap_err(),
@@ -6195,7 +6195,7 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
         )
     );
 
-    // Abandone conflicting txs
+    // Abandon conflicting txs
     wallet
         .abandon_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_1_id)
         .unwrap();
@@ -6455,9 +6455,9 @@ fn conflicting_delegation_account_nonce_same_tx(#[case] seed: Seed) {
     );
 }
 
-// Issue and mint some tokens
-// Create an order selling tokens for coins
-// Create 2 fill order txs and add them to a wallet as unconfirmed
+// Issue and mint some tokens.
+// Create an order selling tokens for coins.
+// Create 2 fill order txs and add them to a wallet as unconfirmed.
 // Confirm the first tx in a block and check that it is accounted in confirmed balance
 // and also that unconfirmed balance has second tx.
 #[rstest]
@@ -6589,7 +6589,7 @@ fn conflicting_order_account_nonce(#[case] seed: Seed) {
     };
 
     let spend_coins_1 = Amount::from_atoms(10);
-    let recieved_tokens_1 = spend_coins_1;
+    let received_tokens_1 = spend_coins_1;
     let fill_order_tx_1 = wallet
         .create_fill_order_tx(
             DEFAULT_ACCOUNT_INDEX,
@@ -6625,7 +6625,7 @@ fn conflicting_order_account_nonce(#[case] seed: Seed) {
     };
 
     let spend_coins_2 = Amount::from_atoms(3);
-    let recieved_tokens_2 = spend_coins_2;
+    let received_tokens_2 = spend_coins_2;
     let fill_order_tx_2 = wallet
         .create_fill_order_tx(
             DEFAULT_ACCOUNT_INDEX,
@@ -6698,7 +6698,7 @@ fn conflicting_order_account_nonce(#[case] seed: Seed) {
         UtxoState::Confirmed.into(),
         WithLocked::Any,
     );
-    assert_eq!(token_balance_confirmed, recieved_tokens_1);
+    assert_eq!(token_balance_confirmed, received_tokens_1);
 
     let token_balance_unconfirmed = get_balance_with(
         &wallet,
@@ -6708,6 +6708,6 @@ fn conflicting_order_account_nonce(#[case] seed: Seed) {
     );
     assert_eq!(
         token_balance_unconfirmed,
-        (recieved_tokens_1 + recieved_tokens_2).unwrap()
+        (received_tokens_1 + received_tokens_2).unwrap()
     );
 }