Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

p2p: Signal support for compact block filters with NODE_COMPACT_FILTERS #19070

Merged
merged 3 commits into from Aug 13, 2020
Merged

p2p: Signal support for compact block filters with NODE_COMPACT_FILTERS #19070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b3fbc94
Apply cfilters review fixups
jnewbery Jun 1, 2020
132b30d
[net] Signal NODE_COMPACT_FILTERS if we're serving compact filters.
jimpo May 4, 2020
f5c003d
[test] Add test for NODE_COMPACT_FILTER.
jimpo May 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/init.cpp
View file
Open in desktop
Expand Up @@ -996,11 +996,13 @@ bool AppInitParameterInteraction()
}
}

// Basic filters are the only supported filters. The basic filters index must be enabled
// to serve compact filters
if (gArgs.GetBoolArg("-peerblockfilters", DEFAULT_PEERBLOCKFILTERS) &&
g_enabled_filter_types.count(BlockFilterType::BASIC) != 1) {
return InitError(_("Cannot set -peerblockfilters without -blockfilterindex."));
// Signal NODE_COMPACT_FILTERS if peerblockfilters and basic filters index are both enabled.
if (gArgs.GetBoolArg("-peerblockfilters", DEFAULT_PEERBLOCKFILTERS)) {
if (g_enabled_filter_types.count(BlockFilterType::BASIC) != 1) {
return InitError(_("Cannot set -peerblockfilters without -blockfilterindex."));
}

nLocalServices = ServiceFlags(nLocalServices | NODE_COMPACT_FILTERS);
}

// if using block pruning, then disallow txindex
Expand Down
53 changes: 26 additions & 27 deletions src/net_processing.cpp
View file
Open in desktop
Expand Up @@ -1991,7 +1991,7 @@ void static ProcessOrphanTx(CConnman* connman, CTxMemPool& mempool, std::set<uin
*
* 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] peer The peer that we received the request from
* @param[in] chain_params Chain parameters
* @param[in] filter_type The filter type the request is for. Must be basic filters.
* @param[in] start_height The start height for the request
Expand All @@ -2001,19 +2001,19 @@ void static ProcessOrphanTx(CConnman* connman, CTxMemPool& mempool, std::set<uin
* @param[out] filter_index The filter index, if the request can be serviced.
* @return True if the request can be serviced.
*/
static bool PrepareBlockFilterRequest(CNode& pfrom, const CChainParams& chain_params,
static bool PrepareBlockFilterRequest(CNode& peer, const CChainParams& chain_params,
BlockFilterType filter_type, uint32_t start_height,
const uint256& stop_hash, uint32_t max_height_diff,
const CBlockIndex*& stop_index,
BlockFilterIndex*& filter_index)
{
const bool supported_filter_type =
(filter_type == BlockFilterType::BASIC &&
gArgs.GetBoolArg("-peerblockfilters", DEFAULT_PEERBLOCKFILTERS));
(peer.GetLocalServices() & NODE_COMPACT_FILTERS));
if (!supported_filter_type) {
LogPrint(BCLog::NET, "peer %d requested unsupported block filter type: %d\n",
pfrom.GetId(), static_cast<uint8_t>(filter_type));
pfrom.fDisconnect = true;
peer.GetId(), static_cast<uint8_t>(filter_type));
peer.fDisconnect = true;
return false;
}

Expand All @@ -2024,8 +2024,8 @@ static bool PrepareBlockFilterRequest(CNode& pfrom, const CChainParams& chain_pa
// Check that the stop block exists and the peer would be allowed to fetch it.
if (!stop_index || !BlockRequestAllowed(stop_index, chain_params.GetConsensus())) {
LogPrint(BCLog::NET, "peer %d requested invalid block hash: %s\n",
pfrom.GetId(), stop_hash.ToString());
pfrom.fDisconnect = true;
peer.GetId(), stop_hash.ToString());
peer.fDisconnect = true;
return false;
}
}
Expand All @@ -2034,14 +2034,14 @@ static bool PrepareBlockFilterRequest(CNode& pfrom, const CChainParams& chain_pa
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;
peer.GetId(), start_height, stop_height);
peer.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;
peer.GetId(), stop_height - start_height + 1, max_height_diff);
peer.fDisconnect = true;
return false;
}

