diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 06a998ca..cea453e1 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -26,7 +26,7 @@ use std::{ use miniscript::{ bitcoin::{ - self, address, + self, address, bip32, locktime::absolute, psbt::{Input as PsbtIn, Output as PsbtOut, PartiallySignedTransaction as Psbt}, }, @@ -237,6 +237,25 @@ fn serializable_size(t: &T) -> u64 { bitcoin::consensus::serialize(t).len().try_into().unwrap() } +// Whether a given key was derived from the keys of a specific spending path. +fn is_key_for_spending_path( + pubkey: &descriptors::keys::DerivedPublicKey, + path_origins: &HashMap>, +) -> bool { + // If it comes from a signer used in this path + if let Some(der_paths) = path_origins.get(&pubkey.origin.0) { + // Get the derivation path of the parent used to derive this key. Note this is fine + // to only drop the last derivation step, because the keys in the policy are + // normalized (so the derivation path up to the wildcard is part of the origin). + if let Some((_, der_path_no_wildcard)) = pubkey.origin.1[..].split_last() { + // Now make sure it was actually derived from the xpub used in this derivation + // path, and not from the same signer used in another path. + return der_paths.contains(&(der_path_no_wildcard.into())); + } + } + false +} + impl DaemonControl { // Get the derived descriptor for this coin fn derived_desc(&self, coin: &Coin) -> descriptors::DerivedSinglePathLianaDesc { @@ -370,7 +389,7 @@ impl DaemonControl { .primary_path() .thresh_origins(); let is_prim_path_key = - |pubkey: &keys::DerivedPublicKey| prim_path_origins.contains_key(&pubkey.origin.0); + |pubkey: &keys::DerivedPublicKey| is_key_for_spending_path(pubkey, &prim_path_origins); // Iterate through given outpoints to fetch the coins (hence checking their existence // at the same time). We checked there is at least one, therefore after this loop the @@ -814,7 +833,7 @@ impl DaemonControl { .ok_or(CommandError::UnknownRecoveryTimelock(timelock))? .thresh_origins(); let is_key_for_this_path = - |pubkey: &keys::DerivedPublicKey| reco_path_origins.contains_key(&pubkey.origin.0); + |pubkey: &keys::DerivedPublicKey| is_key_for_spending_path(pubkey, &reco_path_origins); // 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. diff --git a/tests/fixtures.py b/tests/fixtures.py index ee8d5b7c..8827abf6 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -156,6 +156,41 @@ def lianad(bitcoind, directory): lianad.cleanup() +@pytest.fixture +def lianad_same_signer(bitcoind, directory): + """A simple lianad but the same signer is used for primary and recovery.""" + datadir = os.path.join(directory, "lianad") + os.makedirs(datadir, exist_ok=True) + bitcoind_cookie = os.path.join(bitcoind.bitcoin_dir, "regtest", ".cookie") + + # We use the same xpub for both paths, but /0/<0;1>/* for the primary and /1/<0;1>/* + # for the recovery. + signer = SingleSigner() + fingerprint = xpub_fingerprint(signer.primary_hd) + xpub = signer.primary_hd.get_xpub() + csv_value = 10 + main_desc = Descriptor.from_str( + f"wsh(or_d(pk([{fingerprint}]{xpub}/0/<0;1>/*),and_v(v:pkh([{fingerprint}]{xpub}/1/<0;1>/*),older({csv_value}))))" + ) + + lianad = Lianad( + datadir, + signer, + main_desc, + bitcoind.rpcport, + bitcoind_cookie, + ) + + try: + lianad.start() + yield lianad + except Exception: + lianad.cleanup() + raise + + lianad.cleanup() + + def multi_expression(thresh, keys): exp = f"multi({thresh}," for i, key in enumerate(keys): diff --git a/tests/test_framework/lianad.py b/tests/test_framework/lianad.py index dfd81cd9..0061ba5c 100644 --- a/tests/test_framework/lianad.py +++ b/tests/test_framework/lianad.py @@ -79,14 +79,14 @@ class Lianad(TailableProc): int.from_bytes(raw_der_path[i : i + 4], byteorder="little", signed=True) for i in range(0, len(raw_der_path), 4) ] - assert len(der_path) == 2 # Create a copy of the descriptor to derive it at the index used in this input. # Then create a satisfaction for it using the signature we just created. + is_change = der_path[len(der_path) - 2] == 1 desc = Descriptor.from_str( - str(self.receive_desc if der_path[0] == 0 else self.change_desc) + str(self.change_desc if is_change else self.receive_desc) ) - desc.derive(der_path[1]) + desc.derive(der_path[len(der_path) - 1]) sat_material = SatisfactionMaterial( signatures=psbt_in.map[PSBT_IN_PARTIAL_SIG], ) diff --git a/tests/test_rpc.py b/tests/test_rpc.py index a2dbc614..4e420ca9 100644 --- a/tests/test_rpc.py +++ b/tests/test_rpc.py @@ -398,14 +398,24 @@ def test_create_spend(lianad, bitcoind): res_spend = lianad.rpc.createspend(destinations, outpoints, 1) res_reco = lianad.rpc.createrecovery(bitcoind.rpc.getnewaddress(), 2) - # The two PSBTs don't share any BIP32 derivation paths in their input. + # The two PSBTs don't share any BIP32 derivation paths in their inputs. res_spend_psbt = PSBT.from_base64(res_spend["psbt"]) + res_spend_keys = set() + for i in res_spend_psbt.i: + res_spend_keys = res_spend_keys | set(i.map[PSBT_IN_BIP32_DERIVATION]) res_reco_psbt = PSBT.from_base64(res_reco["psbt"]) - res_spend_keys = set(res_spend_psbt.i[0].map[PSBT_IN_BIP32_DERIVATION]) - res_reco_keys = set(res_reco_psbt.i[0].map[PSBT_IN_BIP32_DERIVATION]) + res_reco_keys = set() + for i in res_reco_psbt.i: + res_reco_keys = res_reco_keys | set(i.map[PSBT_IN_BIP32_DERIVATION]) assert res_spend_keys.intersection(res_reco_keys) == set() +def test_create_spend_duplicate_signer(lianad_same_signer, bitcoind): + """Test spend creation (esp. around what bip32 derivations are added to the PSBT) but + with a Liana setup where a single signer is repeated between paths.""" + test_create_spend(lianad_same_signer, bitcoind) + + def test_list_spend(lianad, bitcoind): # Start by creating two conflicting Spend PSBTs. The first one will have a change # output but not the second one.