NDRS-571: LinearChainSync: Verify parent hashes. #508
Conversation
| @@ -445,12 +450,16 @@ where | |||
| self.block_downloaded(rng, effect_builder, block.header()) | |||
There was a problem hiding this comment.
I guess this is yet another place where syncing while reusing local storage would fail – we would not execute blocks that we already have in the storage, so we would not get consensus instances they would have created.
There was a problem hiding this comment.
But we do the same thing we do with incoming blocks: just call block_downloaded. I think that will execute them?
| fn current_block(&self) -> Option<&BlockHeader> { | ||
| match &self.state { | ||
| State::SyncingTrustedHash { current_block, .. } => Option::as_ref(&*current_block), | ||
| State::SyncingDescendants { current_block, .. } => Some(&*current_block), |
There was a problem hiding this comment.
How come you can do Option::as_ref(…) above and Some(…) here? Can you use the same version in both places?
There was a problem hiding this comment.
I think the problem I ran into is that Box has as_ref, too, so if you call it on a Box<Option<_>>, it will call the Box one and return an &Option<_>. SyncingDescendants' current_block is not an Option anymore.
| @@ -86,7 +86,7 @@ enum State { | |||
| /// Linear chain block being downloaded. | |||
| linear_chain_block: Box<Option<BlockHeader>>, | |||
| /// Block we received from a node and are currently executing. | |||
| current_block: Box<Option<BlockHeader>>, | |||
| current_block: Box<BlockHeader>, | |||
There was a problem hiding this comment.
If I understand correctly, it's not a current block anymore but the last block we had successfully downloaded and handled. The comment is also off.
There was a problem hiding this comment.
I didn't change when it gets set, except that I also set it when transitioning to SyncingDescendants.
Actually, the comment on the other current_block above also isn't correct, is it? current_block is not set when we start downloading it, but when we download its deploys. And it stays until we download the next block's deploys.
I still think the name is kind of okay; not sure what's the alternative? latest_block?
Also, do we need linear_chain_block at all?
|
bors r+ |
|
Build succeeded: |
https://casperlabs.atlassian.net/browse/NDRS-571