descriptor: compare x-only public keys in Tapscript Miniscript duplicate check #36420

In Tapscript Miniscript descriptors (tr()), public keys serialize as 32-byte x-only pubkeys per BIP-340, BIP-342, and BIP-379, discarding parity. However, KeyParser::KeyCompare in src/script/descriptor.cpp compared keys as full 33-byte CPubKeys, retaining the parity byte.

When a descriptor contains public keys that differ only in parity prefix (such as pk(X) alongside pk(03X) where X defaults to 02, or pk(02X) alongside pk(03X)), KeyParser::KeyCompare treated them as distinct keys. Miniscript duplicate key checking passed, declaring the descriptor sane even though both branches serialize on-chain to <X> OP_CHECKSIG. Because a single signature satisfies both branches, third parties could malleate transactions by altering branch selectors without holding private keys, violating BIP-379 non-malleability invariants.

This patch updates KeyParser::KeyCompare in src/script/descriptor.cpp and KeyConverter::KeyCompare in src/test/miniscript_tests.cpp to compare keys as XOnlyPubKey whenever miniscript::IsTapscript(m_script_ctx) evaluates true. Under Tapscript context, keys sharing identical x-coordinates compare equivalent (!comp(a, b) && !comp(b, a)), ensuring DuplicateKeyCheck() detects the collision and rejects the descriptor. In src/test/descriptor_tests.cpp, unit tests verify that tr() descriptors containing parity-differing duplicate keys (including public hex and private WIF formats) are rejected as not sane, while wsh() descriptors continue to treat 33-byte compressed keys with differing parities as distinct.

Fixes #36414.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

External sites

• Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36420
• Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36420.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36420#author-ralph

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline and AI policy for information on the review process.

If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

Tested at cb85c2cdde. descriptor_tests fails on the multi_a check because that descriptor parses: a multi_a directly in tr() is not Miniscript and has no duplicate-key check, so even multi_a(2,X,X) is accepted. wallet_taproot.py repeats a key inside one multi_a, so refusing duplicates there would be a separate change. I suggest dropping the check here and in the description. The two or_i cases are refused with this change and accepted without it.

Good catch on multi_a parsing directly as MultiADescriptor rather than Miniscript. Dropped that test and updated the PR description. Thanks for testing!

ACK a1c65a776c252ce253972d1712936f315cbc5abc

descriptor_tests and miniscript_tests pass, and both new tr() checks fail without the descriptor.cpp change.

The optional WIF case in #36420 (review) still adds coverage: without the fix its private form is accepted, and this PR's cases all use public keys.

Re-pushed a1c65a7 -> 0956ace45d:

• Added the suggested WIF test case from #36420 (review) to cover the private descriptor path in KeyConverter.

<details><summary>Range-diff</summary>

1:  a1c65a776c ! 1:  0956ace45d descriptor: compare x-only public keys in Tapscript Miniscript duplicate check
    @@ src/test/descriptor_tests.cpp: BOOST_AUTO_TEST_CASE(descriptor_test)
     +    CheckUnparsable("tr(a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,or_i(pk(026116733562ba4df653e4ec6b98c904c8dd3c677428264d33f4ae0089b72abaa4),pk(036116733562ba4df653e4ec6b98c904c8dd3c677428264d33f4ae0089b72abaa4)))",
     +                    "tr(a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,or_i(pk(026116733562ba4df653e4ec6b98c904c8dd3c677428264d33f4ae0089b72abaa4),pk(036116733562ba4df653e4ec6b98c904c8dd3c677428264d33f4ae0089b72abaa4)))",
     +                    "or_i(pk(026116733562ba4df653e4ec6b98c904c8dd3c677428264d33f4ae0089b72abaa4),pk(036116733562ba4df653e4ec6b98c904c8dd3c677428264d33f4ae0089b72abaa4)) is not sane: contains duplicate public keys");
    ++    CheckUnparsable("tr(a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,or_i(pk(6116733562ba4df653e4ec6b98c904c8dd3c677428264d33f4ae0089b72abaa4),pk(KztWktoNG1biXs3GSD6M7YDEynGaQPgjLguiWhMXiJM7sDA5vZFt)))",
    ++                    "tr(a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,or_i(pk(6116733562ba4df653e4ec6b98c904c8dd3c677428264d33f4ae0089b72abaa4),pk(6116733562ba4df653e4ec6b98c904c8dd3c677428264d33f4ae0089b72abaa4)))",
    ++                    "or_i(pk(6116733562ba4df653e4ec6b98c904c8dd3c677428264d33f4ae0089b72abaa4),pk(6116733562ba4df653e4ec6b98c904c8dd3c677428264d33f4ae0089b72abaa4)) is not sane: contains duplicate public keys");
     +    // But in wsh(), keys differing only in parity are distinct 33-byte compressed pubkeys and not duplicate.
     +    {
     +        FlatSigningProvider keys;

</details>

reACK 0956ace45dac99bc11256f4db1ca59f53016af6b

Only the WIF test case was added since a1c65a77. descriptor_tests and miniscript_tests pass; without the descriptor.cpp change, the two tr() cases and the WIF case fail, the WIF one because its private form parses.