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.

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

commands: add rbfpsbt command #816

Merged
merged 6 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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). |
Comment on lines +289 to +290
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not fond of having both as separate parameters since the feerate should always be set if it's not a cancel. But i don't have a better suggestion.


#### 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
Loading