fix(wallet): treat unreachable relative locktimes as unsatisfied

src/descriptor/policy.rs

@@ -64,7 +64,7 @@ use crate::descriptor::ExtractPolicy;
 use crate::keys::ExtScriptContext;
 use crate::types::IndexOutOfBoundsError;
 use crate::wallet::signer::{SignerId, SignersContainer};
-use crate::wallet::utils::{After, Older, SecpCtx};
+use crate::wallet::utils::{After, ConfirmationHeight, Older, SecpCtx};
 
 use super::XKeyUtils;
 use super::checksum::calc_checksum;
@@ -980,7 +980,11 @@ impl<Ctx: ScriptContext + 'static> ExtractPolicy for Miniscript<DescriptorPublic
                     psbt,
                 } = build_sat
                 {
-                    let older = Older::new(Some(current_height), Some(input_max_height), false);
+                    let older = Older::new(
+                        Some(current_height),
+                        Some(ConfirmationHeight::Confirmed(input_max_height)),
+                        false,
+                    );
                     let older_sat =
                         Satisfier::<bitcoin::PublicKey>::check_older(&older, (*value).into());
                     let inputs_sat = psbt_inputs_sat(psbt).all(|sat| {

src/wallet/mod.rs

@@ -76,7 +76,7 @@ use crate::wallet::{
     error::{BuildFeeBumpError, CreateTxError, MiniscriptPsbtError},
     signer::{SignOptions, SignerError, SignerOrdering, SignersContainer, TransactionSigner},
     tx_builder::{FeePolicy, TxBuilder, TxParams},
-    utils::{After, Older, SecpCtx, check_nsequence_rbf},
+    utils::{After, ConfirmationHeight, Older, SecpCtx, check_nsequence_rbf},
 };
 // Unstable bdk-tx imports — only active with --cfg bdk_wallet_unstable and feature = "bdk-tx".
 #[cfg(all(bdk_wallet_unstable, feature = "bdk-tx", feature = "std"))]
@@ -2017,11 +2017,13 @@ impl Wallet {
             .map(|canon_tx| {
                 let txid = canon_tx.tx_node.txid;
                 match canon_tx.chain_position {
-                    ChainPosition::Confirmed { anchor, .. } => (txid, anchor.block_id.height),
-                    ChainPosition::Unconfirmed { .. } => (txid, u32::MAX),
+                    ChainPosition::Confirmed { anchor, .. } => {
+                        (txid, ConfirmationHeight::Confirmed(anchor.block_id.height))
+                    }
+                    ChainPosition::Unconfirmed { .. } => (txid, ConfirmationHeight::Unconfirmed),
                 }
             })
-            .collect::<HashMap<Txid, u32>>();
+            .collect::<HashMap<Txid, ConfirmationHeight>>();
         let current_height = sign_options
             .assume_height
             .unwrap_or_else(|| self.chain.tip().height());
@@ -2066,7 +2068,7 @@ impl Wallet {
         clear_output_derivations: bool,
     ) -> Result<FinalizePsbtOutcome, IndexOutOfBoundsError>
     where
-        F: FnMut(usize, &bitcoin::TxIn) -> Option<u32>,
+        F: FnMut(usize, &bitcoin::TxIn) -> Option<ConfirmationHeight>,
     {
         let tx = &psbt.unsigned_tx;
         if psbt.inputs.len() < tx.input.len() {

src/wallet/utils.rs

@@ -85,16 +85,25 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk> for After {
     }
 }
 
+/// Chain position of the transaction that created an input, as seen by [`Older`].
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) enum ConfirmationHeight {
+    /// The transaction is confirmed at this height.
+    Confirmed(u32),
+    /// The transaction is not confirmed yet.
+    Unconfirmed,
+}
+
 pub struct Older {
     pub current_height: Option<u32>,
-    pub create_height: Option<u32>,
+    pub create_height: Option<ConfirmationHeight>,
     pub assume_height_reached: bool,
 }
 
 impl Older {
     pub(crate) fn new(
         current_height: Option<u32>,
-        create_height: Option<u32>,
+        create_height: Option<ConfirmationHeight>,
         assume_height_reached: bool,
     ) -> Older {
         Older {
@@ -109,12 +118,18 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk> for Older {
     fn check_older(&self, n: relative::LockTime) -> bool {
         if let Some(current_height) = self.current_height {
             // TODO: test >= / >
-            current_height
-                >= self
-                    .create_height
-                    .unwrap_or(0)
+            match self
+                .create_height
+                .unwrap_or(ConfirmationHeight::Confirmed(0))
+            {
+                // A height that does not fit in a `u32` can never be reached.
+                ConfirmationHeight::Confirmed(create_height) => create_height
                     .checked_add(n.to_consensus_u32())
-                    .expect("Overflowing addition")
+                    .is_some_and(|satisfaction_height| current_height >= satisfaction_height),
+                // A relative locktime counts from the block that confirms the previous
+                // transaction, so it cannot be met while that transaction is unconfirmed.
+                ConfirmationHeight::Unconfirmed => false,
+            }
         } else {
             self.assume_height_reached
         }
@@ -170,10 +185,11 @@ mod test {
     // otherwise it's time-based
     pub(crate) const SEQUENCE_LOCKTIME_TYPE_FLAG: u32 = 1 << 22;
 
-    use super::{IsDust, check_nsequence_rbf, shuffle_slice};
-    use crate::bitcoin::{Address, Network, Sequence};
+    use super::{ConfirmationHeight, IsDust, Older, check_nsequence_rbf, shuffle_slice};
+    use crate::bitcoin::{Address, Network, PublicKey, Sequence, relative};
     use alloc::vec::Vec;
     use core::str::FromStr;
+    use miniscript::Satisfier;
     use rand::{SeedableRng, rngs::StdRng, thread_rng};
 
     #[test]
@@ -278,4 +294,56 @@ mod test {
         shuffle_slice(&mut test, &mut rng);
         assert_eq!(test, &[0, 4, 1, 2, 5]);
     }
+
+    #[test]
+    fn test_check_older_compares_against_the_satisfaction_height() {
+        let older = Older::new(Some(300), Some(ConfirmationHeight::Confirmed(100)), false);
+        assert!(<Older as Satisfier<PublicKey>>::check_older(
+            &older,
+            relative::LockTime::from_height(144)
+        ));
+
+        let older = Older::new(Some(200), Some(ConfirmationHeight::Confirmed(100)), false);
+        assert!(!<Older as Satisfier<PublicKey>>::check_older(
+            &older,
+            relative::LockTime::from_height(144)
+        ));
+    }
+
+    #[test]
+    fn test_check_older_unconfirmed_is_not_satisfied() {
+        let older = Older::new(Some(u32::MAX), Some(ConfirmationHeight::Unconfirmed), false);
+        assert!(!<Older as Satisfier<PublicKey>>::check_older(
+            &older,
+            relative::LockTime::from_height(144)
+        ));
+    }
+
+    #[test]
+    fn test_check_older_unreachable_satisfaction_height_is_not_satisfied() {
+        let older = Older::new(
+            Some(u32::MAX),
+            Some(ConfirmationHeight::Confirmed(u32::MAX)),
+            false,
+        );
+        assert!(!<Older as Satisfier<PublicKey>>::check_older(
+            &older,
+            relative::LockTime::from_height(144)
+        ));
+    }
+
+    #[test]
+    fn test_check_older_not_found_counts_from_height_zero() {
+        let older = Older::new(Some(144), None, false);
+        assert!(<Older as Satisfier<PublicKey>>::check_older(
+            &older,
+            relative::LockTime::from_height(144)
+        ));
+
+        let older = Older::new(Some(143), None, false);
+        assert!(!<Older as Satisfier<PublicKey>>::check_older(
+            &older,
+            relative::LockTime::from_height(144)
+        ));
+    }
 }

tests/wallet.rs

@@ -2038,6 +2038,54 @@ fn test_try_finalize_psbt_uses_psbt_timelocks() {
     }
 }
 
+#[test]
+fn test_finalize_psbt_with_unconfirmed_input_and_unused_csv_branch() {
+    // `wsh(or_d(pk(A),and_v(v:pk(B),older(144))))`. Spending through the `pk(A)` branch leaves
+    // the `older(144)` branch for the finalizer to evaluate while the previous transaction is
+    // still unconfirmed. `finalize_psbt` used to stand in `u32::MAX` for the confirmation height
+    // of an unconfirmed transaction, and adding the relative locktime to it panicked.
+    let descriptor = get_test_a_or_b_plus_csv();
+    let mut wallet = Wallet::create_single(descriptor)
+        .network(Network::Regtest)
+        .create_wallet_no_persist()
+        .unwrap();
+    let addr = wallet.next_unused_address(KeychainKind::External);
+
+    // Fund the wallet with a transaction that stays unconfirmed.
+    let funding_tx = Transaction {
+        output: vec![TxOut {
+            value: Amount::from_sat(50_000),
+            script_pubkey: addr.script_pubkey(),
+        }],
+        ..new_tx(0)
+    };
+    insert_tx(&mut wallet, funding_tx);
+
+    let policy = wallet
+        .public_descriptor(KeychainKind::External)
+        .extract_policy(
+            &SignersContainer::default(),
+            BuildSatisfaction::None,
+            wallet.secp_ctx(),
+        )
+        .unwrap()
+        .expect("descriptor has a spending policy");
+    // Child #0 is `pk(A)`, so the `older(144)` branch is not the one being used.
+    let path = [(policy.id.clone(), vec![0])].into_iter().collect();
+    let condition = policy.get_condition(&path).unwrap();
+
+    let mut builder = wallet.build_tx();
+    builder
+        .add_recipient(addr.script_pubkey(), Amount::from_sat(10_000))
+        .set_condition(condition);
+    let mut psbt = builder.finish().unwrap();
+
+    let finalized = wallet
+        .finalize_psbt(&mut psbt, SignOptions::default())
+        .unwrap();
+    assert!(!finalized, "an unsigned PSBT cannot be finalized");
+}
+
 #[test]
 fn test_taproot_try_finalize_sign_option() {
     let descriptor = get_test_tr_with_taptree();