Move taptree hash computation in the inner loop of produce_musig2_pubnonces
What the developer wrote
Authored by Salvatore Ingala
Move taptree hash computation in the inner loop of produce_musig2_pubnonces
What changed, and why it matters
This commit is a small performance refactor inside the Ledger Bitcoin app. It moves the calculation of a Taproot tree hash (used for advanced multi-signature scripts) so it only runs when actually needed, rather than once per input unconditionally. There is no direct evidence this fixes a security bug; it appears to be an optimization, though it slightly reduces the chance of unnecessary computation or state errors.
No immediate security action required. Treat as routine code maintenance. If auditing, verify that `input.taptree_hash` is correctly consumed only when `!keyexpr_info->is_tapscript` and that lazy computation does not skip required initialization for any downstream caller.
Security signals we found
Code movement only: same function called with same arguments
Refactor explicitly described as performance optimization by commit author
No input validation changes, no bounds checks added/removed
No cryptographic constants or algorithms changed
No error-handling paths added or removed
Evidence from the diff
In produce_musig2_pubnonces() in src/handler/sign_psbt/sign_input.c, the compute_taptree_hash() call was moved from outside the i_keyexpr loop to inside it, and is now guarded by !keyexpr_info->is_tapscript. The commit message and added comment explain this as an efficiency improvement: the taptree hash is only required for at most one key placeholder (the taproot keypath), so computing it lazily inside the loop avoids work when it is not needed. The same data (address index, change flag, wallet header fields, policy tree pointer, and output buffer input.taptree_hash) is still passed to compute_taptree_hash(). No logic changes to how the hash is computed or used are visible in the diff.
Changed components
src/handler/sign_psbt/sign_input.cproduce_musig2_pubnonces functionTaproot / MuSig2 nonce production pathInspect captured patch +28 / −22
diff --git a/src/handler/sign_psbt/sign_input.c b/src/handler/sign_psbt/sign_input.c
index 6cc3d94..0759f2f 100644
--- a/src/handler/sign_psbt/sign_input.c
+++ b/src/handler/sign_psbt/sign_input.c
@@ -585,28 +585,6 @@ bool __attribute__((noinline)) produce_musig2_pubnonces(
return false;
}
- // The taptree hash only depends on the input (change, address_index) and the policy,
- // so it is computed once per input and reused for every MuSig2 key expression below.
- policy_node_tr_t *policy = (policy_node_tr_t *) st->account.policy_map;
- bool has_taptree = !isnull_policy_node_tree(&policy->tree);
- if (has_taptree) {
- if (0 > compute_taptree_hash(
- dc,
- &(wallet_derivation_info_t){
- .address_index = input.in_out.address_index,
- .change = input.in_out.is_change ? 1 : 0,
- .keys_merkle_root = st->account.wallet_header.keys_info_merkle_root,
- .n_keys = st->account.wallet_header.n_keys,
- .wallet_version = st->account.wallet_header.version,
- .sign_psbt_cache = sign_psbt_cache},
- r_policy_node_tree(&policy->tree),
- input.taptree_hash)) {
- PRINTF("Error while computing taptree hash\n");
- SEND_SW(dc, SW_BAD_STATE);
- return false;
- }
- }
-
for (size_t i_keyexpr = 0; i_keyexpr < st->account.n_internal_key_expressions;
i_keyexpr++) {
if (!keyexpr_to_process[i_keyexpr]) {
@@ -614,6 +592,34 @@ bool __attribute__((noinline)) produce_musig2_pubnonces(
}
keyexpr_info_t *keyexpr_info = &st->account.internal_key_expressions[i_keyexpr];
+ if (!keyexpr_info->is_tapscript) {
+ // The taptree hash only depends on the input (change, address_index) and the
+ // policy, so it could be computed outside of this loop. However, it is only needed
+ // for at most a single key placeholder (the taproot keypath, if it's a musig with
+ // an internal key), and might not be needed at all otherwise. Therefore, it is
+ // actually more efficient to compute it here.
+ policy_node_tr_t *policy = (policy_node_tr_t *) st->account.policy_map;
+ bool has_taptree = !isnull_policy_node_tree(&policy->tree);
+ if (has_taptree) {
+ if (0 >
+ compute_taptree_hash(
+ dc,
+ &(wallet_derivation_info_t){
+ .address_index = input.in_out.address_index,
+ .change = input.in_out.is_change ? 1 : 0,
+ .keys_merkle_root = st->account.wallet_header.keys_info_merkle_root,
+ .n_keys = st->account.wallet_header.n_keys,
+ .wallet_version = st->account.wallet_header.version,
+ .sign_psbt_cache = sign_psbt_cache},
+ r_policy_node_tree(&policy->tree),
+ input.taptree_hash)) {
+ PRINTF("Error while computing taptree hash\n");
+ SEND_SW(dc, SW_BAD_STATE);
+ return false;
+ }
+ }
+ }
+
// TODO: code duplication with sign_transaction_input
if (keyexpr_info->tapleaf_ptr != NULL) {
if (!fill_taproot_keyexpr_info(dc,
Why this scored 17/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.