Skip to content

Commit

Permalink
Merge #816: commands: add rbfpsbt command
Browse files Browse the repository at this point in the history
0ab00cd gui: keep conflicting PSBTs as Pending until confirmation (jp1ac4)
5391bfe commands: add `rbfpsbt` command (jp1ac4)
d5f3167 commands: add `create_spend_internal` function (jp1ac4)
714fd5e bitcoin: add `mempool_spenders` to Bitcoin interface (jp1ac4)
68b2503 func tests: move function to utils (jp1ac4)
fdab722 func tests: run black (jp1ac4)

Pull request description:

  This PR relates to #43 and #236.

  It adds a `rbfpsbt` command that generates a PSBT to replace an existing transaction using RBF. This replacement can either preserve non-change outputs and simply bump fees or remove non-change outputs and effectively cancel the transaction. The inputs and change output may need to be updated in accordance with the higher fee.

  I've also added a `getmempoolentry` call to the bitcoin interface that is used for checking information about descendant transactions.

  To facilitate development, I've made some temporary changes in the GUI so that replacement PSBTs can be signed and broadcast, but these changes might not be part of this PR in the end.

ACKs for top commit:
  darosior:
    ACK 0ab00cd

Tree-SHA512: a172ad895fac13be294451f2ffeccb91af521d58a3bc6d08e09688996f9a3e07a3e230091982ef5e92472d44db77b34f93b81d1111d2c570d9c5dd85b7c21f0f
  • Loading branch information
darosior committed Dec 6, 2023
2 parents bcfab48 + 0ab00cd commit f4d22a9
Show file tree
Hide file tree
Showing 14 changed files with 1,009 additions and 132 deletions.
38 changes: 38 additions & 0 deletions doc/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion gui/src/daemon/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
78 changes: 76 additions & 2 deletions src/bitcoin/d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<MempoolEntry> {
match self
.make_fallible_node_request("getmempoolentry", &params!(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<bitcoin::Txid> {
let prevouts: Json = outpoints
.iter()
.map(|op| serde_json::json!({"txid": op.txid.to_string(), "vout": op.vout}))
.collect();
self.make_node_request("gettxspendingprevout", &params!(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", &[]);
Expand Down Expand Up @@ -1394,3 +1423,48 @@ impl<'a> CachedTxGetter<'a> {
}
}
}

#[derive(Debug, Clone)]
pub struct MempoolEntry {
pub vsize: u64,
pub fees: MempoolEntryFees,
}

impl From<Json> 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 }
}
}
16 changes: 15 additions & 1 deletion src/bitcoin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -113,6 +113,9 @@ pub trait BitcoinInterface: Send {
&self,
txid: &bitcoin::Txid,
) -> Option<(bitcoin::Transaction, Option<Block>)>;

/// Get the details of unconfirmed transactions spending these outpoints, if any.
fn mempool_spenders(&self, outpoints: &[bitcoin::OutPoint]) -> Vec<MempoolEntry>;
}

impl BitcoinInterface for d::BitcoinD {
Expand Down Expand Up @@ -356,6 +359,13 @@ impl BitcoinInterface for d::BitcoinD {
) -> Option<(bitcoin::Transaction, Option<Block>)> {
self.get_transaction(txid).map(|res| (res.tx, res.block))
}

fn mempool_spenders(&self, outpoints: &[bitcoin::OutPoint]) -> Vec<MempoolEntry> {
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?
Expand Down Expand Up @@ -442,6 +452,10 @@ impl BitcoinInterface for sync::Arc<sync::Mutex<dyn BitcoinInterface + 'static>>
) -> Option<(bitcoin::Transaction, Option<Block>)> {
self.lock().unwrap().wallet_transaction(txid)
}

fn mempool_spenders(&self, outpoints: &[bitcoin::OutPoint]) -> Vec<MempoolEntry> {
self.lock().unwrap().mempool_spenders(outpoints)
}
}

// FIXME: We could avoid this type (and all the conversions entailing allocations) if bitcoind
Expand Down
Loading

0 comments on commit f4d22a9

Please sign in to comment.