Remove fNetworkNode and pnodeLocalHost. #9226

pull gmaxwell wants to merge 2 commits into bitcoin:master from gmaxwell:node_is_this_i_dont_even changing 3 files +3 −22

gmaxwell commented at 2:56 AM on November 27, 2016: contributor

Matt pointed out to me that this appeared to be doing nothing (except involving itself in data races).

Also removes pnodeLocalHost.
gmaxwell commented at 2:56 AM on November 27, 2016: contributor

Have I missed something here? I just don't want to be left out of the finds-code-he-doesn't-understand-so-opens-a-pr-to-remove-it club!
fanquake added the label Refactoring on Nov 27, 2016
in src/main.cpp:None in e876896625 outdated
```
5241 | @@ -5242,8 +5242,8 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
5242 |      {
5243 |          pfrom->SetRecvVersion(min(pfrom->nVersion, PROTOCOL_VERSION));
5244 |  
5245 | -        // Mark this node as currently connected, so we update its timestamp later.
5246 | -        if (pfrom->fNetworkNode) {
5247 | +        {
5248 | +            // Mark this node as currently connected, so we update its timestamp later.
```
TheBlueMatt commented at 3:24 AM on November 27, 2016:

I believe this should now be if (!pfrom->fInbound) to avoid changing behavior, as it is used to decide whether to call addrman.Connected from DeleteNode.

gmaxwell commented at 4:11 AM on November 27, 2016:

I believe we should be addrman.Connected(pnode->addr) on inbound connections too. I could document this better in the commit message.

gmaxwell commented at 4:13 AM on November 27, 2016:

Hm perhaps not, okay I reasoned myself out of that. I'll change it.
TheBlueMatt commented at 3:27 AM on November 27, 2016: member

You might also remove pnodeLocalHost as well, since, afaict, it makes fNetworkNode a synonym for !fInbound.

Remove fNetworkNode.

Matt pointed out to me that this appeared to be doing nothing (except involving itself in data races).

083f203698

Remove pnodeLocalHost.

Mostly a legacy of the long removed pub/sub system.

bdb922b34c

gmaxwell renamed this:
~~Remove fNetworkNode.~~
Remove fNetworkNode and pnodeLocalHost.
on Nov 27, 2016
fanquake added the label P2P on Nov 27, 2016
TheBlueMatt commented at 9:30 AM on November 27, 2016: member

Slightly-tested ACK bdb922b34c9a477c4ebf358653f6bcfb9bee48ce
theuni commented at 7:53 PM on November 27, 2016: member

Aww, I was looking forward to nuking a bunch of this useless stuff all at once :)

utACK
sipa commented at 7:57 PM on November 27, 2016: member

utACK bdb922b34c9a477c4ebf358653f6bcfb9bee48ce
sdaftuar commented at 7:48 PM on November 28, 2016: member

Lightly tested ACK bdb922b (though I have zero idea what this pnodeLocalHost thing was ever for).
sipa commented at 7:57 PM on November 28, 2016: member

though I have zero idea what this pnodeLocalHost thing was ever for

It seems to have originally been needed for the pubsub mechanism (which was removed in #1036).
gmaxwell commented at 10:22 AM on November 29, 2016: contributor

@sdaftuar My commit message said! (as Pieter says, it was the 'pub sub' system in the original release of Bitcoin)
laanwj commented at 11:49 AM on November 29, 2016: member

Oh wow, I'm surprised this stuck around for so long as it is indeed used nowhere. Good find! utACK https://github.com/bitcoin/bitcoin/commit/bdb922b34c9a477c4ebf358653f6bcfb9bee48ce
sipa merged this on Dec 1, 2016
sipa closed this on Dec 1, 2016
sipa referenced this in commit 3bf06e9bac on Dec 1, 2016
codablock referenced this in commit 49cecb470e on Jan 16, 2018
codablock referenced this in commit 0227b0df37 on Jan 16, 2018
codablock referenced this in commit d54ef6d7e3 on Jan 17, 2018
andvgal referenced this in commit db3f15c8d4 on Jan 6, 2019
CryptoCentric referenced this in commit 43363f582e on Feb 25, 2019
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels