Fix taptree hash computation in test_utils
What the developer wrote
Authored by Salvatore Ingala
Fix taptree hash computation in test_utils
What changed, and why it matters
This commit fixes a copy-paste bug in test helper code used during automated testing of Ledger's Bitcoin app. The bug caused the right-hand branch of a Taproot script tree to be ignored and the left-hand branch to be hashed twice when computing the tree's root hash. This only affects Python test utilities, not the device firmware or production wallet code, so real user funds are not at risk. However, the bug could have made some tests silently pass with an incorrect taptree hash, weakening test coverage for Taproot/MuSig2 features.
No immediate user action required. Ledger should verify that any tests relying on these utilities now produce the expected taptree hashes and, if any test vectors were previously wrong, regenerate or update them. Consider adding regression tests for asymmetric taptrees to catch similar mistakes.
Security signals we found
Incorrect cryptographic hash construction in Taproot taptree root computation
Copy-paste error causing left subtree to be hashed twice and right subtree omitted
Test-only code path, not production firmware
Silent test-coverage degradation risk for Taproot/MuSig2 wallet policies
Evidence from the diff
In both test_utils/musig2.py and test_utils/wallet_policy.py, the Tree.get_taptree_hash() method incorrectly computed right_h by calling self.left.get_taptree_hash() instead of self.right.get_taptree_hash(). The patch corrects this and also updates the TrDescriptorTemplate.get_taptree_hash() signature to accept and forward keys_info, which was already required by Tree.get_taptree_hash(). These files are test-support utilities; no device-side code is changed.
Changed components
test_utils/musig2.pytest_utils/wallet_policy.pyLedger Bitcoin app test suite (Taproot/MuSig2 policy tests)Inspect captured patch +6 / −5
diff --git a/test_utils/musig2.py b/test_utils/musig2.py
index 68e8d19..adbfe11 100644
--- a/test_utils/musig2.py
+++ b/test_utils/musig2.py
@@ -230,7 +230,7 @@ class Tree:
assert self.left is not None and self.right is not None
left_h = self.left.get_taptree_hash(
keys_info, is_change, address_index)
- right_h = self.left.get_taptree_hash(
+ right_h = self.right.get_taptree_hash(
keys_info, is_change, address_index)
if left_h <= right_h:
return taproot.tagged_hash("TapBranch", left_h + right_h)
@@ -359,10 +359,10 @@ class TrDescriptorTemplate:
for placeholder, script in self.tree.placeholders():
yield (placeholder, script)
- def get_taptree_hash(self, is_change: bool, address_index: int) -> bytes:
+ def get_taptree_hash(self, keys_info: List[str], is_change: bool, address_index: int) -> bytes:
if self.tree is None:
raise ValueError("There is no taptree")
- return self.tree.get_taptree_hash(is_change, address_index)
+ return self.tree.get_taptree_hash(keys_info, is_change, address_index)
class PsbtMusig2Cosigner(ABC):
@@ -506,7 +506,8 @@ def process_placeholder(
if tapleaf_desc is None:
t = der_key.pubkey[-32:]
if desc_tmpl.tree is not None:
- t += desc_tmpl.get_taptree_hash(is_change, address_index)
+ t += desc_tmpl.get_taptree_hash(
+ wallet_policy.keys_info, is_change, address_index)
tweaks.append(taproot.tagged_hash("TapTweak", t))
is_xonly_tweak.append(True)
diff --git a/test_utils/wallet_policy.py b/test_utils/wallet_policy.py
index 8285414..34bbba3 100644
--- a/test_utils/wallet_policy.py
+++ b/test_utils/wallet_policy.py
@@ -177,7 +177,7 @@ class Tree:
assert self.left is not None and self.right is not None
left_h = self.left.get_taptree_hash(
keys_info, is_change, address_index)
- right_h = self.left.get_taptree_hash(
+ right_h = self.right.get_taptree_hash(
keys_info, is_change, address_index)
if left_h <= right_h:
return tagged_hash("TapBranch", left_h + right_h)
Why this scored 18/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.