descriptors: improve satisfaction size estimate for primary path

Thanks to darosior for providing the changes to `max_sat_weight`
and the Taproot sanity checks test.

src/commands/mod.rs

@@ -1488,9 +1488,9 @@ mod tests {
         // rust-bitcoin's serialization of transactions with no input silently affected our fee
         // calculation.
 
-        // Transaction is 1 in (P2WSH satisfaction), 2 outs. At 1sat/vb, it's 170 sats fees.
+        // Transaction is 1 in (P2WSH satisfaction), 2 outs. At 1sat/vb, it's 161 sats fees.
         // At 2sats/vb, it's twice that.
-        assert_eq!(tx.output[1].value.to_sat(), 89_830);
+        assert_eq!(tx.output[1].value.to_sat(), 89_839);
         let psbt = if let CreateSpendResult::Success { psbt, .. } = control
             .create_spend(&destinations, &[dummy_op], 2, None)
             .unwrap()
@@ -1500,7 +1500,7 @@ mod tests {
             panic!("expect successful spend creation")
         };
         let tx = psbt.unsigned_tx;
-        assert_eq!(tx.output[1].value.to_sat(), 89_660);
+        assert_eq!(tx.output[1].value.to_sat(), 89_678);
 
         // A feerate of 555 won't trigger the sanity checks (they were previously not taking the
         // satisfaction size into account and overestimating the feerate).
@@ -1563,13 +1563,13 @@ mod tests {
             warnings,
             vec![
                 "Dust UTXO. The minimal change output allowed by Liana is 5000 sats. \
-                Instead of creating a change of 4830 sats, it was added to the \
+                Instead of creating a change of 4839 sats, it was added to the \
                 transaction fee. Select a larger input to avoid this from happening."
             ]
         );
 
         // Increase the target value by the change amount and the warning will disappear.
-        *destinations.get_mut(&dummy_addr).unwrap() = 95_000 + 4_830;
+        *destinations.get_mut(&dummy_addr).unwrap() = 95_000 + 4_839;
         let (psbt, warnings) = if let CreateSpendResult::Success { psbt, warnings } = control
             .create_spend(&destinations, &[dummy_op], 1, None)
             .unwrap()
@@ -1599,7 +1599,7 @@ mod tests {
 
         // Now increase the target by 1 more sat and we will have insufficient funds.
         *destinations.get_mut(&dummy_addr).unwrap() =
-            95_000 + 4_830 + /* fee for change output */ 43 + 1;
+            95_000 + 4_839 + /* fee for change output */ 43 + 1;
         assert_eq!(
             control.create_spend(&destinations, &[dummy_op], 1, None),
             Ok(CreateSpendResult::InsufficientFunds { missing: 1 }),
@@ -1607,7 +1607,7 @@ mod tests {
 
         // Now decrease the target so that the lost change is just 1 sat.
         *destinations.get_mut(&dummy_addr).unwrap() =
-            100_000 - /* fee without change */ 127 - /* extra fee for change output */ 43 - 1;
+            100_000 - /* fee without change */ 118 - /* extra fee for change output */ 43 - 1;
         let warnings = if let CreateSpendResult::Success { warnings, .. } = control
             .create_spend(&destinations, &[dummy_op], 1, None)
             .unwrap()
@@ -1628,7 +1628,7 @@ mod tests {
 
         // Now decrease the target value so that we have enough for a change output.
         *destinations.get_mut(&dummy_addr).unwrap() =
-            95_000 - /* fee without change */ 127 - /* extra fee for change output */ 43;
+            95_000 - /* fee without change */ 118 - /* extra fee for change output */ 43;
         let (psbt, warnings) = if let CreateSpendResult::Success { psbt, warnings } = control
             .create_spend(&destinations, &[dummy_op], 1, None)
             .unwrap()
@@ -1644,7 +1644,7 @@ mod tests {
 
         // Now increase the target by 1 and we'll get a warning again, this time for 1 less than the dust threshold.
         *destinations.get_mut(&dummy_addr).unwrap() =
-            95_000 - /* fee without change */ 127 - /* extra fee for change output */ 43 + 1;
+            95_000 - /* fee without change */ 118 - /* extra fee for change output */ 43 + 1;
         let warnings = if let CreateSpendResult::Success { warnings, .. } = control
             .create_spend(&destinations, &[dummy_op], 1, None)
             .unwrap()
@@ -1697,12 +1697,8 @@ mod tests {
             spend_txid: None,
             spend_block: None,
         }]);
-        // Even though 1_000 is the max feerate allowed by our sanity check, we need to
-        // use 1_003 in order to exceed it and fail this test since coin selection is
-        // based on a minimum feerate of `feerate_vb / 4.0` sats/wu, which can result in
-        // the sats/vb feerate being lower than `feerate_vb`.
         assert_eq!(
-            control.create_spend(&destinations, &[dummy_op_dup], 1_003, None),
+            control.create_spend(&destinations, &[dummy_op_dup], 1_001, None),
             Err(CommandError::SpendCreation(SpendCreationError::InsaneFees(
                 InsaneFeeInfo::TooHighFeerate(1_001)
             )))

src/descriptors/mod.rs

@@ -6,12 +6,13 @@ use miniscript::{
         secp256k1,
     },
     descriptor,
+    plan::{Assets, CanSign},
     psbt::{PsbtInputExt, PsbtOutputExt},
     translate_hash_clone, ForEachKey, TranslatePk, Translator,
 };
 
 use std::{
-    collections::{BTreeMap, HashMap, HashSet},
+    collections::{BTreeMap, BTreeSet, HashMap, HashSet},
     convert::TryInto,
     error, fmt,
     str::{self, FromStr},
@@ -56,6 +57,10 @@ impl From<LianaPolicyError> for LianaDescError {
     }
 }
 
+fn varint_len(n: usize) -> usize {
+    bitcoin::VarInt(n as u64).size()
+}
+
 // Whether the key identified by its fingerprint+derivation path was derived from one of the xpubs
 // for this spending path.
 fn key_is_for_path(
@@ -228,23 +233,64 @@ impl LianaDescriptor {
             .0
     }
 
-    // TODO: on Taproot we should use this for recovery but keyspend size if there is a spendable
-    // internal key.
     /// Get the maximum size difference of a transaction input spending a Script derived from this
     /// descriptor before and after satisfaction. The returned value is in weight units.
     /// Callers are expected to account for the Segwit marker (2 WU). This takes into account the
     /// size of the witness stack length varint.
-    pub fn max_sat_weight(&self) -> usize {
-        // We add one to account for the witness stack size, as the `max_weight_to_satisfy` method
-        // computes the difference in size for a satisfied input that was *already* in a
-        // transaction that spent one or more Segwit coins (and thus already have 1 WU accounted
-        // for the emtpy witness). But this method is used to account between a completely "nude"
-        // transaction (and therefore no Segwit marker nor empty witness in inputs) and a satisfied
-        // transaction.
-        self.multi_desc
-            .max_weight_to_satisfy()
-            .expect("Always satisfiable")
-            + 1
+    pub fn max_sat_weight(&self, use_primary_path: bool) -> usize {
+        if use_primary_path {
+            // Get the keys from the primary path, to get a satisfaction size estimation only
+            // considering those.
+            let keys = self
+                .policy()
+                .primary_path
+                .thresh_origins()
+                .1
+                .into_iter()
+                .fold(BTreeSet::new(), |mut keys, (fg, der_paths)| {
+                    for der_path in der_paths {
+                        keys.insert(((fg, der_path), CanSign::default()));
+                    }
+                    keys
+                });
+            let assets = Assets {
+                keys,
+                ..Default::default()
+            };
+
+            // Unfortunately rust-miniscript satisfaction size estimation is inconsistent. For
+            // Taproot it considers the whole witness (including the control block size + the
+            // script size) but under P2WSH it does not consider the witscript! Therefore we
+            // manually add the size of the witscript, but only under P2WSH by the mean of the
+            // `explicit_script()` helper.
+            let der_desc = self
+                .receive_desc
+                .0
+                .at_derivation_index(0)
+                .expect("unhardened index");
+            let witscript_size = der_desc
+                .explicit_script()
+                .map(|s| varint_len(s.len()) + s.len())
+                .unwrap_or(0);
+
+            // Finally, compute the satisfaction template for the primary path and get its size.
+            der_desc
+                .plan(&assets)
+                .expect("Always satisfiable")
+                .witness_size()
+                + witscript_size
+        } else {
+            // We add one to account for the witness stack size, as the values above give the
+            // difference in size for a satisfied input that was *already* in a transaction
+            // that spent one or more Segwit coins (and thus already have 1 WU accounted for the
+            // emtpy witness). But this method is used to account between a completely "nude"
+            // transaction (and therefore no Segwit marker nor empty witness in inputs) and a
+            // satisfied transaction.
+            self.multi_desc
+                .max_weight_to_satisfy()
+                .expect("Always satisfiable")
+                + 1
+        }
     }
 
     /// Get the maximum size difference of a transaction input spending a Script derived from this
@@ -252,8 +298,8 @@ impl LianaDescriptor {
     /// bytes.
     /// Callers are expected to account for the Segwit marker (2 WU). This takes into account the
     /// size of the witness stack length varint.
-    pub fn max_sat_vbytes(&self) -> usize {
-        self.max_sat_weight()
+    pub fn max_sat_vbytes(&self, use_primary_path: bool) -> usize {
+        self.max_sat_weight(use_primary_path)
             .checked_add(WITNESS_SCALE_FACTOR - 1)
             .unwrap()
             .checked_div(WITNESS_SCALE_FACTOR)
@@ -262,9 +308,9 @@ impl LianaDescriptor {
 
     /// Get the maximum size in virtual bytes of the whole input in a transaction spending
     /// a coin with this Script.
-    pub fn spender_input_size(&self) -> usize {
+    pub fn spender_input_size(&self, use_primary_path: bool) -> usize {
         // txid + vout + nSequence + empty scriptSig + witness
-        32 + 4 + 4 + 1 + self.max_sat_vbytes()
+        32 + 4 + 4 + 1 + self.max_sat_vbytes(use_primary_path)
     }
 
     /// Whether this is a Taproot descriptor.
@@ -492,10 +538,10 @@ impl LianaDescriptor {
     /// Maximum possible size in vbytes of an unsigned transaction, `tx`,
     /// after satisfaction, assuming all inputs of `tx` are from this
     /// descriptor.
-    pub fn unsigned_tx_max_vbytes(&self, tx: &bitcoin::Transaction) -> u64 {
+    pub fn unsigned_tx_max_vbytes(&self, tx: &bitcoin::Transaction, use_primary_path: bool) -> u64 {
         let witness_factor: u64 = WITNESS_SCALE_FACTOR.try_into().unwrap();
         let num_inputs: u64 = tx.input.len().try_into().unwrap();
-        let max_sat_weight: u64 = self.max_sat_weight().try_into().unwrap();
+        let max_sat_weight: u64 = self.max_sat_weight(use_primary_path).try_into().unwrap();
         // Add weights together before converting to vbytes to avoid rounding up multiple times.
         let tx_wu = tx
             .weight()
@@ -1055,17 +1101,31 @@ mod tests {
     #[test]
     fn inheritance_descriptor_sat_size() {
         let desc = LianaDescriptor::from_str("wsh(or_d(pk([92162c45]tpubD6NzVbkrYhZ4WzTf9SsD6h7AH7oQEippXK2KP8qvhMMqFoNeN5YFVi7vRyeRSDGtgd2bPyMxUNmHui8t5yCgszxPPxMafu1VVzDpg9aruYW/<0;1>/*),and_v(v:pkh([abcdef01]tpubD6NzVbkrYhZ4Wdgu2yfdmrce5g4fiH1ZLmKhewsnNKupbi4sxjH1ZVAorkBLWSkhsjhg8kiq8C4BrBjMy3SjAKDyDdbuvUa1ToAHbiR98js/<0;1>/*),older(2))))#ravw7jw5").unwrap();
-        assert_eq!(desc.max_sat_vbytes(), (1 + 66 + 1 + 34 + 73 + 3) / 4); // See the stack details below.
+        // See the stack details below.
+        assert_eq!(desc.max_sat_vbytes(true), (1 + 66 + 73 + 3) / 4);
+        assert_eq!(desc.max_sat_vbytes(false), (1 + 66 + 1 + 34 + 73 + 3) / 4);
 
         // Maximum input size is (txid + vout + scriptsig + nSequence + max_sat).
         // Where max_sat is:
         // - Push the witness stack size
         // - Push the script
+        // If recovery:
         // - Push an empty vector for using the recovery path
         // - Push the recovery key
-        // - Push a signature for the recovery key
+        // EndIf
+        // - Push a signature for the primary/recovery key
         // NOTE: The specific value is asserted because this was tested against a regtest
         // transaction.
+        let stack = vec![vec![0; 65], vec![0; 72]];
+        let witness_size = bitcoin::VarInt(stack.len() as u64).size()
+            + stack
+                .iter()
+                .map(|item| bitcoin::VarInt(item.len() as u64).size() + item.len())
+                .sum::<usize>();
+        assert_eq!(
+            desc.spender_input_size(true),
+            32 + 4 + 1 + 4 + wu_to_vb(witness_size),
+        );
         let stack = vec![vec![0; 65], vec![0; 0], vec![0; 33], vec![0; 72]];
         let witness_size = bitcoin::VarInt(stack.len() as u64).size()
             + stack
@@ -1073,9 +1133,51 @@ mod tests {
                 .map(|item| bitcoin::VarInt(item.len() as u64).size() + item.len())
                 .sum::<usize>();
         assert_eq!(
-            desc.spender_input_size(),
+            desc.spender_input_size(false),
             32 + 4 + 1 + 4 + wu_to_vb(witness_size),
         );
+
+        // Now perform the sanity checks under Taproot.
+        let owner_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap());
+        let heir_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub688Hn4wScQAAiYJLPg9yH27hUpfZAUnmJejRQBCiwfP5PEDzjWMNW1wChcninxr5gyavFqbbDjdV1aK5USJz8NDVjUy7FRQaaqqXHh5SbXe/<0;1>/*").unwrap());
+        let timelock = 52560;
+        let desc = LianaDescriptor::new(
+            LianaPolicy::new(
+                owner_key.clone(),
+                [(timelock, heir_key.clone())].iter().cloned().collect(),
+            )
+            .unwrap(),
+        );
+
+        // If using the primary path, it's a keypath spend.
+        assert_eq!(desc.max_sat_vbytes(true), (1 + 65 + 3) / 4);
+        // If using the recovery path, it's a script path spend. The script is 40 bytes long. The
+        // control block is just the internal key and parity, so 33 bytes long.
+        assert_eq!(
+            desc.max_sat_vbytes(false),
+            (1 + 65 + 1 + 40 + 1 + 33 + 3) / 4
+        );
+
+        // The same against the spender_input_size() helper, adding the size of the txin and
+        // checking against a dummy witness stack.
+        fn wit_size(stack: &[Vec<u8>]) -> usize {
+            varint_len(stack.len())
+                + stack
+                    .iter()
+                    .map(|item| varint_len(item.len()) + item.len())
+                    .sum::<usize>()
+        }
+        let txin_boilerplate = 32 + 4 + 1 + 4;
+        let stack = vec![vec![0; 64]];
+        assert_eq!(
+            desc.spender_input_size(true),
+            txin_boilerplate + wu_to_vb(wit_size(&stack)),
+        );
+        let stack = vec![vec![0; 33], vec![0; 40], vec![0; 64]];
+        assert_eq!(
+            desc.spender_input_size(false),
+            txin_boilerplate + wu_to_vb(wit_size(&stack)),
+        );
     }
 
     #[test]

src/spend.rs

@@ -99,6 +99,7 @@ fn check_output_value(value: bitcoin::Amount) -> Result<(), SpendCreationError>
 fn sanity_check_psbt(
     spent_desc: &descriptors::LianaDescriptor,
     psbt: &Psbt,
+    use_primary_path: bool,
 ) -> Result<(), SpendCreationError> {
     let tx = &psbt.unsigned_tx;
 
@@ -137,7 +138,7 @@ fn sanity_check_psbt(
     }
 
     // Check the feerate isn't insane.
-    let tx_vb = spent_desc.unsigned_tx_max_vbytes(tx);
+    let tx_vb = spent_desc.unsigned_tx_max_vbytes(tx, use_primary_path);
     let feerate_sats_vb = abs_fee
         .checked_div(tx_vb)
         .ok_or(SpendCreationError::InsaneFees(
@@ -663,6 +664,13 @@ pub fn create_spend(
         value: bitcoin::Amount::MAX,
         script_pubkey: change_addr.addr.script_pubkey(),
     };
+    // If no candidates have relative locktime, then we should use the primary spending path.
+    // Note we set this value before actually selecting the coins, but we expect either all
+    // candidates or none to have relative locktime sequence so this is fine.
+    let use_primary_path = !candidate_coins
+        .iter()
+        .filter_map(|cand| cand.sequence)
+        .any(|seq| seq.is_relative_lock_time());
     // Now select the coins necessary using the provided candidates and determine whether
     // there is any leftover to create a change output.
     let CoinSelectionRes {
@@ -684,7 +692,7 @@ pub fn create_spend(
         }
         .into();
         let max_sat_wu = main_descriptor
-            .max_sat_weight()
+            .max_sat_weight(use_primary_path)
             .try_into()
             .expect("Weight must fit in a u32");
         select_coins_for_spend(
@@ -771,7 +779,7 @@ pub fn create_spend(
         inputs: psbt_ins,
         outputs: psbt_outs,
     };
-    sanity_check_psbt(main_descriptor, &psbt)?;
+    sanity_check_psbt(main_descriptor, &psbt, use_primary_path)?;
     // TODO: maybe check for common standardness rules (max size, ..)?
 
     Ok(CreateSpendRes {

tests/test_chain.py

@@ -446,10 +446,10 @@ def test_spend_replacement(lianad, bitcoind):
     destinations = {
         bitcoind.rpc.getnewaddress(): 650_000,
     }
-    second_res = lianad.rpc.createspend(destinations, second_outpoints, 2)
+    second_res = lianad.rpc.createspend(destinations, second_outpoints, 3)
     second_psbt = PSBT.from_base64(second_res["psbt"])
     destinations = {}
-    third_res = lianad.rpc.createspend(destinations, second_outpoints, 4)
+    third_res = lianad.rpc.createspend(destinations, second_outpoints, 5)
     third_psbt = PSBT.from_base64(third_res["psbt"])
 
     # Broadcast the first transaction. Make sure it's detected.

tests/test_rpc.py

@@ -1118,13 +1118,19 @@ def test_rbfpsbt_bump_fee(lianad, bitcoind):
             for c in lianad.rpc.listcoins([], first_outpoints)["coins"]
         )
     )
+    mempool_rbf_1 = bitcoind.rpc.getmempoolentry(rbf_1_txid)
+    # Note that in the mempool entry, "ancestor" includes rbf_1_txid itself.
+    rbf_1_feerate = (
+        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 of 10 sat/vb to set the min feerate, instead of 1 sat/vb of first
+    # feerate to set the min feerate, instead of 1 sat/vb of first
     # transaction:
-    with pytest.raises(RpcError, match=f"Feerate too low: 10."):
-        lianad.rpc.rbfpsbt(first_txid, False, 10)
-    # Using 11 for feerate works for P2WSH. For Taproot we need 12.
-    feerate = 12 if USE_TAPROOT else 11
+    with pytest.raises(RpcError, match=f"Feerate too low: {int(rbf_1_feerate)}."):
+        lianad.rpc.rbfpsbt(first_txid, False, int(rbf_1_feerate))
+    # Using 1 more for feerate works.
+    feerate = int(rbf_1_feerate) + 1
     lianad.rpc.rbfpsbt(first_txid, False, feerate)
     # Add a new transaction spending the change from the first RBF.
     desc_1_destinations = {

tests/test_spend.py

@@ -134,7 +134,7 @@ def test_coin_marked_spent(lianad, bitcoind):
     res = lianad.rpc.createspend(destinations, [outpoint], 1)
     psbt = PSBT.from_base64(res["psbt"])
     sign_and_broadcast(psbt)
-    change_amount = 840 if USE_TAPROOT else 830
+    change_amount = 858 if USE_TAPROOT else 839
     assert len(psbt.o) == 1
     assert len(res["warnings"]) == 1
     assert (
@@ -153,7 +153,7 @@ def test_coin_marked_spent(lianad, bitcoind):
     res = lianad.rpc.createspend(destinations, [outpoint_3], 1)
     psbt = PSBT.from_base64(res["psbt"])
     sign_and_broadcast(psbt)
-    change_amount = 828 if USE_TAPROOT else 818
+    change_amount = 846 if USE_TAPROOT else 827
     assert len(psbt.o) == 1
     assert len(res["warnings"]) == 1
     assert (
@@ -252,17 +252,17 @@ def test_send_to_self(lianad, bitcoind):
     lianad.rpc.broadcastspend(spend_txid)
 
     # The only output is the change output so the feerate of the transaction must
-    # not be lower than the one provided, and only possibly slightly higher (since
-    # we slightly overestimate the satisfaction size).
-    # FIXME: a 15% increase is huge.
+    # not be much lower than the one provided (it could be slightly lower since
+    # the change amount is determined using feerate in terms of sat/wu which, due
+    # to rounding, can lead to a slightly lower feerate in terms of sat/vb,
+    # especially when the number of inputs increases), and only possibly slightly
+    # higher (since we slightly overestimate the satisfaction size).
     res = bitcoind.rpc.getmempoolentry(spend_txid)
-    spend_feerate = int(res["fees"]["base"] * COIN / res["vsize"])
-    if not USE_TAPROOT:
-        assert specified_feerate <= spend_feerate <= int(specified_feerate * 115 / 100)
+    spend_feerate = res["fees"]["base"] * COIN / res["vsize"]  # keep as decimal
+    if USE_TAPROOT:
+        assert specified_feerate <= spend_feerate < specified_feerate + 0.5
     else:
-        # FIXME: under Taproot we should not consider the max feerate of all leaves if there
-        # is a spendable internal key.
-        assert specified_feerate <= spend_feerate <= int(specified_feerate * 125 / 100)
+        assert specified_feerate - 0.5 < spend_feerate < specified_feerate + 0.5
 
     # We should by now only have one coin.
     bitcoind.generate_block(1, wait_for_mempool=spend_txid)
@@ -275,7 +275,9 @@ def test_send_to_self(lianad, bitcoind):
     assert len(lianad.rpc.listaddresses()["addresses"]) == 3
     # Create a new spend to the receive address with index 3.
     recv_addr = lianad.rpc.listaddresses(3, 1)["addresses"][0]["receive"]
-    res = lianad.rpc.createspend({recv_addr: 11_955_000}, [], 2)
+    res = lianad.rpc.createspend(
+        {recv_addr: 11_965_000 if USE_TAPROOT else 11_955_000}, [], 2
+    )
     assert "psbt" in res
     # Max(receive_index, change_index) is now 4:
     assert len(lianad.rpc.listaddresses()["addresses"]) == 4
@@ -323,6 +325,14 @@ def test_coin_selection(lianad, bitcoind):
 
     # Sign and broadcast this Spend transaction.
     spend_txid_1 = sign_and_broadcast_psbt(lianad, spend_psbt_1)
+    # Check its feerate is approx 2 sat/vb
+    anc_vsize = bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"]
+    anc_fees = int(
+        bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN
+    )
+    # txid_1's feerate is approx 2 sat/vb as required.
+    txid_1_feerate = anc_fees / anc_vsize
+    assert 1.99 < txid_1_feerate < 2.01
     wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 2)
     # Check that change output is unconfirmed.
     assert len(lianad.rpc.listcoins(["unconfirmed"])["coins"]) == 1
@@ -342,17 +352,14 @@ def test_coin_selection(lianad, bitcoind):
     assert spend_psbt_2.tx.vin[0].prevout.hash == uint256_from_str(
         bytes.fromhex(spend_txid_1)[::-1]
     )
-    anc_vsize = bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"]
-    anc_fees = int(
-        bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN
-    )
-    additional_fee = additional_fees(anc_vsize, anc_fees, feerate)
+    additional_fee_at_10satvb = additional_fees(anc_vsize, anc_fees, feerate)
     assert len(spend_res_2["warnings"]) == 1
     assert (
         spend_res_2["warnings"][0]
         == "CPFP: an unconfirmed input was selected. The current transaction fee "
-        f"was increased by {additional_fee} sats to make the average feerate of "
-        "both the input and current transaction equal to the selected feerate."
+        f"was increased by {additional_fee_at_10satvb} sats to make the average "
+        "feerate of both the input and current transaction equal to the selected "
+        "feerate."
     )
 
     # Try 3 sat/vb:
@@ -364,24 +371,39 @@ def test_coin_selection(lianad, bitcoind):
     assert spend_psbt_2.tx.vin[0].prevout.hash == uint256_from_str(
         bytes.fromhex(spend_txid_1)[::-1]
     )
-    anc_vsize = bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"]
-    anc_fees = int(
-        bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN
-    )
-    additional_fee = additional_fees(anc_vsize, anc_fees, feerate)
+    additional_fee_at_3satvb = additional_fees(anc_vsize, anc_fees, feerate)
+    assert additional_fee_at_10satvb > additional_fee_at_3satvb
     assert len(spend_res_2["warnings"]) == 1
     assert (
         spend_res_2["warnings"][0]
         == "CPFP: an unconfirmed input was selected. The current transaction fee "
-        f"was increased by {additional_fee} sats to make the average feerate of "
-        "both the input and current transaction equal to the selected feerate."
+        f"was increased by {additional_fee_at_3satvb} sats to make the average "
+        "feerate of both the input and current transaction equal to the selected "
+        "feerate."
     )
 
-    # 2 sat/vb is same feerate as ancestor and we have no warnings:
-    spend_res_2 = lianad.rpc.createspend({dest_addr_2: 10_000}, [], 2)
+    # 2 sat/vb is approx same feerate as ancestor:
+    feerate = 2
+    spend_res_2 = lianad.rpc.createspend({dest_addr_2: 10_000}, [], feerate)
     assert "psbt" in spend_res_2
-    assert len(spend_res_2["warnings"]) == 0
     spend_psbt_2 = PSBT.from_base64(spend_res_2["psbt"])
+    spend_txid_2 = spend_psbt_2.tx.txid().hex()
+    if USE_TAPROOT:
+        assert len(spend_res_2["warnings"]) == 0
+    else:
+        # We still get a warning in the non-taproot case.
+        assert len(spend_res_2["warnings"]) == 1
+        additional_fee_at_2satvb = additional_fees(anc_vsize, anc_fees, feerate)
+        assert additional_fee_at_3satvb > additional_fee_at_2satvb
+        assert len(spend_res_2["warnings"]) == 1
+        assert (
+            spend_res_2["warnings"][0]
+            == "CPFP: an unconfirmed input was selected. The current transaction fee "
+            f"was increased by {additional_fee_at_2satvb} sats to make the average "
+            "feerate of both the input and current transaction equal to the selected "
+            "feerate."
+        )
+
     # The spend is using the unconfirmed change.
     assert spend_psbt_2.tx.vin[0].prevout.hash == uint256_from_str(
         bytes.fromhex(spend_txid_1)[::-1]
@@ -435,14 +457,18 @@ def test_coin_selection(lianad, bitcoind):
     # Note that in the mempool entry, "ancestor" includes spend_txid_3 itself.
     assert (
         mempool_txid_3["fees"]["ancestor"] * COIN // mempool_txid_3["ancestorsize"]
-        == 10
+        == feerate
     )
     # The spend_txid_3 transaction itself has a higher feerate.
     assert (mempool_txid_3["fees"]["base"] * COIN) // mempool_txid_3["vsize"] > 10
-    # If we subtract the extra that pays for the ancestor, the feerate is at the target value.
-    assert ((mempool_txid_3["fees"]["base"] * COIN) - 2770) // mempool_txid_3[
-        "vsize"
-    ] == 10
+    # If we subtract the extra that pays for the ancestor, the feerate is approximately
+    # at the target value.
+    assert (
+        feerate - 0.5
+        < ((mempool_txid_3["fees"]["base"] * COIN) - additional_fee)
+        / mempool_txid_3["vsize"]
+        < feerate + 0.5
+    )
 
     # Now confirm the spend.
     bitcoind.generate_block(1, wait_for_mempool=spend_txid_3)