Expand All @@ -2059,12 +2059,12 @@ static bool PrepareBlockFilterRequest(CNode& pfrom, const CChainParams& chain_pa
*
* 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] peer 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 ProcessGetCFilters(CNode& pfrom, CDataStream& vRecv, const CChainParams& chain_params,
static void ProcessGetCFilters(CNode& peer, CDataStream& vRecv, const CChainParams& chain_params,
CConnman& connman)
{
uint8_t filter_type_ser;
Expand All @@ -2077,23 +2077,22 @@ static void ProcessGetCFilters(CNode& pfrom, CDataStream& vRecv, const CChainPar

const CBlockIndex* stop_index;
BlockFilterIndex* filter_index;
if (!PrepareBlockFilterRequest(pfrom, chain_params, filter_type, start_height, stop_hash,
if (!PrepareBlockFilterRequest(peer, chain_params, filter_type, start_height, stop_hash,
MAX_GETCFILTERS_SIZE, stop_index, filter_index)) {
return;
}

std::vector<BlockFilter> filters;

if (!filter_index->LookupFilterRange(start_height, stop_index, filters)) {
LogPrint(BCLog::NET, "Failed to find block filter in index: filter_type=%s, start_height=%d, stop_hash=%s\n",
BlockFilterTypeName(filter_type), start_height, stop_hash.ToString());
return;
}

for (const auto& filter : filters) {
CSerializedNetMsg msg = CNetMsgMaker(pfrom.GetSendVersion())
CSerializedNetMsg msg = CNetMsgMaker(peer.GetSendVersion())
.Make(NetMsgType::CFILTER, filter);
connman.PushMessage(&pfrom, std::move(msg));
connman.PushMessage(&peer, std::move(msg));
}
}

Expand All @@ -2102,12 +2101,12 @@ static void ProcessGetCFilters(CNode& pfrom, CDataStream& vRecv, const CChainPar
*
* 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] peer 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,
static void ProcessGetCFHeaders(CNode& peer, CDataStream& vRecv, const CChainParams& chain_params,
CConnman& connman)
{
uint8_t filter_type_ser;
Expand All @@ -2120,7 +2119,7 @@ static void ProcessGetCFHeaders(CNode& pfrom, CDataStream& vRecv, const CChainPa

const CBlockIndex* stop_index;
BlockFilterIndex* filter_index;
if (!PrepareBlockFilterRequest(pfrom, chain_params, filter_type, start_height, stop_hash,
if (!PrepareBlockFilterRequest(peer, chain_params, filter_type, start_height, stop_hash,
MAX_GETCFHEADERS_SIZE, stop_index, filter_index)) {
return;
}
Expand All @@ -2143,26 +2142,26 @@ static void ProcessGetCFHeaders(CNode& pfrom, CDataStream& vRecv, const CChainPa
return;
}

CSerializedNetMsg msg = CNetMsgMaker(pfrom.GetSendVersion())
CSerializedNetMsg msg = CNetMsgMaker(peer.GetSendVersion())
.Make(NetMsgType::CFHEADERS,
filter_type_ser,
stop_index->GetBlockHash(),
prev_header,
filter_hashes);
connman.PushMessage(&pfrom, std::move(msg));
connman.PushMessage(&peer, std::move(msg));
}

/**
* Handle a getcfcheckpt 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] peer 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 ProcessGetCFCheckPt(CNode& pfrom, CDataStream& vRecv, const CChainParams& chain_params,
static void ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv, const CChainParams& chain_params,
CConnman& connman)
{
uint8_t filter_type_ser;
Expand All @@ -2174,7 +2173,7 @@ static void ProcessGetCFCheckPt(CNode& pfrom, CDataStream& vRecv, const CChainPa

const CBlockIndex* stop_index;
BlockFilterIndex* filter_index;
if (!PrepareBlockFilterRequest(pfrom, chain_params, filter_type, /*start_height=*/0, stop_hash,
if (!PrepareBlockFilterRequest(peer, chain_params, filter_type, /*start_height=*/0, stop_hash,
/*max_height_diff=*/std::numeric_limits<uint32_t>::max(),
stop_index, filter_index)) {
return;
Expand All @@ -2195,12 +2194,12 @@ static void ProcessGetCFCheckPt(CNode& pfrom, CDataStream& vRecv, const CChainPa
}
}

CSerializedNetMsg msg = CNetMsgMaker(pfrom.GetSendVersion())
CSerializedNetMsg msg = CNetMsgMaker(peer.GetSendVersion())
.Make(NetMsgType::CFCHECKPT,
filter_type_ser,
stop_index->GetBlockHash(),
headers);
connman.PushMessage(&pfrom, std::move(msg));
connman.PushMessage(&peer, std::move(msg));
}

bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool, CConnman* connman, BanMan* banman, const std::atomic<bool>& interruptMsgProc)
Expand Down
1 change: 1 addition & 0 deletions src/protocol.cpp
View file
Open in desktop
Expand Up @@ -213,6 +213,7 @@ static std::string serviceFlagToStr(size_t bit)
case NODE_GETUTXO: return "GETUTXO";
case NODE_BLOOM: return "BLOOM";
case NODE_WITNESS: return "WITNESS";
case NODE_COMPACT_FILTERS: return "COMPACT_FILTERS";
case NODE_NETWORK_LIMITED: return "NETWORK_LIMITED";
// Not using default, so we get warned when a case is missing
}
Expand Down
3 changes: 3 additions & 0 deletions src/protocol.h
View file
Open in desktop
Expand Up @@ -285,6 +285,9 @@ enum ServiceFlags : uint64_t {
// NODE_WITNESS indicates that a node can be asked for blocks and transactions including
// witness data.
NODE_WITNESS = (1 << 3),
// NODE_COMPACT_FILTERS means the node will service basic block filter requests.
// See BIP157 and BIP158 for details on how this is implemented.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's likely implementation will diverge from BIP with regards to error handling, it should be hinted somewhere. Better in release notes than here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm happy to add release notes describing the behavior. I won't do it in this branch unless I need to retouch it (to not invalidate the three ACKs).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jnewbery you just requested an ack from MarcoFalke so I dunno if the ACKs have been invalidated?

NODE_COMPACT_FILTERS = (1 << 6),
// NODE_NETWORK_LIMITED means the same as NODE_NETWORK with the limitation of only
// serving the last 288 (2 day) blocks
// See BIP159 for details on how this is implemented.
Expand Down
13 changes: 11 additions & 2 deletions test/functional/p2p_blockfilters.py
View file
Open in desktop
Expand Up @@ -4,12 +4,13 @@
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Tests NODE_COMPACT_FILTERS (BIP 157/158).

Tests that a node configured with -blockfilterindex and -peerblockfilters can serve
cfheaders and cfcheckpts.
Tests that a node configured with -blockfilterindex and -peerblockfilters signals
NODE_COMPACT_FILTERS and can serve cfilters, cfheaders and cfcheckpts.
"""

from test_framework.messages import (
FILTER_TYPE_BASIC,
NODE_COMPACT_FILTERS,
hash256,
msg_getcfcheckpt,
msg_getcfheaders,
Expand Down Expand Up @@ -70,6 +71,14 @@ def run_test(self):
self.nodes[1].generate(1001)
wait_until(lambda: self.nodes[1].getblockcount() == 2000)

# Check that nodes have signalled NODE_COMPACT_FILTERS correctly.
assert node0.nServices & NODE_COMPACT_FILTERS != 0
assert node1.nServices & NODE_COMPACT_FILTERS == 0

# Check that the localservices is as expected.
assert int(self.nodes[0].getnetworkinfo()['localservices'], 16) & NODE_COMPACT_FILTERS != 0
assert int(self.nodes[1].getnetworkinfo()['localservices'], 16) & NODE_COMPACT_FILTERS == 0

self.log.info("get cfcheckpt on chain to be re-orged out.")
request = msg_getcfcheckpt(
filter_type=FILTER_TYPE_BASIC,
Expand Down
1 change: 1 addition & 0 deletions test/functional/test_framework/messages.py
View file
Open in desktop
Expand Up @@ -49,6 +49,7 @@
NODE_GETUTXO = (1 << 1)
NODE_BLOOM = (1 << 2)
NODE_WITNESS = (1 << 3)
NODE_COMPACT_FILTERS = (1 << 6)
NODE_NETWORK_LIMITED = (1 << 10)

MSG_TX = 1
Expand Down