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

session-info: add new fields + migration #4545

Merged
merged 22 commits into from
Dec 27, 2021

Conversation

ordian
Copy link
Member

@ordian ordian commented Dec 16, 2021

This PR adds new fields to SessionInfo module in preparation for implementing dispute offences and validator disabling.
In order to disable a validator, we need an index into the original set, not the one reduced to (shuffled) parachain validators.

  • is moving ParachainHost to v2 gonna cause problems?
  • confirm it's working (test?)
  • fix failing tests

cumulus companion: paritytech/cumulus#875

@ordian ordian added A0-please_review Pull request needs code review. B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade. labels Dec 16, 2021
@github-actions github-actions bot added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels Dec 16, 2021
@ordian ordian added the D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. label Dec 16, 2021
* master:
  Fix fmt on master (#4546)
  Support new version of zombienet (#4528)
  naming consistency (#4539)
  paras: add log target (#4478)
…kadot into ao-session-info-migration

* 'ao-session-info-migration' of github.com:paritytech/polkadot:
* master:
  dispute-coordinator: fix underflow panic (#4557)
  Bump futures from 0.3.18 to 0.3.19 (#4567)
  Bump lru from 0.7.0 to 0.7.1 (#4566)
  Update Polkadot (#4561)
  Suggest installing graphviz before book building (#4565)
  [Zombienet] fix test creds (#4562)
  chain-api: stop ancestors lookup at block #0 (#4560)
  use v1.0.2 of zombienet (#4553)
  remove invalid dispute subsystem replace (#4559)
  Bump hyper from 0.14.15 to 0.14.16 (#4550)
  Create a README for XCMv2 detailing notable changes (#4059)
  enable disputes for known chains, except for polkadot (#4464)
  dispute statements node side limiting (#4541)
  Bump serde from 1.0.131 to 1.0.132 (#4551)
  Bump nix from 0.23.0 to 0.23.1 (#4552)
  Dispute coordinator: look for included candidates in non-finalized chain (#4508)
* master:
  Fix currently-checking-cache (#4410)
  Companion for Substrate#10445 (#4506)
  pvf-precheck: update rustdoc for paras module (#4459)
ordian added a commit to paritytech/cumulus that referenced this pull request Dec 22, 2021
@ordian ordian marked this pull request as ready for review December 22, 2021 11:41
@ordian ordian added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Dec 22, 2021
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 to me. I'd argue we eventually want to find a more streamlined way of versioning (which is mostly expanding structs) beyond copying structs and adding fields. Not sure if nesting is the best thing either. A discussion to be had in the future. I digress. LGTM

@@ -740,6 +740,22 @@ impl ChainBuilder {
}
}

fn session_info(keys: &[Sr25519Keyring]) -> SessionInfo {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably prefix this with dummy_ or make_ to make it explicit this is only every used for test and not some internal fn.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree, although this is a private function in tests module :P

Copy link
Contributor

@drahnr drahnr Dec 22, 2021

Choose a reason for hiding this comment

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

Should have Prefix It With "Micro nit:" 🌻

@ordian ordian added C3-medium PR touches the given topic and has a medium impact on builders. and removed C1-low PR touches the given topic and has a low impact on builders. labels Dec 22, 2021
@ordian
Copy link
Member Author

ordian commented Dec 22, 2021

bumping to medium priority, because we want validators to update before the runtime is enacted
thanks to @pepyakin for bringing this up

@ordian
Copy link
Member Author

ordian commented Dec 22, 2021

bot merge

@paritytech-processbot
Copy link

Error: It's not possible to meet the minimal approval count of 1 in paritytech/cumulus#872

ordian added a commit to paritytech/cumulus that referenced this pull request Dec 22, 2021
ordian added a commit to paritytech/cumulus that referenced this pull request Dec 22, 2021
@ordian
Copy link
Member Author

ordian commented Dec 27, 2021

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 6eacc1f into master Dec 27, 2021
@paritytech-processbot paritytech-processbot bot deleted the ao-session-info-migration branch December 27, 2021 08:01
paritytech-processbot bot pushed a commit to paritytech/cumulus that referenced this pull request Dec 27, 2021
* companion for paritytech/polkadot#4545

* update lockfile for polkadot

Co-authored-by: parity-processbot <>
ordian added a commit that referenced this pull request Dec 27, 2021
* master:
  make check-dependent-* only be executed in PRs (#4588)
  session_info: add dispute_period and random_seed (#4547)
  session-info: add new fields + migration (#4545)
  Bump zstd from 0.9.0+zstd.1.5.0 to 0.9.1+zstd.1.5.1 (#4597)
  Relaunch Rococo (#4577)
  Companion for substrate#9732 (#4104)
  Better logs and metrics on PoV fetching. (#4593)
drahnr pushed a commit that referenced this pull request Jan 4, 2022
* session_info: v2 + migration

* use primitives::v2

* use polkadot_primitives::v2

* impl primitives::v2

* fix approval-voting tests

* fix other tests

* hook storage migration up

* backwards compat (1)

* backwards compat (2)

* fmt

* fix tests

* FMT

* do not reexport v1 in v2

* fmt

* set storage version to 1

Co-authored-by: Javier Viola <[email protected]>
Wizdave97 pushed a commit to ComposableFi/polkadot that referenced this pull request Feb 3, 2022
* session_info: v2 + migration

* use primitives::v2

* use polkadot_primitives::v2

* impl primitives::v2

* fix approval-voting tests

* fix other tests

* hook storage migration up

* backwards compat (1)

* backwards compat (2)

* fmt

* fix tests

* FMT

* do not reexport v1 in v2

* fmt

* set storage version to 1

Co-authored-by: Javier Viola <[email protected]>
drahnr pushed a commit that referenced this pull request Feb 16, 2022
* session_info: v2 + migration

* use primitives::v2

* use polkadot_primitives::v2

* impl primitives::v2

* fix approval-voting tests

* fix other tests

* hook storage migration up

* backwards compat (1)

* backwards compat (2)

* fmt

* fix tests

* FMT

* do not reexport v1 in v2

* fmt

* set storage version to 1

Co-authored-by: Javier Viola <[email protected]>
@louismerlin louismerlin added D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited. and removed D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Jul 5, 2022
WebWizrd8 added a commit to WebWizrd8/cumulus that referenced this pull request Nov 21, 2022
* companion for paritytech/polkadot#4545

* update lockfile for polkadot

Co-authored-by: parity-processbot <>
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. C3-medium PR touches the given topic and has a medium impact on builders. D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited. E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants