rbf: make feerate optional

src/commands/mod.rs

@@ -963,8 +963,8 @@ impl DaemonControl {
     pub fn rbf_psbt(
         &self,
         txid: &bitcoin::Txid,
-        feerate_vb: u64,
         is_cancel: bool,
+        feerate_vb: Option<u64>,
     ) -> Result<CreateSpendResult, CommandError> {
         // The transaction to be replaced must be one of ours and so we can assume it signals replaceability.
         // The inputs in the replacement transaction will be either those coins used as inputs in the transaction
@@ -1006,8 +1006,10 @@ impl DaemonControl {
             .checked_div(mempool_entry.vsize)
             .ok_or(CommandError::InsaneFees(InsaneFeeInfo::InvalidFeerate))?;
         // Check replacement transaction's feerate is greater than previous feerate (the only directly conflicting transaction).
-        // TODO: Should we throw error or just set feerate_vb = prev_feerate_vb + 1 and check `rbf_psbt` below?
-        if feerate_vb < prev_feerate_vb.to_sat().checked_add(1).unwrap() {
+        // In case no feerate has been given, set it to prev_feerate_vb + 1.
+        let min_feerate_vb = prev_feerate_vb.to_sat().checked_add(1).unwrap();
+        let feerate_vb = feerate_vb.unwrap_or(min_feerate_vb);
+        if feerate_vb < min_feerate_vb && !is_cancel {
             return Err(CommandError::RbfError(RbfErrorInfo::TooLowFeerate(
                 feerate_vb,
             )));

src/jsonrpc/api.rs

@@ -94,18 +94,21 @@ fn rbf_psbt(control: &DaemonControl, params: Params) -> Result<serde_json::Value
         .as_str()
         .and_then(|s| bitcoin::Txid::from_str(s).ok())
         .ok_or_else(|| Error::invalid_params("Invalid 'txid' parameter."))?;
-    let feerate: u64 = params
-        .get(1, "feerate")
-        .ok_or_else(|| Error::invalid_params("Missing 'feerate' parameter."))?
-        .as_u64()
-        .ok_or_else(|| Error::invalid_params("Invalid 'feerate' parameter."))?;
     let is_cancel: bool = params
-        .get(2, "is_cancel")
+        .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 res = control.rbf_psbt(&txid, feerate, is_cancel)?;
+    let feerate_vb: Option<u64> = 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))
 }
 

tests/test_rpc.py

@@ -1023,23 +1023,27 @@ def test_rbf_psbt(lianad, bitcoind):
     # 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, 1, False)
+        lianad.rpc.rbfpsbt(first_txid, False, 1)
     # Now save the PSBT.
     lianad.rpc.updatespend(first_res["psbt"])
     # We still cannot use RBF as it has not been broadcast.
     with pytest.raises(
         RpcError, match=f"Transaction '{first_txid}' has not yet been broadcast."
     ):
-        lianad.rpc.rbfpsbt(first_txid, 1, False)
+        lianad.rpc.rbfpsbt(first_txid, False, 1)
     # Signing the spend still won't enable RBF.
     first_psbt = lianad.signer.sign_psbt(first_psbt)
     lianad.rpc.updatespend(first_psbt.to_base64())
     with pytest.raises(
         RpcError, match=f"Transaction '{first_txid}' has not yet been broadcast."
     ):
-        lianad.rpc.rbfpsbt(first_txid, 1, False)
+        lianad.rpc.rbfpsbt(first_txid, False, 1)
     # Now broadcast the spend and wait for it to be detected.
     lianad.rpc.broadcastspend(first_txid)
     wait_for(
@@ -1050,9 +1054,9 @@ def test_rbf_psbt(lianad, bitcoind):
     )
     # 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, 1, False)
+        lianad.rpc.rbfpsbt(first_txid, False, 1)
     # Using a higher feerate works.
-    rbf_1_res = lianad.rpc.rbfpsbt(first_txid, 2, False)
+    rbf_1_res = lianad.rpc.rbfpsbt(first_txid, False, 2)
     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(
@@ -1077,7 +1081,7 @@ def test_rbf_psbt(lianad, bitcoind):
     with pytest.raises(
         RpcError, match=f"Transaction '{first_txid}' has not yet been broadcast."
     ):
-        lianad.rpc.rbfpsbt(first_txid, 1, False)
+        lianad.rpc.rbfpsbt(first_txid, False, 1)
     # Add a new transaction spending the change from the first RBF.
     desc_1_destinations = {
         bitcoind.rpc.getnewaddress(): 500_000,
@@ -1109,7 +1113,7 @@ def test_rbf_psbt(lianad, bitcoind):
         )
     )
     # Now replace the first RBF, which will also remove its descendants.
-    rbf_2_res = lianad.rpc.rbfpsbt(rbf_1_txid, 4, False)
+    rbf_2_res = lianad.rpc.rbfpsbt(rbf_1_txid, False, 4)
     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(