test: introduce ExtendedPrivateKey and ExtendedPublicKey classes #35543

rkrux commented at 3:23 PM on June 16, 2026: contributor

Many a times there has been a need to come up with dynamic xprvs and xpubs in the functional tests, but the lack of code that creates them dynamically has led to the presence of several hardcoded keys in the testing framework. This is not developer friendly and not self-documenting, clutters the testing code, and makes it difficult to update the tests in the future.

This PR introduces two utility classes ExtendedPrivateKey and ExtendedPublicKey that allows the developer to create them on the fly to be used in the tests. I have intentionally not introduced any library for this purpose and have reused the existing libraries and functions in the framework. The implementation is supposed to provide basic functionality for creating xprv randomly or from a fixed seed, creating corresponding xpub, and deriving child xprvs and xpubs at custom derivation paths.

I've updated many tests to show how these can be used, there are more tests as well that can be updated in the future to completely remove such non-deterministic hardcoded keys.

test: generalise byte_to_base58 utility function to allow more version types

Passing a version in the byte form is allowed now in addition to the integral
version type. This will be helpful in the next commit.

4dbaa7cc65

DrahtBot added the label Tests on Jun 16, 2026

DrahtBot commented at 3:24 PM on June 16, 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/35543.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	achow101, w0xlt
Concept ACK	pseudoramdom

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:

#35493 (wallet, descriptor: Fix MuSig private key completeness checks on importdescriptors by w0xlt)
#35442 (test: remove usages of MAX_BIP125_RBF_SEQUENCE constant from functional tests by rkrux)
#35433 (wallet: deprecate replaceable argument from transaction (and psbt) creation (and modification) RPCs by rkrux)
#35377 (wallet: Allow importing of descriptors without private keys when the wallet has the private keys by achow101)
#32857 (wallet: allow skipping script paths by Sjors)

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.

rkrux force-pushed on Jun 16, 2026

DrahtBot added the label CI failed on Jun 16, 2026

rkrux force-pushed on Jun 17, 2026

rkrux commented at 7:43 AM on June 17, 2026: contributor

Forced-pushed to fix the failing lint CI job.

rkrux force-pushed on Jun 17, 2026

DrahtBot removed the label CI failed on Jun 17, 2026

rkrux force-pushed on Jun 17, 2026

rkrux commented at 10:49 AM on June 17, 2026: contributor

Force pushed to use the ExtendedPrivateKey in more functional tests.

in test/functional/feature_notifications.py:78 in d583a358f4 outdated

  74 | @@ -74,7 +75,7 @@ def setup_network(self):
  75 |      def run_test(self):
  76 |          if self.is_wallet_compiled():
  77 |              # Setup the descriptors to be imported to the wallet
  78 | -            xpriv = "tprv8ZgxMBicQKsPfHCsTwkiM1KT56RXbGGTqvc2hgqzycpwbHqqpcajQeMRZoBD35kW4RtyCemu6j34Ku5DEspmgjKdt2qe4SvRch5Kk8B8A2v"
  79 | +            xpriv = ExtendedPrivateKey.generate().to_string()

pseudoramdom commented at 5:12 PM on June 17, 2026:

I’m not sure if ExtendedPrivateKey.generate() is a good fit for functional tests. Randomized key material can make failures harder to reproduce and debug, especially if a failure only occurs for a particular generated key/path combination.

Should we be using ExtendedPrivateKey.from_seed() instead to get deterministic results?

rkrux commented at 10:27 AM on June 18, 2026:

I have noticed two kinds of usages in the functional tests - deterministic and non-deterministic, and the latter is more prevalent in the functional tests where the test just needs some extended keys on the fly to be used in the process. It's for these use cases the generate function is helpful. For deterministic use-cases, the from_seed function can be used.

rkrux commented at 10:31 AM on June 18, 2026:

This style also builds upon the existing style we have in the form of the generate_keypair function in wallet_util.py that also generates keys on the fly - the use cases of which are many.

in test/functional/wallet_importdescriptors.py:619 in d583a358f4 outdated

 615 | @@ -606,9 +616,9 @@ def run_test(self):
 616 |  
 617 |          assert_equal(wmulti_priv.getwalletinfo()['keypoolsize'], 1001) # Range end (1000) is inclusive, so 1001 addresses generated
 618 |          addr = wmulti_priv.getnewaddress('', 'bech32') # uses receive 0
 619 | -        assert_equal(addr, 'bcrt1qdt0qy5p7dzhxzmegnn4ulzhard33s2809arjqgjndx87rv5vd0fq2czhy8') # Derived at m/84'/0'/0'/0
 620 | +        assert_equal(wmulti_priv.getaddressinfo(addr)["desc"].count(f"{derivation_path}/0"), 3) # Derived at m/84h/0h/0h/0 for all three keys

