This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Import multiple authority set change blocks #1808
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
andresilva
added
A3-in_progress
Contributor
Author
|
@svyatonik Should I update |
andresilva
changed the title
Contributor
|
I don't think so - the logic of |
andresilva
added
A0-please_review
Contributor
Author
andresilva
commented
Feb 19, 2019
| /// assumes that changes on the same branch will be added in-order. The | ||
| /// given function `is_descendent_of` should return `true` if the second | ||
| /// hash (target) is a descendent of the first hash (base). | ||
| pub(crate) fn add_pending_change<F, E>( |
Contributor
Author
There was a problem hiding this comment.
I wonder if as a safety fallback we should cap the number of pending changes in a given fork. I need to think how this will interact with the offline fallback as well.
rphmeier
approved these changes
Feb 19, 2019
rphmeier
added
A8-looksgood
and removed
A0-please_review
andresilva
added a commit
that referenced
this pull request
Feb 22, 2019
Fixing the conflicts requires (re)writing some code to properly integrate with the changes from #1808. This commit just comments all code that is conflicting/non-working replacing it with `unreachable!()` statements. I prefer for the integration to be explicit over follow-up commits rather than being buried in a merge commit.
MTDK1
pushed a commit
to bdevux/substrate
that referenced
this pull request
Apr 12, 2019
* core: implement logic for tracking dag of possible pending changes * core: move pending justifications dag to its own crate * core: remove unnecessary clone bounds on dag * core: request justifications in-order from the dag * core: dag: rename changes variables to node * core: dag: allow finalizing blocks not part of dag * core: dag: track best finalized number * core: dag: add more tests * core: sync: clean up pending justifications dag * core: dag: derive codec decode encode * core: dag: better error support * core: dag: add finalization guarded by predicate * core: grandpa: track multiple authority set changes in dag * core: dag: add pre-order iterator * core: grandpa: request justifications on startup * core: dag: rearrange order of definitions * core: rename util/dag to util/fork_tree * core: fork_tree: add docs * core: fork_tree: add more tests * core: fork_tree: fix issues found in tests * core: grandpa: fix authorities tests * core: grandpa: add docs for is_descendent_of * core: sync: add docs for PendingJustifications * core: sync: add test for justification requests across forks * core: sync: don't resend import or finality notifications in tests * core: grandpa: add test for importing multiple change blocks * core: grandpa: fix logic for checking if a block enacts a change * core: grandpa: fix authorities tests
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.
This PR changes grandpa block import to allow importing multiple pending authority set changes. The changes are tracked in a tree that tracks changes across all unfinalized forks, the changes must be non-overlapping (this is enforced by the runtime API) and must be finalized in-order. As blocks are finalized stale forks of the tree are pruned and their signalled changes discarded.
Additionally, the sync protocol for requesting justifications also tracks the requests in a tree using similar semantics.
TODO:
Fixes #1497.