-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
eth: Add sidecars validation to sender and recipient #2685
Conversation
if distance between the target block and the latest block is larger than |
when download from others |
65d1b64
to
7a4b540
Compare
I wasn't aware of that, thanks! I updated the PR so that it takes it into account. The reasoning here is just to make the same check for body := chain.GetBody(hash)
if body == nil {
continue
} and if we're missing a required
The issue is that currently, the Having this check in |
7a4b540
to
9ae3b8a
Compare
@@ -837,6 +841,9 @@ func (q *queue) DeliverBodies(id string, txLists [][]*types.Transaction, txListH | |||
} | |||
|
|||
// do some sanity check for sidecar | |||
if blobTxs != len(sidecars[index]) { |
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.
when do full sync,
if distance between the target block and the latest block is larger than MinBlocksForBlobRequests, then the blob data is not need
this check will reject the target block in the wrong way
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.
Same answer as below, I think it's a good thing to not mark a peer as invalid if some sidecars are missing. I could add the check for MinBlocksForBlobRequests
here though, WDYT?
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.
Same answer as below, I think it's a good thing to not mark a peer as invalid if some sidecars are missing. I could add the check for
MinBlocksForBlobRequests
here though, WDYT?
👌
I'm closing this PR. After thinking about it, adding the check for It seems that issues related to sidecars not being delivered have vanished, at least for now... |
Some "sidecars" validation steps were missing in a few places still: the downloader when validating a Header, and the handler when delivering Blocks.