fix(ckbtc): unstuck ckbtc withdrawal requests

rs/bitcoin/ckbtc/minter/canbench/results.yml

@@ -2,94 +2,94 @@ benches:
   build_estimate_retrieve_btc_fee_1_50k_sats:
     total:
       calls: 1
-      instructions: 419243
+      instructions: 419639
       heap_increase: 0
       stable_memory_increase: 0
     scopes:
       build_unsigned_transaction_from_inputs:
         calls: 1
-        instructions: 3413
+        instructions: 3441
         heap_increase: 0
         stable_memory_increase: 0
       greedy:
         calls: 1
-        instructions: 5029
+        instructions: 5043
         heap_increase: 0
         stable_memory_increase: 0
       utxos_selection:
         calls: 1
-        instructions: 413053
+        instructions: 413500
         heap_increase: 0
         stable_memory_increase: 0
   build_unsigned_transaction_1_50k_sats:
     total:
       calls: 1
-      instructions: 421431
+      instructions: 421753
       heap_increase: 0
       stable_memory_increase: 0
     scopes:
       build_unsigned_transaction:
         calls: 1
-        instructions: 418774
+        instructions: 419172
         heap_increase: 0
         stable_memory_increase: 0
       build_unsigned_transaction_from_inputs:
         calls: 1
-        instructions: 3413
+        instructions: 3441
         heap_increase: 0
         stable_memory_increase: 0
       greedy:
         calls: 1
-        instructions: 5029
+        instructions: 5043
         heap_increase: 0
         stable_memory_increase: 0
       utxos_selection:
         calls: 1
-        instructions: 413053
+        instructions: 413500
         heap_increase: 0
         stable_memory_increase: 0
   build_unsigned_transaction_2_100k_sats:
     total:
       calls: 1
-      instructions: 421480
+      instructions: 421799
       heap_increase: 0
       stable_memory_increase: 0
     scopes:
       build_unsigned_transaction:
         calls: 1
-        instructions: 418823
+        instructions: 419218
         heap_increase: 0
         stable_memory_increase: 0
       build_unsigned_transaction_from_inputs:
         calls: 1
-        instructions: 3413
+        instructions: 3441
         heap_increase: 0
         stable_memory_increase: 0
       greedy:
         calls: 1
-        instructions: 5097
+        instructions: 5089
         heap_increase: 0
         stable_memory_increase: 0
       utxos_selection:
         calls: 1
-        instructions: 413102
+        instructions: 413546
         heap_increase: 0
         stable_memory_increase: 0
   build_unsigned_transaction_3_1m_sats:
     total:
       calls: 1
-      instructions: 421375
+      instructions: 421702
       heap_increase: 0
       stable_memory_increase: 0
     scopes:
       build_unsigned_transaction:
         calls: 1
-        instructions: 418718
+        instructions: 419121
         heap_increase: 0
         stable_memory_increase: 0
       build_unsigned_transaction_from_inputs:
         calls: 1
-        instructions: 3413
+        instructions: 3441
         heap_increase: 0
         stable_memory_increase: 0
       greedy:
@@ -99,88 +99,88 @@ benches:
         stable_memory_increase: 0
       utxos_selection:
         calls: 1
-        instructions: 412997
+        instructions: 413449
         heap_increase: 0
         stable_memory_increase: 0
   build_unsigned_transaction_4_10m_sats:
     total:
       calls: 1
-      instructions: 423622
+      instructions: 422877
       heap_increase: 0
       stable_memory_increase: 0
     scopes:
       build_unsigned_transaction:
         calls: 1
-        instructions: 420965
+        instructions: 420296
         heap_increase: 0
         stable_memory_increase: 0
       build_unsigned_transaction_from_inputs:
         calls: 1
-        instructions: 3413
+        instructions: 3441
         heap_increase: 0
         stable_memory_increase: 0
       greedy:
         calls: 1
-        instructions: 5894
+        instructions: 4791
         heap_increase: 0
         stable_memory_increase: 0
       utxos_selection:
         calls: 1
-        instructions: 415244
+        instructions: 414624
         heap_increase: 0
         stable_memory_increase: 0
   build_unsigned_transaction_5_1_btc:
     total:
       calls: 1
-      instructions: 423622
+      instructions: 422877
       heap_increase: 0
       stable_memory_increase: 0
     scopes:
       build_unsigned_transaction:
         calls: 1
-        instructions: 420965
+        instructions: 420296
         heap_increase: 0
         stable_memory_increase: 0
       build_unsigned_transaction_from_inputs:
         calls: 1
-        instructions: 3413
+        instructions: 3441
         heap_increase: 0
         stable_memory_increase: 0
       greedy:
         calls: 1
