From 80e001d4d04ee19107d2383c86890489df39a9a0 Mon Sep 17 00:00:00 2001
From: Antoine Poinsot <darosior@protonmail.com>
Date: Mon, 13 Dec 2021 13:44:07 +0100
Subject: [PATCH] CPFP related code cleanups

---
 src/daemon/bitcoind/poller.rs    | 28 ++++++++++++++--------------
 src/daemon/database/interface.rs | 18 +++++-------------
 2 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/src/daemon/bitcoind/poller.rs b/src/daemon/bitcoind/poller.rs
index 73a3e4bb..cecc7c0e 100644
--- a/src/daemon/bitcoind/poller.rs
+++ b/src/daemon/bitcoind/poller.rs
@@ -446,6 +446,7 @@ fn cpfp_package(
 ) -> Result<(), BitcoindError> {
     let revaultd = revaultd.read().unwrap();
 
+    // First of all, compute all the information we need from the to-be-cpfped transactions.
     let mut txids = HashSet::with_capacity(tx_package.len());
     let mut package_weight = 0;
     let mut package_fees = Amount::from_sat(0);
@@ -457,15 +458,16 @@ fn cpfp_package(
     }
     let tx_feerate = (package_fees.as_sat() * 1_000 / package_weight) as u64; // to sats/kWU
     assert!(tx_feerate < target_feerate);
-
     let added_feerate = target_feerate - tx_feerate;
-    let listunspent: Vec<_> = bitcoind.list_unspent_cpfp()?;
 
+    // FIXME: it's a shame to have to get the derivation paths from bitcoind, but we need to
+    // for the Spend CPFP output derivation index. We have all the info and should be able to
+    // make it work without this hack.
+    let listunspent: Vec<_> = bitcoind.list_unspent_cpfp()?;
     // FIXME: drain_filter would be PERFECT for this but it's nightly only :(
     let (mut my_listunspent, listunspent): (Vec<_>, Vec<_>) = listunspent
         .into_iter()
         .partition(|l| txids.contains(&l.outpoint.txid));
-
     if my_listunspent.len() != tx_package.len() {
         log::warn!(
             "We need to feebump a package containing the following txids: {:?},\n
@@ -478,7 +480,6 @@ fn cpfp_package(
         );
         return Ok(());
     }
-
     my_listunspent.sort_by_key(|l| l.outpoint.txid);
     tx_package.sort_by_key(|tx| tx.txid());
     // I can do this as I just ordered by txid
@@ -495,6 +496,7 @@ fn cpfp_package(
         }
     }
 
+    // Then construct the child PSBT
     let confirmed_cpfp_utxos: Vec<_> = listunspent
         .into_iter()
         .filter_map(|l| {
@@ -532,6 +534,7 @@ fn cpfp_package(
         }
     };
 
+    // Finally, sign and (try to) broadcast the CPFP transaction
     let (complete, psbt_signed) = bitcoind.sign_psbt(psbt.psbt())?;
     if !complete {
         log::error!(
@@ -597,16 +600,13 @@ fn maybe_cpfp_txs(
         .chain(
             db_cpfpable_unvaults(&db_path)?
                 .into_iter()
-                .map(|unvaults| {
-                    unvaults.into_iter().filter_map(|unvault| {
-                        if should_cpfp(bitcoind, &unvault, current_feerate) {
-                            Some(ToBeCpfped::Unvault(unvault))
-                        } else {
-                            None
-                        }
-                    })
-                })
-                .flatten(),
+                .filter_map(|unvault| {
+                    if should_cpfp(bitcoind, &unvault, current_feerate) {
+                        Some(ToBeCpfped::Unvault(unvault))
+                    } else {
+                        None
+                    }
+                }),
         )
         .collect();
 
diff --git a/src/daemon/database/interface.rs b/src/daemon/database/interface.rs
index ca7eed8e..00f7170b 100644
--- a/src/daemon/database/interface.rs
+++ b/src/daemon/database/interface.rs
@@ -879,8 +879,7 @@ pub fn db_cpfpable_spends(db_path: &Path) -> Result<Vec<SpendTransaction>, Datab
 
 /// Returns all the unvaults that have priority and for which their spend has not
 /// been broadcasted, which are eligible for CPFP if still unconfirmed
-pub fn db_cpfpable_unvaults(db_path: &Path) -> Result<Vec<Vec<UnvaultTransaction>>, DatabaseError> {
-    let mut unvaults: HashMap<i64, Vec<UnvaultTransaction>> = HashMap::new();
+pub fn db_cpfpable_unvaults(db_path: &Path) -> Result<Vec<UnvaultTransaction>, DatabaseError> {
     db_query(
         db_path,
         "SELECT ptx.*, stx.id FROM spend_transactions stx \
@@ -891,17 +890,10 @@ pub fn db_cpfpable_unvaults(db_path: &Path) -> Result<Vec<Vec<UnvaultTransaction
         params![TransactionType::Unvault as u32,],
         |row| {
             let tx: DbTransaction = row.try_into()?;
-            let unvault_tx = match tx.psbt {
-                RevaultTx::Unvault(tx) => tx,
+            match tx.psbt {
+                RevaultTx::Unvault(tx) => Ok(tx),
                 _ => unreachable!(),
-            };
-            let spend_id = row.get::<_, i64>(6)?;
-            unvaults
-                .entry(spend_id)
-                .or_insert(Vec::new())
-                .push(unvault_tx);
-            Ok(())
+            }
         },
-    )?;
-    Ok(unvaults.values().cloned().collect())
+    )
 }