← Watch feed
Improve signing performance in case of several internal keys
Public commit record
73/100 · Adequate
What the developer wrote
Authored by Salvatore Ingala
Improve signing performance in case of several internal keys
Instead of structuring the sign_transaction and (musig2 pubnonces)
functions as iterating:
for all internal keys:
for all inputs:
<do stuff>
we reverse the nested loop to:
for all inputs
for all internal keys:
<do stuff>
This avoids processing all the psbt keys for the inputs once for
every key placeholder (running it instead once per input), with
no functional change.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit is a performance optimization for the Ledger Bitcoin app. It reorders nested loops in two signing-related functions so that the app reads each transaction input's data once and then processes all relevant internal keys, rather than re-reading the input data once per key. The commit message and diff show no functional change—only a reduction in redundant work.
Recommended action
No security action required. Treat as a routine performance refactor. Standard regression testing for PSBT signing with multiple internal keys and MuSig2 pubnonce generation is sufficient.
Security signals we found
01
No security-relevant behavior change identified in commit message or diff
02
Refactoring only: loop reordering with identical helper invocations
03
Per-input taptree hash now computed once instead of once per key, reducing redundant computation
04
No new memory allocations, bounds checks removed, or cryptographic operations altered
Evidence from the diff
In src/handler/sign_psbt/sign_input.c, the functions produce_musig2_pubnonces and sign_transaction previously iterated outer loop over internal key expressions and inner loop over transaction inputs. This caused input PSBT maps (and, for MuSig2, taptree hashes) to be fetched/recomputed for every key. The patch reverses the nesting: it first marks which internal key expressions need processing, then iterates inputs once, fetches each input, computes the per-input taptree hash once, and finally loops over the marked key expressions. The same helper calls (fill_keyexpr_info_if_internal, fill_taproot_keyexpr_info, produce_and_yield_pubnonce/sign_transaction_input) are preserved with the same arguments and control flow, supporting the claim of no functional change.
Changed components
src/handler/sign_psbt/sign_input.cproduce_musig2_pubnoncessign_transactionInspect captured patch +128 / −89
diff --git a/src/handler/sign_psbt/sign_input.c b/src/handler/sign_psbt/sign_input.c
index 78ef3b1..a73b181 100644
--- a/src/handler/sign_psbt/sign_input.c
+++ b/src/handler/sign_psbt/sign_input.c
@@ -527,73 +527,93 @@ bool __attribute__((noinline)) produce_musig2_pubnonces(
return true; // nothing to do
}
- // Iterate over all the key expressions that correspond to keys owned by us
+ // Pre-pass: identify the MuSig2 key expressions we will produce pubnonces for
+ bool keyexpr_to_process[MAX_INTERNAL_KEY_EXPRESSIONS] = {0};
+ bool any_keyexpr_to_process = false;
for (size_t i_keyexpr = 0; i_keyexpr < st->account.n_internal_key_expressions; i_keyexpr++) {
keyexpr_info_t *keyexpr_info = &st->account.internal_key_expressions[i_keyexpr];
if (!keyexpr_info->to_sign ||
keyexpr_info->key_expression_ptr->type != KEY_EXPRESSION_MUSIG) {
continue;
}
-
if (!fill_keyexpr_info_if_internal(dc, st, keyexpr_info)) {
continue;
}
+ keyexpr_to_process[i_keyexpr] = true;
+ any_keyexpr_to_process = true;
+ }
- for (unsigned int i = 0; i < st->n_inputs; i++) {
- if (bitvector_get(internal_inputs, i)) {
- input_info_t input;
- memset(&input, 0, sizeof(input));
+ if (!any_keyexpr_to_process) {
+ return true; // nothing to do
+ }
- input_keys_callback_data_t callback_data = {.input = &input, .state = st};
- int res = call_get_merkleized_map_with_callback(
- dc,
- (void *) &callback_data,
- st->inputs_root,
- st->n_inputs,
- i,
- (merkle_tree_elements_callback_t) input_keys_callback,
- &input.in_out.map);
- if (res < 0) {
- SEND_SW(dc, SW_INCORRECT_DATA);
- return false;
- }
+ // Iterate over all internal inputs
+ for (unsigned int i = 0; i < st->n_inputs; i++) {
+ if (!bitvector_get(internal_inputs, i)) {
+ continue;
+ }
- // TODO: code duplication with sign_transaction_input
- if (keyexpr_info->tapleaf_ptr != NULL) {
- if (!fill_taproot_keyexpr_info(dc,
- st,
- &input,
- keyexpr_info->tapleaf_ptr,
- keyexpr_info,
- sign_psbt_cache)) {
- return false;
- }
- }
+ input_info_t input;
+ memset(&input, 0, sizeof(input));
+
+ input_keys_callback_data_t callback_data = {.input = &input, .state = st};
+ int res = call_get_merkleized_map_with_callback(
+ dc,
+ (void *) &callback_data,
+ st->inputs_root,
+ st->n_inputs,
+ i,
+ (merkle_tree_elements_callback_t) input_keys_callback,
+ &input.in_out.map);
+ if (res < 0) {
+ SEND_SW(dc, SW_INCORRECT_DATA);
+ return false;
+ }
- policy_node_tr_t *policy = (policy_node_tr_t *) st->account.policy_map;
- if (!isnull_policy_node_tree(&policy->tree)) {
- 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;
- }
- }
+ // 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]) {
+ continue;
+ }
+ keyexpr_info_t *keyexpr_info = &st->account.internal_key_expressions[i_keyexpr];
- if (!produce_and_yield_pubnonce(dc, st, signing_state, keyexpr_info, &input, i)) {
+ // TODO: code duplication with sign_transaction_input
+ if (keyexpr_info->tapleaf_ptr != NULL) {
+ if (!fill_taproot_keyexpr_info(dc,
+ st,
+ &input,
+ keyexpr_info->tapleaf_ptr,
+ keyexpr_info,
+ sign_psbt_cache)) {
return false;
}
}
+
+ if (!produce_and_yield_pubnonce(dc, st, signing_state, keyexpr_info, &input, i)) {
+ return false;
+ }
}
}
@@ -608,56 +628,75 @@ sign_transaction(dispatcher_context_t *dc,
const uint8_t internal_inputs[static BITVECTOR_REAL_SIZE(MAX_N_INPUTS_CAN_SIGN)]) {
LOG_PROCESSOR(__FILE__, __LINE__, __func__);
- // Iterate over all the key expressions that correspond to keys owned by us
+ // Pre-pass: identify the key expressions we will sign for.
+ bool keyexpr_to_process[MAX_INTERNAL_KEY_EXPRESSIONS] = {0};
+ bool any_keyexpr_to_process = false;
for (size_t i_keyexpr = 0; i_keyexpr < st->account.n_internal_key_expressions; i_keyexpr++) {
keyexpr_info_t *keyexpr_info = &st->account.internal_key_expressions[i_keyexpr];
if (!keyexpr_info->to_sign) {
continue;
}
-
if (!fill_keyexpr_info_if_internal(dc, st, keyexpr_info)) {
continue;
}
+ keyexpr_to_process[i_keyexpr] = true;
+ any_keyexpr_to_process = true;
+ }
- for (unsigned int i = 0; i < st->n_inputs; i++) {
- if (bitvector_get(internal_inputs, i)) {
- input_info_t input;
- memset(&input, 0, sizeof(input));
+ if (!any_keyexpr_to_process) {
+ return true;
+ }
- input_keys_callback_data_t callback_data = {.input = &input, .state = st};
- int res = call_get_merkleized_map_with_callback(
- dc,
- (void *) &callback_data,
- st->inputs_root,
- st->n_inputs,
- i,
- (merkle_tree_elements_callback_t) input_keys_callback,
- &input.in_out.map);
- if (res < 0) {
- SEND_SW(dc, SW_INCORRECT_DATA);
- return false;
- }
- if (keyexpr_info->tapleaf_ptr != NULL &&
- !fill_taproot_keyexpr_info(dc,
- st,
- &input,
- keyexpr_info->tapleaf_ptr,
- keyexpr_info,
- sign_psbt_cache)) {
- return false;
- }
+ // Iterate over all internal inputs
+ for (unsigned int i = 0; i < st->n_inputs; i++) {
+ if (!bitvector_get(internal_inputs, i)) {
+ continue;
+ }
- if (!sign_transaction_input(dc,
- st,
- sign_psbt_cache,
- signing_state,
- keyexpr_info,
- &input,
- i)) {
- // we do not send a status word, since sign_transaction_input
- // already does it on failure
- return false;
- }
+ input_info_t input;
+ memset(&input, 0, sizeof(input));
+
+ input_keys_callback_data_t callback_data = {.input = &input, .state = st};
+ int res = call_get_merkleized_map_with_callback(
+ dc,
+ (void *) &callback_data,
+ st->inputs_root,
+ st->n_inputs,
+ i,
+ (merkle_tree_elements_callback_t) input_keys_callback,
+ &input.in_out.map);
+ if (res < 0) {
+ SEND_SW(dc, SW_INCORRECT_DATA);
+ return false;
+ }
+
+ for (size_t i_keyexpr = 0; i_keyexpr < st->account.n_internal_key_expressions;
+ i_keyexpr++) {
+ if (!keyexpr_to_process[i_keyexpr]) {
+ continue;
+ }
+ keyexpr_info_t *keyexpr_info = &st->account.internal_key_expressions[i_keyexpr];
+
+ if (keyexpr_info->tapleaf_ptr != NULL &&
+ !fill_taproot_keyexpr_info(dc,
+ st,
+ &input,
+ keyexpr_info->tapleaf_ptr,
+ keyexpr_info,
+ sign_psbt_cache)) {
+ return false;
+ }
+
+ if (!sign_transaction_input(dc,
+ st,
+ sign_psbt_cache,
+ signing_state,
+ keyexpr_info,
+ &input,
+ i)) {
+ // we do not send a status word, since sign_transaction_input
+ // already does it on failure
+ return false;
}
}
}
Risk score
Our methodology →Why this scored 13/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.