descriptor: fix musig duplicate checks and origin handling

shuv-amp commented at 11:17 pm on February 27, 2026: none

Miniscript duplicate key checking resolves keys while checking descriptor sanity. For musig() expressions with hardened participant derivation, doing that with an empty signing provider can make distinct keys both fail to resolve and get treated as duplicates.

Use the signing data collected during parsing for duplicate key comparison, and fall back to the key string when pubkey resolution is still unavailable.

Also fix OriginPubkeyProvider::GetPubKey() so repeated expansions of the same origin-wrapped musig participant key do not prefix the same origin path more than once. This was producing incorrect taproot BIP32 derivation paths in PSBTs.

Add descriptor tests for the duplicate check and origin/cache cases, and a wallet functional test for the PSBT derivation-path regression.

Tested:

./bin/test_bitcoin --run_test=descriptor_tests --catch_system_errors=no
./bin/test_bitcoin --run_test=miniscript_tests --catch_system_errors=no
./bin/test_bitcoin --run_test=bip328_tests --catch_system_errors=no
./bin/test_bitcoin --run_test=psbt_wallet_tests --catch_system_errors=no
python3 test/functional/test_runner.py wallet_musig.py
python3 test/functional/test_runner.py wallet_listdescriptors.py
python3 test/functional/test_runner.py rpc_psbt.py
python3 test/functional/test_runner.py wallet_taproot.py

DrahtBot added the label Consensus on Feb 27, 2026

DrahtBot commented at 11:17 pm on February 27, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	scgbckbone
Stale ACK	Bortlesboat

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

DrahtBot added the label CI failed on Feb 28, 2026

DrahtBot removed the label CI failed on Mar 3, 2026

Bortlesboat commented at 10:44 pm on March 7, 2026: none

Code Review ACK d2c5b2ffbd

The bug is clear and the fix is correct:

Before: PubkeyProvider::operator< derived pubkeys using an empty FlatSigningProvider. For hardened derivation steps, GetPubKey() needs private key material from the signing provider, so it returned nullopt. Two distinct musig() expressions with hardened-path participants both resolved to nullopt, causing nullopt < nullopt == false in both directions, making CheckDuplicateKey treat them as duplicates.

After: KeyCompare uses m_out (the signing provider populated during parsing, which has the private keys from xprv participants) instead of an empty provider. This allows hardened-path keys to resolve correctly, so distinct keys compare as distinct.

The removal of PubkeyProvider::operator< is a nice cleanup — the method was only used by KeyCompare and its empty-provider approach was fundamentally broken for hardened keys.

Regression test covers the exact scenario from #34273.

shuv-amp force-pushed on Mar 7, 2026

shuv-amp force-pushed on Mar 8, 2026

shuv-amp renamed this:
~~script: fix false-positive duplicate key detection in musig miniscript~~
descriptor: fix false duplicate-key detection for hardened musig()
on Mar 8, 2026

shuv-amp force-pushed on Mar 8, 2026

shuv-amp commented at 1:14 am on March 8, 2026: none

For attribution, I also referred to @w0xlt’s work here while reworking this:

https://github.com/w0xlt/bitcoin/commit/921cbdb08b271e8e866a1c0e6912e6241c964c9e

Not a direct copy, but it helped point me to the fix direction in the latest revision.

scgbckbone commented at 8:53 pm on March 19, 2026: contributor

this is broken & produces incorrect PSBTs where PSBT_IN_TAP_BIP32_DERIVATION and PSBT_OUT_TAP_BIP32_DERIVATION have wrong key derivation info. So instead for example m/86h/1h/0h has m/86h/1h/0h/86h/1h/0h (doubled).

Example policy with identical musig key expression with different key derivation info:

