foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-08 10:31:50 -05:00
Code Issues Activity

Merge #19583: p2p: clean up Misbehaving()

Browse source 
a8865f8b72 [net processing] Tidy up Misbehaving() (John Newbery)
d15b3afb4c [net processing] Always supply debug message to Misbehaving() (John Newbery)
634144a1c2 [net processing] Fixup MaybeDiscourageAndDisconnect() style (John Newbery)

Pull request description:

  This PR makes a few minor clean-ups to `Misbehaving()` in preparation to move it out of the cs_main lock.

  There are very minor logging changes but otherwise no functional changes.

ACKs for top commit:
  troygiorshev:
    tACK a8865f8b72
  jonatack:
    ACK a8865f8
  fjahr:
    Code review ACK a8865f8b72
  promag:
    Code review ACK a8865f8b72.

Tree-SHA512: 98fb4f5f76399715545a1ea19290dcebfc8cb4eff72a1d3555dd3de6e184040bb8668c9651dab21db0dfd8e674e53a5977105ef76547146c9f6fa6b4b9d2ba59
This commit is contained in:
fanquake 2020-07-28 14:57:44 +08:00
commit 2979a7aff0
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
1 changed files with 24 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

52
src/net_processing.cpp
View file

@ -157,9 +157,6 @@ std::map<uint256, std::map<uint256, COrphanTx>::iterator> g_orphans_by_wtxid GUA
void EraseOrphansFor(NodeId peer); void EraseOrphansFor(NodeId peer);
/** Increase a node's misbehavior score. */
void Misbehaving(NodeId nodeid, int howmuch, const std::string& message="") EXCLUSIVE_LOCKS_REQUIRED(cs_main);
// Internal stuff // Internal stuff
namespace { namespace {
/** Number of nodes with fSyncStarted. */ /** Number of nodes with fSyncStarted. */
@ -1062,23 +1059,22 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
* Increment peer's misbehavior score. If the new value >= DISCOURAGEMENT_THRESHOLD, mark the node * Increment peer's misbehavior score. If the new value >= DISCOURAGEMENT_THRESHOLD, mark the node
* to be discouraged, meaning the peer might be disconnected and added to the discouragement filter. * to be discouraged, meaning the peer might be disconnected and added to the discouragement filter.
*/ */
void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIVE_LOCKS_REQUIRED(cs_main) void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{ {
if (howmuch == 0) assert(howmuch > 0);
return;
CNodeState *state = State(pnode); CNodeState* const state = State(pnode);
if (state == nullptr) if (state == nullptr) return;
return;
state->nMisbehavior += howmuch; state->nMisbehavior += howmuch;
std::string message_prefixed = message.empty() ? "" : (": " + message); const std::string message_prefixed = message.empty() ? "" : (": " + message);
if (state->nMisbehavior >= DISCOURAGEMENT_THRESHOLD && state->nMisbehavior - howmuch < DISCOURAGEMENT_THRESHOLD) if (state->nMisbehavior >= DISCOURAGEMENT_THRESHOLD && state->nMisbehavior - howmuch < DISCOURAGEMENT_THRESHOLD)
{ {
LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed); LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", pnode, state->nMisbehavior - howmuch, state->nMisbehavior, message_prefixed);
state->m_should_discourage = true; state->m_should_discourage = true;
} else } else {
LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d)%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed); LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d)%s\n", pnode, state->nMisbehavior - howmuch, state->nMisbehavior, message_prefixed);
}
} }
/** /**
@ -1799,7 +1795,7 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac
for (size_t i = 0; i < req.indexes.size(); i++) { for (size_t i = 0; i < req.indexes.size(); i++) {
if (req.indexes[i] >= block.vtx.size()) { if (req.indexes[i] >= block.vtx.size()) {
LOCK(cs_main); LOCK(cs_main);
Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us a getblocktxn with out-of-bounds tx indices", pfrom.GetId())); Misbehaving(pfrom.GetId(), 100, "getblocktxn with out-of-bounds tx indices");
return; return;
} }
resp.txn[i] = block.vtx[req.indexes[i]]; resp.txn[i] = block.vtx[req.indexes[i]];
@ -1848,7 +1844,7 @@ static void ProcessHeadersMessage(CNode& pfrom, CConnman& connman, ChainstateMan
UpdateBlockAvailability(pfrom.GetId(), headers.back().GetHash()); UpdateBlockAvailability(pfrom.GetId(), headers.back().GetHash());
if (nodestate->nUnconnectingHeaders % MAX_UNCONNECTING_HEADERS == 0) { if (nodestate->nUnconnectingHeaders % MAX_UNCONNECTING_HEADERS == 0) {
Misbehaving(pfrom.GetId(), 20); Misbehaving(pfrom.GetId(), 20, strprintf("%d non-connecting headers", nodestate->nUnconnectingHeaders));
} }
return; return;
} }
@ -2307,7 +2303,7 @@ void ProcessMessage(
if (pfrom.nVersion != 0) if (pfrom.nVersion != 0)
{ {
LOCK(cs_main); LOCK(cs_main);
Misbehaving(pfrom.GetId(), 1); Misbehaving(pfrom.GetId(), 1, "redundant version message");
return; return;
} }
@ -2468,7 +2464,7 @@ void ProcessMessage(
if (pfrom.nVersion == 0) { if (pfrom.nVersion == 0) {
// Must have a version message before anything else // Must have a version message before anything else
LOCK(cs_main); LOCK(cs_main);
Misbehaving(pfrom.GetId(), 1); Misbehaving(pfrom.GetId(), 1, "non-version message before version handshake");
return; return;
} }
@ -2535,7 +2531,7 @@ void ProcessMessage(
if (!pfrom.fSuccessfullyConnected) { if (!pfrom.fSuccessfullyConnected) {
// Must have a verack message before anything else // Must have a verack message before anything else
LOCK(cs_main); LOCK(cs_main);
Misbehaving(pfrom.GetId(), 1); Misbehaving(pfrom.GetId(), 1, "non-verack message before version handshake");
return; return;
} }
@ -3203,7 +3199,7 @@ void ProcessMessage(
ReadStatus status = partialBlock.InitData(cmpctblock, vExtraTxnForCompact); ReadStatus status = partialBlock.InitData(cmpctblock, vExtraTxnForCompact);
if (status == READ_STATUS_INVALID) { if (status == READ_STATUS_INVALID) {
MarkBlockAsReceived(pindex->GetBlockHash()); // Reset in-flight state in case Misbehaving does not result in a disconnect MarkBlockAsReceived(pindex->GetBlockHash()); // Reset in-flight state in case Misbehaving does not result in a disconnect
Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us invalid compact block\n", pfrom.GetId())); Misbehaving(pfrom.GetId(), 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 // Duplicate txindexes, the block is now in-flight, so just request it
@ -3336,7 +3332,7 @@ void ProcessMessage(
ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn); ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn);
if (status == READ_STATUS_INVALID) { if (status == READ_STATUS_INVALID) {
MarkBlockAsReceived(resp.blockhash); // Reset in-flight state in case Misbehaving does not result in a disconnect MarkBlockAsReceived(resp.blockhash); // Reset in-flight state in case Misbehaving does not result in a disconnect
Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us invalid compact block/non-matching block transactions\n", pfrom.GetId())); Misbehaving(pfrom.GetId(), 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 :( // Might have collided, fall back to getdata now :(
@ -3605,7 +3601,7 @@ void ProcessMessage(
{ {
// There is no excuse for sending a too-large filter // There is no excuse for sending a too-large filter
LOCK(cs_main); LOCK(cs_main);
Misbehaving(pfrom.GetId(), 100); Misbehaving(pfrom.GetId(), 100, "too-large bloom filter");
} }
else if (pfrom.m_tx_relay != nullptr) else if (pfrom.m_tx_relay != nullptr)
{ {
@ -3639,7 +3635,7 @@ void ProcessMessage(
} }
if (bad) { if (bad) {
LOCK(cs_main); LOCK(cs_main);
Misbehaving(pfrom.GetId(), 100); Misbehaving(pfrom.GetId(), 100, "bad filteradd message");
} }
return; return;
} }
@ -3723,32 +3719,32 @@ void ProcessMessage(
*/ */
bool PeerLogicValidation::MaybeDiscourageAndDisconnect(CNode& pnode) bool PeerLogicValidation::MaybeDiscourageAndDisconnect(CNode& pnode)
{ {
NodeId peer_id{pnode.GetId()}; const NodeId peer_id{pnode.GetId()};
{ {
LOCK(cs_main); LOCK(cs_main);
CNodeState &state = *State(peer_id); CNodeState& state = *State(peer_id);
// There's nothing to do if the m_should_discourage flag isn't set // There's nothing to do if the m_should_discourage flag isn't set
if (!state.m_should_discourage) return false; if (!state.m_should_discourage) return false;
// Reset m_should_discourage
state.m_should_discourage = false; state.m_should_discourage = false;
} // cs_main } // cs_main
if (pnode.HasPermission(PF_NOBAN)) { if (pnode.HasPermission(PF_NOBAN)) {
// Peer has the NOBAN permission flag - log but don't disconnect // We never disconnect or discourage peers for bad behavior if they have the NOBAN permission flag
LogPrintf("Warning: not punishing noban peer %d!\n", peer_id); LogPrintf("Warning: not punishing noban peer %d!\n", peer_id);
return false; return false;
} }
if (pnode.m_manual_connection) { if (pnode.m_manual_connection) {
// Peer is a manual connection - log but don't disconnect // We never disconnect or discourage manual peers for bad behavior
LogPrintf("Warning: not punishing manually connected peer %d!\n", peer_id); LogPrintf("Warning: not punishing manually connected peer %d!\n", peer_id);
return false; return false;
} }
if (pnode.addr.IsLocal()) { if (pnode.addr.IsLocal()) {
// Peer is on a local address. Disconnect this peer, but don't discourage the local address // We disconnect local peers for bad behavior but don't discourage (since that would discourage
// all peers on the same local address)
LogPrintf("Warning: disconnecting but not discouraging local peer %d!\n", peer_id); LogPrintf("Warning: disconnecting but not discouraging local peer %d!\n", peer_id);
pnode.fDisconnect = true; pnode.fDisconnect = true;
return true; return true;