From 799214b2c8d3a85706bd2db01887bb5e697e872b Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 9 Jun 2024 00:46:06 +0000 Subject: [PATCH 1/8] merge bitcoin#19776: expose high bandwidth mode state via getpeerinfo --- doc/release-notes-19776.md | 9 ++++++++ src/net.cpp | 2 ++ src/net.h | 7 +++++++ src/net_processing.cpp | 7 +++++++ src/rpc/net.cpp | 4 ++++ test/functional/p2p_compactblocks.py | 31 ++++++++++++++++++++++++++++ 6 files changed, 60 insertions(+) create mode 100644 doc/release-notes-19776.md diff --git a/doc/release-notes-19776.md b/doc/release-notes-19776.md new file mode 100644 index 0000000000..beb4e353d7 --- /dev/null +++ b/doc/release-notes-19776.md @@ -0,0 +1,9 @@ +Updated RPCs +------------ + +- The `getpeerinfo` RPC returns two new boolean fields, `bip152_hb_to` and + `bip152_hb_from`, that respectively indicate whether we selected a peer to be + in compact blocks high-bandwidth mode or whether a peer selected us as a + compact blocks high-bandwidth peer. High-bandwidth peers send new block + announcements via a `cmpctblock` message rather than the usual inv/headers + announcements. See BIP 152 for more details. diff --git a/src/net.cpp b/src/net.cpp index d4cecd4758..4387d96564 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -706,6 +706,8 @@ void CNode::CopyStats(CNodeStats& stats) } stats.fInbound = IsInboundConn(); stats.m_manual_connection = IsManualConn(); + X(m_bip152_highbandwidth_to); + X(m_bip152_highbandwidth_from); { LOCK(cs_vSend); X(mapSendBytesPerMsgCmd); diff --git a/src/net.h b/src/net.h index f734900853..13ae676c60 100644 --- a/src/net.h +++ b/src/net.h @@ -284,6 +284,8 @@ public: std::string cleanSubVer; bool fInbound; bool m_manual_connection; + bool m_bip152_highbandwidth_to; + bool m_bip152_highbandwidth_from; int m_starting_height; uint64_t nSendBytes; mapMsgCmdSize mapSendBytesPerMsgCmd; @@ -582,6 +584,11 @@ public: } public: + // We selected peer as (compact blocks) high-bandwidth peer (BIP152) + std::atomic m_bip152_highbandwidth_to{false}; + // Peer selected us as (compact blocks) high-bandwidth peer (BIP152) + std::atomic m_bip152_highbandwidth_from{false}; + /** Whether we should relay transactions to this peer (their version * message did not include fRelay=false and this is not a block-relay-only * connection). This only changes from false to true. It will never change diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f41f00aaa3..d67de85360 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1073,11 +1073,15 @@ void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid) // blocks using compact encodings. m_connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [this, nCMPCTBLOCKVersion](CNode* pnodeStop){ m_connman.PushMessage(pnodeStop, CNetMsgMaker(pnodeStop->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/false, nCMPCTBLOCKVersion)); + // save BIP152 bandwidth state: we select peer to be low-bandwidth + pnodeStop->m_bip152_highbandwidth_to = false; return true; }); lNodesAnnouncingHeaderAndIDs.pop_front(); } m_connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/true, nCMPCTBLOCKVersion)); + // save BIP152 bandwidth state: we select peer to be high-bandwidth + pfrom->m_bip152_highbandwidth_to = true; lNodesAnnouncingHeaderAndIDs.push_back(pfrom->GetId()); return true; }); @@ -3687,6 +3691,9 @@ void PeerManagerImpl::ProcessMessage( State(pfrom.GetId())->fProvidesHeaderAndIDs = true; State(pfrom.GetId())->fPreferHeaderAndIDs = fAnnounceUsingCMPCTBLOCK; State(pfrom.GetId())->fSupportsDesiredCmpctVersion = true; + // save whether peer selects us as BIP152 high-bandwidth peer + // (receiving sendcmpct(1) signals high-bandwidth, sendcmpct(0) low-bandwidth) + pfrom.m_bip152_highbandwidth_from = fAnnounceUsingCMPCTBLOCK; } return; } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 4070f0c83a..9f25c8dfd5 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -145,6 +145,8 @@ static RPCHelpMan getpeerinfo() {RPCResult::Type::NUM, "version", "The peer version, such as 70001"}, {RPCResult::Type::STR, "subver", "The string version"}, {RPCResult::Type::BOOL, "inbound", "Inbound (true) or Outbound (false)"}, + {RPCResult::Type::BOOL, "bip152_hb_to", "Whether we selected peer as (compact blocks) high-bandwidth peer"}, + {RPCResult::Type::BOOL, "bip152_hb_from", "Whether peer selected us as (compact blocks) high-bandwidth peer"}, {RPCResult::Type::BOOL, "addnode", "Whether connection was due to addnode/-connect or if it was an automatic/inbound connection\n" "(DEPRECATED, returned only if the config option -deprecatedrpc=getpeerinfo_addnode is passed)"}, {RPCResult::Type::BOOL, "masternode", "Whether connection was due to masternode connection attempt"}, @@ -244,6 +246,8 @@ static RPCHelpMan getpeerinfo() // their ver message. obj.pushKV("subver", stats.cleanSubVer); obj.pushKV("inbound", stats.fInbound); + obj.pushKV("bip152_hb_to", stats.m_bip152_highbandwidth_to); + obj.pushKV("bip152_hb_from", stats.m_bip152_highbandwidth_from); if (IsDeprecatedRPCEnabled("getpeerinfo_addnode")) { // addnode is deprecated in v21 for removal in v22 obj.pushKV("addnode", stats.m_manual_connection); diff --git a/test/functional/p2p_compactblocks.py b/test/functional/p2p_compactblocks.py index 0867b17aaa..a6ca6a0d33 100755 --- a/test/functional/p2p_compactblocks.py +++ b/test/functional/p2p_compactblocks.py @@ -767,6 +767,34 @@ class CompactBlocksTest(BitcoinTestFramework): stalling_peer.send_and_ping(msg) assert_equal(int(node.getbestblockhash(), 16), block.sha256) + def test_highbandwidth_mode_states_via_getpeerinfo(self): + # create new p2p connection for a fresh state w/o any prior sendcmpct messages sent + hb_test_node = self.nodes[0].add_p2p_connection(TestP2PConn(cmpct_version=1)) + + # assert the RPC getpeerinfo boolean fields `bip152_hb_{to, from}` + # match the given parameters for the last peer of a given node + def assert_highbandwidth_states(node, hb_to, hb_from): + peerinfo = node.getpeerinfo()[-1] + assert_equal(peerinfo['bip152_hb_to'], hb_to) + assert_equal(peerinfo['bip152_hb_from'], hb_from) + + # initially, neither node has selected the other peer as high-bandwidth yet + assert_highbandwidth_states(self.nodes[0], hb_to=False, hb_from=False) + + # peer requests high-bandwidth mode by sending sendcmpct(1) + hb_test_node.send_and_ping(msg_sendcmpct(announce=True, version=1)) + assert_highbandwidth_states(self.nodes[0], hb_to=False, hb_from=True) + + # peer generates a block and sends it to node, which should + # select the peer as high-bandwidth (up to 3 peers according to BIP 152) + block = self.build_block_on_tip(self.nodes[0]) + hb_test_node.send_and_ping(msg_block(block)) + assert_highbandwidth_states(self.nodes[0], hb_to=True, hb_from=True) + + # peer requests low-bandwidth mode by sending sendcmpct(0) + hb_test_node.send_and_ping(msg_sendcmpct(announce=False, version=1)) + assert_highbandwidth_states(self.nodes[0], hb_to=True, hb_from=False) + def run_test(self): # Get the nodes out of IBD self.nodes[0].generate(1) @@ -817,5 +845,8 @@ class CompactBlocksTest(BitcoinTestFramework): self.log.info("Testing invalid index in cmpctblock message...") self.test_invalid_cmpctblock_message() + self.log.info("Testing high-bandwidth mode states via getpeerinfo...") + self.test_highbandwidth_mode_states_via_getpeerinfo() + if __name__ == '__main__': CompactBlocksTest().main() From 2ce481849a2f41cadf2e404c773349d3d1408e5a Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 9 Jun 2024 01:16:47 +0000 Subject: [PATCH 2/8] merge bitcoin#20599: Tolerate sendheaders and sendcmpct messages before verack --- src/net_processing.cpp | 57 +++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index d67de85360..de6375f65c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3532,6 +3532,34 @@ void PeerManagerImpl::ProcessMessage( return; } + if (msg_type == NetMsgType::SENDHEADERS) { + LOCK(cs_main); + State(pfrom.GetId())->fPreferHeaders = true; + return; + } + + if (msg_type == NetMsgType::SENDHEADERS2) { + LOCK(cs_main); + State(pfrom.GetId())->fPreferHeadersCompressed = true; + return; + } + + if (msg_type == NetMsgType::SENDCMPCT) { + bool fAnnounceUsingCMPCTBLOCK = false; + uint64_t nCMPCTBLOCKVersion = 1; + vRecv >> fAnnounceUsingCMPCTBLOCK >> nCMPCTBLOCKVersion; + if (nCMPCTBLOCKVersion == 1) { + LOCK(cs_main); + State(pfrom.GetId())->fProvidesHeaderAndIDs = true; + State(pfrom.GetId())->fPreferHeaderAndIDs = fAnnounceUsingCMPCTBLOCK; + State(pfrom.GetId())->fSupportsDesiredCmpctVersion = true; + // save whether peer selects us as BIP152 high-bandwidth peer + // (receiving sendcmpct(1) signals high-bandwidth, sendcmpct(0) low-bandwidth) + pfrom.m_bip152_highbandwidth_from = fAnnounceUsingCMPCTBLOCK; + } + return; + } + // BIP155 defines feature negotiation of addrv2 and sendaddrv2, which must happen // between VERSION and VERACK. if (msg_type == NetMsgType::SENDADDRV2) { @@ -3670,35 +3698,6 @@ void PeerManagerImpl::ProcessMessage( return; } - if (msg_type == NetMsgType::SENDHEADERS) { - LOCK(cs_main); - State(pfrom.GetId())->fPreferHeaders = true; - return; - } - - if (msg_type == NetMsgType::SENDHEADERS2) { - LOCK(cs_main); - State(pfrom.GetId())->fPreferHeadersCompressed = true; - return; - } - - if (msg_type == NetMsgType::SENDCMPCT) { - bool fAnnounceUsingCMPCTBLOCK = false; - uint64_t nCMPCTBLOCKVersion = 1; - vRecv >> fAnnounceUsingCMPCTBLOCK >> nCMPCTBLOCKVersion; - if (nCMPCTBLOCKVersion == 1) { - LOCK(cs_main); - State(pfrom.GetId())->fProvidesHeaderAndIDs = true; - State(pfrom.GetId())->fPreferHeaderAndIDs = fAnnounceUsingCMPCTBLOCK; - State(pfrom.GetId())->fSupportsDesiredCmpctVersion = true; - // save whether peer selects us as BIP152 high-bandwidth peer - // (receiving sendcmpct(1) signals high-bandwidth, sendcmpct(0) low-bandwidth) - pfrom.m_bip152_highbandwidth_from = fAnnounceUsingCMPCTBLOCK; - } - return; - } - - if (msg_type == NetMsgType::SENDDSQUEUE) { bool b; From 73b8f84fdb2bdf4df2f87792b5ecac93266e3bd3 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 3 Jun 2021 16:38:59 -0400 Subject: [PATCH 3/8] merge bitcoin#22147: p2p: Protect last outbound HB compact block peer --- src/net_processing.cpp | 15 ++++ test/functional/p2p_compactblocks_hb.py | 97 +++++++++++++++++++++++++ test/functional/test_runner.py | 1 + 3 files changed, 113 insertions(+) create mode 100755 test/functional/p2p_compactblocks_hb.py diff --git a/src/net_processing.cpp b/src/net_processing.cpp index de6375f65c..c0ecd173c5 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1058,12 +1058,27 @@ void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid) return; } if (nodestate->fProvidesHeaderAndIDs) { + int num_outbound_hb_peers = 0; for (std::list::iterator it = lNodesAnnouncingHeaderAndIDs.begin(); it != lNodesAnnouncingHeaderAndIDs.end(); it++) { if (*it == nodeid) { lNodesAnnouncingHeaderAndIDs.erase(it); lNodesAnnouncingHeaderAndIDs.push_back(nodeid); return; } + CNodeState *state = State(*it); + if (state != nullptr && !state->m_is_inbound) ++num_outbound_hb_peers; + } + if (nodestate->m_is_inbound) { + // If we're adding an inbound HB peer, make sure we're not removing + // our last outbound HB peer in the process. + if (lNodesAnnouncingHeaderAndIDs.size() >= 3 && num_outbound_hb_peers == 1) { + CNodeState *remove_node = State(lNodesAnnouncingHeaderAndIDs.front()); + if (remove_node != nullptr && !remove_node->m_is_inbound) { + // Put the HB outbound peer in the second slot, so that it + // doesn't get removed. + std::swap(lNodesAnnouncingHeaderAndIDs.front(), *std::next(lNodesAnnouncingHeaderAndIDs.begin())); + } + } } m_connman.ForNode(nodeid, [this](CNode* pfrom){ LockAssertion lock(::cs_main); diff --git a/test/functional/p2p_compactblocks_hb.py b/test/functional/p2p_compactblocks_hb.py new file mode 100755 index 0000000000..a3d30a6f04 --- /dev/null +++ b/test/functional/p2p_compactblocks_hb.py @@ -0,0 +1,97 @@ +#!/usr/bin/env python3 +# Copyright (c) 2021 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test compact blocks HB selection logic.""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + + +class CompactBlocksConnectionTest(BitcoinTestFramework): + """Test class for verifying selection of HB peer connections.""" + + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 6 + + def peer_info(self, from_node, to_node): + """Query from_node for its getpeerinfo about to_node.""" + for peerinfo in self.nodes[from_node].getpeerinfo(): + if "(testnode%i)" % to_node in peerinfo['subver']: + return peerinfo + return None + + def setup_network(self): + self.setup_nodes() + # Start network with everyone disconnected + self.sync_all() + + def relay_block_through(self, peer): + """Relay a new block through peer peer, and return HB status between 1 and [2,3,4,5].""" + self.connect_nodes(peer, 0) + self.nodes[0].generate(1) + self.sync_blocks() + self.disconnect_nodes(peer, 0) + status_to = [self.peer_info(1, i)['bip152_hb_to'] for i in range(2, 6)] + status_from = [self.peer_info(i, 1)['bip152_hb_from'] for i in range(2, 6)] + assert_equal(status_to, status_from) + return status_to + + def run_test(self): + self.log.info("Testing reserved high-bandwidth mode slot for outbound peer...") + + # Connect everyone to node 0, and mine some blocks to get all nodes out of IBD. + for i in range(1, 6): + self.connect_nodes(i, 0) + self.nodes[0].generate(2) + self.sync_blocks() + for i in range(1, 6): + self.disconnect_nodes(i, 0) + + # Construct network topology: + # - Node 0 is the block producer + # - Node 1 is the "target" node being tested + # - Nodes 2-5 are intermediaries. + # - Node 1 has an outbound connection to node 2 + # - Node 1 has inbound connections from nodes 3-5 + self.connect_nodes(3, 1) + self.connect_nodes(4, 1) + self.connect_nodes(5, 1) + self.connect_nodes(1, 2) + + # Mine blocks subsequently relaying through nodes 3,4,5 (inbound to node 1) + for nodeid in range(3, 6): + status = self.relay_block_through(nodeid) + assert_equal(status, [False, nodeid >= 3, nodeid >= 4, nodeid >= 5]) + + # And again through each. This should not change HB status. + for nodeid in range(3, 6): + status = self.relay_block_through(nodeid) + assert_equal(status, [False, True, True, True]) + + # Now relay one block through peer 2 (outbound from node 1), so it should take HB status + # from one of the inbounds. + status = self.relay_block_through(2) + assert_equal(status[0], True) + assert_equal(sum(status), 3) + + # Now relay again through nodes 3,4,5. Since 2 is outbound, it should remain HB. + for nodeid in range(3, 6): + status = self.relay_block_through(nodeid) + assert status[0] + assert status[nodeid - 2] + assert_equal(sum(status), 3) + + # Reconnect peer 2, and retry. Now the three inbounds should be HB again. + self.disconnect_nodes(1, 2) + self.connect_nodes(1, 2) + for nodeid in range(3, 6): + status = self.relay_block_through(nodeid) + assert not status[0] + assert status[nodeid - 2] + assert_equal(status, [False, True, True, True]) + + +if __name__ == '__main__': + CompactBlocksConnectionTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 85d9c9a8f9..0f51e7a596 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -194,6 +194,7 @@ BASE_SCRIPTS = [ 'p2p_addrv2_relay.py', 'wallet_groups.py --legacy-wallet', 'wallet_groups.py --descriptors', + 'p2p_compactblocks_hb.py', 'p2p_disconnect_ban.py', 'feature_addressindex.py', 'feature_timestampindex.py', From f4ce573538c46d5f07e81b2df0b0c945961608a3 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 9 Jun 2024 00:41:34 +0000 Subject: [PATCH 4/8] merge bitcoin#22340: Use legacy relaying to download blocks in blocks-only mode --- src/net_processing.cpp | 12 +- .../p2p_compactblocks_blocksonly.py | 129 ++++++++++++++++++ test/functional/test_runner.py | 1 + 3 files changed, 141 insertions(+), 1 deletion(-) create mode 100755 test/functional/p2p_compactblocks_blocksonly.py diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c0ecd173c5..a53b116d40 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1052,6 +1052,12 @@ bool PeerManagerImpl::MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, co void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid) { AssertLockHeld(cs_main); + + // Never request high-bandwidth mode from peers if we're blocks-only. Our + // mempool will not contain the transactions necessary to reconstruct the + // compact block. + if (m_ignore_incoming_txs) return; + CNodeState* nodestate = State(nodeid); if (!nodestate || !nodestate->fSupportsDesiredCmpctVersion) { // Never ask from peers who can't provide desired version. @@ -2897,7 +2903,11 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer, pindexLast->GetBlockHash().ToString(), pindexLast->nHeight); } if (vGetData.size() > 0) { - if (nodestate->fSupportsDesiredCmpctVersion && vGetData.size() == 1 && mapBlocksInFlight.size() == 1 && pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) { + if (!m_ignore_incoming_txs && + nodestate->fSupportsDesiredCmpctVersion && + vGetData.size() == 1 && + mapBlocksInFlight.size() == 1 && + pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) { // In any case, we want to download using a compact block, not a regular one vGetData[0] = CInv(MSG_CMPCT_BLOCK, vGetData[0].hash); } diff --git a/test/functional/p2p_compactblocks_blocksonly.py b/test/functional/p2p_compactblocks_blocksonly.py new file mode 100755 index 0000000000..0d4d8ce22f --- /dev/null +++ b/test/functional/p2p_compactblocks_blocksonly.py @@ -0,0 +1,129 @@ +#!/usr/bin/env python3 +# Copyright (c) 2021-2021 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test that a node in blocksonly mode does not request compact blocks.""" + +from test_framework.messages import ( + MSG_BLOCK, + MSG_CMPCT_BLOCK, + CBlock, + CBlockHeader, + CInv, + from_hex, + msg_block, + msg_getdata, + msg_headers, + msg_sendcmpct, +) +from test_framework.p2p import P2PInterface +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + + +class P2PCompactBlocksBlocksOnly(BitcoinTestFramework): + def set_test_params(self): + self.extra_args = [["-blocksonly"], [], [], []] + self.num_nodes = 4 + + def setup_network(self): + self.setup_nodes() + # Start network with everyone disconnected + self.sync_all() + + def build_block_on_tip(self): + blockhash = self.nodes[2].generate(1)[0] + block_hex = self.nodes[2].getblock(blockhash=blockhash, verbosity=0) + block = from_hex(CBlock(), block_hex) + block.rehash() + return block + + def run_test(self): + # Nodes will only request hb compact blocks mode when they're out of IBD + for node in self.nodes: + assert not node.getblockchaininfo()['initialblockdownload'] + + p2p_conn_blocksonly = self.nodes[0].add_p2p_connection(P2PInterface()) + p2p_conn_high_bw = self.nodes[1].add_p2p_connection(P2PInterface()) + p2p_conn_low_bw = self.nodes[3].add_p2p_connection(P2PInterface()) + for conn in [p2p_conn_blocksonly, p2p_conn_high_bw, p2p_conn_low_bw]: + assert_equal(conn.message_count['sendcmpct'], 1) + conn.send_and_ping(msg_sendcmpct(announce=False, version=1)) + + # Nodes: + # 0 -> blocksonly + # 1 -> high bandwidth + # 2 -> miner + # 3 -> low bandwidth + # + # Topology: + # p2p_conn_blocksonly ---> node0 + # p2p_conn_high_bw ---> node1 + # p2p_conn_low_bw ---> node3 + # node2 (no connections) + # + # node2 produces blocks that are passed to the rest of the nodes + # through the respective p2p connections. + + self.log.info("Test that -blocksonly nodes do not select peers for BIP152 high bandwidth mode") + + block0 = self.build_block_on_tip() + + # A -blocksonly node should not request BIP152 high bandwidth mode upon + # receiving a new valid block at the tip. + p2p_conn_blocksonly.send_and_ping(msg_block(block0)) + assert_equal(int(self.nodes[0].getbestblockhash(), 16), block0.sha256) + assert_equal(p2p_conn_blocksonly.message_count['sendcmpct'], 1) + assert_equal(p2p_conn_blocksonly.last_message['sendcmpct'].announce, False) + + # A normal node participating in transaction relay should request BIP152 + # high bandwidth mode upon receiving a new valid block at the tip. + p2p_conn_high_bw.send_and_ping(msg_block(block0)) + assert_equal(int(self.nodes[1].getbestblockhash(), 16), block0.sha256) + p2p_conn_high_bw.wait_until(lambda: p2p_conn_high_bw.message_count['sendcmpct'] == 2) + assert_equal(p2p_conn_high_bw.last_message['sendcmpct'].announce, True) + + # Don't send a block from the p2p_conn_low_bw so the low bandwidth node + # doesn't select it for BIP152 high bandwidth relay. + self.nodes[3].submitblock(block0.serialize().hex()) + + self.log.info("Test that -blocksonly nodes send getdata(BLOCK) instead" + " of getdata(CMPCT) in BIP152 low bandwidth mode") + + block1 = self.build_block_on_tip() + + p2p_conn_blocksonly.send_message(msg_headers(headers=[CBlockHeader(block1)])) + p2p_conn_blocksonly.sync_send_with_ping() + assert_equal(p2p_conn_blocksonly.last_message['getdata'].inv, [CInv(MSG_BLOCK, block1.sha256)]) + + p2p_conn_high_bw.send_message(msg_headers(headers=[CBlockHeader(block1)])) + p2p_conn_high_bw.sync_send_with_ping() + assert_equal(p2p_conn_high_bw.last_message['getdata'].inv, [CInv(MSG_CMPCT_BLOCK, block1.sha256)]) + + self.log.info("Test that getdata(CMPCT) is still sent on BIP152 low bandwidth connections" + " when no -blocksonly nodes are involved") + + p2p_conn_low_bw.send_and_ping(msg_headers(headers=[CBlockHeader(block1)])) + p2p_conn_low_bw.sync_with_ping() + assert_equal(p2p_conn_low_bw.last_message['getdata'].inv, [CInv(MSG_CMPCT_BLOCK, block1.sha256)]) + + self.log.info("Test that -blocksonly nodes still serve compact blocks") + + def test_for_cmpctblock(block): + if 'cmpctblock' not in p2p_conn_blocksonly.last_message: + return False + return p2p_conn_blocksonly.last_message['cmpctblock'].header_and_shortids.header.rehash() == block.sha256 + + p2p_conn_blocksonly.send_message(msg_getdata([CInv(MSG_CMPCT_BLOCK, block0.sha256)])) + p2p_conn_blocksonly.wait_until(lambda: test_for_cmpctblock(block0)) + + # Request BIP152 high bandwidth mode from the -blocksonly node. + p2p_conn_blocksonly.send_and_ping(msg_sendcmpct(announce=True, version=1)) + + block2 = self.build_block_on_tip() + self.nodes[0].submitblock(block1.serialize().hex()) + self.nodes[0].submitblock(block2.serialize().hex()) + p2p_conn_blocksonly.wait_until(lambda: test_for_cmpctblock(block2)) + +if __name__ == '__main__': + P2PCompactBlocksBlocksOnly().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 0f51e7a596..ae8cd8ee2c 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -273,6 +273,7 @@ BASE_SCRIPTS = [ 'wallet_listdescriptors.py --descriptors', 'p2p_leak.py', 'p2p_compactblocks.py', + 'p2p_compactblocks_blocksonly.py', 'p2p_connect_to_devnet.py', 'feature_sporks.py', 'rpc_getblockstats.py', From 6274a571b7b69eb78329bdea2347b242b36492ae Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 8 Jun 2024 23:03:52 +0000 Subject: [PATCH 5/8] merge bitcoin#20799: Only support version 2 compact blocks --- src/net_processing.cpp | 147 +++++++++++++-------------- test/functional/p2p_compactblocks.py | 71 ++++++------- 2 files changed, 97 insertions(+), 121 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a53b116d40..f95825f5a6 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -183,6 +183,8 @@ static constexpr double MAX_ADDR_RATE_PER_SECOND{0.1}; * based increments won't go above this, but the MAX_ADDR_TO_SEND increment following GETADDR * is exempt from this limit). */ static constexpr size_t MAX_ADDR_PROCESSING_TOKEN_BUCKET{MAX_ADDR_TO_SEND}; +/** The compactblocks version we support. See BIP 152. */ +static constexpr uint64_t CMPCTBLOCKS_VERSION{1}; struct COrphanTx { // When modifying, adapt the copy of this definition in tests/DoS_tests. @@ -777,15 +779,10 @@ struct CNodeState { bool fPreferHeaders{false}; //! Whether this peer wants invs or compressed headers (when possible) for block announcements. bool fPreferHeadersCompressed{false}; - //! Whether this peer wants invs or cmpctblocks (when possible) for block announcements. - bool fPreferHeaderAndIDs{false}; - //! Whether this peer will send us cmpctblocks if we request them - bool fProvidesHeaderAndIDs{false}; - /** - * If we've announced last version to this peer: whether the peer sends last version in cmpctblocks/blocktxns, - * otherwise: whether this peer sends non-last version in cmpctblocks/blocktxns. - */ - bool fSupportsDesiredCmpctVersion{false}; + /** Whether this peer wants invs or cmpctblocks (when possible) for block announcements. */ + bool m_requested_hb_cmpctblocks{false}; + /** Whether this peer will send us cmpctblocks if we request them. */ + bool m_provides_cmpctblocks{false}; /** State used to enforce CHAIN_SYNC_TIMEOUT and EXTRA_PEER_CHECK_INTERVAL logic. * @@ -1059,54 +1056,52 @@ void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid) if (m_ignore_incoming_txs) return; CNodeState* nodestate = State(nodeid); - if (!nodestate || !nodestate->fSupportsDesiredCmpctVersion) { - // Never ask from peers who can't provide desired version. + if (!nodestate || !nodestate->m_provides_cmpctblocks) { + // Don't request compact blocks if the peer has not signalled support return; } - if (nodestate->fProvidesHeaderAndIDs) { - int num_outbound_hb_peers = 0; - for (std::list::iterator it = lNodesAnnouncingHeaderAndIDs.begin(); it != lNodesAnnouncingHeaderAndIDs.end(); it++) { - if (*it == nodeid) { - lNodesAnnouncingHeaderAndIDs.erase(it); - lNodesAnnouncingHeaderAndIDs.push_back(nodeid); - return; - } - CNodeState *state = State(*it); - if (state != nullptr && !state->m_is_inbound) ++num_outbound_hb_peers; + + int num_outbound_hb_peers = 0; + for (std::list::iterator it = lNodesAnnouncingHeaderAndIDs.begin(); it != lNodesAnnouncingHeaderAndIDs.end(); it++) { + if (*it == nodeid) { + lNodesAnnouncingHeaderAndIDs.erase(it); + lNodesAnnouncingHeaderAndIDs.push_back(nodeid); + return; } - if (nodestate->m_is_inbound) { - // If we're adding an inbound HB peer, make sure we're not removing - // our last outbound HB peer in the process. - if (lNodesAnnouncingHeaderAndIDs.size() >= 3 && num_outbound_hb_peers == 1) { - CNodeState *remove_node = State(lNodesAnnouncingHeaderAndIDs.front()); - if (remove_node != nullptr && !remove_node->m_is_inbound) { - // Put the HB outbound peer in the second slot, so that it - // doesn't get removed. - std::swap(lNodesAnnouncingHeaderAndIDs.front(), *std::next(lNodesAnnouncingHeaderAndIDs.begin())); - } - } - } - m_connman.ForNode(nodeid, [this](CNode* pfrom){ - LockAssertion lock(::cs_main); - uint64_t nCMPCTBLOCKVersion = 1; - if (lNodesAnnouncingHeaderAndIDs.size() >= 3) { - // As per BIP152, we only get 3 of our peers to announce - // blocks using compact encodings. - m_connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [this, nCMPCTBLOCKVersion](CNode* pnodeStop){ - m_connman.PushMessage(pnodeStop, CNetMsgMaker(pnodeStop->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/false, nCMPCTBLOCKVersion)); - // save BIP152 bandwidth state: we select peer to be low-bandwidth - pnodeStop->m_bip152_highbandwidth_to = false; - return true; - }); - lNodesAnnouncingHeaderAndIDs.pop_front(); - } - m_connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/true, nCMPCTBLOCKVersion)); - // save BIP152 bandwidth state: we select peer to be high-bandwidth - pfrom->m_bip152_highbandwidth_to = true; - lNodesAnnouncingHeaderAndIDs.push_back(pfrom->GetId()); - return true; - }); + CNodeState *state = State(*it); + if (state != nullptr && !state->m_is_inbound) ++num_outbound_hb_peers; } + if (nodestate->m_is_inbound) { + // If we're adding an inbound HB peer, make sure we're not removing + // our last outbound HB peer in the process. + if (lNodesAnnouncingHeaderAndIDs.size() >= 3 && num_outbound_hb_peers == 1) { + CNodeState *remove_node = State(lNodesAnnouncingHeaderAndIDs.front()); + if (remove_node != nullptr && !remove_node->m_is_inbound) { + // Put the HB outbound peer in the second slot, so that it + // doesn't get removed. + std::swap(lNodesAnnouncingHeaderAndIDs.front(), *std::next(lNodesAnnouncingHeaderAndIDs.begin())); + } + } + } + m_connman.ForNode(nodeid, [this](CNode* pfrom) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + AssertLockHeld(::cs_main); + if (lNodesAnnouncingHeaderAndIDs.size() >= 3) { + // As per BIP152, we only get 3 of our peers to announce + // blocks using compact encodings. + m_connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [this](CNode* pnodeStop){ + m_connman.PushMessage(pnodeStop, CNetMsgMaker(pnodeStop->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION)); + // save BIP152 bandwidth state: we select peer to be low-bandwidth + pnodeStop->m_bip152_highbandwidth_to = false; + return true; + }); + lNodesAnnouncingHeaderAndIDs.pop_front(); + } + m_connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/true, /*version=*/CMPCTBLOCKS_VERSION)); + // save BIP152 bandwidth state: we select peer to be high-bandwidth + pfrom->m_bip152_highbandwidth_to = true; + lNodesAnnouncingHeaderAndIDs.push_back(pfrom->GetId()); + return true; + }); } bool PeerManagerImpl::TipMayBeStale() @@ -2024,9 +2019,7 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha CNodeState &state = *State(pnode->GetId()); // If the peer has, or we announced to them the previous block already, // but we don't think they have this one, go ahead and announce it - if (state.fPreferHeaderAndIDs && - !PeerHasHeader(&state, pindex) && PeerHasHeader(&state, pindex->pprev)) { - + if (state.m_requested_hb_cmpctblocks && !PeerHasHeader(&state, pindex) && PeerHasHeader(&state, pindex->pprev)) { LogPrint(BCLog::NET, "%s sending header-and-ids %s to peer=%d\n", "PeerManager::NewPoWValidBlock", hashBlock.ToString(), pnode->GetId()); m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::CMPCTBLOCK, *pcmpctblock)); @@ -2757,7 +2750,6 @@ void PeerManagerImpl::SendBlockTransactions(CNode& pfrom, const CBlock& block, c } resp.txn[i] = block.vtx[req.indexes[i]]; } - LOCK(cs_main); CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCKTXN, resp)); } @@ -2904,7 +2896,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer, } if (vGetData.size() > 0) { if (!m_ignore_incoming_txs && - nodestate->fSupportsDesiredCmpctVersion && + nodestate->m_provides_cmpctblocks && vGetData.size() == 1 && mapBlocksInFlight.size() == 1 && pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) { @@ -3533,14 +3525,12 @@ void PeerManagerImpl::ProcessMessage( m_connman.PushMessage(&pfrom, msgMaker.Make((pfrom.nServices & NODE_HEADERS_COMPRESSED) ? NetMsgType::SENDHEADERS2 : NetMsgType::SENDHEADERS)); if (pfrom.CanRelay()) { - // Tell our peer we are willing to provide version-1 cmpctblocks + // Tell our peer we are willing to provide version 1 cmpctblocks. // However, we do not request new block announcements using // cmpctblock messages. // We send this to non-NODE NETWORK peers as well, because // they may wish to request compact blocks from us - bool fAnnounceUsingCMPCTBLOCK = false; - uint64_t nCMPCTBLOCKVersion = 1; - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion)); + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION)); } if (!fBlocksOnly) { @@ -3570,18 +3560,19 @@ void PeerManagerImpl::ProcessMessage( } if (msg_type == NetMsgType::SENDCMPCT) { - bool fAnnounceUsingCMPCTBLOCK = false; - uint64_t nCMPCTBLOCKVersion = 1; - vRecv >> fAnnounceUsingCMPCTBLOCK >> nCMPCTBLOCKVersion; - if (nCMPCTBLOCKVersion == 1) { - LOCK(cs_main); - State(pfrom.GetId())->fProvidesHeaderAndIDs = true; - State(pfrom.GetId())->fPreferHeaderAndIDs = fAnnounceUsingCMPCTBLOCK; - State(pfrom.GetId())->fSupportsDesiredCmpctVersion = true; - // save whether peer selects us as BIP152 high-bandwidth peer - // (receiving sendcmpct(1) signals high-bandwidth, sendcmpct(0) low-bandwidth) - pfrom.m_bip152_highbandwidth_from = fAnnounceUsingCMPCTBLOCK; - } + bool sendcmpct_hb{false}; + uint64_t sendcmpct_version{0}; + vRecv >> sendcmpct_hb >> sendcmpct_version; + + if (sendcmpct_version != CMPCTBLOCKS_VERSION) return; + + LOCK(cs_main); + CNodeState *nodestate = State(pfrom.GetId()); + nodestate->m_provides_cmpctblocks = true; + nodestate->m_requested_hb_cmpctblocks = sendcmpct_hb; + // save whether peer selects us as BIP152 high-bandwidth peer + // (receiving sendcmpct(1) signals high-bandwidth, sendcmpct(0) low-bandwidth) + pfrom.m_bip152_highbandwidth_from = sendcmpct_hb; return; } @@ -3976,9 +3967,7 @@ void PeerManagerImpl::ProcessMessage( // expensive disk reads, because it will require the peer to // actually receive all the data read from disk over the network. LogPrint(BCLog::NET, "Peer %d sent us a getblocktxn for a block > %i deep\n", pfrom.GetId(), MAX_BLOCKTXN_DEPTH); - CInv inv; - WITH_LOCK(cs_main, inv.type = MSG_BLOCK); - inv.hash = req.blockhash; + CInv inv{MSG_BLOCK, req.blockhash}; WITH_LOCK(peer->m_getdata_requests_mutex, peer->m_getdata_requests.push_back(inv)); // The message processing loop will go around again (without pausing) and we'll respond then (without cs_main) return; @@ -5449,7 +5438,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) LOCK(peer->m_block_inv_mutex); std::vector vHeaders; bool fRevertToInv = ((!state.fPreferHeaders && !state.fPreferHeadersCompressed && - (!state.fPreferHeaderAndIDs || peer->m_blocks_for_headers_relay.size() > 1)) || + (!state.m_requested_hb_cmpctblocks || peer->m_blocks_for_headers_relay.size() > 1)) || peer->m_blocks_for_headers_relay.size() > MAX_BLOCKS_TO_ANNOUNCE); const CBlockIndex *pBestIndex = nullptr; // last header queued for delivery ProcessBlockAvailability(pto->GetId()); // ensure pindexBestKnownBlock is up-to-date @@ -5511,7 +5500,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) } } if (!fRevertToInv && !vHeaders.empty()) { - if (vHeaders.size() == 1 && state.fPreferHeaderAndIDs) { + if (vHeaders.size() == 1 && state.m_requested_hb_cmpctblocks) { // We only send up to 1 block as header-and-ids, as otherwise // probably means we're doing an initial-ish-sync or they're slow LogPrint(BCLog::NET, "%s sending header-and-ids %s to peer=%d\n", __func__, diff --git a/test/functional/p2p_compactblocks.py b/test/functional/p2p_compactblocks.py index a6ca6a0d33..839a034d71 100755 --- a/test/functional/p2p_compactblocks.py +++ b/test/functional/p2p_compactblocks.py @@ -60,7 +60,7 @@ from test_framework.util import ( # TestP2PConn: A peer we use to send messages to dashd, and store responses. class TestP2PConn(P2PInterface): - def __init__(self, cmpct_version): + def __init__(self): super().__init__() self.last_sendcmpct = [] self.block_announced = False @@ -68,7 +68,6 @@ class TestP2PConn(P2PInterface): # This is for synchronizing the p2p message traffic, # so we can eg wait until a particular block is announced. self.announced_blockhashes = set() - self.cmpct_version = cmpct_version def on_sendcmpct(self, message): self.last_sendcmpct.append(message) @@ -177,14 +176,13 @@ class CompactBlocksTest(BitcoinTestFramework): # Test "sendcmpct" (between peers with the same version): # - No compact block announcements unless sendcmpct is sent. + # - If sendcmpct is sent with version < 1, the message is ignored. + # - If sendcmpct is sent with version > 1, the message is ignored. # - If sendcmpct is sent with boolean 0, then block announcements are not # made with compact blocks. # - If sendcmpct is then sent with boolean 1, then new block announcements # are made with compact blocks. - # If old_node is passed in, request compact blocks with version=preferred-1 - # and verify that it receives block announcements via compact block. - def test_sendcmpct(self, test_node, old_node=None): - preferred_version = test_node.cmpct_version + def test_sendcmpct(self, test_node): node = self.nodes[0] # Make sure we get a SENDCMPCT message from our peer @@ -192,10 +190,8 @@ class CompactBlocksTest(BitcoinTestFramework): return (len(test_node.last_sendcmpct) > 0) test_node.wait_until(received_sendcmpct, timeout=30) with p2p_lock: - # Check that the first version received is the preferred one - assert_equal(test_node.last_sendcmpct[0].version, preferred_version) - # And that we receive versions down to 1. - assert_equal(test_node.last_sendcmpct[-1].version, 1) + # Check that version 1 is received. + assert_equal(test_node.last_sendcmpct[0].version, 1) test_node.last_sendcmpct = [] tip = int(node.getbestblockhash(), 16) @@ -223,22 +219,29 @@ class CompactBlocksTest(BitcoinTestFramework): # Before each test, sync the headers chain. test_node.request_headers_and_sync(locator=[tip]) + # Now try a SENDCMPCT message with too-low version + test_node.send_and_ping(msg_sendcmpct(announce=True, version=0)) + check_announcement_of_new_block(node, test_node, lambda p: "cmpctblock" not in p.last_message) + + # Headers sync before next test. + test_node.request_headers_and_sync(locator=[tip]) + # Now try a SENDCMPCT message with too-high version - test_node.send_and_ping(msg_sendcmpct(announce=True, version=preferred_version+1)) + test_node.send_and_ping(msg_sendcmpct(announce=True, version=2)) check_announcement_of_new_block(node, test_node, lambda p: "cmpctblock" not in p.last_message) # Headers sync before next test. test_node.request_headers_and_sync(locator=[tip]) # Now try a SENDCMPCT message with valid version, but announce=False - test_node.send_and_ping(msg_sendcmpct(announce=False, version=preferred_version)) + test_node.send_and_ping(msg_sendcmpct(announce=False, version=1)) check_announcement_of_new_block(node, test_node, lambda p: "cmpctblock" not in p.last_message) # Headers sync before next test. test_node.request_headers_and_sync(locator=[tip]) # Finally, try a SENDCMPCT message with announce=True - test_node.send_and_ping(msg_sendcmpct(announce=True, version=preferred_version)) + test_node.send_and_ping(msg_sendcmpct(announce=True, version=1)) check_announcement_of_new_block(node, test_node, lambda p: "cmpctblock" in p.last_message) # Try one more time (no headers sync should be needed!) @@ -249,23 +252,13 @@ class CompactBlocksTest(BitcoinTestFramework): check_announcement_of_new_block(node, test_node, lambda p: "cmpctblock" in p.last_message) # Try one more time, after sending a version-1, announce=false message. - test_node.send_and_ping(msg_sendcmpct(announce=False, version=preferred_version-1)) + test_node.send_and_ping(msg_sendcmpct(announce=False, version=0)) check_announcement_of_new_block(node, test_node, lambda p: "cmpctblock" in p.last_message) # Now turn off announcements - test_node.send_and_ping(msg_sendcmpct(announce=False, version=preferred_version)) + test_node.send_and_ping(msg_sendcmpct(announce=False, version=1)) check_announcement_of_new_block(node, test_node, lambda p: "cmpctblock" not in p.last_message and "headers" in p.last_message) - # This code should be enabled after increasing cmctblk version - to_validate = False - if to_validate and old_node is not None: - # Verify that a peer using an older protocol version can receive - # announcements from this node. - old_node.send_and_ping(msg_sendcmpct(announce=True, version=preferred_version-1)) - # Header sync - old_node.request_headers_and_sync(locator=[tip]) - check_announcement_of_new_block(node, old_node, lambda p: "cmpctblock" in p.last_message) - # This test actually causes dashd to (reasonably!) disconnect us, so do this last. def test_invalid_cmpctblock_message(self): self.nodes[0].generate(COINBASE_MATURITY + 1) @@ -283,7 +276,6 @@ class CompactBlocksTest(BitcoinTestFramework): # Compare the generated shortids to what we expect based on BIP 152, given # dashd's choice of nonce. def test_compactblock_construction(self, test_node): - version = test_node.cmpct_version node = self.nodes[0] # Generate a bunch of transactions. node.generate(COINBASE_MATURITY + 1) @@ -321,7 +313,7 @@ class CompactBlocksTest(BitcoinTestFramework): assert "cmpctblock" in test_node.last_message # Convert the on-the-wire representation to absolute indexes header_and_shortids = HeaderAndShortIDs(test_node.last_message["cmpctblock"].header_and_shortids) - self.check_compactblock_construction_from_block(version, header_and_shortids, block_hash, block) + self.check_compactblock_construction_from_block(header_and_shortids, block_hash, block) # Now fetch the compact block using a normal non-announce getdata test_node.clear_block_announcement() @@ -336,9 +328,9 @@ class CompactBlocksTest(BitcoinTestFramework): assert "cmpctblock" in test_node.last_message # Convert the on-the-wire representation to absolute indexes header_and_shortids = HeaderAndShortIDs(test_node.last_message["cmpctblock"].header_and_shortids) - self.check_compactblock_construction_from_block(version, header_and_shortids, block_hash, block) + self.check_compactblock_construction_from_block(header_and_shortids, block_hash, block) - def check_compactblock_construction_from_block(self, version, header_and_shortids, block_hash, block): + def check_compactblock_construction_from_block(self, header_and_shortids, block_hash, block): # Check that we got the right block! header_and_shortids.header.calc_sha256() assert_equal(header_and_shortids.header.sha256, block_hash) @@ -606,7 +598,7 @@ class CompactBlocksTest(BitcoinTestFramework): assert "blocktxn" not in test_node.last_message # Request with out-of-bounds tx index results in disconnect - bad_peer = self.nodes[0].add_p2p_connection(TestP2PConn(cmpct_version=1)) + bad_peer = self.nodes[0].add_p2p_connection(TestP2PConn()) block_hash = node.getblockhash(chain_height) block = from_hex(CBlock(), node.getblock(block_hash, False)) msg.block_txn_request = BlockTransactionsRequest(int(block_hash, 16), [len(block.vtx)]) @@ -717,7 +709,7 @@ class CompactBlocksTest(BitcoinTestFramework): node = self.nodes[0] tip = node.getbestblockhash() peer.get_headers(locator=[int(tip, 16)], hashstop=0) - peer.send_and_ping(msg_sendcmpct(announce=True, version=peer.cmpct_version)) + peer.send_and_ping(msg_sendcmpct(announce=True, version=1)) def test_compactblock_reconstruction_multiple_peers(self, stalling_peer, delivery_peer): node = self.nodes[0] @@ -769,7 +761,7 @@ class CompactBlocksTest(BitcoinTestFramework): def test_highbandwidth_mode_states_via_getpeerinfo(self): # create new p2p connection for a fresh state w/o any prior sendcmpct messages sent - hb_test_node = self.nodes[0].add_p2p_connection(TestP2PConn(cmpct_version=1)) + hb_test_node = self.nodes[0].add_p2p_connection(TestP2PConn()) # assert the RPC getpeerinfo boolean fields `bip152_hb_{to, from}` # match the given parameters for the last peer of a given node @@ -800,19 +792,17 @@ class CompactBlocksTest(BitcoinTestFramework): self.nodes[0].generate(1) # Setup the p2p connections - self.test_node = self.nodes[0].add_p2p_connection(TestP2PConn(cmpct_version=1)) - self.old_node = self.nodes[0].add_p2p_connection(TestP2PConn(cmpct_version=1), services=NODE_NETWORK | NODE_HEADERS_COMPRESSED) - self.additional_test_node = self.nodes[0].add_p2p_connection(TestP2PConn(cmpct_version=1), services=NODE_NETWORK | NODE_HEADERS_COMPRESSED) + self.test_node = self.nodes[0].add_p2p_connection(TestP2PConn()) + self.additional_test_node = self.nodes[0].add_p2p_connection(TestP2PConn(), services=NODE_NETWORK | NODE_HEADERS_COMPRESSED) # We will need UTXOs to construct transactions in later tests. self.make_utxos() self.log.info("Testing SENDCMPCT p2p message... ") - self.test_sendcmpct(self.test_node, old_node=self.old_node) + self.test_sendcmpct(self.test_node) self.test_sendcmpct(self.additional_test_node) self.log.info("Testing compactblock construction...") - self.test_compactblock_construction(self.old_node) self.test_compactblock_construction(self.test_node) self.log.info("Testing compactblock requests... ") @@ -820,11 +810,9 @@ class CompactBlocksTest(BitcoinTestFramework): self.log.info("Testing getblocktxn handler...") self.test_getblocktxn_handler(self.test_node) - self.test_getblocktxn_handler(self.old_node) self.log.info("Testing compactblock requests/announcements not at chain tip...") self.test_compactblocks_not_at_tip(self.test_node) - self.test_compactblocks_not_at_tip(self.old_node) self.log.info("Testing handling of incorrect blocktxn responses...") self.test_incorrect_blocktxn_response(self.test_node) @@ -834,13 +822,12 @@ class CompactBlocksTest(BitcoinTestFramework): # End-to-end block relay tests self.log.info("Testing end-to-end block relay...") - self.request_cb_announcements(self.old_node) self.request_cb_announcements(self.test_node) - self.test_end_to_end_block_relay([self.test_node, self.old_node]) + self.request_cb_announcements(self.additional_test_node) + self.test_end_to_end_block_relay([self.test_node, self.additional_test_node]) self.log.info("Testing handling of invalid compact blocks...") self.test_invalid_tx_in_compactblock(self.test_node) - self.test_invalid_tx_in_compactblock(self.old_node) self.log.info("Testing invalid index in cmpctblock message...") self.test_invalid_cmpctblock_message() From 06a6f8444cf3cfd3364774315b70255c66b3edd8 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 9 Jun 2024 11:18:53 +0000 Subject: [PATCH 6/8] merge bitcoin#25147: follow ups to #20799 (removing support for v1 compact blocks) --- src/net_processing.cpp | 6 +++--- src/test/blockencodings_tests.cpp | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f95825f5a6..4a37a27a8b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1991,7 +1991,7 @@ static uint256 most_recent_block_hash GUARDED_BY(cs_most_recent_block); * to compatible peers. */ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& pblock) { - std::shared_ptr pcmpctblock = std::make_shared (*pblock); + auto pcmpctblock = std::make_shared(*pblock); const CNetMsgMaker msgMaker(PROTOCOL_VERSION); LOCK(cs_main); @@ -2468,7 +2468,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) { m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); } else { - CBlockHeaderAndShortTxIDs cmpctblock(*pblock); + CBlockHeaderAndShortTxIDs cmpctblock{*pblock}; m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); } } else { @@ -5518,7 +5518,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) CBlock block; bool ret = ReadBlockFromDisk(block, pBestIndex, consensusParams); assert(ret); - CBlockHeaderAndShortTxIDs cmpctblock(block); + CBlockHeaderAndShortTxIDs cmpctblock{block}; m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); } state.pindexBestHeaderSent = pBestIndex; diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index e9dd3bfc3f..d8e9ef2be4 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -64,7 +64,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) // Do a simple ShortTxIDs RT { - CBlockHeaderAndShortTxIDs shortIDs(block); + CBlockHeaderAndShortTxIDs shortIDs{block}; CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << shortIDs; @@ -122,7 +122,7 @@ public: stream >> *this; } explicit TestHeaderAndShortIDs(const CBlock& block) : - TestHeaderAndShortIDs(CBlockHeaderAndShortTxIDs(block)) {} + TestHeaderAndShortIDs(CBlockHeaderAndShortTxIDs{block}) {} uint64_t GetShortID(const uint256& txhash) const { CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); @@ -279,7 +279,7 @@ BOOST_AUTO_TEST_CASE(EmptyBlockRoundTripTest) // Test simple header round-trip with only coinbase { - CBlockHeaderAndShortTxIDs shortIDs(block); + CBlockHeaderAndShortTxIDs shortIDs{block}; CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << shortIDs; From 239062192eba2b70ada3c9aee98e381406d159df Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 24 Dec 2020 20:03:50 +0100 Subject: [PATCH 7/8] merge bitcoin#20764: cli -netinfo peer connections dashboard updates continuation of bd934c71eb232c2787593bf4766aa92ce5743ae3 from dash#6034 includes: - 9d6aeca2c5ec1df579c27c39e82fa3ddf1d25986 --- src/bitcoin-cli.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 005f28747a..0869cdd7d6 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -401,7 +401,7 @@ private: bool IsVersionSelected() const { return m_details_level == 3 || m_details_level == 4; } bool m_is_asmap_on{false}; size_t m_max_addr_length{0}; - size_t m_max_age_length{4}; + size_t m_max_age_length{3}; size_t m_max_id_length{2}; struct Peer { std::string addr; @@ -418,6 +418,8 @@ private: int id; int mapped_as; int version; + bool is_bip152_hb_from; + bool is_bip152_hb_to; bool is_block_relay; bool is_outbound; bool operator<(const Peer& rhs) const { return std::tie(is_outbound, min_ping) < std::tie(rhs.is_outbound, rhs.min_ping); } @@ -506,7 +508,9 @@ public: const std::string addr{peer["addr"].get_str()}; const std::string age{conn_time == 0 ? "" : ToString((m_time_now - conn_time) / 60)}; const std::string sub_version{peer["subver"].get_str()}; - m_peers.push_back({addr, sub_version, conn_type, network, age, min_ping, ping, last_blck, last_recv, last_send, last_trxn, peer_id, mapped_as, version, is_block_relay, is_outbound}); + const bool is_bip152_hb_from{peer["bip152_hb_from"].get_bool()}; + const bool is_bip152_hb_to{peer["bip152_hb_to"].get_bool()}; + m_peers.push_back({addr, sub_version, conn_type, network, age, min_ping, ping, last_blck, last_recv, last_send, last_trxn, peer_id, mapped_as, version, is_bip152_hb_from, is_bip152_hb_to, is_block_relay, is_outbound}); m_max_addr_length = std::max(addr.length() + 1, m_max_addr_length); m_max_age_length = std::max(age.length(), m_max_age_length); m_max_id_length = std::max(ToString(peer_id).length(), m_max_id_length); @@ -520,13 +524,13 @@ public: // Report detailed peer connections list sorted by direction and minimum ping time. if (DetailsRequested() && !m_peers.empty()) { std::sort(m_peers.begin(), m_peers.end()); - result += strprintf("<-> type net mping ping send recv txn blk %*s ", m_max_age_length, "age"); + result += strprintf("<-> type net mping ping send recv txn blk hb %*s ", m_max_age_length, "age"); if (m_is_asmap_on) result += " asmap "; result += strprintf("%*s %-*s%s\n", m_max_id_length, "id", IsAddressSelected() ? m_max_addr_length : 0, IsAddressSelected() ? "address" : "", IsVersionSelected() ? "version" : ""); for (const Peer& peer : m_peers) { std::string version{ToString(peer.version) + peer.sub_version}; result += strprintf( - "%3s %6s %5s%7s%7s%5s%5s%5s%5s %*s%*i %*s %-*s%s\n", + "%3s %6s %5s%7s%7s%5s%5s%5s%5s %2s %*s%*i %*s %-*s%s\n", peer.is_outbound ? "out" : "in", ConnectionTypeForNetinfo(peer.conn_type), peer.network, @@ -536,6 +540,7 @@ public: peer.last_recv == 0 ? "" : ToString(m_time_now - peer.last_recv), peer.last_trxn == 0 ? "" : ToString((m_time_now - peer.last_trxn) / 60), peer.last_blck == 0 ? "" : ToString((m_time_now - peer.last_blck) / 60), + strprintf("%s%s", peer.is_bip152_hb_to ? "." : " ", peer.is_bip152_hb_from ? "*" : " "), m_max_age_length, // variable spacing peer.age, m_is_asmap_on ? 7 : 0, // variable spacing @@ -546,7 +551,7 @@ public: IsAddressSelected() ? peer.addr : "", IsVersionSelected() && version != "0" ? version : ""); } - result += strprintf(" ms ms sec sec min min %*s\n\n", m_max_age_length, "min"); + result += strprintf(" ms ms sec sec min min %*s\n\n", m_max_age_length, "min"); } // Report peer connection totals by type. @@ -632,6 +637,9 @@ public: " recv Time since last message received from the peer, in seconds\n" " txn Time since last novel transaction received from the peer and accepted into our mempool, in minutes\n" " blk Time since last novel block passing initial validity checks received from the peer, in minutes\n" + " hb High-bandwidth BIP152 compact block relay\n" + " \".\" (to) - we selected the peer as a high-bandwidth peer\n" + " \"*\" (from) - the peer selected us as a high-bandwidth peer\n" " age Duration of connection to the peer, in minutes\n" " asmap Mapped AS (Autonomous System) number in the BGP route to the peer, used for diversifying\n" " peer selection (only displayed if the -asmap config option is set)\n" From 1cbf3b9a53e09d36df89c14690b699aa282e1ec0 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 9 Jun 2024 11:28:00 +0000 Subject: [PATCH 8/8] merge bitcoin-core/gui#206: Display fRelayTxes and bip152_highbandwidth_{to, from} in peer details continuation of 3e8ba24c87e32c2669c7be6924e750e29805a30e from dash#5964 includes: - 142807af8b82e2372a03df893c50df4f4a96aca4 --- src/qt/forms/debugwindow.ui | 160 +++++++++++++++++++++--------------- src/qt/rpcconsole.cpp | 10 +++ 2 files changed, 103 insertions(+), 67 deletions(-) diff --git a/src/qt/forms/debugwindow.ui b/src/qt/forms/debugwindow.ui index 6e79ad9d2c..d6ea9fd3eb 100644 --- a/src/qt/forms/debugwindow.ui +++ b/src/qt/forms/debugwindow.ui @@ -1150,14 +1150,17 @@ - + + + High bandwidth BIP152 compact block relay: %1 + - Starting Block + High Bandwidth - + IBeamCursor @@ -1173,14 +1176,14 @@ - + - Synced Headers + Starting Block - + IBeamCursor @@ -1196,14 +1199,14 @@ - + - Synced Blocks + Synced Headers - + IBeamCursor @@ -1219,14 +1222,14 @@ - + - Connection Time + Synced Blocks - + IBeamCursor @@ -1242,17 +1245,14 @@ - - - Elapsed time since a novel block passing initial validity checks was received from this peer. - + - Last Block + Connection Time - + IBeamCursor @@ -1268,17 +1268,17 @@ - + - Elapsed time since a novel transaction accepted into our mempool was received from this peer. + Elapsed time since a novel block passing initial validity checks was received from this peer. - Last Transaction + Last Block - + IBeamCursor @@ -1294,14 +1294,17 @@ - + + + Elapsed time since a novel transaction accepted into our mempool was received from this peer. + - Last Send + Last Transaction - + IBeamCursor @@ -1317,14 +1320,14 @@ - + - Last Receive + Last Send - + IBeamCursor @@ -1340,14 +1343,14 @@ - + - Sent + Last Receive - + IBeamCursor @@ -1363,14 +1366,14 @@ - + - Received + Sent - + IBeamCursor @@ -1386,14 +1389,14 @@ - + - Ping Time + Received - + IBeamCursor @@ -1409,17 +1412,14 @@ - - - The duration of a currently outstanding ping. - + - Ping Wait + Ping Time - + IBeamCursor @@ -1435,14 +1435,17 @@ - + + + The duration of a currently outstanding ping. + - Min Ping + Ping Wait - + IBeamCursor @@ -1458,14 +1461,14 @@ - + - Time Offset + Min Ping - + IBeamCursor @@ -1481,17 +1484,14 @@ - - - The mapped Autonomous System used for diversifying peer selection. - + - Mapped AS + Time Offset - + IBeamCursor @@ -1507,17 +1507,17 @@ - + - Whether we relay addresses to this peer. + The mapped Autonomous System used for diversifying peer selection. - Address Relay + Mapped AS - + IBeamCursor @@ -1533,17 +1533,17 @@ - + - Total number of addresses processed, excluding those dropped due to rate-limiting. + Whether we relay addresses to this peer. - Addresses Processed + Address Relay - + IBeamCursor @@ -1559,17 +1559,17 @@ - + - Total number of addresses dropped due to rate-limiting. + Total number of addresses processed, excluding those dropped due to rate-limiting. - Addresses Rate-Limited + Addresses Processed - + IBeamCursor @@ -1585,6 +1585,32 @@ + + + Total number of addresses dropped due to rate-limiting. + + + Addresses Rate-Limited + + + + + + + IBeamCursor + + + N/A + + + Qt::PlainText + + + Qt::LinksAccessibleByMouse|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse + + + + Qt::Vertical diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index f15dce30e2..c31c6b1ba7 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -514,6 +514,11 @@ RPCConsole::RPCConsole(interfaces::Node& node, QWidget* parent, Qt::WindowFlags tr("Outbound Address Fetch: short-lived, for soliciting addresses")}; const QString list{"
  • " + Join(CONNECTION_TYPE_DOC, QString("
  • ")) + "
