scripts: require minimal CSV

We don't use the bits without consensus meaning, so better to not create
confusion by always having the numeric value passed to the descriptor
represent the minimum number of blocks needed to unlock the coin, no
matter what.

Signed-off-by: Antoine Poinsot <darosior@protonmail.com>

src/scripts.rs

@@ -30,6 +30,10 @@ use std::{
 #[cfg(feature = "use-serde")]
 use serde::de;
 
+/// Flag applied to the nSequence and CSV value before comparing them.
+/// https://github.com/bitcoin/bitcoin/blob/4a540683ec40393d6369da1a9e02e45614db936d/src/primitives/transaction.h#L87-L89
+pub const SEQUENCE_LOCKTIME_MASK: u32 = 0x00_00_ff_ff;
+
 // These are useful to create TxOuts out of the right Script descriptor
 
 macro_rules! impl_descriptor_newtype {
@@ -135,9 +139,13 @@ macro_rules! unvault_desc_checks {
             return Err(ScriptCreationError::BadParameters);
         }
 
-        // We require the locktime to be in number of blocks, and of course to not be disabled.
+        // We require the locktime to:
+        //  - not be disabled
+        //  - be in number of blocks
+        //  - be 'clean' / minimal, ie all bits without consensus meaning should be 0
         if ($csv_value & SEQUENCE_LOCKTIME_DISABLE_FLAG) != 0
             || ($csv_value & SEQUENCE_LOCKTIME_TYPE_FLAG) != 0
+            || ($csv_value & SEQUENCE_LOCKTIME_MASK) != $csv_value
         {
             return Err(ScriptCreationError::BadParameters);
         }

src/transactions.rs

@@ -724,11 +724,16 @@ impl UnvaultTransaction {
     }
 
     /// Get the Unvault txo to be referenced in a spending transaction
+    ///
+    /// # Panic
+    /// Will panic if passed a csv higher than
+    /// [SEQUENCE_LOCKTIME_MASK](crate::scripts::SEQUENCE_LOCKTIME_MASK)
     pub fn spend_unvault_txin(
         &self,
         unvault_descriptor: &DerivedUnvaultDescriptor,
         csv: u32,
     ) -> UnvaultTxIn {
+        assert!(csv <= SEQUENCE_LOCKTIME_MASK, "{}", csv);
         self.unvault_txin(unvault_descriptor, csv)
     }
 
@@ -1636,7 +1641,7 @@ mod tests {
     #[test]
     fn transaction_derivation() {
         let secp = secp256k1::Secp256k1::new();
-        let csv = fastrand::u32(..1 << 22);
+        let csv = fastrand::u32(..SEQUENCE_LOCKTIME_MASK);
         eprintln!("Using a CSV of '{}'", csv);
 
         // Test the dust limit
@@ -1646,6 +1651,10 @@ mod tests {
                 .to_string(),
             Error::TransactionCreation(TransactionCreationError::Dust).to_string()
         );
+        // Non-minimal CSV
+        derive_transactions(2, 1, SEQUENCE_LOCKTIME_MASK + 1, 300_000, &secp)
+            .expect_err("Unclean CSV");
+
         // Absolute minimum
         derive_transactions(2, 1, csv, 234_632, &secp).expect(&format!(
             "Tx chain with 2 stakeholders, 1 manager, {} csv, 235_250 deposit",
@@ -1692,8 +1701,7 @@ mod tests {
             managers.len(),
             cosigners.clone(),
             csv,
-        )
-        .expect("Unvault descriptor generation error");
+        )?;
         let cpfp_descriptor =
             CpfpDescriptor::new(managers).expect("Unvault CPFP descriptor generation error");
         let deposit_descriptor =