Skip to content

Commit

Permalink
refactor: drop flag m_block_relay_peer and use m_addr_relay object in…
Browse files Browse the repository at this point in the history 
…stead (#5339)

## Issue being fixed or feature implemented
This refactoring is a follow-up changes to backport bitcoin#17164 (PR
#5314)

These changes are reduce difference in implementation for our code and
bitcoin's


## What was done?
Removed a flag m_block_relay_peer. Instead I call IsAddrRelayPeer() that
has same information now.
It changes logic introduced in #4888 due to dash-specific code.


## How Has This Been Tested?
Run unit/functional tests.

## Breaking Changes
No breaking changes

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
  • Loading branch information
@knst
knst authored Apr 19, 2023
1 parent 6499917 commit b026f86
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 30 deletions.
2 changes: 1 addition & 1 deletion src/llmq/instantsend.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1526,7 +1526,7 @@ void CInstantSendManager::AskNodesForLockedTx(const uint256& txid, const CConnma
if (nodesToAskFor.size() >= 4) {
return;
}
if (!pnode->m_block_relay_only_peer) {
if (pnode->IsAddrRelayPeer()) {
LOCK(pnode->m_tx_relay->cs_tx_inventory);
if (pnode->m_tx_relay->filterInventoryKnown.contains(txid)) {
pnode->AddRef();
Expand Down
15 changes: 6 additions & 9 deletions src/net.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
X(addrBind);
stats.m_network = GetNetworkName(ConnectedThroughNetwork());
stats.m_mapped_as = addr.GetMappedAS(m_asmap);
if (!m_block_relay_only_peer) {
if (IsAddrRelayPeer()) {
LOCK(m_tx_relay->cs_filter);
stats.fRelayTxes = m_tx_relay->fRelayTxes;
} else {
Expand Down Expand Up @@ -1014,7 +1014,7 @@ bool CConnman::AttemptToEvictConnection()

bool peer_relay_txes = false;
bool peer_filter_not_null = false;
if (!node->m_block_relay_only_peer) {
if (node->IsAddrRelayPeer()) {
LOCK(node->m_tx_relay->cs_filter);
peer_relay_txes = node->m_tx_relay->fRelayTxes;
peer_filter_not_null = node->m_tx_relay->pfilter != nullptr;
Expand Down Expand Up @@ -2272,7 +2272,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
// also have the added issue that they're attacker controlled and could be used
// to prevent us from connecting to particular hosts if we used them here.
setConnected.insert(pnode->addr.GetGroup(addrman.m_asmap));
if (pnode->m_block_relay_only_peer) {
if (!pnode->IsAddrRelayPeer()) {
nOutboundBlockRelay++;
} else if (!pnode->fFeeler) {
nOutboundFullRelay++;
Expand Down Expand Up @@ -2480,8 +2480,7 @@ void CConnman::ThreadOpenAddedConnections()
}
tried = true;
CAddress addr(CService(), NODE_NONE);
// KNST should be false
OpenNetworkConnection(addr, false, &grant, info.strAddedNode.c_str(), false, false, true /* ,false*/);
OpenNetworkConnection(addr, false, &grant, info.strAddedNode.c_str(), false, false, true);
if (!interruptNet.sleep_for(std::chrono::milliseconds(500)))
return;
}
Expand Down Expand Up @@ -3619,7 +3618,7 @@ void CConnman::RelayInvFiltered(CInv &inv, const CTransaction& relatedTx, const
{
LOCK(cs_vNodes);
for (const auto& pnode : vNodes) {
if (pnode->nVersion < minProtoVersion || !pnode->CanRelay() || pnode->m_block_relay_only_peer) {
if (pnode->nVersion < minProtoVersion || !pnode->CanRelay() || !pnode->IsAddrRelayPeer()) {
continue;
}
{
Expand All @@ -3639,7 +3638,7 @@ void CConnman::RelayInvFiltered(CInv &inv, const uint256& relatedTxHash, const i
{
LOCK(cs_vNodes);
for (const auto& pnode : vNodes) {
if (pnode->nVersion < minProtoVersion || !pnode->CanRelay() || pnode->m_block_relay_only_peer) {
if (pnode->nVersion < minProtoVersion || !pnode->CanRelay() || !pnode->IsAddrRelayPeer()) {
continue;
}
{
Expand Down Expand Up @@ -3772,7 +3771,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
fInbound(fInboundIn),
nKeyedNetGroup(nKeyedNetGroupIn),
m_addr_known{block_relay_only ? nullptr : std::make_unique<CRollingBloomFilter>(5000, 0.001)},
m_block_relay_only_peer(block_relay_only),
id(idIn),
nLocalHostNonce(nLocalHostNonceIn),
nLocalServices(nLocalServicesIn),
Expand All @@ -3782,7 +3780,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
hSocket = hSocketIn;
addrName = addrNameIn == "" ? addr.ToStringIPPort() : addrNameIn;
hashContinue = uint256();
m_tx_relay = std::make_unique<TxRelay>();

for (const std::string &msg : getAllNetMessageTypes())
mapRecvBytesPerMsgCmd[msg] = 0;
Expand Down
8 changes: 3 additions & 5 deletions src/net.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1051,8 +1051,6 @@ class CNode
int64_t nNextAddrSend GUARDED_BY(cs_sendProcessing){0};
int64_t nNextLocalAddrSend GUARDED_BY(cs_sendProcessing){0};

const bool m_block_relay_only_peer;

// Don't relay addr messages to peers that we connect to as block-relay-only
// peers (to prevent adversaries from inferring these links from addr
// traffic).
Expand All @@ -1063,7 +1061,7 @@ class CNode
// Stop processing non-block data early if
// 1) We are in blocks only mode and peer has no relay permission
// 2) This peer is a block-relay-only peer
return (!g_relay_txes && !HasPermission(PF_RELAY)) || m_block_relay_only_peer;
return (!g_relay_txes && !HasPermission(PF_RELAY)) || !IsAddrRelayPeer();
}

// List of block ids we still have announce.
Expand Down Expand Up @@ -1110,8 +1108,8 @@ class CNode
};

// in bitcoin: m_tx_relay == nullptr if we're not relaying transactions with this peer
// in dash: m_tx_relay should never be nullptr, use m_block_relay_only_peer == true instead
std::unique_ptr<TxRelay> m_tx_relay;
// in dash: m_tx_relay should never be nullptr, use `IsAddrRelayPeer() == false` instead
std::unique_ptr<TxRelay> m_tx_relay{std::make_unique<TxRelay>()};

// Used for headers announcements - unfiltered blocks to relay
std::vector<uint256> vBlockHashesToAnnounce GUARDED_BY(cs_inventory);
Expand Down
30 changes: 15 additions & 15 deletions src/net_processing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ static void PushNodeVersion(CNode& pnode, CConnman& connman, int64_t nTime)
}

connman.PushMessage(&pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, nProtocolVersion, (uint64_t)nLocalNodeServices, nTime, addrYou, addrMe,
nonce, strSubVersion, nNodeStartingHeight, ::g_relay_txes && !pnode.m_block_relay_only_peer, mnauthChallenge, pnode.m_masternode_connection.load()));
nonce, strSubVersion, nNodeStartingHeight, ::g_relay_txes && pnode.IsAddrRelayPeer(), mnauthChallenge, pnode.m_masternode_connection.load()));

if (fLogIPs) {
LogPrint(BCLog::NET, "send version message: version %d, blocks=%d, us=%s, them=%s, peer=%d\n", nProtocolVersion, nNodeStartingHeight, addrMe.ToString(), addrYou.ToString(), nodeid);
Expand Down Expand Up @@ -1010,7 +1010,7 @@ void PeerLogicValidation::FinalizeNode(const CNode& node) {
}
} // cs_main

if (node.fSuccessfullyConnected && misbehavior == 0 && !node.m_block_relay_only_peer && !node.fInbound) {
if (node.fSuccessfullyConnected && misbehavior == 0 && node.IsAddrRelayPeer() && !node.fInbound) {
// Only change visible addrman state for full outbound peers. We don't
// call Connected() for feeler connections since they don't have
// fSuccessfullyConnected set.
Expand Down Expand Up @@ -1794,7 +1794,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c
else if (inv.type == MSG_FILTERED_BLOCK) {
bool sendMerkleBlock = false;
CMerkleBlock merkleBlock;
if (!pfrom.m_block_relay_only_peer) {
if (pfrom.IsAddrRelayPeer()) {
LOCK(pfrom.m_tx_relay->cs_filter);
if (pfrom.m_tx_relay->pfilter) {
sendMerkleBlock = true;
Expand Down Expand Up @@ -1867,7 +1867,7 @@ void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnm
// mempool entries added before this time have likely expired from mapRelay
const std::chrono::seconds longlived_mempool_time = GetTime<std::chrono::seconds>() - RELAY_TX_CACHE_TIME;
// Get last mempool request time
const std::chrono::seconds mempool_req = pfrom.m_block_relay_only_peer ? pfrom.m_tx_relay->m_last_mempool_req.load()
const std::chrono::seconds mempool_req = !pfrom.IsAddrRelayPeer() ? pfrom.m_tx_relay->m_last_mempool_req.load()
: std::chrono::seconds::min();

{
Expand All @@ -1891,7 +1891,7 @@ void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnm
}
++it;

if (pfrom.m_block_relay_only_peer && NetMessageViolatesBlocksOnly(inv.GetCommand())) {
if (!pfrom.IsAddrRelayPeer() && NetMessageViolatesBlocksOnly(inv.GetCommand())) {
// Note that if we receive a getdata for non-block messages
// from a block-relay-only outbound peer that violate the policy,
// we skip such getdata messages from this peer
Expand Down Expand Up @@ -2261,7 +2261,7 @@ static void ProcessHeadersMessage(CNode& pfrom, CConnman& connman, ChainstateMan
}
}

if (!pfrom.fDisconnect && IsOutboundDisconnectionCandidate(pfrom) && nodestate->pindexBestKnownBlock != nullptr && !pfrom.m_block_relay_only_peer) {
if (!pfrom.fDisconnect && IsOutboundDisconnectionCandidate(pfrom) && nodestate->pindexBestKnownBlock != nullptr && pfrom.IsAddrRelayPeer()) {
// If this is an outbound full-relay peer, check to see if we should protect
// it from the bad/lagging chain logic.
// Note that block-relay-only peers are already implicitly protected, so we
Expand Down Expand Up @@ -2764,7 +2764,7 @@ void PeerLogicValidation::ProcessMessage(
// set nodes not capable of serving the complete blockchain history as "limited nodes"
pfrom.m_limited_node = (!(nServices & NODE_NETWORK) && (nServices & NODE_NETWORK_LIMITED));

if (!pfrom.m_block_relay_only_peer) {
if (pfrom.IsAddrRelayPeer()) {
LOCK(pfrom.m_tx_relay->cs_filter);
pfrom.m_tx_relay->fRelayTxes = fRelay; // set to true after we get the first filter* message
}
Expand Down Expand Up @@ -2843,7 +2843,7 @@ void PeerLogicValidation::ProcessMessage(
LogPrintf("New outbound peer connected: version: %d, blocks=%d, peer=%d%s (%s)\n",
pfrom.nVersion.load(), pfrom.nStartingHeight,
pfrom.GetId(), (fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToString()) : ""),
pfrom.m_block_relay_only_peer ? "block-relay" : "full-relay");
pfrom.IsAddrRelayPeer()? "full-relay" : "block-relay");
}

if (!pfrom.m_masternode_probe_connection) {
Expand Down Expand Up @@ -3912,7 +3912,7 @@ void PeerLogicValidation::ProcessMessage(
return;
}

if (!pfrom.m_block_relay_only_peer) {
if (pfrom.IsAddrRelayPeer()) {
LOCK(pfrom.m_tx_relay->cs_tx_inventory);
pfrom.m_tx_relay->fSendMempool = true;
}
Expand Down Expand Up @@ -4002,7 +4002,7 @@ void PeerLogicValidation::ProcessMessage(
// There is no excuse for sending a too-large filter
Misbehaving(pfrom.GetId(), 100, "too-large bloom filter");
}
else if (!pfrom.m_block_relay_only_peer)
else if (pfrom.IsAddrRelayPeer())
{
LOCK(pfrom.m_tx_relay->cs_filter);
pfrom.m_tx_relay->pfilter.reset(new CBloomFilter(filter));
Expand All @@ -4020,7 +4020,7 @@ void PeerLogicValidation::ProcessMessage(
bool bad = false;
if (vData.size() > MAX_SCRIPT_ELEMENT_SIZE) {
bad = true;
} else if (!pfrom.m_block_relay_only_peer) {
} else if (pfrom.IsAddrRelayPeer()) {
LOCK(pfrom.m_tx_relay->cs_filter);
if (pfrom.m_tx_relay->pfilter) {
pfrom.m_tx_relay->pfilter->insert(vData);
Expand All @@ -4035,7 +4035,7 @@ void PeerLogicValidation::ProcessMessage(
}

if (msg_type == NetMsgType::FILTERCLEAR) {
if (pfrom.m_block_relay_only_peer) {
if (!pfrom.IsAddrRelayPeer()) {
return;
}
LOCK(pfrom.m_tx_relay->cs_filter);
Expand Down Expand Up @@ -4381,7 +4381,7 @@ void PeerLogicValidation::EvictExtraOutboundPeers(int64_t time_in_seconds)
// Don't evict our protected peers
if (state->m_chain_sync.m_protect) return;
// Don't evict our block-relay-only peers.
if (pnode->m_block_relay_only_peer) return;
if (!pnode->IsAddrRelayPeer()) return;
if (state->m_last_block_announcement < oldest_block_announcement || (state->m_last_block_announcement == oldest_block_announcement && pnode->GetId() > worst_peer)) {
worst_peer = pnode->GetId();
oldest_block_announcement = state->m_last_block_announcement;
Expand Down Expand Up @@ -4742,7 +4742,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
LOCK2(m_mempool.cs, pto->cs_inventory);

size_t reserve = INVENTORY_BROADCAST_MAX_PER_1MB_BLOCK * MaxBlockSize() / 1000000;
if (!pto->m_block_relay_only_peer) {
if (pto->IsAddrRelayPeer()) {
LOCK(pto->m_tx_relay->cs_tx_inventory);
reserve = std::min<size_t>(pto->m_tx_relay->setInventoryTxToSend.size(), reserve);
}
Expand Down Expand Up @@ -4772,7 +4772,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
}
};

if (!pto->m_block_relay_only_peer) {
if (pto->IsAddrRelayPeer()) {
LOCK(pto->m_tx_relay->cs_tx_inventory);
// Check whether periodic sends should happen
// Note: If this node is running in a Masternode mode, it makes no sense to delay outgoing txes
Expand Down

0 comments on commit b026f86

Please sign in to comment.