From ec0009113a8405396950f5c347d0dace3c3c33d0 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Fri, 24 Mar 2023 19:16:58 +0100 Subject: [PATCH] descriptors: lift the bounds to create a Liana policy Since we now use the Miniscript compiler, there is no reason not to allow more than 20 keys. --- src/descriptors/analysis.rs | 64 +++++++++++++++++++++---------------- src/descriptors/mod.rs | 22 +++++-------- 2 files changed, 45 insertions(+), 41 deletions(-) diff --git a/src/descriptors/analysis.rs b/src/descriptors/analysis.rs index 1f2f04b0..4d9b3960 100644 --- a/src/descriptors/analysis.rs +++ b/src/descriptors/analysis.rs @@ -1,7 +1,7 @@ use miniscript::{ bitcoin::{util::bip32, Sequence}, descriptor, - policy::{Concrete as ConcretePolicy, Liftable, Semantic as SemanticPolicy}, + policy::{compiler, Concrete as ConcretePolicy, Liftable, Semantic as SemanticPolicy}, ScriptContext, }; @@ -19,6 +19,9 @@ pub enum LianaPolicyError { InvalidMultiThresh(usize), InvalidMultiKeys(usize), IncompatibleDesc, + /// The spending policy is not a valid Miniscript policy: it may for instance be malleable, or + /// overflow some limit. + InvalidPolicy(compiler::CompilerError), } impl std::fmt::Display for LianaPolicyError { @@ -43,6 +46,7 @@ impl std::fmt::Display for LianaPolicyError { f, "Descriptor is not compatible with a Liana spending policy." ), + Self::InvalidPolicy(e) => write!(f, "Invalid Miniscript policy: {}", e), } } } @@ -303,6 +307,8 @@ impl PathInfo { /// A Liana spending policy. Can be created from some settings (the primary and recovery keys, the /// timelock(s)) and be used to derive a descriptor. It can also be inferred from a descriptor and /// be used to retrieve the settings. +/// Do note however that the descriptor generation process is not deterministic, therefore you +/// **cannot roundtrip** a descriptor through a `LianaPolicy`. #[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] pub struct LianaPolicy { pub(super) primary_path: PathInfo, @@ -327,18 +333,6 @@ impl LianaPolicy { return Err(LianaPolicyError::InsaneTimelock(recovery_timelock as u32)); } - // If any of the paths is a multisig, make sure they are within the CHECKMULTISIG bounds. - for path_info in &[&primary_path, &recovery_path] { - if let PathInfo::Multi(thresh, keys) = path_info { - if keys.len() < 2 || keys.len() > 20 { - return Err(LianaPolicyError::InvalidMultiKeys(keys.len())); - } - if thresh == &0 || thresh > &keys.len() { - return Err(LianaPolicyError::InvalidMultiThresh(*thresh)); - } - } - } - // Check all keys are valid according to our standard (this checks all are multipath keys). let (prim_keys, rec_keys) = (primary_path.keys(), recovery_path.keys()); let all_keys = prim_keys.iter().chain(rec_keys.iter()); @@ -346,7 +340,8 @@ impl LianaPolicy { return Err(LianaPolicyError::InvalidKey((*key).clone().into())); } - // Check for key duplicates. They are invalid in (nonmalleable) miniscripts. + // Check for key duplicates. They are invalid in (nonmalleable) miniscripts. This is + // checked by the Miniscript policy compiler too but not at the raw xpub level. let mut key_set = HashSet::new(); for key in all_keys { let xpub = match key { @@ -360,10 +355,13 @@ impl LianaPolicy { } assert!(!key_set.is_empty()); - Ok(LianaPolicy { + // Make sure it is a valid Miniscript policy by (ab)using the compiler. + let policy = LianaPolicy { primary_path, recovery_path: (recovery_timelock, recovery_path), - }) + }; + policy.clone().into_miniscript()?; + Ok(policy) } /// Create a Liana policy from a descriptor. This will check the descriptor is correctly formed @@ -439,10 +437,10 @@ impl LianaPolicy { } } - Ok(LianaPolicy { - primary_path: primary_path.ok_or(LianaPolicyError::IncompatibleDesc)?, - recovery_path: recovery_path.ok_or(LianaPolicyError::IncompatibleDesc)?, - }) + // Use the constructor for the sanity checks (especially around the Miniscript policy). + let prim_path = primary_path.ok_or(LianaPolicyError::IncompatibleDesc)?; + let (timelock, reco_path) = recovery_path.ok_or(LianaPolicyError::IncompatibleDesc)?; + LianaPolicy::new(prim_path, reco_path, timelock) } pub fn primary_path(&self) -> &PathInfo { @@ -454,12 +452,12 @@ impl LianaPolicy { (self.recovery_path.0, &self.recovery_path.1) } - /// Create a descriptor from this spending policy with multipath key expressions. - /// - /// Although for now this function is deterministic, it **will not** be in the future. - pub fn into_multipath_descriptor( + fn into_miniscript( self, - ) -> descriptor::Descriptor { + ) -> Result< + miniscript::Miniscript, + LianaPolicyError, + > { let LianaPolicy { primary_path, recovery_path: (timelock, recovery_path), @@ -475,9 +473,21 @@ impl LianaPolicy { let primary_keys = primary_path.into_ms_policy(); let tl_policy = ConcretePolicy::Or(vec![(99, primary_keys), (1, recovery_branch)]); - let ms = tl_policy + tl_policy .compile::() - .expect("Compilation must never fail, nothing overflows."); + .map_err(LianaPolicyError::InvalidPolicy) + } + + /// Create a descriptor from this spending policy with multipath key expressions. Note this + /// involves a Miniscript policy compilation: this function is **not deterministic**. If you + /// are inferring a `LianaPolicy` from a descriptor, generating a descriptor from this + /// `LianaPolicy` may not yield the same descriptor. + pub fn into_multipath_descriptor( + self, + ) -> descriptor::Descriptor { + let ms = self + .into_miniscript() + .expect("This is always checked when creating a LianaPolicy."); miniscript::Segwitv0::check_local_validity(&ms).expect("Miniscript must be sane"); descriptor::Descriptor::Wsh(descriptor::Wsh::new(ms).expect("Must pass sanity checks")) } diff --git a/src/descriptors/mod.rs b/src/descriptors/mod.rs index c6767de3..bf73e0a1 100644 --- a/src/descriptors/mod.rs +++ b/src/descriptors/mod.rs @@ -584,17 +584,11 @@ mod tests { descriptor::DescriptorPublicKey::from_str(&xpub_str).unwrap() }; let prim_path = PathInfo::Single(random_desc_key()); - let twenty_keys: Vec = - (0..20).map(|_| random_desc_key()).collect(); - let mut twenty_one_keys = twenty_keys.clone(); - twenty_one_keys.push(random_desc_key()); + let twenty_eight_keys: Vec = + (0..28).map(|_| random_desc_key()).collect(); + let mut twenty_nine_keys = twenty_eight_keys.clone(); + twenty_nine_keys.push(random_desc_key()); - LianaPolicy::new( - prim_path.clone(), - PathInfo::Multi(1, vec![random_desc_key()]), - 1, - ) - .unwrap_err(); LianaPolicy::new( prim_path.clone(), PathInfo::Multi(2, vec![random_desc_key()]), @@ -627,12 +621,12 @@ mod tests { .unwrap_err(); LianaPolicy::new( prim_path.clone(), - PathInfo::Multi(3, twenty_keys.clone()), + PathInfo::Multi(3, twenty_eight_keys.clone()), 1, ) .unwrap(); - LianaPolicy::new(prim_path.clone(), PathInfo::Multi(20, twenty_keys), 1).unwrap(); - LianaPolicy::new(prim_path, PathInfo::Multi(20, twenty_one_keys), 1).unwrap_err(); + LianaPolicy::new(prim_path.clone(), PathInfo::Multi(20, twenty_eight_keys), 1).unwrap(); + LianaPolicy::new(prim_path, PathInfo::Multi(20, twenty_nine_keys), 1).unwrap_err(); } fn roundtrip(desc_str: &str) { @@ -649,7 +643,7 @@ mod tests { // A single key as primary path, a multisig as recovery roundtrip("wsh(or_d(pk([aabbccdd]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*),and_v(v:multi(2,[aabbccdd]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*,[aabbccdd]xpub6AA2N8RALRYgLD6jT1iXYCEDkndTeZndMtWPbtNX6sY5dPiLtf2T88ahdxrGXMUPoNadgR86sFhBXWQVgifPzDYbY9ZtwK4gqzx4y5Da1DW/<0;1>/*,[aabb0011/10/4893]xpub6AyxexvxizZJffF153evmfqHcE9MV88fCNCAtP3jQjXJHwrAKri71Tq9jWUkPxj9pja4u6AkCPHY7atgxzSEa2HtDwJfrRWKK4fsfQg4o77/<0;1>/*),older(26352))))#7du8x4v7"); // The other way around - roundtrip("wsh(or_d(multi(3,[aabbccdd]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*,[aabb0011/10/4893]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*,[aabbccdd]xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/<0;1>/*),and_v(v:pk([aabbccdd]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*),older(26352))))#y9l4ldvr"); + roundtrip("wsh(or_d(multi(3,[aabbccdd]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*,[aabb0011/10/4893]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*,[aabbccdd]xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/<0;1>/*),and_v(v:pk([aabbccdd]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*),older(26352))))#0y77q9d6"); } fn psbt_from_str(psbt_str: &str) -> Psbt {