diff --git a/doc/API.md b/doc/API.md index 96396167f..559894909 100644 --- a/doc/API.md +++ b/doc/API.md @@ -17,6 +17,7 @@ Commands must be sent as valid JSONRPC 2.0 requests, ending with a `\n`. | [`listspendtxs`](#listspendtxs) | List all stored Spend transactions | | [`delspendtx`](#delspendtx) | Delete a stored Spend transaction | | [`broadcastspend`](#broadcastspend) | Finalize a stored Spend PSBT, and broadcast it | +| [`rbfpsbt`](#rbfpsbt) | Create a new RBF Spend transaction | | [`startrescan`](#startrescan) | Start rescanning the block chain from a given date | | [`listconfirmed`](#listconfirmed) | List of confirmed transactions of incoming and outgoing funds | | [`listtransactions`](#listtransactions) | List of transactions with the given txids | @@ -257,6 +258,43 @@ This command does not return anything for now. | Field | Type | Description | | -------------- | --------- | ---------------------------------------------------- | +### `rbfpsbt` + +Create PSBT to replace the given transaction, which must point to a PSBT in our database, using RBF. + +This command can be used to either: +- "cancel" the transaction: the replacement will include at least one input from the previous transaction and will have only +a single output (change). +- bump the fee: the replacement will include all inputs from the previous transaction and all non-change outputs +will be kept the same, with only the change amount being modified as required. + +In both cases, the replacement transaction may include additional confirmed coins as inputs if required +in order to pay the higher fee (this applies also when replacing a self-send). + +If the transaction includes a change output to one of our own change addresses, +this same address will be used for change in the replacement transaction, if required. + +If the transaction pays to more than one of our change addresses, then the one receiving the highest value +will be used as a change address in the replacement and the others will be treated as non-change outputs +(i.e. removed for cancel or otherwise kept the same). + +If `feerate` is not passed to the command, the target feerate of the replacement will be set to the minimum value +allowed in order to replace this transaction using RBF (see https://github.com/bitcoin/bitcoin/blob/master/doc/policy/mempool-replacements.md#current-replace-by-fee-policy for further details about this and other conditions that must be satisfied when using RBF). + +#### Request + +| Field | Type | Description | +| ----------- | ----------------- | --------------------------------------------------------------- | +| `txid` | string | Hex encoded txid of the Spend transaction to be replaced. | +| `is_cancel` | bool | Whether to "cancel" the transaction or simply bump the fee. | +| `feerate` | integer(optional) | Target feerate for the RBF transaction (in sat/vb). | + +#### Response + +| Field | Type | Description | +| -------------- | --------- | ---------------------------------------------------- | +| `psbt` | string | PSBT of the spending transaction, encoded as base64. | + ### `startrescan` #### Request diff --git a/gui/src/daemon/model.rs b/gui/src/daemon/model.rs index 585c918fc..39c248072 100644 --- a/gui/src/daemon/model.rs +++ b/gui/src/daemon/model.rs @@ -87,7 +87,12 @@ impl SpendTx { } else { status = SpendStatus::Broadcast } - } else { + // The txid will be different if this PSBT is to replace another transaction + // that is currently spending the coin. + // The PSBT status should remain as Pending so that it can be signed and broadcast. + // Once the replacement transaction has been confirmed, the PSBT for the + // transaction currently spending this coin will be shown as Deprecated. + } else if info.height.is_some() { status = SpendStatus::Deprecated } } diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs index 93f8af7be..601ca10cc 100644 --- a/src/bitcoin/d/mod.rs +++ b/src/bitcoin/d/mod.rs @@ -1120,20 +1120,49 @@ impl BitcoinD { /// Whether this transaction is in the mempool. pub fn is_in_mempool(&self, txid: &bitcoin::Txid) -> bool { + self.mempool_entry(txid).is_some() + } + + /// Get mempool entry of the given transaction. + /// Returns `None` if it is not in the mempool. + pub fn mempool_entry(&self, txid: &bitcoin::Txid) -> Option { match self .make_fallible_node_request("getmempoolentry", ¶ms!(Json::String(txid.to_string()))) { - Ok(_) => true, + Ok(json) => Some(MempoolEntry::from(json)), Err(BitcoindError::Server(jsonrpc::Error::Rpc(jsonrpc::error::RpcError { code: -5, .. - }))) => false, + }))) => None, Err(e) => { panic!("Unexpected error returned by bitcoind {}", e); } } } + /// Get the list of txids spending those outpoints in mempool. + pub fn mempool_txs_spending_prevouts( + &self, + outpoints: &[bitcoin::OutPoint], + ) -> Vec { + let prevouts: Json = outpoints + .iter() + .map(|op| serde_json::json!({"txid": op.txid.to_string(), "vout": op.vout})) + .collect(); + self.make_node_request("gettxspendingprevout", ¶ms!(prevouts)) + .as_array() + .expect("Always returns an array") + .iter() + .filter_map(|e| { + e.get("spendingtxid").map(|e| { + e.as_str() + .and_then(|s| bitcoin::Txid::from_str(s).ok()) + .expect("Must be a valid txid if present") + }) + }) + .collect() + } + /// Stop bitcoind. pub fn stop(&self) { self.make_node_request("stop", &[]); @@ -1394,3 +1423,48 @@ impl<'a> CachedTxGetter<'a> { } } } + +#[derive(Debug, Clone)] +pub struct MempoolEntry { + pub vsize: u64, + pub fees: MempoolEntryFees, +} + +impl From for MempoolEntry { + fn from(json: Json) -> MempoolEntry { + let vsize = json + .get("vsize") + .and_then(Json::as_u64) + .expect("Must be present in bitcoind response"); + let fees = json + .get("fees") + .as_ref() + .expect("Must be present in bitcoind response") + .into(); + + MempoolEntry { vsize, fees } + } +} + +#[derive(Debug, Clone)] +pub struct MempoolEntryFees { + pub base: bitcoin::Amount, + pub descendant: bitcoin::Amount, +} + +impl From<&&Json> for MempoolEntryFees { + fn from(json: &&Json) -> MempoolEntryFees { + let json = json.as_object().expect("fees must be an object"); + let base = json + .get("base") + .and_then(Json::as_f64) + .and_then(|a| bitcoin::Amount::from_btc(a).ok()) + .expect("Must be present and a valid amount"); + let descendant = json + .get("descendant") + .and_then(Json::as_f64) + .and_then(|a| bitcoin::Amount::from_btc(a).ok()) + .expect("Must be present and a valid amount"); + MempoolEntryFees { base, descendant } + } +} diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs index bfed816a8..4ac4680d4 100644 --- a/src/bitcoin/mod.rs +++ b/src/bitcoin/mod.rs @@ -9,7 +9,7 @@ use crate::{ bitcoin::d::{BitcoindError, CachedTxGetter, LSBlockEntry}, descriptors, }; -pub use d::SyncProgress; +pub use d::{MempoolEntry, MempoolEntryFees, SyncProgress}; use std::{fmt, sync}; @@ -113,6 +113,9 @@ pub trait BitcoinInterface: Send { &self, txid: &bitcoin::Txid, ) -> Option<(bitcoin::Transaction, Option)>; + + /// Get the details of unconfirmed transactions spending these outpoints, if any. + fn mempool_spenders(&self, outpoints: &[bitcoin::OutPoint]) -> Vec; } impl BitcoinInterface for d::BitcoinD { @@ -356,6 +359,13 @@ impl BitcoinInterface for d::BitcoinD { ) -> Option<(bitcoin::Transaction, Option)> { self.get_transaction(txid).map(|res| (res.tx, res.block)) } + + fn mempool_spenders(&self, outpoints: &[bitcoin::OutPoint]) -> Vec { + self.mempool_txs_spending_prevouts(outpoints) + .into_iter() + .filter_map(|txid| self.mempool_entry(&txid)) + .collect() + } } // FIXME: do we need to repeat the entire trait implemenation? Isn't there a nicer way? @@ -442,6 +452,10 @@ impl BitcoinInterface for sync::Arc> ) -> Option<(bitcoin::Transaction, Option)> { self.lock().unwrap().wallet_transaction(txid) } + + fn mempool_spenders(&self, outpoints: &[bitcoin::OutPoint]) -> Vec { + self.lock().unwrap().mempool_spenders(outpoints) + } } // FIXME: We could avoid this type (and all the conversions entailing allocations) if bitcoind diff --git a/src/commands/mod.rs b/src/commands/mod.rs index bc0b5fb00..fa7d65cc1 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -15,7 +15,7 @@ pub use crate::database::{CoinStatus, LabelItem}; use bdk_coin_select::InsufficientFunds; use utils::{ deser_addr_assume_checked, deser_amount_from_sats, deser_fromstr, deser_hex, - select_coins_for_spend, ser_amount, ser_hex, ser_to_string, + select_coins_for_spend, ser_amount, ser_hex, ser_to_string, unsigned_tx_max_vbytes, }; use std::{ @@ -79,6 +79,7 @@ pub enum CommandError { /// Overflowing or unhardened derivation index. InvalidDerivationIndex, CoinSelectionError(InsufficientFunds), + RbfError(RbfErrorInfo), } impl fmt::Display for CommandError { @@ -144,7 +145,8 @@ impl fmt::Display for CommandError { "No coin currently spendable through this timelocked recovery path." ), Self::InvalidDerivationIndex => write!(f, "Unhardened or overflowing BIP32 derivation index."), - Self::CoinSelectionError(e) => write!(f, "Coin selection error: '{}'", e), + Self::CoinSelectionError(e) => write!(f, "Coin selection error: '{}'", e), + Self::RbfError(e) => write!(f, "RBF error: '{}'.", e) } } } @@ -171,6 +173,29 @@ pub enum InsaneFeeInfo { TooHighFeerate(u64), } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RbfErrorInfo { + MissingFeerate, + SuperfluousFeerate, + TooLowFeerate(u64), + NotSignaling, +} + +impl fmt::Display for RbfErrorInfo { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match *self { + Self::MissingFeerate => { + write!(f, "A feerate must be provided if not creating a cancel.") + } + Self::SuperfluousFeerate => { + write!(f, "A feerate must not be provided if creating a cancel. We'll always use the smallest one which satisfies the RBF rules.") + } + Self::TooLowFeerate(r) => write!(f, "Feerate too low: {}.", r), + Self::NotSignaling => write!(f, "Replacement candidate does not signal for RBF."), + } + } +} + /// A candidate for coin selection when creating a transaction. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct CandidateCoin { @@ -413,6 +438,134 @@ impl DaemonControl { coins_outpoints: &[bitcoin::OutPoint], feerate_vb: u64, change_address: Option>, + ) -> Result { + let is_self_send = destinations.is_empty(); + // For self-send, the coins must be specified. + if is_self_send && coins_outpoints.is_empty() { + return Err(CommandError::NoOutpointForSelfSend); + } + if feerate_vb < 1 { + return Err(CommandError::InvalidFeerate(feerate_vb)); + } + let mut db_conn = self.db.connection(); + + // Check the destination addresses are valid for the network and + // sanity check each output's value. + let mut destinations_checked = HashMap::with_capacity(destinations.len()); + for (address, value_sat) in destinations { + let address = self.validate_address(address.clone())?; + let amount = bitcoin::Amount::from_sat(*value_sat); + check_output_value(amount)?; + destinations_checked.insert(address, amount); + } + // Check also the change address if one has been given. + let change_address = change_address + .map(|addr| self.validate_address(addr)) + .transpose()?; + // The candidate coins will be either all optional or all mandatory. + // If no coins have been specified, then coins will be selected automatically for + // the spend from a set of optional candidates. + // Otherwise, only the specified coins will be used, all as mandatory candidates. + let candidate_coins: Vec = if coins_outpoints.is_empty() { + // We only select confirmed coins for now. Including unconfirmed ones as well would + // introduce a whole bunch of additional complexity. + db_conn + .coins(&[CoinStatus::Confirmed], &[]) + .into_values() + .map(|c| CandidateCoin { + coin: c, + must_select: false, // No coin is mandatory. + }) + .collect() + } else { + // Query from DB and sanity check the provided coins to spend. + let coins = db_conn.coins(&[], coins_outpoints); + for op in coins_outpoints { + let coin = coins.get(op).ok_or(CommandError::UnknownOutpoint(*op))?; + if coin.is_spent() { + return Err(CommandError::AlreadySpent(*op)); + } + if coin.is_immature { + return Err(CommandError::ImmatureCoinbase(*op)); + } + } + coins + .into_values() + .map(|c| CandidateCoin { + coin: c, + must_select: true, // All coins must be selected. + }) + .collect() + }; + + self.create_spend_internal( + &destinations_checked, + &candidate_coins, + feerate_vb, + 0, // No min fee required. + change_address, + ) + } + + pub fn update_spend(&self, mut psbt: Psbt) -> Result<(), CommandError> { + let mut db_conn = self.db.connection(); + let tx = &psbt.unsigned_tx; + + // If the transaction already exists in DB, merge the signatures for each input on a best + // effort basis. + // We work on the newly provided PSBT, in case its content was updated. + let txid = tx.txid(); + if let Some(db_psbt) = db_conn.spend_tx(&txid) { + let db_tx = db_psbt.unsigned_tx; + for i in 0..db_tx.input.len() { + if tx + .input + .get(i) + .map(|tx_in| tx_in.previous_output == db_tx.input[i].previous_output) + != Some(true) + { + continue; + } + let psbtin = match psbt.inputs.get_mut(i) { + Some(psbtin) => psbtin, + None => continue, + }; + let db_psbtin = match db_psbt.inputs.get(i) { + Some(db_psbtin) => db_psbtin, + None => continue, + }; + psbtin + .partial_sigs + .extend(db_psbtin.partial_sigs.clone().into_iter()); + } + } else { + // If the transaction doesn't exist in DB already, sanity check its inputs. + // FIXME: should we allow for external inputs? + let outpoints: Vec = + tx.input.iter().map(|txin| txin.previous_output).collect(); + let coins = db_conn.coins_by_outpoints(&outpoints); + if coins.len() != outpoints.len() { + for op in outpoints { + if coins.get(&op).is_none() { + return Err(CommandError::UnknownOutpoint(op)); + } + } + } + } + + // Finally, insert (or update) the PSBT in database. + db_conn.store_spend(&psbt); + + Ok(()) + } + + fn create_spend_internal( + &self, + destinations: &HashMap, + candidate_coins: &[CandidateCoin], + feerate_vb: u64, + min_fee: u64, + change_address: Option, ) -> Result { // This method is a bit convoluted, but it's the nature of creating a Bitcoin transaction // with a target feerate and outputs. In addition, we support different modes (coin control @@ -426,14 +579,10 @@ impl DaemonControl { // The change output is also (ab)used to implement a "sweep" functionality. We allow to // set it to an external address to send all the inputs' value minus the fee and the // other output's value to a specific, external, address. - // 3. Fetch the selected coins from database and add them as inputs to the transaction. + // 3. Add the selected coins as inputs to the transaction. // 4. Finalize the PSBT and sanity check it before returning it. let is_self_send = destinations.is_empty(); - // For self-send, the coins must be specified. - if is_self_send && coins_outpoints.is_empty() { - return Err(CommandError::NoOutpointForSelfSend); - } if feerate_vb < 1 { return Err(CommandError::InvalidFeerate(feerate_vb)); } @@ -443,16 +592,13 @@ impl DaemonControl { let mut tx = bitcoin::Transaction { version: 2, lock_time: absolute::LockTime::Blocks(absolute::Height::ZERO), // TODO: randomized anti fee sniping - input: Vec::with_capacity(coins_outpoints.len()), // Will be zero capacity for coin selection. + input: Vec::with_capacity(candidate_coins.iter().filter(|c| c.must_select).count()), output: Vec::with_capacity(destinations.len()), }; // Add the destinations outputs to the transaction and PSBT. At the same time // sanity check each output's value. let mut psbt_outs = Vec::with_capacity(destinations.len()); - for (address, value_sat) in destinations { - let address = self.validate_address(address.clone())?; - - let amount = bitcoin::Amount::from_sat(*value_sat); + for (address, &amount) in destinations { check_output_value(amount)?; tx.output.push(bitcoin::TxOut { @@ -462,7 +608,7 @@ impl DaemonControl { // If it's an address of ours, signal it as change to signing devices by adding the // BIP32 derivation path to the PSBT output. let bip32_derivation = - if let Some((index, is_change)) = db_conn.derivation_index_by_address(&address) { + if let Some((index, is_change)) = db_conn.derivation_index_by_address(address) { let desc = if is_change { self.config.main_descriptor.change_descriptor() } else { @@ -491,7 +637,6 @@ impl DaemonControl { pub index: bip32::ChildNumber, } let (change_addr, int_change_info) = if let Some(addr) = change_address { - let addr = self.validate_address(addr)?; (addr, None) } else { let index = db_conn.change_index(); @@ -509,41 +654,9 @@ impl DaemonControl { value: std::u64::MAX, script_pubkey: change_addr.script_pubkey(), }; - // Now, either select the coins necessary or use the ones provided (verifying they do in - // fact exist and are still unspent) and determine whether there is any leftover to create a - // change output. + // Now select the coins necessary using the provided candidates and determine whether + // there is any leftover to create a change output. let (selected_coins, change_amount) = { - let candidate_coins: Vec = if coins_outpoints.is_empty() { - // We only select confirmed coins for now. Including unconfirmed ones as well would - // introduce a whole bunch of additional complexity. - db_conn - .coins(&[CoinStatus::Confirmed], &[]) - .into_values() - .map(|c| CandidateCoin { - coin: c, - must_select: false, // No coin is mandatory. - }) - .collect() - } else { - // Query from DB and sanity check the provided coins to spend. - let coins = db_conn.coins(&[], coins_outpoints); - for op in coins_outpoints { - let coin = coins.get(op).ok_or(CommandError::UnknownOutpoint(*op))?; - if coin.is_spent() { - return Err(CommandError::AlreadySpent(*op)); - } - if coin.is_immature { - return Err(CommandError::ImmatureCoinbase(*op)); - } - } - coins - .into_values() - .map(|c| CandidateCoin { - coin: c, - must_select: true, // All coins must be selected. - }) - .collect() - }; // At this point the transaction still has no input and no change output, as expected // by the coins selection helper function. assert!(tx.input.is_empty()); @@ -564,13 +677,13 @@ impl DaemonControl { .try_into() .expect("Weight must fit in a u32"); select_coins_for_spend( - &candidate_coins, + candidate_coins, tx.clone(), change_txo.clone(), feerate_vb, - 0, // We only constrain the feerate. + min_fee, max_sat_wu, - is_self_send, // Must have change if self-send. + is_self_send, ) .map_err(CommandError::CoinSelectionError)? }; @@ -675,58 +788,6 @@ impl DaemonControl { Ok(CreateSpendResult { psbt }) } - pub fn update_spend(&self, mut psbt: Psbt) -> Result<(), CommandError> { - let mut db_conn = self.db.connection(); - let tx = &psbt.unsigned_tx; - - // If the transaction already exists in DB, merge the signatures for each input on a best - // effort basis. - // We work on the newly provided PSBT, in case its content was updated. - let txid = tx.txid(); - if let Some(db_psbt) = db_conn.spend_tx(&txid) { - let db_tx = db_psbt.unsigned_tx; - for i in 0..db_tx.input.len() { - if tx - .input - .get(i) - .map(|tx_in| tx_in.previous_output == db_tx.input[i].previous_output) - != Some(true) - { - continue; - } - let psbtin = match psbt.inputs.get_mut(i) { - Some(psbtin) => psbtin, - None => continue, - }; - let db_psbtin = match db_psbt.inputs.get(i) { - Some(db_psbtin) => db_psbtin, - None => continue, - }; - psbtin - .partial_sigs - .extend(db_psbtin.partial_sigs.clone().into_iter()); - } - } else { - // If the transaction doesn't exist in DB already, sanity check its inputs. - // FIXME: should we allow for external inputs? - let outpoints: Vec = - tx.input.iter().map(|txin| txin.previous_output).collect(); - let coins = db_conn.coins_by_outpoints(&outpoints); - if coins.len() != outpoints.len() { - for op in outpoints { - if coins.get(&op).is_none() { - return Err(CommandError::UnknownOutpoint(op)); - } - } - } - } - - // Finally, insert (or update) the PSBT in database. - db_conn.store_spend(&psbt); - - Ok(()) - } - pub fn update_labels(&self, items: &HashMap>) { let mut db_conn = self.db.connection(); db_conn.update_labels(items); @@ -780,6 +841,251 @@ impl DaemonControl { .map_err(CommandError::TxBroadcast) } + /// Create PSBT to replace the given transaction using RBF. + /// + /// `txid` must point to a PSBT in our database. + /// + /// `is_cancel` indicates whether to "cancel" the transaction by including only a single (change) + /// output in the replacement or otherwise to keep the same (non-change) outputs and simply + /// bump the fee. + /// If `true`, the only output of the RBF transaction will be change and the inputs will include + /// at least one of the inputs from the previous transaction. If `false`, all inputs from the previous + /// transaction will be used in the replacement. + /// In both cases: + /// - if the previous transaction includes a change output to one of our own change addresses, + /// this same address will be used for change in the RBF transaction, if required. If the previous + /// transaction pays to more than one of our change addresses, then the one receiving the highest + /// value will be used as a change address and the others will be treated as non-change outputs. + /// - the RBF transaction may include additional confirmed coins as inputs if required + /// in order to pay the higher fee (this applies also when replacing a self-send). + /// + /// `feerate_vb` is the target feerate for the RBF transaction (in sat/vb). If `None`, it will be set + /// to 1 sat/vb larger than the feerate of the previous transaction, which is the minimum value allowed + /// when using RBF. + pub fn rbf_psbt( + &self, + txid: &bitcoin::Txid, + is_cancel: bool, + feerate_vb: Option, + ) -> Result { + let mut db_conn = self.db.connection(); + + if is_cancel && feerate_vb.is_some() { + return Err(CommandError::RbfError(RbfErrorInfo::SuperfluousFeerate)); + } + + let prev_psbt = db_conn + .spend_tx(txid) + .ok_or(CommandError::UnknownSpend(*txid))?; + if !prev_psbt.unsigned_tx.is_explicitly_rbf() { + return Err(CommandError::RbfError(RbfErrorInfo::NotSignaling)); + } + let prev_outpoints: Vec = prev_psbt + .unsigned_tx + .input + .iter() + .map(|txin| txin.previous_output) + .collect(); + let prev_coins = db_conn.coins_by_outpoints(&prev_outpoints); + // Make sure all prev outpoints are coins in our DB. + if let Some(op) = prev_outpoints + .iter() + .find(|op| !prev_coins.contains_key(op)) + { + return Err(CommandError::UnknownOutpoint(*op)); + } + if let Some(op) = prev_coins.iter().find_map(|(_, coin)| { + if coin.spend_block.is_some() { + Some(coin.outpoint) + } else { + None + } + }) { + return Err(CommandError::AlreadySpent(op)); + } + // Compute the minimal feerate and fee the replacement transaction must have to satisfy RBF + // rules #3, #4 and #6 (see + // https://github.com/bitcoin/bitcoin/blob/master/doc/policy/mempool-replacements.md). By + // default (ie if the transaction we are replacing was dropped from the mempool) there is + // no minimum absolute fee and the minimum feerate is 1, the minimum relay feerate. + let (min_feerate_vb, descendant_fees) = self + .bitcoin + .mempool_spenders(&prev_outpoints) + .into_iter() + .fold( + (1, bitcoin::Amount::from_sat(0)), + |(min_feerate, descendant_fee), entry| { + let entry_feerate = entry + .fees + .base + .checked_div(entry.vsize) + .expect("Can't have a null vsize or tx would be invalid") + .to_sat() + .checked_add(1) + .expect("Can't overflow or tx would be invalid"); + ( + std::cmp::max(min_feerate, entry_feerate), + descendant_fee + entry.fees.descendant, + ) + }, + ); + // Check replacement transaction's target feerate, if set, is high enough, + // and otherwise set it to the min feerate found above. + let feerate_vb = if is_cancel { + min_feerate_vb + } else { + feerate_vb.ok_or(CommandError::RbfError(RbfErrorInfo::MissingFeerate))? + }; + if feerate_vb < min_feerate_vb { + return Err(CommandError::RbfError(RbfErrorInfo::TooLowFeerate( + feerate_vb, + ))); + } + // Get info about prev outputs to determine replacement outputs. + let prev_derivs: Vec<_> = prev_psbt + .unsigned_tx + .output + .iter() + .map(|txo| { + let address = bitcoin::Address::from_script( + &txo.script_pubkey, + self.config.bitcoin_config.network, + ) + .expect("address already used in finalized transaction"); + ( + address.clone(), + bitcoin::Amount::from_sat(txo.value), + db_conn.derivation_index_by_address(&address), + ) + }) + .collect(); + // Set the previous change address to that of the change output with the largest value + // and then largest index. + let prev_change_address = prev_derivs + .iter() + .filter_map(|(addr, amt, deriv)| { + if let Some((ind, true)) = &deriv { + Some((addr, amt, ind)) + } else { + None + } + }) + .max_by(|(_, amt_1, ind_1), (_, amt_2, ind_2)| amt_1.cmp(amt_2).then(ind_1.cmp(ind_2))) + .map(|(addr, _, _)| addr) + .cloned(); + // If not cancel, use all previous outputs as destinations, except for + // the output corresponding to the change address we found above. + // If cancel, the replacement will not have any destinations, only a change output. + let destinations = if !is_cancel { + prev_derivs + .into_iter() + .filter_map(|(addr, amt, _)| { + if prev_change_address.as_ref() != Some(&addr) { + Some((addr, amt)) + } else { + None + } + }) + .collect() + } else { + HashMap::new() + }; + + // If there was no previous change address, we set the change address for the replacement + // to our next change address. This way, we won't increment the change index with each attempt + // at creating the replacement PSBT below. + let change_address = prev_change_address.unwrap_or_else(|| { + let index = db_conn.change_index(); + let desc = self + .config + .main_descriptor + .change_descriptor() + .derive(index, &self.secp); + desc.address(self.config.bitcoin_config.network) + }); + // If `!is_cancel`, we take the previous coins as mandatory candidates and add confirmed coins as optional. + // Otherwise, we take the previous coins as optional candidates and let coin selection find the + // best solution that includes at least one of these. If there are insufficient funds to create the replacement + // transaction in this way, then we set candidates in the same way as for the `!is_cancel` case. + let mut candidate_coins: Vec = prev_coins + .values() + .map(|c| CandidateCoin { + coin: *c, + must_select: !is_cancel, + }) + .collect(); + let confirmed_cands: Vec = db_conn + .coins(&[CoinStatus::Confirmed], &[]) + .into_values() + .filter_map(|c| { + // Make sure we don't have duplicate candidates in case any of the coins are not + // currently set as spending in the DB (and are therefore still confirmed). + if !prev_coins.contains_key(&c.outpoint) { + Some(CandidateCoin { + coin: c, + must_select: false, + }) + } else { + None + } + }) + .collect(); + if !is_cancel { + candidate_coins.extend(&confirmed_cands); + } + let max_sat_weight: u64 = self + .config + .main_descriptor + .max_sat_weight() + .try_into() + .expect("it must fit"); + // Try with increasing fee until fee paid by replacement transaction is high enough. + // Replacement fee must be at least: + // sum of fees paid by original transactions + incremental feerate * replacement size. + // Loop will continue until either we find a suitable replacement or we have insufficient funds. + let mut replacement_vsize = 0; + for incremental_feerate in 0.. { + let min_fee = descendant_fees.to_sat() + replacement_vsize * incremental_feerate; + let rbf_psbt = match self.create_spend_internal( + &destinations, + &candidate_coins, + feerate_vb, + min_fee, + Some(change_address.clone()), + ) { + Ok(psbt) => psbt, + // If we get a coin selection error due to insufficient funds and we want to cancel the + // transaction, then set all previous coins as mandatory and add confirmed coins as + // optional, unless we have already done this. + Err(CommandError::CoinSelectionError(_)) + if is_cancel && candidate_coins.iter().all(|c| !c.must_select) => + { + for cand in candidate_coins.iter_mut() { + cand.must_select = true; + } + candidate_coins.extend(&confirmed_cands); + continue; + } + Err(e) => { + return Err(e); + } + }; + replacement_vsize = unsigned_tx_max_vbytes(&rbf_psbt.psbt.unsigned_tx, max_sat_weight); + + // Make sure it satisfies RBF rule 4. + if rbf_psbt + .psbt + .fee() + .expect("has already been sanity checked") + >= descendant_fees + bitcoin::Amount::from_sat(replacement_vsize) + { + return Ok(rbf_psbt); + } + } + + unreachable!("We keep increasing the min fee until we run out of funds or satisfy rule 4.") + } + /// Trigger a rescan of the block chain for transactions involving our main descriptor between /// the given date and the current tip. /// The date must be after the genesis block time and before the current tip blocktime. @@ -1708,6 +2014,81 @@ mod tests { ms.shutdown(); } + #[test] + fn rbf_psbt() { + let dummy_op_a = bitcoin::OutPoint::from_str( + "3753a1d74c0af8dd0a0f3b763c14faf3bd9ed03cbdf33337a074fb0e9f6c7810:0", + ) + .unwrap(); + let mut dummy_bitcoind = DummyBitcoind::new(); + // Transaction spends outpoint a. + let dummy_tx_a = bitcoin::Transaction { + version: 2, + lock_time: absolute::LockTime::Blocks(absolute::Height::ZERO), + input: vec![bitcoin::TxIn { + previous_output: dummy_op_a, + sequence: bitcoin::Sequence::ENABLE_RBF_NO_LOCKTIME, + ..bitcoin::TxIn::default() + }], + output: vec![], + }; + // PSBT corresponding to the above transaction. + let dummy_psbt_a = Psbt { + unsigned_tx: dummy_tx_a.clone(), + version: 0, + xpub: BTreeMap::new(), + proprietary: BTreeMap::new(), + unknown: BTreeMap::new(), + inputs: vec![], + outputs: vec![], + }; + let dummy_txid_a = dummy_psbt_a.unsigned_tx.txid(); + dummy_bitcoind.txs.insert(dummy_txid_a, (dummy_tx_a, None)); + let ms = DummyLiana::new(dummy_bitcoind, DummyDatabase::new()); + let control = &ms.handle.control; + let mut db_conn = control.db().lock().unwrap().connection(); + // The spend needs to be in DB before using RBF. + assert_eq!( + control.rbf_psbt(&dummy_txid_a, true, None), + Err(CommandError::UnknownSpend(dummy_txid_a)) + ); + // Store the spend. + db_conn.store_spend(&dummy_psbt_a); + // Now add the coin to DB, but as spent. + db_conn.new_unspent_coins(&[Coin { + outpoint: dummy_op_a, + is_immature: false, + block_info: Some(BlockInfo { + height: 174500, + time: 174500, + }), + amount: bitcoin::Amount::from_sat(300_000), + derivation_index: bip32::ChildNumber::from(11), + is_change: false, + spend_txid: Some(dummy_txid_a), + spend_block: Some(BlockInfo { + height: 184500, + time: 184500, + }), + }]); + // The coin is spent so we cannot RBF. + assert_eq!( + control.rbf_psbt(&dummy_txid_a, true, None), + Err(CommandError::AlreadySpent(dummy_op_a)) + ); + db_conn.unspend_coins(&[dummy_op_a]); + // Now remove the coin. + db_conn.remove_coins(&[dummy_op_a]); + assert_eq!( + control.rbf_psbt(&dummy_txid_a, true, None), + Err(CommandError::UnknownOutpoint(dummy_op_a)) + ); + // A target feerate not higher than the previous should return an error. This is tested in + // the functional tests. + + ms.shutdown(); + } + #[test] fn list_confirmed_transactions() { let outpoint = OutPoint::new( diff --git a/src/commands/utils.rs b/src/commands/utils.rs index ed2301d4c..d5f5e561a 100644 --- a/src/commands/utils.rs +++ b/src/commands/utils.rs @@ -5,7 +5,7 @@ use bdk_coin_select::{ use log::warn; use std::{convert::TryInto, str::FromStr}; -use miniscript::bitcoin::{self, consensus, hashes::hex::FromHex}; +use miniscript::bitcoin::{self, consensus, constants::WITNESS_SCALE_FACTOR, hashes::hex::FromHex}; use serde::{de, Deserialize, Deserializer, Serializer}; use crate::database::Coin; @@ -244,3 +244,26 @@ pub fn select_coins_for_spend( change_amount, )) } + +/// An unsigned transaction's maximum possible size in vbytes after satisfaction. +/// +/// This assumes all inputs are internal (or have the same `max_sat_weight` value). +/// +/// `tx` is the unsigned transaction. +/// +/// `max_sat_weight` is the maximum weight difference of an input in the +/// transaction before and after satisfaction. Must be in weight units. +pub fn unsigned_tx_max_vbytes(tx: &bitcoin::Transaction, max_sat_weight: u64) -> u64 { + let witness_factor: u64 = WITNESS_SCALE_FACTOR.try_into().unwrap(); + let num_inputs: u64 = tx.input.len().try_into().unwrap(); + let tx_wu: u64 = tx + .weight() + .to_wu() + .checked_add(max_sat_weight.checked_mul(num_inputs).unwrap()) + .unwrap(); + tx_wu + .checked_add(witness_factor.checked_sub(1).unwrap()) + .unwrap() + .checked_div(witness_factor) + .unwrap() +} diff --git a/src/jsonrpc/api.rs b/src/jsonrpc/api.rs index 4eb8f59db..403563cf1 100644 --- a/src/jsonrpc/api.rs +++ b/src/jsonrpc/api.rs @@ -98,6 +98,31 @@ fn broadcast_spend(control: &DaemonControl, params: Params) -> Result Result { + let txid = params + .get(0, "txid") + .ok_or_else(|| Error::invalid_params("Missing 'txid' parameter."))? + .as_str() + .and_then(|s| bitcoin::Txid::from_str(s).ok()) + .ok_or_else(|| Error::invalid_params("Invalid 'txid' parameter."))?; + let is_cancel: bool = params + .get(1, "is_cancel") + .ok_or_else(|| Error::invalid_params("Missing 'is_cancel' parameter."))? + .as_bool() + .ok_or_else(|| Error::invalid_params("Invalid 'is_cancel' parameter."))?; + let feerate_vb: Option = if let Some(feerate) = params.get(2, "feerate") { + Some( + feerate + .as_u64() + .ok_or_else(|| Error::invalid_params("Invalid 'feerate' parameter."))?, + ) + } else { + None + }; + let res = control.rbf_psbt(&txid, is_cancel, feerate_vb)?; + Ok(serde_json::json!(&res)) +} + fn list_coins(control: &DaemonControl, params: Option) -> Result { let statuses_arg = params .as_ref() @@ -342,6 +367,12 @@ pub fn handle_request(control: &DaemonControl, req: Request) -> Result { + let params = req.params.ok_or_else(|| { + Error::invalid_params("Missing 'txid', 'feerate' and 'is_cancel' parameters.") + })?; + rbf_psbt(control, params)? + } "getinfo" => serde_json::json!(&control.get_info()), "getnewaddress" => serde_json::json!(&control.get_new_address()), "listcoins" => { diff --git a/src/jsonrpc/mod.rs b/src/jsonrpc/mod.rs index 14865c013..bf0b92203 100644 --- a/src/jsonrpc/mod.rs +++ b/src/jsonrpc/mod.rs @@ -165,6 +165,7 @@ impl From for Error { | commands::CommandError::InsaneRescanTimestamp(..) | commands::CommandError::AlreadyRescanning | commands::CommandError::InvalidDerivationIndex + | commands::CommandError::RbfError(..) | commands::CommandError::RecoveryNotAvailable => { Error::new(ErrorCode::InvalidParams, e.to_string()) } diff --git a/src/testutils.rs b/src/testutils.rs index 4f0505b3c..3bd663b08 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -1,5 +1,5 @@ use crate::{ - bitcoin::{BitcoinInterface, Block, BlockChainTip, SyncProgress, UTxO}, + bitcoin::{BitcoinInterface, Block, BlockChainTip, MempoolEntry, SyncProgress, UTxO}, config::{BitcoinConfig, Config}, database::{BlockInfo, Coin, CoinStatus, DatabaseConnection, DatabaseInterface, LabelItem}, descriptors, DaemonHandle, @@ -119,6 +119,10 @@ impl BitcoinInterface for DummyBitcoind { ) -> Option<(bitcoin::Transaction, Option)> { self.txs.get(txid).cloned() } + + fn mempool_spenders(&self, _: &[bitcoin::OutPoint]) -> Vec { + Vec::new() + } } struct DummyDbState { diff --git a/tests/test_chain.py b/tests/test_chain.py index aeda76cb0..e93a088ae 100644 --- a/tests/test_chain.py +++ b/tests/test_chain.py @@ -8,6 +8,7 @@ RpcError, COIN, sign_and_broadcast, + sign_and_broadcast_psbt, ) from test_framework.serializations import PSBT @@ -405,15 +406,8 @@ def is_spent_by(lianad, outpoint, txid): if coin["spend_info"] is None: return False return coin["spend_info"]["txid"] == txid.hex() - wait_for(lambda: is_spent_by(lianad, spent_coin["outpoint"], txid_b)) - -def sign_and_broadcast_psbt(lianad, psbt): - txid = psbt.tx.txid().hex() - psbt = lianad.signer.sign_psbt(psbt) - lianad.rpc.updatespend(psbt.to_base64()) - lianad.rpc.broadcastspend(txid) - return txid + wait_for(lambda: is_spent_by(lianad, spent_coin["outpoint"], txid_b)) def test_spend_replacement(lianad, bitcoind): diff --git a/tests/test_framework/utils.py b/tests/test_framework/utils.py index c599e2118..b38a88e1c 100644 --- a/tests/test_framework/utils.py +++ b/tests/test_framework/utils.py @@ -26,7 +26,7 @@ IS_BITCOIND_25 = bool(int(os.getenv("IS_BITCOIND_25", True))) -COIN = 10 ** 8 +COIN = 10**8 def wait_for(success, timeout=TIMEOUT, debug_fn=None): @@ -85,6 +85,15 @@ def sign_and_broadcast(lianad, bitcoind, psbt, recovery=False): return bitcoind.rpc.sendrawtransaction(tx) +def sign_and_broadcast_psbt(lianad, psbt): + """Sign a PSBT, save it to the DB and broadcast it.""" + txid = psbt.tx.txid().hex() + psbt = lianad.signer.sign_psbt(psbt) + lianad.rpc.updatespend(psbt.to_base64()) + lianad.rpc.broadcastspend(txid) + return txid + + class RpcError(ValueError): def __init__(self, method: str, params: dict, error: str): super(ValueError, self).__init__( diff --git a/tests/test_misc.py b/tests/test_misc.py index a1b23e6ba..0628c00b6 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -274,7 +274,9 @@ def test_migration(lianad_multisig, bitcoind): assert len(spend_txs) == 2 and all(s["updated_at"] is not None for s in spend_txs) -@pytest.mark.skipif(not IS_BITCOIND_25, reason="Need 'generateblock' with 'submit=False'") +@pytest.mark.skipif( + not IS_BITCOIND_25, reason="Need 'generateblock' with 'submit=False'" +) def test_bitcoind_submit_block(bitcoind): block_count = bitcoind.rpc.getblockcount() block = bitcoind.rpc.generateblock(bitcoind.rpc.getnewaddress(), [], False) @@ -294,7 +296,9 @@ def bitcoind_wait_new_block(bitcoind): continue -@pytest.mark.skipif(not IS_BITCOIND_25, reason="Need 'generateblock' with 'submit=False'") +@pytest.mark.skipif( + not IS_BITCOIND_25, reason="Need 'generateblock' with 'submit=False'" +) def test_retry_on_workqueue_exceeded(lianad, bitcoind, executor): """Make sure we retry requests to bitcoind if it is temporarily overloaded.""" # Start by reducing the work queue to a single slot. Note we need to stop lianad @@ -363,5 +367,7 @@ def test_wo_wallet_copy_from_bitcoind_datadir(lianad, bitcoind): # Now restart lianad. It should detect it within the bitcoind datadir and copy it to its own. lianad.start() assert os.path.isdir(new_wo_path) - assert lianad.is_in_log("A data directory exists with no watchonly wallet. This is most likely due to.*") + assert lianad.is_in_log( + "A data directory exists with no watchonly wallet. This is most likely due to.*" + ) assert lianad.is_in_log("Successfully copied the watchonly wallet file.") diff --git a/tests/test_rpc.py b/tests/test_rpc.py index f1bed23b9..b3bee3dc4 100644 --- a/tests/test_rpc.py +++ b/tests/test_rpc.py @@ -16,6 +16,7 @@ get_txid, spend_coins, sign_and_broadcast, + sign_and_broadcast_psbt, ) @@ -663,13 +664,6 @@ def all_spent(coins): def test_listtransactions(lianad, bitcoind): """Test listing of transactions by txid and timespan""" - def sign_and_broadcast(psbt): - txid = psbt.tx.txid().hex() - psbt = lianad.signer.sign_psbt(psbt) - lianad.rpc.updatespend(psbt.to_base64()) - lianad.rpc.broadcastspend(txid) - return txid - def wait_synced(): wait_for( lambda: lianad.rpc.getinfo()["block_height"] == bitcoind.rpc.getblockcount() @@ -715,7 +709,7 @@ def wait_synced(): } res = lianad.rpc.createspend(destinations, [outpoint], 6) psbt = PSBT.from_base64(res["psbt"]) - txid = sign_and_broadcast(psbt) + txid = sign_and_broadcast_psbt(lianad, psbt) bitcoind.generate_block(1, wait_for_mempool=txid) # Mine 12 blocks to force the blocktime to increase @@ -742,7 +736,7 @@ def wait_synced(): } res = lianad.rpc.createspend(destinations, [outpoint], 6) psbt = PSBT.from_base64(res["psbt"]) - txid = sign_and_broadcast(psbt) + txid = sign_and_broadcast_psbt(lianad, psbt) bitcoind.generate_block(1, wait_for_mempool=txid) # Deposit a coin that will be spending (unconfirmed spend transaction) @@ -758,7 +752,7 @@ def wait_synced(): } res = lianad.rpc.createspend(destinations, [outpoint], 6) psbt = PSBT.from_base64(res["psbt"]) - txid = sign_and_broadcast(psbt) + txid = sign_and_broadcast_psbt(lianad, psbt) # At this point we have 12 spent and unspent coins, one of them is unconfirmed. wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 12) @@ -1003,3 +997,304 @@ def test_labels(lianad, bitcoind): assert addr not in res assert sec_addr not in res assert res[random_address] == "this address is random" + + +def test_rbfpsbt_bump_fee(lianad, bitcoind): + """Test the use of RBF to bump the fee of a transaction.""" + + # Get three coins. + destinations = { + lianad.rpc.getnewaddress()["address"]: 0.003, + lianad.rpc.getnewaddress()["address"]: 0.004, + lianad.rpc.getnewaddress()["address"]: 0.005, + } + txid = bitcoind.rpc.sendmany("", destinations) + bitcoind.generate_block(1, wait_for_mempool=txid) + wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 3) + coins = lianad.rpc.listcoins(["confirmed"])["coins"] + + # Create a spend that will later be replaced. + first_outpoints = [c["outpoint"] for c in coins[:2]] + destinations = { + bitcoind.rpc.getnewaddress(): 650_000, + } + first_res = lianad.rpc.createspend(destinations, first_outpoints, 1) + first_psbt = PSBT.from_base64(first_res["psbt"]) + # The transaction has a change output. + assert len(first_psbt.o) == len(first_psbt.tx.vout) == 2 + first_txid = first_psbt.tx.txid().hex() + # We must provide a valid feerate. + for bad_feerate in [-1, "foo", 18_446_744_073_709_551_616]: + with pytest.raises(RpcError, match=f"Invalid 'feerate' parameter."): + lianad.rpc.rbfpsbt(first_txid, False, bad_feerate) + # We cannot RBF yet as first PSBT has not been saved. + with pytest.raises(RpcError, match=f"Unknown spend transaction '{first_txid}'."): + lianad.rpc.rbfpsbt(first_txid, False, 1) + # Now save the PSBT. + lianad.rpc.updatespend(first_res["psbt"]) + # The RBF command succeeds even if transaction has not been signed. + lianad.rpc.rbfpsbt(first_txid, False, 2) + # The RBF command also succeeds if transaction has been signed but not broadcast. + first_psbt = lianad.signer.sign_psbt(first_psbt) + lianad.rpc.updatespend(first_psbt.to_base64()) + lianad.rpc.rbfpsbt(first_txid, False, 2) + # Now broadcast the spend and wait for it to be detected. + lianad.rpc.broadcastspend(first_txid) + wait_for( + lambda: all( + c["spend_info"] is not None and c["spend_info"]["txid"] == first_txid + for c in lianad.rpc.listcoins([], first_outpoints)["coins"] + ) + ) + # We can now use RBF, but the feerate must be higher than that of the first transaction. + with pytest.raises(RpcError, match=f"Feerate too low: 1."): + lianad.rpc.rbfpsbt(first_txid, False, 1) + # Using a higher feerate works. + lianad.rpc.rbfpsbt(first_txid, False, 2) + # Let's use an even higher feerate. + rbf_1_res = lianad.rpc.rbfpsbt(first_txid, False, 10) + rbf_1_psbt = PSBT.from_base64(rbf_1_res["psbt"]) + # The inputs are the same in both (no new inputs needed in the replacement). + assert sorted( + psbt_in.map[PSBT_IN_NON_WITNESS_UTXO] for psbt_in in first_psbt.i + ) == sorted(psbt_in.map[PSBT_IN_NON_WITNESS_UTXO] for psbt_in in rbf_1_psbt.i) + # Check non-change output is the same in both. + assert first_psbt.tx.vout[0].nValue == rbf_1_psbt.tx.vout[0].nValue + assert first_psbt.tx.vout[0].scriptPubKey == rbf_1_psbt.tx.vout[0].scriptPubKey + # Change address is the same but change amount will be lower in the replacement to pay higher fee. + assert first_psbt.tx.vout[1].nValue > rbf_1_psbt.tx.vout[1].nValue + assert first_psbt.tx.vout[1].scriptPubKey == rbf_1_psbt.tx.vout[1].scriptPubKey + # Broadcast the replacement and wait for it to be detected. + rbf_1_txid = sign_and_broadcast_psbt(lianad, rbf_1_psbt) + wait_for( + lambda: all( + c["spend_info"] is not None and c["spend_info"]["txid"] == rbf_1_txid + for c in lianad.rpc.listcoins([], first_outpoints)["coins"] + ) + ) + # If we try to RBF the first transaction again, it will use the first RBF's + # feerate of 10 sat/vb to set the min feerate, instead of 1 sat/vb of first + # transaction: + with pytest.raises(RpcError, match=f"Feerate too low: 10."): + lianad.rpc.rbfpsbt(first_txid, False, 10) + # Using 11 for feerate works. + lianad.rpc.rbfpsbt(first_txid, False, 11) + # Add a new transaction spending the change from the first RBF. + desc_1_destinations = { + bitcoind.rpc.getnewaddress(): 500_000, + } + desc_1_outpoints = [f"{rbf_1_txid}:1", coins[2]["outpoint"]] + wait_for(lambda: len(lianad.rpc.listcoins([], desc_1_outpoints)["coins"]) == 2) + desc_1_res = lianad.rpc.createspend(desc_1_destinations, desc_1_outpoints, 1) + desc_1_psbt = PSBT.from_base64(desc_1_res["psbt"]) + assert len(desc_1_psbt.tx.vout) == 2 + desc_1_txid = sign_and_broadcast_psbt(lianad, desc_1_psbt) + wait_for( + lambda: all( + c["spend_info"] is not None and c["spend_info"]["txid"] == desc_1_txid + for c in lianad.rpc.listcoins([], desc_1_outpoints)["coins"] + ) + ) + # Add a new transaction spending the change from the first descendant. + desc_2_destinations = { + bitcoind.rpc.getnewaddress(): 25_000, + } + desc_2_outpoints = [f"{desc_1_txid}:1"] + wait_for(lambda: len(lianad.rpc.listcoins([], desc_2_outpoints)["coins"]) == 1) + desc_2_res = lianad.rpc.createspend(desc_2_destinations, desc_2_outpoints, 1) + desc_2_psbt = PSBT.from_base64(desc_2_res["psbt"]) + assert len(desc_2_psbt.tx.vout) == 2 + desc_2_txid = sign_and_broadcast_psbt(lianad, desc_2_psbt) + wait_for( + lambda: all( + c["spend_info"] is not None and c["spend_info"]["txid"] == desc_2_txid + for c in lianad.rpc.listcoins([], desc_2_outpoints)["coins"] + ) + ) + # Now replace the first RBF, which will also remove its descendants. + rbf_2_res = lianad.rpc.rbfpsbt(rbf_1_txid, False, 11) + rbf_2_psbt = PSBT.from_base64(rbf_2_res["psbt"]) + # The inputs are the same in both (no new inputs needed in the replacement). + assert sorted( + psbt_in.map[PSBT_IN_NON_WITNESS_UTXO] for psbt_in in rbf_1_psbt.i + ) == sorted(psbt_in.map[PSBT_IN_NON_WITNESS_UTXO] for psbt_in in rbf_2_psbt.i) + # Check non-change output is the same in both. + assert rbf_1_psbt.tx.vout[0].nValue == rbf_2_psbt.tx.vout[0].nValue + assert rbf_1_psbt.tx.vout[0].scriptPubKey == rbf_2_psbt.tx.vout[0].scriptPubKey + # Change address is the same but change amount will be lower in the replacement to pay higher fee. + assert rbf_1_psbt.tx.vout[1].nValue > rbf_2_psbt.tx.vout[1].nValue + assert rbf_1_psbt.tx.vout[1].scriptPubKey == rbf_2_psbt.tx.vout[1].scriptPubKey + + # Broadcast the replacement and wait for it to be detected. + rbf_2_txid = sign_and_broadcast_psbt(lianad, rbf_2_psbt) + wait_for( + lambda: all( + c["spend_info"] is not None and c["spend_info"]["txid"] == rbf_2_txid + for c in lianad.rpc.listcoins([], first_outpoints)["coins"] + ) + ) + # The unconfirmed coins used in the descendant transactions have been removed so that + # only one of the input coins remains, and its spend info has been wiped so that it is as before. + assert lianad.rpc.listcoins([], desc_1_outpoints + desc_2_outpoints)["coins"] == [ + coins[2] + ] + # Now confirm the replacement transaction. + bitcoind.generate_block(1, wait_for_mempool=rbf_2_txid) + wait_for( + lambda: all( + c["spend_info"]["txid"] == rbf_2_txid + and c["spend_info"]["height"] is not None + for c in lianad.rpc.listcoins([], first_outpoints)["coins"] + ) + ) + + +def test_rbfpsbt_cancel(lianad, bitcoind): + """Test the use of RBF to cancel a transaction.""" + + # Get three coins. + destinations = { + lianad.rpc.getnewaddress()["address"]: 0.003, + lianad.rpc.getnewaddress()["address"]: 0.004, + lianad.rpc.getnewaddress()["address"]: 0.005, + } + txid = bitcoind.rpc.sendmany("", destinations) + bitcoind.generate_block(1, wait_for_mempool=txid) + wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 3) + coins = lianad.rpc.listcoins(["confirmed"])["coins"] + + # Create a spend that will later be replaced. + first_outpoints = [c["outpoint"] for c in coins[:2]] + destinations = { + bitcoind.rpc.getnewaddress(): 650_000, + } + first_res = lianad.rpc.createspend(destinations, first_outpoints, 1) + first_psbt = PSBT.from_base64(first_res["psbt"]) + # The transaction has a change output. + assert len(first_psbt.o) == len(first_psbt.tx.vout) == 2 + first_txid = first_psbt.tx.txid().hex() + # Broadcast the spend and wait for it to be detected. + first_txid = sign_and_broadcast_psbt(lianad, first_psbt) + wait_for( + lambda: all( + c["spend_info"] is not None and c["spend_info"]["txid"] == first_txid + for c in lianad.rpc.listcoins([], first_outpoints)["coins"] + ) + ) + # We can use RBF and let the command choose the min possible feerate (1 larger than previous). + rbf_1_res = lianad.rpc.rbfpsbt(first_txid, True) + # But we can't set the feerate explicitly. + with pytest.raises( + RpcError, + match=re.escape( + "A feerate must not be provided if creating a cancel." + ), + ): + rbf_1_res = lianad.rpc.rbfpsbt(first_txid, True, 2) + rbf_1_psbt = PSBT.from_base64(rbf_1_res["psbt"]) + # Replacement only has a single input. + assert len(rbf_1_psbt.i) == 1 + # This input is one of the two from the previous transaction. + assert rbf_1_psbt.i[0].map[PSBT_IN_NON_WITNESS_UTXO] in [ + psbt_in.map[PSBT_IN_NON_WITNESS_UTXO] for psbt_in in rbf_1_psbt.i + ] + # The replacement only has a change output. + assert len(rbf_1_psbt.tx.vout) == 1 + # Change address is the same but change amount will be higher in the replacement as it is the only output. + assert first_psbt.tx.vout[1].nValue < rbf_1_psbt.tx.vout[0].nValue + assert first_psbt.tx.vout[1].scriptPubKey == rbf_1_psbt.tx.vout[0].scriptPubKey + # Broadcast the replacement and wait for it to be detected. + rbf_1_txid = sign_and_broadcast_psbt(lianad, rbf_1_psbt) + # The spend info of the coin used in the replacement will be updated. + + rbf_1_outpoint = ( + f"{rbf_1_psbt.tx.vin[0].prevout.hash:064x}:{rbf_1_psbt.tx.vin[0].prevout.n}" + ) + assert rbf_1_outpoint in first_outpoints + + wait_for( + lambda: all( + c["spend_info"] is not None and c["spend_info"]["txid"] == rbf_1_txid + for c in lianad.rpc.listcoins([], [rbf_1_outpoint])["coins"] + ) + ) + # The other coin will have its spend info removed. + wait_for( + lambda: all( + c["spend_info"] is None + for c in lianad.rpc.listcoins( + [], [op for op in first_outpoints if op != rbf_1_outpoint] + )["coins"] + ) + ) + # Add a new transaction spending the only output (change) from the first RBF. + desc_1_destinations = { + bitcoind.rpc.getnewaddress(): 500_000, + } + desc_1_outpoints = [f"{rbf_1_txid}:0", coins[2]["outpoint"]] + wait_for(lambda: len(lianad.rpc.listcoins([], desc_1_outpoints)["coins"]) == 2) + desc_1_res = lianad.rpc.createspend(desc_1_destinations, desc_1_outpoints, 1) + desc_1_psbt = PSBT.from_base64(desc_1_res["psbt"]) + assert len(desc_1_psbt.tx.vout) == 2 + desc_1_txid = sign_and_broadcast_psbt(lianad, desc_1_psbt) + wait_for( + lambda: all( + c["spend_info"] is not None and c["spend_info"]["txid"] == desc_1_txid + for c in lianad.rpc.listcoins([], desc_1_outpoints)["coins"] + ) + ) + # Add a new transaction spending the change from the first descendant. + desc_2_destinations = { + bitcoind.rpc.getnewaddress(): 25_000, + } + desc_2_outpoints = [f"{desc_1_txid}:1"] + wait_for(lambda: len(lianad.rpc.listcoins([], desc_2_outpoints)["coins"]) == 1) + desc_2_res = lianad.rpc.createspend(desc_2_destinations, desc_2_outpoints, 1) + desc_2_psbt = PSBT.from_base64(desc_2_res["psbt"]) + assert len(desc_2_psbt.tx.vout) == 2 + desc_2_txid = sign_and_broadcast_psbt(lianad, desc_2_psbt) + wait_for( + lambda: all( + c["spend_info"] is not None and c["spend_info"]["txid"] == desc_2_txid + for c in lianad.rpc.listcoins([], desc_2_outpoints)["coins"] + ) + ) + # Now cancel the first RBF, which will also remove its descendants. + rbf_2_res = lianad.rpc.rbfpsbt(rbf_1_txid, True) + rbf_2_psbt = PSBT.from_base64(rbf_2_res["psbt"]) + # + assert len(rbf_2_psbt.i) == 1 + assert ( + rbf_1_psbt.i[0].map[PSBT_IN_NON_WITNESS_UTXO] + == rbf_2_psbt.i[0].map[PSBT_IN_NON_WITNESS_UTXO] + ) + # The inputs are the same in both (no new inputs needed in the replacement). + + # Only a single output (change) in the replacement. + assert len(rbf_2_psbt.tx.vout) == 1 + # Change address is the same but change amount will be lower in the replacement to pay higher fee. + assert rbf_1_psbt.tx.vout[0].nValue > rbf_2_psbt.tx.vout[0].nValue + assert rbf_1_psbt.tx.vout[0].scriptPubKey == rbf_2_psbt.tx.vout[0].scriptPubKey + + # Broadcast the replacement and wait for it to be detected. + rbf_2_txid = sign_and_broadcast_psbt(lianad, rbf_2_psbt) + wait_for( + lambda: all( + c["spend_info"] is not None and c["spend_info"]["txid"] == rbf_2_txid + for c in lianad.rpc.listcoins([], [rbf_1_outpoint])["coins"] + ) + ) + # The unconfirmed coins used in the descendant transactions have been removed so that + # only one of the input coins remains, and its spend info has been wiped so that it is as before. + assert lianad.rpc.listcoins([], desc_1_outpoints + desc_2_outpoints)["coins"] == [ + coins[2] + ] + # Now confirm the replacement transaction. + bitcoind.generate_block(1, wait_for_mempool=rbf_2_txid) + wait_for( + lambda: all( + c["spend_info"]["txid"] == rbf_2_txid + and c["spend_info"]["height"] is not None + for c in lianad.rpc.listcoins([], [rbf_1_outpoint])["coins"] + ) + ) diff --git a/tests/test_spend.py b/tests/test_spend.py index 1480133ff..910c9ed2b 100644 --- a/tests/test_spend.py +++ b/tests/test_spend.py @@ -391,5 +391,7 @@ def test_sweep(lianad, bitcoind): sign_and_broadcast_psbt(lianad, psbt) wait_for(lambda: len(lianad.rpc.listcoins(["unconfirmed"])["coins"]) == 1) wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 2) - balance = sum(c["amount"] for c in lianad.rpc.listcoins(["unconfirmed", "confirmed"])["coins"]) + balance = sum( + c["amount"] for c in lianad.rpc.listcoins(["unconfirmed", "confirmed"])["coins"] + ) assert balance == int((0.2 + 0.1 + 0.3) * COIN)