tr(unspend(),and_v(v:pk(musig(@0,@1,@2)/<0;1>/*),pk(musig(@0,@1,@2)/<2;3>/*)))

Descriptors (from listdescriptors):

0{'wallet_name': 'ident_musig_subder', 'descriptors': [{'desc': 'tr(tpubD6NzVbkrYhZ4WMCcYXEgDrM1AFATSGmrFyMnA2Dgx1Y551VueaE3iemHhQBxhKWjepG76Cy5QmXv8z4KwN6ARTnZ5hVuShfxL129NVGXZuE/0/*,and_v(v:pk(musig([0f056943/86h/1h/0h]tpubDCeEX49avtiXrBTv3JWTtco99Ka499jXdZHBRtm7va2gkMAui11ctZjqNAT9dLVNaEozt2C1kfTM88cnvZCXsWLJN2p4viGvsyGjtKVV7A1,[d4cb267a/44h/1h/0h]tpubDDjr2zotTQ9bCX5WFpj4YmUxUnU8fvk8RKGQGEivdbA3rvnbPJmBDC9H5BqQpN8L65ZM5RrGtyUrkbaux1Ys6R54MK2dUz1mgjF9eV9qkR6,[be242744/44h/1h/0h]tpubDDbRaxzhrtUw7XCP9n9kEHymwHhHjYuA51kBDTMNwRD8bM7HZ4vsaPgqZqSzjQyVpUaCRsgWr7egTVbjCTJHy39E3bLP2wwVHuW2K7R3DmY)/0/*),pk(musig([0f056943/86h/1h/0h]tpubDCeEX49avtiXrBTv3JWTtco99Ka499jXdZHBRtm7va2gkMAui11ctZjqNAT9dLVNaEozt2C1kfTM88cnvZCXsWLJN2p4viGvsyGjtKVV7A1,[d4cb267a/44h/1h/0h]tpubDDjr2zotTQ9bCX5WFpj4YmUxUnU8fvk8RKGQGEivdbA3rvnbPJmBDC9H5BqQpN8L65ZM5RrGtyUrkbaux1Ys6R54MK2dUz1mgjF9eV9qkR6,[be242744/44h/1h/0h]tpubDDbRaxzhrtUw7XCP9n9kEHymwHhHjYuA51kBDTMNwRD8bM7HZ4vsaPgqZqSzjQyVpUaCRsgWr7egTVbjCTJHy39E3bLP2wwVHuW2K7R3DmY)/2/*)))#cagg4sg0', 'timestamp': 1773952443, 'active': True, 'internal': False, 'range': [0, 100], 'next': 3, 'next_index': 3}, {'desc': 'tr(tpubD6NzVbkrYhZ4WMCcYXEgDrM1AFATSGmrFyMnA2Dgx1Y551VueaE3iemHhQBxhKWjepG76Cy5QmXv8z4KwN6ARTnZ5hVuShfxL129NVGXZuE/1/*,and_v(v:pk(musig([0f056943/86h/1h/0h]tpubDCeEX49avtiXrBTv3JWTtco99Ka499jXdZHBRtm7va2gkMAui11ctZjqNAT9dLVNaEozt2C1kfTM88cnvZCXsWLJN2p4viGvsyGjtKVV7A1,[d4cb267a/44h/1h/0h]tpubDDjr2zotTQ9bCX5WFpj4YmUxUnU8fvk8RKGQGEivdbA3rvnbPJmBDC9H5BqQpN8L65ZM5RrGtyUrkbaux1Ys6R54MK2dUz1mgjF9eV9qkR6,[be242744/44h/1h/0h]tpubDDbRaxzhrtUw7XCP9n9kEHymwHhHjYuA51kBDTMNwRD8bM7HZ4vsaPgqZqSzjQyVpUaCRsgWr7egTVbjCTJHy39E3bLP2wwVHuW2K7R3DmY)/1/*),pk(musig([0f056943/86h/1h/0h]tpubDCeEX49avtiXrBTv3JWTtco99Ka499jXdZHBRtm7va2gkMAui11ctZjqNAT9dLVNaEozt2C1kfTM88cnvZCXsWLJN2p4viGvsyGjtKVV7A1,[d4cb267a/44h/1h/0h]tpubDDjr2zotTQ9bCX5WFpj4YmUxUnU8fvk8RKGQGEivdbA3rvnbPJmBDC9H5BqQpN8L65ZM5RrGtyUrkbaux1Ys6R54MK2dUz1mgjF9eV9qkR6,[be242744/44h/1h/0h]tpubDDbRaxzhrtUw7XCP9n9kEHymwHhHjYuA51kBDTMNwRD8bM7HZ4vsaPgqZqSzjQyVpUaCRsgWr7egTVbjCTJHy39E3bLP2wwVHuW2K7R3DmY)/3/*)))#wshkxfmd', 'timestamp': 1773952443, 'active': True, 'internal': True, 'range': [0, 100], 'next': 2, 'next_index': 2}]}

Invalid PSBT produced with walletcreatefundedpsbt:

cHNidP8BAIkCAAAAAWGlTAOe5CR+5kIuveSdj4GrIfLLjYDpab2pLTWAUQfiAQAAAAD9////AkoHCJIAAAAAIlEglAEsB4z+Jx2RIT47AF9gnkQMSofYSF/OCnShqqOhPL+ACAiSAAAAACJRIA5PL3SfKi+OfX3IdVQ3DwXhI1WzIYk6V1foWAC2oCzhAAAAAAABASsAERAkAQAAACJRIP/Wc/ylbCwkQAqnhNV+hGRf4ADn7YQVD3t35s6GWSOSIhXBFPtzJ01QvvG2wWKC70L1wQoVa/tEj0ZV8z7WCZ8fWn9FIBlVHXyKX/dzYVa328zOGmjQzL4wOIxMz1HvQPnHNzjRrSCmWXdGmau8dTa2O8EOxiSwiZgmcHFvQ5ol79ewUgJyZqzAIRYU+3MnTVC+8bbBYoLvQvXBChVr+0SPRlXzPtYJnx9afw0AfEYeXQAAAAAAAAAAIRYZVR18il/3c2FWt9vMzhpo0My+MDiMTM9R70D5xzc40S0BzV7ymTROB3u+1SMM80p5oOk4YHcqjFujfePdZCw3bg78+MvlAAAAAAAAAAAhFlfuNxx6b6kxHwwkYH27XQ+N/oRWMHWeWg4yG75U/IqpPQHNXvKZNE4He77VIwzzSnmg6ThgdyqMW6N9491kLDduDr4kJ0QsAACAAQAAgAAAAIAsAACAAQAAgAAAAIAhFqZZd0aZq7x1NrY7wQ7GJLCJmCZwcW9DmiXv17BSAnJmLQHNXvKZNE4He77VIwzzSnmg6ThgdyqMW6N9491kLDduDvz4y+UCAAAAAAAAACEWrylR8DuGe6dyJqmNY8cey+q7FSn6si/uFfYnoHEGGnU9Ac1e8pk0Tgd7vtUjDPNKeaDpOGB3Koxbo33j3WQsN24ODwVpQ1YAAIABAACAAAAAgFYAAIABAACAAAAAgCEWtYiF4esSAfbBlQgaFJBVpMajf8WWFsWRJNb/UIBxgSc9Ac1e8pk0Tgd7vtUjDPNKeaDpOGB3Koxbo33j3WQsN24O1MsmeiwAAIABAACAAAAAgCwAAIABAACAAAAAgAEXIBT7cydNUL7xtsFigu9C9cEKFWv7RI9GVfM+1gmfH1p/ARggzV7ymTROB3u+1SMM80p5oOk4YHcqjFujfePdZCw3bg4iGgMVFs+43JrSwExLgMMLz345ka/0HJsWRVDbnFc1xoG1imMCrylR8DuGe6dyJqmNY8cey+q7FSn6si/uFfYnoHEGGnUCtYiF4esSAfbBlQgaFJBVpMajf8WWFsWRJNb/UIBxgScDV+43HHpvqTEfDCRgfbtdD43+hFYwdZ5aDjIbvlT8iqkAAQUgddcbN0Ssw6D0YVHzmS+O5GkFJH7f9Bbq62BGzadKAPkBBkcAwEQg5eUHYUwmV8VurjFwTSABM5kI9fWZWmSQ5xP0yRNAvIOtIPkQ5Vja06GfNZl7UFPO99YVBPupS/rJFyE1YvFa28furCEHV+43HHpvqTEfDCRgfbtdD43+hFYwdZ5aDjIbvlT8iqk9AYGHvCToOGbNfuwT9YRCMDypanJNofChYliNc4zDrv75viQnRCwAAIABAACAAAAAgCwAAIABAACAAAAAgCEHddcbN0Ssw6D0YVHzmS+O5GkFJH7f9Bbq62BGzadKAPkNAHxGHl0AAAAAAQAAACEHrylR8DuGe6dyJqmNY8cey+q7FSn6si/uFfYnoHEGGnU9AYGHvCToOGbNfuwT9YRCMDypanJNofChYliNc4zDrv75DwVpQ1YAAIABAACAAAAAgFYAAIABAACAAAAAgCEHtYiF4esSAfbBlQgaFJBVpMajf8WWFsWRJNb/UIBxgSc9AYGHvCToOGbNfuwT9YRCMDypanJNofChYliNc4zDrv751MsmeiwAAIABAACAAAAAgCwAAIABAACAAAAAgCEH5eUHYUwmV8VurjFwTSABM5kI9fWZWmSQ5xP0yRNAvIMtAYGHvCToOGbNfuwT9YRCMDypanJNofChYliNc4zDrv75/PjL5QAAAAABAAAAIQf5EOVY2tOhnzWZe1BTzvfWFQT7qUv6yRchNWLxWtvH7i0BgYe8JOg4Zs1+7BP1hEIwPKlqck2h8KFiWI1zjMOu/vn8+MvlAgAAAAEAAAAiCAMVFs+43JrSwExLgMMLz345ka/0HJsWRVDbnFc1xoG1imMCrylR8DuGe6dyJqmNY8cey+q7FSn6si/uFfYnoHEGGnUCtYiF4esSAfbBlQgaFJBVpMajf8WWFsWRJNb/UIBxgScDV+43HHpvqTEfDCRgfbtdD43+hFYwdZ5aDjIbvlT8iqkAAQUgtHS7swPWdk/zmt7YcdWW4D+b3Ug1ewMrGS+2znlJyhgBBkcAwEQgSvLepLCSlDPDiTP61uRCZWHoCteg6uhJN51vcwHFdy6tIIIYPJwweXnPUEFXm/xb7vGCUjQr+3E6Vh/S0jdsX2NirCEHSvLepLCSlDPDiTP61uRCZWHoCteg6uhJN51vcwHFdy4tAU01q8wKs/JAgBbBgMSz35C2d5S4Kqo9dLSQdHUflkdW/PjL5QAAAAACAAAAIQdX7jccem+pMR8MJGB9u10Pjf6EVjB1nloOMhu+VPyKqT0BTTWrzAqz8kCAFsGAxLPfkLZ3lLgqqj10tJB0dR+WR1a+JCdELAAAgAEAAIAAAACALAAAgAEAAIAAAACAIQeCGDycMHl5z1BBV5v8W+7xglI0K/txOlYf0tI3bF9jYi0BTTWrzAqz8kCAFsGAxLPfkLZ3lLgqqj10tJB0dR+WR1b8+MvlAgAAAAIAAAAhB68pUfA7hnunciapjWPHHsvquxUp+rIv7hX2J6BxBhp1PQFNNavMCrPyQIAWwYDEs9+QtneUuCqqPXS0kHR1H5ZHVg8FaUNWAACAAQAAgAAAAIBWAACAAQAAgAAAAIAhB7R0u7MD1nZP85re2HHVluA/m91INXsDKxkvts55ScoYDQB8Rh5dAAAAAAIAAAAhB7WIheHrEgH2wZUIGhSQVaTGo3/FlhbFkSTW/1CAcYEnPQFNNavMCrPyQIAWwYDEs9+QtneUuCqqPXS0kHR1H5ZHVtTLJnosAACAAQAAgAAAAIAsAACAAQAAgAAAAIAiCAMVFs+43JrSwExLgMMLz345ka/0HJsWRVDbnFc1xoG1imMCrylR8DuGe6dyJqmNY8cey+q7FSn6si/uFfYnoHEGGnUCtYiF4esSAfbBlQgaFJBVpMajf8WWFsWRJNb/UIBxgScDV+43HHpvqTEfDCRgfbtdD43+hFYwdZ5aDjIbvlT8iqkA

shuv-amp commented at 9:19 pm on March 19, 2026: none

Thanks, this looks like a real issue. Marking draft while I rework this.

shuv-amp marked this as a draft on Mar 19, 2026

descriptor: fix musig duplicate checks and origin handling

Miniscript duplicate key checking resolves keys while checking descriptor
sanity. For musig() expressions with hardened participant derivation,
resolving with an empty signing provider can make distinct keys both fail
to resolve and get treated as duplicates.

Use the signing data collected during parsing for duplicate key
comparison, and fall back to the key string when pubkey resolution is
still unavailable.

Also fix OriginPubkeyProvider::GetPubKey() so repeated expansions of the
same origin-wrapped musig participant key do not prefix the same origin
path more than once. This was producing incorrect taproot BIP32
derivation paths in PSBTs.

Add descriptor and wallet tests for both cases.

5d44a9a253

shuv-amp force-pushed on Mar 20, 2026

shuv-amp renamed this:
~~descriptor: fix false duplicate-key detection for hardened musig()~~
descriptor: fix musig duplicate checks and origin handling
on Mar 20, 2026

shuv-amp marked this as ready for review on Mar 20, 2026

scgbckbone commented at 8:45 am on March 20, 2026: contributor

tACK

descriptor: fix musig duplicate checks and origin handling #34697

Reviews