Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

enable disputes #3478

Merged
merged 18 commits into from
Jul 26, 2021
Merged

enable disputes #3478

merged 18 commits into from
Jul 26, 2021

Conversation

ordian
Copy link
Member

@ordian ordian commented Jul 15, 2021

Fixes #3473

  • Use RelayChainSelection when instantiating BABE and GRANDPA
  • Test on Rococo

WARNING: this PR bums the parachain's DB version, which means it's a one-way migration unless you delete the parachain's DB folder.

@ordian ordian added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jul 15, 2021
@ordian ordian marked this pull request as ready for review July 15, 2021 15:37
Copy link
Contributor

@drahnr drahnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍 , also shows why we need to get rid of AllSubsystems eventually.

Will give it a second pass as soon as the remaining checkmarks are set :)

/// A Dispute Distribution subsystem.
pub dispute_distribution: DD,
/// A Chain Selection subsystem.
pub chain_selection: CS,
Copy link
Contributor

@rphmeier rphmeier Jul 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this automatically generate communication handling for the subsystems?

Copy link
Contributor

@drahnr drahnr Jul 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a barely used abstraction for Overseer::new which will be removed soon™ ( tracked as chore in #3427 ), removing the wip keyword from the #[subsystem(..)] annotation does it.

node/service/src/lib.rs Outdated Show resolved Hide resolved
Co-authored-by: Robert Habermeier <rphmeier@gmail.com>
#[subsystem(no_dispatch, wip, DisputeDistributionMessage)]
dipute_distribution: DisputeDistribution,
#[subsystem(no_dispatch, DisputeDistributionMessage)]
dispute_distribution: DisputeDistribution,
Copy link
Contributor

@drahnr drahnr Jul 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rphmeier : Removing the wip hooks up all the channels as expected. I'll write some proper documentation once #3454 is complete.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woah, I like this "wip" thing.

node/service/src/lib.rs Outdated Show resolved Hide resolved
* master:
  Update secp256k1 and remove unrequired usage (#3502)
  Bump libc from 0.2.91 to 0.2.98 (#3496)
  Bump slotmap from 1.0.2 to 1.0.5 (#3495)
  Gossip rebroadcast rate limiter (#3494)
  dependabot: ignore another git dep (#3493)
  add rustfmt toml (#3491)
  Disputes runtime (#2947)
  Bump async-process from 1.0.1 to 1.1.0 (#3122)
  remove the kubernetes helm chart (#3483)
  added pallet-proxy in rococo feature dependencies (#3486)
  Update BEEFY+MMR integration. (#3480)
  more verbose asserts (#3476)
  ci: use srtool-actions to build runtimes (#3423)
  overseer gen minor chore fixes (#3479)
@ordian ordian linked an issue Jul 21, 2021 that may be closed by this pull request
@ordian ordian added the E1-database_migration PR introduces code that does a one-way migration of the database. label Jul 21, 2021
@andresilva
Copy link
Contributor

andresilva commented Jul 21, 2021

I think we should probably play it safer with enabling the stagnant checks, maybe there should be a mode where we only log about this before rolling it out on Kusama.

@rphmeier
Copy link
Contributor

Ok, let's make a small PR where we change the configuration of the chain selection subsystem to prevent turning on the stagnant check worker.

@rphmeier
Copy link
Contributor

I pushed a commit to address @andresilva's concerns. However, we should introduce the stagnant check at the same time as removing the training wheels. Otherwise the fork-choice rule may follow a stagnant chain indefinitely.

* master:
  Reduce staking miner reward (companion `substrate/pull/9395`) (#3465)
  Parachains shared.rs to Frame V2 (#3425)
  Parachains hrmp.rs to Frame V2 (#3475)
  Migrate slots pallet to pallet attribute macro. (#3218)
  Improve test in bridge (#3507)
  parachain dmp.rs to Frame V2 (#3426)
  Parachains inclusion.rs to Frame V2 (#3440)
  Dispute coordinator - Recover disputes on startup (#3481)
  Use correct syntax for owning all files in a folder (#3510)
  Add wococo-local chain spec (#3509)
  Dispute vote filtering for block authors (#3498)
  Bump indexmap from 1.6.1 to 1.7.0 (#3497)
  Companion for substrate #9315 (#3477)
@@ -159,16 +160,22 @@ impl<Client> HeadSupportsParachains for Arc<Client> where
}


/// A handler used to communicate with the [`Overseer`].
/// A handle used to communicate with the [`Overseer`].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ty

pub struct Handle(pub Option<OverseerHandle>);
pub enum Handle {
/// Used only at initialization to break the cyclic dependency.
// TODO: refactor in https://github.com/paritytech/polkadot/issues/3427
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ordian ordian merged commit 2d19780 into master Jul 26, 2021
@ordian ordian deleted the ao-enable-disputes branch July 26, 2021 12:46
ordian added a commit that referenced this pull request Jul 26, 2021
* master:
  enable disputes (#3478)
  Do not leak active head data in statement distribution (#3519)
  Fix TransactAsset Implementation (#3345)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. E1-database_migration PR introduces code that does a one-way migration of the database.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable Chain Selection, Dispute Coordinator, and Dispute Participation
4 participants