"}; ui->peerConnectionTypeLabel->setToolTip(ui->peerConnectionTypeLabel->toolTip().arg(list)); + const QString hb_list{"
  • \"" + + tr("To") + "\" – " + tr("we selected the peer for high bandwidth relay") + "
  • \"" + + tr("From") + "\" – " + tr("the peer selected us for high bandwidth relay") + "
  • \"" + + tr("No") + "\" – " + tr("no high bandwidth relay selected") + "
"}; + ui->peerHighBandwidthLabel->setToolTip(ui->peerHighBandwidthLabel->toolTip().arg(hb_list)); ui->dataDir->setToolTip(ui->dataDir->toolTip().arg(QString(nonbreaking_hyphen) + "datadir")); ui->blocksDir->setToolTip(ui->blocksDir->toolTip().arg(QString(nonbreaking_hyphen) + "blocksdir")); ui->openDebugLogfileButton->setToolTip(ui->openDebugLogfileButton->toolTip().arg(PACKAGE_NAME)); @@ -1279,6 +1284,11 @@ void RPCConsole::updateDetailWidget() ui->peerConnTime->setText(GUIUtil::formatDurationStr(time_now - std::chrono::seconds{stats->nodeStats.nTimeConnected})); ui->peerLastBlock->setText(TimeDurationField(time_now, std::chrono::seconds{stats->nodeStats.nLastBlockTime})); ui->peerLastTx->setText(TimeDurationField(time_now, std::chrono::seconds{stats->nodeStats.nLastTXTime})); + QString bip152_hb_settings; + if (stats->nodeStats.m_bip152_highbandwidth_to) bip152_hb_settings += "To"; + if (stats->nodeStats.m_bip152_highbandwidth_from) bip152_hb_settings += (bip152_hb_settings == "" ? "From" : "/From"); + if (bip152_hb_settings == "") bip152_hb_settings = "No"; + ui->peerHighBandwidth->setText(bip152_hb_settings); ui->peerLastSend->setText(TimeDurationField(time_now, stats->nodeStats.m_last_send)); ui->peerLastRecv->setText(TimeDurationField(time_now, stats->nodeStats.m_last_recv)); ui->peerBytesSent->setText(GUIUtil::formatBytes(stats->nodeStats.nSendBytes));