mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-22 06:52:36 +01:00
Code Issues Projects Releases Wiki Activity

Support up to 3 parallel compact block txn fetchings

Browse source 
A single outbound slot is required, so if the first two slots
are taken by inbound in-flights, the node will reject additional
unless they are coming from outbound.

This means in the case where a fast sybil peer is attempting to
stall out a node, a single high bandwidth outbound peer can
mitigate the attack.
This commit is contained in:
Greg Sanders 2023-05-16 15:36:38 -04:00
parent 13f9b20b4c
commit 03423f8bd1
4 changed files with 90 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/net.h
View file

@ -200,7 +200,9 @@ public:
int nVersion; int nVersion;
std::string cleanSubVer; std::string cleanSubVer;
bool fInbound; bool fInbound;
// We requested high bandwidth connection to peer
bool m_bip152_highbandwidth_to; bool m_bip152_highbandwidth_to;
// Peer requested high bandwidth connection
bool m_bip152_highbandwidth_from; bool m_bip152_highbandwidth_from;
int m_starting_height; int m_starting_height;
uint64_t nSendBytes; uint64_t nSendBytes;

122
src/net_processing.cpp
View file

@ -876,6 +876,9 @@ private:
/** Have we requested this block from a peer */ /** Have we requested this block from a peer */
bool IsBlockRequested(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool IsBlockRequested(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/** Have we requested this block from an outbound peer */
bool IsBlockRequestedFromOutbound(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/** Remove this block from our tracked requested blocks. Called if: /** Remove this block from our tracked requested blocks. Called if:
* - the block has been received from a peer * - the block has been received from a peer
* - the request for the block has timed out * - the request for the block has timed out
@ -1121,6 +1124,17 @@ bool PeerManagerImpl::IsBlockRequested(const uint256& hash)
return mapBlocksInFlight.count(hash); return mapBlocksInFlight.count(hash);
} }
bool PeerManagerImpl::IsBlockRequestedFromOutbound(const uint256& hash)
{
for (auto range = mapBlocksInFlight.equal_range(hash); range.first != range.second; range.first++) {
auto [nodeid, block_it] = range.first->second;
CNodeState& nodestate = *Assert(State(nodeid));
if (!nodestate.m_is_inbound) return true;
}
return false;
}
void PeerManagerImpl::RemoveBlockRequest(const uint256& hash, std::optional<NodeId> from_peer) void PeerManagerImpl::RemoveBlockRequest(const uint256& hash, std::optional<NodeId> from_peer)
{ {
auto range = mapBlocksInFlight.equal_range(hash); auto range = mapBlocksInFlight.equal_range(hash);
@ -1129,8 +1143,8 @@ void PeerManagerImpl::RemoveBlockRequest(const uint256& hash, std::optional<Node
return; return;
} }
// Currently we don't request more than one peer for same block // We should not have requested too many of this block
Assume(mapBlocksInFlight.count(hash) == 1); Assume(mapBlocksInFlight.count(hash) <= MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK);
while (range.first != range.second) { while (range.first != range.second) {
auto [node_id, list_it] = range.first->second; auto [node_id, list_it] = range.first->second;
@ -1140,20 +1154,19 @@ void PeerManagerImpl::RemoveBlockRequest(const uint256& hash, std::optional<Node
continue; continue;
} }
CNodeState *state = State(node_id); CNodeState& state = *Assert(State(node_id));
assert(state != nullptr);
if (state->vBlocksInFlight.begin() == list_it) { if (state.vBlocksInFlight.begin() == list_it) {
// First block on the queue was received, update the start download time for the next one // First block on the queue was received, update the start download time for the next one
state->m_downloading_since = std::max(state->m_downloading_since, GetTime<std::chrono::microseconds>()); state.m_downloading_since = std::max(state.m_downloading_since, GetTime<std::chrono::microseconds>());
} }
state->vBlocksInFlight.erase(list_it); state.vBlocksInFlight.erase(list_it);
if (state->vBlocksInFlight.empty()) { if (state.vBlocksInFlight.empty()) {
// Last validated block on the queue for this peer was received. // Last validated block on the queue for this peer was received.
m_peers_downloading_from--; m_peers_downloading_from--;
} }
state->m_stalling_since = 0us; state.m_stalling_since = 0us;
range.first = mapBlocksInFlight.erase(range.first); range.first = mapBlocksInFlight.erase(range.first);
} }
@ -1166,6 +1179,8 @@ bool PeerManagerImpl::BlockRequested(NodeId nodeid, const CBlockIndex& block, st
CNodeState *state = State(nodeid); CNodeState *state = State(nodeid);
assert(state != nullptr); assert(state != nullptr);
Assume(mapBlocksInFlight.count(hash) <= MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK);
// Short-circuit most stuff in case it is from the same node // Short-circuit most stuff in case it is from the same node
for (auto range = mapBlocksInFlight.equal_range(hash); range.first != range.second; range.first++) { for (auto range = mapBlocksInFlight.equal_range(hash); range.first != range.second; range.first++) {
if (range.first->second.first == nodeid) { if (range.first->second.first == nodeid) {
@ -1176,8 +1191,8 @@ bool PeerManagerImpl::BlockRequested(NodeId nodeid, const CBlockIndex& block, st
} }
} }
// Make sure it's not listed somewhere already. // Make sure it's not being fetched already from same peer.
RemoveBlockRequest(hash, std::nullopt); RemoveBlockRequest(hash, nodeid);
std::list<QueuedBlock>::iterator it = state->vBlocksInFlight.insert(state->vBlocksInFlight.end(), std::list<QueuedBlock>::iterator it = state->vBlocksInFlight.insert(state->vBlocksInFlight.end(),
{&block, std::unique_ptr<PartiallyDownloadedBlock>(pit ? new PartiallyDownloadedBlock(&m_mempool) : nullptr)}); {&block, std::unique_ptr<PartiallyDownloadedBlock>(pit ? new PartiallyDownloadedBlock(&m_mempool) : nullptr)});
@ -1774,11 +1789,10 @@ std::optional<std::string> PeerManagerImpl::FetchBlock(NodeId peer_id, const CBl
LOCK(cs_main); LOCK(cs_main);
// Mark block as in-flight unless it already is (for this peer). // Forget about all prior requests
// If the peer does not send us a block, vBlocksInFlight remains non-empty, RemoveBlockRequest(block_index.GetBlockHash(), std::nullopt);
// causing us to timeout and disconnect.
// If a block was already in-flight for a different peer, its BLOCKTXN // Mark block as in-flight
// response will be dropped.
if (!BlockRequested(peer_id, block_index)) return "Already requested from this peer"; if (!BlockRequested(peer_id, block_index)) return "Already requested from this peer";
// Construct message to request the block // Construct message to request the block
@ -4292,12 +4306,15 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
return; return;
auto range_flight = mapBlocksInFlight.equal_range(pindex->GetBlockHash()); auto range_flight = mapBlocksInFlight.equal_range(pindex->GetBlockHash());
bool fAlreadyInFlight = range_flight.first != range_flight.second; size_t already_in_flight = std::distance(range_flight.first, range_flight.second);
bool in_flight_same_peer{false}; bool requested_block_from_this_peer{false};
// Multimap ensures ordering of outstanding requests. It's either empty or first in line.
bool first_in_flight = already_in_flight == 0 || (range_flight.first->second.first == pfrom.GetId());
while (range_flight.first != range_flight.second) { while (range_flight.first != range_flight.second) {
if (range_flight.first->second.first == pfrom.GetId()) { if (range_flight.first->second.first == pfrom.GetId()) {
in_flight_same_peer = true; requested_block_from_this_peer = true;
break; break;
} }
range_flight.first++; range_flight.first++;
@ -4305,7 +4322,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
if (pindex->nChainWork <= m_chainman.ActiveChain().Tip()->nChainWork || // We know something better 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 pindex->nTx != 0) { // We had this block at some point, but pruned it
if (in_flight_same_peer) { if (requested_block_from_this_peer) {
// We requested this block for some reason, but our mempool will probably be useless // We requested this block for some reason, but our mempool will probably be useless
// so we just grab the block via normal getdata // so we just grab the block via normal getdata
std::vector<CInv> vInv(1); std::vector<CInv> vInv(1);
@ -4316,15 +4333,15 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
} }
// If we're not close to tip yet, give up and let parallel block fetch work its magic // If we're not close to tip yet, give up and let parallel block fetch work its magic
if (!fAlreadyInFlight && !CanDirectFetch()) { if (!already_in_flight && !CanDirectFetch()) {
return; return;
} }
// We want to be a bit conservative just to be extra careful about DoS // We want to be a bit conservative just to be extra careful about DoS
// possibilities in compact block processing... // possibilities in compact block processing...
if (pindex->nHeight <= m_chainman.ActiveChain().Height() + 2) { if (pindex->nHeight <= m_chainman.ActiveChain().Height() + 2) {
if ((!fAlreadyInFlight && nodestate->vBlocksInFlight.size() < MAX_BLOCKS_IN_TRANSIT_PER_PEER) || if ((already_in_flight < MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK && nodestate->vBlocksInFlight.size() < MAX_BLOCKS_IN_TRANSIT_PER_PEER) ||
in_flight_same_peer) { requested_block_from_this_peer) {
std::list<QueuedBlock>::iterator* queuedBlockIt = nullptr; std::list<QueuedBlock>::iterator* queuedBlockIt = nullptr;
if (!BlockRequested(pfrom.GetId(), *pindex, &queuedBlockIt)) { if (!BlockRequested(pfrom.GetId(), *pindex, &queuedBlockIt)) {
if (!(*queuedBlockIt)->partialBlock) if (!(*queuedBlockIt)->partialBlock)
@ -4343,11 +4360,16 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
Misbehaving(*peer, 100, "invalid compact block"); Misbehaving(*peer, 100, "invalid compact block");
return; return;
} else if (status == READ_STATUS_FAILED) { } else if (status == READ_STATUS_FAILED) {
// Duplicate txindexes, the block is now in-flight, so just request it if (first_in_flight) {
std::vector<CInv> vInv(1); // Duplicate txindexes, the block is now in-flight, so just request it
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), blockhash); std::vector<CInv> vInv(1);
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), blockhash);
return; m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv));
return;
} else {
// Give up for this peer and wait for other peer(s)
RemoveBlockRequest(pindex->GetBlockHash(), pfrom.GetId());
}
} }
BlockTransactionsRequest req; BlockTransactionsRequest req;
@ -4361,9 +4383,24 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
txn.blockhash = blockhash; txn.blockhash = blockhash;
blockTxnMsg << txn; blockTxnMsg << txn;
fProcessBLOCKTXN = true; fProcessBLOCKTXN = true;
} else { } else if (first_in_flight) {
// We will try to round-trip any compact blocks we get on failure,
// as long as it's first...
req.blockhash = pindex->GetBlockHash(); req.blockhash = pindex->GetBlockHash();
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETBLOCKTXN, req)); m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETBLOCKTXN, req));
} else if (pfrom.m_bip152_highbandwidth_to &&
(!pfrom.IsInboundConn() ||
IsBlockRequestedFromOutbound(blockhash) ||
already_in_flight < MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK - 1)) {
// ... or it's a hb relay peer and:
// - peer is outbound, or
// - we already have an outbound attempt in flight(so we'll take what we can get), or
// - it's not the final parallel download slot (which we may reserve for first outbound)
req.blockhash = pindex->GetBlockHash();
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETBLOCKTXN, req));
} else {
// Give up for this peer and wait for other peer(s)
RemoveBlockRequest(pindex->GetBlockHash(), pfrom.GetId());
} }
} else { } else {
// This block is either already in flight from a different // This block is either already in flight from a different
@ -4384,7 +4421,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
} }
} }
} else { } else {
if (in_flight_same_peer) { if (requested_block_from_this_peer) {
// We requested this block, but its far into the future, so our // We requested this block, but its far into the future, so our
// mempool will probably be useless - request the block normally // mempool will probably be useless - request the block normally
std::vector<CInv> vInv(1); std::vector<CInv> vInv(1);
@ -4456,18 +4493,23 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
{ {
LOCK(cs_main); LOCK(cs_main);
bool expected_blocktxn = false;
auto range_flight = mapBlocksInFlight.equal_range(resp.blockhash); auto range_flight = mapBlocksInFlight.equal_range(resp.blockhash);
size_t already_in_flight = std::distance(range_flight.first, range_flight.second);
bool requested_block_from_this_peer{false};
// Multimap ensures ordering of outstanding requests. It's either empty or first in line.
bool first_in_flight = already_in_flight == 0 || (range_flight.first->second.first == pfrom.GetId());
while (range_flight.first != range_flight.second) { while (range_flight.first != range_flight.second) {
auto [node_id, block_it] = range_flight.first->second; auto [node_id, block_it] = range_flight.first->second;
if (node_id == pfrom.GetId() && block_it->partialBlock) { if (node_id == pfrom.GetId() && block_it->partialBlock) {
expected_blocktxn = true; requested_block_from_this_peer = true;
break; break;
} }
range_flight.first++; range_flight.first++;
} }
if (!expected_blocktxn) { if (!requested_block_from_this_peer) {
LogPrint(BCLog::NET, "Peer %d sent us block transactions for block we weren't expecting\n", pfrom.GetId()); LogPrint(BCLog::NET, "Peer %d sent us block transactions for block we weren't expecting\n", pfrom.GetId());
return; return;
} }
@ -4479,10 +4521,16 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
Misbehaving(*peer, 100, "invalid compact block/non-matching block transactions"); Misbehaving(*peer, 100, "invalid compact block/non-matching block transactions");
return; return;
} else if (status == READ_STATUS_FAILED) { } else if (status == READ_STATUS_FAILED) {
// Might have collided, fall back to getdata now :( if (first_in_flight) {
std::vector<CInv> invs; // Might have collided, fall back to getdata now :(
invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(*peer), resp.blockhash)); std::vector<CInv> invs;
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, invs)); invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(*peer), resp.blockhash));
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, invs));
} else {
RemoveBlockRequest(resp.blockhash, pfrom.GetId());
LogPrint(BCLog::NET, "Peer %d sent us a compact block but it failed to reconstruct, waiting on first download to complete\n", pfrom.GetId());
return;
}
} else { } else {
// Block is either okay, or possibly we received // Block is either okay, or possibly we received
// READ_STATUS_CHECKBLOCK_FAILED. // READ_STATUS_CHECKBLOCK_FAILED.

2
src/net_processing.h
View file

@ -22,6 +22,8 @@ static const bool DEFAULT_PEERBLOOMFILTERS = false;
static const bool DEFAULT_PEERBLOCKFILTERS = false; static const bool DEFAULT_PEERBLOCKFILTERS = false;
/** Threshold for marking a node to be discouraged, e.g. disconnected and added to the discouragement filter. */ /** Threshold for marking a node to be discouraged, e.g. disconnected and added to the discouragement filter. */
static const int DISCOURAGEMENT_THRESHOLD{100}; static const int DISCOURAGEMENT_THRESHOLD{100};
/** Maximum number of outstanding CMPCTBLOCK requests for the same block. */
static const unsigned int MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK = 3;
struct CNodeStateStats { struct CNodeStateStats {
int nSyncHeight = -1; int nSyncHeight = -1;

2
src/rpc/blockchain.cpp
View file

@ -428,7 +428,7 @@ static RPCHelpMan getblockfrompeer()
"getblockfrompeer", "getblockfrompeer",
"Attempt to fetch block from a given peer.\n\n" "Attempt to fetch block from a given peer.\n\n"
"We must have the header for this block, e.g. using submitheader.\n" "We must have the header for this block, e.g. using submitheader.\n"
"Subsequent calls for the same block and a new peer will cause the response from the previous peer to be ignored.\n" "Subsequent calls for the same block may cause the response from the previous peer to be ignored.\n"
"Peers generally ignore requests for a stale block that they never fully verified, or one that is more than a month old.\n" "Peers generally ignore requests for a stale block that they never fully verified, or one that is more than a month old.\n"
"When a peer does not respond with a block, we will disconnect.\n" "When a peer does not respond with a block, we will disconnect.\n"
"Note: The block could be re-pruned as soon as it is received.\n\n" "Note: The block could be re-pruned as soon as it is received.\n\n"