Revert "Merge #742: commands: don't add derivation paths for keys from different path but same signer"

This reverts commit ec0c2426aa5fa6cee2efabd3ee6f175b41c35f64, reversing
changes made to 26d750d09c84734f56c2dc18cb332a232e24fb6d.

This reverts the fixes to the pruning of BIP32 derivation paths when
creating a PSBT, in preparation of reverting the merge of this feature
altogether. This is because always creating PSBT with only the BIP32
derivations for a single path broke the Ledger support.

src/commands/mod.rs

@@ -26,7 +26,7 @@ use std::{
 
 use miniscript::{
     bitcoin::{
-        self, address, bip32,
+        self, address,
         locktime::absolute,
         psbt::{Input as PsbtIn, Output as PsbtOut, PartiallySignedTransaction as Psbt},
     },
@@ -237,25 +237,6 @@ fn serializable_size<T: bitcoin::consensus::Encodable + ?Sized>(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<bip32::Fingerprint, HashSet<bip32::DerivationPath>>,
-) -> 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 {
@@ -389,7 +370,7 @@ impl DaemonControl {
             .primary_path()
             .thresh_origins();
         let is_prim_path_key =
-            |pubkey: &keys::DerivedPublicKey| is_key_for_spending_path(pubkey, &prim_path_origins);
+            |pubkey: &keys::DerivedPublicKey| prim_path_origins.contains_key(&pubkey.origin.0);
 
         // 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
@@ -833,7 +814,7 @@ impl DaemonControl {
             .ok_or(CommandError::UnknownRecoveryTimelock(timelock))?
             .thresh_origins();
         let is_key_for_this_path =
-            |pubkey: &keys::DerivedPublicKey| is_key_for_spending_path(pubkey, &reco_path_origins);
+            |pubkey: &keys::DerivedPublicKey| reco_path_origins.contains_key(&pubkey.origin.0);
 
         // 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.

tests/fixtures.py

@@ -156,41 +156,6 @@ 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):

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.change_desc if is_change else self.receive_desc)
+                str(self.receive_desc if der_path[0] == 0 else self.change_desc)
             )
-            desc.derive(der_path[len(der_path) - 1])
+            desc.derive(der_path[1])
             sat_material = SatisfactionMaterial(
                 signatures=psbt_in.map[PSBT_IN_PARTIAL_SIG],
             )

tests/test_rpc.py

@@ -398,24 +398,14 @@ 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 inputs.
+    # The two PSBTs don't share any BIP32 derivation paths in their input.
     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_reco_keys = set()
-    for i in res_reco_psbt.i:
-        res_reco_keys = res_reco_keys | set(i.map[PSBT_IN_BIP32_DERIVATION])
+    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])
     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.