This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
grandpa: send neighbor packets to light clients on set transition #13559
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andresilva
added
A0-please_review
andresilva
commented
Mar 7, 2023
Comment on lines
794
to
818
| let local_view = match self.local_view { | ||
| None => return None, | ||
| Some(ref mut v) => | ||
| if v.round == round { | ||
| // Do not send neighbor packets out if `round` has not changed --- | ||
| // such behavior is punishable. | ||
| return None | ||
| } else { | ||
| v | ||
| }, | ||
| }; | ||
|
|
||
| let set_id = local_view.set_id; | ||
| let set_id = local_view.set_id; | ||
|
|
||
| debug!( | ||
| target: LOG_TARGET, | ||
| "Voter {} noting beginning of round {:?} to network.", | ||
| self.config.name(), | ||
| (round, set_id) | ||
| ); | ||
| debug!( | ||
| target: LOG_TARGET, | ||
| "Voter {} noting beginning of round {:?} to network.", | ||
| self.config.name(), | ||
| (round, set_id) | ||
| ); | ||
|
|
||
| local_view.update_round(round); | ||
| local_view.update_round(round); | ||
|
|
||
| self.live_topics.push(round, set_id); | ||
| self.peers.reshuffle(); | ||
| } | ||
| self.multicast_neighbor_packet() | ||
| self.live_topics.push(round, set_id); | ||
| self.peers.reshuffle(); |
There was a problem hiding this comment.
Nothing changed here, I just removed this useless scope (rustc was dumber when this code was written).
andresilva
commented
Mar 7, 2023
Comment on lines
+826
to
+854
| let local_view = match self.local_view { | ||
| ref mut x @ None => x.get_or_insert(LocalView::new(set_id, Round(1))), | ||
| Some(ref mut v) => | ||
| if v.set_id == set_id { | ||
| let diff_authorities = self.authorities.iter().collect::<HashSet<_>>() != | ||
| authorities.iter().collect::<HashSet<_>>(); | ||
|
|
||
| if diff_authorities { | ||
| debug!( | ||
| target: LOG_TARGET, | ||
| "Gossip validator noted set {:?} twice with different authorities. \ | ||
| Was the authority set hard forked?", | ||
| set_id, | ||
| ); | ||
|
|
||
| self.authorities = authorities; | ||
| } | ||
|
|
||
| local_view.update_set(set_id); | ||
| self.live_topics.push(Round(1), set_id); | ||
| self.authorities = authorities; | ||
| } | ||
| self.multicast_neighbor_packet() | ||
| // Do not send neighbor packets out if the `set_id` has not changed --- | ||
| // such behavior is punishable. | ||
| return None | ||
| } else { | ||
| v | ||
| }, | ||
| }; | ||
|
|
||
| local_view.update_set(set_id); | ||
| self.live_topics.push(Round(1), set_id); | ||
| self.authorities = authorities; |
bkchr
approved these changes
Mar 7, 2023
skunert
approved these changes
Mar 9, 2023
Co-authored-by: Bastian Köcher <git@kchr.de>
andresilva
force-pushed
the
andre/grandpa-neighbor-packets-light-clients
branch
from
77feeae
to
89ed9f2
Compare
|
bot merge |
paritytech-processbot
bot
deleted the
andre/grandpa-neighbor-packets-light-clients
branch
melekes
approved these changes
Mar 9, 2023
|
Is this change included in Polkadot v0.9.40? Or is it for 0.9.41? |
|
Is part of 0.9.40 |
ukint-vs
pushed a commit
to gear-tech/substrate
that referenced
this pull request
Apr 10, 2023
…ritytech#13559) * grandpa: send neighbor packets to light clients on set transition * Apply suggestions from code review Co-authored-by: Bastian Köcher <git@kchr.de> * remove match statement --------- Co-authored-by: Bastian Köcher <git@kchr.de>
15 tasks
nathanwhit
pushed a commit
to nathanwhit/substrate
that referenced
this pull request
Jul 19, 2023
…ritytech#13559) * grandpa: send neighbor packets to light clients on set transition * Apply suggestions from code review Co-authored-by: Bastian Köcher <git@kchr.de> * remove match statement --------- Co-authored-by: Bastian Köcher <git@kchr.de>
Closed
46 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As part of the GRANDPA gossip protocol peers exchange small messages (neighbor packets) to inform their peers on what is their local view of the GRANDPA protocol, i.e. what round and set id they are currently observing. This is then used by the gossip logic to avoid gossiping information that peers aren't interested in (either stale or future). Currently these neighbor packets aren't sent to light clients, the reasoning being that since light clients aren't following the full GRANDPA protocol (i.e. they just import commits to finalize blocks), they don't need to care about the state of the peers they are connected to. Still this information can be useful to light clients around validator set boundaries, since it allows light clients to make an informed decision on who to ask justifications from (if necessary), i.e. if I'm looking for a justification for a block that changes the set id from 1 to 2, then any peer that announces being on set id 2 (or higher) should be able to provide such a justification.
This PR changes the gossip logic to also send neighbor packets to light clients when transitioning to a new set. Additionally, the light client also gets a neighbor packet on the initial connection to the peer (this was already the case before this PR).
Related: smol-dot/smoldot#239