From 3bdc7c2d3977a7864aacea80bffc4df7f37cac51 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 21 May 2020 14:36:42 -0400 Subject: [PATCH 1/3] [doc] Add comment for m_headers_cache --- src/index/blockfilterindex.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h index 7ca43540c7f..317f8c0e40f 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -39,6 +39,7 @@ private: size_t WriteFilterToDisk(FlatFilePos& pos, const BlockFilter& filter); Mutex m_cs_headers_cache; + /** cache of block hash to filter header, to avoid disk access when responding to getcfcheckpt. */ std::unordered_map m_headers_cache GUARDED_BY(m_cs_headers_cache); protected: From f6b58c150686e90bc4952976e488b1605f3ae02a Mon Sep 17 00:00:00 2001 From: Jim Posen Date: Mon, 4 May 2020 19:47:26 -0400 Subject: [PATCH 2/3] [net processing] Message handling for getcfheaders. if -peerblockfilters is configured, handle requests for cfheaders. --- src/net_processing.cpp | 86 ++++++++++++++++++++++++++++++++++++++++-- src/protocol.cpp | 4 ++ src/protocol.h | 15 +++++++- 3 files changed, 100 insertions(+), 5 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 9d9e31d3ed8..9f7bde89cfd 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -129,6 +129,8 @@ static constexpr unsigned int INVENTORY_BROADCAST_MAX = 7 * INVENTORY_BROADCAST_ static constexpr unsigned int AVG_FEEFILTER_BROADCAST_INTERVAL = 10 * 60; /** Maximum feefilter broadcast delay after significant change. */ static constexpr unsigned int MAX_FEEFILTER_CHANGE_DELAY = 5 * 60; +/** Maximum number of cf hashes that may be requested with one getcfheaders. See BIP 157. */ +static constexpr uint32_t MAX_GETCFHEADERS_SIZE = 2000; struct COrphanTx { // When modifying, adapt the copy of this definition in tests/DoS_tests. @@ -1983,14 +1985,16 @@ void static ProcessOrphanTx(CConnman* connman, CTxMemPool& mempool, std::setnHeight; + if (start_height > stop_height) { + LogPrint(BCLog::NET, "peer %d sent invalid getcfilters/getcfheaders with " /* Continued */ + "start height %d and stop height %d\n", + pfrom->GetId(), start_height, stop_height); + pfrom->fDisconnect = true; + return false; + } + if (stop_height - start_height >= max_height_diff) { + LogPrint(BCLog::NET, "peer %d requested too many cfilters/cfheaders: %d / %d\n", + pfrom->GetId(), stop_height - start_height + 1, max_height_diff); + pfrom->fDisconnect = true; + return false; + } + filter_index = GetBlockFilterIndex(filter_type); if (!filter_index) { LogPrint(BCLog::NET, "Filter index for supported type %s not found\n", BlockFilterTypeName(filter_type)); @@ -2026,6 +2045,61 @@ static bool PrepareBlockFilterRequest(CNode* pfrom, const CChainParams& chain_pa return true; } +/** + * Handle a cfheaders request. + * + * May disconnect from the peer in the case of a bad request. + * + * @param[in] pfrom The peer that we received the request from + * @param[in] vRecv The raw message received + * @param[in] chain_params Chain parameters + * @param[in] connman Pointer to the connection manager + */ +static void ProcessGetCFHeaders(CNode* pfrom, CDataStream& vRecv, const CChainParams& chain_params, + CConnman* connman) +{ + uint8_t filter_type_ser; + uint32_t start_height; + uint256 stop_hash; + + vRecv >> filter_type_ser >> start_height >> stop_hash; + + const BlockFilterType filter_type = static_cast(filter_type_ser); + + const CBlockIndex* stop_index; + BlockFilterIndex* filter_index; + if (!PrepareBlockFilterRequest(pfrom, chain_params, filter_type, start_height, stop_hash, + MAX_GETCFHEADERS_SIZE, stop_index, filter_index)) { + return; + } + + uint256 prev_header; + if (start_height > 0) { + const CBlockIndex* const prev_block = + stop_index->GetAncestor(static_cast(start_height - 1)); + if (!filter_index->LookupFilterHeader(prev_block, prev_header)) { + LogPrint(BCLog::NET, "Failed to find block filter header in index: filter_type=%s, block_hash=%s\n", + BlockFilterTypeName(filter_type), prev_block->GetBlockHash().ToString()); + return; + } + } + + std::vector filter_hashes; + if (!filter_index->LookupFilterHashRange(start_height, stop_index, filter_hashes)) { + LogPrint(BCLog::NET, "Failed to find block filter hashes in index: filter_type=%s, start_height=%d, stop_hash=%s\n", + BlockFilterTypeName(filter_type), start_height, stop_hash.ToString()); + return; + } + + CSerializedNetMsg msg = CNetMsgMaker(pfrom->GetSendVersion()) + .Make(NetMsgType::CFHEADERS, + filter_type_ser, + stop_index->GetBlockHash(), + prev_header, + filter_hashes); + connman->PushMessage(pfrom, std::move(msg)); +} + /** * Handle a getcfcheckpt request. * @@ -2048,7 +2122,8 @@ static void ProcessGetCFCheckPt(CNode* pfrom, CDataStream& vRecv, const CChainPa const CBlockIndex* stop_index; BlockFilterIndex* filter_index; - if (!PrepareBlockFilterRequest(pfrom, chain_params, filter_type, stop_hash, + if (!PrepareBlockFilterRequest(pfrom, chain_params, filter_type, /*start_height=*/0, stop_hash, + /*max_height_diff=*/std::numeric_limits::max(), stop_index, filter_index)) { return; } @@ -3385,6 +3460,11 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec return true; } + if (msg_type == NetMsgType::GETCFHEADERS) { + ProcessGetCFHeaders(pfrom, vRecv, chainparams, connman); + return true; + } + if (msg_type == NetMsgType::GETCFCHECKPT) { ProcessGetCFCheckPt(pfrom, vRecv, chainparams, connman); return true; diff --git a/src/protocol.cpp b/src/protocol.cpp index e929cff1102..243111c4491 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -40,6 +40,8 @@ const char *SENDCMPCT="sendcmpct"; const char *CMPCTBLOCK="cmpctblock"; const char *GETBLOCKTXN="getblocktxn"; const char *BLOCKTXN="blocktxn"; +const char *GETCFHEADERS="getcfheaders"; +const char *CFHEADERS="cfheaders"; const char *GETCFCHECKPT="getcfcheckpt"; const char *CFCHECKPT="cfcheckpt"; } // namespace NetMsgType @@ -73,6 +75,8 @@ const static std::string allNetMessageTypes[] = { NetMsgType::CMPCTBLOCK, NetMsgType::GETBLOCKTXN, NetMsgType::BLOCKTXN, + NetMsgType::GETCFHEADERS, + NetMsgType::CFHEADERS, NetMsgType::GETCFCHECKPT, NetMsgType::CFCHECKPT, }; diff --git a/src/protocol.h b/src/protocol.h index 8092ad7c132..9527dce9605 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -225,6 +225,19 @@ extern const char* GETBLOCKTXN; * @since protocol version 70014 as described by BIP 152 */ extern const char* BLOCKTXN; +/** + * getcfheaders requests a compact filter header and the filter hashes for a + * range of blocks, which can then be used to reconstruct the filter headers + * for those blocks. + * Only available with service bit NODE_COMPACT_FILTERS as described by + * BIP 157 & 158. + */ +extern const char* GETCFHEADERS; +/** + * cfheaders is a response to a getcfheaders request containing a filter header + * and a vector of filter hashes for each subsequent block in the requested range. + */ +extern const char* CFHEADERS; /** * getcfcheckpt requests evenly spaced compact filter headers, enabling * parallelized download and validation of the headers between them. @@ -235,8 +248,6 @@ extern const char* GETCFCHECKPT; /** * cfcheckpt is a response to a getcfcheckpt request containing a vector of * evenly spaced filter headers for blocks on the requested chain. - * Only available with service bit NODE_COMPACT_FILTERS as described by - * BIP 157 & 158. */ extern const char* CFCHECKPT; }; // namespace NetMsgType From 5308c97ccaf0955e5840956bc1636108a43e6f46 Mon Sep 17 00:00:00 2001 From: Jim Posen Date: Mon, 4 May 2020 14:25:18 -0400 Subject: [PATCH 3/3] [test] Add test for cfheaders --- test/functional/p2p_blockfilters.py | 52 ++++++++++++++++++++- test/functional/test_framework/messages.py | 53 ++++++++++++++++++++++ test/functional/test_framework/mininode.py | 3 ++ 3 files changed, 107 insertions(+), 1 deletion(-) diff --git a/test/functional/p2p_blockfilters.py b/test/functional/p2p_blockfilters.py index 4d00a6dc07e..9ff76b4b3d7 100755 --- a/test/functional/p2p_blockfilters.py +++ b/test/functional/p2p_blockfilters.py @@ -5,12 +5,16 @@ """Tests NODE_COMPACT_FILTERS (BIP 157/158). Tests that a node configured with -blockfilterindex and -peerblockfilters can serve -cfcheckpts. +cfheaders and cfcheckpts. """ from test_framework.messages import ( FILTER_TYPE_BASIC, + hash256, msg_getcfcheckpt, + msg_getcfheaders, + ser_uint256, + uint256_from_str, ) from test_framework.mininode import P2PInterface from test_framework.test_framework import BitcoinTestFramework @@ -100,12 +104,45 @@ class CompactFiltersTest(BitcoinTestFramework): [int(header, 16) for header in (stale_cfcheckpt,)] ) + self.log.info("Check that peers can fetch cfheaders on active chain.") + request = msg_getcfheaders( + filter_type=FILTER_TYPE_BASIC, + start_height=1, + stop_hash=int(main_block_hash, 16) + ) + node0.send_and_ping(request) + response = node0.last_message['cfheaders'] + assert_equal(len(response.hashes), 1000) + assert_equal( + compute_last_header(response.prev_header, response.hashes), + int(main_cfcheckpt, 16) + ) + + self.log.info("Check that peers can fetch cfheaders on stale chain.") + request = msg_getcfheaders( + filter_type=FILTER_TYPE_BASIC, + start_height=1, + stop_hash=int(stale_block_hash, 16) + ) + node0.send_and_ping(request) + response = node0.last_message['cfheaders'] + assert_equal(len(response.hashes), 1000) + assert_equal( + compute_last_header(response.prev_header, response.hashes), + int(stale_cfcheckpt, 16) + ) + self.log.info("Requests to node 1 without NODE_COMPACT_FILTERS results in disconnection.") requests = [ msg_getcfcheckpt( filter_type=FILTER_TYPE_BASIC, stop_hash=int(main_block_hash, 16) ), + msg_getcfheaders( + filter_type=FILTER_TYPE_BASIC, + start_height=1000, + stop_hash=int(main_block_hash, 16) + ), ] for request in requests: node1 = self.nodes[1].add_p2p_connection(P2PInterface()) @@ -114,6 +151,12 @@ class CompactFiltersTest(BitcoinTestFramework): self.log.info("Check that invalid requests result in disconnection.") requests = [ + # Requesting too many filter headers results in disconnection. + msg_getcfheaders( + filter_type=FILTER_TYPE_BASIC, + start_height=0, + stop_hash=int(tip_hash, 16) + ), # Requesting unknown filter type results in disconnection. msg_getcfcheckpt( filter_type=255, @@ -130,5 +173,12 @@ class CompactFiltersTest(BitcoinTestFramework): node0.send_message(request) node0.wait_for_disconnect() +def compute_last_header(prev_header, hashes): + """Compute the last filter header from a starting header and a sequence of filter hashes.""" + header = ser_uint256(prev_header) + for filter_hash in hashes: + header = hash256(ser_uint256(filter_hash) + header) + return uint256_from_str(header) + if __name__ == '__main__': CompactFiltersTest().main() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 6c9c8a7397a..d178e795416 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1516,6 +1516,59 @@ class msg_no_witness_blocktxn(msg_blocktxn): def serialize(self): return self.block_transactions.serialize(with_witness=False) +class msg_getcfheaders: + __slots__ = ("filter_type", "start_height", "stop_hash") + msgtype = b"getcfheaders" + + def __init__(self, filter_type, start_height, stop_hash): + self.filter_type = filter_type + self.start_height = start_height + self.stop_hash = stop_hash + + def deserialize(self, f): + self.filter_type = struct.unpack("