Merge #20291: [net] Consolidate logic around calling CAddrMan::Connected()

0bfce9dc46 [addrman] Fix Connected() comment (John Newbery)
eefe194718 [net] Consolidate logic around calling CAddrMan::Connected() (John Newbery)

Pull request description:

  Currently, the logic around whether we called CAddrMan::Connected() for
  a peer is spread between verack processing (where we discard inbound
  peers) and FinalizeNode (where we discard misbehaving and
  block-relay-only peers). Consolidate that logic to a single place.

  Also remove the CNode.fCurrentlyConnected bool, which is now
  redundant. We can rely on CNode.fSuccessfullyConnected, since the two
  bools were only ever flipped to true in the same place.

ACKs for top commit:
  mzumsande:
    Code review ACK 0bfce9dc46
  amitiuttarwar:
    code review ACK 0bfce9dc46. nice tidy, and bonus that we get to remove an unnecessary call to `cs_main`

Tree-SHA512: 1ab74dae3bc12a6846da57c853033e546bb4f91caa39f4c50bf0cf7eca59cb917bdb2ef795da55363e7e9f70949cb28bb3be004cb3afa4389f970d2fe097d932
This commit is contained in:
MarcoFalke 2020-11-19 16:30:48 +01:00
commit 884bde510e
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
2 changed files with 16 additions and 11 deletions

View file

@ -281,7 +281,17 @@ protected:
//! Select several addresses at once.
void GetAddr_(std::vector<CAddress> &vAddr, size_t max_addresses, size_t max_pct) EXCLUSIVE_LOCKS_REQUIRED(cs);
//! Mark an entry as currently-connected-to.
/** We have successfully connected to this peer. Calling this function
* updates the CAddress's nTime, which is used in our IsTerrible()
* decisions and gossiped to peers. Callers should be careful that updating
* this information doesn't leak topology information to network spies.
*
* net_processing calls this function when it *disconnects* from a peer to
* not leak information about currently connected peers.
*
* @param[in] addr The address of the peer we were connected to
* @param[in] nTime The time that we were last connected to this peer
*/
void Connected_(const CService& addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs);
//! Update an entry's service bits.
@ -704,7 +714,7 @@ public:
return vAddr;
}
//! Mark an entry as currently-connected-to.
//! Outer function for Connected_()
void Connected(const CService &addr, int64_t nTime = GetAdjustedTime())
{
LOCK(cs);

View file

@ -290,8 +290,6 @@ namespace {
struct CNodeState {
//! The peer's address
const CService address;
//! Whether we have a fully established connection.
bool fCurrentlyConnected;
//! The best known block we know this peer has announced.
const CBlockIndex *pindexBestKnownBlock;
//! The hash of the last unknown block this peer has announced.
@ -390,7 +388,6 @@ struct CNodeState {
CNodeState(CAddress addrIn, bool is_inbound, bool is_manual)
: address(addrIn), m_is_inbound(is_inbound), m_is_manual_connection(is_manual)
{
fCurrentlyConnected = false;
pindexBestKnownBlock = nullptr;
hashLastUnknownBlock.SetNull();
pindexLastCommonBlock = nullptr;
@ -857,8 +854,9 @@ void PeerManager::FinalizeNode(const CNode& node, bool& fUpdateConnectionTime) {
if (state->fSyncStarted)
nSyncStarted--;
if (misbehavior == 0 && state->fCurrentlyConnected && !node.IsBlockOnlyConn()) {
// Note: we avoid changing visible addrman state for block-relay-only peers
if (node.fSuccessfullyConnected && misbehavior == 0 &&
!node.IsBlockOnlyConn() && !node.IsInboundConn()) {
// Only change visible addrman state for outbound, full-relay peers
fUpdateConnectionTime = true;
}
@ -2488,9 +2486,6 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
if (pfrom.fSuccessfullyConnected) return;
if (!pfrom.IsInboundConn()) {
// Mark this node as currently connected, so we update its timestamp later.
LOCK(cs_main);
State(pfrom.GetId())->fCurrentlyConnected = true;
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()) : ""),