This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
Bypass chain-selection subsystem until disputes are enabled. #3676
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
drahnr
added
A1-needsburnin
A0-please_review
rphmeier
changed the title
eskimor
approved these changes
Aug 20, 2021
|
(as discussed, needs to feature-gate disputes lag checking as well) |
|
The check labels bot is wrong. |
s3krit
added
B1-releasenotes
and removed
B0-silent
|
check_labels should be happy now |
eskimor
reviewed
Aug 20, 2021
TODO move the fallback into selection
rphmeier
reviewed
Aug 20, 2021
rphmeier
reviewed
Aug 20, 2021
rphmeier
reviewed
Aug 20, 2021
| .map_err(Error::OverseerDisconnected) | ||
| .map_err(|e| ConsensusError::Other(Box::new(e)))? | ||
| .unwrap_or_else(|| (subchain_number, subchain_head)); | ||
| let lag = if cfg!(feature = "disputes") { |
drahnr
commented
Aug 20, 2021
Lldenaurois
force-pushed
the
bernhard-cleanup-robs-patch
branch
from
01eca29
to
1015bf3
Compare
| lag_disputes | ||
| } else { | ||
| lag | ||
| }; | ||
|
|
||
| // 4. Apply the maximum safeguard to the finality lag. | ||
| if lag > MAX_FINALITY_LAG { |
There was a problem hiding this comment.
im reviewing without context of all the surrounding code, but the behavior here before and after is different.
This check before was always checking against lag, and with the new code, could check against lag_disputes with the feature flag.
Was this a bug before? or a bug now?
There was a problem hiding this comment.
Bug before, addressed here. Approval and disputes both apply constraints to the finalizable chain and previously the disputes lag wasn't accounted for in the 'max' check.
GopherJ
pushed a commit
to parallel-finance/polkadot
that referenced
this pull request
Aug 23, 2021
…ech#3676) * cleanup of rob's patch * depend on feature disputes, both lag and query * typo * make it work TODO move the fallback into selection * option * Update node/service/src/relay_chain_selection.rs * remove trait implementation and fix errors * hotfix: tests * fmt * remove unused trait * make pub(crate) for test * disable tests with disputed blocks * Fix warnings Co-authored-by: Robert Habermeier <rphmeier@gmail.com> Co-authored-by: Lldenaurois <Ljdenaurois@gmail.com>
bkchr
pushed a commit
that referenced
this pull request
Aug 23, 2021
* Bypass chain-selection subsystem until disputes are enabled. (#3676) * cleanup of rob's patch * depend on feature disputes, both lag and query * typo * make it work TODO move the fallback into selection * option * Update node/service/src/relay_chain_selection.rs * remove trait implementation and fix errors * hotfix: tests * fmt * remove unused trait * make pub(crate) for test * disable tests with disputed blocks * Fix warnings Co-authored-by: Robert Habermeier <rphmeier@gmail.com> Co-authored-by: Lldenaurois <Ljdenaurois@gmail.com> * properly gate sanity check (#3684) Co-authored-by: Bernhard Schuster <bernhard@ahoi.io> Co-authored-by: Robert Habermeier <rphmeier@gmail.com> Co-authored-by: Lldenaurois <Ljdenaurois@gmail.com>
15 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.
TheUses thecfg!(feature = "fx")does not exist yet, but it does not matter for compilation.feature = "disputes"as gate for the relay chain selection rule.