Merge bitcoin/bitcoin#34697: descriptor: fix musig() duplicate key checks and doubled PSBT origin paths

b42f7fade0 descriptor: don't prepend key origins twice (Shuvam Pandey)
7b15e2cb44 descriptor: fix duplicate check for hardened keys (Shuvam Pandey)

Pull request description:

  Fixes #34273.

  Importing a descriptor that uses the same `musig()` participants twice in one
  tapleaf, with different musig subderivations, fails with
  `is not sane: contains duplicate public keys`. It only fails when one of the
  participants is a private key on a hardened path. The all-xpub version of the
  same descriptor imports fine. That's what gave it away.

  The duplicate check (`KeyCompare`) resolves each key expression to a pubkey and
  compares the results. It does this at index 0, and the old code used an empty
  signing provider. With that empty provider, a `musig()` expression can't resolve
  when one of its participants is on a hardened path, because deriving that
  participant needs its private key, so the whole aggregate key comes back empty.
  Two different musig expressions both came back empty, so the check treated them
  as duplicates. The fix derives against the signing provider populated during
  parsing, which holds the private keys, and only compares the expression strings
  when neither side resolves. 151henry151 had suggested looking at the empty
  signing provider on the issue.

  scgbckbone found a second, separate bug in the same descriptors. When another
  expression that reuses those participants is handled in the same expansion, its
  participant origin in the PSBT is added twice, so `m/86h/1h/0h` becomes
  `m/86h/1h/0h/86h/1h/0h` in both the input and output Taproot BIP32 derivation
  maps. `OriginPubkeyProvider::GetPubKey()` now derives into a temporary provider,
  merges it, and writes the corrected origin once, so a later expression can't
  prepend the same origin again.

  Tested:
  ```
  ./build/bin/test_bitcoin --run_test=descriptor_tests
  ./build/bin/test_bitcoin --run_test=miniscript_tests
  ./build/bin/test_bitcoin --run_test=bip328_tests
  ./build/bin/test_bitcoin --run_test=psbt_wallet_tests
  ./build/test/functional/test_runner.py wallet_musig.py --jobs=1
  ```

ACKs for top commit:
  achow101:
    ACK b42f7fade0
  scgbckbone:
    ACK b42f7fade0

Tree-SHA512: ab36caa6bc484fa1fc3289c79e9a3d713278d82f80de478e53e1bdbe645037e07776ac798eba085733abc139c11a9dbf0d3f49d3c0eee9632e4ddf33d2242f92

test/functional/wallet_musig.py

@@ -202,6 +202,9 @@ class WalletMuSigTest(BitcoinTestFramework):
         wallets, keys = self.create_wallets_and_keys_from_pattern(pat)
         self.construct_and_import_musig_descriptor_in_wallets(pat, wallets, keys, only_one_musig_wallet)
 
+        # The participant maps are keyed by the aggregate pubkey, which does not depend on the
+        # order of the participants nor on the derivation applied to the aggregate.
+        expected_participant_maps = len({tuple(sorted(musig.split(","))) for musig in MUSIG_RE.findall(pat)})
         expected_pubnonces = 0
         expected_partial_sigs = 0
         for musig in MUSIG_RE.findall(pat):
@@ -252,14 +255,15 @@ class WalletMuSigTest(BitcoinTestFramework):
 
         dec_psbt = self.nodes[0].decodepsbt(psbt)
         assert_equal(len(dec_psbt["inputs"]), 1)
-        assert_equal(len(dec_psbt["inputs"][0]["musig2_participant_pubkeys"]), pattern.count("musig("))
+        assert_equal(len(dec_psbt["inputs"][0]["musig2_participant_pubkeys"]), expected_participant_maps)
         if has_internal:
-            assert_equal(len(dec_psbt["outputs"][1]["musig2_participant_pubkeys"]), pattern.count("musig("))
+            assert_equal(len(dec_psbt["outputs"][1]["musig2_participant_pubkeys"]), expected_participant_maps)
 
         # Check all participant pubkeys in the input and change output
         psbt_maps = [dec_psbt["inputs"][0]]
         if has_internal:
             psbt_maps.append(dec_psbt["outputs"][1])
+        origin_paths = {ORIGIN_PATH_RE.search(pub).group(1) for _, pub in keys}
         for psbt_map in psbt_maps:
             part_pks = set()
             for agg in psbt_map["musig2_participant_pubkeys"]:
@@ -267,9 +271,13 @@ class WalletMuSigTest(BitcoinTestFramework):
                     part_pks.add(part_pub[2:])
             # Check that there are as many participants as we expected
             assert_equal(len(part_pks), len(keys))
-            # Check that each participant has a derivation path
+            # Check that each participant has a derivation path, and that its origin appears in
+            # that path just once no matter how many musig() expressions the participant is in
             for deriv_path in psbt_map["taproot_bip32_derivs"]:
                 if deriv_path["pubkey"] in part_pks:
+                    origin = next((o for o in origin_paths if deriv_path["path"].startswith(f"m{o}")), None)
+                    assert origin is not None, deriv_path["path"]
+                    assert_equal(deriv_path["path"].count(origin), 1)
                     part_pks.remove(deriv_path["pubkey"])
             assert_equal(len(part_pks), 0)
 
@@ -346,6 +354,7 @@ class WalletMuSigTest(BitcoinTestFramework):
         self.test_success_case("tr(H,pk(musig/*))", "tr($H,pk(musig($0,$1,$2)/<0;1>/*))", scriptpath=True)
         self.test_success_case("tr(H,{pk(musig/*), pk(musig/*)})", "tr($H,{pk(musig($0,$1,$2)/<0;1>/*),pk(musig($3,$4,$5)/0/*)})", scriptpath=True)
         self.test_success_case("tr(H,{pk(musig/*), pk(same keys different musig/*)})", "tr($H,{pk(musig($0,$1,$2)/<0;1>/*),pk(musig($1,$2)/0/*)})", scriptpath=True)
+        self.test_success_case("tr(H,and(pk(musig/*),pk(same musig, other derivation/*)))", "tr($H,and_v(v:pk(musig($0,$1,$2)/<0;1>/*),pk(musig($0,$1,$2)/<2;3>/*)))", scriptpath=True)
         self.test_success_case("tr(musig/*,{pk(partial keys diff musig-1/*),pk(partial keys diff musig-2/*)})}", "tr(musig($0,$1,$2)/<3;4>/*,{pk(musig($0,$1)/<5;6>/*),pk(musig($1,$2)/7/*)})")
         self.test_success_case("tr(musig/*,{pk(partial keys diff musig-1/*),pk(partial keys diff musig-2/*)})} script-path", "tr(musig($0,$1,$2)/<3;4>/*,{pk(musig($0,$1)/<5;6>/*),pk(musig($1,$2)/7/*)})", scriptpath=True, nosign_wallets=[0])
         self.test_success_case("tr(H,and(pk(musig/*),after(1)))", "tr($H,and_v(v:pk(musig($0,$1,$2)/<0;1>/*),after(1)))", scriptpath=True)