Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore getheaders requests when not synced #6172

Merged

Conversation

sdaftuar
Copy link
Member

@sdaftuar sdaftuar commented May 21, 2015

Sending headers suggests to our peers that they can download from us, so don't respond to getheaders requests while in initial block download (just as we don't relay blocks on tip updates either).

@laanwj laanwj added the P2P label May 21, 2015
@sdaftuar sdaftuar force-pushed the fix-getheaders-response-when-syncing branch from 892bfc7 to 310ddb1 Compare May 22, 2015
@laanwj
Copy link
Member

laanwj commented May 24, 2015

utACK

@@ -4174,6 +4174,8 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,

LOCK(cs_main);

if (IsInitialBlockDownload()) return true;
Copy link

@Diapolo Diapolo May 24, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope we soon and finally apply the clang-format ;).

Copy link
Member

@laanwj laanwj May 27, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's completely inconsequential, but pedantically the return true should be on the next line and indented.

@morcos
Copy link
Member

morcos commented May 24, 2015

Tested ACK

@sdaftuar sdaftuar force-pushed the fix-getheaders-response-when-syncing branch from 310ddb1 to a1ba077 Compare May 27, 2015
@sdaftuar
Copy link
Member Author

sdaftuar commented May 27, 2015

Fixed nit

@laanwj
Copy link
Member

laanwj commented Jun 1, 2015

Needs to be backported to 0.11 at least

@laanwj laanwj added this to the 0.11.0 milestone Jun 1, 2015
@laanwj laanwj merged commit a1ba077 into bitcoin:master Jun 2, 2015
laanwj added a commit that referenced this pull request Jun 2, 2015
a1ba077 Ignore getheaders requests when not synced. (Suhas Daftuar)
laanwj pushed a commit that referenced this pull request Jun 2, 2015
@rebroad
Copy link
Contributor

rebroad commented Jun 8, 2015

Shouldn't we still send headers for the blocks we have?

@sdaftuar
Copy link
Member Author

sdaftuar commented Jun 10, 2015

@rebroad Please see related comment here: #5927 (comment). Before IBD has finished, it's possible to be on a chain that would violate a checkpoint, and responding to a getheaders request at that point could then cause an honest peer to disconnect.

rebroad added a commit to rebroad/bitcoin that referenced this pull request Nov 2, 2016
An improvement over bitcoin#6172. Fixes bitcoin#6971 rather than bypasses it
as bitcoin#6974 did, and reduces overloading of whitelisting.
@rebroad
Copy link
Contributor

rebroad commented Nov 4, 2016

https://botbot.me/freenode/bitcoin-core-dev/msg/75840350/ @gmaxwell your comment recently is MORE applicable here...

pyritepirate referenced this pull request in pyritepirate/pyrite Jan 25, 2019
reddink pushed a commit to reddcoin-project/reddcoin that referenced this pull request Aug 14, 2020
Rebased-From: a1ba077
Github-Pull: bitcoin#6172
(cherry picked from commit b4bbad1)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants