fix(wallet): return error instead of panicking on invalid current_height

Author: none34829

src/wallet/error.rs

@@ -215,6 +215,10 @@ pub enum CreateTxError {
     MissingNonWitnessUtxo(OutPoint),
     /// Miniscript PSBT error
     MiniscriptPsbt(MiniscriptPsbtError),
+    /// Provided height is not a valid block height for use as a locktime
+    ///
+    /// Block-based locktimes require a height below 500_000_000.
+    InvalidCurrentHeight(u32),
 }
 
 impl fmt::Display for CreateTxError {
@@ -281,6 +285,12 @@ impl fmt::Display for CreateTxError {
             CreateTxError::MiniscriptPsbt(err) => {
                 write!(f, "Miniscript PSBT error: {err}")
             }
+            CreateTxError::InvalidCurrentHeight(height) => {
+                write!(
+                    f,
+                    "Cannot use height {height} as a locktime (must be below 500_000_000)"
+                )
+            }
         }
     }
 }

src/wallet/mod.rs

@@ -1303,13 +1303,13 @@ impl Wallet {
         // We use a match here instead of a unwrap_or_else as it's way more readable :)
         let current_height = match params.current_height {
             // If they didn't tell us the current height, we assume it's the latest sync height.
-            None => {
-                let tip_height = self.chain.tip().height();
-                absolute::LockTime::from_height(tip_height).expect("invalid height")
-            }
+            None => self.chain.tip().height(),
             Some(h) => h,
         };
 
+        let current_height = absolute::LockTime::from_height(current_height)
+            .map_err(|_| CreateTxError::InvalidCurrentHeight(current_height))?;
+
         let lock_time = match params.locktime {
             // When no `nLockTime` is specified, we try to prevent fee sniping, if possible.
             None => {

src/wallet/tx_builder.rs

@@ -138,7 +138,7 @@ pub(crate) struct TxParams {
     pub(crate) only_witness_utxo: bool,
     pub(crate) add_global_xpubs: bool,
     pub(crate) bumping_fee: Option<PreviousFee>,
-    pub(crate) current_height: Option<absolute::LockTime>,
+    pub(crate) current_height: Option<u32>,
     pub(crate) allow_dust: bool,
 }
 
@@ -650,8 +650,7 @@ impl<'a, Cs> TxBuilder<'a, Cs> {
     ///
     /// In both cases, if you don't provide a current height, we use the last sync height.
     pub fn current_height(&mut self, height: u32) -> &mut Self {
-        self.params.current_height =
-            Some(absolute::LockTime::from_height(height).expect("Invalid height"));
+        self.params.current_height = Some(height);
         self
     }
 

tests/wallet.rs

@@ -308,6 +308,26 @@ fn test_create_tx_custom_locktime() {
     assert_eq!(psbt.unsigned_tx.lock_time.to_consensus_u32(), 630_000);
 }
 
+#[test]
+fn test_create_tx_invalid_current_height_returns_error() {
+    // `absolute::LockTime::from_height` rejects heights >= 500_000_000 because the
+    // consensus encoding of nLockTime uses those values to mean "unix timestamp" rather
+    // than "block height". Passing such a value to `TxBuilder::current_height` used to
+    // panic; it should now surface as a typed error.
+    let (mut wallet, _) = get_funded_wallet_wpkh();
+    let addr = wallet.next_unused_address(KeychainKind::External);
+
+    let mut builder = wallet.build_tx();
+    builder
+        .add_recipient(addr.script_pubkey(), Amount::from_sat(25_000))
+        .current_height(500_000_000);
+
+    assert_matches!(
+        builder.finish(),
+        Err(CreateTxError::InvalidCurrentHeight(500_000_000))
+    );
+}
+
 #[test]
 fn test_create_tx_custom_locktime_compatible_with_cltv() {
     let (mut wallet, _) = get_funded_wallet_single(get_test_single_sig_cltv());