From 55d86c62a59bcfe632b866d1b24653219f4c4399 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Fri, 24 Mar 2023 18:46:25 +0100 Subject: [PATCH 1/9] descriptors: use the Miniscript compiler to create a descriptor --- Cargo.toml | 2 +- src/descriptors/analysis.rs | 80 +++++++++++-------------------------- src/descriptors/mod.rs | 2 +- 3 files changed, 25 insertions(+), 59 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 15dd0bf1..c6c4b1b6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,7 +25,7 @@ daemon = ["libc"] [dependencies] # For managing transactions (it re-exports the bitcoin crate) -miniscript = { git = "https://github.com/darosior/rust-miniscript", branch = "multipath_descriptors_on_9.0", features = ["serde"] } +miniscript = { git = "https://github.com/darosior/rust-miniscript", branch = "multipath_descriptors_on_9.0", features = ["serde", "compiler"] } # Don't reinvent the wheel dirs = "5.0" diff --git a/src/descriptors/analysis.rs b/src/descriptors/analysis.rs index c34ea2e6..1f2f04b0 100644 --- a/src/descriptors/analysis.rs +++ b/src/descriptors/analysis.rs @@ -1,14 +1,14 @@ use miniscript::{ bitcoin::{util::bip32, Sequence}, descriptor, - policy::{Liftable, Semantic as SemanticPolicy}, - Miniscript, ScriptContext, Terminal, + policy::{Concrete as ConcretePolicy, Liftable, Semantic as SemanticPolicy}, + ScriptContext, }; use std::{ collections::{HashMap, HashSet}, convert::TryFrom, - error, fmt, sync, + error, fmt, }; #[derive(Debug)] @@ -288,33 +288,14 @@ impl PathInfo { } } - /// Returns `None` if it is a multisig that does not fit inside a CHECKMULTISIG. - pub fn into_miniscript( - self, - as_hash: bool, - ) -> Option> { + /// Get a Miniscript Policy for this path. + pub fn into_ms_policy(self) -> ConcretePolicy { match self { - PathInfo::Single(key) => Some( - Miniscript::from_ast(Terminal::Check(sync::Arc::from( - Miniscript::from_ast(if as_hash { - Terminal::PkH(key) - } else { - Terminal::PkK(key) - }) - .expect("pk_k is a valid Miniscript"), - ))) - .expect("Well typed"), + PathInfo::Single(key) => ConcretePolicy::Key(key), + PathInfo::Multi(thresh, keys) => ConcretePolicy::Threshold( + thresh, + keys.into_iter().map(ConcretePolicy::Key).collect(), ), - PathInfo::Multi(thresh, keys) => { - if thresh < 1 || keys.len() > 20 || thresh > keys.len() { - None - } else { - Some( - Miniscript::from_ast(Terminal::Multi(thresh, keys)) - .expect("multi is a valid Miniscript"), - ) - } - } } } } @@ -484,36 +465,21 @@ impl LianaPolicy { recovery_path: (timelock, recovery_path), } = self; - // Create the timelocked spending path. If there is a single key we make it a pk_h() in - // order to save on the script size (since we assume the timelocked recovery path will - // seldom be used). - let recovery_timelock = Terminal::Older(Sequence::from_height(timelock)); - let recovery_keys = recovery_path - .into_miniscript(true) - .expect("We check the multisig never overflows in our constructors."); - let recovery_branch = Miniscript::from_ast(Terminal::AndV( - Miniscript::from_ast(Terminal::Verify(recovery_keys.into())) - .expect("Well typed") - .into(), - Miniscript::from_ast(recovery_timelock) - .expect("Well typed") - .into(), - )) - .expect("Well typed"); + // Create the timelocked recovery spending path. + let recovery_timelock = ConcretePolicy::Older(Sequence::from_height(timelock)); + let recovery_keys = recovery_path.into_ms_policy(); + let recovery_branch = ConcretePolicy::And(vec![recovery_keys, recovery_timelock]); - // Combine the timelocked spending path with the simple "primary" path. For the primary key - // we don't use a pkh since it's the one that will likely always be used. - let primary_keys = primary_path - .into_miniscript(false) - .expect("We check the multisig never overflows in our constructors."); - let tl_miniscript = - Miniscript::from_ast(Terminal::OrD(primary_keys.into(), recovery_branch.into())) - .expect("Well typed"); - miniscript::Segwitv0::check_local_validity(&tl_miniscript) - .expect("Miniscript must be sane"); - descriptor::Descriptor::Wsh( - descriptor::Wsh::new(tl_miniscript).expect("Must pass sanity checks"), - ) + // Create the primary spending path and combine both, assuming the recovery path will + // seldom be used. + let primary_keys = primary_path.into_ms_policy(); + let tl_policy = ConcretePolicy::Or(vec![(99, primary_keys), (1, recovery_branch)]); + + let ms = tl_policy + .compile::() + .expect("Compilation must never fail, nothing overflows."); + 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 b8d169ea..c6767de3 100644 --- a/src/descriptors/mod.rs +++ b/src/descriptors/mod.rs @@ -421,7 +421,7 @@ mod tests { ], ); let policy = LianaPolicy::new(primary_keys, recovery_keys, 26352).unwrap(); - assert_eq!(LianaDescriptor::new(policy).to_string(), "wsh(or_d(multi(3,[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*,[aabb0011/10/4893]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*,[abcdef01]xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/<0;1>/*),and_v(v:multi(2,[abcdef01]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*,[abcdef01]xpub6AA2N8RALRYgLD6jT1iXYCEDkndTeZndMtWPbtNX6sY5dPiLtf2T88ahdxrGXMUPoNadgR86sFhBXWQVgifPzDYbY9ZtwK4gqzx4y5Da1DW/<0;1>/*,[aabb0011/10/4893]xpub6AyxexvxizZJffF153evmfqHcE9MV88fCNCAtP3jQjXJHwrAKri71Tq9jWUkPxj9pja4u6AkCPHY7atgxzSEa2HtDwJfrRWKK4fsfQg4o77/<0;1>/*),older(26352))))#s0zsa6uc"); + assert_eq!(LianaDescriptor::new(policy).to_string(), "wsh(or_d(multi(3,[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*,[aabb0011/10/4893]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*,[abcdef01]xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/<0;1>/*),and_v(v:thresh(2,pkh([abcdef01]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*),a:pkh([abcdef01]xpub6AA2N8RALRYgLD6jT1iXYCEDkndTeZndMtWPbtNX6sY5dPiLtf2T88ahdxrGXMUPoNadgR86sFhBXWQVgifPzDYbY9ZtwK4gqzx4y5Da1DW/<0;1>/*),a:pkh([aabb0011/10/4893]xpub6AyxexvxizZJffF153evmfqHcE9MV88fCNCAtP3jQjXJHwrAKri71Tq9jWUkPxj9pja4u6AkCPHY7atgxzSEa2HtDwJfrRWKK4fsfQg4o77/<0;1>/*)),older(26352))))#hmsqemgr"); // We prevent footguns with timelocks by requiring a u16. Note how the following wouldn't // compile: From ec0009113a8405396950f5c347d0dace3c3c33d0 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Fri, 24 Mar 2023 19:16:58 +0100 Subject: [PATCH 2/9] 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 { From cfbb02c7c8cda56216cfefc84bc6be3ed37a7847 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Sat, 25 Mar 2023 16:16:28 +0100 Subject: [PATCH 3/9] descriptors: multi-recovery-path Liana descriptor This makes it possible to have more than one recovery path in a Liana descriptor. The descriptor and partial spend analysis are adapted to report information about all recovery paths. --- src/commands/mod.rs | 2 +- src/descriptors/analysis.rs | 188 +++++++++--------- src/descriptors/mod.rs | 387 ++++++++++++++++++++++++++++-------- src/signer.rs | 4 +- src/testutils.rs | 6 +- 5 files changed, 416 insertions(+), 171 deletions(-) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 844f628e..a86f9a95 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -704,7 +704,7 @@ impl DaemonControl { // Query the coins that we can spend through the recovery path from the database. let current_height = self.bitcoin.chain_tip().height; - let desc_timelock = self.config.main_descriptor.timelock_value(); + let desc_timelock = self.config.main_descriptor.first_timelock_value(); let timelock: i32 = desc_timelock .try_into() .expect("Must fit, it's effectively a u16"); diff --git a/src/descriptors/analysis.rs b/src/descriptors/analysis.rs index 4d9b3960..f8019336 100644 --- a/src/descriptors/analysis.rs +++ b/src/descriptors/analysis.rs @@ -6,13 +6,14 @@ use miniscript::{ }; use std::{ - collections::{HashMap, HashSet}, + collections::{BTreeMap, HashMap, HashSet}, convert::TryFrom, error, fmt, }; #[derive(Debug)] pub enum LianaPolicyError { + MissingRecoveryPath, InsaneTimelock(u32), InvalidKey(Box), DuplicateKey(Box), @@ -27,6 +28,7 @@ pub enum LianaPolicyError { impl std::fmt::Display for LianaPolicyError { fn fmt(&self, f: &mut fmt::Formatter) -> std::fmt::Result { match self { + Self::MissingRecoveryPath => write!(f, "A Liana policy requires at least one recovery path."), Self::InsaneTimelock(tl) => { write!(f, "Timelock value '{}' isn't valid or safe to use", tl) } @@ -64,17 +66,31 @@ fn is_single_key_or_multisig(policy: &SemanticPolicy bool { - match *key { - descriptor::DescriptorPublicKey::Single(..) | descriptor::DescriptorPublicKey::XPub(..) => { - false +struct DescKeyChecker { + keys_set: HashSet, +} + +impl DescKeyChecker { + pub fn new() -> DescKeyChecker { + DescKeyChecker { + keys_set: HashSet::new(), } - descriptor::DescriptorPublicKey::MultiXPub(ref xpub) => { + } + + /// We require the descriptor key to: + /// - Be deriveable (to contain a wildcard) + /// - Be multipath (to contain a step in the derivation path with multiple indexes) + /// - The multipath step to only contain two indexes, 0 and 1. + /// - Be 'signable' by an external signer (to contain an origin) + /// - Have an xpub that is not a duplicate. + pub fn check(&mut self, key: &descriptor::DescriptorPublicKey) -> Result<(), LianaPolicyError> { + if let descriptor::DescriptorPublicKey::MultiXPub(ref xpub) = *key { + // First make sure it's not a duplicate and record seeing it. + if self.keys_set.contains(&xpub.xkey) { + return Err(LianaPolicyError::DuplicateKey(key.clone().into())); + } + self.keys_set.insert(xpub.xkey); + // Then perform the contextless checks. let der_paths = xpub.derivation_paths.paths(); // Rust-miniscript enforces BIP389 which states that all paths must have the same len. let len = der_paths.get(0).expect("Cannot be empty").len(); @@ -82,12 +98,16 @@ fn is_valid_desc_key(key: &descriptor::DescriptorPublicKey) -> bool { // no unlikely (and easily fixable) while users shooting themselves in the foot by // forgetting to provide the origin is so likely that it's worth ruling out xpubs // without origin entirely. - xpub.origin.is_some() + let valid = xpub.origin.is_some() && xpub.wildcard == descriptor::Wildcard::Unhardened && der_paths.len() == 2 && der_paths[0][len - 1] == 0.into() - && der_paths[1][len - 1] == 1.into() + && der_paths[1][len - 1] == 1.into(); + if valid { + return Ok(()); + } } + Err(LianaPolicyError::InvalidKey(key.clone().into())) } } @@ -283,15 +303,6 @@ impl PathInfo { } } - // TODO: avoid using a vec... - /// Get the keys contained in this spending path. - pub fn keys(&self) -> Vec { - match self { - PathInfo::Single(ref key) => vec![key.clone()], - PathInfo::Multi(_, keys) => keys.clone(), - } - } - /// Get a Miniscript Policy for this path. pub fn into_ms_policy(self) -> ConcretePolicy { match self { @@ -304,24 +315,31 @@ impl PathInfo { } } -/// A Liana spending policy. Can be created from some settings (the primary and recovery keys, the +/// A Liana spending policy is one composed of at least two spending paths: +/// - A directly available path with any number of keys checks; or +/// - One or more recovery paths with any number of keys checks, behind increasing relative +/// timelocks. No two recovery paths may have the same timelock. +/// A Liana 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)] +#[derive(Debug, Eq, PartialEq, Clone)] pub struct LianaPolicy { pub(super) primary_path: PathInfo, - pub(super) recovery_path: (u16, PathInfo), + pub(super) recovery_paths: BTreeMap, } impl LianaPolicy { /// Create a new Liana policy from a given configuration. pub fn new( primary_path: PathInfo, - recovery_path: PathInfo, - recovery_timelock: u16, + recovery_paths: BTreeMap, ) -> Result { + if recovery_paths.is_empty() { + return Err(LianaPolicyError::MissingRecoveryPath); + } + // We require the locktime to: // - not be disabled // - be in number of blocks @@ -329,36 +347,33 @@ impl LianaPolicy { // - be positive (Miniscript requires it not to be 0) // // All this is achieved through asking for a 16-bit integer. - if recovery_timelock == 0 { - return Err(LianaPolicyError::InsaneTimelock(recovery_timelock as u32)); + if recovery_paths.contains_key(&0) { + return Err(LianaPolicyError::InsaneTimelock(0)); } // 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()); - if let Some(key) = all_keys.clone().find(|k| !is_valid_desc_key(k)) { - return Err(LianaPolicyError::InvalidKey((*key).clone().into())); - } - - // 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 { - descriptor::DescriptorPublicKey::MultiXPub(ref multi_xpub) => multi_xpub.xkey, - _ => unreachable!("Just checked it was a multixpub above"), - }; - if key_set.contains(&xpub) { - return Err(LianaPolicyError::DuplicateKey(key.clone().into())); + // Note while the Miniscript compiler does check for duplicate, it does so at the + // "descriptor key expression" level. We don't want duplicate xpubs at all so we do it + // ourselves here. + let spending_paths = recovery_paths + .values() + .chain(std::iter::once(&primary_path)); + let mut key_checker = DescKeyChecker::new(); + for path in spending_paths { + match path { + PathInfo::Single(ref key) => key_checker.check(key)?, + PathInfo::Multi(_, ref keys) => { + for key in keys { + key_checker.check(key)? + } + } } - key_set.insert(xpub); } - assert!(!key_set.is_empty()); // Make sure it is a valid Miniscript policy by (ab)using the compiler. let policy = LianaPolicy { primary_path, - recovery_path: (recovery_timelock, recovery_path), + recovery_paths, }; policy.clone().into_miniscript()?; Ok(policy) @@ -375,25 +390,12 @@ impl LianaPolicy { _ => return Err(LianaPolicyError::IncompatibleDesc), }; - // Get the Miniscript from the descriptor and make sure it only contains valid multipath - // descriptor keys. + // Lift a semantic policy out of this Miniscript and normalize it to make sure we compare + // apples to apples below. let ms = match wsh_desc.as_inner() { descriptor::WshInner::Ms(ms) => ms, _ => return Err(LianaPolicyError::IncompatibleDesc), }; - let invalid_key = ms.iter_pk().find_map(|pk| { - if is_valid_desc_key(&pk) { - None - } else { - Some(pk) - } - }); - if let Some(key) = invalid_key { - return Err(LianaPolicyError::InvalidKey(key.into())); - } - - // Now lift a semantic policy out of this Miniscript and normalize it to make sure we - // compare apples to apples below. let policy = ms .lift() .expect("Lifting can't fail on a Miniscript") @@ -410,7 +412,7 @@ impl LianaPolicy { // Fetch the two spending paths' semantic policies. The primary path is identified as the // only one that isn't timelocked. - let (mut primary_path, mut recovery_path) = (None::, None); + let (mut primary_path, mut recovery_paths) = (None::, BTreeMap::new()); for sub in subs { // This is a (multi)key check. It must be the primary path. if is_single_key_or_multisig(&sub) { @@ -429,27 +431,29 @@ impl LianaPolicy { } } else { // If it's not a simple (multi)key check, it must be the timelocked recovery path. - // For now, we only support a single recovery path. - if recovery_path.is_some() { + let (timelock, path_info) = PathInfo::from_recovery_path(sub)?; + if recovery_paths.contains_key(&timelock) { return Err(LianaPolicyError::IncompatibleDesc); } - recovery_path = Some(PathInfo::from_recovery_path(sub)?); + recovery_paths.insert(timelock, path_info); } } - // Use the constructor for the sanity checks (especially around the Miniscript policy). + // Use the constructor for sanity checking the keys and the Miniscript policy. Note this + // makes sure the recovery paths mapping isn't empty, too. 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) + LianaPolicy::new(prim_path, recovery_paths) } pub fn primary_path(&self) -> &PathInfo { &self.primary_path } - /// Timelock and path info for the recovery path. - pub fn recovery_path(&self) -> (u16, &PathInfo) { - (self.recovery_path.0, &self.recovery_path.1) + /// Timelocks and path info of the recovery paths. Note we guarantee this mapping is never + /// empty, as there is always at least one recovery path. + pub fn recovery_paths(&self) -> &BTreeMap { + assert!(!self.recovery_paths.is_empty()); + &self.recovery_paths } fn into_miniscript( @@ -460,18 +464,24 @@ impl LianaPolicy { > { let LianaPolicy { primary_path, - recovery_path: (timelock, recovery_path), + recovery_paths, } = self; - // Create the timelocked recovery spending path. - let recovery_timelock = ConcretePolicy::Older(Sequence::from_height(timelock)); - let recovery_keys = recovery_path.into_ms_policy(); - let recovery_branch = ConcretePolicy::And(vec![recovery_keys, recovery_timelock]); - - // Create the primary spending path and combine both, assuming the recovery path will - // seldom be used. + // Start with the primary spending path. We'll then or() all the recovery paths to it. let primary_keys = primary_path.into_ms_policy(); - let tl_policy = ConcretePolicy::Or(vec![(99, primary_keys), (1, recovery_branch)]); + + // Incrementally create the top-level policy using all recovery paths. + assert!(!recovery_paths.is_empty()); + let tl_policy = + recovery_paths + .into_iter() + .fold(primary_keys, |tl_policy, (timelock, path_info)| { + let timelock = ConcretePolicy::Older(Sequence::from_height(timelock)); + let keys = path_info.into_ms_policy(); + let recovery_branch = ConcretePolicy::And(vec![keys, timelock]); + // We assume the larger the timelock the less likely a branch would be used. + ConcretePolicy::Or(vec![(99, tl_policy), (1, recovery_branch)]) + }); tl_policy .compile::() @@ -510,9 +520,9 @@ pub struct PathSpendInfo { pub struct PartialSpendInfo { /// Number of signatures present for the primary path pub(super) primary_path: PathSpendInfo, - /// Number of signatures present for the recovery path, only present if the path is available - /// in the first place. - pub(super) recovery_path: Option, + /// Number of signatures present for the recovery path, only present for the recovery paths + /// that are available. + pub(super) recovery_paths: BTreeMap, } impl PartialSpendInfo { @@ -521,9 +531,9 @@ impl PartialSpendInfo { &self.primary_path } - /// Get the number of signatures present for the recovery path. Only present if the path is - /// available in the first place. - pub fn recovery_path(&self) -> &Option { - &self.recovery_path + /// Get the number of signatures present for each recovery path. Only present for available + /// paths. + pub fn recovery_paths(&self) -> &BTreeMap { + &self.recovery_paths } } diff --git a/src/descriptors/mod.rs b/src/descriptors/mod.rs index bf73e0a1..947586da 100644 --- a/src/descriptors/mod.rs +++ b/src/descriptors/mod.rs @@ -178,10 +178,15 @@ impl LianaDescriptor { .expect("We never create a Liana descriptor with an invalid Liana policy.") } - /// Get the value (in blocks) of the relative timelock for the heir's spending path. - pub fn timelock_value(&self) -> u32 { - // TODO: make it return a u16 - self.policy().recovery_path.0 as u32 + /// Get the value (in blocks) of the smallest relative timelock of the recovery paths. + pub fn first_timelock_value(&self) -> u16 { + *self + .policy() + .recovery_paths + .iter() + .next() + .expect("There is always at least one recovery path") + .0 } /// Get the maximum size in WU of a satisfaction for this descriptor. @@ -229,17 +234,21 @@ impl LianaDescriptor { // (ie if the nSequence is >= to the chosen CSV value). let desc_info = self.policy(); let primary_path = desc_info.primary_path.spend_info(pubkeys_signed.clone()); - let recovery_path = if txin.sequence.is_height_locked() - && txin.sequence.0 >= desc_info.recovery_path.0 as u32 - { - Some(desc_info.recovery_path.1.spend_info(pubkeys_signed)) - } else { - None - }; + let recovery_paths = desc_info + .recovery_paths + .iter() + .filter_map(|(timelock, path_info)| { + if txin.sequence.is_height_locked() && txin.sequence.0 >= *timelock as u32 { + Some((*timelock, path_info.spend_info(pubkeys_signed.clone()))) + } else { + None + } + }) + .collect(); PartialSpendInfo { primary_path, - recovery_path, + recovery_paths, } } @@ -394,12 +403,28 @@ mod tests { use crate::signer::HotSigner; + fn random_desc_key( + secp: &secp256k1::Secp256k1, + ) -> descriptor::DescriptorPublicKey { + let signer = HotSigner::generate(bitcoin::Network::Bitcoin).unwrap(); + let xpub_str = format!( + "[{}]{}/<0;1>/*", + signer.fingerprint(secp), + signer.xpub_at(&bip32::DerivationPath::from_str("m").unwrap(), secp) + ); + descriptor::DescriptorPublicKey::from_str(&xpub_str).unwrap() + } + #[test] fn descriptor_creation() { let owner_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap()); let heir_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub688Hn4wScQAAiYJLPg9yH27hUpfZAUnmJejRQBCiwfP5PEDzjWMNW1wChcninxr5gyavFqbbDjdV1aK5USJz8NDVjUy7FRQaaqqXHh5SbXe/<0;1>/*").unwrap()); let timelock = 52560; - let policy = LianaPolicy::new(owner_key.clone(), heir_key.clone(), timelock).unwrap(); + let policy = LianaPolicy::new( + owner_key.clone(), + [(timelock, heir_key.clone())].iter().cloned().collect(), + ) + .unwrap(); assert_eq!(LianaDescriptor::new(policy).to_string(), "wsh(or_d(pk([abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*),and_v(v:pkh([abcdef01]xpub688Hn4wScQAAiYJLPg9yH27hUpfZAUnmJejRQBCiwfP5PEDzjWMNW1wChcninxr5gyavFqbbDjdV1aK5USJz8NDVjUy7FRQaaqqXHh5SbXe/<0;1>/*),older(52560))))#g7vk9r5l"); // A decaying multisig after 6 months. Note we can't duplicate the keys, so different ones @@ -420,7 +445,11 @@ mod tests { descriptor::DescriptorPublicKey::from_str("[aabb0011/10/4893]xpub6AyxexvxizZJffF153evmfqHcE9MV88fCNCAtP3jQjXJHwrAKri71Tq9jWUkPxj9pja4u6AkCPHY7atgxzSEa2HtDwJfrRWKK4fsfQg4o77/<0;1>/*").unwrap(), ], ); - let policy = LianaPolicy::new(primary_keys, recovery_keys, 26352).unwrap(); + let policy = LianaPolicy::new( + primary_keys, + [(26352, recovery_keys)].iter().cloned().collect(), + ) + .unwrap(); assert_eq!(LianaDescriptor::new(policy).to_string(), "wsh(or_d(multi(3,[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*,[aabb0011/10/4893]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*,[abcdef01]xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/<0;1>/*),and_v(v:thresh(2,pkh([abcdef01]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*),a:pkh([abcdef01]xpub6AA2N8RALRYgLD6jT1iXYCEDkndTeZndMtWPbtNX6sY5dPiLtf2T88ahdxrGXMUPoNadgR86sFhBXWQVgifPzDYbY9ZtwK4gqzx4y5Da1DW/<0;1>/*),a:pkh([aabb0011/10/4893]xpub6AyxexvxizZJffF153evmfqHcE9MV88fCNCAtP3jQjXJHwrAKri71Tq9jWUkPxj9pja4u6AkCPHY7atgxzSEa2HtDwJfrRWKK4fsfQg4o77/<0;1>/*)),older(26352))))#hmsqemgr"); // We prevent footguns with timelocks by requiring a u16. Note how the following wouldn't @@ -430,32 +459,52 @@ mod tests { //LianaPolicy::new(owner_key, heir_key, (1 << 22) + 1).unwrap_err(); // You can't use a null timelock in Miniscript. - LianaPolicy::new(owner_key, heir_key, 0).unwrap_err(); + LianaPolicy::new(owner_key, [(0, heir_key)].iter().cloned().collect()).unwrap_err(); let owner_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[aabb0011/10/4893]xpub661MyMwAqRbcFG59fiikD8UV762quhruT8K8bdjqy6N2o3LG7yohoCdLg1m2HAY1W6rfBrtauHkBhbfA4AQ3iazaJj5wVPhwgaRCHBW2DBg/<0;1>/*").unwrap()); let heir_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub661MyMwAqRbcFfxf71L4Dx4w5TmyNXrBicTEAM7vLzumxangwATWWgdJPb6xH1JHcJH9S3jNZx3fCnkkB1WyqrqGgavj1rehHcbythmruvZ/24/32/<0;1>/*").unwrap()); let timelock = 57600; - let policy = LianaPolicy::new(owner_key.clone(), heir_key, timelock).unwrap(); + let policy = LianaPolicy::new( + owner_key.clone(), + [(timelock, heir_key)].iter().cloned().collect(), + ) + .unwrap(); assert_eq!(LianaDescriptor::new(policy).to_string(), "wsh(or_d(pk([aabb0011/10/4893]xpub661MyMwAqRbcFG59fiikD8UV762quhruT8K8bdjqy6N2o3LG7yohoCdLg1m2HAY1W6rfBrtauHkBhbfA4AQ3iazaJj5wVPhwgaRCHBW2DBg/<0;1>/*),and_v(v:pkh([abcdef01]xpub661MyMwAqRbcFfxf71L4Dx4w5TmyNXrBicTEAM7vLzumxangwATWWgdJPb6xH1JHcJH9S3jNZx3fCnkkB1WyqrqGgavj1rehHcbythmruvZ/24/32/<0;1>/*),older(57600))))#ak4cm093"); // We can't pass a raw key, an xpub that is not deriveable, only hardened derivable, // without both the change and receive derivation paths, or with more than 2 different // derivation paths. let heir_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub661MyMwAqRbcFfxf71L4Dx4w5TmyNXrBicTEAM7vLzumxangwATWWgdJPb6xH1JHcJH9S3jNZx3fCnkkB1WyqrqGgavj1rehHcbythmruvZ/0/<0;1>/354").unwrap()); - LianaPolicy::new(owner_key.clone(), heir_key, timelock).unwrap_err(); + LianaPolicy::new( + owner_key.clone(), + [(timelock, heir_key)].iter().cloned().collect(), + ) + .unwrap_err(); let heir_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub661MyMwAqRbcFfxf71L4Dx4w5TmyNXrBicTEAM7vLzumxangwATWWgdJPb6xH1JHcJH9S3jNZx3fCnkkB1WyqrqGgavj1rehHcbythmruvZ/0/<0;1>/*'").unwrap()); - LianaPolicy::new(owner_key.clone(), heir_key, timelock).unwrap_err(); + LianaPolicy::new( + owner_key.clone(), + [(timelock, heir_key)].iter().cloned().collect(), + ) + .unwrap_err(); let heir_key = PathInfo::Single( descriptor::DescriptorPublicKey::from_str( "[abcdef01]02e24913be26dbcfdf8e8e94870b28725cdae09b448b6c127767bf0154e3a3c8e5", ) .unwrap(), ); - LianaPolicy::new(owner_key.clone(), heir_key, timelock).unwrap_err(); + LianaPolicy::new( + owner_key.clone(), + [(timelock, heir_key)].iter().cloned().collect(), + ) + .unwrap_err(); let heir_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub661MyMwAqRbcFfxf71L4Dx4w5TmyNXrBicTEAM7vLzumxangwATWWgdJPb6xH1JHcJH9S3jNZx3fCnkkB1WyqrqGgavj1rehHcbythmruvZ/0/*'").unwrap()); - LianaPolicy::new(owner_key.clone(), heir_key, timelock).unwrap_err(); + LianaPolicy::new( + owner_key.clone(), + [(timelock, heir_key)].iter().cloned().collect(), + ) + .unwrap_err(); let heir_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub661MyMwAqRbcFfxf71L4Dx4w5TmyNXrBicTEAM7vLzumxangwATWWgdJPb6xH1JHcJH9S3jNZx3fCnkkB1WyqrqGgavj1rehHcbythmruvZ/<0;1;2>/*'").unwrap()); - LianaPolicy::new(owner_key, heir_key, timelock).unwrap_err(); + LianaPolicy::new(owner_key, [(timelock, heir_key)].iter().cloned().collect()).unwrap_err(); // And it's checked even in a multisig. For instance: let primary_keys = PathInfo::Multi( @@ -472,18 +521,22 @@ mod tests { descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6AA2N8RALRYgLD6jT1iXYCEDkndTeZndMtWPbtNX6sY5dPiLtf2T88ahdxrGXMUPoNadgR86sFhBXWQVgifPzDYbY9ZtwK4gqzx4y5Da1DW/<0;1>/*").unwrap(), ], ); - LianaPolicy::new(primary_keys, recovery_keys, 26352).unwrap_err(); + LianaPolicy::new( + primary_keys, + [(26352, recovery_keys)].iter().cloned().collect(), + ) + .unwrap_err(); // You can't pass duplicate keys, even if they are encoded differently. let owner_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap()); let heir_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap()); - LianaPolicy::new(owner_key, heir_key, timelock).unwrap_err(); + LianaPolicy::new(owner_key, [(timelock, heir_key)].iter().cloned().collect()).unwrap_err(); let owner_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[00aabb44]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap()); let heir_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap()); - LianaPolicy::new(owner_key, heir_key, timelock).unwrap_err(); + LianaPolicy::new(owner_key, [(timelock, heir_key)].iter().cloned().collect()).unwrap_err(); let owner_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[00aabb44]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap()); let heir_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[11223344/2/98]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap()); - LianaPolicy::new(owner_key, heir_key, timelock).unwrap_err(); + LianaPolicy::new(owner_key, [(timelock, heir_key)].iter().cloned().collect()).unwrap_err(); // You can't pass duplicate keys, even across multisigs. let primary_keys = PathInfo::Multi( @@ -502,13 +555,17 @@ mod tests { descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*").unwrap(), ], ); - LianaPolicy::new(primary_keys, recovery_keys, 26352).unwrap_err(); + LianaPolicy::new( + primary_keys, + [(26352, recovery_keys)].iter().cloned().collect(), + ) + .unwrap_err(); // No origin in one of the keys let owner_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap()); let heir_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("xpub688Hn4wScQAAiYJLPg9yH27hUpfZAUnmJejRQBCiwfP5PEDzjWMNW1wChcninxr5gyavFqbbDjdV1aK5USJz8NDVjUy7FRQaaqqXHh5SbXe/<0;1>/*").unwrap()); let timelock = 52560; - LianaPolicy::new(owner_key, heir_key, timelock).unwrap_err(); + LianaPolicy::new(owner_key, [(timelock, heir_key)].iter().cloned().collect()).unwrap_err(); // A 1-of-N multisig as primary path. LianaDescriptor::from_str("wsh(or_d(multi(1,[573fb35b/48'/1'/0'/2']tpubDFKp9T7WAYDcENSjoifkrpq1gMDF47KGJcJrpxzX23Qor8wuGbrEVs9utNq1MDS8E2WXJSBk1qoPQLpwyokW7DiUNPwFuxQkL7owNkLAb9W/<0;1>/*,[573fb35b/48'/1'/1'/2']tpubDFGezyzuHJPhdP3jHGW7v7Hwes4Hihqv5W2yyCmRY9VZJCRchETvxrMC8uECeJZdxQ14V4iD4DecoArkUSDwj8ogYE9WEv4MNZr12thNHCs/<0;1>/*),and_v(v:multi(2,[573fb35b/48'/1'/2'/2']tpubDDwxQauiaU964vPzt5Vd7jnDHEUtp2Vc34PaWpEXg5TQ3bRccxnc1MKKh88Hi7xiMeZo9Tm6fBcq4UGXqnDtGUniJLjqAD8SjQ8Eci3aSR7/<0;1>/*,[573fb35b/48'/1'/3'/2']tpubDE37XAVB5CQ1x85md3BQ5uHCoMwT5fgT8X13zzCUQ3x5o2jskYxKjj7Qcxt1Jpj4QB8tqspn2dooPCekRuQDYrDHov7J1ueUNu2wcvgRDxr/<0;1>/*),older(1000))))#qjx6ycpc").unwrap(); @@ -536,13 +593,13 @@ mod tests { LianaDescriptor::from_str("wsh(or_i(pk([abcdef01]tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/<0;1>/*),pk([abcdef01]tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/<0;1>/*)))").unwrap_err(); let desc = LianaDescriptor::from_str("wsh(andor(pk([abcdef01]tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/<0;1>/*),older(1),pk([abcdef01]tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/<0;1>/*)))").unwrap(); - assert_eq!(desc.timelock_value(), 1); + assert_eq!(desc.first_timelock_value(), 1); let desc = LianaDescriptor::from_str("wsh(andor(pk([abcdef01]tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/<0;1>/*),older(42000),pk([abcdef01]tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/<0;1>/*)))").unwrap(); - assert_eq!(desc.timelock_value(), 42000); + assert_eq!(desc.first_timelock_value(), 42000); let desc = LianaDescriptor::from_str("wsh(andor(pk([abcdef01]tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/<0;1>/*),older(65535),pk([abcdef01]tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/<0;1>/*)))").unwrap(); - assert_eq!(desc.timelock_value(), 0xffff); + assert_eq!(desc.first_timelock_value(), 0xffff); } #[test] @@ -574,59 +631,88 @@ mod tests { #[test] fn liana_desc_keys() { let secp = secp256k1::Secp256k1::signing_only(); - let random_desc_key = || { - let xpub_str = format!( - "[aabbccdd]{}/<0;1>/*", - HotSigner::generate(bitcoin::Network::Bitcoin) - .unwrap() - .xpub_at(&bip32::DerivationPath::from_str("m").unwrap(), &secp) - ); - descriptor::DescriptorPublicKey::from_str(&xpub_str).unwrap() - }; - let prim_path = PathInfo::Single(random_desc_key()); + let prim_path = PathInfo::Single(random_desc_key(&secp)); let twenty_eight_keys: Vec = - (0..28).map(|_| random_desc_key()).collect(); + (0..28).map(|_| random_desc_key(&secp)).collect(); let mut twenty_nine_keys = twenty_eight_keys.clone(); - twenty_nine_keys.push(random_desc_key()); + twenty_nine_keys.push(random_desc_key(&secp)); LianaPolicy::new( prim_path.clone(), - PathInfo::Multi(2, vec![random_desc_key()]), - 1, + [(1, PathInfo::Multi(2, vec![random_desc_key(&secp)]))] + .iter() + .cloned() + .collect(), ) .unwrap_err(); LianaPolicy::new( prim_path.clone(), - PathInfo::Multi(1, vec![random_desc_key(), random_desc_key()]), - 1, + [( + 1, + PathInfo::Multi(1, vec![random_desc_key(&secp), random_desc_key(&secp)]), + )] + .iter() + .cloned() + .collect(), ) .unwrap(); LianaPolicy::new( prim_path.clone(), - PathInfo::Multi(0, vec![random_desc_key(), random_desc_key()]), - 1, + [( + 1, + PathInfo::Multi(0, vec![random_desc_key(&secp), random_desc_key(&secp)]), + )] + .iter() + .cloned() + .collect(), ) .unwrap_err(); LianaPolicy::new( prim_path.clone(), - PathInfo::Multi(2, vec![random_desc_key(), random_desc_key()]), - 1, + [( + 1, + PathInfo::Multi(2, vec![random_desc_key(&secp), random_desc_key(&secp)]), + )] + .iter() + .cloned() + .collect(), ) .unwrap(); LianaPolicy::new( prim_path.clone(), - PathInfo::Multi(3, vec![random_desc_key(), random_desc_key()]), - 1, + [( + 1, + PathInfo::Multi(3, vec![random_desc_key(&secp), random_desc_key(&secp)]), + )] + .iter() + .cloned() + .collect(), ) .unwrap_err(); LianaPolicy::new( prim_path.clone(), - PathInfo::Multi(3, twenty_eight_keys.clone()), - 1, + [(1, PathInfo::Multi(3, twenty_eight_keys.clone()))] + .iter() + .cloned() + .collect(), ) .unwrap(); - 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(); + LianaPolicy::new( + prim_path.clone(), + [(1, PathInfo::Multi(20, twenty_eight_keys))] + .iter() + .cloned() + .collect(), + ) + .unwrap(); + LianaPolicy::new( + prim_path, + [(1, PathInfo::Multi(20, twenty_nine_keys))] + .iter() + .cloned() + .collect(), + ) + .unwrap_err(); } fn roundtrip(desc_str: &str) { @@ -652,6 +738,8 @@ mod tests { #[test] fn partial_spend_info() { + let secp = secp256k1::Secp256k1::signing_only(); + // A simple descriptor with 1 keys as primary path and 1 recovery key. let desc = LianaDescriptor::from_str("wsh(or_d(pk([f5acc2fd]tpubD6NzVbkrYhZ4YgUx2ZLNt2rLYAMTdYysCRzKoLu2BeSHKvzqPaBDvf17GeBPnExUVPkuBpx4kniP964e2MxyzzazcXLptxLXModSVCVEV1T/<0;1>/*),and_v(v:pkh([8a64f2a9]tpubD6NzVbkrYhZ4WmzFjvQrp7sDa4ECUxTi9oby8K4FZkd3XCBtEdKwUiQyYJaxiJo5y42gyDWEczrFpozEjeLxMPxjf2WtkfcbpUdfvNnozWF/<0;1>/*),older(10))))#d72le4dr").unwrap(); let desc_info = desc.policy(); @@ -671,25 +759,24 @@ mod tests { assert_eq!(info.primary_path.threshold, 1); assert_eq!(info.primary_path.sigs_count, 0); assert!(info.primary_path.signed_pubkeys.is_empty()); - assert!(info.recovery_path.is_none()); + assert!(info.recovery_paths.is_empty()); // If we set the sequence too low we still won't have the recovery path info. unsigned_single_psbt.unsigned_tx.input[0].sequence = - Sequence::from_height(desc_info.recovery_path.0 - 1); + Sequence::from_height(desc_info.recovery_paths.keys().next().unwrap() - 1); let info = desc.partial_spend_info(&unsigned_single_psbt).unwrap(); - assert!(info.recovery_path.is_none()); + assert!(info.recovery_paths.is_empty()); // Now if we set the sequence at the right value we'll have it. - unsigned_single_psbt.unsigned_tx.input[0].sequence = - Sequence::from_height(desc_info.recovery_path.0); + let timelock = *desc_info.recovery_paths.keys().next().unwrap(); + unsigned_single_psbt.unsigned_tx.input[0].sequence = Sequence::from_height(timelock); let info = desc.partial_spend_info(&unsigned_single_psbt).unwrap(); - assert!(info.recovery_path.is_some()); + assert!(info.recovery_paths.contains_key(&timelock)); // Even if it's a bit too high (as long as it's still a block height and activated) - unsigned_single_psbt.unsigned_tx.input[0].sequence = - Sequence::from_height(desc_info.recovery_path.0 + 42); + unsigned_single_psbt.unsigned_tx.input[0].sequence = Sequence::from_height(timelock + 42); let info = desc.partial_spend_info(&unsigned_single_psbt).unwrap(); - let recov_info = info.recovery_path.unwrap(); + let recov_info = info.recovery_paths.get(&timelock).unwrap(); assert_eq!(recov_info.threshold, 1); assert_eq!(recov_info.sigs_count, 0); assert!(recov_info.signed_pubkeys.is_empty()); @@ -707,11 +794,10 @@ mod tests { .signed_pubkeys .contains_key(&prim_key_origin) ); - assert!(info.recovery_path.is_none()); + assert!(info.recovery_paths.is_empty()); // Now enable the recovery path and add a signature for the recovery key. - signed_single_psbt.unsigned_tx.input[0].sequence = - Sequence::from_height(desc_info.recovery_path.0); + signed_single_psbt.unsigned_tx.input[0].sequence = Sequence::from_height(timelock); let recov_pubkey = bitcoin::PublicKey { compressed: true, inner: *signed_single_psbt.inputs[0] @@ -742,7 +828,7 @@ mod tests { assert_eq!(info.primary_path.threshold, 1); assert_eq!(info.primary_path.sigs_count, 0); assert!(info.primary_path.signed_pubkeys.is_empty()); - let recov_info = info.recovery_path.unwrap(); + let recov_info = info.recovery_paths.get(&timelock).unwrap(); assert_eq!(recov_info.threshold, 1); assert_eq!(recov_info.sigs_count, 1); assert!( @@ -766,12 +852,12 @@ mod tests { .signed_pubkeys .contains_key(&prim_key_origin) ); - assert!(info.recovery_path.is_none()); + assert!(info.recovery_paths.is_empty()); // Enable the recovery path, it should show no recovery sig. let mut rec_psbt = psbt.clone(); for txin in rec_psbt.unsigned_tx.input.iter_mut() { - txin.sequence = Sequence::from_height(desc_info.recovery_path.0); + txin.sequence = Sequence::from_height(timelock); } let info = desc.partial_spend_info(&rec_psbt).unwrap(); assert!(rec_psbt @@ -787,7 +873,7 @@ mod tests { .signed_pubkeys .contains_key(&prim_key_origin) ); - let recov_info = info.recovery_path.unwrap(); + let recov_info = info.recovery_paths.get(&timelock).unwrap(); assert_eq!(recov_info.threshold, 1); assert_eq!(recov_info.sigs_count, 0); assert!(recov_info.signed_pubkeys.is_empty()); @@ -795,8 +881,7 @@ mod tests { // If the sequence of one of the input is different from the other ones, it'll return // an error since the analysis is on the whole transaction. let mut inconsistent_psbt = psbt.clone(); - inconsistent_psbt.unsigned_tx.input[0].sequence = - Sequence::from_height(desc_info.recovery_path.0 + 1); + inconsistent_psbt.unsigned_tx.input[0].sequence = Sequence::from_height(timelock + 1); assert!(desc .partial_spend_info(&inconsistent_psbt) .unwrap_err() @@ -828,7 +913,7 @@ mod tests { .signed_pubkeys .contains_key(&prim_key_origin) ); - assert!(info.recovery_path.is_none()); + assert!(info.recovery_paths.is_empty()); let desc = LianaDescriptor::from_str("wsh(or_d(multi(2,[636adf3f/48'/1'/0'/2']tpubDEE9FvWbG4kg4gxDNrALgrWLiHwNMXNs8hk6nXNPw4VHKot16xd2251vwi2M6nsyQTkak5FJNHVHkCcuzmvpSbWHdumX3DxpDm89iTfSBaL/<0;1>/*,[ffd63c8d/48'/1'/0'/2']tpubDExA3EC3iAsPxPhFn4j6gMiVup6V2eH3qKyk69RcTc9TTNRfFYVPad8bJD5FCHVQxyBT4izKsvr7Btd2R4xmQ1hZkvsqGBaeE82J71uTK4N/<0;1>/*),and_v(v:multi(2,[636adf3f/48'/1'/1'/2']tpubDDvF2khuoBBj8vcSjQfa7iKaxsQZE7YjJ7cJL8A8eaneadMPKbHSpoSr4JD1F5LUvWD82HCxdtSppGfrMUmiNbFxrA2EHEVLnrdCFNFe75D/<0;1>/*,[ffd63c8d/48'/1'/1'/2']tpubDFMs44FD4kFt3M7Z317cFh5tdKEGN8tyQRY6Q5gcSha4NtxZfGmTVRMbsD1bWN469LstXU4aVSARDxrvxFCUjHeegfEY2cLSazMBkNCmDPD/<0;1>/*),older(2))))#xcf6jr2r").unwrap(); let info = desc.policy(); @@ -839,19 +924,163 @@ mod tests { descriptor::DescriptorPublicKey::from_str("[ffd63c8d/48'/1'/0'/2']tpubDExA3EC3iAsPxPhFn4j6gMiVup6V2eH3qKyk69RcTc9TTNRfFYVPad8bJD5FCHVQxyBT4izKsvr7Btd2R4xmQ1hZkvsqGBaeE82J71uTK4N/<0;1>/*").unwrap(), ], )); - assert_eq!(info.recovery_path, (2, PathInfo::Multi( + assert_eq!(info.recovery_paths, [(2, PathInfo::Multi( 2, vec![ descriptor::DescriptorPublicKey::from_str("[636adf3f/48'/1'/1'/2']tpubDDvF2khuoBBj8vcSjQfa7iKaxsQZE7YjJ7cJL8A8eaneadMPKbHSpoSr4JD1F5LUvWD82HCxdtSppGfrMUmiNbFxrA2EHEVLnrdCFNFe75D/<0;1>/*").unwrap(), descriptor::DescriptorPublicKey::from_str("[ffd63c8d/48'/1'/1'/2']tpubDFMs44FD4kFt3M7Z317cFh5tdKEGN8tyQRY6Q5gcSha4NtxZfGmTVRMbsD1bWN469LstXU4aVSARDxrvxFCUjHeegfEY2cLSazMBkNCmDPD/<0;1>/*").unwrap(), ], - ))); - let psbt = psbt_from_str("cHNidP8BAIkCAAAAAWi3OFgkj1CqCDT3Swm8kbxZS9lxz4L3i4W2v9KGC7nqAQAAAAD9////AkANAwAAAAAAIgAg27lNc1rog+dOq80ohRuds4Hgg/RcpxVun2XwgpuLSrFYMwwAAAAAACIAIDyWveqaElWmFGkTbFojg1zXWHODtiipSNjfgi2DqBy9AAAAAAABAOoCAAAAAAEBsRWl70USoAFFozxc86pC7Dovttdg4kvja//3WMEJskEBAAAAAP7///8CWKmCIk4GAAAWABRKBWYWkCNS46jgF0r69Ehdnq+7T0BCDwAAAAAAIgAgTt5fs+CiB+FRzNC8lHcgWLH205sNjz1pT59ghXlG5tQCRzBEAiBXK9MF8z3bX/VnY2aefgBBmiAHPL4tyDbUOe7+KpYA4AIgL5kU0DFG8szKd+szRzz/OTUWJ0tZqij41h2eU9rSe1IBIQNBB1hy+jKsg1TihMT0dXw7etpu9TkO3NuvhBDFJlBj1cP2AQABAStAQg8AAAAAACIAIE7eX7PgogfhUczQvJR3IFix9tObDY89aU+fYIV5RubUIgICSKJsNs0zFJN58yd2aYQ+C3vhMbi0x7k0FV3wBhR4THlIMEUCIQCPWWWOhs2lThxOq/G8X2fYBRvM9MXSm7qPH+dRVYQZEwIgfut2vx3RvwZWcgEj4ohQJD5lNJlwOkA4PAiN1fjx6dABIgID3mvj1zerZKohOVhKCiskYk+3qrCum6PIwDhQ16ePACpHMEQCICZNR+0/1hPkrDQwPFmg5VjUHkh6aK9cXUu3kPbM8hirAiAyE/5NUXKfmFKij30isuyysJbq8HrURjivd+S9vdRGKQEBBZNSIQJIomw2zTMUk3nzJ3ZphD4Le+ExuLTHuTQVXfAGFHhMeSEC9OfCXl+sJOrxUFLBuMV4ZUlJYjuzNGZSld5ioY14y8FSrnNkUSED3mvj1zerZKohOVhKCiskYk+3qrCum6PIwDhQ16ePACohA+ECH+HlR+8Sf3pumaXH3IwSsoqSLCH7H1THiBP93z3ZUq9SsmgiBgJIomw2zTMUk3nzJ3ZphD4Le+ExuLTHuTQVXfAGFHhMeRxjat8/MAAAgAEAAIAAAACAAgAAgAAAAAABAAAAIgYC9OfCXl+sJOrxUFLBuMV4ZUlJYjuzNGZSld5ioY14y8Ec/9Y8jTAAAIABAACAAAAAgAIAAIAAAAAAAQAAACIGA95r49c3q2SqITlYSgorJGJPt6qwrpujyMA4UNenjwAqHGNq3z8wAACAAQAAgAEAAIACAACAAAAAAAEAAAAiBgPhAh/h5UfvEn96bpmlx9yMErKKkiwh+x9Ux4gT/d892Rz/1jyNMAAAgAEAAIABAACAAgAAgAAAAAABAAAAACICAlBQ7gGocg7eF3sXrCio+zusAC9+xfoyIV95AeR69DWvHGNq3z8wAACAAQAAgAEAAIACAACAAAAAAAMAAAAiAgMvVy984eg8Kgvj058PBHetFayWbRGb7L0DMnS9KHSJzBxjat8/MAAAgAEAAIAAAACAAgAAgAAAAAADAAAAIgIDSRIG1dn6njdjsDXenHa2lUvQHWGPLKBVrSzbQOhiIxgc/9Y8jTAAAIABAACAAAAAgAIAAIAAAAAAAwAAACICA0/epE59sVEj7Et0I4R9qJQNuX23RNvDZKCRL7eUps9FHP/WPI0wAACAAQAAgAEAAIACAACAAAAAAAMAAAAAIgICgldCOK6iHscv//2NipgaMABLV5TICU/zlP7HlQmlg08cY2rfPzAAAIABAACAAQAAgAIAAIABAAAAAQAAACICApb0p9rfpJshB3J186PGWrvzQdixcwQZWmebOUMdkquZHP/WPI0wAACAAQAAgAAAAIACAACAAQAAAAEAAAAiAgLY5q+unoDxC/HI5BaNiPq12ei1REZIcUAN304JfKXUwxz/1jyNMAAAgAEAAIABAACAAgAAgAEAAAABAAAAIgIDg6cUVCJB79cMcofiURHojxFARWyS4YEhJNRixuOZZRgcY2rfPzAAAIABAACAAAAAgAIAAIABAAAAAQAAAAA="); + ))].iter().cloned().collect()); + let mut psbt = psbt_from_str("cHNidP8BAIkCAAAAAWi3OFgkj1CqCDT3Swm8kbxZS9lxz4L3i4W2v9KGC7nqAQAAAAD9////AkANAwAAAAAAIgAg27lNc1rog+dOq80ohRuds4Hgg/RcpxVun2XwgpuLSrFYMwwAAAAAACIAIDyWveqaElWmFGkTbFojg1zXWHODtiipSNjfgi2DqBy9AAAAAAABAOoCAAAAAAEBsRWl70USoAFFozxc86pC7Dovttdg4kvja//3WMEJskEBAAAAAP7///8CWKmCIk4GAAAWABRKBWYWkCNS46jgF0r69Ehdnq+7T0BCDwAAAAAAIgAgTt5fs+CiB+FRzNC8lHcgWLH205sNjz1pT59ghXlG5tQCRzBEAiBXK9MF8z3bX/VnY2aefgBBmiAHPL4tyDbUOe7+KpYA4AIgL5kU0DFG8szKd+szRzz/OTUWJ0tZqij41h2eU9rSe1IBIQNBB1hy+jKsg1TihMT0dXw7etpu9TkO3NuvhBDFJlBj1cP2AQABAStAQg8AAAAAACIAIE7eX7PgogfhUczQvJR3IFix9tObDY89aU+fYIV5RubUIgICSKJsNs0zFJN58yd2aYQ+C3vhMbi0x7k0FV3wBhR4THlIMEUCIQCPWWWOhs2lThxOq/G8X2fYBRvM9MXSm7qPH+dRVYQZEwIgfut2vx3RvwZWcgEj4ohQJD5lNJlwOkA4PAiN1fjx6dABIgID3mvj1zerZKohOVhKCiskYk+3qrCum6PIwDhQ16ePACpHMEQCICZNR+0/1hPkrDQwPFmg5VjUHkh6aK9cXUu3kPbM8hirAiAyE/5NUXKfmFKij30isuyysJbq8HrURjivd+S9vdRGKQEBBZNSIQJIomw2zTMUk3nzJ3ZphD4Le+ExuLTHuTQVXfAGFHhMeSEC9OfCXl+sJOrxUFLBuMV4ZUlJYjuzNGZSld5ioY14y8FSrnNkUSED3mvj1zerZKohOVhKCiskYk+3qrCum6PIwDhQ16ePACohA+ECH+HlR+8Sf3pumaXH3IwSsoqSLCH7H1THiBP93z3ZUq9SsmgiBgJIomw2zTMUk3nzJ3ZphD4Le+ExuLTHuTQVXfAGFHhMeRxjat8/MAAAgAEAAIAAAACAAgAAgAAAAAABAAAAIgYC9OfCXl+sJOrxUFLBuMV4ZUlJYjuzNGZSld5ioY14y8Ec/9Y8jTAAAIABAACAAAAAgAIAAIAAAAAAAQAAACIGA95r49c3q2SqITlYSgorJGJPt6qwrpujyMA4UNenjwAqHGNq3z8wAACAAQAAgAEAAIACAACAAAAAAAEAAAAiBgPhAh/h5UfvEn96bpmlx9yMErKKkiwh+x9Ux4gT/d892Rz/1jyNMAAAgAEAAIABAACAAgAAgAAAAAABAAAAACICAlBQ7gGocg7eF3sXrCio+zusAC9+xfoyIV95AeR69DWvHGNq3z8wAACAAQAAgAEAAIACAACAAAAAAAMAAAAiAgMvVy984eg8Kgvj058PBHetFayWbRGb7L0DMnS9KHSJzBxjat8/MAAAgAEAAIAAAACAAgAAgAAAAAADAAAAIgIDSRIG1dn6njdjsDXenHa2lUvQHWGPLKBVrSzbQOhiIxgc/9Y8jTAAAIABAACAAAAAgAIAAIAAAAAAAwAAACICA0/epE59sVEj7Et0I4R9qJQNuX23RNvDZKCRL7eUps9FHP/WPI0wAACAAQAAgAEAAIACAACAAAAAAAMAAAAAIgICgldCOK6iHscv//2NipgaMABLV5TICU/zlP7HlQmlg08cY2rfPzAAAIABAACAAQAAgAIAAIABAAAAAQAAACICApb0p9rfpJshB3J186PGWrvzQdixcwQZWmebOUMdkquZHP/WPI0wAACAAQAAgAAAAIACAACAAQAAAAEAAAAiAgLY5q+unoDxC/HI5BaNiPq12ei1REZIcUAN304JfKXUwxz/1jyNMAAAgAEAAIABAACAAgAAgAEAAAABAAAAIgIDg6cUVCJB79cMcofiURHojxFARWyS4YEhJNRixuOZZRgcY2rfPzAAAIABAACAAAAAgAIAAIABAAAAAQAAAAA="); let partial_info = desc.partial_spend_info(&psbt).unwrap(); assert_eq!(partial_info.primary_path.threshold, 2); assert_eq!(partial_info.primary_path.sigs_count, 1); assert_eq!(partial_info.primary_path.signed_pubkeys.len(), 1); - assert!(partial_info.recovery_path.is_none()); + assert!(partial_info.recovery_paths.is_empty()); + + // A not very well thought-out decaying multisig. + let prim_path = PathInfo::Multi(3, (0..3).map(|_| random_desc_key(&secp)).collect()); + let first_reco_path = PathInfo::Multi(3, (0..5).map(|_| random_desc_key(&secp)).collect()); + let sec_reco_path = PathInfo::Multi(2, (0..5).map(|_| random_desc_key(&secp)).collect()); + let third_reco_path = PathInfo::Multi(1, (0..5).map(|_| random_desc_key(&secp)).collect()); + let liana_policy = LianaPolicy::new( + prim_path.clone(), + [ + (26784, first_reco_path.clone()), + (53568, sec_reco_path.clone()), + (62496, third_reco_path.clone()), + ] + .iter() + .cloned() + .collect(), + ) + .unwrap(); + let desc = LianaDescriptor::new(liana_policy.clone()); + let policy = desc.policy(); + assert_eq!(policy, liana_policy); + let empty_partial_info = desc.partial_spend_info(&psbt).unwrap(); + assert_eq!(empty_partial_info.primary_path.threshold, 3); + assert_eq!(empty_partial_info.primary_path.sigs_count, 0); + assert_eq!( + empty_partial_info.primary_path.sigs_count, + empty_partial_info.primary_path.signed_pubkeys.len() + ); + assert!(empty_partial_info.recovery_paths.is_empty()); + + // Now set a signature for the primary path. All recovery paths still empty, a signature is + // present for the primary path. + let dummy_pubkey = bitcoin::PublicKey::from_str( + "0282574238aea21ec72ffffd8d8a981a30004b5794c8094ff394fec79509a5834f", + ) + .unwrap(); + let dummy_sig = bitcoin::EcdsaSig::from_str ("30440220264d47ed3fd613e4ac34303c59a0e558d41e487a68af5c5d4bb790f6ccf218ab02203213fe4d51729f9852a28f7d22b2ecb2b096eaf07ad44638af77e4bdbdd4462901").unwrap(); + let dummy_der_path = bip32::DerivationPath::from_str("m/0/1").unwrap(); + let fingerprint = prim_path.thresh_origins().1.into_iter().next().unwrap().0; + psbt.inputs[0] + .bip32_derivation + .insert(dummy_pubkey.inner, (fingerprint, dummy_der_path)); + psbt.inputs[0].partial_sigs.insert(dummy_pubkey, dummy_sig); + let partial_info = desc.partial_spend_info(&psbt).unwrap(); + assert_eq!(partial_info.primary_path.threshold, 3); + assert_eq!(partial_info.primary_path.sigs_count, 1); + assert_eq!( + partial_info.primary_path.sigs_count, + partial_info.primary_path.signed_pubkeys.len() + ); + assert!(partial_info.recovery_paths.is_empty()); + + // Now enable the first recovery path and make the signature be for this path. + let fingerprint = first_reco_path + .thresh_origins() + .1 + .into_iter() + .next() + .unwrap() + .0; + psbt.inputs[0] + .bip32_derivation + .get_mut(&dummy_pubkey.inner) + .unwrap() + .0 = fingerprint; + let partial_info = desc.partial_spend_info(&psbt).unwrap(); + assert_eq!(partial_info.primary_path.threshold, 3); + assert_eq!(partial_info.primary_path.sigs_count, 0); + assert_eq!( + partial_info.primary_path.sigs_count, + partial_info.primary_path.signed_pubkeys.len() + ); + assert!(partial_info.recovery_paths.is_empty()); + psbt.unsigned_tx.input[0].sequence = bitcoin::Sequence::from_height(26784); + let partial_info = desc.partial_spend_info(&psbt).unwrap(); + assert_eq!(partial_info.recovery_paths.len(), 1); + assert_eq!(partial_info.recovery_paths[&26784].threshold, 3); + assert_eq!(partial_info.recovery_paths[&26784].sigs_count, 1); + assert_eq!( + partial_info.recovery_paths[&26784].signed_pubkeys.len(), + partial_info.recovery_paths[&26784].sigs_count + ); + + // Now enable the second recovery path and make the signature be for this path. + let fingerprint = sec_reco_path + .thresh_origins() + .1 + .into_iter() + .next() + .unwrap() + .0; + psbt.inputs[0] + .bip32_derivation + .get_mut(&dummy_pubkey.inner) + .unwrap() + .0 = fingerprint; + psbt.unsigned_tx.input[0].sequence = bitcoin::Sequence::from_height(53568); + let partial_info = desc.partial_spend_info(&psbt).unwrap(); + assert_eq!(partial_info.primary_path.threshold, 3); + assert_eq!(partial_info.primary_path.sigs_count, 0); + assert_eq!( + partial_info.primary_path.sigs_count, + partial_info.primary_path.signed_pubkeys.len() + ); + assert_eq!(partial_info.recovery_paths.len(), 2); + assert_eq!(partial_info.recovery_paths[&26784].threshold, 3); + assert_eq!(partial_info.recovery_paths[&26784].sigs_count, 0); + assert_eq!(partial_info.recovery_paths[&53568].threshold, 2); + assert_eq!(partial_info.recovery_paths[&53568].sigs_count, 1); + for rec_path in partial_info.recovery_paths.values() { + assert_eq!(rec_path.sigs_count, rec_path.signed_pubkeys.len()); + } + + // Finally do the same for the third recovery path. + let fingerprint = third_reco_path + .thresh_origins() + .1 + .into_iter() + .next() + .unwrap() + .0; + psbt.inputs[0] + .bip32_derivation + .get_mut(&dummy_pubkey.inner) + .unwrap() + .0 = fingerprint; + psbt.unsigned_tx.input[0].sequence = bitcoin::Sequence::from_height(62496); + let partial_info = desc.partial_spend_info(&psbt).unwrap(); + assert_eq!(partial_info.primary_path.threshold, 3); + assert_eq!(partial_info.primary_path.sigs_count, 0); + assert_eq!( + partial_info.primary_path.sigs_count, + partial_info.primary_path.signed_pubkeys.len() + ); + assert_eq!(partial_info.recovery_paths.len(), 3); + assert_eq!(partial_info.recovery_paths[&26784].threshold, 3); + assert_eq!(partial_info.recovery_paths[&26784].sigs_count, 0); + assert_eq!(partial_info.recovery_paths[&53568].threshold, 2); + assert_eq!(partial_info.recovery_paths[&53568].sigs_count, 0); + assert_eq!(partial_info.recovery_paths[&62496].threshold, 1); + assert_eq!(partial_info.recovery_paths[&62496].sigs_count, 1); + for rec_path in partial_info.recovery_paths.values() { + assert_eq!(rec_path.sigs_count, rec_path.signed_pubkeys.len()); + } } // TODO: test error conditions of deserialization. diff --git a/src/signer.rs b/src/signer.rs index 646eabdd..76650a9b 100644 --- a/src/signer.rs +++ b/src/signer.rs @@ -423,7 +423,9 @@ mod tests { wildcard: Wildcard::Unhardened, }); let recov_keys = descriptors::PathInfo::Single(recov_key); - let policy = descriptors::LianaPolicy::new(prim_keys, recov_keys, 42).unwrap(); + let policy = + descriptors::LianaPolicy::new(prim_keys, [(46, recov_keys)].iter().cloned().collect()) + .unwrap(); let desc = descriptors::LianaDescriptor::new(policy); // Create a dummy PSBT spending a coin from this descriptor with a single input and single diff --git a/src/testutils.rs b/src/testutils.rs index d5d61c2b..d91f222d 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -402,7 +402,11 @@ impl DummyLiana { let owner_key = descriptors::PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[aabbccdd]xpub68JJTXc1MWK8KLW4HGLXZBJknja7kDUJuFHnM424LbziEXsfkh1WQCiEjjHw4zLqSUm4rvhgyGkkuRowE9tCJSgt3TQB5J3SKAbZ2SdcKST/<0;1>/*").unwrap()); let heir_key = descriptors::PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[aabbccdd]xpub68JJTXc1MWK8PEQozKsRatrUHXKFNkD1Cb1BuQU9Xr5moCv87anqGyXLyUd4KpnDyZgo3gz4aN1r3NiaoweFW8UutBsBbgKHzaD5HkTkifK/<0;1>/*").unwrap()); - let policy = descriptors::LianaPolicy::new(owner_key, heir_key, 10_000).unwrap(); + let policy = descriptors::LianaPolicy::new( + owner_key, + [(10_000, heir_key)].iter().cloned().collect(), + ) + .unwrap(); let desc = descriptors::LianaDescriptor::new(policy); let config = Config { bitcoin_config, From d68d0e113473e59310723f805fd86ca204e5f1ed Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 28 Mar 2023 13:06:23 +0200 Subject: [PATCH 4/9] commands: adapt 'createrecovery' to multiple recovery paths --- doc/API.md | 24 ++++++++++++++++-------- src/commands/mod.rs | 31 ++++++++++++++++--------------- src/jsonrpc/api.rs | 10 +++++++++- tests/test_rpc.py | 2 +- 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/doc/API.md b/doc/API.md index 9c0a160f..bcb93b58 100644 --- a/doc/API.md +++ b/doc/API.md @@ -267,18 +267,26 @@ Confirmation time is based on the timestamp of blocks. ### `createrecovery` -Create a transaction that sweeps all coins whose timelocked recovery path is available to a provided -address at a provided feerate. +Create a transaction that sweeps all coins for which a timelocked recovery path is +currently available to a provided address with the provided feerate. -Will error if no such coins are available or the sum of their value is not enough to cover the -requested feerate. +The `timelock` parameter can be used to specify which recovery path to use. By default, +we'll use the first recovery path available. If created for a later timelock a recovery +transaction may be satisfied using an earlier timelock but not the opposite. + +Due to the fact coins are generally received at different block heights, not all coins may be +spendable through a single recovery path at the same time. + +This command will error if no such coins are available or the sum of their value is not enough to +cover the requested feerate. #### Request -| Field | Type | Description | -| ---------- | ----------------- | ----------------------------------------------------------------- | -| `address` | str | The Bitcoin address to sweep the coins to. | -| `feerate` | integer | Target feerate for the transaction, in satoshis per virtual byte. | +| Field | Type | Description | +| ---------- | ----------------- | ----------------------------------------------------------------------------------------- | +| `address` | str | The Bitcoin address to sweep the coins to. | +| `feerate` | integer | Target feerate for the transaction, in satoshis per virtual byte. | +| `timelock` | int or `null` | Recovery path to be used, identified by the number of blocks after which it is available. | #### Response diff --git a/src/commands/mod.rs b/src/commands/mod.rs index a86f9a95..1f54f900 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -124,8 +124,8 @@ impl fmt::Display for CommandError { Self::RescanTrigger(s) => write!(f, "Error while starting rescan: '{}'", s), Self::RecoveryNotAvailable => write!( f, - "No coin currently available through the timelocked recovery path." - ), + "No coin currently spendable through this timelocked recovery path." + ), } } } @@ -668,14 +668,18 @@ impl DaemonControl { ListTransactionsResult { transactions } } - /// Create a transaction that sweeps all coins whose timelocked recovery path is currently - /// available to a provided address with the provided feerate. + /// Create a transaction that sweeps all coins for which a timelocked recovery path is + /// currently available to a provided address with the provided feerate. /// - /// Note that not all coins may be spendable through the recovery path at the same time. + /// The `timelock` parameter can be used to specify which recovery path to use. By default, + /// we'll use the first recovery path available. + /// + /// Note that not all coins may be spendable through a single recovery path at the same time. pub fn create_recovery( &self, address: bitcoin::Address, feerate_vb: u64, + timelock: Option, ) -> Result { if feerate_vb < 1 { return Err(CommandError::InvalidFeerate(feerate_vb)); @@ -702,27 +706,24 @@ impl DaemonControl { outputs: vec![PsbtOut::default()], }; - // Query the coins that we can spend through the recovery path from the database. + // Query the coins that we can spend through the specified recovery path (if no recovery + // path specified, use the first available one) from the database. let current_height = self.bitcoin.chain_tip().height; - let desc_timelock = self.config.main_descriptor.first_timelock_value(); - let timelock: i32 = desc_timelock - .try_into() - .expect("Must fit, it's effectively a u16"); + let timelock = + timelock.unwrap_or_else(|| self.config.main_descriptor.first_timelock_value()); + let height_delta: i32 = timelock.try_into().expect("Must fit, it's a u16"); let sweepable_coins = db_conn .coins(CoinType::Unspent) .into_iter() .filter(|(_, c)| { // We are interested in coins available at the *next* block c.block_info - .map(|b| current_height + 1 >= b.height + timelock) + .map(|b| current_height + 1 >= b.height + height_delta) .unwrap_or(false) }); // Fill-in the transaction inputs and PSBT inputs information. Record the value // that is fed to the transaction while doing so, to compute the fees afterward. - let csv_value: u16 = desc_timelock - .try_into() - .expect("Must fit, it's effectively a u16"); let mut in_value = bitcoin::Amount::from_sat(0); let txin_sat_vb = self.config.main_descriptor.max_sat_vbytes(); let mut sat_vb = 0; @@ -731,7 +732,7 @@ impl DaemonControl { in_value += coin.amount; psbt.unsigned_tx.input.push(bitcoin::TxIn { previous_output: coin.outpoint, - sequence: bitcoin::Sequence::from_height(csv_value), + sequence: bitcoin::Sequence::from_height(timelock), // TODO: once we move to Taproot, anti-fee-sniping using nSequence ..bitcoin::TxIn::default() }); diff --git a/src/jsonrpc/api.rs b/src/jsonrpc/api.rs index b7c85e3d..3e67659d 100644 --- a/src/jsonrpc/api.rs +++ b/src/jsonrpc/api.rs @@ -148,8 +148,16 @@ fn create_recovery(control: &DaemonControl, params: Params) -> Result = params + .get(2, "timelock") + .map(|tl| { + tl.as_u64() + .and_then(|tl| tl.try_into().ok()) + .ok_or_else(|| Error::invalid_params("Invalid 'timelock' parameter.")) + }) + .transpose()?; - let res = control.create_recovery(address, feerate)?; + let res = control.create_recovery(address, feerate, timelock)?; Ok(serde_json::json!(&res)) } diff --git a/tests/test_rpc.py b/tests/test_rpc.py index 2e8cc87a..55c76858 100644 --- a/tests/test_rpc.py +++ b/tests/test_rpc.py @@ -554,7 +554,7 @@ def test_create_recovery(lianad, bitcoind): # There's nothing to sweep with pytest.raises( RpcError, - match="No coin currently available through the timelocked recovery path", + match="No coin currently spendable through this timelocked recovery path", ): lianad.rpc.createrecovery(bitcoind.rpc.getnewaddress(), 2) From 75ea3672357e30418c49fa4d444f85d7d248e0ba Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 28 Mar 2023 15:16:39 +0200 Subject: [PATCH 5/9] tests: make most of the body of the multisig test a general helper We'll exercise the same commands for the multipath test, except for the recovery. DRY. --- tests/test_misc.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/test_misc.py b/tests/test_misc.py index ff709b98..483c3f3c 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -5,11 +5,7 @@ from test_framework.serializations import PSBT from test_framework.utils import wait_for, RpcError -def test_multisig(lianad_multisig, bitcoind): - """Test using lianad with a descriptor that contains multiple keys for both - the primary and recovery paths.""" - lianad = lianad_multisig - +def receive_and_send(lianad, bitcoind): # Receive 3 coins in different blocks on different addresses. for _ in range(3): addr = lianad.rpc.getnewaddress()["address"] @@ -17,7 +13,6 @@ def test_multisig(lianad_multisig, bitcoind): bitcoind.generate_block(1, wait_for_mempool=txid) wait_for(lambda: len(lianad.rpc.listcoins()["coins"]) == 3) - print(lianad.rpc.listcoins()) # Create a spend that will create a change output, sign and broadcast it. outpoints = [lianad.rpc.listcoins()["coins"][0]["outpoint"]] destinations = { @@ -64,14 +59,20 @@ def test_multisig(lianad_multisig, bitcoind): lambda: lianad.rpc.getinfo()["block_height"] == bitcoind.rpc.getblockcount() ) + +def test_multisig(lianad_multisig, bitcoind): + """Test using lianad with a descriptor that contains multiple keys for both + the primary and recovery paths.""" + receive_and_send(lianad_multisig, bitcoind) + # Sweep all coins through the recovery path. It needs 2 signatures out of # 5 keys. Sign with the second and the fifth ones. - res = lianad.rpc.createrecovery(bitcoind.rpc.getnewaddress(), 2) + res = lianad_multisig.rpc.createrecovery(bitcoind.rpc.getnewaddress(), 2) reco_psbt = PSBT.from_base64(res["psbt"]) txid = reco_psbt.tx.txid().hex() - signed_psbt = lianad.signer.sign_psbt(reco_psbt, [1, 4], recovery=True) - lianad.rpc.updatespend(signed_psbt.to_base64()) - lianad.rpc.broadcastspend(txid) + signed_psbt = lianad_multisig.signer.sign_psbt(reco_psbt, [1, 4], recovery=True) + lianad_multisig.rpc.updatespend(signed_psbt.to_base64()) + lianad_multisig.rpc.broadcastspend(txid) def test_coinbase_deposit(lianad, bitcoind): From 3aa99806359efd7043b79becd71ad26877e63a9e Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 28 Mar 2023 15:22:57 +0200 Subject: [PATCH 6/9] tests: simplify the MultiSigner --- tests/fixtures.py | 6 +++--- tests/test_framework/signer.py | 4 +--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/fixtures.py b/tests/fixtures.py index 57d1036f..0ef108a5 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -163,11 +163,11 @@ def lianad_multisig(bitcoind, directory): bitcoind_cookie = os.path.join(bitcoind.bitcoin_dir, "regtest", ".cookie") # A 3-of-4 that degrades into a 2-of-5 after 10 blocks - signer = MultiSigner(3, 4, 2, 5) + signer = MultiSigner(4, 5) csv_value = 10 prim_multi, recov_multi = ( - multi_expression(signer.prim_thresh, signer.prim_hds), - multi_expression(signer.recov_thresh, signer.recov_hds), + multi_expression(3, signer.prim_hds), + multi_expression(2, signer.recov_hds), ) main_desc = Descriptor.from_str( f"wsh(or_d({prim_multi},and_v(v:{recov_multi},older({csv_value}))))" diff --git a/tests/test_framework/signer.py b/tests/test_framework/signer.py index a4f40f23..e1af3772 100644 --- a/tests/test_framework/signer.py +++ b/tests/test_framework/signer.py @@ -77,14 +77,12 @@ class SingleSigner: class MultiSigner: def __init__( - self, primary_thresh, primary_hds_count, recovery_thresh, recovery_hds_count + self, primary_hds_count, recovery_hds_count ): - self.prim_thresh = primary_thresh self.prim_hds = [ BIP32.from_seed(os.urandom(32), network="test") for _ in range(primary_hds_count) ] - self.recov_thresh = recovery_thresh self.recov_hds = [ BIP32.from_seed(os.urandom(32), network="test") for _ in range(recovery_hds_count) From 0d75f0a2c77b05b247adac3a08371203d12ac1d6 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 28 Mar 2023 15:48:42 +0200 Subject: [PATCH 7/9] tests: handle multiple recovery paths in MultiSigner --- tests/fixtures.py | 4 ++-- tests/test_framework/signer.py | 35 +++++++++++++++++++++++----------- tests/test_misc.py | 2 +- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/tests/fixtures.py b/tests/fixtures.py index 0ef108a5..4122e13b 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -163,11 +163,11 @@ def lianad_multisig(bitcoind, directory): bitcoind_cookie = os.path.join(bitcoind.bitcoin_dir, "regtest", ".cookie") # A 3-of-4 that degrades into a 2-of-5 after 10 blocks - signer = MultiSigner(4, 5) csv_value = 10 + signer = MultiSigner(4, {csv_value: 5}) prim_multi, recov_multi = ( multi_expression(3, signer.prim_hds), - multi_expression(2, signer.recov_hds), + multi_expression(2, signer.recov_hds[csv_value]), ) main_desc = Descriptor.from_str( f"wsh(or_d({prim_multi},and_v(v:{recov_multi},older({csv_value}))))" diff --git a/tests/test_framework/signer.py b/tests/test_framework/signer.py index e1af3772..9e0f86cc 100644 --- a/tests/test_framework/signer.py +++ b/tests/test_framework/signer.py @@ -59,6 +59,8 @@ def sign_psbt(psbt, hds): class SingleSigner: + """Assumes a simple 1-primary path 1-recovery path Liana descriptor.""" + def __init__(self): self.primary_hd = BIP32.from_seed(os.urandom(32), network="test") self.recovery_hd = BIP32.from_seed(os.urandom(32), network="test") @@ -76,20 +78,31 @@ class SingleSigner: class MultiSigner: - def __init__( - self, primary_hds_count, recovery_hds_count - ): + """A signer that has multiple keys and may have multiple recovery path.""" + + def __init__(self, primary_hds_count, recovery_hds_counts): self.prim_hds = [ BIP32.from_seed(os.urandom(32), network="test") for _ in range(primary_hds_count) ] - self.recov_hds = [ - BIP32.from_seed(os.urandom(32), network="test") - for _ in range(recovery_hds_count) - ] + self.recov_hds = {} + for timelock, count in recovery_hds_counts.items(): + self.recov_hds[timelock] = [ + BIP32.from_seed(os.urandom(32), network="test") for _ in range(count) + ] - def sign_psbt(self, psbt, key_indices, recovery=False): - """Sign a transaction with the keys at the specified indices.""" - hds = self.recov_hds if recovery else self.prim_hds - hds = [hds[i] for i in key_indices] + def sign_psbt(self, psbt, key_indices): + """Sign a transaction with the keys at the specified indices. + + The key indices may be specified as a mapping from timelock value to list of + indices to sign with the keys of a specific recovery path. + """ + if isinstance(key_indices, dict): + hds = [ + self.recov_hds[timelock][i] + for timelock, indices in key_indices.items() + for i in indices + ] + else: + hds = [self.prim_hds[i] for i in key_indices] return sign_psbt(psbt, hds) diff --git a/tests/test_misc.py b/tests/test_misc.py index 483c3f3c..5222f713 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -70,7 +70,7 @@ def test_multisig(lianad_multisig, bitcoind): res = lianad_multisig.rpc.createrecovery(bitcoind.rpc.getnewaddress(), 2) reco_psbt = PSBT.from_base64(res["psbt"]) txid = reco_psbt.tx.txid().hex() - signed_psbt = lianad_multisig.signer.sign_psbt(reco_psbt, [1, 4], recovery=True) + signed_psbt = lianad_multisig.signer.sign_psbt(reco_psbt, {10: [1, 4]}) lianad_multisig.rpc.updatespend(signed_psbt.to_base64()) lianad_multisig.rpc.broadcastspend(txid) From b20cdd0cd81ed0f90ce36192aa8607206c12f3df Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 28 Mar 2023 15:57:35 +0200 Subject: [PATCH 8/9] tests: introduce a lianad_multipath fixture as well as a trivial test --- tests/fixtures.py | 34 ++++++++++++++++++++++++++++++++++ tests/test_misc.py | 6 ++++++ 2 files changed, 40 insertions(+) diff --git a/tests/fixtures.py b/tests/fixtures.py index 4122e13b..f36d24cb 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -189,3 +189,37 @@ def lianad_multisig(bitcoind, directory): raise lianad.cleanup() + + +@pytest.fixture +def lianad_multipath(bitcoind, directory): + datadir = os.path.join(directory, "lianad") + os.makedirs(datadir, exist_ok=True) + bitcoind_cookie = os.path.join(bitcoind.bitcoin_dir, "regtest", ".cookie") + + # A 3-of-4 that degrades into a 3-of-5 after 10 blocks and into a 1-of-10 after 20 blocks. + csv_values = [10, 20] + signer = MultiSigner(4, {csv_values[0]: 5, csv_values[1]: 10}) + prim_multi = multi_expression(3, signer.prim_hds) + first_recov_multi = multi_expression(3, signer.recov_hds[csv_values[0]]) + second_recov_multi = multi_expression(1, signer.recov_hds[csv_values[1]]) + main_desc = Descriptor.from_str( + f"wsh(or_d({prim_multi},or_i(and_v(v:{first_recov_multi},older({csv_values[0]})),and_v(v:{second_recov_multi},older({csv_values[1]})))))" + ) + + lianad = Lianad( + datadir, + signer, + main_desc, + bitcoind.rpcport, + bitcoind_cookie, + ) + + try: + lianad.start() + yield lianad + except Exception: + lianad.cleanup() + raise + + lianad.cleanup() diff --git a/tests/test_misc.py b/tests/test_misc.py index 5222f713..6ea3b285 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -75,6 +75,12 @@ def test_multisig(lianad_multisig, bitcoind): lianad_multisig.rpc.broadcastspend(txid) +def test_multipath(lianad_multipath, bitcoind): + """Exercise various commands as well as recovery with a descriptor with multiple + recovery paths.""" + receive_and_send(lianad_multipath, bitcoind) + + def test_coinbase_deposit(lianad, bitcoind): """Check we detect deposits from (mature) coinbase transactions.""" # Create a new deposit in a coinbase transaction. From 7211b96ca936a449916b0cca3e0a64b09463e813 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 28 Mar 2023 17:19:00 +0200 Subject: [PATCH 9/9] tests: extend the recovery functional tests for multipath descriptors --- tests/test_misc.py | 103 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 96 insertions(+), 7 deletions(-) diff --git a/tests/test_misc.py b/tests/test_misc.py index 6ea3b285..f268ec99 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -44,7 +44,9 @@ def receive_and_send(lianad, bitcoind): psbt = PSBT.from_base64(res["psbt"]) txid = psbt.tx.txid().hex() # If we sign only with two keys it won't be able to finalize - with pytest.raises(RpcError, match="Miniscript Error: could not satisfy at index 0"): + with pytest.raises( + RpcError, match="Miniscript Error: could not satisfy at index 0" + ): signed_psbt = lianad.signer.sign_psbt(psbt, range(2)) lianad.rpc.updatespend(signed_psbt.to_base64()) lianad.rpc.broadcastspend(txid) @@ -52,12 +54,7 @@ def receive_and_send(lianad, bitcoind): signed_psbt = lianad.signer.sign_psbt(psbt, range(1, 4)) lianad.rpc.updatespend(signed_psbt.to_base64()) lianad.rpc.broadcastspend(txid) - - # Generate 10 blocks to test the recovery path - bitcoind.generate_block(10, wait_for_mempool=txid) - wait_for( - lambda: lianad.rpc.getinfo()["block_height"] == bitcoind.rpc.getblockcount() - ) + bitcoind.generate_block(1, wait_for_mempool=txid) def test_multisig(lianad_multisig, bitcoind): @@ -65,6 +62,13 @@ def test_multisig(lianad_multisig, bitcoind): the primary and recovery paths.""" receive_and_send(lianad_multisig, bitcoind) + # Generate 10 blocks to test the recovery path + bitcoind.generate_block(10) + wait_for( + lambda: lianad_multisig.rpc.getinfo()["block_height"] + == bitcoind.rpc.getblockcount() + ) + # Sweep all coins through the recovery path. It needs 2 signatures out of # 5 keys. Sign with the second and the fifth ones. res = lianad_multisig.rpc.createrecovery(bitcoind.rpc.getnewaddress(), 2) @@ -80,6 +84,91 @@ def test_multipath(lianad_multipath, bitcoind): recovery paths.""" receive_and_send(lianad_multipath, bitcoind) + # Generate 10 blocks to test the recovery path + bitcoind.generate_block(10) + wait_for( + lambda: lianad_multipath.rpc.getinfo()["block_height"] + == bitcoind.rpc.getblockcount() + ) + + # We can't create a recovery tx for the second recovery path, as all coins were confirmed + # within the last 19 blocks. + with pytest.raises( + RpcError, + match="No coin currently spendable through this timelocked recovery path", + ): + lianad_multipath.rpc.createrecovery(bitcoind.rpc.getnewaddress(), 2, 20) + + # Sweep all coins through the first recovery path (that is available after 10 blocks). + # It needs 3 signatures out of 5 keys. + res = lianad_multipath.rpc.createrecovery(bitcoind.rpc.getnewaddress(), 2) + reco_psbt = PSBT.from_base64(res["psbt"]) + txid = reco_psbt.tx.txid().hex() + + # Try to sign with the keys for the next recovery spending path, it'll fail. + signed_psbt = lianad_multipath.signer.sign_psbt(reco_psbt, {20: range(3)}) + lianad_multipath.rpc.updatespend(signed_psbt.to_base64()) + with pytest.raises(RpcError, match="Failed to finalize"): + lianad_multipath.rpc.broadcastspend(txid) + + # Try to sign with the right keys but only two of them, it'll fail. + signed_psbt = lianad_multipath.signer.sign_psbt(reco_psbt, {10: range(2)}) + lianad_multipath.rpc.updatespend(signed_psbt.to_base64()) + with pytest.raises(RpcError, match="Failed to finalize"): + lianad_multipath.rpc.broadcastspend(txid) + + # Finally add one more signature with an unused key from the right keyset. + signed_psbt = lianad_multipath.signer.sign_psbt(reco_psbt, {10: [2]}) + lianad_multipath.rpc.updatespend(signed_psbt.to_base64()) + lianad_multipath.rpc.broadcastspend(txid) + + # Receive 3 more coins and make the second recovery path (20 blocks) available. + txids = [] + for _ in range(3): + addr = lianad_multipath.rpc.getnewaddress()["address"] + txids.append(bitcoind.rpc.sendtoaddress(addr, 0.42)) + bitcoind.generate_block(20, wait_for_mempool=txids) + wait_for( + lambda: lianad_multipath.rpc.getinfo()["block_height"] + == bitcoind.rpc.getblockcount() + ) + + # We can create a recovery transaction for an earlier timelock. + lianad_multipath.rpc.createrecovery(bitcoind.rpc.getnewaddress(), 2) + + # Sweep all coins through the second recovery path (that is available after 20 blocks). + # It needs 3 signatures out of 5 keys. + res = lianad_multipath.rpc.createrecovery(bitcoind.rpc.getnewaddress(), 2, 20) + reco_psbt = PSBT.from_base64(res["psbt"]) + txid = reco_psbt.tx.txid().hex() + + # We can sign with any keys for the second recovery path (we need only 1 out of 10) + signed_psbt = lianad_multipath.signer.sign_psbt(reco_psbt, {20: [8]}) + lianad_multipath.rpc.updatespend(signed_psbt.to_base64()) + lianad_multipath.rpc.broadcastspend(txid) + + # Now do this again but with signing using keys for the first recovery path. + # Receive 3 more coins and make the second recovery path (20 blocks) available. Note this + # is possible since the CSV checks the nSequence is >= to the value, not ==. + txids = [] + for _ in range(3): + addr = lianad_multipath.rpc.getnewaddress()["address"] + txids.append(bitcoind.rpc.sendtoaddress(addr, 0.398)) + bitcoind.generate_block(20, wait_for_mempool=txids) + wait_for( + lambda: lianad_multipath.rpc.getinfo()["block_height"] + == bitcoind.rpc.getblockcount() + ) + # Sweep all coins through the second recovery path (that is available after 20 blocks). + # It needs 3 signatures out of 5 keys. + res = lianad_multipath.rpc.createrecovery(bitcoind.rpc.getnewaddress(), 2, 20) + reco_psbt = PSBT.from_base64(res["psbt"]) + txid = reco_psbt.tx.txid().hex() + # We can sign with keys for the first recovery path (we need 3 out of 5) + signed_psbt = lianad_multipath.signer.sign_psbt(reco_psbt, {10: range(2, 5)}) + lianad_multipath.rpc.updatespend(signed_psbt.to_base64()) + lianad_multipath.rpc.broadcastspend(txid) + def test_coinbase_deposit(lianad, bitcoind): """Check we detect deposits from (mature) coinbase transactions."""