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

p2p: Run checks ealier when processing headers #2399

Merged
merged 3 commits into from
Aug 29, 2024

Conversation

matheusd
Copy link
Member

Running these earlier helps disconnect faster from non-compliant peers.

The existing full node software (dcrd) always sends batches of 2000
headers when requesting headers during Initial Block Download.

The check added in this commit ensures the wallet disconnects from any
peers that deviate from this behavior. While receiving less than 2000
headers could not cause any consistency or safety issues, it could slow
down sync time.
This moves the check for requested headers earlier in the call stack for
processing headers messages.

This ensures misbehaving peers are disconnected earlier, before spending
cpu processing for block hash calculation.
This moves the check that ensures that a reply to a getheaders connects
to the existing locators ealier in the call stack for processing a
headers response.

This ensures the wallet disconnects from misbehaving peers before
processing block hashes.
@jrick
Copy link
Member

jrick commented Aug 29, 2024

how does this work in the 1999/2000 cases when the final set of headers in a sync are sent to the wallet? does it also disconnect the peer?

@jrick
Copy link
Member

jrick commented Aug 29, 2024

nvm, it's also checking against the handshake's initial height

@matheusd
Copy link
Member Author

Right. The last block (which may have < 2k headers) is accepted, as long as the last header is >= the initial advertised height

@jrick jrick merged commit 08c05e9 into decred:master Aug 29, 2024
2 checks passed
@matheusd matheusd deleted the improve-p2p-checks branch August 29, 2024 14:19
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

Successfully merging this pull request may close these issues.

2 participants