Skip to content

[net] Tighten scope in net_processing #13417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sipa merged 4 commits into from
Jul 14, 2018
Merged

[net] Tighten scope in net_processing #13417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
02bbc05
Rescope g_enable_bip61 to net_processing
skeees Jun 8, 2018
1d4df02
[move-only] Move things only referenced in net_processing out of head…
skeees Jun 7, 2018
6690a28
Restrict as much as possible in net_processing to translation unit
skeees Jun 8, 2018
3339ba2
Make g_enable_bip61 a member variable of PeerLogicValidation
skeees Jul 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1134,8 +1134,6 @@ bool AppInitParameterInteraction()
if (gArgs.GetBoolArg("-peerbloomfilters", DEFAULT_PEERBLOOMFILTERS))
nLocalServices = ServiceFlags(nLocalServices | NODE_BLOOM);

g_enable_bip61 = gArgs.GetBoolArg("-enablebip61", DEFAULT_ENABLE_BIP61);

if (gArgs.GetArg("-rpcserialversion", DEFAULT_RPC_SERIALIZE_VERSION) < 0)
return InitError("rpcserialversion must be non-negative.");

Expand Down Expand Up @@ -1319,7 +1317,7 @@ bool AppInitMain()
g_connman = std::unique_ptr<CConnman>(new CConnman(GetRand(std::numeric_limits<uint64_t>::max()), GetRand(std::numeric_limits<uint64_t>::max())));
CConnman& connman = *g_connman;

peerLogic.reset(new PeerLogicValidation(&connman, scheduler));
peerLogic.reset(new PeerLogicValidation(&connman, scheduler, gArgs.GetBoolArg("-enablebip61", DEFAULT_ENABLE_BIP61)));
RegisterValidationInterface(peerLogic.get());

// sanitize comments per BIP-0014, format user agent and check total size
Expand Down
111 changes: 68 additions & 43 deletions src/net_processing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,34 @@
# error "Bitcoin cannot be compiled without assertions."
#endif

std::atomic<int64_t> nTimeBestReceived(0); // Used only to inform the wallet of when we last received a block
bool g_enable_bip61 = DEFAULT_ENABLE_BIP61;

struct IteratorComparator
{
template<typename I>
bool operator()(const I& a, const I& b) const
{
return &(*a) < &(*b);
}
};
/** Expiration time for orphan transactions in seconds */
static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60;
/** Minimum time between orphan transactions expire time checks in seconds */
static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60;
/** Headers download timeout expressed in microseconds
* Timeout = base + per_header * (expected number of headers) */
static constexpr int64_t HEADERS_DOWNLOAD_TIMEOUT_BASE = 15 * 60 * 1000000; // 15 minutes
static constexpr int64_t HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER = 1000; // 1ms/header
/** Protect at least this many outbound peers from disconnection due to slow/
* behind headers chain.
*/
static constexpr int32_t MAX_OUTBOUND_PEERS_TO_PROTECT_FROM_DISCONNECT = 4;
/** Timeout for (unprotected) outbound peers to sync to our chainwork, in seconds */
static constexpr int64_t CHAIN_SYNC_TIMEOUT = 20 * 60; // 20 minutes
/** How frequently to check for stale tips, in seconds */
static constexpr int64_t STALE_CHECK_INTERVAL = 10 * 60; // 10 minutes
/** How frequently to check for extra outbound peers and disconnect, in seconds */
static constexpr int64_t EXTRA_PEER_CHECK_INTERVAL = 45;
/** Minimum time an outbound-peer-eviction candidate must be connected for, in order to evict, in seconds */
static constexpr int64_t MINIMUM_CONNECT_TIME = 30;
/** SHA256("main address relay")[0:8] */
static constexpr uint64_t RANDOMIZER_ID_ADDRESS_RELAY = 0x3cac0035b5866b90ULL;
/// Age after which a stale block will no longer be served if requested as
/// protection against fingerprinting. Set to one month, denominated in seconds.
static constexpr int STALE_RELAY_AGE_LIMIT = 30 * 24 * 60 * 60;
/// Age after which a block is considered historical for purposes of rate
/// limiting block relay. Set to one week, denominated in seconds.
static constexpr int HISTORICAL_BLOCK_AGE = 7 * 24 * 60 * 60;

