From d4c08322d84d7cf8134b655fb264d6a5ce81da96 Mon Sep 17 00:00:00 2001 From: Zshan0 Date: Fri, 27 May 2022 20:43:30 +0530 Subject: [PATCH] Addition of new RPC command for manual cpfp. Along with tests. --- doc/API.md | 15 +++++++++ src/bitcoind/mod.rs | 61 +++++++++++++++++++++++++++++++++-- src/bitcoind/poller.rs | 4 +-- src/commands/mod.rs | 17 ++++++++++ src/jsonrpc/api.rs | 25 +++++++++++++++ src/threadmessages.rs | 14 ++++++++ src/utils.rs | 3 ++ tests/servers/coordinatord | 2 +- tests/servers/cosignerd | 2 +- tests/servers/miradord | 2 +- tests/test_rpc.py | 66 ++++++++++++++++++++++++++++++++++++++ 11 files changed, 203 insertions(+), 8 deletions(-) diff --git a/doc/API.md b/doc/API.md index 096abb0b..8a5845ee 100644 --- a/doc/API.md +++ b/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 diff --git a/src/bitcoind/mod.rs b/src/bitcoind/mod.rs index 834a8b0a..ce3b6693 100644 --- a/src/bitcoind/mod.rs +++ b/src/bitcoind/mod.rs @@ -3,9 +3,16 @@ pub mod poller; pub mod utils; use crate::config::BitcoindConfig; -use crate::{database::DatabaseError, revaultd::RevaultD, threadmessages::BitcoindMessageOut}; +use crate::{ + database::{ + interface::{db_spend_transaction, db_vault_by_unvault_txid}, + DatabaseError, + }, + revaultd::RevaultD, + threadmessages::BitcoindMessageOut, +}; use interface::{BitcoinD, WalletTransaction}; -use poller::poller_main; +use poller::{cpfp_package, poller_main, ToBeCpfped}; use revault_tx::bitcoin::{Network, Txid}; use std::{ @@ -187,6 +194,43 @@ fn wallet_transaction(bitcoind: &BitcoinD, txid: Txid) -> Option>, + bitcoind: Arc>, + txids: Vec, + feerate: f64, +) -> Result<(), BitcoindError> { + let db_path = revaultd.read().unwrap().db_file(); + assert!(revaultd.read().unwrap().is_manager()); + + let mut cpfp_txs = Vec::with_capacity(txids.len()); + + 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 + cpfp_txs.push(ToBeCpfped::Spend(unwrap_spend_tx.psbt)); + } else { + // The transaction type is asserted to be UnvaultTransaction + let unvault_tx = db_vault_by_unvault_txid(&db_path, &txid) + .expect("Database must be available") + .unwrap() + .1 + .psbt + .assert_unvault(); + cpfp_txs.push(ToBeCpfped::Unvault(unvault_tx)); + } + } + + // sats/vbyte -> sats/WU + let sats_wu = feerate / 4.0; + // sats/WU -> msats/WU + let msats_wu = (sats_wu * 1000.0) as u64; + + cpfp_package(&revaultd, &bitcoind.read().unwrap(), cpfp_txs, msats_wu) +} + /// The bitcoind event loop. /// Listens for bitcoind requests (wallet / chain) and poll bitcoind every 30 seconds, /// updating our state accordingly. @@ -208,7 +252,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 +297,16 @@ pub fn bitcoind_main_loop( )) })?; } + BitcoindMessageOut::CPFPTransaction(txids, feerate, resp_tx) => { + log::trace!("Received 'cpfptransaction' from main thread"); + + resp_tx + .send(cpfp(revaultd, bitcoind, txids, feerate)) + .map_err(|e| { + BitcoindError::Custom(format!("Sending transaction for CPFP: {}", e)) + })?; + return Ok(()); + } } } diff --git a/src/bitcoind/poller.rs b/src/bitcoind/poller.rs index 977f4378..42a85eaa 100644 --- a/src/bitcoind/poller.rs +++ b/src/bitcoind/poller.rs @@ -408,7 +408,7 @@ fn mark_confirmed_emers( Ok(()) } -enum ToBeCpfped { +pub enum ToBeCpfped { Spend(SpendTransaction), Unvault(UnvaultTransaction), } @@ -450,7 +450,7 @@ 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( +pub fn cpfp_package( revaultd: &Arc>, bitcoind: &BitcoinD, to_be_cpfped: Vec, diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 97e5040f..c9835e40 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -1443,6 +1443,23 @@ impl DaemonControl { let revaultd = self.revaultd.read().unwrap(); gethistory(&revaultd, &self.bitcoind_conn, start, end, limit, kind) } + + /// Manually trigger a CPFP for the given transaction ID. + /// + /// ## Errors + /// - we don't have access to a CPFP private key + /// - the caller is not a manager + pub fn manual_cpfp(&self, txids: &Vec, feerate: f64) -> Result<(), CommandError> { + let revaultd = self.revaultd.read().unwrap(); + + if revaultd.cpfp_key.is_none() { + return Err(CommandError::MissingCpfpKey); + } + manager_only!(revaultd); + + self.bitcoind_conn.cpfp_tx(txids.to_vec(), feerate)?; + Ok(()) + } } /// Descriptors the daemon was configured with diff --git a/src/jsonrpc/api.rs b/src/jsonrpc/api.rs index 0f696cac..8d917dea 100644 --- a/src/jsonrpc/api.rs +++ b/src/jsonrpc/api.rs @@ -214,6 +214,15 @@ pub trait RpcApi { end: u32, limit: u64, ) -> jsonrpc_core::Result; + + // Manually cpfp the given transaction id. + #[rpc(meta, name = "cpfp")] + fn cpfp( + &self, + meta: Self::Metadata, + txids: Vec, + feerate: f64, + ) -> jsonrpc_core::Result; } macro_rules! parse_vault_status { @@ -301,6 +310,10 @@ impl RpcApi for RpcImpl { "emergency": [ ], + "cpfp": [ + "txids", + "feerate", + ], })) } @@ -513,4 +526,16 @@ impl RpcApi for RpcImpl { "events": events, })) } + + // manual CPFP command + // feerate will be in sat/vbyte [TODO API.md] + fn cpfp( + &self, + meta: Self::Metadata, + txids: Vec, + feerate: f64, + ) -> jsonrpc_core::Result { + meta.daemon_control.manual_cpfp(&txids, feerate)?; + Ok(json!({})) + } } diff --git a/src/threadmessages.rs b/src/threadmessages.rs index ba86bef7..49217fa6 100644 --- a/src/threadmessages.rs +++ b/src/threadmessages.rs @@ -41,6 +41,7 @@ pub enum BitcoindMessageOut { Vec, SyncSender>, ), + CPFPTransaction(Vec, f64, SyncSender>), } /// Interface to communicate with bitcoind client thread. @@ -49,6 +50,7 @@ pub trait BitcoindThread { fn broadcast(&self, transactions: Vec) -> Result<(), BitcoindError>; fn shutdown(&self); fn sync_progress(&self) -> f64; + fn cpfp_tx(&self, txids: Vec, feerate: f64) -> Result<(), BitcoindError>; } /// Interface to the bitcoind thread using synchronous MPSCs @@ -98,6 +100,18 @@ impl<'a> BitcoindThread for BitcoindSender { bitrep_rx.recv().expect("Receiving from bitcoind thread") } + + fn cpfp_tx(&self, txids: Vec, feerate: f64) -> Result<(), BitcoindError> { + let (bitrep_tx, bitrep_rx) = sync_channel(0); + self.0 + .send(BitcoindMessageOut::CPFPTransaction( + txids, feerate, bitrep_tx, + )) + .expect("Sending to bitcoind thread"); + bitrep_rx.recv().expect("Receiving from bitcoind thread")?; + + Ok(()) + } } impl From> for BitcoindSender { diff --git a/src/utils.rs b/src/utils.rs index 7a52c2b2..c0519cb4 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -178,5 +178,8 @@ addr = "127.0.0.1:8332" fn sync_progress(&self) -> f64 { 1.0 } + fn cpfp_tx(&self, _txid: Vec, _feerate: f64) -> Result<(), BitcoindError> { + Ok(()) + } } } diff --git a/tests/servers/coordinatord b/tests/servers/coordinatord index d4ebc106..3b19b1ba 160000 --- a/tests/servers/coordinatord +++ b/tests/servers/coordinatord @@ -1 +1 @@ -Subproject commit d4ebc10638f23549fd1f80f3265c39e6c52418b5 +Subproject commit 3b19b1ba5947919e5c85e1b26cfc156d529e13fa diff --git a/tests/servers/cosignerd b/tests/servers/cosignerd index bcb4b64c..644ead5d 160000 --- a/tests/servers/cosignerd +++ b/tests/servers/cosignerd @@ -1 +1 @@ -Subproject commit bcb4b64c8382a4de93fbdb66e799b070ff4b37be +Subproject commit 644ead5d5fd6f1c994e8d45015ee564a3b56e16b diff --git a/tests/servers/miradord b/tests/servers/miradord index 011527e7..e589c355 160000 --- a/tests/servers/miradord +++ b/tests/servers/miradord @@ -1 +1 @@ -Subproject commit 011527e7fcfa77521b9f86e34a886ebce37efdfd +Subproject commit e589c35551972abfc6ba9468894400a83879c416 diff --git a/tests/test_rpc.py b/tests/test_rpc.py index 3ef54809..7a7329c3 100644 --- a/tests/test_rpc.py +++ b/tests/test_rpc.py @@ -1348,3 +1348,69 @@ def test_gethistory(revault_network, bitcoind, executor): ) == 5 ) + + +COIN = 10**8 + + +def get_unvault_txids(wallet, vaults): + unvault_txids = [] + for vault in vaults: + deposit = f"{vault['txid']}:{vault['vout']}" + unvault_psbt = serializations.PSBT() + unvault_b64 = wallet.rpc.listpresignedtransactions([deposit])[ + "presigned_transactions" + ][0]["unvault"] + unvault_psbt.deserialize(unvault_b64) + unvault_psbt.tx.calc_sha256() + unvault_txids.append(unvault_psbt.tx.hash) + return unvault_txids + + +@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db") +def test_manual_cpfp(revault_network, bitcoind): + CSV = 12 + revault_network.deploy( + 2, + 1, + csv=CSV, + bitcoind_rpc_mocks={"estimatesmartfee": {"feerate": 0.0005}}, # 50 sats/vbyte + ) + man = revault_network.mans()[0] + vaults = revault_network.fundmany([1, 2, 3]) + + # Broadcast the unvaults and get their txids + for vault in vaults: + revault_network.secure_vault(vault) + revault_network.activate_vault(vault) + revault_network.broadcast_unvaults_anyhow(vaults, priority=True) + unvault_txids = get_unvault_txids(man, vaults) + for w in revault_network.participants(): + wait_for( + lambda: len(w.rpc.listvaults(["unvaulting"])["vaults"]) == len(vaults), + ) + + # If the feerate isn't significantly lower than the estimate, we won't feebump. + # Note the Unvault txs have a fixed 24sat/vb feerate. + entry = bitcoind.rpc.getmempoolentry(unvault_txids[0]) + assert int(entry["fees"]["base"] * COIN / entry["vsize"]) == 24 + revault_network.bitcoind_proxy.mocks["estimatesmartfee"] = { + "feerate": 26 * 1_000 / COIN + } + bitcoind.generate_blocks_censor(1, unvault_txids) + man.wait_for_logs(["Checking if transactions need CPFP...", "Nothing to CPFP"]) + + # The feerate is still kept the same and we will trigger CPFP manually using + # rpc call. This should behave in the same way as an automatic CPFP trigger + man.rpc.cpfp(unvault_txids, 50) + revault_network.bitcoind_proxy.mocks["estimatesmartfee"] = { + "feerate": 26 * 1_000 / COIN + } + bitcoind.generate_blocks_censor(1, unvault_txids) + man.wait_for_log("CPFPed transactions") + wait_for(lambda: len(bitcoind.rpc.getrawmempool()) == len(unvault_txids) + 1) + for unvault_txid in unvault_txids: + entry = bitcoind.rpc.getmempoolentry(unvault_txid) + assert entry["descendantcount"] == 2 + package_feerate = entry["fees"]["descendant"] * COIN / entry["descendantsize"] + assert package_feerate >= 50