-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[testnet] Fix missing committee blobs handling. #4986
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
Conversation
| builder | ||
| }; | ||
|
|
||
| runtime.thread_stack_size(4 << 20); |
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.
Maybe a comment?
| let committee_blob = self | ||
| .context() | ||
| .extra() | ||
| .get_blob(blob_id) | ||
| .await? | ||
| .ok_or(ExecutionError::BlobsNotFound(vec![blob_id]))?; |
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.
Probably not introduced in this PR but maybe we can return all missing committee blobs at once? Then the client can choose whether to udpate one-by-one or in a single message.
|
Closing this for now. I will apply your comments on #4978 and backport once it's merged. |
Motivation
#4978 revealed that the client fails to send the admin chain to validators that are missing a committee in some cases.
Proposal
Remove
committees_forfrom storage. ReturnBlobsNotFoundandEventsNotFoundinstead ofViewErrors.Also remove some redundant code, and check height before epoch when handling a validated block.
I got stack overflows in some cases, so I also doubled the client's Tokio thread stack size.
Test Plan
CI
#4978 failed without this.
Release Plan
Links