wallet, descriptor: Fix MuSig private key completeness checks on `importdescriptors` #35493

w0xlt commented at 6:49 AM on June 9, 2026: contributor

importdescriptors currently checks whether all private keys are present by expanding the descriptor and verifying that every expanded origin pubkey has a private key.

This is wrong for MuSig descriptors because expansion includes the synthetic aggregate pubkey. There is no individual private key for that aggregate pubkey, so importing a fully private MuSig descriptor such as rawtr(musig(A_priv,B_priv)) incorrectly returns:

Not all private keys provided. Some wallet functionality may return unexpected errors

This PR fixes the issue by making descriptor private-key completeness account for MuSig participant keys, and by having importdescriptors use Descriptor::HavePrivateKeys() instead of duplicating its own manual completeness check.

The functional test covers both cases:

rawtr(musig(A_priv,B_priv)) imports without warnings.
rawtr(musig(A_priv,B_pub)) still warns that not all private keys were provided.

DrahtBot commented at 6:49 AM on June 9, 2026: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35493.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	rkrux, theStack
Approach ACK	pablomartin4btc, b-l-u-e

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35543 (test: introduce ExtendedPrivateKey and ExtendedPublicKey classes by rkrux)
#35377 (wallet: Allow importing of descriptors without private keys when the wallet has the private keys by achow101)
#34861 (wallet: Add importdescriptors interface by polespinasa)
#33392 (wallet, rpc: add UTXO set check and incremental rescan to importdescriptors by musaHaruna)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

sedited requested review from theStack on Jun 9, 2026

pablomartin4btc commented at 5:42 PM on June 9, 2026: member

Approach ACK.

I verified the regression by running the functional test without the fix, where the fully-private rawtr(musig(...)) import incorrectly warns that not all private keys were provided. With this PR, the fully-private MuSig descriptor imports without warnings, while the partial-private case still warns as expected.

The approach makes sense to me: as the previous origin/pubkey counting heuristic does not correctly model MuSig descriptors where the aggregate key is derived from the participant keys. The added functional test covers both the regression and the expected warning case.

b-l-u-e commented at 7:18 AM on June 10, 2026: contributor

Approach ACK. The fix looks okay I manually tested importdescriptors for rawtr(musig(...)/0/*): full participant privkeys import without warning; partial import still warns. maybe add an importdescriptors case to cover aggregate-derivation imports.

--- a/test/functional/wallet_musig.py
+++ b/test/functional/wallet_musig.py
@@ -217,6 +217,26 @@ class WalletMuSigTest(BitcoinTestFramework):
         assert_equal(res[1]["success"], True)
         assert_equal(res[1]["warnings"], [missing_keys_warning])
 
+        # Check musig participant completeness for aggregate-key derivation descriptors.
+        # Full private participant set should not warn, partial should.
+        res = wallet.importdescriptors([
+            {
+                "desc": descsum_create(f"rawtr(musig({keys[0][0]},{keys[1][0]})/0/*)"),
+                "timestamp": "now",
+                "range": [0, 1],
+            },
+            {
+                "desc": descsum_create(f"rawtr(musig({keys[0][0]},{keys[1][1]})/1/*)"),
+                "timestamp": "now",
+                "range": [0, 1],
+            },
+        ])
+
+        assert_equal(res[0]["success"], True)
+        assert "warnings" not in res[0]
+        assert_equal(res[1]["success"], True)
+        assert_equal(res[1]["warnings"], [missing_keys_warning])
+

rkrux commented at 10:55 AM on June 12, 2026: contributor

Good catch, Concept ACK 9d75efa.

theStack commented at 2:16 PM on June 12, 2026: contributor

Concept ACK

in test/functional/wallet_musig.py:197 in 9d75efa10f

 193 | @@ -194,6 +194,29 @@ def test_failure_case_3(self, comment, pat):
 194 |          assert "musig2_pubnonces" in dec["inputs"][0]
 195 |          assert "musig2_partial_sigs" not in dec["inputs"][0]
 196 |  
 197 | +    def test_importdescriptors_private_key_warnings(self):

rkrux commented at 1:35 PM on June 15, 2026:

This test is more suitable for wallet_importdescriptors.py file. Even the missing_keys_warning is hardcoded there few times that could be extracted out. https://github.com/bitcoin/bitcoin/blob/6921f5df011abb8df9b2e7234613ca8a369492ab/test/functional/wallet_importdescriptors.py#L682-L707

This file wallet_musig.py is more suitable for testing the musig signing flow, the file name unfortunately falls short of its intent.

w0xlt commented at 4:25 AM on June 18, 2026:

Done. Thanks.

in test/functional/wallet_musig.py:199 in 9d75efa10f

 193 | @@ -194,6 +194,29 @@ def test_failure_case_3(self, comment, pat):
 194 |          assert "musig2_pubnonces" in dec["inputs"][0]
 195 |          assert "musig2_partial_sigs" not in dec["inputs"][0]
 196 |  
 197 | +    def test_importdescriptors_private_key_warnings(self):
 198 | +        self.log.info("Testing importdescriptors MuSig private key warnings")
 199 | +        _, keys = self.create_wallets_and_keys_from_pattern("rawtr(musig($0,$1))")

rkrux commented at 1:38 PM on June 15, 2026:

The creation of multiple wallets to extract the keys is not needed for this test.

Related to moving this test, there are already few hardcoded xprvs and xpubs in wallet_importdescriptors.py that are sufficient for this test, keeping the test light.

https://github.com/bitcoin/bitcoin/blob/6921f5df011abb8df9b2e7234613ca8a369492ab/test/functional/wallet_importdescriptors.py#L580-L589

w0xlt commented at 4:25 AM on June 18, 2026:

Done. Thanks.

rkrux changes_requested

rkrux commented at 1:39 PM on June 15, 2026: contributor

Code review 9d75efa10f09428697f76c6693149214319d6a24

w0xlt force-pushed on Jun 18, 2026

descriptors: require complete MuSig private keys b53e5b2c23

wallet: check descriptor private key completeness on import ee7d8c0b09

test: check MuSig import private key warnings 86dbbead1c

w0xlt force-pushed on Jun 18, 2026

DrahtBot added the label CI failed on Jun 18, 2026

w0xlt commented at 4:25 AM on June 18, 2026: contributor

@b-l-u-e Thanks. Done.

w0xlt commented at 4:26 AM on June 18, 2026: contributor

Rebased. CI error is unrelated.

DrahtBot removed the label CI failed on Jun 19, 2026