Merge pull request #752 from evoskuil/master

Refactor blockchain_block_headers().

Author: evoskuil

include/bitcoin/server/protocols/protocol_electrum.hpp

@@ -237,7 +237,7 @@ class BCS_API protocol_electrum
 
     /// Common implementation for block_header/s.
     void blockchain_block_headers(size_t starting, size_t quantity,
-        size_t waypoint, bool multiplicity) NOEXCEPT;
+        size_t waypoint, bool single) NOEXCEPT;
 
     /// Completion handlers (for long-running address queries).
     /// -----------------------------------------------------------------------

src/protocols/electrum/protocol_electrum_headers.cpp

@@ -160,7 +160,7 @@ void protocol_electrum::handle_blockchain_block_header(const code& ec,
         return;
     }
 
-    blockchain_block_headers(starting, one, waypoint, false);
+    blockchain_block_headers(starting, one, waypoint, true);
 }
 
 void protocol_electrum::handle_blockchain_block_headers(const code& ec,
@@ -194,12 +194,12 @@ void protocol_electrum::handle_blockchain_block_headers(const code& ec,
         return;
     }
 
-    blockchain_block_headers(starting, quantity, waypoint, true);
+    blockchain_block_headers(starting, quantity, waypoint, false);
 }
 
 // Common implementation for blockchain_block_header/s.
 void protocol_electrum::blockchain_block_headers(size_t starting,
-    size_t quantity, size_t waypoint, bool multiplicity) NOEXCEPT
+    size_t quantity, size_t waypoint, bool single) NOEXCEPT
 {
     const auto prove = !is_zero(quantity) && !is_zero(waypoint);
     const auto target = starting + sub1(quantity);
@@ -235,115 +235,108 @@ void protocol_electrum::blockchain_block_headers(size_t starting,
     const auto links = query.get_confirmed_headers(starting, count);
     auto size = two * chain::header::serialized_size() * links.size();
 
-    // Single header, no proof: spec requires a plain hex string result,
-    // not a {"hex":…} or {"header":…} wrapper object.
-    if (!multiplicity && !prove)
+    if (single && !is_one(links.size()))
     {
-        if (links.empty())
+        send_code(error::server_error);
+        return;
+    }
+
+    value_t value{};
+
+    if (single && !prove)
+    {
+        const auto header = query.get_wire_header(links.front());
+        if (header.empty())
         {
             send_code(error::server_error);
             return;
         }
+
+        value = encode_base16(header);
+    }
+    else
+    {
+        object_t result{};
+
         if (at_least(electrum::version::v1_6))
         {
-            const auto header = query.get_wire_header(links.front());
-            if (header.empty())
+            // Collect headers into array.
+            array_t headers{};
+            headers.reserve(links.size());
+            for (const auto& link: links)
             {
-                send_code(error::server_error);
-                return;
+                const auto header = query.get_wire_header(link);
+                if (header.empty())
+                {
+                    send_code(error::server_error);
+                    return;
+                }
+
+                headers.push_back(encode_base16(header));
+            }
+
+            if (single)
+            {
+                result["header"] = std::move(headers.front());
+            }
+            else
+            {
+                result["max"] = maximum_headers;
+                result["count"] = links.size();
+                result["headers"] = std::move(headers);
             }
-            send_result(encode_base16(header), size + 42u);
         }
         else
         {
-            std::string header(size, '\0');
-            stream::out::fast sink{ header };
+            // Stream headers into single buffer.
+            std::string headers(size, '\0');
+            stream::out::fast sink{ headers };
             write::base16::fast writer{ sink };
-            if (!query.get_wire_header(writer, links.front()))
+            for (const auto& link: links)
             {
-                send_code(error::server_error);
-                return;
+                if (!query.get_wire_header(writer, link))
+                {
+                    send_code(error::server_error);
+                    return;
+                }
             }
-            send_result(std::move(header), size + 42u);
-        }
-        return;
-    }
-
-    value_t value{ object_t{} };
-    auto& result = std::get<object_t>(value.value());
-    if (multiplicity)
-    {
-        result["max"] = maximum_headers;
-        result["count"] = links.size();
-    }
-    else if (links.empty())
-    {
-        send_code(error::server_error);
-        return;
-    }
 
-    if (at_least(electrum::version::v1_6))
-    {
-        array_t headers{};
-        headers.reserve(links.size());
-        for (const auto& link: links)
-        {
-            const auto header = query.get_wire_header(link);
-            if (header.empty())
+            if (single)
             {
-                send_code(error::server_error);
-                return;
+                result["header"] = std::move(headers);
             }
+            else
+            {
+                result["max"] = maximum_headers;
+                result["count"] = links.size();
+                result["hex"] = std::move(headers);
+            }
+        }
 
-            headers.push_back(encode_base16(header));
-        };
-
-        if (multiplicity)
-            result["headers"] = std::move(headers);
-        else
-            result["header"] = std::move(headers.front());
-    }
-    else
-    {
-        std::string headers(size, '\0');
-        stream::out::fast sink{ headers };
-        write::base16::fast writer{ sink };
-        for (const auto& link: links)
+        if (prove)
         {
-            if (!query.get_wire_header(writer, link))
+            // A very slim chance of inconsistency given an intervening reorg
+            // because of get_merkle_root_and_proof() and height-based calcs.
+            // This is acceptable as must be verified by caller in any case.
+            hashes proof{};
+            hash_digest root{};
+            if (const auto code = query.get_merkle_root_and_proof(root, proof,
+                target, waypoint))
             {
-                send_code(error::server_error);
+                send_code(code);
                 return;
             }
-        };
 
-        if (multiplicity)
-            result["hex"] = std::move(headers);
-        else
-            result["header"] = std::move(headers);
-    }
+            array_t branch(proof.size());
+            std::ranges::transform(proof, branch.begin(),
+                [](const auto& hash) NOEXCEPT{ return encode_hash(hash); });
 
-    // There is a very slim chance of inconsistency given an intervening reorg
-    // because of get_merkle_root_and_proof() use of height-based calculations.
-    // This is acceptable as it must be verified by caller in any case.
-    if (prove)
-    {
-        hashes proof{};
-        hash_digest root{};
-        if (const auto code = query.get_merkle_root_and_proof(root, proof,
-            target, waypoint))
-        {
-            send_code(code);
-            return;
+            result["branch"] = std::move(branch);
+            result["root"] = encode_hash(root);
+            size += two * hash_size * add1(proof.size());
         }
 
-        array_t branch(proof.size());
-        std::ranges::transform(proof, branch.begin(),
-            [](const auto& hash) NOEXCEPT { return encode_hash(hash); });
-
-        result["branch"] = std::move(branch);
-        result["root"] = encode_hash(root);
-        size += two * hash_size * add1(proof.size());
+        value = std::move(result);
     }
 
     send_result(std::move(value), size + 42u);

src/protocols/electrum/protocol_electrum_server.cpp

@@ -118,11 +118,6 @@ void protocol_electrum::handle_server_features(const code& ec,
         { "pruning", null_t{} }
     };
 
-    if (!at_least(electrum::version::v1_7))
-    {
-        value["hash_function"] = string_t{ "sha256" };
-    }
-
     if (at_least(electrum::version::v1_7))
     {
         value["method_flavours"] = object_t
@@ -138,6 +133,10 @@ void protocol_electrum::handle_server_features(const code& ec,
             }
         };
     }
+    else
+    {
+        value["hash_function"] = string_t{ "sha256" };
+    }
 
     send_result(std::move(value), 1024);
 }
@@ -187,20 +186,19 @@ void protocol_electrum::handle_server_ping(const code& ec,
     }
     else
     {
-        size_t length{};
         data_chunk unused{};
 
         // Base16 encoding validation expects whole octets (even char count).
-        if (!to_integer(length, pong_len) || (length != data.length()) ||
+        if (!to_integer(size, pong_len) || (size != data.length()) ||
             !decode_base16(unused, data))
         {
             send_code(error::invalid_argument);
             return;
         }
 
         // Treat empty as default (args look the same, may not be correct).
-        if (is_nonzero(length))
-            value = string_t(length, '0');
+        if (is_nonzero(size))
+            value = string_t(size, '0');
     }
 
     // Length is limited by maximum_request (DoS protection).