From 852355b72aefeeb2f3f504a503055e561092d0f4 Mon Sep 17 00:00:00 2001 From: Salvatore Ingala <6681844+bigspider@users.noreply.github.com> Date: Wed, 21 Feb 2024 11:46:02 +0100 Subject: [PATCH] Rename "key placeholder" with "key expression" where appropriate; added some comments. Generalizing to key expressions containing musig() makes it necessary to distinguish the key expressions in the wallet policy from the actual key placeholders that are just indexes to the list of key informations (@NUM in the descriptor template), whereas the two concepts were often not clearly separated in the code base. Renaming to "key expressions" makes the distinction more clear. --- src/common/wallet.c | 38 ++--- src/common/wallet.h | 40 +++--- src/handler/lib/policy.c | 240 ++++++++++++++++---------------- src/handler/lib/policy.h | 29 ++-- src/handler/sign_psbt.c | 293 +++++++++++++++++++-------------------- unit-tests/test_wallet.c | 56 ++++---- 6 files changed, 333 insertions(+), 363 deletions(-) diff --git a/src/common/wallet.c b/src/common/wallet.c index 6ee48483c..217f64ae1 100644 --- a/src/common/wallet.c +++ b/src/common/wallet.c @@ -448,7 +448,7 @@ static int parse_keyexpr(buffer_t *in_buf, buffer_t *out_buf) { char c; if (!buffer_read_u8(in_buf, (uint8_t *) &c)) { - return WITH_ERROR(-1, "Expected key placeholder"); + return WITH_ERROR(-1, "Expected key expression"); } if (c == '@') { @@ -547,12 +547,12 @@ static int parse_keyexpr(buffer_t *in_buf, || !buffer_peek(in_buf, &next_character) // we must be able to read the next character || !(next_character == '*' || next_character == '<') // and it must be '*' or '<' ) { - return WITH_ERROR(-1, "Expected /** or //* in key placeholder"); + return WITH_ERROR(-1, "Expected /** or //* in key expression"); } if (next_character == '*') { if (!consume_characters(in_buf, "**", 2)) { - return WITH_ERROR(-1, "Expected /** or //* in key placeholder"); + return WITH_ERROR(-1, "Expected /** or //* in key expression"); } out->num_first = 0; out->num_second = 1; @@ -562,18 +562,18 @@ static int parse_keyexpr(buffer_t *in_buf, out->num_first > 0x80000000u) { return WITH_ERROR( -1, - "Expected /** or //* in key placeholder, with unhardened M and N"); + "Expected /** or //* in key expression, with unhardened M and N"); } if (!consume_character(in_buf, ';')) { - return WITH_ERROR(-1, "Expected /** or //* in key placeholder"); + return WITH_ERROR(-1, "Expected /** or //* in key expression"); } if (parse_unsigned_decimal(in_buf, &out->num_second) == -1 || out->num_second > 0x80000000u) { return WITH_ERROR( -1, - "Expected /** or //* in key placeholder, with unhardened M and N"); + "Expected /** or //* in key expression, with unhardened M and N"); } if (out->num_first == out->num_second) { @@ -581,7 +581,7 @@ static int parse_keyexpr(buffer_t *in_buf, } if (!consume_characters(in_buf, ">/*", 3)) { - return WITH_ERROR(-1, "Expected /** or //* in key placeholder"); + return WITH_ERROR(-1, "Expected /** or //* in key expression"); } } } else { @@ -1477,13 +1477,13 @@ static int parse_script(buffer_t *in_buf, return WITH_ERROR(-1, "Out of memory"); } - policy_node_keyexpr_t *key_placeholder = + policy_node_keyexpr_t *key_expr = buffer_alloc(out_buf, sizeof(policy_node_keyexpr_t), true); - if (key_placeholder == NULL) { + if (key_expr == NULL) { return WITH_ERROR(-1, "Out of memory"); } - i_policy_node_keyexpr(&node->key_placeholder, key_placeholder); + i_policy_node_keyexpr(&node->key, key_expr); if (token == TOKEN_WPKH) { if (depth > 0 && ((context_flags & CONTEXT_WITHIN_SH) == 0)) { @@ -1496,7 +1496,7 @@ static int parse_script(buffer_t *in_buf, node->base.type = token; bool is_taproot = (context_flags & CONTEXT_WITHIN_TR) != 0; - if (0 > parse_keyexpr(in_buf, version, key_placeholder, is_taproot, out_buf)) { + if (0 > parse_keyexpr(in_buf, version, key_expr, is_taproot, out_buf)) { return WITH_ERROR(-1, "Couldn't parse key placeholder"); } @@ -1559,14 +1559,14 @@ static int parse_script(buffer_t *in_buf, return WITH_ERROR(-1, "Out of memory"); } - policy_node_keyexpr_t *key_placeholder = + policy_node_keyexpr_t *key_expr = buffer_alloc(out_buf, sizeof(policy_node_keyexpr_t), true); - if (key_placeholder == NULL) { + if (key_expr == NULL) { return WITH_ERROR(-1, "Out of memory"); } - i_policy_node_keyexpr(&node->key_placeholder, key_placeholder); + i_policy_node_keyexpr(&node->key, key_expr); - if (0 > parse_keyexpr(in_buf, version, key_placeholder, true, out_buf)) { + if (0 > parse_keyexpr(in_buf, version, key_expr, true, out_buf)) { return WITH_ERROR(-1, "Couldn't parse key placeholder"); } @@ -1682,7 +1682,7 @@ static int parse_script(buffer_t *in_buf, // We allocate the array of key indices at the current position in the output buffer // (on success) buffer_alloc(out_buf, 0, true); // ensure alignment of current pointer - i_policy_node_keyexpr(&node->key_placeholders, buffer_get_cur(out_buf)); + i_policy_node_keyexpr(&node->keys, buffer_get_cur(out_buf)); node->n = 0; while (true) { @@ -1697,16 +1697,16 @@ static int parse_script(buffer_t *in_buf, return WITH_ERROR(-1, "Expected ','"); } - policy_node_keyexpr_t *key_placeholder = (policy_node_keyexpr_t *) buffer_alloc( + policy_node_keyexpr_t *key_expr = (policy_node_keyexpr_t *) buffer_alloc( out_buf, sizeof(policy_node_keyexpr_t), true); // we align this pointer, as there's padding in an array of // structures - if (key_placeholder == NULL) { + if (key_expr == NULL) { return WITH_ERROR(-1, "Out of memory"); } - if (0 > parse_keyexpr(in_buf, version, key_placeholder, is_taproot, out_buf)) { + if (0 > parse_keyexpr(in_buf, version, key_expr, is_taproot, out_buf)) { return WITH_ERROR(-1, "Error parsing key placeholder"); } diff --git a/src/common/wallet.h b/src/common/wallet.h index 899db8b77..fe2751e8e 100644 --- a/src/common/wallet.h +++ b/src/common/wallet.h @@ -297,19 +297,6 @@ typedef struct policy_node_ext_info_s { unsigned int x : 1; // the last opcode is not EQUAL, CHECKSIG, or CHECKMULTISIG } policy_node_ext_info_t; -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wcomment" -// The compiler doesn't like /** inside a block comment, so we disable this warning temporarily. - -/** Structure representing a key placeholder. - * In V1, it's the index of a key expression in the key informations array, which includes the final - * / ** step. - * In V2, it's the index of a key expression in the key informations array, plus the two - * numbers a, b in the //* derivation steps; here, the xpubs in the key informations - * array don't have extra derivation steps. - */ -#pragma GCC diagnostic pop - DEFINE_REL_PTR(uint16, uint16_t) typedef struct { @@ -320,10 +307,23 @@ typedef struct { DEFINE_REL_PTR(musig_aggr_key_info, musig_aggr_key_info_t) typedef enum { - KEY_EXPRESSION_NORMAL = 0, // a key expression with a single key placeholder + KEY_EXPRESSION_NORMAL = 0, // a key expression with a single key KEY_EXPRESSION_MUSIG = 1 // a key expression containing a musig() } KeyExpressionType; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wcomment" +// The compiler doesn't like /** inside a block comment, so we disable this warning temporarily. + +/** Structure representing a key expression. + * In V1, it's the index of a key expression in the key informations array, which includes the final + * / ** step. + * In V2, it's the index of a key expression in the key informations array, plus the two + * numbers a, b in the //* derivation steps; here, the xpubs in the key informations + * array don't have extra derivation steps. + * In V2, musig() key expressions are also represented in this struct. + */ +#pragma GCC diagnostic pop // 12 bytes typedef struct { // the following fields are only used in V2 @@ -370,7 +370,7 @@ typedef policy_node_with_script3_t policy_node_with_scripts_t; // 4 bytes typedef struct { struct policy_node_s base; - rptr_policy_node_keyexpr_t key_placeholder; + rptr_policy_node_keyexpr_t key; } policy_node_with_key_t; // 8 bytes @@ -381,10 +381,10 @@ typedef struct { // 12 bytes typedef struct { - struct policy_node_s base; // type is TOKEN_MULTI or TOKEN_SORTEDMULTI - int16_t k; // threshold - int16_t n; // number of keys - rptr_policy_node_keyexpr_t key_placeholders; // pointer to array of exactly n key placeholders + struct policy_node_s base; // type is TOKEN_MULTI or TOKEN_SORTEDMULTI + int16_t k; // threshold + int16_t n; // number of keys + rptr_policy_node_keyexpr_t keys; // pointer to array of exactly n key expressions } policy_node_multisig_t; // 8 bytes @@ -434,7 +434,7 @@ typedef struct policy_node_tree_s { typedef struct { struct policy_node_s base; - rptr_policy_node_keyexpr_t key_placeholder; + rptr_policy_node_keyexpr_t key; rptr_policy_node_tree_t tree; // NULL if tr(KP) } policy_node_tr_t; diff --git a/src/handler/lib/policy.c b/src/handler/lib/policy.c index 131237945..a155a743c 100644 --- a/src/handler/lib/policy.c +++ b/src/handler/lib/policy.c @@ -456,13 +456,13 @@ __attribute__((noinline, warn_unused_result)) static int get_extended_pubkey( __attribute__((warn_unused_result)) static int get_derived_pubkey( dispatcher_context_t *dispatcher_context, const wallet_derivation_info_t *wdi, - const policy_node_keyexpr_t *key_placeholder, + const policy_node_keyexpr_t *key_expr, uint8_t out[static 33]) { PRINT_STACK_POINTER(); serialized_extended_pubkey_t ext_pubkey; - int ret = get_extended_pubkey(dispatcher_context, wdi, key_placeholder->key_index, &ext_pubkey); + int ret = get_extended_pubkey(dispatcher_context, wdi, key_expr->key_index, &ext_pubkey); if (ret < 0) { return -1; } @@ -470,7 +470,7 @@ __attribute__((warn_unused_result)) static int get_derived_pubkey( // we derive the // child of this pubkey // we reuse the same memory of ext_pubkey bip32_CKDpub(&ext_pubkey, - wdi->change ? key_placeholder->num_second : key_placeholder->num_first, + wdi->change ? key_expr->num_second : key_expr->num_first, &ext_pubkey); bip32_CKDpub(&ext_pubkey, wdi->address_index, &ext_pubkey); @@ -569,11 +569,10 @@ __attribute__((warn_unused_result)) static int process_generic_node(policy_parse const policy_node_with_key_t *policy = (const policy_node_with_key_t *) node->policy_node; uint8_t compressed_pubkey[33]; - if (-1 == - get_derived_pubkey(state->dispatcher_context, - state->wdi, - r_policy_node_keyexpr(&policy->key_placeholder), - compressed_pubkey)) { + if (-1 == get_derived_pubkey(state->dispatcher_context, + state->wdi, + r_policy_node_keyexpr(&policy->key), + compressed_pubkey)) { return -1; } @@ -591,11 +590,10 @@ __attribute__((warn_unused_result)) static int process_generic_node(policy_parse const policy_node_with_key_t *policy = (const policy_node_with_key_t *) node->policy_node; uint8_t compressed_pubkey[33]; - if (-1 == - get_derived_pubkey(state->dispatcher_context, - state->wdi, - r_policy_node_keyexpr(&policy->key_placeholder), - compressed_pubkey)) { + if (-1 == get_derived_pubkey(state->dispatcher_context, + state->wdi, + r_policy_node_keyexpr(&policy->key), + compressed_pubkey)) { return -1; } if (!state->is_taproot) { @@ -684,7 +682,7 @@ __attribute__((warn_unused_result)) static int process_pkh_wpkh_node(policy_pars if (-1 == get_derived_pubkey(state->dispatcher_context, state->wdi, - r_policy_node_keyexpr(&policy->key_placeholder), + r_policy_node_keyexpr(&policy->key), compressed_pubkey)) { return -1; } else if (policy->base.type == TOKEN_PKH) { @@ -811,11 +809,10 @@ __attribute__((warn_unused_result)) static int process_multi_sortedmulti_node( uint8_t compressed_pubkey[33]; if (policy->base.type == TOKEN_MULTI) { - if (-1 == - get_derived_pubkey(state->dispatcher_context, - state->wdi, - &r_policy_node_keyexpr(&policy->key_placeholders)[i], - compressed_pubkey)) { + if (-1 == get_derived_pubkey(state->dispatcher_context, + state->wdi, + &r_policy_node_keyexpr(&policy->keys)[i], + compressed_pubkey)) { return -1; } } else { @@ -837,11 +834,10 @@ __attribute__((warn_unused_result)) static int process_multi_sortedmulti_node( for (int j = 0; j < policy->n; j++) { if (!bitvector_get(used, j)) { uint8_t cur_pubkey[33]; - if (-1 == get_derived_pubkey( - state->dispatcher_context, - state->wdi, - &r_policy_node_keyexpr(&policy->key_placeholders)[j], - cur_pubkey)) { + if (-1 == get_derived_pubkey(state->dispatcher_context, + state->wdi, + &r_policy_node_keyexpr(&policy->keys)[j], + cur_pubkey)) { return -1; } @@ -889,11 +885,10 @@ __attribute__((warn_unused_result)) static int process_multi_a_sortedmulti_a_nod uint8_t compressed_pubkey[33]; if (policy->base.type == TOKEN_MULTI_A) { - if (-1 == - get_derived_pubkey(state->dispatcher_context, - state->wdi, - &r_policy_node_keyexpr(&policy->key_placeholders)[i], - compressed_pubkey)) { + if (-1 == get_derived_pubkey(state->dispatcher_context, + state->wdi, + &r_policy_node_keyexpr(&policy->keys)[i], + compressed_pubkey)) { return -1; } } else { @@ -905,11 +900,10 @@ __attribute__((warn_unused_result)) static int process_multi_a_sortedmulti_a_nod for (int j = 0; j < policy->n; j++) { if (!bitvector_get(used, j)) { uint8_t cur_pubkey[33]; - if (-1 == get_derived_pubkey( - state->dispatcher_context, - state->wdi, - &r_policy_node_keyexpr(&policy->key_placeholders)[j], - cur_pubkey)) { + if (-1 == get_derived_pubkey(state->dispatcher_context, + state->wdi, + &r_policy_node_keyexpr(&policy->keys)[j], + cur_pubkey)) { return -1; } @@ -1018,7 +1012,7 @@ int get_wallet_script(dispatcher_context_t *dispatcher_context, policy_node_with_key_t *pkh_policy = (policy_node_with_key_t *) policy; if (0 > get_derived_pubkey(dispatcher_context, wdi, - r_policy_node_keyexpr(&pkh_policy->key_placeholder), + r_policy_node_keyexpr(&pkh_policy->key), compressed_pubkey)) { return -1; } @@ -1037,7 +1031,7 @@ int get_wallet_script(dispatcher_context_t *dispatcher_context, policy_node_with_key_t *wpkh_policy = (policy_node_with_key_t *) policy; if (0 > get_derived_pubkey(dispatcher_context, wdi, - r_policy_node_keyexpr(&wpkh_policy->key_placeholder), + r_policy_node_keyexpr(&wpkh_policy->key), compressed_pubkey)) { return -1; } @@ -1116,7 +1110,7 @@ int get_wallet_script(dispatcher_context_t *dispatcher_context, if (0 > get_derived_pubkey(dispatcher_context, wdi, - r_policy_node_keyexpr(&tr_policy->key_placeholder), + r_policy_node_keyexpr(&tr_policy->key), compressed_pubkey)) { return -1; } @@ -1348,13 +1342,13 @@ static int get_bip44_purpose(const policy_node_t *descriptor_template) { int purpose = -1; switch (descriptor_template->type) { case TOKEN_PKH: - kp = r_policy_node_keyexpr( - &((const policy_node_with_key_t *) descriptor_template)->key_placeholder); + kp = + r_policy_node_keyexpr(&((const policy_node_with_key_t *) descriptor_template)->key); purpose = 44; // legacy break; case TOKEN_WPKH: - kp = r_policy_node_keyexpr( - &((const policy_node_with_key_t *) descriptor_template)->key_placeholder); + kp = + r_policy_node_keyexpr(&((const policy_node_with_key_t *) descriptor_template)->key); purpose = 84; // native segwit break; case TOKEN_SH: { @@ -1364,8 +1358,7 @@ static int get_bip44_purpose(const policy_node_t *descriptor_template) { return -1; } - kp = r_policy_node_keyexpr( - &((const policy_node_with_key_t *) inner)->key_placeholder); + kp = r_policy_node_keyexpr(&((const policy_node_with_key_t *) inner)->key); purpose = 49; // nested segwit break; } @@ -1375,8 +1368,7 @@ static int get_bip44_purpose(const policy_node_t *descriptor_template) { return -1; } - kp = r_policy_node_keyexpr( - &((const policy_node_tr_t *) descriptor_template)->key_placeholder); + kp = r_policy_node_keyexpr(&((const policy_node_tr_t *) descriptor_template)->key); purpose = 86; // standard single-key P2TR break; } @@ -1513,44 +1505,47 @@ bool check_wallet_hmac(const uint8_t wallet_id[static 32], const uint8_t wallet_ // make sure that the compiler gives an error if any PolicyNodeType is missed #pragma GCC diagnostic error "-Wswitch-enum" -static int get_key_placeholder_by_index_in_tree(const policy_node_tree_t *tree, - unsigned int i, - const policy_node_t **out_tapleaf_ptr, - policy_node_keyexpr_t *out_placeholder) { +static int get_keyexpr_by_index_in_tree(const policy_node_tree_t *tree, + unsigned int i, + const policy_node_t **out_tapleaf_ptr, + policy_node_keyexpr_t *out_keyexpr) { if (tree->is_leaf) { - int ret = - get_key_placeholder_by_index(r_policy_node(&tree->script), i, NULL, out_placeholder); + int ret = get_keyexpr_by_index(r_policy_node(&tree->script), i, NULL, out_keyexpr); if (ret >= 0 && out_tapleaf_ptr != NULL && i < (unsigned) ret) { *out_tapleaf_ptr = r_policy_node(&tree->script); } return ret; } else { - int ret1 = get_key_placeholder_by_index_in_tree(r_policy_node_tree(&tree->left_tree), - i, - out_tapleaf_ptr, - out_placeholder); + int ret1 = get_keyexpr_by_index_in_tree(r_policy_node_tree(&tree->left_tree), + i, + out_tapleaf_ptr, + out_keyexpr); if (ret1 < 0) return -1; bool found = i < (unsigned int) ret1; - int ret2 = get_key_placeholder_by_index_in_tree(r_policy_node_tree(&tree->right_tree), - found ? 0 : i - ret1, - found ? NULL : out_tapleaf_ptr, - found ? NULL : out_placeholder); + int ret2 = get_keyexpr_by_index_in_tree(r_policy_node_tree(&tree->right_tree), + found ? 0 : i - ret1, + found ? NULL : out_tapleaf_ptr, + found ? NULL : out_keyexpr); if (ret2 < 0) return -1; return ret1 + ret2; } } -int get_key_placeholder_by_index(const policy_node_t *policy, - unsigned int i, - const policy_node_t **out_tapleaf_ptr, - policy_node_keyexpr_t *out_placeholder) { - // make sure that out_placeholder is a valid pointer, if the output is not needed +// TODO: generalize for musig. Note that this is broken for musig, as out_keyexpr +// can't be filled in for musig key expressions (as it's dynamic and contains +// relative pointers). We should probably refactor to return the pointer to the +// key expression and removing the out_keyexpr argument. +int get_keyexpr_by_index(const policy_node_t *policy, + unsigned int i, + const policy_node_t **out_tapleaf_ptr, + policy_node_keyexpr_t *out_keyexpr) { + // make sure that out_keyexpr is a valid pointer, if the output is not needed policy_node_keyexpr_t tmp; - if (out_placeholder == NULL) { - out_placeholder = &tmp; + if (out_keyexpr == NULL) { + out_keyexpr = &tmp; } switch (policy->type) { @@ -1573,8 +1568,8 @@ int get_key_placeholder_by_index(const policy_node_t *policy, case TOKEN_WPKH: { if (i == 0) { policy_node_with_key_t *wpkh = (policy_node_with_key_t *) policy; - memcpy(out_placeholder, - r_policy_node_keyexpr(&wpkh->key_placeholder), + memcpy(out_keyexpr, + r_policy_node_keyexpr(&wpkh->key), sizeof(policy_node_keyexpr_t)); } return 1; @@ -1582,17 +1577,15 @@ int get_key_placeholder_by_index(const policy_node_t *policy, case TOKEN_TR: { policy_node_tr_t *tr = (policy_node_tr_t *) policy; if (i == 0) { - memcpy(out_placeholder, - r_policy_node_keyexpr(&tr->key_placeholder), - sizeof(policy_node_keyexpr_t)); + memcpy(out_keyexpr, r_policy_node_keyexpr(&tr->key), sizeof(policy_node_keyexpr_t)); } if (!isnull_policy_node_tree(&tr->tree)) { - int ret_tree = get_key_placeholder_by_index_in_tree( + int ret_tree = get_keyexpr_by_index_in_tree( r_policy_node_tree(&tr->tree), i == 0 ? 0 : i - 1, i == 0 ? NULL : out_tapleaf_ptr, - i == 0 ? NULL : out_placeholder); // if i == 0, we already found it; so we - // recur with out_placeholder set to NULL + i == 0 ? NULL : out_keyexpr); // if i == 0, we already found it; so we + // recur with out_keyexpr set to NULL if (ret_tree < 0) { return -1; } @@ -1610,9 +1603,8 @@ int get_key_placeholder_by_index(const policy_node_t *policy, const policy_node_multisig_t *node = (const policy_node_multisig_t *) policy; if (i < (unsigned int) node->n) { - policy_node_keyexpr_t *placeholders = - r_policy_node_keyexpr(&node->key_placeholders); - memcpy(out_placeholder, &placeholders[i], sizeof(policy_node_keyexpr_t)); + policy_node_keyexpr_t *key_expressions = r_policy_node_keyexpr(&node->keys); + memcpy(out_keyexpr, &key_expressions[i], sizeof(policy_node_keyexpr_t)); } return node->n; @@ -1631,11 +1623,11 @@ int get_key_placeholder_by_index(const policy_node_t *policy, case TOKEN_N: case TOKEN_L: case TOKEN_U: { - return get_key_placeholder_by_index( + return get_keyexpr_by_index( r_policy_node(&((const policy_node_with_script_t *) policy)->script), i, out_tapleaf_ptr, - out_placeholder); + out_keyexpr); } // nodes with exactly two child scripts @@ -1647,17 +1639,17 @@ int get_key_placeholder_by_index(const policy_node_t *policy, case TOKEN_OR_D: case TOKEN_OR_I: { const policy_node_with_script2_t *node = (const policy_node_with_script2_t *) policy; - int ret1 = get_key_placeholder_by_index(r_policy_node(&node->scripts[0]), - i, - out_tapleaf_ptr, - out_placeholder); + int ret1 = get_keyexpr_by_index(r_policy_node(&node->scripts[0]), + i, + out_tapleaf_ptr, + out_keyexpr); if (ret1 < 0) return -1; bool found = i < (unsigned int) ret1; - int ret2 = get_key_placeholder_by_index(r_policy_node(&node->scripts[1]), - found ? 0 : i - ret1, - found ? NULL : out_tapleaf_ptr, - found ? NULL : out_placeholder); + int ret2 = get_keyexpr_by_index(r_policy_node(&node->scripts[1]), + found ? 0 : i - ret1, + found ? NULL : out_tapleaf_ptr, + found ? NULL : out_keyexpr); if (ret2 < 0) return -1; return ret1 + ret2; @@ -1666,24 +1658,24 @@ int get_key_placeholder_by_index(const policy_node_t *policy, // nodes with exactly three child scripts case TOKEN_ANDOR: { const policy_node_with_script3_t *node = (const policy_node_with_script3_t *) policy; - int ret1 = get_key_placeholder_by_index(r_policy_node(&node->scripts[0]), - i, - out_tapleaf_ptr, - out_placeholder); + int ret1 = get_keyexpr_by_index(r_policy_node(&node->scripts[0]), + i, + out_tapleaf_ptr, + out_keyexpr); if (ret1 < 0) return -1; bool found = i < (unsigned int) ret1; - int ret2 = get_key_placeholder_by_index(r_policy_node(&node->scripts[1]), - found ? 0 : i - ret1, - found ? NULL : out_tapleaf_ptr, - found ? NULL : out_placeholder); + int ret2 = get_keyexpr_by_index(r_policy_node(&node->scripts[1]), + found ? 0 : i - ret1, + found ? NULL : out_tapleaf_ptr, + found ? NULL : out_keyexpr); if (ret2 < 0) return -1; found = i < (unsigned int) (ret1 + ret2); - int ret3 = get_key_placeholder_by_index(r_policy_node(&node->scripts[2]), - found ? 0 : i - ret1 - ret2, - found ? NULL : out_tapleaf_ptr, - found ? NULL : out_placeholder); + int ret3 = get_keyexpr_by_index(r_policy_node(&node->scripts[2]), + found ? 0 : i - ret1 - ret2, + found ? NULL : out_tapleaf_ptr, + found ? NULL : out_keyexpr); if (ret3 < 0) return -1; return ret1 + ret2 + ret3; } @@ -1699,10 +1691,10 @@ int get_key_placeholder_by_index(const policy_node_t *policy, "The script should always have exactly n child scripts"); found = i < (unsigned int) ret; - int ret_partial = get_key_placeholder_by_index(r_policy_node(&cur_child->script), - found ? 0 : i - ret, - found ? NULL : out_tapleaf_ptr, - found ? NULL : out_placeholder); + int ret_partial = get_keyexpr_by_index(r_policy_node(&cur_child->script), + found ? 0 : i - ret, + found ? NULL : out_tapleaf_ptr, + found ? NULL : out_keyexpr); if (ret_partial < 0) return -1; ret += ret_partial; @@ -1723,16 +1715,16 @@ int get_key_placeholder_by_index(const policy_node_t *policy, } int count_distinct_keys_info(const policy_node_t *policy) { - policy_node_keyexpr_t placeholder; - int ret = -1, cur, n_placeholders; + policy_node_keyexpr_t key_expression; + int ret = -1, cur, n_key_expressions; for (cur = 0; - cur < (n_placeholders = get_key_placeholder_by_index(policy, cur, NULL, &placeholder)); + cur < (n_key_expressions = get_keyexpr_by_index(policy, cur, NULL, &key_expression)); ++cur) { - if (n_placeholders < 0) { + if (n_key_expressions < 0) { return -1; } - ret = MAX(ret, placeholder.key_index + 1); + ret = MAX(ret, key_expression.key_index + 1); } return ret; } @@ -1849,6 +1841,7 @@ static int is_taptree_miniscript_sane(const policy_node_tree_t *taptree) { return 0; } +// TODO: generalize for musig. int is_policy_sane(dispatcher_context_t *dispatcher_context, const policy_node_t *policy, int wallet_version, @@ -1906,35 +1899,36 @@ int is_policy_sane(dispatcher_context_t *dispatcher_context, } } - // check that all the key placeholders for the same xpub do indeed have different + // check that all the key expressions for the same xpub do indeed have different // derivations - int n_placeholders = get_key_placeholder_by_index(policy, 0, NULL, NULL); - if (n_placeholders < 0) { - return WITH_ERROR(-1, "Unexpected error while counting placeholders"); + int n_key_expressions = get_keyexpr_by_index(policy, 0, NULL, NULL); + if (n_key_expressions < 0) { + return WITH_ERROR(-1, "Unexpected error while counting key expressions"); } // The following loop computationally very inefficient (quadratic in the number of - // placeholders), but more efficient solutions likely require a substantial amount of RAM - // (proportional to the number of key placeholders). Instead, this only requires stack depth + // key expressions), but more efficient solutions likely require a substantial amount of RAM + // (proportional to the number of key expressions). Instead, this only requires stack depth // proportional to the depth of the wallet policy's abstract syntax tree. - for (int i = 0; i < n_placeholders - 1; - i++) { // no point in running this for the last placeholder + for (int i = 0; i < n_key_expressions - 1; + i++) { // no point in running this for the last key expression policy_node_keyexpr_t kp_i; - if (0 > get_key_placeholder_by_index(policy, i, NULL, &kp_i)) { - return WITH_ERROR(-1, "Unexpected error retrieving placeholders from the policy"); + if (0 > get_keyexpr_by_index(policy, i, NULL, &kp_i)) { + return WITH_ERROR(-1, "Unexpected error retrieving key expressions from the policy"); } - for (int j = i + 1; j < n_placeholders; j++) { + for (int j = i + 1; j < n_key_expressions; j++) { policy_node_keyexpr_t kp_j; - if (0 > get_key_placeholder_by_index(policy, j, NULL, &kp_j)) { - return WITH_ERROR(-1, "Unexpected error retrieving placeholders from the policy"); + if (0 > get_keyexpr_by_index(policy, j, NULL, &kp_j)) { + return WITH_ERROR(-1, + "Unexpected error retrieving key expressions from the policy"); } - // placeholders for the same key must have disjoint derivation options + // key expressions for the same key must have disjoint derivation options if (kp_i.key_index == kp_j.key_index) { if (kp_i.num_first == kp_j.num_first || kp_i.num_first == kp_j.num_second || kp_i.num_second == kp_j.num_first || kp_i.num_second == kp_j.num_second) { return WITH_ERROR(-1, - "Key placeholders with repeated derivations in miniscript"); + "Key expressions with repeated derivations in miniscript"); } } } diff --git a/src/handler/lib/policy.h b/src/handler/lib/policy.h index c17f4c5cc..cfe6a5076 100644 --- a/src/handler/lib/policy.h +++ b/src/handler/lib/policy.h @@ -176,31 +176,30 @@ bool compute_wallet_hmac(const uint8_t wallet_id[static 32], uint8_t wallet_hmac bool check_wallet_hmac(const uint8_t wallet_id[static 32], const uint8_t wallet_hmac[static 32]); /** - * Copies the i-th placeholder (indexing from 0) of the given policy into `out_placeholder` (if not + * Copies the i-th key expression (indexing from 0) of the given policy into `out_keyexpr` (if not * null). * * @param[in] policy * Pointer to the root node of the policy * @param[in] i - * Index of the wanted placeholder. Ignored if out_placeholder is NULL. + * Index of the wanted key expression. Ignored if out_keyexpr is NULL. * @param[out] out_tapleaf_ptr - * If not NULL, and if the i-th placeholder is in a tapleaf of the policy, receives the pointer to - * the tapleaf's script. - * @param[out] out_placeholder - * If not NULL, it is a pointer that will receive the i-th placeholder of the policy. - * @return the number of placeholders in the policy on success; -1 in case of error. + * If not NULL, and if the i-th key expression is in a tapleaf of the policy, receives the pointer + * to the tapleaf's script. + * @param[out] out_keyexpr + * If not NULL, it is a pointer that will receive the i-th key expression of the policy. + * @return the number of key expressions in the policy on success; -1 in case of error. */ -__attribute__((warn_unused_result)) int get_key_placeholder_by_index( - const policy_node_t *policy, - unsigned int i, - const policy_node_t **out_tapleaf_ptr, - policy_node_keyexpr_t *out_placeholder); +__attribute__((warn_unused_result)) int get_keyexpr_by_index(const policy_node_t *policy, + unsigned int i, + const policy_node_t **out_tapleaf_ptr, + policy_node_keyexpr_t *out_keyexpr); /** * Determines the expected number of unique keys in the provided policy's key information. - * The function calculates this by finding the maximum key index from placeholders and increments it - * by 1. For instance, if the maximum key index found in the placeholders is `n`, then the result - * would be `n + 1`. + * The function calculates this by finding the maximum key index from key expressions and increments + * it by 1. For instance, if the maximum key index found in the key expressions is `n`, then the + * result would be `n + 1`. * * @param[in] policy * Pointer to the root node of the policy diff --git a/src/handler/sign_psbt.c b/src/handler/sign_psbt.c index 7af754109..0c825d43d 100644 --- a/src/handler/sign_psbt.c +++ b/src/handler/sign_psbt.c @@ -63,7 +63,8 @@ typedef struct { // PSBT_{IN,OUT}_BIP32_DERIVATION or // PSBT_{IN,OUT}_TAP_BIP32_DERIVATION is not the correct length. - bool placeholder_found; // Set to true if a matching placeholder is found in the input info + bool key_expression_found; // Set to true if the input/output info in the psbt was correctly + // matched with the current key expression in the signing flow bool is_change; int address_index; @@ -104,7 +105,7 @@ typedef struct { } output_info_t; typedef struct { - policy_node_keyexpr_t placeholder; + policy_node_keyexpr_t key_expression; int cur_index; uint32_t fingerprint; uint8_t key_derivation_length; @@ -112,7 +113,7 @@ typedef struct { serialized_extended_pubkey_t pubkey; bool is_tapscript; // true if signing with a BIP342 tapleaf script path spend uint8_t tapleaf_hash[32]; // only used for tapscripts -} placeholder_info_t; +} keyexpr_info_t; // Cache for partial hashes during segwit signing (avoid quadratic hashing for segwit transactions) typedef struct { @@ -380,9 +381,10 @@ static int get_amount_scriptpubkey_from_psbt( // Convenience function to share common logic when processing all the // PSBT_{IN|OUT}_{TAP}?_BIP32_DERIVATION fields. +// TODO: not generalized for musig2 static int read_change_and_index_from_psbt_bip32_derivation( dispatcher_context_t *dc, - placeholder_info_t *placeholder_info, + keyexpr_info_t *keyexpr_info, in_out_info_t *in_out, int psbt_key_type, buffer_t *data, @@ -424,13 +426,13 @@ static int read_change_and_index_from_psbt_bip32_derivation( return -1; } - // if this derivation path matches the internal placeholder, + // if this derivation path matches the key expression, // we use it to detect whether the current input is change or not, // and store its address index - if (fpt_der[0] == placeholder_info->fingerprint && - der_len == placeholder_info->key_derivation_length + 2) { - for (int i = 0; i < placeholder_info->key_derivation_length; i++) { - if (placeholder_info->key_derivation[i] != fpt_der[1 + i]) { + if (fpt_der[0] == keyexpr_info->fingerprint && + der_len == keyexpr_info->key_derivation_length + 2) { + for (int i = 0; i < keyexpr_info->key_derivation_length; i++) { + if (keyexpr_info->key_derivation[i] != fpt_der[1 + i]) { return 0; } } @@ -438,9 +440,9 @@ static int read_change_and_index_from_psbt_bip32_derivation( uint32_t change = fpt_der[1 + der_len - 2]; uint32_t addr_index = fpt_der[1 + der_len - 1]; - // check that we can indeed derive the same key from the current placeholder + // check that we can indeed derive the same key from the current key expression serialized_extended_pubkey_t pubkey; - if (0 > bip32_CKDpub(&placeholder_info->pubkey, change, &pubkey)) return -1; + if (0 > bip32_CKDpub(&keyexpr_info->pubkey, change, &pubkey)) return -1; if (0 > bip32_CKDpub(&pubkey, addr_index, &pubkey)) return -1; int pk_offset = is_tap ? 1 : 0; @@ -448,18 +450,18 @@ static int read_change_and_index_from_psbt_bip32_derivation( return 0; } - // check if the 'change' derivation step is indeed coherent with placeholder - if (change == placeholder_info->placeholder.num_first) { + // check if the 'change' derivation step is indeed coherent with the key expression + if (change == keyexpr_info->key_expression.num_first) { in_out->is_change = false; in_out->address_index = addr_index; - } else if (change == placeholder_info->placeholder.num_second) { + } else if (change == keyexpr_info->key_expression.num_second) { in_out->is_change = true; in_out->address_index = addr_index; } else { return 0; } - in_out->placeholder_found = true; + in_out->key_expression_found = true; return 1; } return 0; @@ -476,9 +478,9 @@ static int is_in_out_internal(dispatcher_context_t *dispatcher_context, const sign_psbt_state_t *state, const in_out_info_t *in_out_info, bool is_input) { - // If we did not find any info about the pubkey associated to the placeholder we're considering, - // then it's external - if (!in_out_info->placeholder_found) { + // If we did not find any info about the pubkey associated to the key expression we're + // considering, then it's external + if (!in_out_info->key_expression_found) { return 0; } @@ -702,17 +704,16 @@ init_global_state(dispatcher_context_t *dc, sign_psbt_state_t *st) { return true; } -static bool __attribute__((noinline)) -fill_placeholder_info_if_internal(dispatcher_context_t *dc, - sign_psbt_state_t *st, - placeholder_info_t *placeholder_info) { +static bool __attribute__((noinline)) fill_keyexpr_info_if_internal(dispatcher_context_t *dc, + sign_psbt_state_t *st, + keyexpr_info_t *keyexpr_info) { policy_map_key_info_t key_info; { uint8_t key_info_str[MAX_POLICY_KEY_INFO_LEN]; int key_info_len = call_get_merkle_leaf_element(dc, st->wallet_header_keys_info_merkle_root, st->wallet_header_n_keys, - placeholder_info->placeholder.key_index, + keyexpr_info->key_expression.key_index, key_info_str, sizeof(key_info_str)); @@ -742,56 +743,55 @@ fill_placeholder_info_if_internal(dispatcher_context_t *dc, if (0 > get_extended_pubkey_at_path(key_info.master_key_derivation, key_info.master_key_derivation_len, BIP32_PUBKEY_VERSION, - &placeholder_info->pubkey)) { + &keyexpr_info->pubkey)) { SEND_SW(dc, SW_BAD_STATE); return false; } - if (memcmp(&key_info.ext_pubkey, - &placeholder_info->pubkey, - sizeof(placeholder_info->pubkey)) != 0) { + if (memcmp(&key_info.ext_pubkey, &keyexpr_info->pubkey, sizeof(keyexpr_info->pubkey)) != + 0) { return false; } - placeholder_info->key_derivation_length = key_info.master_key_derivation_len; + keyexpr_info->key_derivation_length = key_info.master_key_derivation_len; for (int i = 0; i < key_info.master_key_derivation_len; i++) { - placeholder_info->key_derivation[i] = key_info.master_key_derivation[i]; + keyexpr_info->key_derivation[i] = key_info.master_key_derivation[i]; } - placeholder_info->fingerprint = read_u32_be(key_info.master_key_fingerprint, 0); + keyexpr_info->fingerprint = read_u32_be(key_info.master_key_fingerprint, 0); } return true; } -// finds the first placeholder that corresponds to an internal key -static bool find_first_internal_key_placeholder(dispatcher_context_t *dc, - sign_psbt_state_t *st, - placeholder_info_t *placeholder_info) { - placeholder_info->cur_index = 0; +// finds the first key expression that corresponds to an internal key +static bool find_first_internal_keyexpr(dispatcher_context_t *dc, + sign_psbt_state_t *st, + keyexpr_info_t *keyexpr_info) { + keyexpr_info->cur_index = 0; // find and parse our registered key info in the wallet while (true) { - int n_key_placeholders = get_key_placeholder_by_index(st->wallet_policy_map, - placeholder_info->cur_index, - NULL, - &placeholder_info->placeholder); - if (n_key_placeholders < 0) { + int n_key_expressions = get_keyexpr_by_index(st->wallet_policy_map, + keyexpr_info->cur_index, + NULL, + &keyexpr_info->key_expression); + if (n_key_expressions < 0) { SEND_SW(dc, SW_BAD_STATE); // should never happen return false; } - if (placeholder_info->cur_index >= n_key_placeholders) { + if (keyexpr_info->cur_index >= n_key_expressions) { // all keys have been processed break; } - if (fill_placeholder_info_if_internal(dc, st, placeholder_info)) { + if (fill_keyexpr_info_if_internal(dc, st, keyexpr_info)) { return true; } // Not an internal key, move on - ++placeholder_info->cur_index; + ++keyexpr_info->cur_index; } PRINTF("No internal key found in wallet policy"); @@ -800,7 +800,7 @@ static bool find_first_internal_key_placeholder(dispatcher_context_t *dc, } typedef struct { - placeholder_info_t *placeholder_info; + keyexpr_info_t *keyexpr_info; input_info_t *input; } input_keys_callback_data_t; @@ -827,15 +827,14 @@ static void input_keys_callback(dispatcher_context_t *dc, callback_data->input->has_sighash_type = true; } else if ((key_type == PSBT_IN_BIP32_DERIVATION || key_type == PSBT_IN_TAP_BIP32_DERIVATION) && - !callback_data->input->in_out.placeholder_found) { - if (0 > - read_change_and_index_from_psbt_bip32_derivation(dc, - callback_data->placeholder_info, - &callback_data->input->in_out, - key_type, - data, - map_commitment, - i)) { + !callback_data->input->in_out.key_expression_found) { + if (0 > read_change_and_index_from_psbt_bip32_derivation(dc, + callback_data->keyexpr_info, + &callback_data->input->in_out, + key_type, + data, + map_commitment, + i)) { callback_data->input->in_out.unexpected_pubkey_error = true; } } @@ -850,18 +849,17 @@ preprocess_inputs(dispatcher_context_t *dc, memset(internal_inputs, 0, BITVECTOR_REAL_SIZE(MAX_N_INPUTS_CAN_SIGN)); - placeholder_info_t placeholder_info; - memset(&placeholder_info, 0, sizeof(placeholder_info)); + keyexpr_info_t keyexpr_info; + memset(&keyexpr_info, 0, sizeof(keyexpr_info)); - if (!find_first_internal_key_placeholder(dc, st, &placeholder_info)) return false; + if (!find_first_internal_keyexpr(dc, st, &keyexpr_info)) return false; // process each input for (unsigned int cur_input_index = 0; cur_input_index < st->n_inputs; cur_input_index++) { input_info_t input; memset(&input, 0, sizeof(input)); - input_keys_callback_data_t callback_data = {.input = &input, - .placeholder_info = &placeholder_info}; + input_keys_callback_data_t callback_data = {.input = &input, .keyexpr_info = &keyexpr_info}; int res = call_get_merkleized_map_with_callback( dc, (void *) &callback_data, @@ -1099,7 +1097,7 @@ show_alerts(dispatcher_context_t *dc, } typedef struct { - placeholder_info_t *placeholder_info; + keyexpr_info_t *keyexpr_info; output_info_t *output; } output_keys_callback_data_t; @@ -1118,15 +1116,14 @@ static void output_keys_callback(dispatcher_context_t *dc, buffer_read_u8(data, &key_type); if ((key_type == PSBT_OUT_BIP32_DERIVATION || key_type == PSBT_OUT_TAP_BIP32_DERIVATION) && - !callback_data->output->in_out.placeholder_found) { - if (0 > - read_change_and_index_from_psbt_bip32_derivation(dc, - callback_data->placeholder_info, - &callback_data->output->in_out, - key_type, - data, - map_commitment, - i)) { + !callback_data->output->in_out.key_expression_found) { + if (0 > read_change_and_index_from_psbt_bip32_derivation(dc, + callback_data->keyexpr_info, + &callback_data->output->in_out, + key_type, + data, + map_commitment, + i)) { callback_data->output->in_out.unexpected_pubkey_error = true; } } @@ -1193,7 +1190,7 @@ static bool __attribute__((noinline)) display_output(dispatcher_context_t *dc, static bool read_outputs(dispatcher_context_t *dc, sign_psbt_state_t *st, - placeholder_info_t *placeholder_info, + keyexpr_info_t *keyexpr_info, bool dry_run) { // the counter used when showing outputs to the user, which ignores change outputs // (0-indexed here, although the UX starts with 1) @@ -1204,7 +1201,7 @@ static bool read_outputs(dispatcher_context_t *dc, memset(&output, 0, sizeof(output)); output_keys_callback_data_t callback_data = {.output = &output, - .placeholder_info = placeholder_info}; + .keyexpr_info = keyexpr_info}; int res = call_get_merkleized_map_with_callback( dc, (void *) &callback_data, @@ -1297,10 +1294,10 @@ process_outputs(dispatcher_context_t *dc, sign_psbt_state_t *st) { LOG_PROCESSOR(__FILE__, __LINE__, __func__); - placeholder_info_t placeholder_info; - memset(&placeholder_info, 0, sizeof(placeholder_info)); + keyexpr_info_t keyexpr_info; + memset(&keyexpr_info, 0, sizeof(keyexpr_info)); - if (!find_first_internal_key_placeholder(dc, st, &placeholder_info)) return false; + if (!find_first_internal_keyexpr(dc, st, &keyexpr_info)) return false; memset(&st->outputs, 0, sizeof(st->outputs)); @@ -1310,7 +1307,7 @@ process_outputs(dispatcher_context_t *dc, sign_psbt_state_t *st) { // As it's a time-consuming operation, we use avoid doing this useless // work on other models. - if (!read_outputs(dc, st, &placeholder_info, true)) return false; + if (!read_outputs(dc, st, &keyexpr_info, true)) return false; if (!G_swap_state.called_from_swap && !ui_transaction_prompt(dc, st->outputs.n_external)) { SEND_SW(dc, SW_DENY); @@ -1318,7 +1315,7 @@ process_outputs(dispatcher_context_t *dc, sign_psbt_state_t *st) { } #endif - if (!read_outputs(dc, st, &placeholder_info, false)) return false; + if (!read_outputs(dc, st, &keyexpr_info, false)) return false; return true; } @@ -1702,7 +1699,7 @@ static bool __attribute__((noinline)) compute_sighash_segwitv1(dispatcher_contex segwit_hashes_t *hashes, input_info_t *input, unsigned int cur_input_index, - placeholder_info_t *placeholder_info, + keyexpr_info_t *keyexpr_info, uint8_t sighash[static 32]) { LOG_PROCESSOR(__FILE__, __LINE__, __func__); @@ -1737,7 +1734,7 @@ static bool __attribute__((noinline)) compute_sighash_segwitv1(dispatcher_contex } // ext_flag - uint8_t ext_flag = placeholder_info->is_tapscript ? 1 : 0; + uint8_t ext_flag = keyexpr_info->is_tapscript ? 1 : 0; // annex is not supported const uint8_t annex_present = 0; uint8_t spend_type = ext_flag * 2 + annex_present; @@ -1821,9 +1818,9 @@ static bool __attribute__((noinline)) compute_sighash_segwitv1(dispatcher_contex crypto_hash_update(&sighash_context.header, tmp, 32); } - if (placeholder_info->is_tapscript) { + if (keyexpr_info->is_tapscript) { // If spending a tapscript, append the Common Signature Message Extension per BIP-0342 - crypto_hash_update(&sighash_context.header, placeholder_info->tapleaf_hash, 32); + crypto_hash_update(&sighash_context.header, keyexpr_info->tapleaf_hash, 32); crypto_hash_update_u8(&sighash_context.header, 0x00); // key_version crypto_hash_update_u32(&sighash_context.header, 0xffffffff); // no OP_CODESEPARATOR } @@ -1875,25 +1872,24 @@ static bool __attribute__((noinline)) yield_signature(dispatcher_context_t *dc, return true; } -static bool __attribute__((noinline)) -sign_sighash_ecdsa_and_yield(dispatcher_context_t *dc, - sign_psbt_state_t *st, - placeholder_info_t *placeholder_info, - input_info_t *input, - unsigned int cur_input_index, - uint8_t sighash[static 32]) { +static bool __attribute__((noinline)) sign_sighash_ecdsa_and_yield(dispatcher_context_t *dc, + sign_psbt_state_t *st, + keyexpr_info_t *keyexpr_info, + input_info_t *input, + unsigned int cur_input_index, + uint8_t sighash[static 32]) { LOG_PROCESSOR(__FILE__, __LINE__, __func__); uint32_t sign_path[MAX_BIP32_PATH_STEPS]; - for (int i = 0; i < placeholder_info->key_derivation_length; i++) { - sign_path[i] = placeholder_info->key_derivation[i]; + for (int i = 0; i < keyexpr_info->key_derivation_length; i++) { + sign_path[i] = keyexpr_info->key_derivation[i]; } - sign_path[placeholder_info->key_derivation_length] = - input->in_out.is_change ? placeholder_info->placeholder.num_second - : placeholder_info->placeholder.num_first; - sign_path[placeholder_info->key_derivation_length + 1] = input->in_out.address_index; + sign_path[keyexpr_info->key_derivation_length] = input->in_out.is_change + ? keyexpr_info->key_expression.num_second + : keyexpr_info->key_expression.num_first; + sign_path[keyexpr_info->key_derivation_length + 1] = input->in_out.address_index; - int sign_path_len = placeholder_info->key_derivation_length + 2; + int sign_path_len = keyexpr_info->key_derivation_length + 2; uint8_t sig[MAX_DER_SIG_LEN + 1]; // extra byte for the appended sighash-type @@ -1920,13 +1916,12 @@ sign_sighash_ecdsa_and_yield(dispatcher_context_t *dc, return true; } -static bool __attribute__((noinline)) -sign_sighash_schnorr_and_yield(dispatcher_context_t *dc, - sign_psbt_state_t *st, - placeholder_info_t *placeholder_info, - input_info_t *input, - unsigned int cur_input_index, - uint8_t sighash[static 32]) { +static bool __attribute__((noinline)) sign_sighash_schnorr_and_yield(dispatcher_context_t *dc, + sign_psbt_state_t *st, + keyexpr_info_t *keyexpr_info, + input_info_t *input, + unsigned int cur_input_index, + uint8_t sighash[static 32]) { LOG_PROCESSOR(__FILE__, __LINE__, __func__); if (st->wallet_policy_map->type != TOKEN_TR) { @@ -1954,15 +1949,15 @@ sign_sighash_schnorr_and_yield(dispatcher_context_t *dc, uint32_t sign_path[MAX_BIP32_PATH_STEPS]; - for (int i = 0; i < placeholder_info->key_derivation_length; i++) { - sign_path[i] = placeholder_info->key_derivation[i]; + for (int i = 0; i < keyexpr_info->key_derivation_length; i++) { + sign_path[i] = keyexpr_info->key_derivation[i]; } - sign_path[placeholder_info->key_derivation_length] = - input->in_out.is_change ? placeholder_info->placeholder.num_second - : placeholder_info->placeholder.num_first; - sign_path[placeholder_info->key_derivation_length + 1] = input->in_out.address_index; + sign_path[keyexpr_info->key_derivation_length] = + input->in_out.is_change ? keyexpr_info->key_expression.num_second + : keyexpr_info->key_expression.num_first; + sign_path[keyexpr_info->key_derivation_length + 1] = input->in_out.address_index; - int sign_path_len = placeholder_info->key_derivation_length + 2; + int sign_path_len = keyexpr_info->key_derivation_length + 2; if (bip32_derive_init_privkey_256(CX_CURVE_256K1, sign_path, @@ -1975,7 +1970,7 @@ sign_sighash_schnorr_and_yield(dispatcher_context_t *dc, policy_node_tr_t *policy = (policy_node_tr_t *) st->wallet_policy_map; - if (!placeholder_info->is_tapscript) { + if (!keyexpr_info->is_tapscript) { if (isnull_policy_node_tree(&policy->tree)) { // tweak as specified in BIP-86 and BIP-386 crypto_tr_tweak_seckey(seckey, (uint8_t[]){}, 0, seckey); @@ -1987,7 +1982,7 @@ sign_sighash_schnorr_and_yield(dispatcher_context_t *dc, } } else { // tapscript, we need to yield the tapleaf hash together with the pubkey - tapleaf_hash = placeholder_info->tapleaf_hash; + tapleaf_hash = keyexpr_info->tapleaf_hash; } // generate corresponding public key @@ -2174,7 +2169,7 @@ compute_segwit_hashes(dispatcher_context_t *dc, sign_psbt_state_t *st, segwit_ha static bool __attribute__((noinline)) sign_transaction_input(dispatcher_context_t *dc, sign_psbt_state_t *st, segwit_hashes_t *hashes, - placeholder_info_t *placeholder_info, + keyexpr_info_t *keyexpr_info, input_info_t *input, unsigned int cur_input_index) { LOG_PROCESSOR(__FILE__, __LINE__, __func__); @@ -2220,12 +2215,7 @@ static bool __attribute__((noinline)) sign_transaction_input(dispatcher_context_ uint8_t sighash[32]; if (!compute_sighash_legacy(dc, st, input, cur_input_index, sighash)) return false; - if (!sign_sighash_ecdsa_and_yield(dc, - st, - placeholder_info, - input, - cur_input_index, - sighash)) + if (!sign_sighash_ecdsa_and_yield(dc, st, keyexpr_info, input, cur_input_index, sighash)) return false; } else { { @@ -2292,7 +2282,7 @@ static bool __attribute__((noinline)) sign_transaction_input(dispatcher_context_ if (!sign_sighash_ecdsa_and_yield(dc, st, - placeholder_info, + keyexpr_info, input, cur_input_index, sighash)) @@ -2308,12 +2298,12 @@ static bool __attribute__((noinline)) sign_transaction_input(dispatcher_context_ hashes, input, cur_input_index, - placeholder_info, + keyexpr_info, sighash)) return false; policy_node_tr_t *policy = (policy_node_tr_t *) st->wallet_policy_map; - if (!placeholder_info->is_tapscript && !isnull_policy_node_tree(&policy->tree)) { + if (!keyexpr_info->is_tapscript && !isnull_policy_node_tree(&policy->tree)) { // keypath spend, we compute the taptree hash so that we find it ready // later in sign_sighash_schnorr_and_yield (which has less available stack). if (0 > compute_taptree_hash( @@ -2334,7 +2324,7 @@ static bool __attribute__((noinline)) sign_transaction_input(dispatcher_context_ if (!sign_sighash_schnorr_and_yield(dc, st, - placeholder_info, + keyexpr_info, input, cur_input_index, sighash)) @@ -2348,14 +2338,13 @@ static bool __attribute__((noinline)) sign_transaction_input(dispatcher_context_ return true; } -static bool __attribute__((noinline)) -fill_taproot_placeholder_info(dispatcher_context_t *dc, - sign_psbt_state_t *st, - const input_info_t *input, - const policy_node_t *tapleaf_ptr, - placeholder_info_t *placeholder_info) { - uint32_t change = input->in_out.is_change ? placeholder_info->placeholder.num_second - : placeholder_info->placeholder.num_first; +static bool __attribute__((noinline)) fill_taproot_keyexpr_info(dispatcher_context_t *dc, + sign_psbt_state_t *st, + const input_info_t *input, + const policy_node_t *tapleaf_ptr, + keyexpr_info_t *keyexpr_info) { + uint32_t change = input->in_out.is_change ? keyexpr_info->key_expression.num_second + : keyexpr_info->key_expression.num_first; uint32_t address_index = input->in_out.address_index; cx_sha256_t hash_context; @@ -2395,7 +2384,7 @@ fill_taproot_placeholder_info(dispatcher_context_t *dc, &hash_context.header)) { return false; // should never happen! } - crypto_hash_digest(&hash_context.header, placeholder_info->tapleaf_hash, 32); + crypto_hash_digest(&hash_context.header, keyexpr_info->tapleaf_hash, 32); return true; } @@ -2406,7 +2395,7 @@ 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__); - int placeholder_index = 0; + int key_expression_index = 0; segwit_hashes_t hashes; @@ -2415,18 +2404,18 @@ sign_transaction(dispatcher_context_t *dc, // avoid doing it in places that have more stack limitations if (!compute_segwit_hashes(dc, st, &hashes)) return false; - // Iterate over all the placeholders that correspond to keys owned by us + // Iterate over all the key expressions that contain keys owned by us while (true) { - placeholder_info_t placeholder_info; - memset(&placeholder_info, 0, sizeof(placeholder_info)); + keyexpr_info_t keyexpr_info; + memset(&keyexpr_info, 0, sizeof(keyexpr_info)); const policy_node_t *tapleaf_ptr = NULL; - int n_key_placeholders = get_key_placeholder_by_index(st->wallet_policy_map, - placeholder_index, - &tapleaf_ptr, - &placeholder_info.placeholder); + int n_key_expressions = get_keyexpr_by_index(st->wallet_policy_map, + key_expression_index, + &tapleaf_ptr, + &keyexpr_info.key_expression); - if (n_key_placeholders < 0) { + if (n_key_expressions < 0) { SEND_SW(dc, SW_BAD_STATE); // should never happen if (!G_swap_state.called_from_swap) { ui_post_processing_confirm_transaction(dc, false); @@ -2434,26 +2423,25 @@ sign_transaction(dispatcher_context_t *dc, return false; } - if (placeholder_index >= n_key_placeholders) { - // all placeholders were processed + if (key_expression_index >= n_key_expressions) { + // all key expressions were processed break; } if (tapleaf_ptr != NULL) { - // get_key_placeholder_by_index returns the pointer to the tapleaf only if the key being + // get_keyexpr_by_index returns the pointer to the tapleaf only if the key being // spent is indeed in a tapleaf - placeholder_info.is_tapscript = true; + keyexpr_info.is_tapscript = true; } - if (fill_placeholder_info_if_internal(dc, st, &placeholder_info) == true) { + if (fill_keyexpr_info_if_internal(dc, st, &keyexpr_info) == 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)); - input_keys_callback_data_t callback_data = { - .input = &input, - .placeholder_info = &placeholder_info}; + input_keys_callback_data_t callback_data = {.input = &input, + .keyexpr_info = &keyexpr_info}; int res = call_get_merkleized_map_with_callback( dc, (void *) &callback_data, @@ -2470,14 +2458,11 @@ sign_transaction(dispatcher_context_t *dc, return false; } - if (tapleaf_ptr != NULL && !fill_taproot_placeholder_info(dc, - st, - &input, - tapleaf_ptr, - &placeholder_info)) + if (tapleaf_ptr != NULL && + !fill_taproot_keyexpr_info(dc, st, &input, tapleaf_ptr, &keyexpr_info)) return false; - if (!sign_transaction_input(dc, st, &hashes, &placeholder_info, &input, i)) { + if (!sign_transaction_input(dc, st, &hashes, &keyexpr_info, &input, i)) { if (!G_swap_state.called_from_swap) { ui_post_processing_confirm_transaction(dc, false); } @@ -2489,7 +2474,7 @@ sign_transaction(dispatcher_context_t *dc, } } - ++placeholder_index; + ++key_expression_index; } if (!G_swap_state.called_from_swap) { @@ -2547,7 +2532,7 @@ void handler_sign_psbt(dispatcher_context_t *dc, uint8_t protocol_version) { /** SIGNING FLOW * - * For each internal placeholder, and for each internal input, sign using the + * For each internal key expression, and for each internal input, sign using the * appropriate algorithm. */ if (!sign_transaction(dc, &st, internal_inputs)) return; diff --git a/unit-tests/test_wallet.c b/unit-tests/test_wallet.c index ccc2ead6a..d48d5d5dd 100644 --- a/unit-tests/test_wallet.c +++ b/unit-tests/test_wallet.c @@ -72,7 +72,7 @@ static void test_parse_policy_map_singlesig_1(void **state) { policy_node_with_key_t *node_1 = (policy_node_with_key_t *) out; assert_int_equal(node_1->base.type, TOKEN_PKH); - check_key_expr_plain(r_policy_node_keyexpr(&node_1->key_placeholder), 0, 0, 1); + check_key_expr_plain(r_policy_node_keyexpr(&node_1->key), 0, 0, 1); } static void test_parse_policy_map_singlesig_2(void **state) { @@ -90,7 +90,7 @@ static void test_parse_policy_map_singlesig_2(void **state) { policy_node_with_key_t *inner = (policy_node_with_key_t *) r_policy_node(&root->script); assert_int_equal(inner->base.type, TOKEN_WPKH); - check_key_expr_plain(r_policy_node_keyexpr(&inner->key_placeholder), 0, 0, 1); + check_key_expr_plain(r_policy_node_keyexpr(&inner->key), 0, 0, 1); } static void test_parse_policy_map_singlesig_3(void **state) { @@ -112,7 +112,7 @@ static void test_parse_policy_map_singlesig_3(void **state) { policy_node_with_key_t *inner = (policy_node_with_key_t *) r_policy_node(&mid->script); assert_int_equal(inner->base.type, TOKEN_PKH); - check_key_expr_plain(r_policy_node_keyexpr(&inner->key_placeholder), 0, 0, 1); + check_key_expr_plain(r_policy_node_keyexpr(&inner->key), 0, 0, 1); } static void test_parse_policy_map_multisig_1(void **state) { @@ -128,9 +128,9 @@ static void test_parse_policy_map_multisig_1(void **state) { assert_int_equal(node_1->base.type, TOKEN_SORTEDMULTI); assert_int_equal(node_1->k, 2); assert_int_equal(node_1->n, 3); - check_key_expr_plain(&r_policy_node_keyexpr(&node_1->key_placeholders)[0], 0, 0, 1); - check_key_expr_plain(&r_policy_node_keyexpr(&node_1->key_placeholders)[1], 1, 0, 1); - check_key_expr_plain(&r_policy_node_keyexpr(&node_1->key_placeholders)[2], 2, 0, 1); + check_key_expr_plain(&r_policy_node_keyexpr(&node_1->keys)[0], 0, 0, 1); + check_key_expr_plain(&r_policy_node_keyexpr(&node_1->keys)[1], 1, 0, 1); + check_key_expr_plain(&r_policy_node_keyexpr(&node_1->keys)[2], 2, 0, 1); } static void test_parse_policy_map_multisig_2(void **state) { @@ -151,7 +151,7 @@ static void test_parse_policy_map_multisig_2(void **state) { assert_int_equal(inner->k, 3); assert_int_equal(inner->n, 5); for (int i = 0; i < 5; i++) { - check_key_expr_plain(&r_policy_node_keyexpr(&inner->key_placeholders)[i], i, 0, 1); + check_key_expr_plain(&r_policy_node_keyexpr(&inner->keys)[i], i, 0, 1); } } @@ -177,7 +177,7 @@ static void test_parse_policy_map_multisig_3(void **state) { assert_int_equal(inner->k, 3); assert_int_equal(inner->n, 5); for (int i = 0; i < 5; i++) { - check_key_expr_plain(&r_policy_node_keyexpr(&inner->key_placeholders)[i], i, 0, 1); + check_key_expr_plain(&r_policy_node_keyexpr(&inner->keys)[i], i, 0, 1); } } @@ -194,7 +194,7 @@ static void test_parse_policy_tr(void **state) { policy_node_tr_t *root = (policy_node_tr_t *) out; assert_true(isnull_policy_node_tree(&root->tree)); - check_key_expr_plain(r_policy_node_keyexpr(&root->key_placeholder), 0, 0, 1); + check_key_expr_plain(r_policy_node_keyexpr(&root->key), 0, 0, 1); // Simple tr with a TREE that is a simple script res = parse_policy("tr(@0/**,pk(@1/**))", out, sizeof(out)); @@ -202,7 +202,7 @@ static void test_parse_policy_tr(void **state) { assert_true(res >= 0); root = (policy_node_tr_t *) out; - check_key_expr_plain(r_policy_node_keyexpr(&root->key_placeholder), 0, 0, 1); + check_key_expr_plain(r_policy_node_keyexpr(&root->key), 0, 0, 1); assert_int_equal(r_policy_node_tree(&root->tree)->is_leaf, true); @@ -210,7 +210,7 @@ static void test_parse_policy_tr(void **state) { (policy_node_with_key_t *) r_policy_node(&r_policy_node_tree(&root->tree)->script); assert_int_equal(tapscript->base.type, TOKEN_PK); - check_key_expr_plain(r_policy_node_keyexpr(&tapscript->key_placeholder), 1, 0, 1); + check_key_expr_plain(r_policy_node_keyexpr(&tapscript->key), 1, 0, 1); // Simple tr with a TREE with two tapleaves res = parse_policy("tr(@0/**,{pk(@1/**),pk(@2/<5;7>/*)})", out, sizeof(out)); @@ -218,7 +218,7 @@ static void test_parse_policy_tr(void **state) { assert_true(res >= 0); root = (policy_node_tr_t *) out; - check_key_expr_plain(r_policy_node_keyexpr(&root->key_placeholder), 0, 0, 1); + check_key_expr_plain(r_policy_node_keyexpr(&root->key), 0, 0, 1); policy_node_tree_t *taptree = r_policy_node_tree(&root->tree); @@ -231,7 +231,7 @@ static void test_parse_policy_tr(void **state) { (policy_node_with_key_t *) r_policy_node(&taptree_left->script); assert_int_equal(tapscript_left->base.type, TOKEN_PK); - check_key_expr_plain(r_policy_node_keyexpr(&tapscript_left->key_placeholder), 1, 0, 1); + check_key_expr_plain(r_policy_node_keyexpr(&tapscript_left->key), 1, 0, 1); policy_node_tree_t *taptree_right = (policy_node_tree_t *) r_policy_node_tree(&taptree->right_tree); @@ -240,7 +240,7 @@ static void test_parse_policy_tr(void **state) { (policy_node_with_key_t *) r_policy_node(&taptree_right->script); assert_int_equal(tapscript_right->base.type, TOKEN_PK); - check_key_expr_plain(r_policy_node_keyexpr(&tapscript_right->key_placeholder), 2, 5, 7); + check_key_expr_plain(r_policy_node_keyexpr(&tapscript_right->key), 2, 5, 7); } static void test_parse_policy_tr_multisig(void **state) { @@ -258,9 +258,9 @@ static void test_parse_policy_tr_multisig(void **state) { policy_node_tr_t *root = (policy_node_tr_t *) out; - assert_int_equal(r_policy_node_keyexpr(&root->key_placeholder)->key_index, 0); - assert_int_equal(r_policy_node_keyexpr(&root->key_placeholder)->num_first, 0); - assert_int_equal(r_policy_node_keyexpr(&root->key_placeholder)->num_second, 1); + assert_int_equal(r_policy_node_keyexpr(&root->key)->key_index, 0); + assert_int_equal(r_policy_node_keyexpr(&root->key)->num_first, 0); + assert_int_equal(r_policy_node_keyexpr(&root->key)->num_second, 1); policy_node_tree_t *taptree = r_policy_node_tree(&root->tree); @@ -275,8 +275,8 @@ static void test_parse_policy_tr_multisig(void **state) { assert_int_equal(tapscript_left->base.type, TOKEN_MULTI_A); assert_int_equal(tapscript_left->k, 1); assert_int_equal(tapscript_left->n, 2); - check_key_expr_plain(&r_policy_node_keyexpr(&tapscript_left->key_placeholders)[0], 1, 0, 1); - check_key_expr_plain(&r_policy_node_keyexpr(&tapscript_left->key_placeholders)[1], 2, 0, 1); + check_key_expr_plain(&r_policy_node_keyexpr(&tapscript_left->keys)[0], 1, 0, 1); + check_key_expr_plain(&r_policy_node_keyexpr(&tapscript_left->keys)[1], 2, 0, 1); policy_node_tree_t *taptree_right = (policy_node_tree_t *) r_policy_node_tree(&taptree->right_tree); @@ -287,9 +287,9 @@ static void test_parse_policy_tr_multisig(void **state) { assert_int_equal(tapscript_right->base.type, TOKEN_SORTEDMULTI_A); assert_int_equal(tapscript_right->k, 2); assert_int_equal(tapscript_right->n, 3); - check_key_expr_plain(&r_policy_node_keyexpr(&tapscript_right->key_placeholders)[0], 3, 0, 1); - check_key_expr_plain(&r_policy_node_keyexpr(&tapscript_right->key_placeholders)[1], 4, 0, 1); - check_key_expr_plain(&r_policy_node_keyexpr(&tapscript_right->key_placeholders)[2], 5, 0, 1); + check_key_expr_plain(&r_policy_node_keyexpr(&tapscript_right->keys)[0], 3, 0, 1); + check_key_expr_plain(&r_policy_node_keyexpr(&tapscript_right->keys)[1], 4, 0, 1); + check_key_expr_plain(&r_policy_node_keyexpr(&tapscript_right->keys)[2], 5, 0, 1); } static void test_parse_policy_tr_musig_keypath(void **state) { @@ -306,11 +306,7 @@ static void test_parse_policy_tr_musig_keypath(void **state) { assert_int_equal(root->base.type, TOKEN_TR); assert_true(isnull_policy_node_tree(&root->tree)); - check_key_expr_musig(r_policy_node_keyexpr(&root->key_placeholder), - 3, - (uint16_t[]){2, 0, 1}, - 3, - 13); + check_key_expr_musig(r_policy_node_keyexpr(&root->key), 3, (uint16_t[]){2, 0, 1}, 3, 13); } static void test_parse_policy_tr_musig_scriptpath(void **state) { @@ -334,11 +330,7 @@ static void test_parse_policy_tr_musig_scriptpath(void **state) { policy_node_with_key_t *script_pk = (policy_node_with_key_t *) r_policy_node(&tree->script); assert_int_equal(script_pk->base.type, TOKEN_PK); - check_key_expr_musig(r_policy_node_keyexpr(&script_pk->key_placeholder), - 3, - (uint16_t[]){2, 0, 3}, - 0, - 1); + check_key_expr_musig(r_policy_node_keyexpr(&script_pk->key), 3, (uint16_t[]){2, 0, 3}, 0, 1); } static void test_get_policy_segwit_version(void **state) {