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

No response to MsgGetBlocks with stopHash zero makes stall detection kick in #1317

Open
halseth opened this issue Oct 1, 2018 · 3 comments

Comments

@halseth
Copy link
Collaborator

halseth commented Oct 1, 2018

If a GetBlocks message is sent to a peer on the same tip as the sending node, with a stop hash zero, then the receiving node will not reply (as it has no inv to send:

btcd/server.go

Line 710 in 2a560b2

// Send the inventory message if there is anything to send.
) and the sending node will disconnect the node assuming it has stalled: https://github.com/btcsuite/btcd/blob/2a560b2/peer/peer.go#L1279

Currently I am experiencing this during tests on regtest, since regtest is special cased to not do headers-first mode, and such a GetBlocks message is sent at initial connect: https://github.com/btcsuite/btcd/blob/2a560b2/netsync/manager.go#L310

Not sure what would be the correct fix here; to remove this special casing of regtest, or to not always expect an INV in return for this getblocks message.

@halseth
Copy link
Collaborator Author

halseth commented Oct 1, 2018

Looking at the logic in question I guess it is not only on regtest this can happen, so should probably not always expect an INV in return.

@zquestz
Copy link

zquestz commented Oct 26, 2018

@halseth you are thinking about this correctly. It is very possible no INV gets returned. Let me know if you put up a patch!

@cjdelisle
Copy link

cjdelisle commented Nov 17, 2018

Technically we should probably consider INV to be required unless the last known block is the same as our last known block. Otherwise they have something they're withholding. It's tempting to change this:
https://github.com/btcsuite/btcd/blob/2a560b2/netsync/manager.go#L256
to a <=, but following the comment, it's probably not a very good idea and it's likely to break a bunch of tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants