Merge bitcoin/bitcoin#32134: descriptors: Multipath/PR 22838 follow-ups

56f271e descriptors refactor: Clarify multipath data relationships through local struct (Hodlinator)
7e974f4 descriptors refactor: Use range-for and limit scope of seen_multipath (Hodlinator)
99a92ef descriptors doc: Correct Markdown format + wording (Hodlinator)

Pull request description:

  Follows up on unresolved suggestions from #22838. In order of priority:

  1. Fixes a couple of typos [^1][^2] and indentation to conform to Markdown.
  2. Solves `for`-loop nit [^3] and also limits variable scope.
  3. Clarifies data relationships [^4][^5] by introducing `struct` rather than comments.

  [^1]: bitcoin/bitcoin#22838 (comment)
  [^2]: bitcoin/bitcoin#22838 (comment)
  [^3]: bitcoin/bitcoin#22838 (comment)
  [^4]: bitcoin/bitcoin#22838 (comment)
  [^5]: bitcoin/bitcoin#22838 (comment)

ACKs for top commit:
  Prabhat1308:
    re-ACK [`56f271e`](bitcoin/bitcoin@56f271e)
  mabu44:
    tACK 56f271e
  l0rinc:
    utACK 56f271e
  rkrux:
    crACK 56f271e

Tree-SHA512: 75777c911640038a3e0ea48601c0f55463a5f8ff5b3462d81e8992d9fc8f988d5a240e2166befa67a2a246696b0863f8e2508524c14697c490d3b229fe048996

Author: ryanofsky

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.

src/script/descriptor.cpp

@@ -1440,20 +1440,22 @@ std::optional<uint32_t> ParseKeyPathNum(std::span<const char> elem, bool& apostr
 [[nodiscard]] bool ParseKeyPath(const std::vector<std::span<const char>>& split, std::vector<KeyPath>& out, bool& apostrophe, std::string& error, bool allow_multipath)
 {
     KeyPath path;
-    std::optional<size_t> multipath_segment_index;
-    std::vector<uint32_t> multipath_values;
-    std::unordered_set<uint32_t> seen_multipath;
+    struct MultipathSubstitutes {
+        size_t placeholder_index;
+        std::vector<uint32_t> values;
+    };
+    std::optional<MultipathSubstitutes> substitutes;
 
     for (size_t i = 1; i < split.size(); ++i) {
         const std::span<const char>& 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()));
                 return false;
             }
-            if (multipath_segment_index) {
+            if (substitutes) {
                 error = "Multiple multipath key path specifiers found";
                 return false;
             }
@@ -1465,33 +1467,35 @@ std::optional<uint32_t> ParseKeyPathNum(std::span<const char> elem, bool& apostr
                 return false;
             }
 
+            substitutes.emplace();
+            std::unordered_set<uint32_t> 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;
                 }
-                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;
             path.emplace_back(*op_num);
         }
     }
 
-    if (!multipath_segment_index) {
+    if (!substitutes) {
         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 : substitutes->values) {
             KeyPath branch_path = path;
-            branch_path[*multipath_segment_index] = multipath_values[i];
+            branch_path[substitutes->placeholder_index] = substitute;
             out.emplace_back(std::move(branch_path));
         }
     }