Uh oh!
There was an error while loading. Please reload this page.
Snapshot synchronization - #35
Conversation
remagpie
commented
Dec 27, 2019
@sgkim126 Sure. I'm fixing the commits a bit, so I'll add them to the description after that. |
19332ac to
8d25a18Compare| } | ||
| } | ||
| fn on_chunk_response(&mut self, from: &NodeId, roots: &[H256], chunks: &[Vec<u8>]) { |
There was a problem hiding this comment.
Please add an assertion that the size of roots and chunks are the same.
| if !have_chunk_request { | ||
| let mut peer_ids: Vec<_> = self.header_downloaders.keys().cloned().collect(); | ||
| peer_ids.shuffle(&mut thread_rng()); | ||
| if let Some(id) = peer_ids.first() { |
There was a problem hiding this comment.
It will be better if we check the peer has a higher block number than the snapshot header or not.
@remagpie What do you think about adding a FIXME here?
There was a problem hiding this comment.
That's not a strict requirement, but it'd be nice to have. I'll add it.
| @@ -198,7 +198,7 @@ pub trait ImportBlock { | |||
| /// Import a trusted bootstrap header into the blockchain | |||
| if let Some((shard_id, shard_root)) = empty_shard { | ||
| return State::SnapshotShardChunk(shard_id, shard_root) | ||
| if let Some((shard_id, _)) = empty_shard { | ||
| return State::SnapshotShardChunk(shard_id, hash) |
There was a problem hiding this comment.
These lines are not needed. It seems it is included by mistake.
There was a problem hiding this comment.
It seems so. Thanks for noticing this.
All `self.header_downloaders.keys()` are in `connected_nodes`, and `self.header_downloaders` keep tracks of nodes which have sent at least one peer status out of connected nodes. The following commit will prohibit the node sending peer requests before it finishes to snapshot sync, so it can send peer requests before share peer status. So this changes make the node accept requests from the node who didn't send any peer status.
majecty
commented
Jan 16, 2020
@sgkim126 Do you have any concerns about merging this PR? If you don't have it, I want to merge this PR to the Foundry master branch. |
majecty
commented
Jan 16, 2020
@sgkim126 Okay, I'll rebase this PR on the master branch and merge it. |
majecty
commented
Jan 16, 2020
Merged. |
This PR implements snapshot synchronization.
There are lots of commits included in this PR, and they were originally introduced as a number of separated PRs in
CodeChain.Some parts of this PR will be extracted as a separated PR if requested, or if I feel it's required.
Depends:
#20#29#34#37The commits that are not extracted as a separate PR are as follows: