Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manual CPFP #413

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Manual CPFP #413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions doc/API.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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. |



Expand Down Expand Up @@ -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

Expand Down
88 changes: 85 additions & 3 deletions src/bitcoind/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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() {
Zshan0 marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand All @@ -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 {
Expand Down Expand Up @@ -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))
})?;
}
}
}

Expand Down
122 changes: 18 additions & 104 deletions src/bitcoind/poller.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -408,7 +404,7 @@ fn mark_confirmed_emers(
Ok(())
}

enum ToBeCpfped {
pub enum ToBeCpfped {
Spend(SpendTransaction),
Unvault(UnvaultTransaction),
}
Expand Down Expand Up @@ -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(
Zshan0 marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -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");
}
Expand Down
Loading