Addition of new RPC command for manual cpfp.

Along with tests.

Author: Zshan0

doc/API.md

@@ -26,6 +26,7 @@ Note that all addresses are bech32-encoded *version 0* native Segwit `scriptPubK
 | [`setspendtx`](#setspendtx)                                 | Announce and broadcast this Spend transaction        |
 | [`gethistory`](#gethistory)                                 | Retrieve history of funds                            |
 | [`emergency`](#emergency)                                   | Broadcast all Emergency signed transactions          |
+| [`cpfp`](#cpfp)                                             | Manually trigger the cpfp for transactions.          |
 
 
 
@@ -483,6 +484,20 @@ of inflows and outflows net of any change amount (that is technically a transact
 None; the `result` field will be set to the empty object `{}`. Any value should be
 disregarded for forward compatibility.
 
+### `cpfp`
+
+#### Request
+
+| Field          | Type   | Description                                    |
+| -------------- | ------ | ---------------------------------------------- |
+| `txids`        | array  | Array of Txids that must be CPFPed             |
+| `feerate`      | float  | The new target feerate.                        |
+
+#### Response
+
+None; the `result` field will be set to the empty object `{}`. Any value should be
+disregarded for forward compatibility.
+
 
 ## User flows
 

src/bitcoind/mod.rs

@@ -3,10 +3,19 @@ pub mod poller;
 pub mod utils;
 
 use crate::config::BitcoindConfig;
-use crate::{database::DatabaseError, revaultd::RevaultD, threadmessages::BitcoindMessageOut};
+use crate::{
+    commands::CommandError,
+    database::{
+        interface::{db_spend_transaction, db_unvault_transaction_by_txid},
+        DatabaseError,
+    },
+    revaultd::RevaultD,
+    threadmessages::BitcoindMessageOut,
+};
 use interface::{BitcoinD, WalletTransaction};
-use poller::poller_main;
+use poller::{poller_main, should_cpfp, ToBeCpfped};
 use revault_tx::bitcoin::{Network, Txid};
+use utils::cpfp_package;
 
 use std::{
     sync::{
@@ -187,6 +196,61 @@ fn wallet_transaction(bitcoind: &BitcoinD, txid: Txid) -> Option<WalletTransacti
         .ok()
 }
 
+fn cpfp(
+    revaultd: Arc<RwLock<RevaultD>>,
+    bitcoind: Arc<RwLock<BitcoinD>>,
+    txids: Vec<Txid>,
+    feerate: f64,
+) -> Result<Vec<Txid>, CommandError> {
+    let db_path = revaultd.read().unwrap().db_file();
+    assert!(revaultd.read().unwrap().is_manager());
+
+    let mut cpfp_txs = Vec::with_capacity(txids.len());
+    let mut cpfp_txids = Vec::with_capacity(txids.len());
+
+    // sats/vbyte -> sats/WU
+    let sats_wu = feerate / 4.0;
+
+    // sats/WU -> sats/kWU
+    let sats_kwu = (sats_wu * 1000.0) as u64;
+
+    for txid in txids.iter() {
+        let spend_tx = db_spend_transaction(&db_path, &txid).expect("Database must be available");
+
+        if let Some(unwrap_spend_tx) = spend_tx {
+            // If the transaction is of type SpendTransaction
+            let psbt = unwrap_spend_tx.psbt;
+            if should_cpfp(&bitcoind.read().unwrap(), &psbt, sats_kwu) {
+                cpfp_txs.push(ToBeCpfped::Spend(psbt));
+                cpfp_txids.push(txid.clone());
+            }
+        } else {
+            let unvault_tx = match db_unvault_transaction_by_txid(&db_path, &txid)
+                .expect("Database must be available")
+            {
+                Some(tx) => tx,
+                None => return Err(CommandError::InvalidParams("Unknown Txid.".to_string())),
+            };
+            // The transaction type is asserted to be UnvaultTransaction
+            let psbt = unvault_tx.psbt.assert_unvault();
+            if should_cpfp(&bitcoind.read().unwrap(), &psbt, sats_kwu) {
+                cpfp_txs.push(ToBeCpfped::Unvault(psbt));
+                cpfp_txids.push(txid.clone());
+            }
+        }
+    }
+
+    if cpfp_txids.len() > 0 {
+        match cpfp_package(&revaultd, &bitcoind.read().unwrap(), cpfp_txs, sats_kwu) {
+            Err(err) => return Err(CommandError::Bitcoind(err)),
+            Ok(txids) => return Ok(txids),
+        }
+    } else {
+        log::info!("Nothing to CPFP in the given list.");
+    }
+    Ok(cpfp_txids)
+}
+
 /// The bitcoind event loop.
 /// Listens for bitcoind requests (wallet / chain) and poll bitcoind every 30 seconds,
 /// updating our state accordingly.
@@ -208,7 +272,8 @@ pub fn bitcoind_main_loop(
         let _bitcoind = bitcoind.clone();
         let _sync_progress = sync_progress.clone();
         let _shutdown = shutdown.clone();
-        move || poller_main(revaultd, _bitcoind, _sync_progress, _shutdown)
+        let _revaultd = revaultd.clone();
+        move || poller_main(_revaultd, _bitcoind, _sync_progress, _shutdown)
     });
 
     for msg in rx {
@@ -252,6 +317,23 @@ pub fn bitcoind_main_loop(
                         ))
                     })?;
             }
+            BitcoindMessageOut::CPFPTransaction(txids, feerate, resp_tx) => {
+                log::trace!("Received 'cpfptransaction' from main thread");
+
+                let _bitcoind = bitcoind.clone();
+                let _revaultd = revaultd.clone();
+                resp_tx
+                    .send(
+                        cpfp(_revaultd, _bitcoind, txids, feerate)
+                            .map(|_v| {})
+                            .map_err(|e| {
+                                BitcoindError::Custom(format!("Error CPFPing transactions: {}", e))
+                            }),
+                    )
+                    .map_err(|e| {
+                        BitcoindError::Custom(format!("Sending transaction for CPFP: {}", e))
+                    })?;
+            }
         }
     }
 

src/bitcoind/poller.rs

@@ -3,7 +3,7 @@ use crate::{
     bitcoind::{
         interface::{BitcoinD, DepositsState, SyncInfo, UnvaultsState, UtxoInfo},
         utils::{
-            cancel_txids, emer_txid, populate_deposit_cache, populate_unvaults_cache,
+            cancel_txids, cpfp_package, emer_txid, populate_deposit_cache, populate_unvaults_cache,
             presigned_transactions, unemer_txid, unvault_txid, unvault_txin_from_deposit,
             vault_deposit_utxo,
         },
@@ -31,20 +31,16 @@ use crate::{
     revaultd::{BlockchainTip, RevaultD, VaultStatus},
 };
 use revault_tx::{
-    bitcoin::{consensus::encode, secp256k1, Amount, OutPoint, Txid},
-    error::TransactionCreationError,
+    bitcoin::{secp256k1, Amount, OutPoint, Txid},
     miniscript::descriptor::{DescriptorSecretKey, DescriptorXKey, KeyMap, Wildcard},
     scripts::CpfpDescriptor,
-    transactions::{
-        CpfpTransaction, CpfpableTransaction, RevaultTransaction, SpendTransaction,
-        UnvaultTransaction,
-    },
+    transactions::{CpfpableTransaction, RevaultTransaction, SpendTransaction, UnvaultTransaction},
     txins::{CpfpTxIn, RevaultTxIn},
-    txouts::{CpfpTxOut, RevaultTxOut},
+    txouts::RevaultTxOut,
 };
 
 use std::{
-    collections::{HashMap, HashSet},
+    collections::HashMap,
     path::{Path, PathBuf},
     sync::{
         atomic::{AtomicBool, Ordering},
@@ -408,7 +404,7 @@ fn mark_confirmed_emers(
     Ok(())
 }
 
-enum ToBeCpfped {
+pub enum ToBeCpfped {
     Spend(SpendTransaction),
     Unvault(UnvaultTransaction),
 }
@@ -447,101 +443,12 @@ impl ToBeCpfped {
     }
 }
 
-// CPFP a bunch of transactions, bumping their feerate by at least `target_feerate`.
-// `target_feerate` is expressed in sat/kWU.
-// All the transactions' feerate MUST be below `target_feerate`.
-fn cpfp_package(
-    revaultd: &Arc<RwLock<RevaultD>>,
+/// `target_feerate` is in sats/kWU
+pub fn should_cpfp(
     bitcoind: &BitcoinD,
-    to_be_cpfped: Vec<ToBeCpfped>,
+    tx: &impl CpfpableTransaction,
     target_feerate: u64,
-) -> Result<(), BitcoindError> {
-    let revaultd = revaultd.read().unwrap();
-    let cpfp_descriptor = &revaultd.cpfp_descriptor;
-
-    // First of all, compute all the information we need from the to-be-cpfped transactions.
-    let mut txids = HashSet::with_capacity(to_be_cpfped.len());
-    let mut package_weight = 0;
-    let mut package_fees = Amount::from_sat(0);
-    let mut txins = Vec::with_capacity(to_be_cpfped.len());
-    for tx in to_be_cpfped.iter() {
-        txids.insert(tx.txid());
-        package_weight += tx.max_weight();
-        package_fees += tx.fees();
-        assert!(((package_fees.as_sat() * 1000 / package_weight) as u64) < target_feerate);
-        match tx.cpfp_txin(cpfp_descriptor, &revaultd.secp_ctx) {
-            Some(txin) => txins.push(txin),
-            None => {
-                log::error!("No CPFP txin for tx '{}'", tx.txid());
-                return Ok(());
-            }
-        }
-    }
-    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;
-
-    // Then construct the child PSBT
-    let confirmed_cpfp_utxos: Vec<_> = bitcoind
-        .list_unspent_cpfp()?
-        .into_iter()
-        .filter_map(|l| {
-            // Not considering our own outputs nor UTXOs still in mempool
-            if txids.contains(&l.outpoint.txid) || l.confirmations < 1 {
-                None
-            } else {
-                let txout = CpfpTxOut::new(
-                    Amount::from_sat(l.txo.value),
-                    &revaultd.derived_cpfp_descriptor(l.derivation_index.expect("Must be here")),
-                );
-                Some(CpfpTxIn::new(l.outpoint, txout))
-            }
-        })
-        .collect();
-    let psbt = match CpfpTransaction::from_txins(
-        txins,
-        package_weight,
-        package_fees,
-        added_feerate,
-        confirmed_cpfp_utxos,
-    ) {
-        Ok(tx) => tx,
-        Err(TransactionCreationError::InsufficientFunds) => {
-            // Well, we're poor.
-            log::error!(
-                "We wanted to feebump transactions '{:?}', but we don't have enough funds!",
-                txids
-            );
-            return Ok(());
-        }
-        Err(e) => {
-            log::error!("Error while creating CPFP transaction: '{}'", e);
-            return Ok(());
-        }
-    };
-
-    // Finally, sign and (try to) broadcast the CPFP transaction
-    let (complete, psbt_signed) = bitcoind.sign_psbt(psbt.psbt())?;
-    if !complete {
-        log::error!(
-            "Bitcoind returned a non-finalized CPFP PSBT: {}",
-            base64::encode(encode::serialize(&psbt_signed))
-        );
-        return Ok(());
-    }
-
-    let final_tx = psbt_signed.extract_tx();
-    if let Err(e) = bitcoind.broadcast_transaction(&final_tx) {
-        log::error!("Error broadcasting '{:?}' CPFP tx: {}", txids, e);
-    } else {
-        log::info!("CPFPed transactions with ids '{:?}'", txids);
-    }
-
-    Ok(())
-}
-
-// `target_feerate` is in sats/kWU
-fn should_cpfp(bitcoind: &BitcoinD, tx: &impl CpfpableTransaction, target_feerate: u64) -> bool {
+) -> bool {
     bitcoind
         .get_wallet_transaction(&tx.txid())
         // In the unlikely (actually, shouldn't happen but hey) case where
@@ -599,7 +506,14 @@ fn maybe_cpfp_txs(
     // TODO: std transaction max size check and split
     // TODO: smarter RBF (especially opportunistically with the fee delta)
     if !to_cpfp.is_empty() {
-        cpfp_package(revaultd, bitcoind, to_cpfp, current_feerate)?;
+        match cpfp_package(revaultd, bitcoind, to_cpfp, current_feerate) {
+            Err(e) => {
+                log::error!("Error broadcasting CPFP: {}", e);
+            }
+            Ok(txids) => {
+                log::info!("CPFPed transactions with ids '{:?}'", txids);
+            }
+        }
     } else {
         log::debug!("Nothing to CPFP");
     }