From 3b7acef4e7271dd87305d522790e233132c0ea78 Mon Sep 17 00:00:00 2001 From: Jon Griffiths Date: Tue, 28 Jan 2025 14:13:01 +1300 Subject: [PATCH 1/5] descriptor: disallow any parent for taproot descriptors --- src/descriptor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/descriptor.c b/src/descriptor.c index d5b341bbf..d954ea3a8 100644 --- a/src/descriptor.c +++ b/src/descriptor.c @@ -696,7 +696,7 @@ static int verify_raw(ms_ctx *ctx, ms_node *node) static int verify_raw_tr(ms_ctx *ctx, ms_node *node) { - if (node->child->builtin || !(node->child->kind & KIND_KEY) || + if (node->parent || node->child->builtin || !(node->child->kind & KIND_KEY) || node_has_uncompressed_key(ctx, node)) return WALLY_EINVAL; node->type_properties = builtin_get(node)->type_properties; @@ -708,7 +708,7 @@ static int verify_tr(ms_ctx *ctx, ms_node *node) const uint32_t child_count = node_get_child_count(node); if (child_count != 1u) return WALLY_EINVAL; /* FIXME: Support script paths */ - if (node->child->builtin || !(node->child->kind & KIND_KEY) || + if (node->parent || node->child->builtin || !(node->child->kind & KIND_KEY) || node_has_uncompressed_key(ctx, node)) return WALLY_EINVAL; node->type_properties = builtin_get(node)->type_properties; From 839d9973ebd0d7dcb7d8bb477d723a01e27753ea Mon Sep 17 00:00:00 2001 From: Jon Griffiths Date: Tue, 28 Jan 2025 14:13:44 +1300 Subject: [PATCH 2/5] descriptor: update "any parent" tests to use sh() The pk() parent rejects the children in these cases, rather than the intended check (where the child rejects having any parent at all). --- src/ctest/test_descriptor.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ctest/test_descriptor.c b/src/ctest/test_descriptor.c index 812e229fd..7f12d4a68 100644 --- a/src/ctest/test_descriptor.c +++ b/src/ctest/test_descriptor.c @@ -1209,7 +1209,7 @@ static const struct descriptor_test { WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "" },{ "descriptor - combo - any parent", - "pk(combo(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798))", + "sh(combo(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798))", WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "" },{ "descriptor - combo - multi-child", @@ -1275,7 +1275,7 @@ static const struct descriptor_test { WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "" },{ "descriptor - addr - any parent", - "pk(addr(bc1qrp33g0q5c5txsp9arysrx4k6zdkfs4nce4xj0gdcccefvpysxf3qccfmv3))", + "sh(addr(bc1qrp33g0q5c5txsp9arysrx4k6zdkfs4nce4xj0gdcccefvpysxf3qccfmv3))", WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "" },{ "descriptor - raw - multi-child", @@ -1289,7 +1289,7 @@ static const struct descriptor_test { WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "" },{ "descriptor - raw - any parent", - "pk(raw(000102030405060708090a0b0c0d0e0f))", + "sh(raw(000102030405060708090a0b0c0d0e0f))", WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "" },{ "descriptor - empty rawtr", @@ -1301,7 +1301,7 @@ static const struct descriptor_test { WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "" },{ "descriptor - rawtr - any parent", - "pk(rawtr(x_only))", + "sh(rawtr(x_only))", WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "" },{ "descriptor - rawtr - uncompressed key", @@ -1321,7 +1321,7 @@ static const struct descriptor_test { WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "" },{ "descriptor - tr - any parent", - "pk(tr(x_only))", + "sh(tr(x_only))", WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "" },{ "descriptor - tr - uncompressed key", From ccffda1e8400358e6b550f51dc6a3651fdcc3383 Mon Sep 17 00:00:00 2001 From: Jon Griffiths Date: Tue, 28 Jan 2025 14:20:47 +1300 Subject: [PATCH 3/5] descriptor: add (negative) tests for explicit public key via pk() These are disallowed by the spec and core. --- src/ctest/test_descriptor.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/ctest/test_descriptor.c b/src/ctest/test_descriptor.c index 7f12d4a68..efffe8c74 100644 --- a/src/ctest/test_descriptor.c +++ b/src/ctest/test_descriptor.c @@ -1307,6 +1307,10 @@ static const struct descriptor_test { "descriptor - rawtr - uncompressed key", "rawtr(uncompressed)", WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "" + },{ + "descriptor - rawtr - explicit public key", + "rawtr(pk(key_1))", + WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "" },{ "descriptor - rawtr - invalid public key", "rawtr(uncompresseduncompressed)", @@ -1327,6 +1331,10 @@ static const struct descriptor_test { "descriptor - tr - uncompressed key", "tr(uncompressed)", WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "" + },{ + "descriptor - tr - explicit public key", + "tr(pk(key_1))", + WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "" },{ "descriptor - tr - invalid public key", "tr(uncompresseduncompressed)", From 2d0e6fe7be83039c265f075908e37f3f561602c8 Mon Sep 17 00:00:00 2001 From: Jon Griffiths Date: Tue, 28 Jan 2025 22:00:12 +1300 Subject: [PATCH 4/5] descriptor (taproot): check return value as well as returned length --- src/descriptor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/descriptor.c b/src/descriptor.c index d954ea3a8..47984fe13 100644 --- a/src/descriptor.c +++ b/src/descriptor.c @@ -1422,13 +1422,13 @@ static int generate_tr(ms_ctx *ctx, ms_node *node, { unsigned char tweaked[EC_PUBLIC_KEY_LEN]; unsigned char pubkey[EC_PUBLIC_KEY_UNCOMPRESSED_LEN + 1]; - size_t pubkey_len; + size_t pubkey_len = 0; int ret; /* Generate a push of the x-only public key of our child */ const bool force_xonly = true; ret = generate_pk_k_impl(ctx, node, pubkey, sizeof(pubkey), force_xonly, &pubkey_len); - if (pubkey_len != EC_XONLY_PUBLIC_KEY_LEN + 1) + if (ret != WALLY_OK || pubkey_len != EC_XONLY_PUBLIC_KEY_LEN + 1) return WALLY_EINVAL; /* Should be PUSH_32 [x-only pubkey] */ /* Tweak it into a compressed pubkey */ From 2648de7e2c0d3a0ef8ef711a11686c59e0e1d204 Mon Sep 17 00:00:00 2001 From: Jon Griffiths Date: Tue, 28 Jan 2025 15:59:58 +1300 Subject: [PATCH 5/5] descriptor: rename canonicalize to canonicalize_impl canonicalize() is exposed as a math function when _GNU_SOURCE is defined. Since some amalgamation users might need that, rename it. --- src/descriptor.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/descriptor.c b/src/descriptor.c index 47984fe13..d4a36242e 100644 --- a/src/descriptor.c +++ b/src/descriptor.c @@ -375,9 +375,9 @@ static bool is_identifer_char(char c) static bool is_policy_start_char(char c) { return c == '@'; } static bool is_policy_identifer_char(char c) { return c >= '0' && c <= '9'; } -static int canonicalize(const char *descriptor, - const struct wally_map *vars_in, uint32_t flags, - char **output, size_t *num_substitutions) +static int canonicalize_impl(const char *descriptor, + const struct wally_map *vars_in, uint32_t flags, + char **output, size_t *num_substitutions) { const size_t VAR_MAX_NAME_LEN = 16; is_identifer_fn is_id_start = is_identifer_char, is_id_char = is_identifer_char; @@ -2754,8 +2754,8 @@ int wally_descriptor_parse(const char *miniscript, ctx->num_multipaths = 1; ret = wally_map_init(vars_in ? vars_in->num_items : 1, NULL, &ctx->keys); if (ret == WALLY_OK) - ret = canonicalize(miniscript, vars_in, flags & MS_FLAGS_CANONICALIZE, - &ctx->src, &num_substitutions); + ret = canonicalize_impl(miniscript, vars_in, flags & MS_FLAGS_CANONICALIZE, + &ctx->src, &num_substitutions); if (ret == WALLY_OK) { ctx->src_len = strlen(ctx->src); ctx->features = WALLY_MS_IS_DESCRIPTOR; /* Un-set if miniscript found */