Skip to content

Commit

Permalink
[net_processing] Pass chainparams to PeerLogicValidation constructor
Browse files Browse the repository at this point in the history 
Keep a references to chainparams, rather than calling the global
Params() function every time it's needed. This is fine, since
globalChainParams does not get updated once it's been set, and it's
available at the point of constructing the PeerLogicValidation object.
  • Loading branch information
@jnewbery
jnewbery committed Aug 28, 2020
1 parent d45f2bb commit e4f6ec6
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 34 deletions.
2 changes: 1 addition & 1 deletion src/init.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1371,7 +1371,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA
node.chainman = &g_chainman;
ChainstateManager& chainman = *Assert(node.chainman);

node.peer_logic.reset(new PeerLogicValidation(*node.connman, node.banman.get(), *node.scheduler, chainman, *node.mempool));
node.peer_logic.reset(new PeerLogicValidation(chainparams, *node.connman, node.banman.get(), *node.scheduler, chainman, *node.mempool));
RegisterValidationInterface(node.peer_logic.get());

// sanitize comments per BIP-0014, format user agent and check total size
Expand Down
48 changes: 25 additions & 23 deletions src/net_processing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1241,8 +1241,10 @@ static bool BlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Para
(GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, consensusParams) < STALE_RELAY_AGE_LIMIT);
}

PeerLogicValidation::PeerLogicValidation(CConnman& connman, BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, CTxMemPool& pool)
: m_connman(connman),
PeerLogicValidation::PeerLogicValidation(const CChainParams& chainparams, CConnman& connman, BanMan* banman,
CScheduler& scheduler, ChainstateManager& chainman, CTxMemPool& pool)
: m_chainparams(chainparams),
m_connman(connman),
m_banman(banman),
m_chainman(chainman),
m_mempool(pool),
Expand Down Expand Up @@ -2347,7 +2349,7 @@ static void ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv, const CChainPar

void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv,
const std::chrono::microseconds time_received,
const CChainParams& chainparams, const std::atomic<bool>& interruptMsgProc)
const std::atomic<bool>& interruptMsgProc)
{
LogPrint(BCLog::NET, "received: %s (%u bytes) peer=%d\n", SanitizeString(msg_type), vRecv.size(), pfrom.GetId());
if (gArgs.IsArgSet("-dropmessagestest") && GetRand(gArgs.GetArg("-dropmessagestest", 0)) == 0)
Expand Down Expand Up @@ -2763,7 +2765,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
}

pfrom.vRecvGetData.insert(pfrom.vRecvGetData.end(), vInv.begin(), vInv.end());
ProcessGetData(pfrom, chainparams, m_connman, m_mempool, interruptMsgProc);
ProcessGetData(pfrom, m_chainparams, m_connman, m_mempool, interruptMsgProc);
return;
}

Expand Down Expand Up @@ -2816,7 +2818,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
}
// If pruning, don't inv blocks unless we have on disk and are likely to still have
// for some reasonable time window (1 hour) that block relay might require.
const int nPrunedBlocksLikelyToHave = MIN_BLOCKS_TO_KEEP - 3600 / chainparams.GetConsensus().nPowTargetSpacing;
const int nPrunedBlocksLikelyToHave = MIN_BLOCKS_TO_KEEP - 3600 / m_chainparams.GetConsensus().nPowTargetSpacing;
if (fPruneMode && (!(pindex->nStatus & BLOCK_HAVE_DATA) || pindex->nHeight <= ::ChainActive().Tip()->nHeight - nPrunedBlocksLikelyToHave))
{
LogPrint(BCLog::NET, " getblocks stopping, pruned or too old block at %d %s\n", pindex->nHeight, pindex->GetBlockHash().ToString());
Expand Down Expand Up @@ -2877,7 +2879,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
}

CBlock block;
bool ret = ReadBlockFromDisk(block, pindex, chainparams.GetConsensus());
bool ret = ReadBlockFromDisk(block, pindex, m_chainparams.GetConsensus());
assert(ret);

