net: Send post-verack handshake messages at most once#20146
Merged
fanquake merged 1 commit intobitcoin:masterfrom Oct 15, 2020
Merged
net: Send post-verack handshake messages at most once#20146fanquake merged 1 commit intobitcoin:masterfrom
fanquake merged 1 commit intobitcoin:masterfrom
Conversation
Member
|
wont-hurt-ACK |
Member
|
Code review ACK fa1f6f2 |
Contributor
|
ACK fa1f6f2 |
jonatack
reviewed
Oct 14, 2020
Member
jonatack
left a comment
jonatack
left a comment
There was a problem hiding this comment.
Concept ACK fa1f6f2 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?
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Oct 16, 2020
… once fa1f6f2 net: Send post-verack handshake messages at most once (MarcoFalke) Pull request description: 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. ACKs for top commit: jonatack: Concept ACK fa1f6f2 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: ACK fa1f6f2, I have reviewed the code and it looks OK, I agree it can be merged. naumenkogs: ACK fa1f6f2 laanwj: Code review ACK fa1f6f2 Tree-SHA512: c841d5d3807254a49463bbcfac3b32881b34a9d3206899544c86322c20988e17ad2ae243cba227fd3825a914f0cb2584451edda2414aecee6d5e3f5a0636f08a
Member
|
Arguably this was a protocol violation, so perhaps we want to backport? |
Member
Author
|
Shouldn't matter much, but I certainly don't mind a backport, as we are doing a release anyway. |
fanquake
pushed a commit
to fanquake/bitcoin
that referenced
this pull request
Oct 19, 2020
Github-Pull: bitcoin#20146 Rebased-From: fa1f6f2
Member
|
Have added this to #20166 for backporting to the 0.20 branch. |
Merged
maflcko
pushed a commit
that referenced
this pull request
Nov 18, 2020
7566af4 doc: Update data directory path comments (Hennadii Stepanov) 09261de util: Add StripRedundantLastElementsOfPath function (Hennadii Stepanov) 8ef0dac macOS deploy: use the new plistlib API (Jonas Schnelli) 314e795 build: fix mutex detection when building bdb on macOS (fanquake) 1f67a30 random: fixes read buffer resizing in RandAddSeedPerfmon (Ethan Heilman) 6113b54 net: Send post-verack handshake messages at most once (MarcoFalke) bdf15d0 rpc: Adjust witness-tx deserialize error message (MarcoFalke) 731502a rpc: Properly deserialize txs with witness before signing (MarcoFalke) ee0082b Avoid the use of abs64 in timedata (Pieter Wuille) 05bd0c2 docs: Correct description for getblockstats's txs field (Nadav Ivgi) Pull request description: Backports the following PRs to the 0.20 branch: * #19777 - docs: Correct description for getblockstats's txs field * #19836 - rpc: Properly deserialize txs with witness before signing * #20080 - Strip any trailing `/` in -datadir and -blocksdir paths * #20082 - [bugfix] random: fixes read buffer to use min rather than max * #20141 - Avoid the use of abs64 in timedata * #20146 - net: Send post-verack handshake messages at most once * #20195 - build: fix mutex detection when building bdb on macOS * #20298 - macOS deploy: use the new plistlib API Will add additional commits as they become available. ACKs for top commit: MarcoFalke: review ACK 7566af4 🗡 Tree-SHA512: add6bb978313c12c3e07bc232636ae9d1ab0edd0b816705c5c70eeb1cc04097165fd5e29d60c706886943ceb1f749a422020766b4aa2d23be51e9f839157a4bb
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Nov 17, 2021
Summary: 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. This is a backport of [[bitcoin/bitcoin#20146 | core#20146]] Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D10469
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There is no need to send
SENDHEADERSandSENDCMPCTmessages as a reply to eachVERACKthat is received. For alive checks, aPING/PONGcan be used.