Merge #1558: Allow RBF without saved PSBT

4d05c1f0ffb9cb9a7dd070342a7d19b9227ab8a2 commands: allow rbf for spending txs without saved psbt (Michael Mallan)
4ce5ca4d80cde79de60d545a7b992907cc5450e4 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 4d05c1f0ffb9cb9a7dd070342a7d19b9227ab8a2

Tree-SHA512: 2b5425ac2391e7f044c404f7ff449292807b360e14f33b6f2afbcd0c22bb399c371f950e0d43c0ab30e3a4ba76d83d257686de8e862e5dbee909577ed69d7dcd

doc/API.md

@@ -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

lianad/src/commands/mod.rs

@@ -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
@@ -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)
@@ -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| {

tests/test_rpc.py

@@ -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"])
@@ -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}.",