From e1121b9d131f7aea05a6b4685c5bfe1d7505777b Mon Sep 17 00:00:00 2001 From: Salvatore Ingala <6681844+bigspider@users.noreply.github.com> Date: Thu, 11 Jan 2024 09:42:06 +0100 Subject: [PATCH] Removed untyped versions of relative pointers; improved docs, and updated unit tests --- src/common/wallet.h | 22 +---- unit-tests/mock_includes/ledger_assert.h | 1 + unit-tests/test_wallet.c | 114 ++++++++++++++--------- 3 files changed, 75 insertions(+), 62 deletions(-) create mode 100644 unit-tests/mock_includes/ledger_assert.h diff --git a/src/common/wallet.h b/src/common/wallet.h index 7a3a945bb..16b1eb428 100644 --- a/src/common/wallet.h +++ b/src/common/wallet.h @@ -172,23 +172,11 @@ typedef enum { // policy_nodes, representing a non-negative offset from the position of the structure itself. // This reduces the memory utilization of those pointers, and moreover it allows to reduce padding // in other structures, as they no longer contain 32-bit pointers. -typedef struct ptr_rel_s { - uint16_t offset; -} ptr_rel_t; - -// TODO: remove the generic versions -// Converts a relative pointer to the corresponding pointer to the corresponding absolute pointer -static inline const void *resolve_ptr(const ptr_rel_t *ptr) { - return (const void *) ((const uint8_t *) ptr + ptr->offset); -} - -// Initializes a relative pointer so that it points to node. -// IMPORTANT: the assumption is that node is located in memory at an address larger than -// relative_ptr, and at an offset smaller than 65536. No error is detected otherwise, therefore this -// is potentially dangerous to use. -static inline void init_relative_ptr(ptr_rel_t *relative_ptr, void *node) { - relative_ptr->offset = (uint16_t) ((uint8_t *) node - (uint8_t *) relative_ptr); -} +// Moreover, avoiding all pointers makes sure that if the structure can be copied to a different +// location (making sure the destination is aligned due to the platform restrictions). +// The following macro defines the data structure and the helper methods for a relative pointer to a +// type. The code does not depend on the type, but this allows to keep strong types when dealing +// with relative pointers, which otherwise would require numerous type casts. // Defines a relative pointer type for name##t, and the conversion functions to/from a relative // pointer and a pointer to name##_t. diff --git a/unit-tests/mock_includes/ledger_assert.h b/unit-tests/mock_includes/ledger_assert.h new file mode 100644 index 000000000..246cf1653 --- /dev/null +++ b/unit-tests/mock_includes/ledger_assert.h @@ -0,0 +1 @@ +#define LEDGER_ASSERT(test, message) diff --git a/unit-tests/test_wallet.c b/unit-tests/test_wallet.c index 54b563618..13fb28571 100644 --- a/unit-tests/test_wallet.c +++ b/unit-tests/test_wallet.c @@ -53,7 +53,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_placeholder(node_1->key_placeholder, 0, 0, 1); + check_key_placeholder(r_policy_node_key_placeholder(&node_1->key_placeholder), 0, 0, 1); } static void test_parse_policy_map_singlesig_2(void **state) { @@ -68,10 +68,10 @@ static void test_parse_policy_map_singlesig_2(void **state) { assert_int_equal(root->base.type, TOKEN_SH); - policy_node_with_key_t *inner = (policy_node_with_key_t *) resolve_ptr(&root->script); + 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_placeholder(inner->key_placeholder, 0, 0, 1); + check_key_placeholder(r_policy_node_key_placeholder(&inner->key_placeholder), 0, 0, 1); } static void test_parse_policy_map_singlesig_3(void **state) { @@ -86,14 +86,14 @@ static void test_parse_policy_map_singlesig_3(void **state) { assert_int_equal(root->base.type, TOKEN_SH); - policy_node_with_script_t *mid = (policy_node_with_script_t *) resolve_ptr(&root->script); + policy_node_with_script_t *mid = (policy_node_with_script_t *) r_policy_node(&root->script); assert_int_equal(mid->base.type, TOKEN_WSH); - policy_node_with_key_t *inner = (policy_node_with_key_t *) resolve_ptr(&mid->script); + 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_placeholder(inner->key_placeholder, 0, 0, 1); + check_key_placeholder(r_policy_node_key_placeholder(&inner->key_placeholder), 0, 0, 1); } static void test_parse_policy_map_multisig_1(void **state) { @@ -109,9 +109,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_placeholder(&node_1->key_placeholders[0], 0, 0, 1); - check_key_placeholder(&node_1->key_placeholders[1], 1, 0, 1); - check_key_placeholder(&node_1->key_placeholders[2], 2, 0, 1); + check_key_placeholder(&r_policy_node_key_placeholder(&node_1->key_placeholders)[0], 0, 0, 1); + check_key_placeholder(&r_policy_node_key_placeholder(&node_1->key_placeholders)[1], 1, 0, 1); + check_key_placeholder(&r_policy_node_key_placeholder(&node_1->key_placeholders)[2], 2, 0, 1); } static void test_parse_policy_map_multisig_2(void **state) { @@ -126,13 +126,13 @@ static void test_parse_policy_map_multisig_2(void **state) { assert_int_equal(root->base.type, TOKEN_WSH); - policy_node_multisig_t *inner = (policy_node_multisig_t *) resolve_ptr(&root->script); + policy_node_multisig_t *inner = (policy_node_multisig_t *) r_policy_node(&root->script); assert_int_equal(inner->base.type, TOKEN_MULTI); assert_int_equal(inner->k, 3); assert_int_equal(inner->n, 5); for (int i = 0; i < 5; i++) { - check_key_placeholder(&inner->key_placeholders[i], i, 0, 1); + check_key_placeholder(&r_policy_node_key_placeholder(&inner->key_placeholders)[i], i, 0, 1); } } @@ -149,16 +149,16 @@ static void test_parse_policy_map_multisig_3(void **state) { assert_int_equal(root->base.type, TOKEN_SH); - policy_node_with_script_t *mid = (policy_node_with_script_t *) resolve_ptr(&root->script); + policy_node_with_script_t *mid = (policy_node_with_script_t *) r_policy_node(&root->script); assert_int_equal(mid->base.type, TOKEN_WSH); - policy_node_multisig_t *inner = (policy_node_multisig_t *) resolve_ptr(&mid->script); + policy_node_multisig_t *inner = (policy_node_multisig_t *) r_policy_node(&mid->script); assert_int_equal(inner->base.type, TOKEN_SORTEDMULTI); assert_int_equal(inner->k, 3); assert_int_equal(inner->n, 5); for (int i = 0; i < 5; i++) { - check_key_placeholder(&inner->key_placeholders[i], i, 0, 1); + check_key_placeholder(&r_policy_node_key_placeholder(&inner->key_placeholders)[i], i, 0, 1); } } @@ -174,8 +174,8 @@ static void test_parse_policy_tr(void **state) { assert_int_equal(res, 0); policy_node_tr_t *root = (policy_node_tr_t *) out; - assert_ptr_equal(root->tree, NULL); - check_key_placeholder(root->key_placeholder, 0, 0, 1); + assert_true(isnull_policy_node_tree(&root->tree)); + check_key_placeholder(r_policy_node_key_placeholder(&root->key_placeholder), 0, 0, 1); // Simple tr with a TREE that is a simple script res = parse_policy("tr(@0/**,pk(@1/**))", out, sizeof(out)); @@ -183,14 +183,15 @@ static void test_parse_policy_tr(void **state) { assert_int_equal(res, 0); root = (policy_node_tr_t *) out; - check_key_placeholder(root->key_placeholder, 0, 0, 1); + check_key_placeholder(r_policy_node_key_placeholder(&root->key_placeholder), 0, 0, 1); - assert_int_equal(root->tree->is_leaf, true); + assert_int_equal(r_policy_node_tree(&root->tree)->is_leaf, true); - policy_node_with_key_t *tapscript = (policy_node_with_key_t *) resolve_ptr(&root->tree->script); + policy_node_with_key_t *tapscript = + (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_placeholder(tapscript->key_placeholder, 1, 0, 1); + check_key_placeholder(r_policy_node_key_placeholder(&tapscript->key_placeholder), 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)); @@ -198,27 +199,32 @@ static void test_parse_policy_tr(void **state) { assert_int_equal(res, 0); root = (policy_node_tr_t *) out; - check_key_placeholder(root->key_placeholder, 0, 0, 1); + check_key_placeholder(r_policy_node_key_placeholder(&root->key_placeholder), 0, 0, 1); - policy_node_tree_t *taptree = root->tree; + policy_node_tree_t *taptree = r_policy_node_tree(&root->tree); assert_int_equal(taptree->is_leaf, false); - policy_node_tree_t *taptree_left = (policy_node_tree_t *) resolve_ptr(&taptree->left_tree); + policy_node_tree_t *taptree_left = + (policy_node_tree_t *) r_policy_node_tree(&taptree->left_tree); assert_int_equal(taptree_left->is_leaf, true); policy_node_with_key_t *tapscript_left = - (policy_node_with_key_t *) resolve_ptr(&taptree_left->script); + (policy_node_with_key_t *) r_policy_node(&taptree_left->script); assert_int_equal(tapscript_left->base.type, TOKEN_PK); - check_key_placeholder(tapscript_left->key_placeholder, 1, 0, 1); + check_key_placeholder(r_policy_node_key_placeholder(&tapscript_left->key_placeholder), 1, 0, 1); - policy_node_tree_t *taptree_right = (policy_node_tree_t *) resolve_ptr(&taptree->right_tree); + policy_node_tree_t *taptree_right = + (policy_node_tree_t *) r_policy_node_tree(&taptree->right_tree); assert_int_equal(taptree_right->is_leaf, true); policy_node_with_key_t *tapscript_right = - (policy_node_with_key_t *) resolve_ptr(&taptree_right->script); + (policy_node_with_key_t *) r_policy_node(&taptree_right->script); assert_int_equal(tapscript_right->base.type, TOKEN_PK); - check_key_placeholder(tapscript_right->key_placeholder, 2, 5, 7); + check_key_placeholder(r_policy_node_key_placeholder(&tapscript_right->key_placeholder), + 2, + 5, + 7); } static void test_parse_policy_tr_multisig(void **state) { @@ -236,36 +242,53 @@ static void test_parse_policy_tr_multisig(void **state) { policy_node_tr_t *root = (policy_node_tr_t *) out; - assert_int_equal(root->key_placeholder->key_index, 0); - assert_int_equal(root->key_placeholder->num_first, 0); - assert_int_equal(root->key_placeholder->num_second, 1); + assert_int_equal(r_policy_node_key_placeholder(&root->key_placeholder)->key_index, 0); + assert_int_equal(r_policy_node_key_placeholder(&root->key_placeholder)->num_first, 0); + assert_int_equal(r_policy_node_key_placeholder(&root->key_placeholder)->num_second, 1); - policy_node_tree_t *taptree = root->tree; + policy_node_tree_t *taptree = r_policy_node_tree(&root->tree); assert_int_equal(taptree->is_leaf, false); - policy_node_tree_t *taptree_left = (policy_node_tree_t *) resolve_ptr(&taptree->left_tree); + policy_node_tree_t *taptree_left = + (policy_node_tree_t *) r_policy_node_tree(&taptree->left_tree); assert_int_equal(taptree_left->is_leaf, true); policy_node_multisig_t *tapscript_left = - (policy_node_multisig_t *) resolve_ptr(&taptree_left->script); + (policy_node_multisig_t *) r_policy_node(&taptree_left->script); 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_placeholder(&tapscript_left->key_placeholders[0], 1, 0, 1); - check_key_placeholder(&tapscript_left->key_placeholders[1], 2, 0, 1); - - policy_node_tree_t *taptree_right = (policy_node_tree_t *) resolve_ptr(&taptree->right_tree); + check_key_placeholder(&r_policy_node_key_placeholder(&tapscript_left->key_placeholders)[0], + 1, + 0, + 1); + check_key_placeholder(&r_policy_node_key_placeholder(&tapscript_left->key_placeholders)[1], + 2, + 0, + 1); + + policy_node_tree_t *taptree_right = + (policy_node_tree_t *) r_policy_node_tree(&taptree->right_tree); assert_int_equal(taptree_right->is_leaf, true); policy_node_multisig_t *tapscript_right = - (policy_node_multisig_t *) resolve_ptr(&taptree_right->script); + (policy_node_multisig_t *) r_policy_node(&taptree_right->script); 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_placeholder(&tapscript_right->key_placeholders[0], 3, 0, 1); - check_key_placeholder(&tapscript_right->key_placeholders[1], 4, 0, 1); - check_key_placeholder(&tapscript_right->key_placeholders[2], 5, 0, 1); + check_key_placeholder(&r_policy_node_key_placeholder(&tapscript_right->key_placeholders)[0], + 3, + 0, + 1); + check_key_placeholder(&r_policy_node_key_placeholder(&tapscript_right->key_placeholders)[1], + 4, + 0, + 1); + check_key_placeholder(&r_policy_node_key_placeholder(&tapscript_right->key_placeholders)[2], + 5, + 0, + 1); } static void test_get_policy_segwit_version(void **state) { @@ -441,13 +464,14 @@ static void Test(const char *ms, const char *hexscript, int mode, int opslimit, const policy_node_t *miniscript; int context; if (((policy_node_t *) out)->type == TOKEN_WSH) { - miniscript = resolve_ptr(&((policy_node_with_script_t *) out)->script); + miniscript = r_policy_node(&((policy_node_with_script_t *) out)->script); context = MINISCRIPT_CONTEXT_P2WSH; } else { assert_true(((policy_node_t *) out)->type == TOKEN_TR); - assert_true(((policy_node_tr_t *) out)->tree->is_leaf); + policy_node_tr_t *tr = (policy_node_tr_t *) out; + assert_true(r_policy_node_tree(&tr->tree)->is_leaf); - miniscript = resolve_ptr(&((policy_node_tr_t *) out)->tree->script); + miniscript = r_policy_node(&r_policy_node_tree(&tr->tree)->script); context = MINISCRIPT_CONTEXT_TAPSCRIPT; }