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

getheaders does not handle sidechains correctly at all #427

Closed
jrick opened this issue Oct 18, 2016 · 2 comments · Fixed by #1239
Closed

getheaders does not handle sidechains correctly at all #427

jrick opened this issue Oct 18, 2016 · 2 comments · Fixed by #1239
Labels

Comments

@jrick
Copy link
Member

jrick commented Oct 18, 2016

Noticed when working on #426.

getheaders (either wire protocol or the new RPC version) does not behave as expected wrt the stop hash and the first located block. Our code is looking up the heights of these two blocks, and then returning blocks on the main chain until the max is reached or the height of the stop hash is reached (but not the stop block itself). This can result in a couple weird behaviors:

  • If the stop hash is not on the main chain, getheaders will stop providing you headers when you reach that height, even when it has been providing headers on the main chain, and there may still be even more main chain headers to provide.
  • If the first located block is not on the main chain, it will still start providing headers from the main chain instead of headers on the side chain.

For reference, bitcoin core only provides headers on the main chain, up until the stop hash is seen. If the stop hash is not encountered on the main chain, then it will continue to provide more main chain headers. If the first located block is on a side chain, it will actually go back and return headers starting at the fork point between the main chain and that side chain.

https://github.com/bitcoin/bitcoin/blob/53133c1c041d113c2a480a18e6ff38681d135dca/src/main.cpp#L5425

@jrick jrick added the bug label Oct 18, 2016
@jrick
Copy link
Member Author

jrick commented Oct 18, 2016

This is an upstream issue that also affects btcd.

@jrick
Copy link
Member Author

jrick commented Nov 1, 2016

Just a reminder, the "patch" version for the JSON-RPC server API needs to be bumped once this has been fixed. I currently have code in wallet to workaround this bug but it needs to be eventually removed, and wallet will need to check that the dcrd RPC server's API is compatible with that removal.

martelletto added a commit that referenced this issue Nov 2, 2016
Leave endIdx unbounded in locateBlocks() if the stop block is not on
the main chain. This causes as many blocks as possible to be stuffed
in the response, partially addressing #427.

XXX This is tentative at best. The locking needs verification, and the
whole logic behind locateBlocks() might need to change to accomodate
the other requirements lined out in #427.
martelletto added a commit that referenced this issue Nov 2, 2016
If the requested start block is in a side chain, find the block's
earliest ancestor in the main chain, and start from there.

XXX This is a proof of concept at best. It still requires substantial
rework and proper error handling.
@marcopeereboom marcopeereboom modified the milestones: 0.7.0, 0.6.0 Nov 4, 2016
@marcopeereboom marcopeereboom removed this from the 0.7.0 milestone Dec 15, 2016
@davecgh davecgh mentioned this issue Mar 11, 2018
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants