net: Send post-verack handshake messages at most once #20146

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2010-netPostVerackHandshake changing 1 files +3 −2

MarcoFalke commented at 8:18 AM on October 14, 2020: member

There is no need to send SENDHEADERS and SENDCMPCT messages as a reply to each VERACK that is received. For alive checks, a PING/PONG can be used.
net: Send post-verack handshake messages at most once fa1f6f237d
sipa commented at 8:20 AM on October 14, 2020: member

wont-hurt-ACK
laanwj commented at 8:30 AM on October 14, 2020: member

Code review ACK fa1f6f237d02265af616129402fa2b8a3019dda5 This is how I'd expected it to work in the first place, so the change imo adheres to the principle of least surprise.
laanwj added the label P2P on Oct 14, 2020
naumenkogs commented at 9:11 AM on October 14, 2020: member

ACK fa1f6f237d02265af616129402fa2b8a3019dda5
jonatack commented at 10:05 AM on October 14, 2020: member

Concept ACK fa1f6f237d02265af616129402fa2b8a3019dda5 this is the only code section that sets fCurrentlyConnected and fSuccessfullyConnected to true. Could add a test. I did not verify if this code is actually being called repeatedly post initial verack; was it?
hebasto approved
hebasto commented at 1:19 PM on October 14, 2020: member

ACK fa1f6f237d02265af616129402fa2b8a3019dda5, I have reviewed the code and it looks OK, I agree it can be merged.
fanquake merged this on Oct 15, 2020
fanquake closed this on Oct 15, 2020
MarcoFalke deleted the branch on Oct 15, 2020
sidhujag referenced this in commit 267633c194 on Oct 16, 2020
sipa commented at 6:27 PM on October 17, 2020: member

Arguably this was a protocol violation, so perhaps we want to backport?
MarcoFalke commented at 6:42 PM on October 17, 2020: member

Shouldn't matter much, but I certainly don't mind a backport, as we are doing a release anyway.
MarcoFalke added the label Needs backport (0.20) on Oct 17, 2020
fanquake referenced this in commit 6113b547f4 on Oct 19, 2020
fanquake commented at 3:17 AM on October 19, 2020: member

Have added this to #20166 for backporting to the 0.20 branch.
fanquake removed the label Needs backport (0.20) on Oct 19, 2020
MarcoFalke referenced this in commit a2fa11f9de on Nov 18, 2020
MarkLTZ referenced this in commit ea258e3e15 on Nov 21, 2020
MarkLTZ referenced this in commit c61e643b75 on Nov 21, 2020
Fabcien referenced this in commit d0db96b021 on Nov 17, 2021
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#20166 [0.20] Backports