diff --git a/doc/REST-interface.md b/doc/REST-interface.md index 0fee78129a6..ed46e22262b 100644 --- a/doc/REST-interface.md +++ b/doc/REST-interface.md @@ -47,6 +47,11 @@ The HTTP request and response are both handled entirely in-memory. With the /notxdetails/ option JSON response will only contain the transaction hash instead of the complete transaction details. The option only affects the JSON response. +- `GET /rest/blockpart/.?offset=&size=` + +Given a block hash: returns a block part, in binary or hex-encoded binary formats. +Responds with 404 if the block or the byte range doesn't exist. + #### Blockheaders `GET /rest/headers/.?count=` diff --git a/doc/release-notes-33657.md b/doc/release-notes-33657.md new file mode 100644 index 00000000000..f9e6841bf74 --- /dev/null +++ b/doc/release-notes-33657.md @@ -0,0 +1,5 @@ +New REST API +------------ + +- A new REST API endpoint (`/rest/blockpart/BLOCKHASH.bin?offset=X&size=Y`) has been introduced + for efficiently fetching a range of bytes from block `BLOCKHASH`. diff --git a/src/bench/readwriteblock.cpp b/src/bench/readwriteblock.cpp index 4ca5b8ebe26..a742a1e8dd4 100644 --- a/src/bench/readwriteblock.cpp +++ b/src/bench/readwriteblock.cpp @@ -57,11 +57,9 @@ static void ReadRawBlockBench(benchmark::Bench& bench) const auto testing_setup{MakeNoLogFileContext(ChainType::MAIN)}; auto& blockman{testing_setup->m_node.chainman->m_blockman}; const auto pos{blockman.WriteBlock(CreateTestBlock(), 413'567)}; - std::vector block_data; - blockman.ReadRawBlock(block_data, pos); // warmup bench.run([&] { - const auto success{blockman.ReadRawBlock(block_data, pos)}; - assert(success); + const auto res{blockman.ReadRawBlock(pos)}; + assert(res); }); } diff --git a/src/init.cpp b/src/init.cpp index 09e7523e329..e283e89eab7 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1759,7 +1759,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) g_zmq_notification_interface = CZMQNotificationInterface::Create( [&chainman = node.chainman](std::vector& block, const CBlockIndex& index) { assert(chainman); - return chainman->m_blockman.ReadRawBlock(block, WITH_LOCK(cs_main, return index.GetBlockPos())); + if (auto ret{chainman->m_blockman.ReadRawBlock(WITH_LOCK(cs_main, return index.GetBlockPos()))}) { + block = std::move(*ret); + return true; + } + return false; }); if (g_zmq_notification_interface) { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c8bc4a846df..00acebd11c3 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2276,8 +2276,9 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } else if (inv.IsMsgWitnessBlk()) { // Fast-path: in this case it is possible to serve the block directly from disk, // as the network format matches the format on disk - std::vector block_data; - if (!m_chainman.m_blockman.ReadRawBlock(block_data, block_pos)) { + if (const auto block_data{m_chainman.m_blockman.ReadRawBlock(block_pos)}) { + MakeAndPushMessage(pfrom, NetMsgType::BLOCK, std::span{*block_data}); + } else { if (WITH_LOCK(m_chainman.GetMutex(), return m_chainman.m_blockman.IsBlockPruned(*pindex))) { LogDebug(BCLog::NET, "Block was pruned before it could be read, %s\n", pfrom.DisconnectMsg(fLogIPs)); } else { @@ -2286,7 +2287,6 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& pfrom.fDisconnect = true; return; } - MakeAndPushMessage(pfrom, NetMsgType::BLOCK, std::span{block_data}); // Don't set pblock as we've sent the block } else { // Send block from disk diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index a5984df6171..22c8f619e60 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -1006,14 +1006,14 @@ bool BlockManager::ReadBlock(CBlock& block, const FlatFilePos& pos, const std::o block.SetNull(); // Open history file to read - std::vector block_data; - if (!ReadRawBlock(block_data, pos)) { + const auto block_data{ReadRawBlock(pos)}; + if (!block_data) { return false; } try { // Read block - SpanReader{block_data} >> TX_WITH_WITNESS(block); + SpanReader{*block_data} >> TX_WITH_WITNESS(block); } catch (const std::exception& e) { LogError("Deserialize or I/O error - %s at %s while reading block", e.what(), pos.ToString()); return false; @@ -1048,19 +1048,19 @@ bool BlockManager::ReadBlock(CBlock& block, const CBlockIndex& index) const return ReadBlock(block, block_pos, index.GetBlockHash()); } -bool BlockManager::ReadRawBlock(std::vector& block, const FlatFilePos& pos) const +BlockManager::ReadRawBlockResult BlockManager::ReadRawBlock(const FlatFilePos& pos, std::optional> block_part) const { if (pos.nPos < STORAGE_HEADER_BYTES) { // If nPos is less than STORAGE_HEADER_BYTES, we can't read the header that precedes the block data // This would cause an unsigned integer underflow when trying to position the file cursor // This can happen after pruning or default constructed positions LogError("Failed for %s while reading raw block storage header", pos.ToString()); - return false; + return util::Unexpected{ReadRawError::IO}; } AutoFile filein{OpenBlockFile({pos.nFile, pos.nPos - STORAGE_HEADER_BYTES}, /*fReadOnly=*/true)}; if (filein.IsNull()) { LogError("OpenBlockFile failed for %s while reading raw block", pos.ToString()); - return false; + return util::Unexpected{ReadRawError::IO}; } try { @@ -1072,23 +1072,31 @@ bool BlockManager::ReadRawBlock(std::vector& block, const FlatFilePos if (blk_start != GetParams().MessageStart()) { LogError("Block magic mismatch for %s: %s versus expected %s while reading raw block", pos.ToString(), HexStr(blk_start), HexStr(GetParams().MessageStart())); - return false; + return util::Unexpected{ReadRawError::IO}; } if (blk_size > MAX_SIZE) { LogError("Block data is larger than maximum deserialization size for %s: %s versus %s while reading raw block", pos.ToString(), blk_size, MAX_SIZE); - return false; + return util::Unexpected{ReadRawError::IO}; } - block.resize(blk_size); // Zeroing of memory is intentional here - filein.read(block); + if (block_part) { + const auto [offset, size]{*block_part}; + if (size == 0 || offset >= blk_size || size > blk_size - offset) { + return util::Unexpected{ReadRawError::BadPartRange}; // Avoid logging - offset/size come from untrusted REST input + } + filein.seek(offset, SEEK_CUR); + blk_size = size; + } + + std::vector data(blk_size); // Zeroing of memory is intentional here + filein.read(data); + return data; } catch (const std::exception& e) { LogError("Read from block file failed: %s for %s while reading raw block", e.what(), pos.ToString()); - return false; + return util::Unexpected{ReadRawError::IO}; } - - return true; } FlatFilePos BlockManager::WriteBlock(const CBlock& block, int nHeight) diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 9ba7873cdca..e3f9c445ead 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -169,6 +170,10 @@ struct BlockfileCursor { std::ostream& operator<<(std::ostream& os, const BlockfileCursor& cursor); +enum class ReadRawError { + IO, + BadPartRange, +}; /** * Maintains a tree of blocks (stored in `m_block_index`) which is consulted @@ -302,6 +307,7 @@ private: public: using Options = kernel::BlockManagerOpts; + using ReadRawBlockResult = util::Expected, ReadRawError>; explicit BlockManager(const util::SignalInterrupt& interrupt, Options opts); @@ -455,7 +461,7 @@ public: /** Functions for disk access for blocks */ bool ReadBlock(CBlock& block, const FlatFilePos& pos, const std::optional& expected_hash) const; bool ReadBlock(CBlock& block, const CBlockIndex& index) const; - bool ReadRawBlock(std::vector& block, const FlatFilePos& pos) const; + ReadRawBlockResult ReadRawBlock(const FlatFilePos& pos, std::optional> block_part = std::nullopt) const; bool ReadBlockUndo(CBlockUndo& blockundo, const CBlockIndex& index) const; diff --git a/src/rest.cpp b/src/rest.cpp index 40a07ba166c..06da3906649 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -379,10 +379,17 @@ static bool rest_spent_txouts(const std::any& context, HTTPRequest* req, const s } } +/** + * This handler is used by multiple HTTP endpoints: + * - `/block/` via `rest_block_extended()` + * - `/block/notxdetails/` via `rest_block_notxdetails()` + * - `/blockpart/` via `rest_block_part()` (doesn't support JSON response, so `tx_verbosity` is unset) + */ static bool rest_block(const std::any& context, HTTPRequest* req, const std::string& uri_part, - TxVerbosity tx_verbosity) + std::optional tx_verbosity, + std::optional> block_part = std::nullopt) { if (!CheckWarmup(req)) return false; @@ -416,34 +423,43 @@ static bool rest_block(const std::any& context, pos = pblockindex->GetBlockPos(); } - std::vector block_data{}; - if (!chainman.m_blockman.ReadRawBlock(block_data, pos)) { - return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found"); + const auto block_data{chainman.m_blockman.ReadRawBlock(pos, block_part)}; + if (!block_data) { + switch (block_data.error()) { + case node::ReadRawError::IO: return RESTERR(req, HTTP_INTERNAL_SERVER_ERROR, "I/O error reading " + hashStr); + case node::ReadRawError::BadPartRange: + assert(block_part); + return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Bad block part offset/size %d/%d for %s", block_part->first, block_part->second, hashStr)); + } // no default case, so the compiler can warn about missing cases + assert(false); } switch (rf) { case RESTResponseFormat::BINARY: { req->WriteHeader("Content-Type", "application/octet-stream"); - req->WriteReply(HTTP_OK, block_data); + req->WriteReply(HTTP_OK, *block_data); return true; } case RESTResponseFormat::HEX: { - const std::string strHex{HexStr(block_data) + "\n"}; + const std::string strHex{HexStr(*block_data) + "\n"}; req->WriteHeader("Content-Type", "text/plain"); req->WriteReply(HTTP_OK, strHex); return true; } case RESTResponseFormat::JSON: { - CBlock block{}; - DataStream block_stream{block_data}; - block_stream >> TX_WITH_WITNESS(block); - UniValue objBlock = blockToJSON(chainman.m_blockman, block, *tip, *pblockindex, tx_verbosity, chainman.GetConsensus().powLimit); - std::string strJSON = objBlock.write() + "\n"; - req->WriteHeader("Content-Type", "application/json"); - req->WriteReply(HTTP_OK, strJSON); - return true; + if (tx_verbosity) { + CBlock block{}; + DataStream block_stream{*block_data}; + block_stream >> TX_WITH_WITNESS(block); + UniValue objBlock = blockToJSON(chainman.m_blockman, block, *tip, *pblockindex, *tx_verbosity, chainman.GetConsensus().powLimit); + std::string strJSON = objBlock.write() + "\n"; + req->WriteHeader("Content-Type", "application/json"); + req->WriteReply(HTTP_OK, strJSON); + return true; + } + return RESTERR(req, HTTP_BAD_REQUEST, "JSON output is not supported for this request type"); } default: { @@ -462,6 +478,25 @@ static bool rest_block_notxdetails(const std::any& context, HTTPRequest* req, co return rest_block(context, req, uri_part, TxVerbosity::SHOW_TXID); } +static bool rest_block_part(const std::any& context, HTTPRequest* req, const std::string& uri_part) +{ + try { + if (const auto opt_offset{ToIntegral(req->GetQueryParameter("offset").value_or(""))}) { + if (const auto opt_size{ToIntegral(req->GetQueryParameter("size").value_or(""))}) { + return rest_block(context, req, uri_part, + /*tx_verbosity=*/std::nullopt, + /*block_part=*/{{*opt_offset, *opt_size}}); + } else { + return RESTERR(req, HTTP_BAD_REQUEST, "Block part size missing or invalid"); + } + } else { + return RESTERR(req, HTTP_BAD_REQUEST, "Block part offset missing or invalid"); + } + } catch (const std::runtime_error& e) { + return RESTERR(req, HTTP_BAD_REQUEST, e.what()); + } +} + static bool rest_filter_header(const std::any& context, HTTPRequest* req, const std::string& uri_part) { if (!CheckWarmup(req)) return false; @@ -1110,6 +1145,7 @@ static const struct { {"/rest/tx/", rest_tx}, {"/rest/block/notxdetails/", rest_block_notxdetails}, {"/rest/block/", rest_block_extended}, + {"/rest/blockpart/", rest_block_part}, {"/rest/blockfilter/", rest_block_filter}, {"/rest/blockfilterheaders/", rest_filter_header}, {"/rest/chaininfo", rest_chaininfo}, diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 8f283ff4f03..c68bfb8c25a 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -680,7 +680,6 @@ static CBlock GetBlockChecked(BlockManager& blockman, const CBlockIndex& blockin static std::vector GetRawBlockChecked(BlockManager& blockman, const CBlockIndex& blockindex) { - std::vector data{}; FlatFilePos pos{}; { LOCK(cs_main); @@ -688,13 +687,10 @@ static std::vector GetRawBlockChecked(BlockManager& blockman, const C pos = blockindex.GetBlockPos(); } - if (!blockman.ReadRawBlock(data, pos)) { - // Block not found on disk. This shouldn't normally happen unless the block was - // pruned right after we released the lock above. - throw JSONRPCError(RPC_MISC_ERROR, "Block not found on disk"); - } - - return data; + if (auto data{blockman.ReadRawBlock(pos)}) return std::move(*data); + // Block not found on disk. This shouldn't normally happen unless the block was + // pruned right after we released the lock above. + throw JSONRPCError(RPC_MISC_ERROR, "Block not found on disk"); } static CBlockUndo GetUndoChecked(BlockManager& blockman, const CBlockIndex& blockindex) diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index a155199437b..4a326458464 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -138,6 +138,68 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup) BOOST_CHECK(!blockman.CheckBlockDataAvailability(tip, *last_pruned_block)); } +BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_part, TestChain100Setup) +{ + LOCK(::cs_main); + auto& chainman{m_node.chainman}; + auto& blockman{chainman->m_blockman}; + const CBlockIndex& tip{*chainman->ActiveTip()}; + const FlatFilePos tip_block_pos{tip.GetBlockPos()}; + + auto block{blockman.ReadRawBlock(tip_block_pos)}; + BOOST_REQUIRE(block); + BOOST_REQUIRE_GE(block->size(), 200); + + const auto expect_part{[&](size_t offset, size_t size) { + auto res{blockman.ReadRawBlock(tip_block_pos, std::pair{offset, size})}; + BOOST_CHECK(res); + const auto& part{res.value()}; + BOOST_CHECK_EQUAL_COLLECTIONS(part.begin(), part.end(), block->begin() + offset, block->begin() + offset + size); + }}; + + expect_part(0, 20); + expect_part(0, block->size() - 1); + expect_part(0, block->size() - 10); + expect_part(0, block->size()); + expect_part(1, block->size() - 1); + expect_part(10, 20); + expect_part(block->size() - 1, 1); +} + +BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_part_error, TestChain100Setup) +{ + LOCK(::cs_main); + auto& chainman{m_node.chainman}; + auto& blockman{chainman->m_blockman}; + const CBlockIndex& tip{*chainman->ActiveTip()}; + const FlatFilePos tip_block_pos{tip.GetBlockPos()}; + + auto block{blockman.ReadRawBlock(tip_block_pos)}; + BOOST_REQUIRE(block); + BOOST_REQUIRE_GE(block->size(), 200); + + const auto expect_part_error{[&](size_t offset, size_t size) { + auto res{blockman.ReadRawBlock(tip_block_pos, std::pair{offset, size})}; + BOOST_CHECK(!res); + BOOST_CHECK_EQUAL(res.error(), node::ReadRawError::BadPartRange); + }}; + + expect_part_error(0, 0); + expect_part_error(0, block->size() + 1); + expect_part_error(0, std::numeric_limits::max()); + expect_part_error(1, block->size()); + expect_part_error(2, block->size() - 1); + expect_part_error(block->size() - 1, 2); + expect_part_error(block->size() - 2, 3); + expect_part_error(block->size() + 1, 0); + expect_part_error(block->size() + 1, 1); + expect_part_error(block->size() + 2, 2); + expect_part_error(block->size(), 0); + expect_part_error(block->size(), 1); + expect_part_error(std::numeric_limits::max(), 1); + expect_part_error(std::numeric_limits::max(), std::numeric_limits::max()); +} + BOOST_FIXTURE_TEST_CASE(blockmanager_readblock_hash_mismatch, TestingSetup) { CBlockIndex index; diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index 54fc908a14a..be5bb78b10e 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -28,7 +28,6 @@ from test_framework.wallet import ( MiniWallet, getnewdestination, ) -from typing import Optional INVALID_PARAM = "abc" @@ -66,13 +65,16 @@ class RESTTest (BitcoinTestFramework): body: str = '', status: int = 200, ret_type: RetType = RetType.JSON, - query_params: Optional[dict[str, typing.Any]] = None, + query_params: typing.Union[dict[str, typing.Any], str, None] = None, ) -> typing.Union[http.client.HTTPResponse, bytes, str, None]: rest_uri = '/rest' + uri if req_type in ReqType: rest_uri += f'.{req_type.name.lower()}' if query_params: - rest_uri += f'?{urllib.parse.urlencode(query_params)}' + if isinstance(query_params, str): + rest_uri += f'?{query_params}' + else: + rest_uri += f'?{urllib.parse.urlencode(query_params)}' conn = http.client.HTTPConnection(self.url.hostname, self.url.port) self.log.debug(f'{http_method} {rest_uri} {body}') @@ -82,7 +84,7 @@ class RESTTest (BitcoinTestFramework): conn.request('POST', rest_uri, body) resp = conn.getresponse() - assert_equal(resp.status, status) + assert resp.status == status, f"Expected: {status}, Got: {resp.status} - Response: {str(resp.read())}" if ret_type == RetType.OBJ: return resp @@ -455,6 +457,52 @@ class RESTTest (BitcoinTestFramework): expected = [(p["scriptPubKey"], p["value"]) for p in prevouts] assert_equal(expected, actual) + self.log.info("Test the /blockpart URI") + + blockhash = self.nodes[0].getbestblockhash() + block_bin = self.test_rest_request(f"/block/{blockhash}", req_type=ReqType.BIN, ret_type=RetType.BYTES) + for req_type in (ReqType.BIN, ReqType.HEX): + def get_block_part(status: int = 200, **kwargs): + resp = self.test_rest_request(f"/blockpart/{blockhash}", status=status, + req_type=req_type, ret_type=RetType.BYTES, **kwargs) + assert isinstance(resp, bytes) + if req_type is ReqType.HEX and status == 200: + resp = bytes.fromhex(resp.decode().strip()) + return resp + + assert_equal(block_bin, get_block_part(query_params={"offset": 0, "size": len(block_bin)})) + + assert len(block_bin) >= 500 + assert_equal(block_bin[20:320], get_block_part(query_params={"offset": 20, "size": 300})) + assert_equal(block_bin[-5:], get_block_part(query_params={"offset": len(block_bin) - 5, "size": 5})) + + get_block_part(status=400, query_params={"offset": 10}) + get_block_part(status=400, query_params={"size": 100}) + get_block_part(status=400, query_params={"offset": "x"}) + get_block_part(status=400, query_params={"size": "y"}) + get_block_part(status=400, query_params={"offset": "x", "size": "y"}) + assert get_block_part(status=400, query_params="%XY").decode("utf-8").startswith("URI parsing failed") + + get_block_part(status=400, query_params={"offset": 0, "size": 0}) + get_block_part(status=400, query_params={"offset": len(block_bin), "size": 0}) + get_block_part(status=400, query_params={"offset": len(block_bin) + 1, "size": 1}) + get_block_part(status=400, query_params={"offset": len(block_bin), "size": 1}) + get_block_part(status=400, query_params={"offset": len(block_bin) + 1, "size": 1}) + get_block_part(status=400, query_params={"offset": 0, "size": len(block_bin) + 1}) + + self.test_rest_request(f"/blockpart/{blockhash}", status=400, req_type=ReqType.JSON, ret_type=RetType.OBJ) + + self.log.info("Missing block data should cause REST API to fail") + + self.test_rest_request(f"/block/{blockhash}", status=200, req_type=ReqType.BIN, ret_type=RetType.OBJ) + self.test_rest_request(f"/blockpart/{blockhash}", query_params={"offset": 0, "size": 1}, status=200, req_type=ReqType.BIN, ret_type=RetType.OBJ) + blk_files = list(self.nodes[0].blocks_path.glob("blk*.dat")) + for blk_file in blk_files: + blk_file.rename(blk_file.with_suffix('.bkp')) + self.test_rest_request(f"/block/{blockhash}", status=500, req_type=ReqType.BIN, ret_type=RetType.OBJ) + self.test_rest_request(f"/blockpart/{blockhash}", query_params={"offset": 0, "size": 1}, status=500, req_type=ReqType.BIN, ret_type=RetType.OBJ) + for blk_file in blk_files: + blk_file.with_suffix('.bkp').rename(blk_file) self.log.info("Test the /deploymentinfo URI")