-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add NetworkBridgeEvent::UpdatedAuthorityIds
#6227
Conversation
@rphmeier I would just like to confirm that I understand the issue correctly. As mentioned we want to introduce I am not sure for which network event should we check and emit the |
@rphmeier Could you PTAL. |
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.
Some thoughts with my understanding:
- Key rotations will lead to a rather quick DHT update, which means that authority discovery cache might be updated soon after.
get_authority_ids_by_peer_id
method should return both oldauthority_id
and the new one after the update. - However, on chain the changes will take place on the new session only.
The only problem here seems to be statement-distribution maintains an additional mapping AuthorityId -> PeerId
which is only updated on peer connection/disconnection, so it will not take into account session rotations.
Since it's a only becomes a problem on session rotations, we could issue this message in gossip-support on each new session.
@ordian I think the PR should be feasible now, if I am not wrong, I think I implemented all of the necessary logic for the |
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.
Thanks!
Co-authored-by: Andronik <[email protected]>
Co-authored-by: Andronik <[email protected]>
Co-authored-by: Andronik <[email protected]>
Co-authored-by: Andronik <[email protected]>
Co-authored-by: Andronik <[email protected]>
Co-authored-by: Andronik <[email protected]>
Co-authored-by: Andronik <[email protected]>
Co-authored-by: Andronik <[email protected]>
Co-authored-by: Andronik <[email protected]>
Co-authored-by: Andronik <[email protected]>
Co-authored-by: Andronik <[email protected]>
Co-authored-by: Andronik <[email protected]>
Co-authored-by: Andronik <[email protected]>
@ordian Could you approve the PR if the changes are ok? |
@bkchr Could you review this? |
@Szegoo sorry for the delay! Could you merge recent master? |
Done. |
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.
Looks good - some guide changes would be nice too!
Sorry, I completely forgot about this PR.
Doing this check every session is a good step. Probably all we'll need. But more frequent checks may be needed in the future.
Closes: #6192