wallet, descriptor: Fix MuSig private key completeness checks on `importdescriptors` by w0xlt · Pull Request #35493 · bitcoin/bitcoin

w0xlt · 2026-06-09T06:49:04Z

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 · 2026-06-09T06:49:10Z

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 and AI policy for information on the review process.

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

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35445 (wallet, descriptor: Revert StringType::COMPAT for Miniscript expressions and drop the concept of a Descriptor ID that can be validated by achow101)

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.

pablomartin4btc

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 · 2026-06-10T07:18:29Z

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 · 2026-06-12T10:55:35Z

Good catch, Concept ACK 9d75efa.

theStack · 2026-06-12T14:16:18Z

Concept ACK

rkrux

Code review 9d75efa

rkrux · 2026-06-15T13:35:50Z

        assert "musig2_pubnonces" in dec["inputs"][0]
        assert "musig2_partial_sigs" not in dec["inputs"][0]

+    def test_importdescriptors_private_key_warnings(self):


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.

bitcoin/test/functional/wallet_importdescriptors.py

Lines 682 to 707 in 6921f5d

assert_equal(res[0]['warnings'][0], 'Not all private keys provided. Some wallet functionality may return unexpected errors')

assert_equal(res[1]['success'], True)

assert_equal(res[1]['warnings'][0], 'Not all private keys provided. Some wallet functionality may return unexpected errors')

self.nodes[1].createwallet(wallet_name='wmulti_priv2', blank=True)

wmulti_priv2 = self.nodes[1].get_wallet_rpc('wmulti_priv2')

res = wmulti_priv2.importdescriptors([

{

"desc": descsum_create("wsh(multi(2,[7b2d0242/84h/0h/0h]" + acc_xpub1 + "/*," + xprv2 + "/84h/0h/0h/*,[e81a0532/84h/0h/0h]" + acc_xpub3 + "/*))"),

"active": True,

"range": 1000,

"next_index": 0,

"timestamp": "now"

},

{

"desc": descsum_create("wsh(multi(2,[7b2d0242/84h/1h/0h]" + chg_xpub1 + "/*," + xprv2 + "/84h/1h/0h/*,[e81a0532/84h/1h/0h]" + chg_xpub3 + "/*))"),

"active": True,

"internal" : True,

"range": 1000,

"next_index": 0,

"timestamp": "now"

}])

assert_equal(res[0]['success'], True)

assert_equal(res[0]['warnings'][0], 'Not all private keys provided. Some wallet functionality may return unexpected errors')

assert_equal(res[1]['success'], True)

assert_equal(res[1]['warnings'][0], 'Not all private keys provided. Some wallet functionality may return unexpected errors')

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

Done. Thanks.

rkrux · 2026-06-15T13:38:36Z


+    def test_importdescriptors_private_key_warnings(self):
+        self.log.info("Testing importdescriptors MuSig private key warnings")
+        _, keys = self.create_wallets_and_keys_from_pattern("rawtr(musig($0,$1))")


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.

bitcoin/test/functional/wallet_importdescriptors.py

Lines 580 to 589 in 6921f5d

xprv1 = 'tprv8ZgxMBicQKsPevADjDCWsa6DfhkVXicu8NQUzfibwX2MexVwW4tCec5mXdCW8kJwkzBRRmAay1KZya4WsehVvjTGVW6JLqiqd8DdZ4xSg52'

acc_xpub1 = 'tpubDCJtdt5dgJpdhW4MtaVYDhG4T4tF6jcLR1PxL43q9pq1mxvXgMS9Mzw1HnXG15vxUGQJMMSqCQHMTy3F1eW5VkgVroWzchsPD5BUojrcWs8' # /84'/0'/0'

chg_xpub1 = 'tpubDCXqdwWZcszwqYJSnZp8eARkxGJfHAk23KDxbztV4BbschfaTfYLTcSkSJ3TN64dRqwa1rnFUScsYormKkGqNbbPwkorQimVevXjxzUV9Gf' # /84'/1'/0'

xprv2 = 'tprv8ZgxMBicQKsPdSNWUhDiwTScDr6JfkZuLshTRwzvZGnMSnGikV6jxpmdDkC3YRc4T3GD6Nvg9uv6hQg73RVv1EiTXDZwxVbsLugVHU8B1aq'