struct COrphanTx {
// When modifying, adapt the copy of this definition in tests/DoS_tests.
Expand All @@ -56,21 +73,11 @@ struct COrphanTx {
};
static CCriticalSection g_cs_orphans;
std::map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(g_cs_orphans);
std::map<COutPoint, std::set<std::map<uint256, COrphanTx>::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(g_cs_orphans);
void EraseOrphansFor(NodeId peer);

static size_t vExtraTxnForCompactIt GUARDED_BY(g_cs_orphans) = 0;
static std::vector<std::pair<uint256, CTransactionRef>> vExtraTxnForCompact GUARDED_BY(g_cs_orphans);

static const uint64_t RANDOMIZER_ID_ADDRESS_RELAY = 0x3cac0035b5866b90ULL; // SHA256("main address relay")[0:8]

/// Age after which a stale block will no longer be served if requested as
/// protection against fingerprinting. Set to one month, denominated in seconds.
static const int STALE_RELAY_AGE_LIMIT = 30 * 24 * 60 * 60;
void EraseOrphansFor(NodeId peer);

/// Age after which a block is considered historical for purposes of rate
/// limiting block relay. Set to one week, denominated in seconds.
static const int HISTORICAL_BLOCK_AGE = 7 * 24 * 60 * 60;
/** Increase a node's misbehavior score. */
void Misbehaving(NodeId nodeid, int howmuch, const std::string& message="");

// Internal stuff
namespace {
Expand Down Expand Up @@ -138,10 +145,24 @@ namespace {
MapRelay mapRelay;
/** Expiration-time ordered list of (expire time, relay map entry) pairs, protected by cs_main). */
std::deque<std::pair<int64_t, MapRelay::iterator>> vRelayExpiration;

std::atomic<int64_t> nTimeBestReceived(0); // Used only to inform the wallet of when we last received a block

struct IteratorComparator
{
template<typename I>
bool operator()(const I& a, const I& b) const
{
return &(*a) < &(*b);
}
};
std::map<COutPoint, std::set<std::map<uint256, COrphanTx>::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(g_cs_orphans);

static size_t vExtraTxnForCompactIt GUARDED_BY(g_cs_orphans) = 0;
static std::vector<std::pair<uint256, CTransactionRef>> vExtraTxnForCompact GUARDED_BY(g_cs_orphans);
} // namespace

namespace {

struct CBlockReject {
unsigned char chRejectCode;
std::string strRejectReason;
Expand Down Expand Up @@ -268,10 +289,10 @@ struct CNodeState {
};

/** Map maintaining per-node state. Requires cs_main. */
std::map<NodeId, CNodeState> mapNodeState;
static std::map<NodeId, CNodeState> mapNodeState;

// Requires cs_main.
CNodeState *State(NodeId pnode) {
static CNodeState *State(NodeId pnode) {
std::map<NodeId, CNodeState>::iterator it = mapNodeState.find(pnode);
if (it == mapNodeState.end())
return nullptr;
Expand Down Expand Up @@ -810,7 +831,9 @@ static bool BlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Para
(GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, consensusParams) < STALE_RELAY_AGE_LIMIT);
}

PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, CScheduler &scheduler) : connman(connmanIn), m_stale_tip_check_time(0) {
PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, CScheduler &scheduler, bool enable_bip61)
: connman(connmanIn), m_stale_tip_check_time(0), m_enable_bip61(enable_bip61) {

// Initialize global variables that cannot be constructed at startup.
recentRejects.reset(new CRollingBloomFilter(120000, 0.000001));

Expand Down Expand Up @@ -867,7 +890,7 @@ static uint256 most_recent_block_hash;
static bool fWitnessesPresentInMostRecentCompactBlock;

/**
* Maintain state about the best-seen block and fast-announce a compact block
* Maintain state about the best-seen block and fast-announce a compact block

@promag promag Jun 10, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit "[net] Tighten scope in net_processing"

Could not include these changes.

* to compatible peers.
*/
void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock>& pblock) {
Expand Down Expand Up @@ -912,7 +935,7 @@ void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std:
}

/**
* Update our best height and announce any block hashes which weren't previously
* Update our best height and announce any block hashes which weren't previously
* in chainActive to our peers.
*/
void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) {
Expand Down Expand Up @@ -948,7 +971,7 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CB
}

/**
* Handle invalid block rejection and consequent peer banning, maintain which
* Handle invalid block rejection and consequent peer banning, maintain which
* peers announce compact blocks.
*/
void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationState& state) {
Expand Down Expand Up @@ -1538,7 +1561,7 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve
return true;
}

bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, CConnman* connman, const std::atomic<bool>& interruptMsgProc)
bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, CConnman* connman, const std::atomic<bool>& interruptMsgProc, bool enable_bip61)
{
LogPrint(BCLog::NET, "received: %s (%u bytes) peer=%d\n", SanitizeString(strCommand), vRecv.size(), pfrom->GetId());
if (gArgs.IsArgSet("-dropmessagestest") && GetRand(gArgs.GetArg("-dropmessagestest", 0)) == 0)
Expand Down Expand Up @@ -1592,7 +1615,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
// Each connection can only send one version message
if (pfrom->nVersion != 0)
{
if (g_enable_bip61) {
if (enable_bip61) {
connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_DUPLICATE, std::string("Duplicate version message")));
}
LOCK(cs_main);
Expand Down Expand Up @@ -1623,7 +1646,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
if (!pfrom->fInbound && !pfrom->fFeeler && !pfrom->m_manual_connection && !HasAllDesirableServiceFlags(nServices))
{
LogPrint(BCLog::NET, "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom->GetId(), nServices, GetDesirableServiceFlags(nServices));
if (g_enable_bip61) {
if (enable_bip61) {
connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_NONSTANDARD,
strprintf("Expected to offer services %08x", GetDesirableServiceFlags(nServices))));
}
Expand All @@ -1646,7 +1669,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
{
// disconnect from peers older than this proto version
LogPrint(BCLog::NET, "peer=%d using obsolete version %i; disconnecting\n", pfrom->GetId(), nVersion);
if (g_enable_bip61) {
if (enable_bip61) {
connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_OBSOLETE,
strprintf("Version must be %d or greater", MIN_PEER_PROTO_VERSION)));
}
Expand Down Expand Up @@ -2347,7 +2370,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
LogPrint(BCLog::MEMPOOLREJ, "%s from peer=%d was not accepted: %s\n", tx.GetHash().ToString(),
pfrom->GetId(),
FormatStateMessage(state));
if (g_enable_bip61 && state.GetRejectCode() > 0 && state.GetRejectCode() < REJECT_INTERNAL) { // Never send AcceptToMemoryPool's internal codes over P2P
if (enable_bip61 && state.GetRejectCode() > 0 && state.GetRejectCode() < REJECT_INTERNAL) { // Never send AcceptToMemoryPool's internal codes over P2P
connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::REJECT, strCommand, (unsigned char)state.GetRejectCode(),
state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), inv.hash));
}
Expand Down Expand Up @@ -2532,7 +2555,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
} // cs_main

