Skip to content

Commit

Permalink
Merge #1558: Allow RBF without saved PSBT
Browse files Browse the repository at this point in the history
4d05c1f commands: allow rbf for spending txs without saved psbt (Michael Mallan)
4ce5ca4 qa: check psbt must be saved in db to use rbf (Michael Mallan)

Pull request description:

  This is to fix #854 when using a local node. If we don't have the PSBT saved in our DB, we query the wallet transaction instead.

  The Liana Connect backend will also need to be updated in a similar way.

ACKs for top commit:
  edouardparis:
    ACK 4d05c1f

Tree-SHA512: 2b5425ac2391e7f044c404f7ff449292807b360e14f33b6f2afbcd0c22bb399c371f950e0d43c0ab30e3a4ba76d83d257686de8e862e5dbee909577ed69d7dcd
  • Loading branch information
edouardparis committed Jan 30, 2025
2 parents 71959b4 + 4d05c1f commit b98cff0
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 13 deletions.
4 changes: 3 additions & 1 deletion doc/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,9 @@ This command does not return anything for now.

### `rbfpsbt`

Create PSBT to replace the given transaction, which must point to a PSBT in our database, using RBF.
Create PSBT to replace, using RBF, the given transaction, which must either point to a PSBT in our database
(not necessarily broadcast) or an unconfirmed spend transaction (whether or not any associated
PSBT is saved in our database).

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
Expand Down
24 changes: 15 additions & 9 deletions lianad/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,8 @@ impl DaemonControl {

/// Create PSBT to replace the given transaction using RBF.
///
/// `txid` must point to a PSBT in our database.
/// `txid` must either point to a PSBT in our database (not necessarily broadcast) or an
/// unconfirmed spend transaction (whether or not any associated PSBT is saved 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
Expand Down Expand Up @@ -798,14 +799,20 @@ impl DaemonControl {
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() {
let prev_tx = if let Some(psbt) = db_conn.spend_tx(txid) {
psbt.unsigned_tx
} else {
db_conn
.coins(&[CoinStatus::Spending], &[])
.into_values()
.find(|c| c.spend_txid == Some(*txid))
.and_then(|_| tx_getter.get_tx(txid))
.ok_or(CommandError::UnknownSpend(*txid))?
};
if !prev_tx.is_explicitly_rbf() {
return Err(CommandError::RbfError(RbfErrorInfo::NotSignaling));
}
let prev_outpoints: Vec<bitcoin::OutPoint> = prev_psbt
.unsigned_tx
let prev_outpoints: Vec<bitcoin::OutPoint> = prev_tx
.input
.iter()
.map(|txin| txin.previous_output)
Expand Down Expand Up @@ -867,8 +874,7 @@ impl DaemonControl {
)));
}
// Get info about prev outputs to determine replacement outputs.
let prev_derivs: Vec<_> = prev_psbt
.unsigned_tx
let prev_derivs: Vec<_> = prev_tx
.output
.iter()
.map(|txo| {
Expand Down
16 changes: 13 additions & 3 deletions tests/test_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,11 @@ def test_rbfpsbt_bump_fee(lianad, bitcoind):
lianad.rpc.rbfpsbt(first_txid, False, 1)
# Using a higher feerate works.
lianad.rpc.rbfpsbt(first_txid, False, 2)

# We can still use RBF if the PSBT is no longer in the DB.
lianad.rpc.delspendtx(first_txid)
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"])
Expand Down Expand Up @@ -1183,9 +1188,14 @@ def test_rbfpsbt_bump_fee(lianad, bitcoind):
mempool_rbf_1["fees"]["ancestor"] * COIN / mempool_rbf_1["ancestorsize"]
)
assert 9.75 < rbf_1_feerate < 10.25
# If we try to RBF the first transaction again, it will use the first RBF's
# feerate to set the min feerate, instead of 1 sat/vb of first
# transaction:
# If we try to RBF the first transaction again, it will not be possible as we
# deleted the PSBT above and the tx is no longer part of our wallet's
# spending txs (even though it's saved in the DB).
with pytest.raises(RpcError, match=f"Unknown spend transaction '{first_txid}'."):
lianad.rpc.rbfpsbt(first_txid, False, 2)
# If we resave the PSBT, then we can use RBF and it will use the first RBF's
# feerate to set the min feerate, instead of 1 sat/vb of the first transaction:
lianad.rpc.updatespend(first_psbt.to_base64())
with pytest.raises(
RpcError,
match=f"Feerate {int(rbf_1_feerate)} too low for minimum feerate {int(rbf_1_feerate) + 1}.",
Expand Down

0 comments on commit b98cff0

Please sign in to comment.