What the developer wrote
Authored by Pac
Simplify internal key validation for taproot (#820)
* Simplify internal key validation for taproot
There is no standard for how to generate the taproot keypath chaincode to be used with the NUMS pubkey.
Liana does it by hashing the taptree keys;
Coldcard uses a random chaincode;
Nunchuck hashes the taptree keys, but remove duplicates and sort the keys before hashing.
This fix changes the verification logic to check only the NUMS pubkey and ignore the chaincode.
* Allow loading of non-deterministic chain code in wallet
The wallet now loads the non-deterministic chain code without raising an error, indicating it is considered valid.
What changed, and why it matters
This change relaxes how Krux checks the 'internal key' used in Taproot multisig wallets. Previously it required the wallet to use a specific deterministic chain code derived from the other public keys; now it only checks that the internal key is the well-known NUMS (provably unspendable) public key and ignores the chain code. This makes Krux compatible with wallets like Coldcard and Nunchuck that use different chain-code conventions, but it removes a validation step that ensured the internal key was constructed in a specific reproducible way.
Review whether ignoring the chain code is acceptable for all supported wallet policies. The NUMS pubkey check still prevents key-path spending, but users should confirm that descriptor parsing and taptree validation otherwise reject malicious descriptors. Consider documenting the rationale and any residual risks for cross-vendor compatibility.
Security signals we found
Validation scope reduced: chain-code derivation check removed
Taproot internal-key trust boundary relaxed
NUMS pubkey check retained, so funds remain provably unspendable via key path
No cryptographic bug fixed; change is interoperability-driven
Evidence from the diff
The patch removes deterministic chain-code validation for the Taproot internal key. It deletes code that SHA-256-hashed the other descriptor keys to produce a chain code, built an HDKey from NUMS_PUBKEY with that chain code, and compared the base58 encoding to the descriptor’s first key. It now only compares descriptor.keys[0].key.get_public_key() to embit.ec.NUMS_PUBKEY. The test is updated to expect a successful load for a non-deterministic chain code instead of a ValueError.
Changed components
src/krux/wallet.pytests/test_wallet.pyInspect captured patch +4 / −25
diff --git a/src/krux/wallet.py b/src/krux/wallet.py
index b3948e1..5533cb3 100644
--- a/src/krux/wallet.py
+++ b/src/krux/wallet.py
@@ -265,32 +265,12 @@ class Wallet:
elif self.descriptor.miniscript is not None or self.descriptor.taptree:
if self.descriptor.taptree:
if not descriptor.keys[0].origin:
- import hashlib
from embit.ec import NUMS_PUBKEY
- from embit.bip32 import HDKey
-
- # In case internal key is disabled, check if NUMS is known
-
- # Hash all pubkeys, except internal, to compute deterministic chain code
- hasher = hashlib.sha256()
- for key in descriptor.keys[1:]:
- hasher.update(key.sec())
- det_chain_code = hasher.digest()
-
- # Create provably unspendable deterministic key
- version = self.descriptor.keys[0].key.version
- provably_unspendable = HDKey(
- NUMS_PUBKEY, det_chain_code, version=version
- )
-
+ # Check if BIP-0341 NUMS was used
# Compare expected provably unspendable key with first descriptor key
- if (
- descriptor.keys[0].key.to_base58()
- != provably_unspendable.to_base58()
- ):
+ if (descriptor.keys[0].key.get_public_key() != NUMS_PUBKEY):
self.wallet_data = None
raise ValueError("Internal key not provably unspendable")
-
taproot_txt = "TR "
miniscript_type = P2TR
else:
diff --git a/tests/test_wallet.py b/tests/test_wallet.py
index d32a427..8737b18 100644
--- a/tests/test_wallet.py
+++ b/tests/test_wallet.py
@@ -1663,9 +1663,8 @@ def test_provably_unspendable_non_deterministic_chain_code(mocker, m5stickv, tda
NON_DETERMINISTIC_CHAIN_CODE = "tr(xpub661MyMwAqRbcFhaVQsdthkuGZQS3e9MENERDseTmnmhX2dFdgitmaFGLSPeXtcRzQ8jQaG3XCYPUknq7jX86V1qU6p981ripVVbvYnE5XpV/<0;1>/*,{and_v(v:multi_a(2,[55f8fc5d/48'/0'/0'/2']xpub6EKmKYGYc1WY6t9d3d9SksR8keSaPZbFa6tqsGiH4xVxx8d2YyxSX7WG6yXEX3CmG54dPCxaapDw1XsjwCmfoqP7tbsAeqMVfKvqSAu4ndy/<2;3>/*,[3e15470d/48'/0'/0'/2']xpub6F2P6Pz5KLPgCc6pTBd2xxCunaSYWc8CdkL28W5z15pJrN3aCYY7mCUAkCMtqrgT2wdhAGgRnJxAkCCUpGKoXKxQ57yffEGmPwtYA3DEXwu/<2;3>/*,[d3a80c8b/48'/0'/0'/2']xpub6FKYY6y3oVi7ihSCszFKRSeZj5SzrfSsUFXhKqjMV4iigrLhxwMX3mrjioNyLTZ5iD3u4wU9S3tyzpJGxhd5geaXoQ68jGz2M6dfh2zJrUv/<0;1>/*),older(65535)),multi_a(2,[55f8fc5d/48'/0'/0'/2']xpub6EKmKYGYc1WY6t9d3d9SksR8keSaPZbFa6tqsGiH4xVxx8d2YyxSX7WG6yXEX3CmG54dPCxaapDw1XsjwCmfoqP7tbsAeqMVfKvqSAu4ndy/<0;1>/*,[3e15470d/48'/0'/0'/2']xpub6F2P6Pz5KLPgCc6pTBd2xxCunaSYWc8CdkL28W5z15pJrN3aCYY7mCUAkCMtqrgT2wdhAGgRnJxAkCCUpGKoXKxQ57yffEGmPwtYA3DEXwu/<0;1>/*)})"
wallet = Wallet(tdata.TAP_MINISCRIPT_KEY)
- with pytest.raises(ValueError, match="Internal key not provably unspendable"):
- wallet.load(NON_DETERMINISTIC_CHAIN_CODE, FORMAT_NONE)
- assert not wallet.is_loaded()
+ wallet.load(NON_DETERMINISTIC_CHAIN_CODE, FORMAT_NONE)
+ assert wallet.is_loaded() #ignored the chaincode
def test_parse_wallet_raises_errors(mocker, m5stickv, tdata):
Why this scored 34/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.