if (fProcessBLOCKTXN)
return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, nTimeReceived, chainparams, connman, interruptMsgProc);
return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, nTimeReceived, chainparams, connman, interruptMsgProc, enable_bip61);

if (fRevertToHeaderProcessing) {
// Headers received from HB compact block peers are permitted to be
Expand Down Expand Up @@ -2918,12 +2941,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
return true;
}

static bool SendRejectsAndCheckIfBanned(CNode* pnode, CConnman* connman)
static bool SendRejectsAndCheckIfBanned(CNode* pnode, CConnman* connman, bool enable_bip61)
{
AssertLockHeld(cs_main);
CNodeState &state = *State(pnode->GetId());

if (g_enable_bip61) {
if (enable_bip61) {
for (const CBlockReject& reject : state.rejects) {
connman->PushMessage(pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, std::string(NetMsgType::BLOCK), reject.chRejectCode, reject.strRejectReason, reject.hashBlock));
}
Expand Down Expand Up @@ -3025,15 +3048,15 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
bool fRet = false;
try
{
fRet = ProcessMessage(pfrom, strCommand, vRecv, msg.nTime, chainparams, connman, interruptMsgProc);
fRet = ProcessMessage(pfrom, strCommand, vRecv, msg.nTime, chainparams, connman, interruptMsgProc, m_enable_bip61);
if (interruptMsgProc)
return false;
if (!pfrom->vRecvGetData.empty())
fMoreWork = true;
}
catch (const std::ios_base::failure& e)
{
if (g_enable_bip61) {
if (m_enable_bip61) {
connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_MALFORMED, std::string("error parsing message")));
}
if (strstr(e.what(), "end of data"))
Expand Down Expand Up @@ -3067,7 +3090,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
}

LOCK(cs_main);
SendRejectsAndCheckIfBanned(pfrom, connman);
SendRejectsAndCheckIfBanned(pfrom, connman, m_enable_bip61);

return fMoreWork;
}
Expand Down Expand Up @@ -3202,6 +3225,7 @@ void PeerLogicValidation::CheckForStaleTipAndEvictPeers(const Consensus::Params
}
}

