diff --git a/src/init.cpp b/src/init.cpp index 36557387f1..0ecfba18c1 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1454,9 +1454,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) #if ENABLE_ZMQ g_zmq_notification_interface = CZMQNotificationInterface::Create( - [&chainman = node.chainman](CBlock& block, const CBlockIndex& index) { + [&chainman = node.chainman](std::vector& block, const CBlockIndex& index) { assert(chainman); - return chainman->m_blockman.ReadBlockFromDisk(block, index); + return chainman->m_blockman.ReadRawBlockFromDisk(block, WITH_LOCK(cs_main, return index.GetBlockPos())); }); if (g_zmq_notification_interface) { diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 211d557826..d0ddfeb0f1 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -1088,6 +1088,12 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const CBlockIndex& index) co bool BlockManager::ReadRawBlockFromDisk(std::vector& block, const FlatFilePos& pos) const { FlatFilePos hpos = pos; + // If nPos is less than 8 the pos is null and we don't have the block data + // Return early to prevent undefined behavior of unsigned int underflow + if (hpos.nPos < 8) { + LogError("%s: OpenBlockFile failed for %s\n", __func__, pos.ToString()); + return false; + } hpos.nPos -= 8; // Seek back 8 bytes for meta header AutoFile filein{OpenBlockFile(hpos, true)}; if (filein.IsNull()) { diff --git a/src/rest.cpp b/src/rest.cpp index 91184745c8..89c033b8a3 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -34,7 +35,7 @@ #include #include -#include +#include #include @@ -295,7 +296,7 @@ static bool rest_block(const std::any& context, if (!ParseHashStr(hashStr, hash)) return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + hashStr); - CBlock block; + FlatFilePos pos{}; const CBlockIndex* pblockindex = nullptr; const CBlockIndex* tip = nullptr; ChainstateManager* maybe_chainman = GetChainman(context, req); @@ -311,32 +312,33 @@ static bool rest_block(const std::any& context, if (chainman.m_blockman.IsBlockPruned(*pblockindex)) { return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not available (pruned data)"); } + pos = pblockindex->GetBlockPos(); } - if (!chainman.m_blockman.ReadBlockFromDisk(block, *pblockindex)) { + std::vector block_data{}; + if (!chainman.m_blockman.ReadRawBlockFromDisk(block_data, pos)) { return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found"); } switch (rf) { case RESTResponseFormat::BINARY: { - DataStream ssBlock; - ssBlock << TX_WITH_WITNESS(block); - std::string binaryBlock = ssBlock.str(); + const std::string binaryBlock{block_data.begin(), block_data.end()}; req->WriteHeader("Content-Type", "application/octet-stream"); req->WriteReply(HTTP_OK, binaryBlock); return true; } case RESTResponseFormat::HEX: { - DataStream ssBlock; - ssBlock << TX_WITH_WITNESS(block); - std::string strHex = HexStr(ssBlock) + "\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); std::string strJSON = objBlock.write() + "\n"; req->WriteHeader("Content-Type", "application/json"); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index dfdddeacea..a1135c27d4 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -595,6 +596,28 @@ static CBlock GetBlockChecked(BlockManager& blockman, const CBlockIndex& blockin return block; } +static std::vector GetRawBlockChecked(BlockManager& blockman, const CBlockIndex& blockindex) +{ + std::vector data{}; + FlatFilePos pos{}; + { + LOCK(cs_main); + if (blockman.IsBlockPruned(blockindex)) { + throw JSONRPCError(RPC_MISC_ERROR, "Block not available (pruned data)"); + } + pos = blockindex.GetBlockPos(); + } + + if (!blockman.ReadRawBlockFromDisk(data, pos)) { + // Block not found on disk. This could be because we have the block + // header in our index but not yet have the block or did not accept the + // block. Or if the block was pruned right after we released the lock above. + throw JSONRPCError(RPC_MISC_ERROR, "Block not found on disk"); + } + + return data; +} + static CBlockUndo GetUndoChecked(BlockManager& blockman, const CBlockIndex& blockindex) { CBlockUndo blockUndo; @@ -735,15 +758,16 @@ static RPCHelpMan getblock() } } - const CBlock block{GetBlockChecked(chainman.m_blockman, *pblockindex)}; + const std::vector block_data{GetRawBlockChecked(chainman.m_blockman, *pblockindex)}; if (verbosity <= 0) { - DataStream ssBlock; - ssBlock << TX_WITH_WITNESS(block); - std::string strHex = HexStr(ssBlock); - return strHex; + return HexStr(block_data); } + DataStream block_stream{block_data}; + CBlock block{}; + block_stream >> TX_WITH_WITNESS(block); + TxVerbosity tx_verbosity; if (verbosity == 1) { tx_verbosity = TxVerbosity::SHOW_TXID; diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index 63c2737706..d10db046f5 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -41,7 +41,7 @@ std::list CZMQNotificationInterface::GetActiveNotif return result; } -std::unique_ptr CZMQNotificationInterface::Create(std::function get_block_by_index) +std::unique_ptr CZMQNotificationInterface::Create(std::function&, const CBlockIndex&)> get_block_by_index) { std::map factories; factories["pubhashblock"] = CZMQAbstractNotifier::Create; diff --git a/src/zmq/zmqnotificationinterface.h b/src/zmq/zmqnotificationinterface.h index 45d0982bd3..c879fdd0dd 100644 --- a/src/zmq/zmqnotificationinterface.h +++ b/src/zmq/zmqnotificationinterface.h @@ -12,6 +12,7 @@ #include #include #include +#include class CBlock; class CBlockIndex; @@ -25,7 +26,7 @@ public: std::list GetActiveNotifiers() const; - static std::unique_ptr Create(std::function get_block_by_index); + static std::unique_ptr Create(std::function&, const CBlockIndex&)> get_block_by_index); protected: bool Initialize(); diff --git a/src/zmq/zmqpublishnotifier.cpp b/src/zmq/zmqpublishnotifier.cpp index 0f20706364..608870c489 100644 --- a/src/zmq/zmqpublishnotifier.cpp +++ b/src/zmq/zmqpublishnotifier.cpp @@ -243,16 +243,13 @@ bool CZMQPublishRawBlockNotifier::NotifyBlock(const CBlockIndex *pindex) { LogPrint(BCLog::ZMQ, "Publish rawblock %s to %s\n", pindex->GetBlockHash().GetHex(), this->address); - DataStream ss; - CBlock block; + std::vector block{}; if (!m_get_block_by_index(block, *pindex)) { zmqError("Can't read block from disk"); return false; } - ss << TX_WITH_WITNESS(block); - - return SendZmqMessage(MSG_RAWBLOCK, &(*ss.begin()), ss.size()); + return SendZmqMessage(MSG_RAWBLOCK, block.data(), block.size()); } bool CZMQPublishRawTransactionNotifier::NotifyTransaction(const CTransaction &transaction) diff --git a/src/zmq/zmqpublishnotifier.h b/src/zmq/zmqpublishnotifier.h index a5cd433761..cc941a899c 100644 --- a/src/zmq/zmqpublishnotifier.h +++ b/src/zmq/zmqpublishnotifier.h @@ -10,8 +10,8 @@ #include #include #include +#include -class CBlock; class CBlockIndex; class CTransaction; @@ -49,10 +49,10 @@ public: class CZMQPublishRawBlockNotifier : public CZMQAbstractPublishNotifier { private: - const std::function m_get_block_by_index; + const std::function&, const CBlockIndex&)> m_get_block_by_index; public: - CZMQPublishRawBlockNotifier(std::function get_block_by_index) + CZMQPublishRawBlockNotifier(std::function&, const CBlockIndex&)> get_block_by_index) : m_get_block_by_index{std::move(get_block_by_index)} {} bool NotifyBlock(const CBlockIndex *pindex) override; }; diff --git a/test/functional/interface_zmq.py b/test/functional/interface_zmq.py index 6fe088c2b0..3c3ff1e4a0 100755 --- a/test/functional/interface_zmq.py +++ b/test/functional/interface_zmq.py @@ -5,6 +5,7 @@ """Test the ZMQ notification interface.""" import struct from time import sleep +from io import BytesIO from test_framework.address import ( ADDRESS_BCRT1_P2WSH_OP_TRUE, @@ -17,6 +18,7 @@ from test_framework.blocktools import ( ) from test_framework.test_framework import BitcoinTestFramework from test_framework.messages import ( + CBlock, hash256, tx_from_hex, ) @@ -201,8 +203,13 @@ class ZMQTest (BitcoinTestFramework): assert_equal(tx.hash, txid.hex()) # Should receive the generated raw block. - block = rawblock.receive() - assert_equal(genhashes[x], hash256_reversed(block[:80]).hex()) + hex = rawblock.receive() + block = CBlock() + block.deserialize(BytesIO(hex)) + assert block.is_valid() + assert_equal(block.vtx[0].hash, tx.hash) + assert_equal(len(block.vtx), 1) + assert_equal(genhashes[x], hash256_reversed(hex[:80]).hex()) # Should receive the generated block hash. hash = hashblock.receive().hex()