-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix: add missing single block body download validation #3563
Conversation
Codecov Report
... and 9 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
fn ensure_valid_body_response( | ||
header: &SealedHeader, | ||
block: &BlockBody, | ||
) -> Result<(), ConsensusError> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we already have validate_block_standalone
fn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this requires chain spec, which we don't have here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could integrate this in a followup but requires a few changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approving with the assumption that ensure_valid_body_response
will be dedupped. it is important to do any validation w/ the chain spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
we didn't properly validate that the body response actually belongs to the header.
this adds missing checks to validate payload