Skip to content

Commit 42bcdf4

Browse files
gandrewstonegandrewstone
authored
Merge pull request bitcoin#669 from kyuupichan/misbehaving
Move 3 members of CNodeState to CNode
2 parents 08f6aa4 + 3ee832c commit 42bcdf4

File tree

15 files changed

+160
-134
lines changed

15 files changed

+160
-134
lines changed

‎src/connmgr.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,22 @@ NodeId CConnMgr::NextNodeId()
6262
return ++next;
6363
}
6464

65+
/**
66+
* Given a node ID, return a node reference to the node.
67+
*/
68+
CNodeRef CConnMgr::FindNodeFromId(NodeId id)
69+
{
70+
LOCK(cs_vNodes);
71+
72+
for (CNode *pNode : vNodes)
73+
{
74+
if (pNode->GetId() == id)
75+
return CNodeRef(pNode);
76+
}
77+
78+
return CNodeRef();
79+
}
80+
6581
void CConnMgr::EnableExpeditedSends(CNode *pNode, bool fBlocks, bool fTxs, bool fForceIfFull)
6682
{
6783
LOCK(cs_expedited);

‎src/connmgr.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,13 @@ class CConnMgr
4141
*/
4242
NodeId NextNodeId();
4343

44+
/**
45+
* Given a node ID, return a node reference to the node.
46+
* @param[in] id The node ID
47+
* @return A CNodeRef. Will be a null CNodeRef if the node was not found.
48+
*/
49+
CNodeRef FindNodeFromId(NodeId id);
50+
4451
/**
4552
* Enable expedited sends to a node. Ignored if already enabled.
4653
* @param[in] pNode The node

‎src/dosman.cpp‎

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,16 @@
66

77
#include "dosman.h"
88
#include "bandb.h"
9-
#include "nodestate.h"
9+
#include "connmgr.h"
1010
#include "ui_interface.h"
1111

1212
#include <boost/foreach.hpp>
1313

14-
14+
CDoSManager::CDoSManager() : setBannedIsDirty(false), nBanThreshold(DEFAULT_BANSCORE_THRESHOLD) {}
15+
/**
16+
* Call once the command line is parsed so dosman configures itself appropriately.
17+
*/
18+
void CDoSManager::HandleCommandLine() { nBanThreshold = GetArg("-banscore", DEFAULT_BANSCORE_THRESHOLD); }
1519
/**
1620
* Checks if this CNetAddr is in the whitelist
1721
*
@@ -253,37 +257,34 @@ bool CDoSManager::BannedSetIsDirty()
253257
}
254258

255259
/**
256-
* Set the ban score for this node and if the threshold is exceeded request the node be banned.
257-
* NOTE: This requires an externally taken lock on cs_main to protect the CNodeState returned by
258-
* the internal call to State()
259-
*
260-
* REVISIT: There are numerous new calls to Misbehaving which have been recently added.
261-
* A subsequent PR will be added to ensure the lock on cs_main is properly taken,
262-
* adding unit tests for full coverage of all cases, and adding static analysis
263-
* tags and an AsserLockHeld to help catch/prevent future developer mistakes.
264-
*
265-
* @param[in] pnode Id of the node which is misbehaving, used to look up the CNodeState
266-
* @param[in] howmuch Ban score for the latest infraction against this node
267-
*/
268-
void CDoSManager::Misbehaving(NodeId pnode, int howmuch)
260+
* Increment the misbehaving count score for this node. If the ban threshold is reached, flag the node to be
261+
* banned. No locks are needed to call this function.
262+
*/
263+
void CDoSManager::Misbehaving(CNode *pNode, int howmuch)
269264
{
270-
if (howmuch == 0)
265+
if (howmuch == 0 || !pNode)
271266
return;
272267

273-
CNodeState *state = State(pnode);
274-
if (state == NULL)
275-
return;
268+
int prior(pNode->nMisbehavior.fetch_add(howmuch));
276269

277-
state->nMisbehavior += howmuch;
278-
int banscore = GetArg("-banscore", DEFAULT_BANSCORE_THRESHOLD);
279-
if (state->nMisbehavior >= banscore && state->nMisbehavior - howmuch < banscore)
270+
if (prior + howmuch >= nBanThreshold && prior < nBanThreshold)
280271
{
281-
LogPrintf("%s: %s (%d -> %d) BAN THRESHOLD EXCEEDED\n", __func__, state->name, state->nMisbehavior - howmuch,
282-
state->nMisbehavior);
283-
state->fShouldBan = true;
272+
LogPrintf("%s: %s (%d -> %d) BAN THRESHOLD EXCEEDED\n", __func__, pNode->GetLogName(), prior, prior + howmuch);
273+
pNode->fShouldBan = true;
284274
}
285275
else
286-
LogPrintf("%s: %s (%d -> %d)\n", __func__, state->name, state->nMisbehavior - howmuch, state->nMisbehavior);
276+
LogPrintf("%s: %s (%d -> %d)\n", __func__, pNode->GetLogName(), prior, prior + howmuch);
277+
}
278+
279+
/**
280+
* Increment the misbehaving count score for this node. If the ban threshold is reached, flag the node to be
281+
* banned. No locks are needed to call this function.
282+
*/
283+
void CDoSManager::Misbehaving(NodeId nodeid, int howmuch)
284+
{
285+
CNodeRef nodeRef(connmgr->FindNodeFromId(nodeid));
286+
287+
Misbehaving(nodeRef.get(), howmuch);
287288
}
288289

289290

‎src/dosman.h‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,35 @@ class CDoSManager
3939
mutable CCriticalSection cs_setBanned;
4040
bool setBannedIsDirty;
4141

42+
// If a node's misbehaving count reaches this value, it is flagged for banning.
43+
int nBanThreshold;
44+
4245
public:
46+
CDoSManager();
47+
48+
/**
49+
* Call once the command line is parsed so dosman configures itself appropriately.
50+
*/
51+
void HandleCommandLine();
52+
53+
/**
54+
* Increment the misbehaving score for this node. If the ban threshold is reached, flag the node to be
55+
* banned. No locks are needed to call this function.
56+
*
57+
* @param[in] pNode The node which is misbehaving. No effect if nullptr.
58+
* @param[in] howmuch Incremental misbehaving score for the latest infraction by this node.
59+
*/
60+
void Misbehaving(CNode *pNode, int howmuch);
61+
62+
/**
63+
* Increment the misbehaving score for this node. If the ban threshold is reached, flag the node to be
64+
* banned. No locks are needed to call this function.
65+
*
66+
* @param[in] nodeid The ID of the misbehaving node. No effect if the CNode is no longer present.
67+
* @param[in] howmuch Incremental misbehaving score for the latest infraction by this node.
68+
*/
69+
void Misbehaving(NodeId nodeid, int howmuch);
70+
4371
bool IsWhitelistedRange(const CNetAddr &ip);
4472
void AddWhitelistedRange(const CSubNet &subnet);
4573

@@ -71,9 +99,6 @@ class CDoSManager
7199
//! clean unused entries (if bantime has expired)
72100
void SweepBanned();
73101

74-
/** Increase a node's misbehavior score. */
75-
void Misbehaving(NodeId nodeid, int howmuch);
76-
77102
//! save banlist to disk
78103
void DumpBanlist();
79104
//! load banlist from disk

‎src/expedited.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,7 @@ bool HandleExpeditedRequest(CDataStream &vRecv, CNode *pfrom)
6767

6868
if (!pfrom->ThinBlockCapable() || !IsThinBlocksEnabled())
6969
{
70-
LOCK(cs_main);
71-
dosMan.Misbehaving(pfrom->GetId(), 5);
70+
dosMan.Misbehaving(pfrom, 5);
7271
return false;
7372
}
7473

‎src/init.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
676676
fCheckpointsEnabled = GetBoolArg("-checkpoints", DEFAULT_CHECKPOINTS_ENABLED);
677677

678678
connmgr->HandleCommandLine();
679+
dosMan.HandleCommandLine();
679680

680681
// mempool limits
681682
int64_t nMempoolSizeMax = GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000;

0 commit comments

Comments
 (0)