Do not set an addr time penalty when a peer advertises itself. #8661

pull gmaxwell wants to merge 1 commits into bitcoin:master from gmaxwell:addrman_nopenalty_self changing 1 files +5 −0

gmaxwell commented at 10:30 am on September 3, 2016: contributor

Claims a peer makes about itself are inherently more credible.
Do not set an addr time penalty when a peer advertises itself.
Claims a peer makes about itself are inherently more credible.
6d0ced1865
gmaxwell commented at 7:06 pm on September 3, 2016: contributor

@sipa @EthanHeilman Is this sensible in your view?
jonasschnelli added the label P2P on Sep 5, 2016
sipa commented at 9:53 am on September 5, 2016: member

utACK
EthanHeilman commented at 3:03 pm on September 6, 2016: contributor

@gmaxwell utACK. Feeler connections don’t currently send addr messages so they would not benefit from this change. I think that is ok for now.

I plan on investigating the benefits of having feeler connections either send an addr message containing only themselves (if they accept incoming connections) or having feeler connections send a full addr message but that goes beyond the scope of this PR and should be discussed later.
gmaxwell commented at 8:28 pm on September 6, 2016: contributor

I think there would be a good argument for feeler connections going as far as trying to determine if the peers have distinct chaintips– and attempting to sync them if they do– before moving along. But right, orthogonal to this. Thanks for taking a look.
rebroad commented at 2:59 pm on September 15, 2016: contributor

@gmaxwell Can you summarise what the impact of this is please? (as if talking to someone who is unfamiliar with addrman)
gmaxwell commented at 4:25 pm on September 15, 2016: contributor

@rebroad The node keeps track of when it last heard from a peer and uses this information when trying to figure out who to connect to (esp when connected to no one). Advertisements rumored through third parties have that time artificially increased, because we’re getting the information second hand and it might not be correct. But when a peer advertises themselves this information is first hand.
btcdrak approved
btcdrak commented at 11:26 pm on September 22, 2016: contributor

utACK 6d0ced1
laanwj commented at 11:55 am on September 23, 2016: member

Yes, this makes sense. utACK https://github.com/bitcoin/bitcoin/pull/8661/commits/6d0ced18655a4f3619a6f478846f3241f1d040f7
laanwj merged this on Sep 23, 2016
laanwj closed this on Sep 23, 2016
laanwj referenced this in commit d2e46558ba on Sep 23, 2016
rebroad commented at 12:41 pm on September 23, 2016: contributor

@gmaxwell thanks for the explanation. I notice that the addresses advertised via 3rd parties go into the same database as the “last heard from a peer database” with fake times ranging from 3 to 7 days… why the need to fake anything - why not simply keep a database of nodes actually heard from, and a database of rumored nodes?
EthanHeilman commented at 6:22 pm on September 23, 2016: contributor

@rebroad The new table is a database of rumored nodes, the tried table is a database of nodes that we have heard from or nodes that we have made an outgoing connection to.

The PR #8594 changes the tried table to just be a database of nodes which we have connected to.

One idea could be to have three tables: new, tried, heard. I think this is a good idea but it requires careful design to insure it could not be manipulated by an attacker attempting to partition or eclipse a node.
codablock referenced this in commit a4a71e01a9 on Sep 19, 2017
codablock referenced this in commit 39d2131142 on Jan 11, 2018
andvgal referenced this in commit 5ca8952dff on Jan 6, 2019
pyritepirate referenced this in commit 9c22e0e1f5 on Jan 25, 2019
random-zebra referenced this in commit 777638e7bc on Aug 27, 2020
DrahtBot locked this on Sep 8, 2021

Contributors
gmaxwell sipa EthanHeilman rebroad btcdrak laanwj

Labels
P2P