Merge #648: test: BDK won't add unconf inputs when fee bumping

5d00f82 test that BDK won't add unconf inputs when fee bumping (Daniela Brozzoni)
9874890 test: fix populate_test_db conf calculation (Daniela Brozzoni)
1d9fdd0 Remove wrong TODO comment in build_fee_bump (Daniela Brozzoni)

Pull request description:

  ### Description

  Closes #144

  ### Notes to reviewers

  #144 is describing a bug that doesn't seem to happen in BDK master anymore (BDK not respecting BIP125 rule 2). This PR just adds a test to check that the bug is fixed.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### Bugfixes:

  * [ ] This pull request breaks the existing API
  * [x] I've added tests to reproduce the issue which are now passing
  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  afilini:
    ACK 5d00f82

Tree-SHA512: 95833f3566f9716762884d65f3f656346482e45525a3e92efa86710b9f574fdd9af7d235f1f425e4298d6ff380db9af60d1d2008ccde2588d971757db2d136b8

Author: afilini

src/database/memory.rs

@@ -512,10 +512,13 @@ macro_rules! populate_test_db {
         };
 
         let txid = tx.txid();
-        let confirmation_time = tx_meta.min_confirmations.map(|conf| $crate::BlockTime {
-            height: current_height.unwrap().checked_sub(conf as u32).unwrap(),
-            timestamp: 0,
-        });
+        let confirmation_time = tx_meta
+            .min_confirmations
+            .and_then(|v| if v == 0 { None } else { Some(v) })
+            .map(|conf| $crate::BlockTime {
+                height: current_height.unwrap().checked_sub(conf as u32).unwrap() + 1,
+                timestamp: 0,
+            });
 
         let tx_details = $crate::TransactionDetails {
             transaction: Some(tx.clone()),

src/wallet/mod.rs

@@ -899,8 +899,6 @@ where
     /// # Ok::<(), bdk::Error>(())
     /// ```
     // TODO: support for merging multiple transactions while bumping the fees
-    // TODO: option to force addition of an extra output? seems bad for privacy to update the
-    // change
     pub fn build_fee_bump(
         &self,
         txid: Txid,
@@ -2264,7 +2262,6 @@ pub(crate) mod test {
             .drain_to(drain_addr.script_pubkey())
             .drain_wallet();
         let (psbt, details) = builder.finish().unwrap();
-        dbg!(&psbt);
         let outputs = psbt.unsigned_tx.output;
 
         assert_eq!(outputs.len(), 2);
@@ -3861,6 +3858,99 @@ pub(crate) mod test {
         assert_eq!(details.fee.unwrap_or(0), 250);
     }
 
+    #[test]
+    #[should_panic(expected = "InsufficientFunds")]
+    fn test_bump_fee_unconfirmed_inputs_only() {
+        // We try to bump the fee, but:
+        // - We can't reduce the change, as we have no change
+        // - All our UTXOs are unconfirmed
+        // So, we fail with "InsufficientFunds", as per RBF rule 2:
+        // The replacement transaction may only include an unconfirmed input
+        // if that input was included in one of the original transactions.
+        let (wallet, descriptors, _) = get_funded_wallet(get_test_wpkh());
+        let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap();
+        let mut builder = wallet.build_tx();
+        builder
+            .drain_wallet()
+            .drain_to(addr.script_pubkey())
+            .enable_rbf();
+        let (psbt, mut original_details) = builder.finish().unwrap();
+        // Now we receive one transaction with 0 confirmations. We won't be able to use that for
+        // fee bumping, as it's still unconfirmed!
+        crate::populate_test_db!(
+            wallet.database.borrow_mut(),
+            testutils! (@tx ( (@external descriptors, 0) => 25_000 ) (@confirmations 0)),
+            Some(100),
+        );
+        let mut tx = psbt.extract_tx();
+        let txid = tx.txid();
+        for txin in &mut tx.input {
+            txin.witness.push([0x00; 108]); // fake signature
+            wallet
+                .database
+                .borrow_mut()
+                .del_utxo(&txin.previous_output)
+                .unwrap();
+        }
+        original_details.transaction = Some(tx);
+        wallet
+            .database
+            .borrow_mut()
+            .set_tx(&original_details)
+            .unwrap();
+
+        let mut builder = wallet.build_fee_bump(txid).unwrap();
+        builder.fee_rate(FeeRate::from_sat_per_vb(25.0));
+        builder.finish().unwrap();
+    }
+
+    #[test]
+    fn test_bump_fee_unconfirmed_input() {
+        // We create a tx draining the wallet and spending one confirmed
+        // and one unconfirmed UTXO. We check that we can fee bump normally
+        // (BIP125 rule 2 only apply to newly added unconfirmed input, you can
+        // always fee bump with an unconfirmed input if it was included in the
+        // original transaction)
+        let (wallet, descriptors, _) = get_funded_wallet(get_test_wpkh());
+        let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap();
+        // We receive a tx with 0 confirmations, which will be used as an input
+        // in the drain tx.
+        crate::populate_test_db!(
+            wallet.database.borrow_mut(),
+            testutils! (@tx ( (@external descriptors, 0) => 25_000 ) (@confirmations 0)),
+            Some(100),
+        );
+        let mut builder = wallet.build_tx();
+        builder
+            .drain_wallet()
+            .drain_to(addr.script_pubkey())
+            .enable_rbf();
+        let (psbt, mut original_details) = builder.finish().unwrap();
+        let mut tx = psbt.extract_tx();
+        let txid = tx.txid();
+        for txin in &mut tx.input {
+            txin.witness.push([0x00; 108]); // fake signature
+            wallet
+                .database
+                .borrow_mut()
+                .del_utxo(&txin.previous_output)
+                .unwrap();
+        }
+        original_details.transaction = Some(tx);
+        wallet
+            .database
+            .borrow_mut()
+            .set_tx(&original_details)
+            .unwrap();
+
+        let mut builder = wallet.build_fee_bump(txid).unwrap();
+        builder
+            .fee_rate(FeeRate::from_sat_per_vb(15.0))
+            .allow_shrinking(addr.script_pubkey())
+            .unwrap();
+        builder.finish().unwrap();
+    }
+
     #[test]
     fn test_sign_single_xprv() {
         let (wallet, _, _) = get_funded_wallet("wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*)");
@@ -4725,7 +4815,7 @@ pub(crate) mod test {
 
         crate::populate_test_db!(
             wallet.database.borrow_mut(),
-            testutils! (@tx ( (@external descriptors, 0) => 25_000 ) (@confirmations 0)),
+            testutils! (@tx ( (@external descriptors, 0) => 25_000 ) (@confirmations 1)),
             Some(confirmation_time),
             (@coinbase true)
         );