SendBlockTransactions(block, req, pfrom, m_connman);
Expand Down Expand Up @@ -2911,7 +2913,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
return;
}

if (!BlockRequestAllowed(pindex, chainparams.GetConsensus())) {
if (!BlockRequestAllowed(pindex, m_chainparams.GetConsensus())) {
LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom.GetId());
return;
}
Expand Down Expand Up @@ -3190,7 +3192,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty

const CBlockIndex *pindex = nullptr;
BlockValidationState state;
if (!m_chainman.ProcessNewBlockHeaders({cmpctblock.header}, state, chainparams, &pindex)) {
if (!m_chainman.ProcessNewBlockHeaders({cmpctblock.header}, state, m_chainparams, &pindex)) {
if (state.IsInvalid()) {
MaybePunishNodeForBlock(pfrom.GetId(), state, /*via_compact_block*/ true, "invalid header via cmpctblock");
return;
Expand Down Expand Up @@ -3246,10 +3248,10 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
}

// If we're not close to tip yet, give up and let parallel block fetch work its magic
if (!fAlreadyInFlight && !CanDirectFetch(chainparams.GetConsensus()))
if (!fAlreadyInFlight && !CanDirectFetch(m_chainparams.GetConsensus()))
return;

if (IsWitnessEnabled(pindex->pprev, chainparams.GetConsensus()) && !nodestate->fSupportsDesiredCmpctVersion) {
if (IsWitnessEnabled(pindex->pprev, m_chainparams.GetConsensus()) && !nodestate->fSupportsDesiredCmpctVersion) {
// Don't bother trying to process compact blocks from v1 peers
// after segwit activates.
return;
Expand Down Expand Up @@ -3333,16 +3335,17 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
}
} // cs_main

if (fProcessBLOCKTXN)
return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, time_received, chainparams, interruptMsgProc);
if (fProcessBLOCKTXN) {
return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, time_received, interruptMsgProc);
}

if (fRevertToHeaderProcessing) {
// Headers received from HB compact block peers are permitted to be
// relayed before full validation (see BIP 152), so we don't want to disconnect
// the peer if the header turns out to be for an invalid block.
// Note that if a peer tries to build on an invalid chain, that
// will be detected and the peer will be disconnected/discouraged.
return ProcessHeadersMessage(pfrom, m_connman, m_chainman, m_mempool, {cmpctblock.header}, chainparams, /*via_compact_block=*/true);
return ProcessHeadersMessage(pfrom, m_connman, m_chainman, m_mempool, {cmpctblock.header}, m_chainparams, /*via_compact_block=*/true);
}

if (fBlockReconstructed) {
Expand All @@ -3362,7 +3365,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
// we have a chain with at least nMinimumChainWork), and we ignore
// compact blocks with less work than our tip, it is safe to treat
// reconstructed compact blocks as having been requested.
m_chainman.ProcessNewBlock(chainparams, pblock, /*fForceProcessing=*/true, &fNewBlock);
m_chainman.ProcessNewBlock(m_chainparams, pblock, /*fForceProcessing=*/true, &fNewBlock);
if (fNewBlock) {
pfrom.nLastBlockTime = GetTime();
} else {
Expand Down Expand Up @@ -3452,7 +3455,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
// disk-space attacks), but this should be safe due to the
// protections in the compact block handler -- see related comment
// in compact block optimistic reconstruction handling.
m_chainman.ProcessNewBlock(chainparams, pblock, /*fForceProcessing=*/true, &fNewBlock);
m_chainman.ProcessNewBlock(m_chainparams, pblock, /*fForceProcessing=*/true, &fNewBlock);
if (fNewBlock) {
pfrom.nLastBlockTime = GetTime();
} else {
Expand Down Expand Up @@ -3485,7 +3488,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
ReadCompactSize(vRecv); // ignore tx count; assume it is 0.
}

return ProcessHeadersMessage(pfrom, m_connman, m_chainman, m_mempool, headers, chainparams, /*via_compact_block=*/false);
return ProcessHeadersMessage(pfrom, m_connman, m_chainman, m_mempool, headers, m_chainparams, /*via_compact_block=*/false);
}

if (msg_type == NetMsgType::BLOCK)
Expand Down Expand Up @@ -3514,7 +3517,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
mapBlockSource.emplace(hash, std::make_pair(pfrom.GetId(), true));
}
bool fNewBlock = false;
m_chainman.ProcessNewBlock(chainparams, pblock, forceProcessing, &fNewBlock);
m_chainman.ProcessNewBlock(m_chainparams, pblock, forceProcessing, &fNewBlock);
if (fNewBlock) {
pfrom.nLastBlockTime = GetTime();
} else {
Expand Down Expand Up @@ -3743,17 +3746,17 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty
}

