Merge bitcoin/bitcoin#23836: rest: Expose block filters follow-ups

4523d28b6bd550bf9f9c724f9aa90c3d2d4ebe3b [test] compare filter and header with the result of the getblockfilter RPC (Niklas Gögge)
3a2464f21619c3065ec2f5d8f7431703c30c964a [rest] drop superfluous rpc serializations flags for block filters (Niklas Gögge)
064abd14a55e0fa1bff530237816a748d01e0ddb [rest] add a more verbose error message for invalid header counts (Niklas Gögge)
83b8f3a8961baa34a136ecfaf62c3ea0d133b6d6 [refactor] various style fix-ups (Niklas Gögge)

Pull request description:

  This PR addresses unresolved review comments from [#17631](https://github.com/bitcoin/bitcoin/pull/17631).
  This includes:
  * various style fix-ups
  * returning a more verbose error message for invalid header counts
  * removing superfluous rpc serializations flags for block filters
  * improving the test to include comparing the block filters returned from the rest  with the ones returned from the `getblockfilter` RPC.

ACKs for top commit:
  jnewbery:
    ACK 4523d28b6bd550bf9f9c724f9aa90c3d2d4ebe3b
  brunoerg:
    tACK 4523d28b6bd550bf9f9c724f9aa90c3d2d4ebe3b

Tree-SHA512: 634e6b2ae3e1d5f31675a50cfe11a5e74bf5a51b9e7b512d9e18879bf2ed424fc0ac6ec633023f192e3ad12cf0c73b0b51de79dd7ec00844dba3e1493d823b8c

2 files changed, 21 insertions, 16 deletions

diff --git a/src/rest.cpp b/src/rest.cpp
index 927196a003..addce6b0ea 100644
--- a/src/rest.cpp
+++ b/src/rest.cpp
@@ -195,7 +195,7 @@ static bool rest_headers(const std::any& context,
 
     const auto parsed_count{ToIntegral<size_t>(path[0])};
     if (!parsed_count.has_value() || *parsed_count < 1 || *parsed_count > MAX_REST_HEADERS_RESULTS) {
-        return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count out of acceptable range (1-%u): %s",  MAX_REST_HEADERS_RESULTS, path[0]));
+        return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count is invalid or out of acceptable range (1-%u): %s", MAX_REST_HEADERS_RESULTS, path[0]));
     }
 
     std::string hashStr = path[1];
@@ -341,11 +341,10 @@ static bool rest_block_notxdetails(const std::any& context, HTTPRequest* req, co
     return rest_block(context, req, strURIPart, TxVerbosity::SHOW_TXID);
 }
 
-
 static bool rest_filter_header(const std::any& context, HTTPRequest* req, const std::string& strURIPart)
 {
-    if (!CheckWarmup(req))
-        return false;
+    if (!CheckWarmup(req)) return false;
+
     std::string param;
     const RetFormat rf = ParseDataFormat(param, strURIPart);
 
@@ -372,10 +371,10 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const
 
     const auto parsed_count{ToIntegral<size_t>(uri_parts[1])};
     if (!parsed_count.has_value() || *parsed_count < 1 || *parsed_count > MAX_REST_HEADERS_RESULTS) {
-        return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count out of acceptable range (1-%u): %s",  MAX_REST_HEADERS_RESULTS, uri_parts[1]));
+        return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count is invalid or out of acceptable range (1-%u): %s", MAX_REST_HEADERS_RESULTS, uri_parts[1]));
     }
 
-    std::vector<const CBlockIndex *> headers;
+    std::vector<const CBlockIndex*> headers;
     headers.reserve(*parsed_count);
     {
         ChainstateManager* maybe_chainman = GetChainman(context, req);
@@ -396,7 +395,7 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const
 
     std::vector<uint256> filter_headers;
     filter_headers.reserve(*parsed_count);
-    for (const CBlockIndex *pindex : headers) {
+    for (const CBlockIndex* pindex : headers) {
         uint256 filter_header;
         if (!index->LookupFilterHeader(pindex, filter_header)) {
             std::string errmsg = "Filter not found.";
@@ -414,7 +413,7 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const
 
     switch (rf) {
     case RetFormat::BINARY: {
-        CDataStream ssHeader(SER_NETWORK, PROTOCOL_VERSION | RPCSerializationFlags());
+        CDataStream ssHeader{SER_NETWORK, PROTOCOL_VERSION};
         for (const uint256& header : filter_headers) {
             ssHeader << header;
         }
@@ -425,7 +424,7 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const
         return true;
     }
     case RetFormat::HEX: {
-        CDataStream ssHeader(SER_NETWORK, PROTOCOL_VERSION | RPCSerializationFlags());
+        CDataStream ssHeader{SER_NETWORK, PROTOCOL_VERSION};
         for (const uint256& header : filter_headers) {
             ssHeader << header;
         }
@@ -454,12 +453,12 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const
 
 static bool rest_block_filter(const std::any& context, HTTPRequest* req, const std::string& strURIPart)
 {
-    if (!CheckWarmup(req))
-        return false;
+    if (!CheckWarmup(req)) return false;
+
     std::string param;
     const RetFormat rf = ParseDataFormat(param, strURIPart);
 
-    //request is sent over URI scheme /rest/blockfilter/filtertype/blockhash
+    // request is sent over URI scheme /rest/blockfilter/filtertype/blockhash
     std::vector<std::string> uri_parts;
     boost::split(uri_parts, param, boost::is_any_of("/"));
     if (uri_parts.size() != 2) {
@@ -514,7 +513,7 @@ static bool rest_block_filter(const std::any& context, HTTPRequest* req, const s
 
     switch (rf) {
     case RetFormat::BINARY: {
-        CDataStream ssResp(SER_NETWORK, PROTOCOL_VERSION | RPCSerializationFlags());
+        CDataStream ssResp{SER_NETWORK, PROTOCOL_VERSION};
         ssResp << filter;
 
         std::string binaryResp = ssResp.str();
@@ -523,7 +522,7 @@ static bool rest_block_filter(const std::any& context, HTTPRequest* req, const s
         return true;
     }
     case RetFormat::HEX: {
-        CDataStream ssResp(SER_NETWORK, PROTOCOL_VERSION | RPCSerializationFlags());
+        CDataStream ssResp{SER_NETWORK, PROTOCOL_VERSION};
         ssResp << filter;
 
         std::string strHex = HexStr(ssResp) + "\n";
diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py
index 9cb452e9c0..2842b2534d 100755
--- a/test/functional/interface_rest.py
+++ b/test/functional/interface_rest.py
@@ -273,13 +273,19 @@ class RESTTest (BitcoinTestFramework):
         json_obj = self.test_rest_request(f"/headers/5/{bb_hash}")
         assert_equal(len(json_obj), 5)  # now we should have 5 header objects
         json_obj = self.test_rest_request(f"/blockfilterheaders/basic/5/{bb_hash}")
+        first_filter_header = json_obj[0]
         assert_equal(len(json_obj), 5)  # now we should have 5 filter header objects
-        self.test_rest_request(f"/blockfilter/basic/{bb_hash}", req_type=ReqType.BIN, ret_type=RetType.OBJ)
+        json_obj = self.test_rest_request(f"/blockfilter/basic/{bb_hash}")
+
+        # Compare with normal RPC blockfilter response
+        rpc_blockfilter = self.nodes[0].getblockfilter(bb_hash)
+        assert_equal(first_filter_header, rpc_blockfilter['header'])
+        assert_equal(json_obj['filter'], rpc_blockfilter['filter'])
 
         # Test number parsing
         for num in ['5a', '-5', '0', '2001', '99999999999999999999999999999999999']:
             assert_equal(
-                bytes(f'Header count out of acceptable range (1-2000): {num}\r\n', 'ascii'),
+                bytes(f'Header count is invalid or out of acceptable range (1-2000): {num}\r\n', 'ascii'),
                 self.test_rest_request(f"/headers/{num}/{bb_hash}", ret_type=RetType.BYTES, status=400),
             )