Skip to content

Commit

Permalink
Replace the use of fWhitelisted by permission checks
Browse files Browse the repository at this point in the history
Summary:
This is a partial backport of Core [[bitcoin/bitcoin#16248 | PR16248]] : bitcoin/bitcoin@d541fa3

Depends on D5928

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D5932
  • Loading branch information
NicolasDorier authored and deadalnix committed May 2, 2020
1 parent aab89ee commit 2d7438d
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 31 deletions.
14 changes: 10 additions & 4 deletions src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ void CNode::copyStats(CNodeStats &stats) {
stats.mapRecvBytesPerMsgCmd = mapRecvBytesPerMsgCmd;
stats.nRecvBytes = nRecvBytes;
}
stats.fWhitelisted = fWhitelisted;
stats.m_legacyWhitelisted = m_legacyWhitelisted;
stats.m_permissionFlags = m_permissionFlags;
{
LOCK(cs_feeFilter);
Expand Down Expand Up @@ -868,8 +868,14 @@ bool CConnman::AttemptToEvictConnection() {
{
LOCK(cs_vNodes);

for (CNode *node : vNodes) {
if (node->fWhitelisted || !node->fInbound || node->fDisconnect) {
for (const CNode *node : vNodes) {
if (node->HasPermission(PF_NOBAN)) {
continue;
}
if (!node->fInbound) {
continue;
}
if (node->fDisconnect) {
continue;
}
LOCK(node->cs_filter);
Expand Down Expand Up @@ -1074,7 +1080,7 @@ void CConnman::AcceptConnection(const ListenSocket &hListenSocket) {
pnode->m_permissionFlags = permissionFlags;
// If this flag is present, the user probably expect that RPC and QT report
// it as whitelisted (backward compatibility)
pnode->fWhitelisted = legacyWhitelisted;
pnode->m_legacyWhitelisted = legacyWhitelisted;
pnode->m_prefer_evict = bannedlevel > 0;
m_msgproc->InitializeNode(*config, pnode);

Expand Down
7 changes: 4 additions & 3 deletions src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ struct CNodeStats {
uint64_t nRecvBytes;
mapMsgCmdSize mapRecvBytesPerMsgCmd;
NetPermissionFlags m_permissionFlags;
bool fWhitelisted;
bool m_legacyWhitelisted;
double dPingTime;
double dPingWait;
double dMinPing;
Expand Down Expand Up @@ -689,8 +689,9 @@ class CNode {
bool HasPermission(NetPermissionFlags permission) const {
return NetPermissions::HasFlag(m_permissionFlags, permission);
}
// This peer can bypass DoS banning.
bool fWhitelisted{false};
// This boolean is unusued in actual processing, only present for backward
// compatibility at RPC/QT level
bool m_legacyWhitelisted{false};
// If true this node is being used as a short lived feeler.
bool fFeeler{false};
bool fOneShot{false};
Expand Down
40 changes: 19 additions & 21 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -476,8 +476,9 @@ static void UpdatePreferredDownload(CNode *node, CNodeState *state)
nPreferredDownload -= state->fPreferredDownload;

// Whether this node should be marked as a preferred download node.
state->fPreferredDownload = (!node->fInbound || node->fWhitelisted) &&
!node->fOneShot && !node->fClient;
state->fPreferredDownload =
(!node->fInbound || node->HasPermission(PF_NOBAN)) && !node->fOneShot &&
!node->fClient;

nPreferredDownload += state->fPreferredDownload;
}
Expand Down Expand Up @@ -1569,7 +1570,7 @@ static void ProcessGetBlockData(const Config &config, CNode *pfrom,
(pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() >
HISTORICAL_BLOCK_AGE)) ||
inv.type == MSG_FILTERED_BLOCK) &&
!pfrom->fWhitelisted) {
!pfrom->HasPermission(PF_NOBAN)) {
LogPrint(BCLog::NET,
"historical block serving limit reached, disconnect peer=%d\n",
pfrom->GetId());
Expand All @@ -1581,7 +1582,7 @@ static void ProcessGetBlockData(const Config &config, CNode *pfrom,
// Avoid leaking prune-height by never sending blocks below the
// NODE_NETWORK_LIMITED threshold.
// Add two blocks buffer extension for possible races
if (send && !pfrom->fWhitelisted &&
if (send && !pfrom->HasPermission(PF_NOBAN) &&
((((pfrom->GetLocalServices() & NODE_NETWORK_LIMITED) ==
NODE_NETWORK_LIMITED) &&
((pfrom->GetLocalServices() & NODE_NETWORK) != NODE_NETWORK) &&
Expand Down Expand Up @@ -2470,8 +2471,7 @@ static bool ProcessMessage(const Config &config, CNode *pfrom,

// Allow whitelisted peers to send data other than blocks in blocks only
// mode if whitelistrelay is true
if (pfrom->fWhitelisted &&
gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY)) {
if (pfrom->HasPermission(PF_RELAY)) {
fBlocksOnly = false;
}

Expand Down Expand Up @@ -2701,7 +2701,7 @@ static bool ProcessMessage(const Config &config, CNode *pfrom,
}

LOCK(cs_main);
if (IsInitialBlockDownload() && !pfrom->fWhitelisted) {
if (IsInitialBlockDownload() && !pfrom->HasPermission(PF_NOBAN)) {
LogPrint(BCLog::NET,
"Ignoring getheaders from peer=%d because node is in "
"initial block download\n",
Expand Down Expand Up @@ -2770,9 +2770,7 @@ static bool ProcessMessage(const Config &config, CNode *pfrom,
// Stop processing the transaction early if
// We are in blocks only mode and peer is either not whitelisted or
// whitelistrelay is off
if (!fRelayTxes &&
(!pfrom->fWhitelisted ||
!gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY))) {
if (!fRelayTxes && !pfrom->HasPermission(PF_RELAY)) {
LogPrint(BCLog::NET,
"transaction sent in violation of protocol peer=%d\n",
pfrom->GetId());
Expand Down Expand Up @@ -2947,9 +2945,7 @@ static bool ProcessMessage(const Config &config, CNode *pfrom,
}
}

if (pfrom->fWhitelisted &&
gArgs.GetBoolArg("-whitelistforcerelay",
DEFAULT_WHITELISTFORCERELAY)) {
if (pfrom->HasPermission(PF_FORCERELAY)) {
// Always relay transactions received from whitelisted peers,
// even if they were already in the mempool or rejected from it
// due to policy, allowing the node to function as a gateway for
Expand Down Expand Up @@ -3402,7 +3398,8 @@ static bool ProcessMessage(const Config &config, CNode *pfrom,
// unless we're still syncing with the network. Such an unrequested
// block may still be processed, subject to the conditions in
// AcceptBlock().
bool forceProcessing = pfrom->fWhitelisted && !IsInitialBlockDownload();
bool forceProcessing =
pfrom->HasPermission(PF_NOBAN) && !IsInitialBlockDownload();
const uint256 hash(pblock->GetHash());
{
LOCK(cs_main);
Expand Down Expand Up @@ -3557,7 +3554,8 @@ static bool ProcessMessage(const Config &config, CNode *pfrom,
}

if (strCommand == NetMsgType::MEMPOOL) {
if (!(pfrom->GetLocalServices() & NODE_BLOOM) && !pfrom->fWhitelisted) {
if (!(pfrom->GetLocalServices() & NODE_BLOOM) &&
!pfrom->HasPermission(PF_MEMPOOL)) {
if (!pfrom->HasPermission(PF_NOBAN)) {
LogPrint(BCLog::NET,
"mempool request with bloom filters disabled, "
Expand All @@ -3568,7 +3566,8 @@ static bool ProcessMessage(const Config &config, CNode *pfrom,
return true;
}

if (connman->OutboundTargetReached(false) && !pfrom->fWhitelisted) {
if (connman->OutboundTargetReached(false) &&
!pfrom->HasPermission(PF_MEMPOOL)) {
if (!pfrom->HasPermission(PF_NOBAN)) {
LogPrint(BCLog::NET,
"mempool request with bandwidth limit reached, "
Expand Down Expand Up @@ -3788,7 +3787,7 @@ bool PeerLogicValidation::SendRejectsAndCheckIfBanned(CNode *pnode,

if (state.fShouldBan) {
state.fShouldBan = false;
if (pnode->fWhitelisted) {
if (pnode->HasPermission(PF_NOBAN)) {
LogPrintf("Warning: not punishing whitelisted peer %s!\n",
pnode->addr.ToString());
} else if (pnode->m_manual_connection) {
Expand Down Expand Up @@ -4503,7 +4502,7 @@ bool PeerLogicValidation::SendMessages(const Config &config, CNode *pto,
pto->vInventoryBlockToSend.clear();

// Check whether periodic sends should happen
bool fSendTrickle = pto->fWhitelisted;
bool fSendTrickle = pto->HasPermission(PF_NOBAN);
if (pto->nNextInvSend < nNow) {
fSendTrickle = true;
if (pto->fInbound) {
Expand Down Expand Up @@ -4697,7 +4696,7 @@ bool PeerLogicValidation::SendMessages(const Config &config, CNode *pto,
// Note: If all our peers are inbound, then we won't disconnect
// our sync peer for stalling; we have bigger problems if we
// can't get any outbound peers.
if (!pto->fWhitelisted) {
if (!pto->HasPermission(PF_NOBAN)) {
LogPrintf("Timeout downloading headers from peer=%d, "
"disconnecting\n",
pto->GetId());
Expand Down Expand Up @@ -4832,8 +4831,7 @@ bool PeerLogicValidation::SendMessages(const Config &config, CNode *pto,
// -whitelistforcerelay
if (pto->nVersion >= FEEFILTER_VERSION &&
gArgs.GetBoolArg("-feefilter", DEFAULT_FEEFILTER) &&
!(pto->fWhitelisted && gArgs.GetBoolArg("-whitelistforcerelay",
DEFAULT_WHITELISTFORCERELAY))) {
!pto->HasPermission(PF_FORCERELAY)) {
Amount currentFilter =
g_mempool
.GetMinFee(
Expand Down
4 changes: 2 additions & 2 deletions src/qt/rpcconsole.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1310,8 +1310,8 @@ void RPCConsole::updateNodeDetail(const CNodeCombinedStats *stats) {
: tr("Outbound"));
ui->peerHeight->setText(
QString("%1").arg(QString::number(stats->nodeStats.nStartingHeight)));
ui->peerWhitelisted->setText(stats->nodeStats.fWhitelisted ? tr("Yes")
: tr("No"));
ui->peerWhitelisted->setText(
stats->nodeStats.m_legacyWhitelisted ? tr("Yes") : tr("No"));

// This check fails for example if the lock was busy and
// nodeStateStats couldn't be fetched.
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ static UniValue getpeerinfo(const Config &config,
}
obj.pushKV("inflight", heights);
}
obj.pushKV("whitelisted", stats.fWhitelisted);
obj.pushKV("whitelisted", stats.m_legacyWhitelisted);
UniValue permissions(UniValue::VARR);
for (const auto &permission :
NetPermissions::ToStrings(stats.m_permissionFlags)) {
Expand Down

0 comments on commit 2d7438d

Please sign in to comment.