-        instructions: 5894
+        instructions: 4791
         heap_increase: 0
         stable_memory_increase: 0
       utxos_selection:
         calls: 1
-        instructions: 415244
+        instructions: 414624
         heap_increase: 0
         stable_memory_increase: 0
   build_unsigned_transaction_6_10_btc:
     total:
       calls: 1
-      instructions: 430570
+      instructions: 429549
       heap_increase: 0
       stable_memory_increase: 0
     scopes:
       build_unsigned_transaction:
         calls: 1
-        instructions: 427912
+        instructions: 426968
         heap_increase: 0
         stable_memory_increase: 0
       build_unsigned_transaction_from_inputs:
         calls: 1
-        instructions: 3829
+        instructions: 3908
         heap_increase: 0
         stable_memory_increase: 0
       greedy:
         calls: 1
-        instructions: 15410
+        instructions: 14050
         heap_increase: 0
         stable_memory_increase: 0
       utxos_selection:
         calls: 1
-        instructions: 421703
+        instructions: 420829
         heap_increase: 0
         stable_memory_increase: 0
 version: 0.4.1

rs/bitcoin/ckbtc/minter/src/state.rs

@@ -582,6 +582,9 @@ pub struct CkBtcMinterState {
 
     /// Block tip height returned in the last get_utxos call.
     pub last_get_utxos_tip_height: Option<Height>,
+
+    /// All OutPoints ever minted.
+    pub minted_outpoints: BTreeSet<OutPoint>,
 }
 
 #[derive(Clone, Eq, PartialEq, Debug, CandidType, Serialize, serde::Deserialize)]
@@ -773,6 +776,7 @@ impl CkBtcMinterState {
         let account_bucket = self.utxos_state_addresses.entry(account).or_default();
 
         for utxo in utxos {
+            self.minted_outpoints.insert(utxo.outpoint.clone());
             self.outpoint_account.insert(utxo.outpoint.clone(), account);
             self.available_utxos.insert(utxo.clone());
             self.checked_utxos.remove(&utxo);
@@ -2065,6 +2069,7 @@ impl From<InitArgs> for CkBtcMinterState {
             last_get_utxos_tip_height: None,
             pending_withdrawal_reimbursements: Default::default(),
             reimbursed_withdrawals: Default::default(),
+            minted_outpoints: Default::default(),
         }
     }
 }

rs/bitcoin/ckbtc/minter/src/state/eventlog.rs

@@ -353,6 +353,9 @@ impl EventLogger for CkBtcEventLogger {
             None => return Err(ReplayLogError::EmptyLog),
         };
 
+        let mut dup_received = BTreeSet::new();
+        let mut rejected_sent = BTreeSet::new();
+
         // Because `kyt_principal` was previously used as a default
         // substitute for `kyt_provider` during kyt_fee accounting,
         // we need to keep track of this value so that `distribute_kyt_fee`
