Skip to content

Commit

Permalink
descriptors: allow the multipath step to be different than '<0;1>'
Browse files Browse the repository at this point in the history
Ledger and wallet policies disallow having more than 2 depth after the
placeholder, therefore we can't do `@1/0/<0;1>/*`, `@1/1/<0;1>/*`, ..
Instead we have to do `@1/<0;1>/*`, `@1/<2;3>/*`, ..

Why not? Salvatore also says the cost of deriving another depth is
non-trivial on a signing device.

Don't pick a fight with Salvatore, instead just let the GUI (or whatever
creates the desc) use different multipath steps for keys derived from
the same xpubs.
  • Loading branch information
darosior committed Aug 10, 2023
1 parent 10acc48 commit 605c3f6
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 6 deletions.
8 changes: 2 additions & 6 deletions src/descriptors/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ impl DescKeyChecker {
/// 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.
/// - 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)
/// - 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 {
let key_identifier = (xpub.xkey, xpub.derivation_paths.clone());
Expand All @@ -94,8 +94,6 @@ impl DescKeyChecker {
// Then perform the contextless checks.
let der_paths = xpub.derivation_paths.paths();
let first_der_path = der_paths.get(0).expect("Cannot be empty");
// Rust-miniscript enforces BIP389 which states that all paths must have the same len.
let len = first_der_path.len();
// 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
Expand All @@ -104,8 +102,6 @@ impl DescKeyChecker {
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()
&& first_der_path.into_iter().all(|step| step.is_normal());
if valid {
return Ok(());
Expand Down
27 changes: 27 additions & 0 deletions src/descriptors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,33 @@ mod tests {
.unwrap();
assert_eq!(LianaDescriptor::new(policy).to_string(), "wsh(or_d(multi(3,[aabb0011/48'/0'/0'/2']xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/0/<0;1>/*,[aabb0012/48'/0'/0'/2']xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/0/<0;1>/*,[aabb0013/48'/0'/0'/2']xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/0/<0;1>/*),and_v(v:thresh(2,pkh([aabb0011/48'/0'/0'/2']xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/1/<0;1>/*),a:pkh([aabb0012/48'/0'/0'/2']xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/1/<0;1>/*),a:pkh([aabb0013/48'/0'/0'/2']xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/1/<0;1>/*)),older(26352))))#prj7nktq");

// The same pseudo-realistic situation as above, but instead of using another derivation
// depth to derive from the same xpub, we reuse the multipath step.
// Note: this is required by Ledger and the wallet policies BIP
// (https://github.com/bitcoin/bips/pull/1389)
let primary_keys = PathInfo::Multi(
3,
vec![
descriptor::DescriptorPublicKey::from_str("[aabb0011/48'/0'/0'/2']xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap(),
descriptor::DescriptorPublicKey::from_str("[aabb0012/48'/0'/0'/2']xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*").unwrap(),
descriptor::DescriptorPublicKey::from_str("[aabb0013/48'/0'/0'/2']xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/<0;1>/*").unwrap()
]
);
let recovery_keys = PathInfo::Multi(
2,
vec![
descriptor::DescriptorPublicKey::from_str("[aabb0011/48'/0'/0'/2']xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<2;3>/*").unwrap(),
descriptor::DescriptorPublicKey::from_str("[aabb0012/48'/0'/0'/2']xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<2;3>/*").unwrap(),
descriptor::DescriptorPublicKey::from_str("[aabb0013/48'/0'/0'/2']xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/<2;3>/*").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,[aabb0011/48'/0'/0'/2']xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*,[aabb0012/48'/0'/0'/2']xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<0;1>/*,[aabb0013/48'/0'/0'/2']xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/<0;1>/*),and_v(v:thresh(2,pkh([aabb0011/48'/0'/0'/2']xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<2;3>/*),a:pkh([aabb0012/48'/0'/0'/2']xpub6Bw79HbNSeS2xXw1sngPE3ehnk1U3iSPCgLYzC9LpN8m9nDuaKLZvkg8QXxL5pDmEmQtYscmUD8B9MkAAZbh6vxPzNXMaLfGQ9Sb3z85qhR/<2;3>/*),a:pkh([aabb0013/48'/0'/0'/2']xpub67zuTXF9Ln4731avKTBSawoVVNRuMfmRvkL7kLUaLBRqma9ZqdHBJg9qx8cPUm3oNQMiXT4TmGovXNoQPuwg17RFcVJ8YrnbcooN7pxVJqC/<2;3>/*)),older(26352))))#d2h994td");

// We prevent footguns with timelocks by requiring a u16. Note how the following wouldn't
// compile:
//LianaPolicy::new(owner_key.clone(), heir_key.clone(), 0x00_01_0f_00).unwrap_err();
Expand Down

0 comments on commit 605c3f6

Please sign in to comment.