if (msg_type == NetMsgType::GETCFILTERS) {
ProcessGetCFilters(pfrom, vRecv, chainparams, m_connman);
ProcessGetCFilters(pfrom, vRecv, m_chainparams, m_connman);
return;
}

if (msg_type == NetMsgType::GETCFHEADERS) {
ProcessGetCFHeaders(pfrom, vRecv, chainparams, m_connman);
ProcessGetCFHeaders(pfrom, vRecv, m_chainparams, m_connman);
return;
}

if (msg_type == NetMsgType::GETCFCHECKPT) {
ProcessGetCFCheckPt(pfrom, vRecv, chainparams, m_connman);
ProcessGetCFCheckPt(pfrom, vRecv, m_chainparams, m_connman);
return;
}

Expand Down Expand Up @@ -3831,7 +3834,6 @@ bool PeerLogicValidation::MaybeDiscourageAndDisconnect(CNode& pnode)

bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& interruptMsgProc)
{
const CChainParams& chainparams = Params();
//
// Message format
// (4) message start
Expand All @@ -3843,7 +3845,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
bool fMoreWork = false;

if (!pfrom->vRecvGetData.empty())
ProcessGetData(*pfrom, chainparams, m_connman, m_mempool, interruptMsgProc);
ProcessGetData(*pfrom, m_chainparams, m_connman, m_mempool, interruptMsgProc);

if (!pfrom->orphan_work_set.empty()) {
std::list<CTransactionRef> removed_txn;
Expand Down Expand Up @@ -3908,7 +3910,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
}

try {
ProcessMessage(*pfrom, msg_type, vRecv, msg.m_time, chainparams, interruptMsgProc);
ProcessMessage(*pfrom, msg_type, vRecv, msg.m_time, interruptMsgProc);
if (interruptMsgProc)
return false;
if (!pfrom->vRecvGetData.empty())
Expand Down
7 changes: 4 additions & 3 deletions src/net_processing.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ static const int DISCOURAGEMENT_THRESHOLD{100};

class PeerLogicValidation final : public CValidationInterface, public NetEventsInterface {
public:
PeerLogicValidation(CConnman& connman, BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, CTxMemPool& pool);
PeerLogicValidation(const CChainParams& chainparams, CConnman& connman, BanMan* banman,
CScheduler& scheduler, ChainstateManager& chainman, CTxMemPool& pool);

/**
* Overridden from CValidationInterface.
Expand Down Expand Up @@ -79,8 +80,7 @@ class PeerLogicValidation final : public CValidationInterface, public NetEventsI

/** Process a single message from a peer. Public for fuzz testing */
void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv,
const std::chrono::microseconds time_received, const CChainParams& chainparams,
const std::atomic<bool>& interruptMsgProc);
const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc);

private:
/** Maybe disconnect a peer and discourage future connections from its address.
Expand All @@ -90,6 +90,7 @@ class PeerLogicValidation final : public CValidationInterface, public NetEventsI
*/
bool MaybeDiscourageAndDisconnect(CNode& pnode);

const CChainParams& m_chainparams;
CConnman& m_connman;
/** Pointer to this node's banman. May be nullptr - check existence before dereferencing. */
BanMan* const m_banman;
Expand Down
12 changes: 8 additions & 4 deletions src/test/denialofservice_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,9 @@ BOOST_FIXTURE_TEST_SUITE(denialofservice_tests, TestingSetup)
// work.
BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
{
const CChainParams& chainparams = Params();
auto connman = MakeUnique<CConnman>(0x1337, 0x1337);
auto peerLogic = MakeUnique<PeerLogicValidation>(*connman, nullptr, *m_node.scheduler, *m_node.chainman, *m_node.mempool);
auto peerLogic = MakeUnique<PeerLogicValidation>(chainparams, *connman, nullptr, *m_node.scheduler, *m_node.chainman, *m_node.mempool);

// Mock an outbound peer
CAddress addr1(ip(0xa0b0c001), NODE_NONE);
Expand Down Expand Up @@ -149,8 +150,9 @@ static void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerLogicValidat

BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
{
const CChainParams& chainparams = Params();
auto connman = MakeUnique<CConnmanTest>(0x1337, 0x1337);
auto peerLogic = MakeUnique<PeerLogicValidation>(*connman, nullptr, *m_node.scheduler, *m_node.chainman, *m_node.mempool);
auto peerLogic = MakeUnique<PeerLogicValidation>(chainparams, *connman, nullptr, *m_node.scheduler, *m_node.chainman, *m_node.mempool);

const Consensus::Params& consensusParams = Params().GetConsensus();
constexpr int max_outbound_full_relay = MAX_OUTBOUND_FULL_RELAY_CONNECTIONS;
Expand Down Expand Up @@ -221,9 +223,10 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)

BOOST_AUTO_TEST_CASE(peer_discouragement)
{
const CChainParams& chainparams = Params();
auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
auto connman = MakeUnique<CConnman>(0x1337, 0x1337);
auto peerLogic = MakeUnique<PeerLogicValidation>(*connman, banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool);
auto peerLogic = MakeUnique<PeerLogicValidation>(chainparams, *connman, banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool);

banman->ClearBanned();
CAddress addr1(ip(0xa0b0c001), NODE_NONE);
Expand Down Expand Up @@ -268,9 +271,10 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)

BOOST_AUTO_TEST_CASE(DoS_bantime)
{
const CChainParams& chainparams = Params();
auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
auto connman = MakeUnique<CConnman>(0x1337, 0x1337);
auto peerLogic = MakeUnique<PeerLogicValidation>(*connman, banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool);
auto peerLogic = MakeUnique<PeerLogicValidation>(chainparams, *connman, banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool);

banman->ClearBanned();
int64_t nStartTime = GetTime();
Expand Down
3 changes: 1 addition & 2 deletions src/test/fuzz/process_message.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
g_setup->m_node.peer_logic->InitializeNode(&p2p_node);
try {
g_setup->m_node.peer_logic->ProcessMessage(p2p_node, random_message_type, random_bytes_data_stream,
GetTime<std::chrono::microseconds>(), Params(),
std::atomic<bool>{false});
GetTime<std::chrono::microseconds>(), std::atomic<bool>{false});
} catch (const std::ios_base::failure&) {
}
SyncWithValidationInterfaceQueue();
Expand Down
2 changes: 1 addition & 1 deletion src/test/util/setup_common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
m_node.mempool->setSanityCheck(1.0);
m_node.banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
m_node.connman = MakeUnique<CConnman>(0x1337, 0x1337); // Deterministic randomness for tests.
m_node.peer_logic = MakeUnique<PeerLogicValidation>(*m_node.connman, m_node.banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool);
m_node.peer_logic = MakeUnique<PeerLogicValidation>(chainparams, *m_node.connman, m_node.banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool);
{
CConnman::Options options;
options.m_msgproc = m_node.peer_logic.get();
Expand Down

0 comments on commit e4f6ec6

Please sign in to comment.