acc_xprv2 = 'tprv8gVCsmRAxVSxyUpsL13Y7ZEWBFPWbgS5E2MmFVNGuANrknvmmn2vWnmHvU8AwEFYzR2ji6EeZLSCLVacsYkvor3Pcb5JY5FGcevqTwYvdYx'

acc_xpub2 = 'tpubDDBF2BTR6s8drwrfDei8WxtckGuSm1cyoKxYY1QaKSBFbHBYQArWhHPA6eJrzZej6nfHGLSURYSLHr7GuYch8aY5n61tGqgn8b4cXrMuoPH'

chg_xpub2 = 'tpubDCYfZY2ceyHzYzMMVPt9MNeiqtQ2T7Uyp9QSFwYXh8Vi9iJFYXcuphJaGXfF3jUQJi5Y3GMNXvM11gaL4txzZgNGK22BFAwMXynnzv4z2Jh'

xprv3 = 'tprv8ZgxMBicQKsPeonDt8Ka2mrQmHa61hQ5FQCsvWBTpSNzBFgM58cV2EuXNAHF14VawVpznnme3SuTbA62sGriwWyKifJmXntfNeK7zeqMCj1'

acc_xpub3 = 'tpubDCsWoW1kuQB9kG5MXewHqkbjPtqPueRnXju7uM2NK7y3JYb2ajAZ9EiuZXNNuE4661RAfriBWhL8UsnAPpk8zrKKnZw1Ug7X4oHgMdZiU4E'

chg_xpub3 = 'tpubDC6UGqnsQStngYuGD4MKsMy7eD1Yg9NTJfPdvjdG2JE5oZ7EsSL3WHg4Gsw2pR5K39ZwJ46M1wZayhedVdQtMGaUhq5S23PH6fnENK3V1sb'

Done. Thanks.

w0xlt · 2026-06-18T04:25:54Z

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

w0xlt · 2026-06-18T04:26:06Z

Rebased.
CI error is unrelated.

rkrux · 2026-06-27T07:57:36Z

+                "desc": descsum_create(f"rawtr(musig({xprv1}/84h/0h/0h/0,{acc_xprv2}/1))"),
+                "timestamp": "now",
+            },
+            {
+                "desc": descsum_create(f"rawtr(musig({xprv1}/84h/0h/0h/2,{acc_xpub2}/3))"),
+                "timestamp": "now",
+            },
+            {
+                "desc": descsum_create(f"rawtr(musig({xprv1}/84h/0h/0h,{acc_xprv2})/0/*)"),
+                "timestamp": "now",
+                "range": [0, 1],
+            },
+            {
+                "desc": descsum_create(f"rawtr(musig({xprv1}/84h/0h/0h,{acc_xpub2})/1/*)"),


84h/0h/0h

It would need a rebase post merging of PR #35543.
While doing that, let's also avoid hardcoding this derivation path and instead use the variable derivation_path here.

Done. Thanks.

pablomartin4btc

ACK 0fd7b53

Since my last review the test was moved to wallet_importdescriptors.py and simplified, the hardcoded warning string extracted to a constant, the derivation path de-hardcoded to use the existing derivation_path variable, and rebased after #35543. Also addresses @b-l-u-e's suggestion — the test now covers both participant-level and aggregate-level derivation cases.

achow101 · 2026-07-09T21:34:12Z

    }
 }

+BOOST_AUTO_TEST_CASE(descriptor_musig_have_private_keys)


In b768584 "descriptors: require complete MuSig private keys"

Instead of adding a new specific unit test just for this case, we should be updating DoCheck so that it performs the proper checks for all of the descriptors being passed in. Any new test descriptors should be added to descriptor_test with appropriate flags set so that the correct things are being checked.

Done. Thanks.

achow101 · 2026-07-09T21:39:34Z

    virtual void GetPrivKey(int pos, const SigningProvider& arg, FlatSigningProvider& out) const = 0;

+    /** Whether private data for this provider is available in arg. */
+    virtual bool HavePrivateKeys(int pos, const SigningProvider& arg) const


