From 99a92efdd9d4b2d278deffe38457a0e15b144c0d Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Sat, 22 Mar 2025 10:03:08 +0100 Subject: [PATCH 1/3] descriptors doc: Correct Markdown format + wording --- doc/descriptors.md | 8 ++++---- src/script/descriptor.cpp | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/descriptors.md b/doc/descriptors.md index 32922b5266..625a511bba 100644 --- a/doc/descriptors.md +++ b/doc/descriptors.md @@ -288,11 +288,11 @@ For example, a descriptor of the form: multi(2,xpub.../<0;1;2>/0/*,xpub.../<2;3;4>/*) -will expand to the two descriptors +will expand to the 3 descriptors - multi(2,xpub.../0/0/*,xpub.../2/*) - multi(2,xpub.../1/0/*,xpub.../3/*) - multi(2,xpub.../2/0/*,xpub.../4*) + multi(2,xpub.../0/0/*,xpub.../2/*) + multi(2,xpub.../1/0/*,xpub.../3/*) + multi(2,xpub.../2/0/*,xpub.../4/*) When this tuple contains only two elements, wallet implementations can use the first descriptor for receiving addresses and the second descriptor for change addresses. diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 2d23ec9ede..ecd315407e 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -1447,7 +1447,7 @@ std::optional ParseKeyPathNum(std::span elem, bool& apostr for (size_t i = 1; i < split.size(); ++i) { const std::span& elem = split[i]; - // Check if element contain multipath specifier + // Check if element contains multipath specifier if (!elem.empty() && elem.front() == '<' && elem.back() == '>') { if (!allow_multipath) { error = strprintf("Key path value '%s' specifies multipath in a section where multipath is not allowed", std::string(elem.begin(), elem.end())); From 7e974f474e2fbc764b1d35d207f19f2d880bc853 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Sat, 29 Mar 2025 20:43:48 +0100 Subject: [PATCH 2/3] descriptors refactor: Use range-for and limit scope of seen_multipath * Range-for avoids ++i/i++ debate and decreases linecount. * seen_multipath is only used if multipath_segment_index hasn't already been set. Rename it to seen_substitutes to better describe what it does, now that the context implies its involved in multipath. --- src/script/descriptor.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index ecd315407e..68a2cc90a1 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -1442,7 +1442,6 @@ std::optional ParseKeyPathNum(std::span elem, bool& apostr KeyPath path; std::optional multipath_segment_index; std::vector multipath_values; - std::unordered_set seen_multipath; for (size_t i = 1; i < split.size(); ++i) { const std::span& elem = split[i]; @@ -1465,10 +1464,11 @@ std::optional ParseKeyPathNum(std::span elem, bool& apostr return false; } + std::unordered_set seen_substitutes; for (const auto& num : nums) { const auto& op_num = ParseKeyPathNum(num, apostrophe, error); if (!op_num) return false; - auto [_, inserted] = seen_multipath.insert(*op_num); + auto [_, inserted] = seen_substitutes.insert(*op_num); if (!inserted) { error = strprintf("Duplicated key path value %u in multipath specifier", *op_num); return false; @@ -1489,9 +1489,9 @@ std::optional ParseKeyPathNum(std::span elem, bool& apostr out.emplace_back(std::move(path)); } else { // Replace the multipath placeholder with each value while generating paths - for (size_t i = 0; i < multipath_values.size(); i++) { + for (uint32_t substitute : multipath_values) { KeyPath branch_path = path; - branch_path[*multipath_segment_index] = multipath_values[i]; + branch_path[*multipath_segment_index] = substitute; out.emplace_back(std::move(branch_path)); } } From 56f271e9b9c82f40054d63d4b638584bd2faef00 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Fri, 28 Mar 2025 11:40:59 +0100 Subject: [PATCH 3/3] descriptors refactor: Clarify multipath data relationships through local struct --- src/script/descriptor.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 68a2cc90a1..e132a0ba22 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -1440,8 +1440,11 @@ std::optional ParseKeyPathNum(std::span elem, bool& apostr [[nodiscard]] bool ParseKeyPath(const std::vector>& split, std::vector& out, bool& apostrophe, std::string& error, bool allow_multipath) { KeyPath path; - std::optional multipath_segment_index; - std::vector multipath_values; + struct MultipathSubstitutes { + size_t placeholder_index; + std::vector values; + }; + std::optional substitutes; for (size_t i = 1; i < split.size(); ++i) { const std::span& elem = split[i]; @@ -1452,7 +1455,7 @@ std::optional ParseKeyPathNum(std::span elem, bool& apostr error = strprintf("Key path value '%s' specifies multipath in a section where multipath is not allowed", std::string(elem.begin(), elem.end())); return false; } - if (multipath_segment_index) { + if (substitutes) { error = "Multiple multipath key path specifiers found"; return false; } @@ -1464,6 +1467,7 @@ std::optional ParseKeyPathNum(std::span elem, bool& apostr return false; } + substitutes.emplace(); std::unordered_set seen_substitutes; for (const auto& num : nums) { const auto& op_num = ParseKeyPathNum(num, apostrophe, error); @@ -1473,11 +1477,11 @@ std::optional ParseKeyPathNum(std::span elem, bool& apostr error = strprintf("Duplicated key path value %u in multipath specifier", *op_num); return false; } - multipath_values.emplace_back(*op_num); + substitutes->values.emplace_back(*op_num); } path.emplace_back(); // Placeholder for multipath segment - multipath_segment_index = path.size()-1; + substitutes->placeholder_index = path.size() - 1; } else { const auto& op_num = ParseKeyPathNum(elem, apostrophe, error); if (!op_num) return false; @@ -1485,13 +1489,13 @@ std::optional ParseKeyPathNum(std::span elem, bool& apostr } } - if (!multipath_segment_index) { + if (!substitutes) { out.emplace_back(std::move(path)); } else { // Replace the multipath placeholder with each value while generating paths - for (uint32_t substitute : multipath_values) { + for (uint32_t substitute : substitutes->values) { KeyPath branch_path = path; - branch_path[*multipath_segment_index] = substitute; + branch_path[substitutes->placeholder_index] = substitute; out.emplace_back(std::move(branch_path)); } }