pseudoramdom commented at 5:23 PM on June 17, 2026:

It looks like we're checking the reported descriptor contains the expected path for the three keys? But we used to have a stronger assertion that the descriptor derived the expected output address. If we generated keys using ExtendedPrivateKey.from_seed with some fixed bytes, we can derive the descriptor for those and then assert a concrete address like before ;)

rkrux commented at 11:01 AM on June 18, 2026:

But we used to have a stronger assertion that the descriptor derived the expected output address.

I have reverted to asserting for the addresses equality but without the hardcoded expected address.

If we generated keys using ExtendedPrivateKey.from_seed with some fixed bytes, we can derive the descriptor for those and then assert a concrete address like before ;)

I don't see why from_seed is needed, the expected address can be derived from derived public keys from the generate function and the corresponding multisig script.

pseudoramdom commented at 6:26 PM on June 18, 2026:

This works too.

I don't see why from_seed is needed

That suggestion was specifically if you needed to use old style of assertion assert_equal(addr, "bcrt1..."). But what you have now is better. :)

in test/functional/wallet_importdescriptors.py:641 in d583a358f4

 638 |                              "next_index": 0,
 639 |                              "timestamp": "now"},
 640 |                              success=True,
 641 |                              wallet=wmulti_pub)
 642 | -        self.test_importdesc({"desc":"wsh(multi(2,[7b2d0242/84h/1h/0h]" + chg_xpub1 + "/*,[59b09cd6/84h/1h/0h]" + chg_xpub2 + "/*,[e81a0532/84h/1h/0h]" + chg_xpub3 + "/*))#c08a2rzv",
 643 | +        self.test_importdesc({"desc": descsum_create("wsh(multi(2,[7b2d0242/84h/1h/0h]" + chg_xpub1 + "/*,[59b09cd6/84h/1h/0h]" + chg_xpub2 + "/*,[e81a0532/84h/1h/0h]" + chg_xpub3 + "/*))"),

pseudoramdom commented at 5:27 PM on June 17, 2026:

These fingerprints are stale and no longer corresponds to the the generated xpubs. Should we add a helper

def fingerprint(self):
    return self._fingerprint().hex()

And use that here for correctness?

pseudoramdom commented at 5:28 PM on June 17, 2026:

If it's not relevant to this test, can we omit them?

rkrux commented at 10:24 AM on June 18, 2026:

Should we add a helper And use that here for correctness?

It's the parent fingerprint here, added a helper for it.

If it's not relevant to this test, can we omit them?

I don't think it's that important but I have kept it because by using the fingerprint helper, the descriptor becomes very much self-documenting and easy to read.

pseudoramdom commented at 5:45 PM on June 17, 2026: none

Concept ACK-ish on reducing hardcoded xpub/xprv blobs in the functional tests.

Review d583a358f4f471eb4edc0e2218e37a0d7bcedd36

Adding a custom BIP32 implementation to the test framework seems like something that'll require ongoing maintenance to adhere to the BIP and handle edge cases.
It is good that this is backed by BIP32 test vectors, but if this helper a bug, it could make tests pass because the test data and expectations may drift together.

That said, I'd feel better if the tests use deterministic seeds instead of generating new random keys, so that failures are reproducible and expected outputs can remain pinned

rkrux force-pushed on Jun 18, 2026

DrahtBot added the label CI failed on Jun 18, 2026

rkrux commented at 10:35 AM on June 18, 2026: contributor

Adding a custom BIP32 implementation to the test framework seems like something that'll require ongoing maintenance to adhere to the BIP and handle edge cases.

These are just basic utilities that are needed for the functional tests. Not everything from the BIP needs to be reimplemented. BIP32 is more than a decade old and has been in the deployed stage for quite a while. I don't see why ongoing maintenance would be needed here. These extended classes act as thin wrappers over the existing ECKey and ECPubKey classes in key.py.

It is good that this is backed by BIP32 test vectors, but if this helper a bug, it could make tests pass because the test data and expectations may drift together.

I don't fully understand this comment. I believe the BIP32 tests vectors in the unit test should be sufficient to uncover a bug in the implementation.

rkrux force-pushed on Jun 18, 2026

DrahtBot removed the label CI failed on Jun 18, 2026

in test/functional/test_framework/extendedkey.py:94 in e56196c1de

  89 | +        IL = I[:32]
  90 | +        IR = I[32:]
  91 | +
  92 | +        child_secret = (int.from_bytes(self.key.get_bytes(), "big") + int.from_bytes(IL, "big")) % ORDER
  93 | +        if child_secret is None:
  94 | +            raise ValueError("Invalid BIP32 child")

w0xlt commented at 11:49 PM on June 18, 2026:

Is this is None unreachable, isn't it ?

Suggestion:

diff --git a/test/functional/test_framework/extendedkey.py b/test/functional/test_framework/extendedkey.py
index f44fa62fe1..c55c660ccb 100644
--- a/test/functional/test_framework/extendedkey.py
+++ b/test/functional/test_framework/extendedkey.py
@@ -89,9 +89,12 @@ class ExtendedPrivateKey:
         IL = I[:32]
         IR = I[32:]
 
-        child_secret = (int.from_bytes(self.key.get_bytes(), "big") + int.from_bytes(IL, "big")) % ORDER
-        if child_secret is None:
-            raise ValueError("Invalid BIP32 child")
+        IL_int = int.from_bytes(IL, "big")
+        child_secret = (IL_int + int.from_bytes(self.key.get_bytes(), "big")) % ORDER
+        # Per BIP32, if IL >= n or the child key is 0 the key is invalid. This is
+        # astronomically unlikely (~1 in 2^127), so reject rather than retrying the next index.
+        if IL_int >= ORDER or child_secret == 0:
+            raise ValueError("Invalid BIP32 child key")
 
         child = ECKey()
         child.set(child_secret.to_bytes(32, 'big'), compressed=True)
@@ -135,13 +138,15 @@ class ExtendedPublicKey:
         IL = I[:32]
         IR = I[32:]
 
-        child_point = int.from_bytes(IL, "big") * secp256k1.G + self.pubkey.p
+        IL_int = int.from_bytes(IL, "big")
+        child_point = IL_int * secp256k1.G + self.pubkey.p
+        # Per BIP32, if IL >= n or the resulting point is infinity the key is invalid.
+        if IL_int >= ORDER or child_point.infinity:
+            raise ValueError("Invalid BIP32 child key")
+
         child_pubkey = ECPubKey()
         child_pubkey.set(child_point.to_bytes_compressed())
 
-        if child_pubkey is None:
-            raise ValueError("Invalid BIP32 child")
-
         return ExtendedPublicKey(child_pubkey, IR, self.depth + 1, self._fingerprint(), index)
 
     def _serialize(self):

</details>

rkrux commented at 10:41 AM on June 19, 2026:

Yes, good point. Used this suggestion, made you co-author.

in test/functional/wallet_importdescriptors.py:346 in e56196c1de

 339 | @@ -336,8 +340,9 @@ def run_test(self):
 340 |                       ismine=False)
 341 |  
 342 |          # # Test ranged descriptors
 343 | -        xpriv = "tprv8ZgxMBicQKsPeuVhWwi6wuMQGfPKi9Li5GtX35jVNknACgqe3CY4g5xgkfDDJcmtF7o1QnxWDRYw4H5P26PXq7sbcUkEqeR4fg3Kxp2tigg"
 344 | -        xpub = "tpubD6NzVbkrYhZ4YNXVQbNhMK1WqguFsUXceaVJKbmno2aZ3B6QfbMeraaYvnBSGpV3vxLyTTK9DYT1yoEck4XUScMzXoQ2U2oSmE2JyMedq3H"
 345 | +        extended_key = ExtendedPrivateKey.generate()
 346 | +        xpriv = extended_key.to_string()
 347 | +        xpub = extended_key.pubkey().to_string()
 348 |          addresses = ["2N7yv4p8G8yEaPddJxY41kPihnWvs39qCMf", "2MsHxyb2JS3pAySeNUsJ7mNnurtpeenDzLA"] # hdkeypath=m/0'/0'/0' and 1'

w0xlt commented at 12:02 AM on June 19, 2026:

The fixed addresses can be replaced with addresses derived from ExtendedPrivateKey.generate().

Suggestion:

diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py
index affeb5ade9..29f5fdd8a4 100755
--- a/test/functional/wallet_importdescriptors.py
+++ b/test/functional/wallet_importdescriptors.py
@@ -19,7 +19,11 @@ import concurrent.futures
 import threading
 import time
 
-from test_framework.address import script_to_p2wsh
+from test_framework.address import (
+    key_to_p2sh_p2wpkh,
+    key_to_p2wpkh,
+    script_to_p2wsh,
+)
 from test_framework.blocktools import COINBASE_MATURITY
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.descriptors import descsum_create
@@ -343,8 +347,9 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         extended_key = ExtendedPrivateKey.generate()
         xpriv = extended_key.to_string()
         xpub = extended_key.pubkey().to_string()
-        addresses = ["2N7yv4p8G8yEaPddJxY41kPihnWvs39qCMf", "2MsHxyb2JS3pAySeNUsJ7mNnurtpeenDzLA"] # hdkeypath=m/0'/0'/0' and 1'
-        addresses += ["bcrt1qrd3n235cj2czsfmsuvqqpr3lu6lg0ju7scl8gn", "bcrt1qfqeppuvj0ww98r6qghmdkj70tv8qpchehegrg8"] # wpkh subscripts corresponding to the above addresses
+        pubkeys = [extended_key.derive_path(f"m/0h/0h/{i}h").pubkey().pubkey.get_bytes() for i in range(2)]
+        addresses = [key_to_p2sh_p2wpkh(pubkey) for pubkey in pubkeys] # hdkeypath=m/0'/0'/0' and 1'
+        addresses += [key_to_p2wpkh(pubkey) for pubkey in pubkeys] # wpkh subscripts corresponding to the above addresses
         desc = "sh(wpkh(" + xpub + "/0/0/*" + "))"
 
         self.log.info("Ranged descriptors cannot have labels")

</details>

rkrux commented at 10:40 AM on June 19, 2026:

Thanks, done.

in test/functional/wallet_importdescriptors.py:589 in e56196c1de outdated

 596 | -        self.test_importdesc({"desc":"wsh(multi(2," + xprv1 + "/84h/0h/0h/*," + xprv2 + "/84h/0h/0h/*," + xprv3 + "/84h/0h/0h/*))#m2sr93jn",
 597 | +        derivation_path = "84h/0h/0h"
 598 | +        change_derivation_path = "84h/1h/0h"
 599 | +        extended_key_1 = ExtendedPrivateKey.generate()
 600 | +        xprv1 = extended_key_1.to_string()
 601 | +        acc_xpub1_key = extended_key_1.derive_path(derivation_path).pubkey()

w0xlt commented at 12:18 AM on June 19, 2026:

The generated root key fingerprints can be used in descriptor origins instead of parent_fingerprint()/stale hardcoded values, so [fingerprint/path]xpub metadata matches the generated keys.

Suggestion:

diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py
index affeb5ade9..9a4eba3d53 100755
--- a/test/functional/wallet_importdescriptors.py
+++ b/test/functional/wallet_importdescriptors.py
@@ -586,6 +586,7 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         change_derivation_path = "84h/1h/0h"
         extended_key_1 = ExtendedPrivateKey.generate()
         xprv1 = extended_key_1.to_string()
+        xprv1_fingerprint = extended_key_1._fingerprint().hex()
         acc_xpub1_key = extended_key_1.derive_path(derivation_path).pubkey()
         acc_xpub1 = acc_xpub1_key.to_string()
         chg_xpub1_key = extended_key_1.derive_path(change_derivation_path).pubkey()
@@ -593,6 +594,7 @@ class ImportDescriptorsTest(BitcoinTestFramework):
 
         extended_key_2 = ExtendedPrivateKey.generate()
         xprv2 = extended_key_2.to_string()
+        xprv2_fingerprint = extended_key_2._fingerprint().hex()
         acc_xprv2 = extended_key_2.derive_path(derivation_path).to_string()
         acc_xpub2_key = extended_key_2.derive_path(derivation_path).pubkey()
         acc_xpub2 = acc_xpub2_key.to_string()
@@ -601,6 +603,7 @@ class ImportDescriptorsTest(BitcoinTestFramework):
 
         extended_key_3 = ExtendedPrivateKey.generate()
         xprv3 = extended_key_3.to_string()
+        xprv3_fingerprint = extended_key_3._fingerprint().hex()
         acc_xpub3_key = extended_key_3.derive_path(derivation_path).pubkey()
         acc_xpub3 = acc_xpub3_key.to_string()
         chg_xpub3_key = extended_key_3.derive_path(change_derivation_path).pubkey()
@@ -644,14 +647,14 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         wmulti_pub = self.nodes[1].get_wallet_rpc("wmulti_pub")
         assert_equal(wmulti_pub.getwalletinfo()['keypoolsize'], 0)
 
-        self.test_importdesc({"desc": descsum_create(f"wsh(multi(2,[{acc_xpub1_key.parent_fingerprint()}/{derivation_path}]{acc_xpub1}/*,[{acc_xpub2_key.parent_fingerprint()}/{derivation_path}]{acc_xpub2}/*,[{acc_xpub3_key.parent_fingerprint()}/{derivation_path}]{acc_xpub3}/*))"),
+        self.test_importdesc({"desc": descsum_create(f"wsh(multi(2,[{xprv1_fingerprint}/{derivation_path}]{acc_xpub1}/*,[{xprv2_fingerprint}/{derivation_path}]{acc_xpub2}/*,[{xprv3_fingerprint}/{derivation_path}]{acc_xpub3}/*))"),
                             "active": True,
                             "range": 1000,
                             "next_index": 0,
                             "timestamp": "now"},
                             success=True,
                             wallet=wmulti_pub)
-        self.test_importdesc({"desc": descsum_create(f"wsh(multi(2,[{chg_xpub1_key.parent_fingerprint()}/{change_derivation_path}]{chg_xpub1}/*,[{chg_xpub2_key.parent_fingerprint()}/{change_derivation_path}]{chg_xpub2}/*,[{chg_xpub3_key.parent_fingerprint()}/{change_derivation_path}]{chg_xpub3}/*))"),
+        self.test_importdesc({"desc": descsum_create(f"wsh(multi(2,[{xprv1_fingerprint}/{change_derivation_path}]{chg_xpub1}/*,[{xprv2_fingerprint}/{change_derivation_path}]{chg_xpub2}/*,[{xprv3_fingerprint}/{change_derivation_path}]{chg_xpub3}/*))"),
                             "active": True,
                             "internal" : True,
                             "range": 1000,
@@ -687,14 +690,14 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         wmulti_priv1 = self.nodes[1].get_wallet_rpc("wmulti_priv1")
         res = wmulti_priv1.importdescriptors([
         {
-            "desc": descsum_create(f"wsh(multi(2,{xprv1}/{derivation_path}/*,[{acc_xpub2_key.parent_fingerprint()}/{derivation_path}]{acc_xpub2}/*,[{acc_xpub3_key.parent_fingerprint()}/{derivation_path}]{acc_xpub3}/*))"),
+            "desc": descsum_create(f"wsh(multi(2,{xprv1}/{derivation_path}/*,[{xprv2_fingerprint}/{derivation_path}]{acc_xpub2}/*,[{xprv3_fingerprint}/{derivation_path}]{acc_xpub3}/*))"),
             "active": True,
             "range": 1000,
             "next_index": 0,
             "timestamp": "now"
         },
         {
-            "desc": descsum_create(f"wsh(multi(2,{xprv1}/{change_derivation_path}/*,[{chg_xpub2_key.parent_fingerprint()}/{change_derivation_path}]{chg_xpub2}/*,[{chg_xpub3_key.parent_fingerprint()}/{change_derivation_path}]{chg_xpub3}/*))"),
+            "desc": descsum_create(f"wsh(multi(2,{xprv1}/{change_derivation_path}/*,[{xprv2_fingerprint}/{change_derivation_path}]{chg_xpub2}/*,[{xprv3_fingerprint}/{change_derivation_path}]{chg_xpub3}/*))"),
             "active": True,
             "internal" : True,
             "range": 1000,
@@ -710,14 +713,14 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         wmulti_priv2 = self.nodes[1].get_wallet_rpc('wmulti_priv2')
         res = wmulti_priv2.importdescriptors([
         {
-            "desc": descsum_create(f"wsh(multi(2,[{acc_xpub1_key.parent_fingerprint()}/{derivation_path}]{acc_xpub1}/*,{xprv2}/{derivation_path}/*,[{acc_xpub3_key.parent_fingerprint()}/{derivation_path}]{acc_xpub3}/*))"),
+            "desc": descsum_create(f"wsh(multi(2,[{xprv1_fingerprint}/{derivation_path}]{acc_xpub1}/*,{xprv2}/{derivation_path}/*,[{xprv3_fingerprint}/{derivation_path}]{acc_xpub3}/*))"),
             "active": True,
             "range": 1000,
             "next_index": 0,
             "timestamp": "now"
         },
         {
-            "desc": descsum_create(f"wsh(multi(2,[{chg_xpub1_key.parent_fingerprint()}/{change_derivation_path}]{chg_xpub1}/*,{xprv2}/{change_derivation_path}/*,[{chg_xpub3_key.parent_fingerprint()}/{change_derivation_path}]{chg_xpub3}/*))"),
+            "desc": descsum_create(f"wsh(multi(2,[{xprv1_fingerprint}/{change_derivation_path}]{chg_xpub1}/*,{xprv2}/{change_derivation_path}/*,[{xprv3_fingerprint}/{change_derivation_path}]{chg_xpub3}/*))"),
             "active": True,
             "internal" : True,
             "range": 1000,
@@ -807,7 +810,7 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         wmulti_priv3 = self.nodes[1].get_wallet_rpc("wmulti_priv3")
         res = wmulti_priv3.importdescriptors([
             {
-                "desc": descsum_create("wsh(multi(2," + xprv1 + "/84h/0h/0h/*,[59b09cd6/84h/0h/0h]" + acc_xpub2 + "/*,[e81a0532/84h/0h/0h]" + acc_xpub3 + "/*))"),
+                "desc": descsum_create(f"wsh(multi(2,{xprv1}/{derivation_path}/*,[{xprv2_fingerprint}/{derivation_path}]{acc_xpub2}/*,[{xprv3_fingerprint}/{derivation_path}]{acc_xpub3}/*))"),
                 "active": True,
                 "range": 1000,
                 "next_index": 0,
@@ -816,7 +819,7 @@ class ImportDescriptorsTest(BitcoinTestFramework):
         assert_equal(res[0]['success'], True)
         res = wmulti_priv3.importdescriptors([
             {
-                "desc": descsum_create("wsh(multi(2," + xprv1 + "/84h/0h/0h/*,[59b09cd6/84h/0h/0h]" + acc_xprv2 + "/*,[e81a0532/84h/0h/0h]" + acc_xpub3 + "/*))"),
+                "desc": descsum_create(f"wsh(multi(2,{xprv1}/{derivation_path}/*,[{xprv2_fingerprint}/{derivation_path}]{acc_xprv2}/*,[{xprv3_fingerprint}/{derivation_path}]{acc_xpub3}/*))"),
                 "active": True,
                 "range": 1000,
                 "next_index": 0,

</details>

rkrux commented at 10:42 AM on June 19, 2026:

Good catch that these are the root fingerprints; using parent fingerprints where the derivation path contains more than one level was incorrect. Taken this suggestion.

test: introduce ExtendedPrivateKey and ExtendedPublicKey classes

Using these classes allows the developers to dynamically create
xprvs and xpubs so that they don't need to hardcode such long keys
in the tests that most of the times clutter the tests and make them
difficult to update.

Co-authored-by: w0xlt <94266259+w0xlt@users.noreply.github.com>

afdb378082

test: add extendedkey.py unit tests by using BIP32 test vectors d2a03d50ac

rkrux force-pushed on Jun 19, 2026

in test/functional/wallet_listdescriptors.py:172 in e85bd7335c

 176 | +            descsum_create(f'tr({node.get_deterministic_priv_key().key},' + '{' + f'pk({pubkey}),pk([d34db33f/44h/0h/0h]{xpubs[0]}/0)' + '})'),
 177 | +            descsum_create('wsh(and_v(v:ripemd160(095ff41131e5946f3c85f79e44adbcf8e27e080e),multi(1,' + f'{node.get_deterministic_priv_key().key},{xpubs[1]}/0)))'),
 178 | +            descsum_create(f'tr({pubkey},pk(musig({xprvs[0]},{xpubs[2]})/7/8/*))'),
 179 | +            descsum_create(f'tr({pubkey},pk(musig({xprvs[1]}/10,{xpubs[3]}/11)/*))'),
 180 | +            descsum_create(f'tr({pubkey},' + '{' + f'pk(musig({xpubs[4]},{xprvs[2]})/12/*),pk(musig({xprvs[3]},{xpubs[4]})/13/*)' + '})'),
 181 | +            descsum_create(f'tr({pubkey},' + '{' + f'pk(musig({xpubs[5]},{xpubs[6]})/12/*),pk(musig({xprvs[4]},{xpubs[7]})/13/*)' + '})')

achow101 commented at 9:29 PM on June 22, 2026:

In e85bd7335c844604e690c44d43bd50aee9f5ffc3 "test: use ExtendedPrivateKey in wallet_listdescriptors.py"

{{ and }} can be used in f-strings to put { and } directly, instead of concatenating strings.

            descsum_create(f'tr({node.get_deterministic_priv_key().key},{{pk({pubkey}),pk([d34db33f/44h/0h/0h]{xpubs[0]}/0)}})'),
            descsum_create(f'wsh(and_v(v:ripemd160(095ff41131e5946f3c85f79e44adbcf8e27e080e),multi(1,{node.get_deterministic_priv_key().key},{xpubs[1]}/0)))'),
            descsum_create(f'tr({pubkey},pk(musig({xprvs[0]},{xpubs[2]})/7/8/*))'),
            descsum_create(f'tr({pubkey},pk(musig({xprvs[1]}/10,{xpubs[3]}/11)/*))'),
            descsum_create(f'tr({pubkey},{{pk(musig({xpubs[4]},{xprvs[2]})/12/*),pk(musig({xprvs[3]},{xpubs[4]})/13/*)}})'),
            descsum_create(f'tr({pubkey},{{pk(musig({xpubs[5]},{xpubs[6]})/12/*),pk(musig({xprvs[4]},{xpubs[7]})/13/*)}})')

rkrux commented at 9:44 AM on June 23, 2026:

Ah nice, thanks.

achow101 commented at 9:33 PM on June 22, 2026: member

Concept ACK

e85bd7335c844604e690c44d43bd50aee9f5ffc3 "test: use ExtendedPrivateKey in wallet_listdescriptors.py" still has hard coded xpubs and xprvs in wallet_listdescriptors.py.

in test/functional/wallet_taproot.py:27 in f2995a9e2f

 155 | -            "6b8747a6bbe4440d7386658476da51f6e49a220508a7ec77fe7bccc3e7baa916",
 156 | -            "4674bf4d9ebbe01bf0aceaca2472f63198655ecf2df810f8d69b38421972318e",
 157 | -        ]
 158 | -    }
 159 | -]
 160 | +xprvs = [ExtendedPrivateKey.generate() for _ in range(0, 13)]

w0xlt commented at 9:35 PM on June 22, 2026:

In the current code, wallet_taproot.py generates KEYS at module import time.

Bitcoin Core’s functional test framework only applies --randomseed later, inside BitcoinTestFramework.main(), before run_test().

So the generated taproot keys are created before the logged seed is applied.

If the test fails because of a specific generated key, rerunning with the same --randomseed will not necessarily regenerate that same key set.

The diff below defers key generation until run_test().

diff --git a/test/functional/wallet_taproot.py b/test/functional/wallet_taproot.py
index 5d7f9143ff..e5e0d36412 100755
--- a/test/functional/wallet_taproot.py
+++ b/test/functional/wallet_taproot.py
@@ -24,17 +24,18 @@ from test_framework.script import (
 )
 from test_framework.segwit_addr import encode_segwit_address
 
-xprvs = [ExtendedPrivateKey.generate() for _ in range(0, 13)]
-KEYS = [{
-    "xprv": xprv.to_string(),
-    "xpub": xprv.pubkey().to_string(),
-    "pubs": [
-        compute_xonly_pubkey(xprv.derive_path("m/0").key.get_bytes())[0].hex(),
-        compute_xonly_pubkey(xprv.derive_path("m/1").key.get_bytes())[0].hex(),
-        compute_xonly_pubkey(xprv.derive_path("m/2").key.get_bytes())[0].hex(),
-        compute_xonly_pubkey(xprv.derive_path("m/3").key.get_bytes())[0].hex(),
-    ]
-} for xprv in xprvs]
+def generate_keys():
+    xprvs = [ExtendedPrivateKey.generate() for _ in range(13)]
+    return [{
+        "xprv": xprv.to_string(),
+        "xpub": xprv.pubkey().to_string(),
+        "pubs": [
+            compute_xonly_pubkey(xprv.derive_path("m/0").key.get_bytes())[0].hex(),
+            compute_xonly_pubkey(xprv.derive_path("m/1").key.get_bytes())[0].hex(),
+            compute_xonly_pubkey(xprv.derive_path("m/2").key.get_bytes())[0].hex(),
+            compute_xonly_pubkey(xprv.derive_path("m/3").key.get_bytes())[0].hex(),
+        ]
+    } for xprv in xprvs]
 
 
 def key(hex_key):
@@ -263,12 +264,14 @@ class WalletTaprootTest(BitcoinTestFramework):
 
     def do_test(self, comment, pattern, privmap, treefn):
         nkeys = len(privmap)
-        keys = random.sample(KEYS, nkeys * 4)
+        keys = random.sample(self.keys, nkeys * 4)
         self.do_test_addr(comment, pattern, privmap, treefn, keys[0:nkeys])
         self.do_test_sendtoaddress(comment, pattern, privmap, treefn, keys[0:nkeys], keys[nkeys:2*nkeys])
         self.do_test_psbt(comment, pattern, privmap, treefn, keys[2*nkeys:3*nkeys], keys[3*nkeys:4*nkeys])
 
     def run_test(self):
+        self.keys = generate_keys()
+
         self.nodes[0].createwallet(wallet_name="boring")
         self.boring = self.nodes[0].get_wallet_rpc("boring")

rkrux commented at 9:44 AM on June 23, 2026:

Good catch, fixed it.

in test/functional/wallet_importdescriptors.py:668 in f2995a9e2f outdated

 665 | @@ -639,9 +666,9 @@ def run_test(self):
 666 |  
 667 |          assert_equal(wmulti_pub.getwalletinfo()['keypoolsize'], 1000) # The first one was already consumed by previous import and is detected as used
 668 |          addr = wmulti_pub.getnewaddress('', 'bech32') # uses receive 1

w0xlt commented at 9:51 PM on June 22, 2026:

The current xpub-only multisig test only checks that the descriptor metadata contains the expected derivation path. That can still pass even if the wallet derives the wrong script/address but preserves the path text.

The suggested below patch keeps that metadata check, but also computes the expected P2WSH multisig address from the generated xpubs and asserts the wallet address matches it.

diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py
index 6706dbab5d..f23b4a9442 100755
--- a/test/functional/wallet_importdescriptors.py
+++ b/test/functional/wallet_importdescriptors.py
@@ -666,9 +666,13 @@ class ImportDescriptorsTest(BitcoinTestFramework):
 
         assert_equal(wmulti_pub.getwalletinfo()['keypoolsize'], 1000) # The first one was already consumed by previous import and is detected as used
         addr = wmulti_pub.getnewaddress('', 'bech32') # uses receive 1
+        expected_addr = script_to_p2wsh(keys_to_multisig_script([k.derive_path("m/1").pubkey.get_bytes() for k in [acc_xpub1_key, acc_xpub2_key, acc_xpub3_key]], k=2))
+        assert_equal(addr, expected_addr) # Derived at m/84'/0'/0'/1
         assert_equal(wmulti_pub.getaddressinfo(addr)["desc"].count(f"{derivation_path}/1"), 3) # Derived at m/84h/0h/0h/1 for all three keys
         change_addr = wmulti_pub.getrawchangeaddress('bech32') # uses change 2
-        assert_equal(wmulti_pub.getaddressinfo(change_addr)["desc"].count(f"{change_derivation_path}/2"), 3) # Derived at m/84h/1h/0h/1 for all three keys
+        expected_change_addr = script_to_p2wsh(keys_to_multisig_script([k.derive_path("m/2").pubkey.get_bytes() for k in [chg_xpub1_key, chg_xpub2_key, chg_xpub3_key]], k=2))
+        assert_equal(change_addr, expected_change_addr)  # Derived at m/84'/1'/0'/2
+        assert_equal(wmulti_pub.getaddressinfo(change_addr)["desc"].count(f"{change_derivation_path}/2"), 3) # Derived at m/84h/1h/0h/2 for all three keys
         assert send_txid in self.nodes[0].getrawmempool(True)
         assert send_txid in (x['txid'] for x in wmulti_pub.listunspent(0))
         assert_equal(wmulti_pub.getwalletinfo()['keypoolsize'], 999)

rkrux commented at 9:44 AM on June 23, 2026:

Thanks, I forgot to update it when I updated for #35543 (review).

w0xlt commented at 9:52 PM on June 22, 2026: contributor

A few review comments:

test: use ExtendedPrivateKey in wallet_importdescriptors.py

This avoids hardcoding xprvs and xpubs in the test.

Also use key's parent fingerprint and derivation path while creating descriptors
so that all of it is self-documenting and not dependent on hardcoded values,
making them easy to read and update.

Also use descsum_create function more to avoid hardcoding the descriptor
checksum.

f988e6d6e6

test: use ExtendedPrivateKey in feature_notifications.py 003f2a01f6

test: use ExtendedPrivateKey in wallet_bumpfee.py ff3f6def9a

test: use ExtendedPrivateKey in wallet_createwallet.py 4100fac20e

test: use ExtendedPrivateKey in wallet_descriptor.py 06af0cddbb

test: use ExtendedPrivateKey in wallet_fundrawtransaction.py 9e20118720

test: use ExtendedPrivateKey in wallet_keypool.py 2ab6e590f7

test: use ExtendedPrivateKey in wallet_send.py bbfffcab58

test: use ExtendedPrivateKey in wallet_listdescriptors.py 89ceafafb9

test: use ExtendedPrivateKey in wallet_taproot.py 8791c4764c

rkrux force-pushed on Jun 23, 2026

rkrux commented at 9:45 AM on June 23, 2026: contributor

https://github.com/bitcoin/bitcoin/commit/e85bd7335c844604e690c44d43bd50aee9f5ffc3 "test: use ExtendedPrivateKey in wallet_listdescriptors.py" still has hard coded xpubs and xprvs in wallet_listdescriptors.py.

Thanks for highlighting, I think I got them all in that test now.

achow101 commented at 5:50 PM on June 23, 2026: member

ACK 8791c4764cad513df20fcf17fb806d2318ead11a

DrahtBot requested review from pseudoramdom on Jun 23, 2026

w0xlt commented at 6:12 PM on June 23, 2026: contributor

ACK 8791c4764cad513df20fcf17fb806d2318ead11a

achow101 merged this on Jun 26, 2026

achow101 closed this on Jun 26, 2026