Repository: wizardsardine/liana
Pull request #873 (2023-12-13 00:22:14)

commands: include unconfirmed change as coin selection candidates
This is a first PR towards resolving #826 that adds unconfirmed change as coin selection candidates when creating a spend.

As per https://github.com/wizardsardine/liana/issues/826#issuecomment-1853058734, I haven't made any changes to the `rbfpsbt` command.

We will also need to apply the same change in the GUI when selecting candidates for coin selection there (see https://github.com/wizardsardine/liana/pull/863#discussion_r1422823362).

darosior: > We will also need to apply the same change in the GUI when selecting candidates for coin selection there (see https://github.com/wizardsardine/liana/pull/863#discussion_r1422823362).

I think this should go in this PR as well, since we don't need to update the `liana` version in the GUI before making the change.
jp1ac4: Thanks for explaining these additional considerations.

I think your first approach could work. If we display extra information in the GUI for transactions spending unconfirmed coins, I think it would help avoid some of the potential confusion for the user.

We may be able to reflect the effect of ancestors in an unconfirmed candidate by increasing its [weight](https://github.com/bitcoindevkit/coin-select/blob/29b187f5509a01ba125a0354f6711e317bb5522a/src/coin_selector.rs#L545) attribute in order to increase the fee of the new transaction in such a way that the effective feerate for the new transaction plus its ancestors would be the same as the target feerate of the new transaction. Any unconfirmed candidates with low-feerate ancestors would be less likely to be selected in that case.
pythcoiner: > We could still do this but display a warning to the user "transaction was created with a feerate of 300sat/vb to pay for its unconfirmed parents, to make the whole package pay 100sat/vb as requested". This brings up #811 once again.

yes I think we should display 2 fees: one related to the `fee debt` of previous transaction and a second one related to the current tx
jp1ac4: > I guess this also changes the effective value of a coin (some will have larger, or lower feerate, parents and will need more fees to be spent).

I think the additional cost of spending an unconfirmed candidate in a way that achieves `target_feerate` should be something like
```
(target_feerate * ancestor_size) - ancestor_fee
```
So we could reduce the candidate's [value](https://github.com/wizardsardine/liana/blob/051957ede7c8b3ed9ed3e960888f2c993cae6021/src/spend.rs#L275) by this amount. If the ancestor feerate is higher than `target_feerate`, this delta would be negative so we would increase the candidate's effective value.
darosior: > (target_feerate * ancestor_size) - ancestor_fee

Yes that makes sense to me. However two coins may (partially) share the same ancestry.. We can just explicitly not take this into account and sometimes count an ancestor twice, that'd be unlikely but quite ugly when hit.

> So we could reduce the candidate's [value](https://github.com/wizardsardine/liana/blob/051957ede7c8b3ed9ed3e960888f2c993cae6021/src/spend.rs#L275) by this amount.

Hmm it sounds like it could have unintended consequences. Instead we could increase the [weight](https://github.com/wizardsardine/liana/blob/051957ede7c8b3ed9ed3e960888f2c993cae6021/src/spend.rs#L276) to achieve the same?

> If the ancestor feerate is higher than target_feerate, this delta would be negative so we would increase the candidate's effective value.

But this would make the selection think it's contributing more value than it really does wouldn't it? If so we certainly shouldn't do that, as parent don't pay for child. :)

I think what you want here is to bias the selection toward coins which aren't holding a large backlog right?
jp1ac4: > Yes that makes sense to me. However two coins may (partially) share the same ancestry.. We can just explicitly not take this into account and sometimes count an ancestor twice, that'd be unlikely but quite ugly when hit.

That's true. Ideally, we'll be able to reflect any such overpayment in a warning.

> But this would make the selection think it's contributing more value than it really does wouldn't it? If so we certainly shouldn't do that, as parent don't pay for child. :)

Ah yes, of course :smile:

> I think what you want here is to bias the selection toward coins which aren't holding a large backlog right?

That's right. I think changing the weight would be better then. We could increase/decrease an unconfirmed candidate's effective weight by
```
ancestor_size - (ancestor_fee / target_feerate)
```
which would then change the cost of spending this candidate at `target_feerate` as required. The effective weight could go down to 0 if the ancestor feerate is much higher.
jp1ac4: >  We could increase/decrease an unconfirmed candidate's effective weight by ...

Perhaps we shouldn't reduce the weight in the case the ancestor feerate is higher as we might end up with a feerate below `target_feerate` if two unconfirmed candidates share the same ancestors.
darosior: Yes, it's also a case of parent-pays-for-child.
jp1ac4: I've updated it to get the ancestor size and fees of any unconfirmed coins, and to modify the corresponding candidate's weight accordingly. This is done for both automated coin selection and manual coin selection. In the latter case, I'm adding ancestor info for all unconfirmed coins that have been specified, whether they are change or not.

In order to return a warning to the user, one option would be to check if any selected candidates have had their weights modified and to return the additional fee paid.
darosior: So we don't have to modify anything in the GUI? The "amount left to select" in the coin control is now using this right?
jp1ac4: For manual selection, the GUI would need to be updated [here](https://github.com/wizardsardine/liana/blob/15b1f0c789d251dc3d420de50e859b6dd4bfda75/gui/src/app/state/spend/step.rs#L219) to apply the same change when creating the `CandidateCoin` in order to calculate the  "amount left to select" in the same way.
jp1ac4: For manual selection, the GUI would need to be updated [here](https://github.com/wizardsardine/liana/blob/15b1f0c789d251dc3d420de50e859b6dd4bfda75/gui/src/app/state/spend/step.rs#L219) to apply the same change when creating the `CandidateCoin` in order to calculate the  "amount left to select" in the same way.
jp1ac4: > For manual selection, the GUI would need to be updated [here](https://github.com/wizardsardine/liana/blob/15b1f0c789d251dc3d420de50e859b6dd4bfda75/gui/src/app/state/spend/step.rs#L219) to apply the same change when creating the `CandidateCoin` in order to calculate the "amount left to select" in the same way.

Would it also be possible to allow `spend::create_spend()` to access the `mempool_entry` function? I guess we would need to add this to the `TxGetter` trait. Then we could add the ancestor info within that function instead of in the GUI.
darosior: That's unfortunate that we should duplicate this in the GUI.. I don't any non-ugly way of doing this. Should we go back to calling the RPC command instead of trying to use the library's `create_spend`?
jp1ac4: Building on the changes from https://github.com/wizardsardine/liana/pull/905, I've now added a warning in case an additional fee is included to pay for ancestors.

With the changes from https://github.com/wizardsardine/liana/pull/927, the GUI will be able to call the `createspend` command directly instead of the library's `create_spend` and so the GUI will not need to duplicate any logic from this PR.

If this approach looks fine, I'll continue adding tests.
darosior: This approach looks good to me.

Patch:
diff --git a/doc/API.md b/doc/API.md
index e85b4f0c..29abc45a 100644
--- a/doc/API.md
+++ b/doc/API.md
@@ -149,7 +149,8 @@ A coin may have one of the following four statuses:
 Create a transaction spending one or more of our coins. All coins must exist and not be spent.
 
 If no coins are specified in `outpoints`, they will be selected automatically from the set of
-confirmed coins (see [`listcoins`](#listcoins) for coin status definitions).
+confirmed coins together with any unconfirmed coins that are change outputs
+(see [`listcoins`](#listcoins) for coin status definitions).
 
 Will error if the given coins are not sufficient to cover the transaction cost at 90% (or more) of
 the given feerate. If on the contrary the transaction is more than sufficiently funded, it will
diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs
index a1b1317c..faaaf371 100644
--- a/src/bitcoin/d/mod.rs
+++ b/src/bitcoin/d/mod.rs
@@ -1473,6 +1473,7 @@ impl<'a> CachedTxGetter<'a> {
 #[derive(Debug, Clone)]
 pub struct MempoolEntry {
     pub vsize: u64,
+    pub ancestor_vsize: u64,
     pub fees: MempoolEntryFees,
 }
 
@@ -1482,19 +1483,28 @@ impl From<Json> for MempoolEntry {
             .get("vsize")
             .and_then(Json::as_u64)
             .expect("Must be present in bitcoind response");
+        let ancestor_vsize = json
+            .get("ancestorsize")
+            .and_then(Json::as_u64)
+            .expect("Must be present in bitcoind response");
         let fees = json
             .get("fees")
             .as_ref()
             .expect("Must be present in bitcoind response")
             .into();
 
-        MempoolEntry { vsize, fees }
+        MempoolEntry {
+            vsize,
+            ancestor_vsize,
+            fees,
+        }
     }
 }
 
 #[derive(Debug, Clone)]
 pub struct MempoolEntryFees {
     pub base: bitcoin::Amount,
+    pub ancestor: bitcoin::Amount,
     pub descendant: bitcoin::Amount,
 }
 
@@ -1506,11 +1516,20 @@ impl From<&&Json> for MempoolEntryFees {
             .and_then(Json::as_f64)
             .and_then(|a| bitcoin::Amount::from_btc(a).ok())
             .expect("Must be present and a valid amount");
+        let ancestor = json
+            .get("ancestor")
+            .and_then(Json::as_f64)
+            .and_then(|a| bitcoin::Amount::from_btc(a).ok())
+            .expect("Must be present and a valid amount");
         let descendant = json
             .get("descendant")
             .and_then(Json::as_f64)
             .and_then(|a| bitcoin::Amount::from_btc(a).ok())
             .expect("Must be present and a valid amount");
-        MempoolEntryFees { base, descendant }
+        MempoolEntryFees {
+            base,
+            ancestor,
+            descendant,
+        }
     }
 }
diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs
index 60388c1e..6c41c40c 100644
--- a/src/bitcoin/mod.rs
+++ b/src/bitcoin/mod.rs
@@ -116,6 +116,11 @@ pub trait BitcoinInterface: Send {
 
     /// Get the details of unconfirmed transactions spending these outpoints, if any.
     fn mempool_spenders(&self, outpoints: &[bitcoin::OutPoint]) -> Vec<MempoolEntry>;
+
+    /// Get mempool data for the given transaction.
+    ///
+    /// Returns `None` if the transaction is not in the mempool.
+    fn mempool_entry(&self, txid: &bitcoin::Txid) -> Option<MempoolEntry>;
 }
 
 impl BitcoinInterface for d::BitcoinD {
@@ -366,6 +371,10 @@ impl BitcoinInterface for d::BitcoinD {
             .filter_map(|txid| self.mempool_entry(&txid))
             .collect()
     }
+
+    fn mempool_entry(&self, txid: &bitcoin::Txid) -> Option<MempoolEntry> {
+        self.mempool_entry(txid)
+    }
 }
 
 // FIXME: do we need to repeat the entire trait implemenation? Isn't there a nicer way?
@@ -456,6 +465,10 @@ impl BitcoinInterface for sync::Arc<sync::Mutex<dyn BitcoinInterface + 'static>>
     fn mempool_spenders(&self, outpoints: &[bitcoin::OutPoint]) -> Vec<MempoolEntry> {
         self.lock().unwrap().mempool_spenders(outpoints)
     }
+
+    fn mempool_entry(&self, txid: &bitcoin::Txid) -> Option<MempoolEntry> {
+        self.lock().unwrap().mempool_entry(txid)
+    }
 }
 
 // FIXME: We could avoid this type (and all the conversions entailing allocations) if bitcoind
diff --git a/src/commands/mod.rs b/src/commands/mod.rs
index bf473bf6..19a5aec7 100644
--- a/src/commands/mod.rs
+++ b/src/commands/mod.rs
@@ -9,7 +9,7 @@ use crate::{
     database::{Coin, DatabaseConnection, DatabaseInterface},
     descriptors,
     spend::{
-        create_spend, AddrInfo, CandidateCoin, CreateSpendRes, SpendCreationError,
+        create_spend, AddrInfo, AncestorInfo, CandidateCoin, CreateSpendRes, SpendCreationError,
         SpendOutputAddress, SpendTxFees, TxGetter,
     },
     DaemonControl, VERSION,
@@ -179,6 +179,7 @@ fn coin_to_candidate(
     coin: &Coin,
     must_select: bool,
     sequence: Option<bitcoin::Sequence>,
+    ancestor_info: Option<AncestorInfo>,
 ) -> CandidateCoin {
     CandidateCoin {
         outpoint: coin.outpoint,
@@ -187,6 +188,7 @@ fn coin_to_candidate(
         is_change: coin.is_change,
         must_select,
         sequence,
+        ancestor_info,
     }
 }
 
@@ -460,13 +462,33 @@ impl DaemonControl {
         // the spend from a set of optional candidates.
         // Otherwise, only the specified coins will be used, all as mandatory candidates.
         let candidate_coins: Vec<CandidateCoin> = if coins_outpoints.is_empty() {
-            // We only select confirmed coins for now. Including unconfirmed ones as well would
-            // introduce a whole bunch of additional complexity.
+            // From our unconfirmed coins, we only include those that are change outputs
+            // since unconfirmed external deposits are more at risk of being dropped
+            // unexpectedly from the mempool as they are beyond the user's control.
             db_conn
-                .coins(&[CoinStatus::Confirmed], &[])
-                .into_values()
-                .map(|c| {
-                    coin_to_candidate(&c, /*must_select=*/ false, /*sequence=*/ None)
+                .coins(&[CoinStatus::Unconfirmed, CoinStatus::Confirmed], &[])
+                .into_iter()
+                .filter_map(|(op, c)| {
+                    if c.block_info.is_some() {
+                        Some((c, None)) // confirmed coins have no ancestor info
+                    } else if c.is_change {
+                        // In case the mempool_entry is None, the coin will be included without
+                        // any ancestor info.
+                        Some((
+                            c,
+                            self.bitcoin.mempool_entry(&op.txid).map(AncestorInfo::from),
+                        ))
+                    } else {
+                        None
+                    }
+                })
+                .map(|(c, ancestor_info)| {
+                    coin_to_candidate(
+                        &c,
+                        /*must_select=*/ false,
+                        /*sequence=*/ None,
+                        ancestor_info,
+                    )
                 })
                 .collect()
         } else {
@@ -482,8 +504,23 @@ impl DaemonControl {
                 }
             }
             coins
-                .into_values()
-                .map(|c| coin_to_candidate(&c, /*must_select=*/ true, /*sequence=*/ None))
+                .into_iter()
+                .map(|(op, c)| {
+                    let ancestor_info = if c.block_info.is_none() {
+                        // We include any non-change coins here as they have been selected by the caller.
+                        // If the unconfirmed coin's transaction is no longer in the mempool, keep the
+                        // coin as a candidate but without any ancestor info (same as confirmed candidate).
+                        self.bitcoin.mempool_entry(&op.txid).map(AncestorInfo::from)
+                    } else {
+                        None
+                    };
+                    coin_to_candidate(
+                        &c,
+                        /*must_select=*/ true,
+                        /*sequence=*/ None,
+                        ancestor_info,
+                    )
+                })
                 .collect()
         };
 
@@ -795,7 +832,14 @@ impl DaemonControl {
         let mut candidate_coins: Vec<CandidateCoin> = prev_coins
             .values()
             .map(|c| {
-                coin_to_candidate(c, /*must_select=*/ !is_cancel, /*sequence=*/ None)
+                // In case any previous coins are unconfirmed, we don't include their ancestor info
+                // in the candidate as the replacement fee and feerate will be higher and any
+                // additional fee to pay for ancestors should already have been taken into account
+                // when including these coins in the previous transaction.
+                coin_to_candidate(
+                    c, /*must_select=*/ !is_cancel, /*sequence=*/ None,
+                    /*ancestor_info=*/ None,
+                )
             })
             .collect();
         let confirmed_cands: Vec<CandidateCoin> = db_conn
@@ -807,6 +851,7 @@ impl DaemonControl {
                 if !prev_coins.contains_key(&c.outpoint) {
                     Some(coin_to_candidate(
                         &c, /*must_select=*/ false, /*sequence=*/ None,
+                        /*ancestor_info=*/ None,
                     ))
                 } else {
                     None
@@ -996,6 +1041,7 @@ impl DaemonControl {
                         &c,
                         /*must_select=*/ true,
                         /*sequence=*/ Some(bitcoin::Sequence::from_height(timelock)),
+                        /*ancestor_info=*/ None,
                     ))
                 } else {
                     None
@@ -1384,8 +1430,8 @@ mod tests {
             spend_txid: None,
             spend_block: None,
         }]);
-        // If we try to use coin selection, the unconfirmed coin will not be used as a candidate
-        // and so we get a coin selection error due to insufficient funds.
+        // If we try to use coin selection, the unconfirmed non-change coin will not be used
+        // as a candidate and so we get a coin selection error due to insufficient funds.
         assert!(matches!(
             control.create_spend(&destinations, &[], 1, None),
             Ok(CreateSpendResult::InsufficientFunds { .. }),
@@ -1616,7 +1662,7 @@ mod tests {
             )))
         );
 
-        // Add a confirmed unspent coin to be used for coin selection.
+        // Add an unconfirmed change coin to be used for coin selection.
         let confirmed_op_1 = bitcoin::OutPoint {
             txid: dummy_op.txid,
             vout: dummy_op.vout + 100,
@@ -1624,13 +1670,10 @@ mod tests {
         db_conn.new_unspent_coins(&[Coin {
             outpoint: confirmed_op_1,
             is_immature: false,
-            block_info: Some(BlockInfo {
-                height: 174500,
-                time: 174500,
-            }),
+            block_info: None,
             amount: bitcoin::Amount::from_sat(80_000),
             derivation_index: bip32::ChildNumber::from(42),
-            is_change: false,
+            is_change: true,
             spend_txid: None,
             spend_block: None,
         }]);
diff --git a/src/spend.rs b/src/spend.rs
index 4b9435d7..d545375e 100644
--- a/src/spend.rs
+++ b/src/spend.rs
@@ -1,6 +1,10 @@
-use crate::descriptors;
+use crate::{bitcoin::MempoolEntry, descriptors};
 
-use std::{collections::BTreeMap, convert::TryInto, fmt};
+use std::{
+    collections::{BTreeMap, HashMap},
+    convert::TryInto,
+    fmt,
+};
 
 pub use bdk_coin_select::InsufficientFunds;
 use bdk_coin_select::{
@@ -11,6 +15,7 @@ use miniscript::bitcoin::{
     self,
     absolute::{Height, LockTime},
     bip32,
+    constants::WITNESS_SCALE_FACTOR,
     psbt::{Input as PsbtIn, Output as PsbtOut, Psbt},
     secp256k1,
 };
@@ -154,6 +159,29 @@ fn sanity_check_psbt(
     Ok(())
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct AncestorInfo {
+    pub vsize: u32,
+    pub fee: u32,
+}
+
+impl From<MempoolEntry> for AncestorInfo {
+    fn from(entry: MempoolEntry) -> Self {
+        Self {
+            vsize: entry
+                .ancestor_vsize
+                .try_into()
+                .expect("vsize must fit in a u32"),
+            fee: entry
+                .fees
+                .ancestor
+                .to_sat()
+                .try_into()
+                .expect("fee in sats should fit in a u32"),
+        }
+    }
+}
+
 /// A candidate for coin selection when creating a transaction.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct CandidateCoin {
@@ -169,6 +197,8 @@ pub struct CandidateCoin {
     pub must_select: bool,
     /// The nSequence field to set for an input spending this coin.
     pub sequence: Option<bitcoin::Sequence>,
+    /// Information about in-mempool ancestors of the coin.
+    pub ancestor_info: Option<AncestorInfo>,
 }
 
 /// A coin selection result.
@@ -184,6 +214,8 @@ pub struct CoinSelectionRes {
     /// Maximum change amount possible with the selection irrespective of any change
     /// policy.
     pub max_change_amount: bitcoin::Amount,
+    /// Fee added to pay for ancestors at the target feerate.
+    pub fee_for_ancestors: bitcoin::Amount,
 }
 
 /// Metric based on [`LowestFee`] that aims to minimize transaction fees
@@ -291,12 +323,48 @@ fn select_coins_for_spend(
         base_weight = base_weight.saturating_sub(2);
     }
     let max_input_weight = TXIN_BASE_WEIGHT + max_sat_weight;
+    // Get feerate as u32 for calculation relating to ancestor below.
+    // We expect `feerate_vb` to be a positive integer, but take ceil()
+    // just in case to be sure we pay enough for ancestors.
+    let feerate_vb_u32 = feerate_vb.ceil() as u32;
+    let witness_factor: u32 = WITNESS_SCALE_FACTOR
+        .try_into()
+        .expect("scale factor must fit in u32");
+    // This will be used to store any extra weight added to candidates.
+    let mut added_weights = HashMap::<bitcoin::OutPoint, u32>::with_capacity(candidate_coins.len());
     let candidates: Vec<Candidate> = candidate_coins
         .iter()
         .map(|cand| Candidate {
             input_count: 1,
             value: cand.amount.to_sat(),
-            weight: max_input_weight,
+            weight: {
+                let extra = cand
+                    .ancestor_info
+                    .map(|info| {
+                        // The implied ancestor vsize if the fee had been paid at our target feerate.
+                        let ancestor_vsize_at_feerate: u32 = info
+                            .fee
+                            .checked_div(feerate_vb_u32)
+                            .expect("feerate is greater than zero");
+                        // If the actual ancestor vsize is bigger than the implied vsize, we will need to
+                        // pay the difference in order for the combined feerate to be at the target value.
+                        // We multiply the vsize by 4 to get the ancestor weight, which is an upper bound
+                        // on its true weight (vsize*4 - 3 <= weight <= vsize*4), to ensure we pay enough.
+                        // Note that if candidates share ancestors, we may add this difference more than
+                        // once in the resulting transaction.
+                        info.vsize
+                            .saturating_sub(ancestor_vsize_at_feerate)
+                            .checked_mul(witness_factor)
+                            .expect("weight difference must fit in u32")
+                    })
+                    .unwrap_or(0);
+                // Store the extra weight for this candidate for use later on.
+                // At the same time, make sure there are no duplicate outpoints.
+                assert!(added_weights.insert(cand.outpoint, extra).is_none());
+                max_input_weight
+                    .checked_add(extra)
+                    .expect("effective weight must fit in u32")
+            },
             is_segwit: true, // We only support receiving on Segwit scripts.
         })
         .collect();
@@ -336,9 +404,10 @@ fn select_coins_for_spend(
 
     // Finally, run the coin selection algorithm. We use an opportunistic BnB and if it couldn't
     // find any solution we fall back to selecting coins by descending value.
+    let feerate = FeeRate::from_sat_per_vb(feerate_vb);
     let target = Target {
         value: out_value_nochange,
-        feerate: FeeRate::from_sat_per_vb(feerate_vb),
+        feerate,
         min_fee,
     };
     let lowest_fee = LowestFee {
@@ -402,14 +471,29 @@ fn select_coins_for_spend(
             .try_into()
             .expect("value is non-negative"),
     );
+    let mut total_added_weight: u32 = 0;
+    let selected = selector
+        .selected_indices()
+        .iter()
+        .map(|i| candidate_coins[*i])
+        .inspect(|cand| {
+            total_added_weight = total_added_weight
+                .checked_add(
+                    *added_weights
+                        .get(&cand.outpoint)
+                        .expect("contains added weight for all candidates"),
+                )
+                .expect("should fit in u32")
+        })
+        .collect();
+    // Calculate added fee based on the feerate in sats/wu, which is the feerate used for coin selection.
+    let fee_for_ancestors =
+        bitcoin::Amount::from_sat(((total_added_weight as f32) * feerate.spwu()).ceil() as u64);
     Ok(CoinSelectionRes {
-        selected: selector
-            .selected_indices()
-            .iter()
-            .map(|i| candidate_coins[*i])
-            .collect(),
+        selected,
         change_amount,
         max_change_amount,
+        fee_for_ancestors,
     })
 }
 
@@ -459,6 +543,7 @@ pub enum SpendTxFees {
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
 pub enum CreateSpendWarning {
     ChangeAddedToFee(u64),
+    AddtionalFeeForAncestors(u64),
 }
 
 impl fmt::Display for CreateSpendWarning {
@@ -470,6 +555,12 @@ impl fmt::Display for CreateSpendWarning {
                 amt,
                 if *amt > 1 {"s"} else {""},
             ),
+            CreateSpendWarning::AddtionalFeeForAncestors(amt) => write!(
+                f,
+                "An additional fee of {} sat{} has been added to pay for ancestors at the target feerate.",
+                amt,
+                if *amt > 1 {"s"} else {""},
+            ),
         }
     }
 }
@@ -589,6 +680,7 @@ pub fn create_spend(
         selected,
         change_amount,
         max_change_amount,
+        fee_for_ancestors,
     } = {
         // At this point the transaction still has no input and no change output, as expected
         // by the coins selection helper function.
@@ -650,6 +742,12 @@ pub fn create_spend(
         ));
     }
 
+    if fee_for_ancestors.to_sat() > 0 {
+        warnings.push(CreateSpendWarning::AddtionalFeeForAncestors(
+            fee_for_ancestors.to_sat(),
+        ));
+    }
+
     // Iterate through selected coins and add necessary information to the PSBT inputs.
     let mut psbt_ins = Vec::with_capacity(selected.len());
     for cand in &selected {


Test patch:
diff --git a/src/testutils.rs b/src/testutils.rs
index 9034aaec..8359b75d 100644
--- a/src/testutils.rs
+++ b/src/testutils.rs
@@ -123,6 +123,10 @@ impl BitcoinInterface for DummyBitcoind {
     fn mempool_spenders(&self, _: &[bitcoin::OutPoint]) -> Vec<MempoolEntry> {
         Vec::new()
     }
+
+    fn mempool_entry(&self, _: &bitcoin::Txid) -> Option<MempoolEntry> {
+        None
+    }
 }
 
 struct DummyDbState {
diff --git a/tests/test_spend.py b/tests/test_spend.py
index 4ae75229..3e7224a2 100644
--- a/tests/test_spend.py
+++ b/tests/test_spend.py
@@ -1,6 +1,6 @@
 from fixtures import *
 from test_framework.serializations import PSBT, uint256_from_str
-from test_framework.utils import wait_for, COIN, RpcError
+from test_framework.utils import sign_and_broadcast_psbt, wait_for, COIN, RpcError
 
 
 def test_spend_change(lianad, bitcoind):
@@ -59,14 +59,6 @@ def test_spend_change(lianad, bitcoind):
     bitcoind.generate_block(1, wait_for_mempool=spend_txid)
 
 
-def sign_and_broadcast_psbt(lianad, psbt):
-    txid = psbt.tx.txid().hex()
-    psbt = lianad.signer.sign_psbt(psbt)
-    lianad.rpc.updatespend(psbt.to_base64())
-    lianad.rpc.broadcastspend(txid)
-    return txid
-
-
 def test_coin_marked_spent(lianad, bitcoind):
     """Test a spent coin is marked as such under various conditions."""
     # Receive a coin in a single transaction
@@ -179,7 +171,16 @@ def test_coin_marked_spent(lianad, bitcoind):
     psbt = PSBT.from_base64(res["psbt"])
     sign_and_broadcast(psbt)
     assert len(psbt.o) == 4
-    assert len(res["warnings"]) == 0
+    assert bitcoind.rpc.getmempoolentry(deposit_d)["ancestorsize"] == 165
+    assert bitcoind.rpc.getmempoolentry(deposit_d)["fees"]["ancestor"] * COIN == 165
+    # ancestor vsize at feerate 2 sat/vb = ancestor_fee / 2 = 165 / 2 = 82
+    # extra_weight <= (extra vsize * witness factor) = (165 - 82) * 4 = 332
+    # additional fee at 2 sat/vb (0.5 sat/wu) = 332 * 0.5 = 166
+    assert len(res["warnings"]) == 1
+    assert (
+        res["warnings"][0]
+        == "An additional fee of 166 sats has been added to pay for ancestors at the target feerate."
+    )
 
     # All the spent coins must have been detected as such
     all_deposits = (deposit_a, deposit_b, deposit_c, deposit_d)
@@ -249,84 +250,183 @@ def test_send_to_self(lianad, bitcoind):
 def test_coin_selection(lianad, bitcoind):
     """We can create a spend using coin selection."""
     # Send to an (external) address.
-    dest_100_000 = {bitcoind.rpc.getnewaddress(): 100_000}
+    dest_addr_1 = bitcoind.rpc.getnewaddress()
     # Coin selection is not possible if we have no coins.
     assert len(lianad.rpc.listcoins()["coins"]) == 0
-    assert "missing" in lianad.rpc.createspend(dest_100_000, [], 2)
+    assert "missing" in lianad.rpc.createspend({dest_addr_1: 100_000}, [], 2)
 
     # Receive a coin in an unconfirmed deposit transaction.
-    recv_addr = lianad.rpc.getnewaddress()["address"]
-    deposit = bitcoind.rpc.sendtoaddress(recv_addr, 0.0008)  # 80_000 sats
+    recv_addr_1 = lianad.rpc.getnewaddress()["address"]
+    deposit_1 = bitcoind.rpc.sendtoaddress(recv_addr_1, 0.0012)  # 120_000 sats
     wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 1)
-    # There are still no confirmed coins to use as candidates for selection.
+    # There are still no confirmed coins or unconfirmed change
+    # to use as candidates for selection.
     assert len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 0
     assert len(lianad.rpc.listcoins(["unconfirmed"])["coins"]) == 1
-    assert "missing" in lianad.rpc.createspend(dest_100_000, [], 2)
+    assert lianad.rpc.listcoins(["unconfirmed"])["coins"][0]["is_change"] is False
+    assert "missing" in lianad.rpc.createspend({dest_addr_1: 100_000}, [], 2)
 
     # Confirm coin.
-    bitcoind.generate_block(1, wait_for_mempool=deposit)
+    bitcoind.generate_block(1, wait_for_mempool=deposit_1)
     wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 1)
-
-    # Insufficient funds for coin selection.
-    assert "missing" in lianad.rpc.createspend(dest_100_000, [], 2)
-
-    # Reduce spend amount.
-    dest_30_000 = {bitcoind.rpc.getnewaddress(): 30_000}
-    res = lianad.rpc.createspend(dest_30_000, [], 2)
-    assert "psbt" in res
-
-    # The transaction must contain a change output.
-    spend_psbt = PSBT.from_base64(res["psbt"])
-    assert len(spend_psbt.o) == 2
-    assert len(spend_psbt.tx.vout) == 2
+    # Coin selection now succeeds.
+    spend_res_1 = lianad.rpc.createspend({dest_addr_1: 100_000}, [], 2)
+    assert "psbt" in spend_res_1
+    assert len(spend_res_1["warnings"]) == 0
+    # Increase spend amount and we have insufficient funds again even though we
+    # now have confirmed coins.
+    assert "missing" in lianad.rpc.createspend({dest_addr_1: 200_000}, [], 2)
+
+    # The transaction contains a change output.
+    spend_psbt_1 = PSBT.from_base64(spend_res_1["psbt"])
+    assert len(spend_psbt_1.o) == 2
+    assert len(spend_psbt_1.tx.vout) == 2
 
     # Sign and broadcast this Spend transaction.
-    signed_psbt = lianad.signer.sign_psbt(spend_psbt)
-    lianad.rpc.updatespend(signed_psbt.to_base64())
-    spend_txid = signed_psbt.tx.txid().hex()
-    lianad.rpc.broadcastspend(spend_txid)
-
+    spend_txid_1 = sign_and_broadcast_psbt(lianad, spend_psbt_1)
     wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 2)
-    coins = lianad.rpc.listcoins()["coins"]
     # Check that change output is unconfirmed.
     assert len(lianad.rpc.listcoins(["unconfirmed"])["coins"]) == 1
+    assert lianad.rpc.listcoins(["unconfirmed"])["coins"][0]["is_change"] is True
     assert len(lianad.rpc.listcoins(["spending"])["coins"]) == 1
-    # Check we cannot use coins as candidates if they are spending/spent or unconfirmed.
-    assert "missing" in lianad.rpc.createspend(dest_30_000, [], 2)
+    # We can use unconfirmed change as candidate.
+    # Depending on the feerate, we'll get a warning about paying extra for the ancestor.
+    dest_addr_2 = bitcoind.rpc.getnewaddress()
+    # If feerate is higher than ancestor, we'll need to pay extra.
+
+    # Try 10 sat/vb:
+    spend_res_2 = lianad.rpc.createspend({dest_addr_2: 10_000}, [], 10)
+    assert "psbt" in spend_res_2
+    spend_psbt_2 = PSBT.from_base64(spend_res_2["psbt"])
+    # 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]
+    )
+    assert bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"] == 161
+    assert bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN == 339
+    # ancestor vsize at feerate 10 sat/vb = ancestor_fee / 10 = 339 / 10 = 33
+    # extra_weight <= (extra vsize * witness factor) = (161 - 33) * 4 = 512
+    # additional fee at 10 sat/vb (2.5 sat/wu) = 512 * 2.5 = 1280
+    assert len(spend_res_2["warnings"]) == 1
+    assert (
+        spend_res_2["warnings"][0]
+        == "An additional fee of 1280 sats has been added to pay for ancestors at the target feerate."
+    )
 
-    # Now confirm the Spend.
-    bitcoind.generate_block(1, wait_for_mempool=spend_txid)
-    wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 1)
-    # But its value is not enough for this Spend.
-    dest_60_000 = {bitcoind.rpc.getnewaddress(): 60_000}
-    assert "missing" in lianad.rpc.createspend(dest_60_000, [], 2)
+    # Try 3 sat/vb:
+    spend_res_2 = lianad.rpc.createspend({dest_addr_2: 10_000}, [], 3)
+    assert "psbt" in spend_res_2
+    spend_psbt_2 = PSBT.from_base64(spend_res_2["psbt"])
+    # 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]
+    )
+    assert bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"] == 161
+    assert bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN == 339
+    # ancestor vsize at feerate 3 sat/vb = ancestor_fee / 3 = 339 / 3 = 113
+    # extra_weight <= (extra vsize * witness factor) = (161 - 113) * 4 = 192
+    # additional fee at 3 sat/vb (0.75 sat/wu) = 192 * 0.75 = 144
+    assert len(spend_res_2["warnings"]) == 1
+    assert (
+        spend_res_2["warnings"][0]
+        == "An additional fee of 144 sats has been added to pay for ancestors at the target 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)
+    assert "psbt" in spend_res_2
+    assert len(spend_res_2["warnings"]) == 0
+    spend_psbt_2 = PSBT.from_base64(spend_res_2["psbt"])
+    # 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]
+    )
 
     # Get another coin to check coin selection with more than one candidate.
-    recv_addr = lianad.rpc.getnewaddress()["address"]
-    deposit = bitcoind.rpc.sendtoaddress(recv_addr, 0.0002)  # 20_000 sats
-    bitcoind.generate_block(1, wait_for_mempool=deposit)
-    wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 2)
+    recv_addr_2 = lianad.rpc.getnewaddress()["address"]
+    deposit_2 = bitcoind.rpc.sendtoaddress(recv_addr_2, 30_000 / COIN)
+    wait_for(lambda: len(lianad.rpc.listcoins(["unconfirmed"])["coins"]) == 2)
+    assert (
+        len(
+            [
+                c
+                for c in lianad.rpc.listcoins(["unconfirmed"])["coins"]
+                if c["is_change"]
+            ]
+        )
+        == 1
+    )
+    dest_addr_3 = bitcoind.rpc.getnewaddress()
+    # As only one unconfirmed coin is change, we have insufficient funds.
+    assert "missing" in lianad.rpc.createspend({dest_addr_3: 20_000}, [], 10)
+
+    # If we include both unconfirmed coins manually, it will succeed.
+    # We'll need to pay extra for each unconfirmed coin's ancestors.
+    outpoints = [c["outpoint"] for c in lianad.rpc.listcoins(["unconfirmed"])["coins"]]
+
+    spend_res_3 = lianad.rpc.createspend({dest_addr_3: 20_000}, outpoints, 10)
+    assert "psbt" in spend_res_3
+    assert bitcoind.rpc.getmempoolentry(deposit_2)["ancestorsize"] == 165
+    assert bitcoind.rpc.getmempoolentry(deposit_2)["fees"]["ancestor"] * COIN == 165
+    # From above, extra fee for unconfirmed change at 10 sat/vb = 1280.
+    # For unconfirmed non-change:
+    # ancestor vsize at feerate 10 sat/vb = ancestor_fee / 10 = 165 / 10 = 16
+    # extra_weight <= (extra vsize * witness factor) = (165 - 16) * 4 = 596
+    # additional fee at 10 sat/vb (2.5 sat/wu) = 596 * 2.5 = 1490
+    # Sum of extra ancestor fees = 1280 + 1490 = 2770.
+    assert len(spend_res_3["warnings"]) == 1
+    assert (
+        spend_res_3["warnings"][0]
+        == "An additional fee of 2770 sats has been added to pay for ancestors at the target feerate."
+    )
+    spend_psbt_3 = PSBT.from_base64(spend_res_3["psbt"])
+    spend_txid_3 = sign_and_broadcast_psbt(lianad, spend_psbt_3)
+    mempool_txid_3 = bitcoind.rpc.getmempoolentry(spend_txid_3)
+    # The effective feerate of new transaction plus ancestor matches the target.
+    # Note that in the mempool entry, "ancestor" includes spend_txid_3 itself.
+    assert (
+        mempool_txid_3["fees"]["ancestor"] * COIN // mempool_txid_3["ancestorsize"]
+        == 10
+    )
+    # 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
+
+    # Now confirm the spend.
+    bitcoind.generate_block(1, wait_for_mempool=spend_txid_3)
+    wait_for(lambda: len(lianad.rpc.listcoins(["confirmed"])["coins"]) == 1)
 
-    res = lianad.rpc.createspend(dest_60_000, [], 2)
-    assert "psbt" in res
+    # Now create the same spend with auto and manual selection:
+    dest_addr_4 = bitcoind.rpc.getnewaddress()
+    spend_res_4 = lianad.rpc.createspend({dest_addr_4: 15_000}, [], 2)
+    assert "psbt" in spend_res_4
+    assert len(spend_res_4["warnings"]) == 0
 
-    # The transaction must contain a change output.
-    auto_psbt = PSBT.from_base64(res["psbt"])
-    assert len(auto_psbt.o) == 2
-    assert len(auto_psbt.tx.vout) == 2
+    # The transaction contains a change output.
+    spend_psbt_4 = PSBT.from_base64(spend_res_4["psbt"])
+    assert len(spend_psbt_4.i) == 1
+    assert len(spend_psbt_4.o) == 2
+    assert len(spend_psbt_4.tx.vout) == 2
 
     # Now create a transaction with manual coin selection using the same outpoints.
     outpoints = [
-        f"{txin.prevout.hash:064x}:{txin.prevout.n}" for txin in auto_psbt.tx.vin
+        f"{txin.prevout.hash:064x}:{txin.prevout.n}" for txin in spend_psbt_4.tx.vin
     ]
-    res_manual = lianad.rpc.createspend(dest_60_000, outpoints, 2)
-    manual_psbt = PSBT.from_base64(res_manual["psbt"])
+    assert len(outpoints) > 0
+    res_manual = lianad.rpc.createspend({dest_addr_4: 15_000}, outpoints, 2)
+    assert len(res_manual["warnings"]) == 0
+    psbt_manual = PSBT.from_base64(res_manual["psbt"])
 
     # Recipient details are the same for both.
-    assert auto_psbt.tx.vout[0].nValue == manual_psbt.tx.vout[0].nValue
-    assert auto_psbt.tx.vout[0].scriptPubKey == manual_psbt.tx.vout[0].scriptPubKey
-    # Change amount is the same (change address will be different).
-    assert auto_psbt.tx.vout[1].nValue == manual_psbt.tx.vout[1].nValue
+    assert spend_psbt_4.tx.vout[0].nValue == psbt_manual.tx.vout[0].nValue
+    assert spend_psbt_4.tx.vout[0].scriptPubKey == psbt_manual.tx.vout[0].scriptPubKey
+    # Change details are also the same
+    # (change address is same as neither transaction has been broadcast)
+    assert spend_psbt_4.tx.vout[1].nValue == psbt_manual.tx.vout[1].nValue
+    assert spend_psbt_4.tx.vout[1].scriptPubKey == psbt_manual.tx.vout[1].scriptPubKey
 
 
 def test_coin_selection_changeless(lianad, bitcoind):