eth: check propagated block malformation on receiption #20546
Merged
Conversation
This file contains 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
holiman
reviewed
Jan 13, 2020
| } | ||
| if hash := types.DeriveSha(request.Block.Transactions()); hash != request.Block.TxHash() { | ||
| log.Warn("Propagated block has invalid body", "have", hash, "exp", request.Block.TxHash()) | ||
| break // TODO(karalabe): return error eventually, but wait a few releases |
There was a problem hiding this comment.
Yes, once we do add that, it might make sense to put this check into request.sanityCheck().
There was a problem hiding this comment.
Speaking of which, shouldn't you do this check after the sanity check?
There was a problem hiding this comment.
Doesn't really matter imho. Sanity check just checks that the numbers aren't enormous. Those shouldn't impact the uncle/tx hash in any way.
7 tasks
19 tasks
wanwiset25
pushed a commit
to XinFinOrg/XDPoSChain
that referenced
this pull request
Jun 19, 2024
wanwiset25
pushed a commit
to XinFinOrg/XDPoSChain
that referenced
this pull request
Jun 28, 2024
wanwiset25
added a commit
to XinFinOrg/XDPoSChain
that referenced
this pull request
Aug 23, 2024
…eum#20546)" This reverts commit 1204fa9.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Malformed blocks are detected and rejected by the chain inserter. If however a malformed block is broadcast to us, we only do PoW checks and quickly forward it to keep latency down. This is unfortunate, because PoW does not protect against a bad body, so every new valid PoW is an opportunity to propagate a (one!) malformed block through the network.
This PR fixes it by moving a malformation detection into block receiption (propagation is the only place where an entire block is transmitted in assembled form, the rest of the protocol already uses split headers/bodies). Long term if we rework block propagation to be smarter, even this one instance can be dropped.
The PR currently just silently discards these blocks, but long term we want to drop the offending peer altogether for protocol violation. The reason we don't do this now is not to nuke the network apart if someone attempts this malformed propagation again.