mirror of https://github.com/bitcoin/bitcoin.git
p2p: Drop unsolicited CMPCTBLOCK from non-HB peer
Processing unsolicited CMPCTBLOCK's from a peer that has not been marked high bandwidth is not well-specified behavior in BIP-0152, in fact the BIP seems to imply that it is not permitted: "[...] method is not useful for compact blocks because `cmpctblock` blocks can be sent unsolicitedly in high-bandwidth mode" See https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki#separate-version-for-segregated-witness This partially mitigates a mempool leak described in [#28272](https://github.com/bitcoin/bitcoin/issues/28272), but that particular issue will persist for peers that have been selected as high bandwidth. This also mitigates potential DoS / bandwidth-wasting / abusive behavior that is discussed in the comments of #28272.
This commit is contained in:
parent
d735e2e9b3
commit
3a4e927c72
|
@ -2481,7 +2481,7 @@ void PeerManagerImpl::SendBlockTransactions(CNode& pfrom, Peer& peer, const CBlo
|
|||
tx_requested_size += resp.txn[i]->GetTotalSize();
|
||||
}
|
||||
|
||||
LogDebug(BCLog::CMPCTBLOCK, "Peer %d sent us a GETBLOCKTXN for block %s, sending a BLOCKTXN with %u txns. (%u bytes)\n", pfrom.GetId(), block.GetHash().ToString(), resp.txn.size(), tx_requested_size);
|
||||
LogDebug(BCLog::CMPCTBLOCK, "Peer %d%s sent us a GETBLOCKTXN for block %s, sending a BLOCKTXN with %u txns. (%u bytes)\n", pfrom.GetId(), pfrom.LogIP(fLogIPs), block.GetHash().ToString(), resp.txn.size(), tx_requested_size);
|
||||
MakeAndPushMessage(pfrom, NetMsgType::BLOCKTXN, resp);
|
||||
}
|
||||
|
||||
|
@ -4412,6 +4412,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
|
|||
range_flight.first++;
|
||||
}
|
||||
|
||||
if (!requested_block_from_this_peer && !pfrom.m_bip152_highbandwidth_to) {
|
||||
LogDebug(BCLog::NET, "Peer %d, not marked as high-bandwidth, sent us an unsolicited compact block!\n", pfrom.GetId());
|
||||
return;
|
||||
}
|
||||
|
||||
if (pindex->nChainWork <= m_chainman.ActiveChain().Tip()->nChainWork || // We know something better
|
||||
pindex->nTx != 0) { // We had this block at some point, but pruned it
|
||||
if (requested_block_from_this_peer) {
|
||||
|
|
|
@ -265,9 +265,13 @@ class CompactBlocksTest(BitcoinTestFramework):
|
|||
|
||||
# This test actually causes bitcoind to (reasonably!) disconnect us, so do this last.
|
||||
def test_invalid_cmpctblock_message(self):
|
||||
self.make_peer_hb_to_candidate(self.nodes[0], self.segwit_node)
|
||||
self.generate(self.nodes[0], COINBASE_MATURITY + 1)
|
||||
block = self.build_block_on_tip(self.nodes[0])
|
||||
|
||||
self.segwit_node.send_header_for_blocks([block])
|
||||
self.segwit_node.wait_for_getdata([block.hash_int], timeout=30)
|
||||
|
||||
cmpct_block = P2PHeaderAndShortIDs()
|
||||
cmpct_block.header = CBlockHeader(block)
|
||||
cmpct_block.prefilled_txn_length = 1
|
||||
|
@ -565,6 +569,10 @@ class CompactBlocksTest(BitcoinTestFramework):
|
|||
|
||||
block = self.build_block_with_transactions(node, utxo, 2)
|
||||
|
||||
# The attacker sends the block header so that we request it.
|
||||
test_node.send_without_ping(msg_headers([block]))
|
||||
test_node.wait_for_getdata([block.hash_int], timeout=30)
|
||||
|
||||
# Send compact block
|
||||
comp_block = HeaderAndShortIDs()
|
||||
comp_block.initialize_from_block(block, prefill_list=[0], use_witness=True)
|
||||
|
@ -794,6 +802,12 @@ class CompactBlocksTest(BitcoinTestFramework):
|
|||
msg = msg_cmpctblock(comp_block.to_p2p())
|
||||
test_node.send_await_disconnect(msg)
|
||||
|
||||
# peer generates a block and sends it to node, which makes the peer a
|
||||
# candidate for high-bandwidth 'to' (up to 3 peers according to BIP 152)
|
||||
def make_peer_hb_to_candidate(self, node, peer):
|
||||
block = self.build_block_on_tip(node)
|
||||
peer.send_and_ping(msg_block(block))
|
||||
|
||||
# Helper for enabling cb announcements
|
||||
# Send the sendcmpct request and sync headers
|
||||
def request_cb_announcements(self, peer):
|
||||
|
@ -806,6 +820,9 @@ class CompactBlocksTest(BitcoinTestFramework):
|
|||
node = self.nodes[0]
|
||||
assert len(self.utxos)
|
||||
|
||||
self.make_peer_hb_to_candidate(node, stalling_peer)
|
||||
self.make_peer_hb_to_candidate(node, delivery_peer)
|
||||
|
||||
def announce_cmpct_block(node, peer):
|
||||
utxo = self.utxos.pop(0)
|
||||
block = self.build_block_with_transactions(node, utxo, 5)
|
||||
|
@ -901,6 +918,7 @@ class CompactBlocksTest(BitcoinTestFramework):
|
|||
|
||||
for name, peer in [("delivery", delivery_peer), ("inbound", inbound_peer), ("outbound", outbound_peer)]:
|
||||
self.log.info(f"Setting {name} as high bandwidth peer")
|
||||
self.make_peer_hb_to_candidate(node, peer)
|
||||
block, cmpct_block = announce_cmpct_block(node, peer, 1)
|
||||
msg = msg_blocktxn()
|
||||
msg.block_transactions.blockhash = block.hash_int
|
||||
|
|
|
@ -14,6 +14,7 @@ from test_framework.messages import (
|
|||
msg_cmpctblock,
|
||||
msg_block,
|
||||
msg_blocktxn,
|
||||
msg_headers,
|
||||
HeaderAndShortIDs,
|
||||
)
|
||||
from test_framework.test_framework import BitcoinTestFramework
|
||||
|
@ -57,6 +58,10 @@ class MutatedBlocksTest(BitcoinTestFramework):
|
|||
mutated_block = copy.deepcopy(block)
|
||||
mutated_block.vtx[1].version = 4
|
||||
|
||||
# Send block header through the honest relayer
|
||||
honest_relayer.send_without_ping(msg_headers([block]))
|
||||
honest_relayer.wait_for_getdata([block.hash_int], timeout=30)
|
||||
|
||||
# Announce the new block via a compact block through the honest relayer
|
||||
cmpctblock = HeaderAndShortIDs()
|
||||
cmpctblock.initialize_from_block(block, use_witness=True)
|
||||
|
|
Loading…
Reference in New Issue