@@ -374,7 +377,21 @@ impl EventLogger for CkBtcEventLogger {
                 }
                 EventType::ReceivedUtxos {
                     to_account, utxos, ..
-                } => state.add_utxos::<I>(to_account, utxos),
+                } => {
+                    let utxos = utxos
+                        .into_iter()
+                        .filter(|utxo| {
+                            if state.minted_outpoints.contains(&utxo.outpoint) {
+                                state.checked_utxos.remove(utxo);
+                                dup_received.insert(utxo.clone());
+                                false
+                            } else {
+                                true
+                            }
+                        })
+                        .collect::<Vec<_>>();
+                    state.add_utxos::<I>(to_account, utxos);
+                }
                 EventType::AcceptedRetrieveBtcRequest(req) => {
                     if let Some(account) = req.reimbursement_account {
                         state
@@ -409,6 +426,11 @@ impl EventLogger for CkBtcEventLogger {
                     withdrawal_fee,
                     signed_tx,
                 } => {
+                    // Ignore this event if the utxos contain a dup.
+                    if utxos.iter().any(|utxo| dup_received.contains(utxo)) {
+                        rejected_sent.insert(txid);
+                        continue;
+                    }
                     let mut retrieve_btc_requests = BTreeSet::new();
                     let mut consolidate_utxos_request = None;
                     for block_index in request_block_indices {
@@ -460,6 +482,11 @@ impl EventLogger for CkBtcEventLogger {
                     reason,
                     new_utxos,
                 } => {
+                    // Ignore if old_txid was already rejected
+                    if rejected_sent.contains(&old_txid) {
+                        rejected_sent.insert(new_txid);
+                        continue;
+                    }
                     let (old_requests, old_utxos) = match state
                         .submitted_transactions
                         .iter()

rs/bitcoin/ckbtc/minter/src/storage.rs

@@ -284,7 +284,7 @@ mod benches {
             // NOTE: Those benchmarks reflect the performance of the minter on **mainnet**.
             // Changing the number of available of UTXOs is unavoidable when updating the retrieved mainnet events used for testing,
             // so that fluctuations in performance is acceptable, but large degradation would indicate a regression.
-            assert_eq!(s.available_utxos.len(), 7_514);
+            assert_eq!(s.available_utxos.len(), 7_519);
         });
 
         let dummy_minter_address = crate::BitcoinAddress::P2wpkhV0([u8::MAX; 20]);
@@ -316,7 +316,7 @@ mod benches {
             // NOTE: Those benchmarks reflect the performance of the minter on **mainnet**.
             // Changing the number of available of UTXOs is unavoidable when updating the retrieved mainnet events used for testing,
             // so that fluctuations in performance is acceptable, but large degradation would indicate a regression.
-            assert_eq!(s.available_utxos.len(), 7_514);
+            assert_eq!(s.available_utxos.len(), 7_519);
         });
         let fee_estimator = state::read_state(|s| IC_CANISTER_RUNTIME.fee_estimator(s));
 

rs/bitcoin/ckbtc/minter/tests/replay_events.rs

@@ -7,7 +7,7 @@
 
 use candid::{CandidType, Deserialize, Principal};
 use ic_agent::Agent;
-use ic_btc_interface::OutPoint;
+use ic_btc_interface::{OutPoint, Txid};
 use ic_ckbtc_minter::state::eventlog::{
     CkBtcEventLogger, CkBtcMinterEvent, EventLogger, EventType,
 };
@@ -96,6 +96,60 @@ static MAINNET_STATE: LazyLock<CkBtcMinterState> = LazyLock::new(|| {
 });
 static TESTNET_EVENTS: LazyLock<GetEventsResult> = LazyLock::new(|| Testnet.deserialize());
 
+#[test]
+fn should_replay_events_and_retain_pending_requests() {
+    use std::str::FromStr;
+
+    let state = &MAINNET_STATE;
+    state
+        .check_invariants()
+        .expect("Failed to check invariants");
+
+    println!(
+        "pending retrieve_btc_request = {:?}",
+        state.pending_retrieve_btc_requests
+    );
+
+    let block_indices = state
+        .pending_retrieve_btc_requests
+        .iter()
+        .map(|request| request.block_index)
+        .collect::<BTreeSet<_>>();
+    // 1st stuck retrieve_btc transits to pending
+    assert!(block_indices.contains(&3459007));
+    assert!(block_indices.contains(&3459009));
+    assert!(block_indices.contains(&3459013));
+    // 2st stuck retrieve_btc transits to pending
+    assert!(block_indices.contains(&3489347));
+    assert!(block_indices.contains(&3489353));
+    // The following transactions and resubmissions should not be found
+    let txids = vec![
+        "fad3348b5e121d07bcd4afc523a1a506edf0e232ad3a6a6fdb214c04719a05fc",
+        "f69e339597b98a3286f586785c33b320f38ff4d2921f07dafecd12de881b769d",
+        "d4bcf28392c327795a4cd7f85ab935c348aa980313daba8b40c71ecc1fc4d0a4",
+        "0282fffcd9cd59352a7e6670219949d5493b95068df5ffe399e1648fa51db83c",
+        "9733ae015a766051f51ac12284c3f821ec60ec0d44ffa14bbcc54ef4f5e575da",
+        "36e9125b299428f18e957dd8ffbc2ecb8e125469f77a11e1dbb8245a2a8ed5a9",
+        "1fd0293a0260c844ef1e5822dbc7b9fce3e934f49bcacd3feea6650c96386476",
+    ];
+    let txids = txids
+        .into_iter()
+        .map(|txid| Txid::from_str(txid).unwrap())
+        .collect::<BTreeSet<_>>();
+    let submitted = state
+        .submitted_transactions
+        .iter()
+        .map(|tx| tx.txid)
+        .collect::<BTreeSet<_>>();
+    let stuck = state
+        .stuck_transactions
+        .iter()
+        .map(|tx| tx.txid)
+        .collect::<BTreeSet<_>>();
+    assert!(txids.is_disjoint(&submitted));
+    assert!(txids.is_disjoint(&stuck));
+}
+
 #[tokio::test]
 async fn should_replay_events_for_mainnet() {
     Mainnet.retrieve_and_store_events_if_env().await;
@@ -106,7 +160,7 @@ async fn should_replay_events_for_mainnet() {
         .expect("Failed to check invariants");
 
     assert_eq!(state.btc_network, Network::Mainnet);
-    assert_eq!(state.get_total_btc_managed(), 28_544_468_650);
+    assert_eq!(state.get_total_btc_managed(), 28_608_213_637);
 }
 
 #[tokio::test]