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

4523d28b6b [test] compare filter and header with the result of the getblockfilter RPC (Niklas Gögge)
3a2464f216 [rest] drop superfluous rpc serializations flags for block filters (Niklas Gögge)
064abd14a5 [rest] add a more verbose error message for invalid header counts (Niklas Gögge)
83b8f3a896 [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 4523d28b6b
  brunoerg:
    tACK 4523d28b6b

Tree-SHA512: 634e6b2ae3e1d5f31675a50cfe11a5e74bf5a51b9e7b512d9e18879bf2ed424fc0ac6ec633023f192e3ad12cf0c73b0b51de79dd7ec00844dba3e1493d823b8c
This commit is contained in:
fanquake 2022-01-02 08:16:31 +08:00
commit 9d099b02d8
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
2 changed files with 21 additions and 16 deletions

View file

@ -195,7 +195,7 @@ static bool rest_headers(const std::any& context,
const auto parsed_count{ToIntegral<size_t>(path[0])}; const auto parsed_count{ToIntegral<size_t>(path[0])};
if (!parsed_count.has_value() || *parsed_count < 1 || *parsed_count > MAX_REST_HEADERS_RESULTS) { 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]; 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); return rest_block(context, req, strURIPart, TxVerbosity::SHOW_TXID);
} }
static bool rest_filter_header(const std::any& context, HTTPRequest* req, const std::string& strURIPart) static bool rest_filter_header(const std::any& context, HTTPRequest* req, const std::string& strURIPart)
{ {
if (!CheckWarmup(req)) if (!CheckWarmup(req)) return false;
return false;
std::string param; std::string param;
const RetFormat rf = ParseDataFormat(param, strURIPart); 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])}; const auto parsed_count{ToIntegral<size_t>(uri_parts[1])};
if (!parsed_count.has_value() || *parsed_count < 1 || *parsed_count > MAX_REST_HEADERS_RESULTS) { 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); headers.reserve(*parsed_count);
{ {
ChainstateManager* maybe_chainman = GetChainman(context, req); 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; std::vector<uint256> filter_headers;
filter_headers.reserve(*parsed_count); filter_headers.reserve(*parsed_count);
for (const CBlockIndex *pindex : headers) { for (const CBlockIndex* pindex : headers) {
uint256 filter_header; uint256 filter_header;
if (!index->LookupFilterHeader(pindex, filter_header)) { if (!index->LookupFilterHeader(pindex, filter_header)) {
std::string errmsg = "Filter not found."; std::string errmsg = "Filter not found.";
@ -414,7 +413,7 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const
switch (rf) { switch (rf) {
case RetFormat::BINARY: { case RetFormat::BINARY: {
CDataStream ssHeader(SER_NETWORK, PROTOCOL_VERSION | RPCSerializationFlags()); CDataStream ssHeader{SER_NETWORK, PROTOCOL_VERSION};
for (const uint256& header : filter_headers) { for (const uint256& header : filter_headers) {
ssHeader << header; ssHeader << header;
} }
@ -425,7 +424,7 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const
return true; return true;
} }
case RetFormat::HEX: { case RetFormat::HEX: {
CDataStream ssHeader(SER_NETWORK, PROTOCOL_VERSION | RPCSerializationFlags()); CDataStream ssHeader{SER_NETWORK, PROTOCOL_VERSION};
for (const uint256& header : filter_headers) { for (const uint256& header : filter_headers) {
ssHeader << header; 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) static bool rest_block_filter(const std::any& context, HTTPRequest* req, const std::string& strURIPart)
{ {
if (!CheckWarmup(req)) if (!CheckWarmup(req)) return false;
return false;
std::string param; std::string param;
const RetFormat rf = ParseDataFormat(param, strURIPart); 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; std::vector<std::string> uri_parts;
boost::split(uri_parts, param, boost::is_any_of("/")); boost::split(uri_parts, param, boost::is_any_of("/"));
if (uri_parts.size() != 2) { if (uri_parts.size() != 2) {
@ -514,7 +513,7 @@ static bool rest_block_filter(const std::any& context, HTTPRequest* req, const s
switch (rf) { switch (rf) {
case RetFormat::BINARY: { case RetFormat::BINARY: {
CDataStream ssResp(SER_NETWORK, PROTOCOL_VERSION | RPCSerializationFlags()); CDataStream ssResp{SER_NETWORK, PROTOCOL_VERSION};
ssResp << filter; ssResp << filter;
std::string binaryResp = ssResp.str(); std::string binaryResp = ssResp.str();
@ -523,7 +522,7 @@ static bool rest_block_filter(const std::any& context, HTTPRequest* req, const s
return true; return true;
} }
case RetFormat::HEX: { case RetFormat::HEX: {
CDataStream ssResp(SER_NETWORK, PROTOCOL_VERSION | RPCSerializationFlags()); CDataStream ssResp{SER_NETWORK, PROTOCOL_VERSION};
ssResp << filter; ssResp << filter;
std::string strHex = HexStr(ssResp) + "\n"; std::string strHex = HexStr(ssResp) + "\n";

View file

@ -273,13 +273,19 @@ class RESTTest (BitcoinTestFramework):
json_obj = self.test_rest_request(f"/headers/5/{bb_hash}") json_obj = self.test_rest_request(f"/headers/5/{bb_hash}")
assert_equal(len(json_obj), 5) # now we should have 5 header objects 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}") 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 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 # Test number parsing
for num in ['5a', '-5', '0', '2001', '99999999999999999999999999999999999']: for num in ['5a', '-5', '0', '2001', '99999999999999999999999999999999999']:
assert_equal( 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), self.test_rest_request(f"/headers/{num}/{bb_hash}", ret_type=RetType.BYTES, status=400),
) )