In b768584 "descriptors: require complete MuSig private keys"

Having pos here is pointless, it has no bearing on whether we do or don't have private keys for this provider. pos is only necessary in GetPrivKey to get a specific private key, but this function only cares about whether we can get private keys at all, especially because all callers pass 0 anyways.

Done. Thanks.

Co-authored-by: b-l-u-e <winnie.gitau282@gmail.com>

w0xlt · 2026-07-10T07:50:55Z

@b-l-u-e I added you as a co-author of the test commit. I had forgotten to do so earlier.

b-l-u-e · 2026-07-10T08:35:45Z

@b-l-u-e I added you as a co-author of the test commit. I had forgotten to do so earlier.

It’s all good in the hood 😊

sedited requested a review from theStack June 9, 2026 08:09

pablomartin4btc reviewed Jun 9, 2026

View reviewed changes

This was referenced Jun 10, 2026

wallet: Allow importing of descriptors without private keys when the wallet has the private keys #35377

Draft

wallet: Add importdescriptors interface #34861

Open

wallet: allow skipping script paths #32857

Open

rkrux suggested changes Jun 15, 2026

View reviewed changes

w0xlt force-pushed the descriptors-musig-import-privkey-completeness branch 2 times, most recently from 3f11191 to 86dbbea Compare June 18, 2026 00:34

DrahtBot added the CI failed label Jun 18, 2026

DrahtBot mentioned this pull request Jun 18, 2026

test: introduce ExtendedPrivateKey and ExtendedPublicKey classes #35543

Merged

DrahtBot removed the CI failed label Jun 19, 2026

This was referenced Jun 20, 2026

wallet, rpc: add UTXO set check and incremental rescan to importdescriptors #33392

Open

Remove boost as a unit test runner #35587

Closed

DrahtBot added the Needs rebase label Jun 26, 2026

rkrux reviewed Jun 27, 2026

View reviewed changes

w0xlt force-pushed the descriptors-musig-import-privkey-completeness branch from 86dbbea to 0fd7b53 Compare July 7, 2026 23:10

DrahtBot removed the Needs rebase label Jul 8, 2026

pablomartin4btc reviewed Jul 9, 2026

View reviewed changes

DrahtBot requested a review from rkrux July 9, 2026 06:10

willcl-ark added Wallet Descriptors labels Jul 9, 2026

achow101 reviewed Jul 9, 2026

View reviewed changes

w0xlt added 2 commits July 10, 2026 00:44

descriptors: require complete MuSig private keys

cd8d01b

wallet: check descriptor private key completeness on import

5e62fbf

test: check MuSig import private key warnings

0390338

Co-authored-by: b-l-u-e <winnie.gitau282@gmail.com>

w0xlt force-pushed the descriptors-musig-import-privkey-completeness branch from 0fd7b53 to 0390338 Compare July 10, 2026 07:48

DrahtBot mentioned this pull request Jul 11, 2026

wallet, descriptor: Revert StringType::COMPAT for Miniscript expressions and drop the concept of a Descriptor ID that can be validated #35445

Open

	assert_equal(res[0]['warnings'][0], 'Not all private keys provided. Some wallet functionality may return unexpected errors')
	assert_equal(res[1]['success'], True)
	assert_equal(res[1]['warnings'][0], 'Not all private keys provided. Some wallet functionality may return unexpected errors')

	self.nodes[1].createwallet(wallet_name='wmulti_priv2', blank=True)
	wmulti_priv2 = self.nodes[1].get_wallet_rpc('wmulti_priv2')
	res = wmulti_priv2.importdescriptors([
	{
	"desc": descsum_create("wsh(multi(2,[7b2d0242/84h/0h/0h]" + acc_xpub1 + "/," + xprv2 + "/84h/0h/0h/,[e81a0532/84h/0h/0h]" + acc_xpub3 + "/*))"),
	"active": True,
	"range": 1000,
	"next_index": 0,
	"timestamp": "now"
	},
	{
	"desc": descsum_create("wsh(multi(2,[7b2d0242/84h/1h/0h]" + chg_xpub1 + "/," + xprv2 + "/84h/1h/0h/,[e81a0532/84h/1h/0h]" + chg_xpub3 + "/*))"),
	"active": True,
	"internal" : True,
	"range": 1000,
	"next_index": 0,
	"timestamp": "now"
	}])
	assert_equal(res[0]['success'], True)
	assert_equal(res[0]['warnings'][0], 'Not all private keys provided. Some wallet functionality may return unexpected errors')
	assert_equal(res[1]['success'], True)
	assert_equal(res[1]['warnings'][0], 'Not all private keys provided. Some wallet functionality may return unexpected errors')

	xprv1 = 'tprv8ZgxMBicQKsPevADjDCWsa6DfhkVXicu8NQUzfibwX2MexVwW4tCec5mXdCW8kJwkzBRRmAay1KZya4WsehVvjTGVW6JLqiqd8DdZ4xSg52'
	acc_xpub1 = 'tpubDCJtdt5dgJpdhW4MtaVYDhG4T4tF6jcLR1PxL43q9pq1mxvXgMS9Mzw1HnXG15vxUGQJMMSqCQHMTy3F1eW5VkgVroWzchsPD5BUojrcWs8' # /84'/0'/0'
	chg_xpub1 = 'tpubDCXqdwWZcszwqYJSnZp8eARkxGJfHAk23KDxbztV4BbschfaTfYLTcSkSJ3TN64dRqwa1rnFUScsYormKkGqNbbPwkorQimVevXjxzUV9Gf' # /84'/1'/0'
	xprv2 = 'tprv8ZgxMBicQKsPdSNWUhDiwTScDr6JfkZuLshTRwzvZGnMSnGikV6jxpmdDkC3YRc4T3GD6Nvg9uv6hQg73RVv1EiTXDZwxVbsLugVHU8B1aq'
	acc_xprv2 = 'tprv8gVCsmRAxVSxyUpsL13Y7ZEWBFPWbgS5E2MmFVNGuANrknvmmn2vWnmHvU8AwEFYzR2ji6EeZLSCLVacsYkvor3Pcb5JY5FGcevqTwYvdYx'
	acc_xpub2 = 'tpubDDBF2BTR6s8drwrfDei8WxtckGuSm1cyoKxYY1QaKSBFbHBYQArWhHPA6eJrzZej6nfHGLSURYSLHr7GuYch8aY5n61tGqgn8b4cXrMuoPH'
	chg_xpub2 = 'tpubDCYfZY2ceyHzYzMMVPt9MNeiqtQ2T7Uyp9QSFwYXh8Vi9iJFYXcuphJaGXfF3jUQJi5Y3GMNXvM11gaL4txzZgNGK22BFAwMXynnzv4z2Jh'
	xprv3 = 'tprv8ZgxMBicQKsPeonDt8Ka2mrQmHa61hQ5FQCsvWBTpSNzBFgM58cV2EuXNAHF14VawVpznnme3SuTbA62sGriwWyKifJmXntfNeK7zeqMCj1'
	acc_xpub3 = 'tpubDCsWoW1kuQB9kG5MXewHqkbjPtqPueRnXju7uM2NK7y3JYb2ajAZ9EiuZXNNuE4661RAfriBWhL8UsnAPpk8zrKKnZw1Ug7X4oHgMdZiU4E'
	chg_xpub3 = 'tpubDC6UGqnsQStngYuGD4MKsMy7eD1Yg9NTJfPdvjdG2JE5oZ7EsSL3WHg4Gsw2pR5K39ZwJ46M1wZayhedVdQtMGaUhq5S23PH6fnENK3V1sb'

Uh oh!

Conversation

w0xlt commented Jun 9, 2026

Uh oh!

DrahtBot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

pablomartin4btc left a comment

Choose a reason for hiding this comment

Uh oh!

b-l-u-e commented Jun 10, 2026

Uh oh!

rkrux commented Jun 12, 2026

Uh oh!

theStack commented Jun 12, 2026

Uh oh!

rkrux left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

w0xlt commented Jun 18, 2026

Uh oh!

w0xlt commented Jun 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pablomartin4btc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

w0xlt commented Jul 10, 2026

Uh oh!

b-l-u-e commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

DrahtBot commented Jun 9, 2026 •

edited

Loading