From 0a95266cce618f83782d3d30362cb2d383b659bf Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Fri, 6 Oct 2023 18:17:45 +0200 Subject: [PATCH 1/2] qa: don't use a static dummy origin for descriptor xpubs --- tests/fixtures.py | 20 +++++++++--- tests/test_misc.py | 77 +++++++++++++++++++++++++--------------------- 2 files changed, 57 insertions(+), 40 deletions(-) diff --git a/tests/fixtures.py b/tests/fixtures.py index 1c4c8624..ee8d5b7c 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -1,4 +1,5 @@ from bip32 import BIP32 +from bip32.utils import _pubkey_to_fingerprint from bip380.descriptors import Descriptor from concurrent import futures from test_framework.bitcoind import Bitcoind @@ -115,6 +116,10 @@ def bitcoind(directory): bitcoind.cleanup() +def xpub_fingerprint(hd): + return _pubkey_to_fingerprint(hd.pubkey).hex() + + @pytest.fixture def lianad(bitcoind, directory): datadir = os.path.join(directory, "lianad") @@ -122,13 +127,15 @@ def lianad(bitcoind, directory): bitcoind_cookie = os.path.join(bitcoind.bitcoin_dir, "regtest", ".cookie") signer = SingleSigner() - primary_xpub, recovery_xpub = ( - signer.primary_hd.get_xpub(), - signer.recovery_hd.get_xpub(), + (prim_fingerprint, primary_xpub), (reco_fingerprint, recovery_xpub) = ( + (xpub_fingerprint(signer.primary_hd), signer.primary_hd.get_xpub()), + (xpub_fingerprint(signer.recovery_hd), signer.recovery_hd.get_xpub()), ) csv_value = 10 + # NOTE: origins are the actual xpub themselves which is incorrect but make it + # possible to differentiate them. main_desc = Descriptor.from_str( - f"wsh(or_d(pk([aabbccdd]{primary_xpub}/<0;1>/*),and_v(v:pkh([aabbccdd]{recovery_xpub}/<0;1>/*),older({csv_value}))))" + f"wsh(or_d(pk([{prim_fingerprint}]{primary_xpub}/<0;1>/*),and_v(v:pkh([{reco_fingerprint}]{recovery_xpub}/<0;1>/*),older({csv_value}))))" ) lianad = Lianad( @@ -152,7 +159,10 @@ def lianad(bitcoind, directory): def multi_expression(thresh, keys): exp = f"multi({thresh}," for i, key in enumerate(keys): - exp += f"[aabbccdd]{key.get_xpub()}/<0;1>/*" + # NOTE: origins are the actual xpub themselves which is incorrect but make it + # possible to differentiate them. + fingerprint = xpub_fingerprint(key) + exp += f"[{fingerprint}]{key.get_xpub()}/<0;1>/*" if i != len(keys) - 1: exp += "," return exp + ")" diff --git a/tests/test_misc.py b/tests/test_misc.py index 55841759..a1b23e6b 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -127,11 +127,16 @@ def test_multipath(lianad_multipath, bitcoind): reco_psbt = PSBT.from_base64(res["psbt"]) txid = reco_psbt.tx.txid().hex() + # NOTE: this test was commented out due to the introduced restriction to not include + # the BIP32 derivations for other spending paths in PSBT inputs to support the Bitbox2 + # signing device (and most likely others). + # TODO: reintroduce these tests once we get rid of this restriction. + # 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) + # 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)}) @@ -144,52 +149,54 @@ def test_multipath(lianad_multipath, bitcoind): lianad_multipath.rpc.updatespend(signed_psbt.to_base64()) lianad_multipath.rpc.broadcastspend(txid) + # NOTE: commented out for the same reason as above. + # 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() - ) + # 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) + # 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() + # 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) + # 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() - ) + # 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() + # 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) + # 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): From 730409eb5299724cdbe1a4da06d272098a791621 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Sat, 7 Oct 2023 13:49:58 +0200 Subject: [PATCH 2/2] descriptors: prevent using a signer more than once in a single path This is necessary to support signers which sign for a single key at once. It also doesn't make any sense to reuse a signer within the same spending path, so rule it out before it creates any new edge cases. For more about the Bitbox signer support, which motivated this change, see https://github.com/wizardsardine/liana/pull/706#issuecomment-1744705808. --- src/descriptors/analysis.rs | 59 +++++++++++++++++++++++--------- src/descriptors/mod.rs | 68 +++++++++++++++++++++++++++++++------ src/signer.rs | 21 ++++++------ 3 files changed, 111 insertions(+), 37 deletions(-) diff --git a/src/descriptors/analysis.rs b/src/descriptors/analysis.rs index cab06e13..b106a068 100644 --- a/src/descriptors/analysis.rs +++ b/src/descriptors/analysis.rs @@ -17,6 +17,8 @@ pub enum LianaPolicyError { InsaneTimelock(u32), InvalidKey(Box), DuplicateKey(Box), + /// The same signer was used more than once in a single spending path. + DuplicateOriginSamePath(Box), InvalidMultiThresh(usize), InvalidMultiKeys(usize), IncompatibleDesc, @@ -44,6 +46,9 @@ impl std::fmt::Display for LianaPolicyError { Self::DuplicateKey(key) => { write!(f, "Duplicate key '{}'.", key) } + Self::DuplicateOriginSamePath(key) => { + write!(f, "Key '{}' is derived from the same origin as another key present in the same spending path. It is not possible to use a signer more than once within a single spending path.", key) + } Self::IncompatibleDesc => write!( f, "Descriptor is not compatible with a Liana spending policy." @@ -83,7 +88,13 @@ impl DescKeyChecker { /// - The multipath step to only contain two indexes. These can be any indexes, which is /// useful for deriving multiple keys from the same xpub. /// - Be 'signable' by an external signer (to contain an origin) - pub fn check(&mut self, key: &descriptor::DescriptorPublicKey) -> Result<(), LianaPolicyError> { + /// + /// This returns the origin fingerprint for this xpub, to make it possible for the caller to + /// check the same signer is never used twice in the same spending path. + pub fn check( + &mut self, + key: &descriptor::DescriptorPublicKey, + ) -> Result { if let descriptor::DescriptorPublicKey::MultiXPub(ref xpub) = *key { let key_identifier = (xpub.xkey, xpub.derivation_paths.clone()); // First make sure it's not a duplicate and record seeing it. @@ -91,20 +102,21 @@ impl DescKeyChecker { return Err(LianaPolicyError::DuplicateKey(key.clone().into())); } self.keys_set.insert(key_identifier); - // Then perform the contextless checks. - let der_paths = xpub.derivation_paths.paths(); - let first_der_path = der_paths.get(0).expect("Cannot be empty"); + // Then perform the contextless checks (origin, deriv paths, ..). // Technically the xpub could be for the master xpub and not have an origin. But it's // 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. - // We also rule out xpubs with hardened derivation steps (non-normalized xpubs). - let valid = xpub.origin.is_some() - && xpub.wildcard == descriptor::Wildcard::Unhardened - && der_paths.len() == 2 - && first_der_path.into_iter().all(|step| step.is_normal()); - if valid { - return Ok(()); + if let Some(ref origin) = xpub.origin { + let der_paths = xpub.derivation_paths.paths(); + let first_der_path = der_paths.get(0).expect("Cannot be empty"); + // We also rule out xpubs with hardened derivation steps (non-normalized xpubs). + let valid = xpub.wildcard == descriptor::Wildcard::Unhardened + && der_paths.len() == 2 + && first_der_path.into_iter().all(|step| step.is_normal()); + if valid { + return Ok(origin.0); + } } } Err(LianaPolicyError::InvalidKey(key.clone().into())) @@ -387,10 +399,24 @@ impl LianaPolicy { let mut key_checker = DescKeyChecker::new(); for path in spending_paths { match path { - PathInfo::Single(ref key) => key_checker.check(key)?, + PathInfo::Single(ref key) => { + let _ = key_checker.check(key)?; + } PathInfo::Multi(_, ref keys) => { + // Record the origins of the keys for this spending path. If any two keys share + // the same origin, they are from the same signer. We restrict using a signer + // more than once within a single spending path as it can lead to surprising + // behaviour. For details see: + // https://github.com/wizardsardine/liana/pull/706#issuecomment-1744705808 + let mut origin_fingerprints = HashSet::with_capacity(keys.len()); for key in keys { - key_checker.check(key)? + let fg = key_checker.check(key)?; + if origin_fingerprints.contains(&fg) { + return Err(LianaPolicyError::DuplicateOriginSamePath( + key.clone().into(), + )); + } + origin_fingerprints.insert(fg); } } } @@ -436,8 +462,8 @@ impl LianaPolicy { } .ok_or(LianaPolicyError::IncompatibleDesc)?; - // Fetch the two spending paths' semantic policies. The primary path is identified as the - // only one that isn't timelocked. + // Fetch all spending paths' semantic policies. The primary path is identified as the only + // one that isn't timelocked. 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. @@ -456,7 +482,8 @@ impl LianaPolicy { primary_path = Some(PathInfo::from_primary_path(sub)?); } } else { - // If it's not a simple (multi)key check, it must be the timelocked recovery path. + // If it's not a simple (multi)key check, it must be (one of) the timelocked + // recovery path(s). let (timelock, path_info) = PathInfo::from_recovery_path(sub)?; if recovery_paths.contains_key(&timelock) { return Err(LianaPolicyError::IncompatibleDesc); diff --git a/src/descriptors/mod.rs b/src/descriptors/mod.rs index 574b153c..941e4d6f 100644 --- a/src/descriptors/mod.rs +++ b/src/descriptors/mod.rs @@ -583,23 +583,71 @@ mod tests { 3, vec![ descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap(), - descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*").unwrap(), - descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/<0;1>/*").unwrap() + descriptor::DescriptorPublicKey::from_str("[abcdef02]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*").unwrap(), + descriptor::DescriptorPublicKey::from_str("[abcdef03]xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/<0;1>/*").unwrap() ] ); let recovery_keys = PathInfo::Multi( 2, vec![ - descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*").unwrap(), - descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6AA2N8RALRYgLD6jT1iXYCEDkndTeZndMtWPbtNX6sY5dPiLtf2T88ahdxrGXMUPoNadgR86sFhBXWQVgifPzDYbY9ZtwK4gqzx4y5Da1DW/<0;1>/*").unwrap(), - descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*").unwrap(), + descriptor::DescriptorPublicKey::from_str("[abcdef05]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*").unwrap(), + descriptor::DescriptorPublicKey::from_str("[abcdef04]xpub6AA2N8RALRYgLD6jT1iXYCEDkndTeZndMtWPbtNX6sY5dPiLtf2T88ahdxrGXMUPoNadgR86sFhBXWQVgifPzDYbY9ZtwK4gqzx4y5Da1DW/<0;1>/*").unwrap(), + descriptor::DescriptorPublicKey::from_str("[abcdef02]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*").unwrap(), ], ); - LianaPolicy::new( + let err = LianaPolicy::new( primary_keys, [(26352, recovery_keys)].iter().cloned().collect(), ) .unwrap_err(); + assert!(matches!(err, LianaPolicyError::DuplicateKey(_))); + + // You can't pass duplicate signers in the primary path. + let primary_keys = PathInfo::Multi( + 2, + vec![ + descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap(), + descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*").unwrap(), + ] + ); + let recovery_keys = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef02]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*").unwrap()); + let err = LianaPolicy::new( + primary_keys, + [(26352, recovery_keys)].iter().cloned().collect(), + ) + .unwrap_err(); + assert!(matches!(err, LianaPolicyError::DuplicateOriginSamePath(_))); + + // You can't pass duplicate signers in the recovery path. + let recovery_keys = PathInfo::Multi( + 2, + vec![ + descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap(), + descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*").unwrap(), + ] + ); + let primary_keys = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef02]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*").unwrap()); + let err = LianaPolicy::new( + primary_keys, + [(26352, recovery_keys)].iter().cloned().collect(), + ) + .unwrap_err(); + assert!(matches!(err, LianaPolicyError::DuplicateOriginSamePath(_))); + + // But the same signer can absolutely be used across spending paths. + let primary_keys = PathInfo::Multi( + 2, + vec![ + descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap(), + descriptor::DescriptorPublicKey::from_str("[abcdef02]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*").unwrap(), + ] + ); + let recovery_keys = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*").unwrap()); + LianaPolicy::new( + primary_keys, + [(26352, recovery_keys)].iter().cloned().collect(), + ) + .unwrap(); // No origin in one of the keys let owner_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap()); @@ -614,7 +662,7 @@ mod tests { 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(); + LianaDescriptor::from_str("wsh(or_d(multi(1,[573fb35b/48'/1'/0'/2']tpubDFKp9T7WAYDcENSjoifkrpq1gMDF47KGJcJrpxzX23Qor8wuGbrEVs9utNq1MDS8E2WXJSBk1qoPQLpwyokW7DiUNPwFuxQkL7owNkLAb9W/<0;1>/*,[573fb35c/48'/1'/1'/2']tpubDFGezyzuHJPhdP3jHGW7v7Hwes4Hihqv5W2yyCmRY9VZJCRchETvxrMC8uECeJZdxQ14V4iD4DecoArkUSDwj8ogYE9WEv4MNZr12thNHCs/<0;1>/*),and_v(v:multi(2,[573fb35b/48'/1'/2'/2']tpubDDwxQauiaU964vPzt5Vd7jnDHEUtp2Vc34PaWpEXg5TQ3bRccxnc1MKKh88Hi7xiMeZo9Tm6fBcq4UGXqnDtGUniJLjqAD8SjQ8Eci3aSR7/<0;1>/*,[573fb35c/48'/1'/3'/2']tpubDE37XAVB5CQ1x85md3BQ5uHCoMwT5fgT8X13zzCUQ3x5o2jskYxKjj7Qcxt1Jpj4QB8tqspn2dooPCekRuQDYrDHov7J1ueUNu2wcvgRDxr/<0;1>/*),older(1000))))#fccaqlhh").unwrap(); } #[test] @@ -771,11 +819,11 @@ mod tests { // A descriptor with single keys in both primary and recovery paths roundtrip("wsh(or_d(pk([aabbccdd]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*),and_v(v:pkh([aabbccdd]xpub688Hn4wScQAAiYJLPg9yH27hUpfZAUnmJejRQBCiwfP5PEDzjWMNW1wChcninxr5gyavFqbbDjdV1aK5USJz8NDVjUy7FRQaaqqXHh5SbXe/<0;1>/*),older(52560))))#7437yjrs"); // One with a multisig in both paths - roundtrip("wsh(or_d(multi(3,[aabbccdd]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*,[aabb0011/10/4893]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*,[aabbccdd]xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/<0;1>/*),and_v(v:multi(2,[aabbccdd]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*,[aabbccdd]xpub6AA2N8RALRYgLD6jT1iXYCEDkndTeZndMtWPbtNX6sY5dPiLtf2T88ahdxrGXMUPoNadgR86sFhBXWQVgifPzDYbY9ZtwK4gqzx4y5Da1DW/<0;1>/*,[aabb0011/10/4893]xpub6AyxexvxizZJffF153evmfqHcE9MV88fCNCAtP3jQjXJHwrAKri71Tq9jWUkPxj9pja4u6AkCPHY7atgxzSEa2HtDwJfrRWKK4fsfQg4o77/<0;1>/*),older(26352))))#ypwt7h7e"); + roundtrip("wsh(or_d(multi(3,[aabbccdd]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*,[aabb0011/10/4893]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*,[aabb0022]xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/<0;1>/*),and_v(v:multi(2,[aabbccdd]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*,[aabb0011]xpub6AA2N8RALRYgLD6jT1iXYCEDkndTeZndMtWPbtNX6sY5dPiLtf2T88ahdxrGXMUPoNadgR86sFhBXWQVgifPzDYbY9ZtwK4gqzx4y5Da1DW/<0;1>/*,[aabb0022/10/4893]xpub6AyxexvxizZJffF153evmfqHcE9MV88fCNCAtP3jQjXJHwrAKri71Tq9jWUkPxj9pja4u6AkCPHY7atgxzSEa2HtDwJfrRWKK4fsfQg4o77/<0;1>/*),older(26352))))#csjdk94l"); // 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"); + roundtrip("wsh(or_d(pk([aabbccdd]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*),and_v(v:multi(2,[aabbccdd]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*,[aabb0011]xpub6AA2N8RALRYgLD6jT1iXYCEDkndTeZndMtWPbtNX6sY5dPiLtf2T88ahdxrGXMUPoNadgR86sFhBXWQVgifPzDYbY9ZtwK4gqzx4y5Da1DW/<0;1>/*,[aabb0022/10/4893]xpub6AyxexvxizZJffF153evmfqHcE9MV88fCNCAtP3jQjXJHwrAKri71Tq9jWUkPxj9pja4u6AkCPHY7atgxzSEa2HtDwJfrRWKK4fsfQg4o77/<0;1>/*),older(26352))))#sc9gw0z0"); // 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]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*),older(26352))))#0y77q9d6"); + roundtrip("wsh(or_d(multi(3,[aabbccdd]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*,[aabb0011/10/4893]xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*,[aabb0022]xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/<0;1>/*),and_v(v:pk([aabbccdd]xpub69cP4Y7S9TWcbSNxmk6CEDBsoaqr3ZEdjHuZcHxEFFKGh569RsJNr2V27XGhsbH9FXgWUEmKXRN7c5wQfq2VPjt31xP9VsYnVUyU8HcVevm/<0;1>/*),older(26352))))#kjajav3j"); } fn psbt_from_str(psbt_str: &str) -> Psbt { diff --git a/src/signer.rs b/src/signer.rs index ebce7510..7197a3b8 100644 --- a/src/signer.rs +++ b/src/signer.rs @@ -362,9 +362,9 @@ mod tests { let secp = secp256k1::Secp256k1::new(); let network = bitcoin::Network::Bitcoin; - // Create a Liana descriptor with as primary path a 2-of-3 with two hot signers (2 keys are - // on the same signer) and a single hot signer as recovery path. Use various random - // derivation paths. + // Create a Liana descriptor with as primary path a 2-of-3 with three hot signers and a + // single hot signer as recovery path. (The recovery path signer is also used in the + // primary path.) Use various random derivation paths. let (prim_signer_a, prim_signer_b, recov_signer) = ( HotSigner::generate(network).unwrap(), HotSigner::generate(network).unwrap(), @@ -395,9 +395,9 @@ mod tests { wildcard: Wildcard::Unhardened, }); let origin_der = bip32::DerivationPath::from_str("m/18'/25'").unwrap(); - let xkey = prim_signer_b.xpub_at(&origin_der, &secp); + let xkey = recov_signer.xpub_at(&origin_der, &secp); let prim_key_c = DescriptorPublicKey::MultiXPub(DescriptorMultiXKey { - origin: Some((prim_signer_b.fingerprint(&secp), origin_der)), + origin: Some((recov_signer.fingerprint(&secp), origin_der)), xkey, derivation_paths: DerivPaths::new(vec![ bip32::DerivationPath::from_str("m/0").unwrap(), @@ -466,15 +466,14 @@ mod tests { outputs: Vec::new(), }; - // Sign the PSBT with the two primary signers. The second signer will sign for the two keys + // Sign the PSBT with the two primary signers. The recovery signer will sign for the two keys // that it manages. - // We can also add a signature for the recovery key with the recovery signer. let psbt = dummy_psbt.clone(); assert!(psbt.inputs[0].partial_sigs.is_empty()); let psbt = prim_signer_a.sign_psbt(psbt, &secp).unwrap(); assert_eq!(psbt.inputs[0].partial_sigs.len(), 1); let psbt = prim_signer_b.sign_psbt(psbt, &secp).unwrap(); - assert_eq!(psbt.inputs[0].partial_sigs.len(), 3); + assert_eq!(psbt.inputs[0].partial_sigs.len(), 2); let psbt = recov_signer.sign_psbt(psbt, &secp).unwrap(); assert_eq!(psbt.inputs[0].partial_sigs.len(), 4); @@ -490,7 +489,7 @@ mod tests { let psbt = prim_signer_a.sign_psbt(psbt, &secp).unwrap(); assert_eq!(psbt.inputs[0].partial_sigs.len(), 1); let psbt = prim_signer_b.sign_psbt(psbt, &secp).unwrap(); - assert_eq!(psbt.inputs[0].partial_sigs.len(), 3); + assert_eq!(psbt.inputs[0].partial_sigs.len(), 2); let psbt = recov_signer.sign_psbt(psbt, &secp).unwrap(); assert_eq!(psbt.inputs[0].partial_sigs.len(), 4); @@ -538,7 +537,7 @@ mod tests { assert!(psbt .inputs .iter() - .all(|psbt_in| psbt_in.partial_sigs.len() == 3)); + .all(|psbt_in| psbt_in.partial_sigs.len() == 2)); let psbt = recov_signer.sign_psbt(psbt, &secp).unwrap(); assert!(psbt .inputs @@ -573,7 +572,7 @@ mod tests { psbt.inputs[0].bip32_derivation.clear(); let psbt = prim_signer_b.sign_psbt(psbt, &secp).unwrap(); assert!(psbt.inputs[0].partial_sigs.is_empty()); - assert_eq!(psbt.inputs[1].partial_sigs.len(), 2); + assert_eq!(psbt.inputs[1].partial_sigs.len(), 1); } #[test]