namespace {
class CompareInvMempoolOrder
{
CTxMemPool *mp;
Expand All @@ -3218,6 +3242,7 @@ class CompareInvMempoolOrder
return mp->CompareDepthAndScore(*b, *a);
}
};
}

bool PeerLogicValidation::SendMessages(CNode* pto, std::atomic<bool>& interruptMsgProc)
{
Expand Down Expand Up @@ -3263,7 +3288,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto, std::atomic<bool>& interruptM
if (!lockMain)
return true;

if (SendRejectsAndCheckIfBanned(pto, connman))
if (SendRejectsAndCheckIfBanned(pto, connman, m_enable_bip61))
return true;
CNodeState &state = *State(pto->GetId());

Expand Down
30 changes: 4 additions & 26 deletions src/net_processing.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,40 +12,17 @@

/** Default for -maxorphantx, maximum number of orphan transactions kept in memory */
static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100;
/** Expiration time for orphan transactions in seconds */
static const int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60;
/** Minimum time between orphan transactions expire time checks in seconds */
static const int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60;
/** Default number of orphan+recently-replaced txn to keep around for block reconstruction */
static const unsigned int DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN = 100;
/** Headers download timeout expressed in microseconds
* Timeout = base + per_header * (expected number of headers) */
static constexpr int64_t HEADERS_DOWNLOAD_TIMEOUT_BASE = 15 * 60 * 1000000; // 15 minutes
static constexpr int64_t HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER = 1000; // 1ms/header
/** Protect at least this many outbound peers from disconnection due to slow/
* behind headers chain.
*/
static constexpr int32_t MAX_OUTBOUND_PEERS_TO_PROTECT_FROM_DISCONNECT = 4;
/** Timeout for (unprotected) outbound peers to sync to our chainwork, in seconds */
static constexpr int64_t CHAIN_SYNC_TIMEOUT = 20 * 60; // 20 minutes
/** How frequently to check for stale tips, in seconds */
static constexpr int64_t STALE_CHECK_INTERVAL = 10 * 60; // 10 minutes
/** How frequently to check for extra outbound peers and disconnect, in seconds */
static constexpr int64_t EXTRA_PEER_CHECK_INTERVAL = 45;
/** Minimum time an outbound-peer-eviction candidate must be connected for, in order to evict, in seconds */
static constexpr int64_t MINIMUM_CONNECT_TIME = 30;

/** Default for BIP61 (sending reject messages) */
static constexpr bool DEFAULT_ENABLE_BIP61 = true;
/** Enable BIP61 (sending reject messages) */
extern bool g_enable_bip61;

class PeerLogicValidation final : public CValidationInterface, public NetEventsInterface {
private:
CConnman* const connman;

public:
explicit PeerLogicValidation(CConnman* connman, CScheduler &scheduler);
explicit PeerLogicValidation(CConnman* connman, CScheduler &scheduler, bool enable_bip61);

/**
* Overridden from CValidationInterface.
Expand Down Expand Up @@ -88,6 +65,9 @@ class PeerLogicValidation final : public CValidationInterface, public NetEventsI

private:
int64_t m_stale_tip_check_time; //! Next time to check for stale tip

/** Enable BIP61 (sending reject messages) */
const bool m_enable_bip61;
};

struct CNodeStateStats {
Expand All @@ -99,7 +79,5 @@ struct CNodeStateStats {

/** Get statistics from node state */
bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats);
/** Increase a node's misbehavior score. */
void Misbehaving(NodeId nodeid, int howmuch, const std::string& message="");

#endif // BITCOIN_NET_PROCESSING_H
2 changes: 2 additions & 0 deletions src/test/denialofservice_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
extern bool AddOrphanTx(const CTransactionRef& tx, NodeId peer);
extern void EraseOrphansFor(NodeId peer);
extern unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans);
extern void Misbehaving(NodeId nodeid, int howmuch, const std::string& message="");

struct COrphanTx {
CTransactionRef tx;
NodeId fromPeer;
Expand Down
2 changes: 1 addition & 1 deletion src/test/test_bitcoin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha
threadGroup.create_thread(&ThreadScriptCheck);
g_connman = std::unique_ptr<CConnman>(new CConnman(0x1337, 0x1337)); // Deterministic randomness for tests.
connman = g_connman.get();
peerLogic.reset(new PeerLogicValidation(connman, scheduler));
peerLogic.reset(new PeerLogicValidation(connman, scheduler, /*enable_bip61=*/true));
}

TestingSetup::~TestingSetup()
Expand Down