-
Notifications
You must be signed in to change notification settings - Fork 662
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
[chunks] Move chain_head maintenance into ShardsManager #7784
Conversation
robin-near
commented
Oct 5, 2022
- chain_head, which used to be passed into the ShardsManager with the process_partial_encoded_* calls, is now kept by the ShardsManager, which is required to send chunk management network messages directly to the ShardsManager. Whenever the chain_head is updated, the Client will provide the new chain_head to the ShardsManager. This makes chain_head updating asynchronous, but that is OK, because this is only used to opportunistically validate orphan chunk headers, and the accuracy of the chain_head does not affect correctness.
- Note that "header_head", which is different from chain_head (i.e. it only waits for a valid block header but not the entire valid block), is still passed in; this is used for the chunk requesting path. This is not a problem, because the request path is initiated by the Client (upon receiving a block with missing chunks), except that we also pass in a header_head when resending requests; the use of header_head for this context does not require an accurate header_head, and this will be handled in a later PR to read from the store directly.
- This PR also moves response handling, forward handling, and incomplete chunks checking to the ShardsManager, bypassing the Client completely. It also makes some functions no longer pub.
Linking it with #7662 |
@@ -481,6 +481,7 @@ pub struct ShardsManager { | |||
encoded_chunks: EncodedChunksCache, | |||
requested_partial_encoded_chunks: RequestPool, | |||
chunk_forwards_cache: lru::LruCache<ChunkHash, HashMap<u64, PartialEncodedChunkPart>>, | |||
chain_head: Option<Tip>, |
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.
Add comment that this is a "best effort" and updated asynchronously, and point to where the source of truth is?
I think it's useful to explicitly mark "this is not source of truth" bit
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.
Agreed! Added a comment.