This repository was archived by the owner on Nov 15, 2023. It is now read-only.
refactor View to include finalized_number #2128
Merged
Conversation
This file contains hidden or 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
ordian
added
B0-silent
github-actions
bot
added
the
A3-in_progress
ordian
commented
Dec 17, 2020
Comment on lines
+658
to
+661
| // we don't send the view updates here, but delay them until the next `Action::ActiveLeaves` | ||
| // otherwise it might break assumptions of some of the subsystems | ||
| // that we never send the same `ActiveLeavesUpdate` | ||
| // this is fine, we will get `Action::ActiveLeaves` on block finalization anyway |
Contributor
There was a problem hiding this comment.
We get ActiveLeavesUpdates on finality also?
Contributor
There was a problem hiding this comment.
I like the idea of deferring, though.
Author
There was a problem hiding this comment.
Hmm, even with 7c91559, this will not going to work if the update is empty. I'll think how to fix that.
github-actions
bot
added
A0-please_review
drahnr
reviewed
Dec 17, 2020
drahnr
reviewed
Dec 17, 2020
drahnr
approved these changes
Dec 17, 2020
Contributor
drahnr
left a comment
drahnr
left a comment
There was a problem hiding this comment.
LGTM, a couple of nits :)
montekki
reviewed
Dec 17, 2020
drahnr
approved these changes
Dec 17, 2020
rphmeier
approved these changes
Dec 17, 2020
Contributor
rphmeier
left a comment
rphmeier
left a comment
There was a problem hiding this comment.
Generally looks good, except for question about active leaves update on finality.
rphmeier
approved these changes
Dec 17, 2020
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.
For #1976 we need to get finalized block number from a view update:
https://w3f.github.io/parachain-implementers-guide/node/approval/approval-distribution.html#networkbridgeeventpeerviewchange
This PR adds block_number to
OverseerSignal::BlockFinalizedand toView.Note: this alters the wire message format for parachain networking, so we after this PR is merged we would likely need to restart our testnet.