More sync and propagation fixes #420
Conversation
|
Propagation logic was changed a bit. Check for latest peer hash being too far behind has been removed - we don't know how far it has synced from other peers and should not keep it in the dark. Instead there is now a limit on how many blocks/hashes to send at once. |
| @@ -437,6 +448,10 @@ impl ChainSync { | |||
| trace!(target: "sync", "{} -> NewBlock ({})", peer_id, h); | |||
| let header: BlockHeader = try!(header_rlp.as_val()); | |||
| let mut unknown = false; | |||
| { | |||
| let peer = self.peers.get_mut(&peer_id).expect("ChainSync: unknown peer"); | |||
There was a problem hiding this comment.
this expect doesn't look like it's a fundamental logic error. if it is, add a comment explaining why it can never ever happen and other pre-condition assert!s in the code to make the proof clear.
There was a problem hiding this comment.
i notice that there are others in the code, so happy for the whole lot to be cleared up in a later PR if preferable.
There was a problem hiding this comment.
Better fix that now
|
quite a logically complex PR - a bit of explanation of what's going on would be appreciated. |
| @@ -162,6 +162,8 @@ struct PeerInfo { | |||
| asking: PeerAsking, | |||
| /// A set of block numbers being requested | |||
| asking_blocks: Vec<BlockNumber>, | |||
| /// Holds requested header hash if currently requesting block header by hash | |||
| asking_hash: Option<H256>, | |||
There was a problem hiding this comment.
Normally headers and bodies are downloaded by number, unless downloading a header known from NewBlockHashes packet. Currently, if two NewBlockHash packets with the same hash arrive at the same time this may result in same header being requested twice. This was added to keep track of the requested hash.
arkpar
added
A4-gotissues 💥
arkpar
added
A0-pleasereview 🤓
arkpar
added
A3-inprogress ⏳
arkpar
added
A0-pleasereview 🤓
NikVolf
added
A8-looksgood 🦄
|
lgtm |
More sync and propagation fixes
fixes #419