Skip to content

Commit c40f3db

Browse files
Merge #7076: fix: only activate BIP44 descriptors for external signers, fix mocked signers
0a72191 test: drop incorrect descriptors (UdjinM6) 6b837a5 test: fix expected descriptor path (UdjinM6) fc73a5a fix: only activate BIP44 descriptors for external signers (UdjinM6) Pull request description: ## Issue being fixed or feature implemented When creating a wallet with an external signer (hardware wallet), the mock signer returns multiple descriptors (BIP44, BIP49, BIP84) for both receive and internal categories. Previously, all valid descriptors were being activated by calling AddActiveScriptPubKeyMan() for each one. This caused a database key collision since multiple descriptors tried to write to the same key (ACTIVEEXTERNALSPK or ACTIVEINTERNALSPK), resulting in non-deterministic behavior where sometimes BIP44 would be active and sometimes BIP84, depending on database flush timing. ## What was done? The fix follows the pattern from the non-external-signer branch which stores multiple descriptors but only activates certain types (External and Internal, but not CoinJoin). Now we: - Store ALL descriptors returned by the signer in m_spk_managers - Only activate descriptors matching the BIP44 path pattern /44'/{cointype}' This ensures deterministic behavior and fixes the intermittent test failure in wallet_signer.py where the test expected m/44' but sometimes got m/84'. Should fix intermittent test failures like https://github.com/dashpay/dash/actions/runs/20364286460/job/58521843159#step:6:1662 ## How Has This Been Tested? Run `wallet_signer.py` multiple times ## Breaking Changes n/a ## Checklist: - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: PastaPastaPasta: utACK 0a72191 knst: ACK 0a72191 Tree-SHA512: a570976023d1319542dd4a236feb4c39e4fc716fcd9c56fc3368180fe8fb95e9af07623fff1bef04c90a478dde34deea860dba4a63f74606dba54d86b0342b3d
2 parents 2a533fe + 0a72191 commit c40f3db

File tree

3 files changed

+11
-16
lines changed

3 files changed

+11
-16
lines changed

src/wallet/wallet.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3967,7 +3967,12 @@ void CWallet::SetupDescriptorScriptPubKeyMans(const SecureString& mnemonic_arg,
39673967
spk_manager->SetupDescriptor(std::move(desc));
39683968
uint256 id = spk_manager->GetID();
39693969
m_spk_managers[id] = std::move(spk_manager);
3970-
AddActiveScriptPubKeyMan(id, internal);
3970+
// Only activate BIP44 descriptors, similar to how the non-external-signer branch
3971+
// only activates certain types (External and Internal, but not CoinJoin)
3972+
std::string bip44_purpose = strprintf("/%d'/%s'", BIP32_PURPOSE_STANDARD, Params().ExtCoinType());
3973+
if (desc_str.find(bip44_purpose) != std::string::npos) {
3974+
AddActiveScriptPubKeyMan(id, internal);
3975+
}
39713976
}
39723977
}
39733978
}

test/functional/mocks/invalid_signer.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,13 @@ def enumerate(args):
2222

2323
def getdescriptors(args):
2424
xpub_pkh = "xpub6CRhJvXV8x2AKWvqi1ZSMFU6cbxzQiYrv3dxSUXCawjMJ1JzpqVsveH4way1yCmJm29KzH1zrVZmVwes4Qo6oXVE1HFn4fdiKrYJngqFFc6"
25-
xpub_sh = "xpub6CoNoq3Tg4tGSpom2BSwL42gy864KHo3TXkHxLxBbhvCkgmdVXADQmiHbLZhX3Me1cYhRx7s25Lpm4LnT5zu395ANHsXB2QvT9tqJDAibTN"
26-
xpub_pkh = "xpub6DUcLgY1DfgDy2RV6q4djwwsLitaoZDumbribqrR8mP78fEtgZa1XEsqT5MWQ7gwLwKsTQPT28XLoVE5A97rDNTwMXjmzPaNijoCApCbWvp"
2725

2826
sys.stdout.write(json.dumps({
2927
"receive": [
30-
"pkh([b3c19bfc/44'/1'/" + args.account + "']" + xpub_pkh + "/0/*)#h26nxtl9",
31-
"sh(pkh([b3c19bfc/49'/1'/" + args.account + "']" + xpub_sh + "/0/*))#32ry02yp",
32-
"pkh([b3c19bfc/84'/1'/" + args.account + "']" + xpub_pkh + "/0/*)#jftn8ppv"
28+
"pkh([b3c19bfc/44'/1'/" + args.account + "']" + xpub_pkh + "/0/*)#h26nxtl9"
3329
],
3430
"internal": [
35-
"pkh([b3c19bfc/44'/1'/" + args.account + "']" + xpub_pkh + "/1/*)#x7ljm70a",
36-
"sh(pkh([b3c19bfc/49'/1'/" + args.account + "']" + xpub_sh + "/1/*))#ytdjh437",
37-
"pkh([b3c19bfc/84'/1'/" + args.account + "']" + xpub_pkh + "/1/*)#rawj6535"
31+
"pkh([b3c19bfc/44'/1'/" + args.account + "']" + xpub_pkh + "/1/*)#x7ljm70a"
3832
]
3933
}))
4034

test/functional/mocks/signer.py

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,10 @@ def getdescriptors(args):
2525

2626
sys.stdout.write(json.dumps({
2727
"receive": [
28-
"pkh([00000001/44'/1'/" + args.account + "']" + xpub + "/0/*)#vt6w3l3j",
29-
"sh(pkh([00000001/49'/1'/" + args.account + "']" + xpub + "/0/*))#az7uyg3n",
30-
"pkh([00000001/84'/1'/" + args.account + "']" + xpub + "/0/*)#58ssc2nq"
28+
"pkh([00000001/44'/1'/" + args.account + "']" + xpub + "/0/*)#vt6w3l3j"
3129
],
3230
"internal": [
33-
"pkh([00000001/44'/1'/" + args.account + "']" + xpub + "/1/*)#all0v2p2",
34-
"sh(pkh([00000001/49'/1'/" + args.account + "']" + xpub + "/1/*))#mpkel968",
35-
"pkh([00000001/84'/1'/" + args.account + "']" + xpub + "/1/*)#9n439lrc"
31+
"pkh([00000001/44'/1'/" + args.account + "']" + xpub + "/1/*)#all0v2p2"
3632
]
3733
}))
3834

@@ -44,7 +40,7 @@ def displayaddress(args):
4440
return sys.stdout.write(json.dumps({"error": "Unexpected fingerprint", "fingerprint": args.fingerprint}))
4541

4642
expected_desc = [
47-
"pkh([00000001/84'/1'/0'/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#0yneg42r"
43+
"pkh([00000001/44'/1'/0'/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#0yneg42r"
4844
]
4945
if args.desc not in expected_desc:
5046
return sys.stdout.write(json.dumps({"error": "Unexpected descriptor", "desc": args.desc}))

0 commit comments

Comments
 (0)