boxtec
/
bitcoin
mirror of https://github.com/bitcoin/bitcoin
4
1
Fork 0
Code Issues Projects Releases Wiki Activity

[net processing] Move misbehavior tracking state to Peer

Browse Source 
Misbehavior tracking state is now contained in Peer instead of
CNode. It is no longer guarded by cs_main, but instead by a
dedicated m_misbehavior_mutex lock.

This allows us to remove 14 cs_main locks from net_processing.
pull/19607/head
John Newbery 4 years ago
parent 7cd4159ac8
commit 1f96d2e673
2 changed files with 53 additions and 72 deletions
Show Stats Download Patch File Download Diff File

105
src/net_processing.cpp
Unescape View File

@ -276,10 +276,6 @@ struct CNodeState {
const CService address;
//! Whether we have a fully established connection.
bool fCurrentlyConnected;
//! Accumulated misbehaviour score for this peer.
int nMisbehavior;
//! Whether this peer should be disconnected and marked as discouraged (unless it has the noban permission).
bool m_should_discourage;
//! The best known block we know this peer has announced.
const CBlockIndex *pindexBestKnownBlock;
//! The hash of the last unknown block this peer has announced.
@ -432,8 +428,6 @@ struct CNodeState {
: address(addrIn), m_is_inbound(is_inbound), m_is_manual_connection(is_manual)
{
fCurrentlyConnected = false;
nMisbehavior = 0;
m_should_discourage = false;
pindexBestKnownBlock = nullptr;
hashLastUnknownBlock.SetNull();
pindexLastCommonBlock = nullptr;
@ -485,6 +479,13 @@ struct Peer {
/** Same id as the CNode object for this peer */
const NodeId m_id{0};
/** Protects misbehavior data members */
Mutex m_misbehavior_mutex;
/** Accumulated misbehavior score for this peer */
int nMisbehavior GUARDED_BY(m_misbehavior_mutex){0};
/** Whether this peer should be disconnected and marked as discouraged (unless it has the noban permission). */
bool m_should_discourage GUARDED_BY(m_misbehavior_mutex){false};
Peer(NodeId id) : m_id(id) {}
};
@ -907,7 +908,11 @@ void PeerLogicValidation::ReattemptInitialBroadcast(CScheduler& scheduler) const
void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime) {
fUpdateConnectionTime = false;
LOCK(cs_main);
int misbehavior{0};
{
PeerRef peer = GetPeerRef(nodeid);
assert(peer != nullptr);
misbehavior = WITH_LOCK(peer->m_misbehavior_mutex, return peer->nMisbehavior);
LOCK(g_peer_mutex);
g_peer_map.erase(nodeid);
}
@ -917,7 +922,7 @@ void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTim
if (state->fSyncStarted)
nSyncStarted--;
if (state->nMisbehavior == 0 && state->fCurrentlyConnected) {
if (misbehavior == 0 && state->fCurrentlyConnected) {
fUpdateConnectionTime = true;
}
@ -947,17 +952,23 @@ void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTim
}
bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats) {
LOCK(cs_main);
CNodeState *state = State(nodeid);
if (state == nullptr)
return false;
stats.nMisbehavior = state->nMisbehavior;
stats.nSyncHeight = state->pindexBestKnownBlock ? state->pindexBestKnownBlock->nHeight : -1;
stats.nCommonHeight = state->pindexLastCommonBlock ? state->pindexLastCommonBlock->nHeight : -1;
for (const QueuedBlock& queue : state->vBlocksInFlight) {
if (queue.pindex)
stats.vHeightInFlight.push_back(queue.pindex->nHeight);
{
LOCK(cs_main);
CNodeState* state = State(nodeid);
if (state == nullptr)
return false;
stats.nSyncHeight = state->pindexBestKnownBlock ? state->pindexBestKnownBlock->nHeight : -1;
stats.nCommonHeight = state->pindexLastCommonBlock ? state->pindexLastCommonBlock->nHeight : -1;
for (const QueuedBlock& queue : state->vBlocksInFlight) {
if (queue.pindex)
stats.vHeightInFlight.push_back(queue.pindex->nHeight);
}
}
PeerRef peer = GetPeerRef(nodeid);
if (peer == nullptr) return false;
stats.nMisbehavior = WITH_LOCK(peer->m_misbehavior_mutex, return peer->nMisbehavior);
return true;
}
@ -1101,21 +1112,21 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
* 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.
*/
void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message)
{
assert(howmuch > 0);
CNodeState* const state = State(pnode);
if (state == nullptr) return;
PeerRef peer = GetPeerRef(pnode);
if (peer == nullptr) return;
state->nMisbehavior += howmuch;
LOCK(peer->m_misbehavior_mutex);
peer->nMisbehavior += howmuch;
const std::string message_prefixed = message.empty() ? "" : (": " + message);
if (state->nMisbehavior >= DISCOURAGEMENT_THRESHOLD && state->nMisbehavior - howmuch < DISCOURAGEMENT_THRESHOLD)
{
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;
if (peer->nMisbehavior >= DISCOURAGEMENT_THRESHOLD && peer->nMisbehavior - howmuch < DISCOURAGEMENT_THRESHOLD) {
LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", pnode, peer->nMisbehavior - howmuch, peer->nMisbehavior, message_prefixed);
peer->m_should_discourage = true;
} else {
LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d)%s\n", pnode, state->nMisbehavior - howmuch, state->nMisbehavior, message_prefixed);
LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d)%s\n", pnode, peer->nMisbehavior - howmuch, peer->nMisbehavior, message_prefixed);
}
}
@ -1137,7 +1148,6 @@ static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& s
case BlockValidationResult::BLOCK_CONSENSUS:
case BlockValidationResult::BLOCK_MUTATED:
if (!via_compact_block) {
LOCK(cs_main);
Misbehaving(nodeid, 100, message);
return true;
}
@ -1161,18 +1171,12 @@ static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& s
case BlockValidationResult::BLOCK_INVALID_HEADER:
case BlockValidationResult::BLOCK_CHECKPOINT:
case BlockValidationResult::BLOCK_INVALID_PREV:
{
LOCK(cs_main);
Misbehaving(nodeid, 100, message);
}
Misbehaving(nodeid, 100, message);
return true;
// Conflicting (but not necessarily invalid) data or different policy:
case BlockValidationResult::BLOCK_MISSING_PREV:
{
// TODO: Handle this much more gracefully (10 DoS points is super arbitrary)
LOCK(cs_main);
Misbehaving(nodeid, 10, message);
}
// TODO: Handle this much more gracefully (10 DoS points is super arbitrary)
Misbehaving(nodeid, 10, message);
return true;
case BlockValidationResult::BLOCK_RECENT_CONSENSUS_CHANGE:
case BlockValidationResult::BLOCK_TIME_FUTURE:
@ -1196,11 +1200,8 @@ static bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state,
break;
// The node is providing invalid data:
case TxValidationResult::TX_CONSENSUS:
{
LOCK(cs_main);
Misbehaving(nodeid, 100, message);
return true;
}
Misbehaving(nodeid, 100, message);
return true;
// Conflicting (but not necessarily invalid) data or different policy:
case TxValidationResult::TX_RECENT_CONSENSUS_CHANGE:
case TxValidationResult::TX_INPUTS_NOT_STANDARD:
@ -1847,7 +1848,6 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac
BlockTransactions resp(req);
for (size_t i = 0; i < req.indexes.size(); i++) {
if (req.indexes[i] >= block.vtx.size()) {
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 100, "getblocktxn with out-of-bounds tx indices");
return;
}
@ -2368,7 +2368,6 @@ void ProcessMessage(
// Each connection can only send one version message
if (pfrom.nVersion != 0)
{
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 1, "redundant version message");
return;
}
@ -2528,7 +2527,6 @@ void ProcessMessage(
if (pfrom.nVersion == 0) {
// Must have a version message before anything else
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 1, "non-version message before version handshake");
return;
}
@ -2595,7 +2593,6 @@ void ProcessMessage(
if (!pfrom.fSuccessfullyConnected) {
// Must have a verack message before anything else
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 1, "non-verack message before version handshake");
return;
}
@ -2609,7 +2606,6 @@ void ProcessMessage(
}
if (vAddr.size() > MAX_ADDR_TO_SEND)
{
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 20, strprintf("addr message size = %u", vAddr.size()));
return;
}
@ -2688,7 +2684,6 @@ void ProcessMessage(
vRecv >> vInv;
if (vInv.size() > MAX_INV_SZ)
{
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 20, strprintf("inv message size = %u", vInv.size()));
return;
}
@ -2764,7 +2759,6 @@ void ProcessMessage(
vRecv >> vInv;
if (vInv.size() > MAX_INV_SZ)
{
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 20, strprintf("getdata message size = %u", vInv.size()));
return;
}
@ -3489,7 +3483,6 @@ void ProcessMessage(
// Bypass the normal CBlock deserialization, as we don't want to risk deserializing 2000 full blocks.
unsigned int nCount = ReadCompactSize(vRecv);
if (nCount > MAX_HEADERS_RESULTS) {
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 20, strprintf("headers message size = %u", nCount));
return;
}
@ -3691,7 +3684,6 @@ void ProcessMessage(
if (!filter.IsWithinSizeConstraints())
{
// There is no excuse for sending a too-large filter
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 100, "too-large bloom filter");
}
else if (pfrom.m_tx_relay != nullptr)
@ -3725,7 +3717,6 @@ void ProcessMessage(
}
}
if (bad) {
LOCK(cs_main);
Misbehaving(pfrom.GetId(), 100, "bad filteradd message");
}
return;
@ -3811,15 +3802,17 @@ void ProcessMessage(
bool PeerLogicValidation::MaybeDiscourageAndDisconnect(CNode& pnode)
{
const NodeId peer_id{pnode.GetId()};
PeerRef peer = GetPeerRef(peer_id);
if (peer == nullptr) return false;
{
LOCK(cs_main);
CNodeState& state = *State(peer_id);
LOCK(peer->m_misbehavior_mutex);
// There's nothing to do if the m_should_discourage flag isn't set
if (!state.m_should_discourage) return false;
if (!peer->m_should_discourage) return false;
state.m_should_discourage = false;
} // cs_main
peer->m_should_discourage = false;
} // peer.m_misbehavior_mutex
if (pnode.HasPermission(PF_NOBAN)) {
// We never disconnect or discourage peers for bad behavior if they have the NOBAN permission flag

20
src/test/denialofservice_tests.cpp
Unescape View File

@ -232,10 +232,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
peerLogic->InitializeNode(&dummyNode1);
dummyNode1.nVersion = 1;
dummyNode1.fSuccessfullyConnected = true;
{
LOCK(cs_main);
Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged
}
Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged
{
LOCK(dummyNode1.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
@ -249,20 +246,14 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
peerLogic->InitializeNode(&dummyNode2);
dummyNode2.nVersion = 1;
dummyNode2.fSuccessfullyConnected = true;
{
LOCK(cs_main);
Misbehaving(dummyNode2.GetId(), DISCOURAGEMENT_THRESHOLD - 1);
}
Misbehaving(dummyNode2.GetId(), DISCOURAGEMENT_THRESHOLD - 1);
{
LOCK(dummyNode2.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode2));
}
BOOST_CHECK(!banman->IsDiscouraged(addr2)); // 2 not discouraged yet...
BOOST_CHECK(banman->IsDiscouraged(addr1)); // ... but 1 still should be
{
LOCK(cs_main);
Misbehaving(dummyNode2.GetId(), 1); // 2 reaches discouragement threshold
}
Misbehaving(dummyNode2.GetId(), 1); // 2 reaches discouragement threshold
{
LOCK(dummyNode2.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode2));
@ -292,10 +283,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
dummyNode.nVersion = 1;
dummyNode.fSuccessfullyConnected = true;
{
LOCK(cs_main);
Misbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD);
}
Misbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD);
{
LOCK(dummyNode.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode));

Write Preview
Loading…
Cancel
Save