Handle substrate construct_runtime change: break runtime `Event` encoding, change some pallets error index #1692

gui1117 · 2020-09-09T13:42:37Z

companion for: paritytech/substrate#6969

construct_runtime changes the way it encodes Origin, Call, Event, this PR make use of newly introduced indices to make Call unchanged, and migrate scheduler usage of old pallet origin.

This PR does break Event but as far as I know this is not an issue, event should be decoded using metadata.

EDIT: If it needs more test, looking at code, and comparing expansion everything looks good. I can also do some call and origin encoding and decoding between master and this PR if wanted.

TODO

double check no module make use of ModuleToIndex in way that could break them.

Note for reviwer

the breaking change is only about the encoding of Event, Call, Origin, and implementation of ModuleToIndex. But there is no breaking usage of ModuleToIndex (as there is no such usage on polkadot (and substrate only usage is for Errors which should be read from metadata)). Call encoding is not breaking as index are used in a way not to break them, Event are breaking but similarly to Error they should be read from metadata as well.
Only Origin is a breaking change, and its only usage on chain is through scheduler, thus migration code has been written.

…me-with-index

gui1117 · 2020-09-09T14:30:35Z

Some expansion to help reviewing:

OriginCaller for polkadot on master:

#[allow(non_camel_case_types)]
pub enum OriginCaller {
    system(frame_system::Origin<Runtime>),
    pallet_collective_Instance1(pallet_collective::Origin<Runtime, pallet_collective::Instance1>),
    pallet_collective_Instance2(pallet_collective::Origin<Runtime, pallet_collective::Instance2>),
    #[allow(dead_code)]
    Void(::frame_support::Void),
}

OriginCaller for kusama on master

#[allow(non_camel_case_types)]
pub enum OriginCaller {
    system(frame_system::Origin<Runtime>),
    pallet_collective_Instance1(pallet_collective::Origin<Runtime, pallet_collective::Instance1>),
    pallet_collective_Instance2(pallet_collective::Origin<Runtime, pallet_collective::Instance2>),
    #[allow(dead_code)]
    Void(::frame_support::Void),
}

OriginCaller for westend on master

#[allow(non_camel_case_types)]
pub enum OriginCaller {
    system(frame_system::Origin<Runtime>),
    #[allow(dead_code)]
    Void(::frame_support::Void),
}

…me-with-index

gui1117 · 2020-09-18T15:36:21Z

I compared some other expansion just to double check:
(using cmd cargo rustc --profile=check -p polkadot-runtime --lib -- -Zunstable-options --pretty=expanded)

polkadot master (with manually written line number in front of the line)

pub enum Call {
00    System(::frame_support::dispatch::CallableCallFor<System, Runtime>),
01    Scheduler(::frame_support::dispatch::CallableCallFor<Scheduler, Runtime>),
02    Babe(::frame_support::dispatch::CallableCallFor<Babe, Runtime>),
03    Timestamp(::frame_support::dispatch::CallableCallFor<Timestamp, Runtime>),
04    Indices(::frame_support::dispatch::CallableCallFor<Indices, Runtime>),
05    Balances(::frame_support::dispatch::CallableCallFor<Balances, Runtime>),
06    Authorship(::frame_support::dispatch::CallableCallFor<Authorship, Runtime>),
07    Staking(::frame_support::dispatch::CallableCallFor<Staking, Runtime>),
08    Offences(::frame_support::dispatch::CallableCallFor<Offences, Runtime>),
09    Session(::frame_support::dispatch::CallableCallFor<Session, Runtime>),
10    FinalityTracker(::frame_support::dispatch::CallableCallFor<FinalityTracker, Runtime>),
11    Grandpa(::frame_support::dispatch::CallableCallFor<Grandpa, Runtime>),
12    ImOnline(::frame_support::dispatch::CallableCallFor<ImOnline, Runtime>),
13    AuthorityDiscovery(::frame_support::dispatch::CallableCallFor<AuthorityDiscovery, Runtime>),
14    Democracy(::frame_support::dispatch::CallableCallFor<Democracy, Runtime>),
15    Council(::frame_support::dispatch::CallableCallFor<Council, Runtime>),
16    TechnicalCommittee(::frame_support::dispatch::CallableCallFor<TechnicalCommittee, Runtime>),
17    ElectionsPhragmen(::frame_support::dispatch::CallableCallFor<ElectionsPhragmen, Runtime>),
18    TechnicalMembership(::frame_support::dispatch::CallableCallFor<TechnicalMembership, Runtime>),
19    Treasury(::frame_support::dispatch::CallableCallFor<Treasury, Runtime>),
20    DummyParachains(::frame_support::dispatch::CallableCallFor<DummyParachains, Runtime>),
21    DummyAttestations(::frame_support::dispatch::CallableCallFor<DummyAttestations, Runtime>),
22    DummySlots(::frame_support::dispatch::CallableCallFor<DummySlots, Runtime>),
23    DummyRegistrar(::frame_support::dispatch::CallableCallFor<DummyRegistrar, Runtime>),
24    Claims(::frame_support::dispatch::CallableCallFor<Claims, Runtime>),
25    Vesting(::frame_support::dispatch::CallableCallFor<Vesting, Runtime>),
26    Utility(::frame_support::dispatch::CallableCallFor<Utility, Runtime>),
27    DummyPurchase(::frame_support::dispatch::CallableCallFor<DummyPurchase, Runtime>),
28    Identity(::frame_support::dispatch::CallableCallFor<Identity, Runtime>),
29    Proxy(::frame_support::dispatch::CallableCallFor<Proxy, Runtime>),
30    Multisig(::frame_support::dispatch::CallableCallFor<Multisig, Runtime>),
}

polkadot with this PR

pub enum Call {
    #[codec(index = "0")] System(::frame_support::dispatch::CallableCallFor<System, Runtime>),
    #[codec(index = "1")] Scheduler(::frame_support::dispatch::CallableCallFor<Scheduler, Runtime>),
    #[codec(index = "2")] Babe(::frame_support::dispatch::CallableCallFor<Babe, Runtime>),
    #[codec(index = "3")] Timestamp(::frame_support::dispatch::CallableCallFor<Timestamp, Runtime>),
    #[codec(index = "4")] Indices(::frame_support::dispatch::CallableCallFor<Indices, Runtime>),
    #[codec(index = "5")] Balances(::frame_support::dispatch::CallableCallFor<Balances, Runtime>),
    #[codec(index = "6")] Authorship(::frame_support::dispatch::CallableCallFor<Authorship, Runtime>),
    #[codec(index = "7")] Staking(::frame_support::dispatch::CallableCallFor<Staking, Runtime>),
    #[codec(index = "8")] Offences(::frame_support::dispatch::CallableCallFor<Offences, Runtime>),
    #[codec(index = "9")] Session(::frame_support::dispatch::CallableCallFor<Session, Runtime>),
    #[codec(index = "10")] FinalityTracker(::frame_support::dispatch::CallableCallFor<FinalityTracker, Runtime>),
    #[codec(index = "11")] Grandpa(::frame_support::dispatch::CallableCallFor<Grandpa, Runtime>),
    #[codec(index = "12")] ImOnline(::frame_support::dispatch::CallableCallFor<ImOnline, Runtime>),
    #[codec(index = "13")] AuthorityDiscovery(::frame_support::dispatch::CallableCallFor<AuthorityDiscovery, Runtime>),
    #[codec(index = "14")] Democracy(::frame_support::dispatch::CallableCallFor<Democracy, Runtime>),
    #[codec(index = "15")] Council(::frame_support::dispatch::CallableCallFor<Council, Runtime>),
    #[codec(index = "16")] TechnicalCommittee(::frame_support::dispatch::CallableCallFor<TechnicalCommittee, Runtime>),
    #[codec(index = "17")] ElectionsPhragmen(::frame_support::dispatch::CallableCallFor<ElectionsPhragmen, Runtime>),
    #[codec(index = "18")] TechnicalMembership(::frame_support::dispatch::CallableCallFor<TechnicalMembership, Runtime>),
    #[codec(index = "19")] Treasury(::frame_support::dispatch::CallableCallFor<Treasury, Runtime>),
    #[codec(index = "24")] Claims(::frame_support::dispatch::CallableCallFor<Claims, Runtime>),
    #[codec(index = "25")] Vesting(::frame_support::dispatch::CallableCallFor<Vesting, Runtime>),
    #[codec(index = "26")] Utility(::frame_support::dispatch::CallableCallFor<Utility, Runtime>),
    #[codec(index = "28")] Identity(::frame_support::dispatch::CallableCallFor<Identity, Runtime>),
    #[codec(index = "29")] Proxy(::frame_support::dispatch::CallableCallFor<Proxy, Runtime>),
    #[codec(index = "30")] Multisig(::frame_support::dispatch::CallableCallFor<Multisig, Runtime>),
}

kusama master (with manually written line number in front of the line)

pub enum Call {
0    System(::frame_support::dispatch::CallableCallFor<System, Runtime>),
1    Babe(::frame_support::dispatch::CallableCallFor<Babe, Runtime>),
2    Timestamp(::frame_support::dispatch::CallableCallFor<Timestamp, Runtime>),
3    Indices(::frame_support::dispatch::CallableCallFor<Indices, Runtime>),
4    Balances(::frame_support::dispatch::CallableCallFor<Balances, Runtime>),
5    Authorship(::frame_support::dispatch::CallableCallFor<Authorship, Runtime>),
6    Staking(::frame_support::dispatch::CallableCallFor<Staking, Runtime>),
7    Offences(::frame_support::dispatch::CallableCallFor<Offences, Runtime>),
8    Session(::frame_support::dispatch::CallableCallFor<Session, Runtime>),
9    FinalityTracker(::frame_support::dispatch::CallableCallFor<FinalityTracker, Runtime>),
10    Grandpa(::frame_support::dispatch::CallableCallFor<Grandpa, Runtime>),
11    ImOnline(::frame_support::dispatch::CallableCallFor<ImOnline, Runtime>),
12    AuthorityDiscovery(::frame_support::dispatch::CallableCallFor<AuthorityDiscovery, Runtime>),
13    Democracy(::frame_support::dispatch::CallableCallFor<Democracy, Runtime>),
14    Council(::frame_support::dispatch::CallableCallFor<Council, Runtime>),
15    TechnicalCommittee(::frame_support::dispatch::CallableCallFor<TechnicalCommittee, Runtime>),
16    ElectionsPhragmen(::frame_support::dispatch::CallableCallFor<ElectionsPhragmen, Runtime>),
17    TechnicalMembership(::frame_support::dispatch::CallableCallFor<TechnicalMembership, Runtime>),
18    Treasury(::frame_support::dispatch::CallableCallFor<Treasury, Runtime>),
19    Claims(::frame_support::dispatch::CallableCallFor<Claims, Runtime>),
20    DummyParachains(::frame_support::dispatch::CallableCallFor<DummyParachains, Runtime>),
21    DummyAttestations(::frame_support::dispatch::CallableCallFor<DummyAttestations, Runtime>),
22    DummySlots(::frame_support::dispatch::CallableCallFor<DummySlots, Runtime>),
23    DummyRegistrar(::frame_support::dispatch::CallableCallFor<DummyRegistrar, Runtime>),
24    Utility(::frame_support::dispatch::CallableCallFor<Utility, Runtime>),
25    Identity(::frame_support::dispatch::CallableCallFor<Identity, Runtime>),
26    Society(::frame_support::dispatch::CallableCallFor<Society, Runtime>),
27    Recovery(::frame_support::dispatch::CallableCallFor<Recovery, Runtime>),
28    Vesting(::frame_support::dispatch::CallableCallFor<Vesting, Runtime>),
29    Scheduler(::frame_support::dispatch::CallableCallFor<Scheduler, Runtime>),
30    Proxy(::frame_support::dispatch::CallableCallFor<Proxy, Runtime>),
31    Multisig(::frame_support::dispatch::CallableCallFor<Multisig, Runtime>),
}

Kusama with this PR:

pub enum Call {
    #[codec(index = "0")] System(::frame_support::dispatch::CallableCallFor<System, Runtime>),
    #[codec(index = "1")] Babe(::frame_support::dispatch::CallableCallFor<Babe, Runtime>),
    #[codec(index = "2")] Timestamp(::frame_support::dispatch::CallableCallFor<Timestamp, Runtime>),
    #[codec(index = "3")] Indices(::frame_support::dispatch::CallableCallFor<Indices, Runtime>),
    #[codec(index = "4")] Balances(::frame_support::dispatch::CallableCallFor<Balances, Runtime>),
    #[codec(index = "5")] Authorship(::frame_support::dispatch::CallableCallFor<Authorship, Runtime>),
    #[codec(index = "6")] Staking(::frame_support::dispatch::CallableCallFor<Staking, Runtime>),
    #[codec(index = "7")] Offences(::frame_support::dispatch::CallableCallFor<Offences, Runtime>),
    #[codec(index = "8")] Session(::frame_support::dispatch::CallableCallFor<Session, Runtime>),
    #[codec(index = "9")] FinalityTracker(::frame_support::dispatch::CallableCallFor<FinalityTracker, Runtime>),
    #[codec(index = "10")] Grandpa(::frame_support::dispatch::CallableCallFor<Grandpa, Runtime>),
    #[codec(index = "11")] ImOnline(::frame_support::dispatch::CallableCallFor<ImOnline, Runtime>),
    #[codec(index = "12")] AuthorityDiscovery(::frame_support::dispatch::CallableCallFor<AuthorityDiscovery, Runtime>),
    #[codec(index = "13")] Democracy(::frame_support::dispatch::CallableCallFor<Democracy, Runtime>),
    #[codec(index = "14")] Council(::frame_support::dispatch::CallableCallFor<Council, Runtime>),
    #[codec(index = "15")] TechnicalCommittee(::frame_support::dispatch::CallableCallFor<TechnicalCommittee, Runtime>),
    #[codec(index = "16")] ElectionsPhragmen(::frame_support::dispatch::CallableCallFor<ElectionsPhragmen, Runtime>),
    #[codec(index = "17")] TechnicalMembership(::frame_support::dispatch::CallableCallFor<TechnicalMembership, Runtime>),
    #[codec(index = "18")] Treasury(::frame_support::dispatch::CallableCallFor<Treasury, Runtime>),
    #[codec(index = "19")] Claims(::frame_support::dispatch::CallableCallFor<Claims, Runtime>),
    #[codec(index = "24")] Utility(::frame_support::dispatch::CallableCallFor<Utility, Runtime>),
    #[codec(index = "25")] Identity(::frame_support::dispatch::CallableCallFor<Identity, Runtime>),
    #[codec(index = "26")] Society(::frame_support::dispatch::CallableCallFor<Society, Runtime>),
    #[codec(index = "27")] Recovery(::frame_support::dispatch::CallableCallFor<Recovery, Runtime>),
    #[codec(index = "28")] Vesting(::frame_support::dispatch::CallableCallFor<Vesting, Runtime>),
    #[codec(index = "29")] Scheduler(::frame_support::dispatch::CallableCallFor<Scheduler, Runtime>),
    #[codec(index = "30")] Proxy(::frame_support::dispatch::CallableCallFor<Proxy, Runtime>),
    #[codec(index = "31")] Multisig(::frame_support::dispatch::CallableCallFor<Multisig, Runtime>),
}

So call is not breaking.
About origin: the migration handles it.

runtime/kusama/src/lib.rs

runtime/polkadot/src/lib.rs

…me-with-index

kianenigma

Looks good with all the expansion examples.

jacogr · 2020-09-22T12:13:37Z

Been testing this alongside the Substrate branch and it does what it says on the tin.

…me-with-index

cheme

LGTM (I did look at the substrate one yesterday but could not give feedback in time (but it was looking good too).

ghost · 2020-09-22T14:54:20Z

Waiting for commit status.

ghost · 2020-09-22T14:58:30Z

Head SHA changed; merge aborted.

bkchr · 2020-09-22T15:01:41Z

bot merge

ghost · 2020-09-22T15:01:44Z

Waiting for commit status.

* master: Update to substrate 2.0 (#1744) Companion: Handle construct_runtime breaking change. (#1692) Companion for `ModuleToIndex` to `PalletInfo` rename (#1743) Companion for substrate/pull/7161 (#1739)

* master: Update to substrate 2.0 (#1744) Companion: Handle construct_runtime breaking change. (#1692) Companion for `ModuleToIndex` to `PalletInfo` rename (#1743) Companion for substrate/pull/7161 (#1739) Companion for 7155 (WeightInfo for Babe and Grandpa) (#1736) Companion PR for #7136 (WeightInfo for Session / Offences) (#1735)

use construct_runtime index

7690d3f

paritytech-ci added the D2-breaksapi label Sep 9, 2020

Merge remote-tracking branch 'origin/master' into gui-construct-runti…

f85cdfa

…me-with-index

gui1117 mentioned this pull request Sep 9, 2020

Allow pallet in construct_runtime to have fixed index paritytech/substrate#6969

Merged

1 task

fix

1657868

gui1117 marked this pull request as ready for review September 9, 2020 15:57

github-actions bot added the A0-please_review Pull request needs code review. label Sep 9, 2020

gui1117 added the A4-companion A PR that needs a companion PR to merge in parallel for one of its downstream dependencies. label Sep 9, 2020

gui1117 changed the title ~~Companion: Handle construct_runtime breaking change: https://github.com/paritytech/substrate/pull/6969~~ Companion: Handle construct_runtime breaking change. Sep 9, 2020

Merge remote-tracking branch 'origin/master' into gui-construct-runti…

66791e4

…me-with-index

shawntabrizi reviewed Sep 21, 2020

View reviewed changes

runtime/kusama/src/lib.rs Show resolved Hide resolved

shawntabrizi reviewed Sep 21, 2020

View reviewed changes

runtime/polkadot/src/lib.rs Show resolved Hide resolved

shawntabrizi approved these changes Sep 21, 2020

View reviewed changes

Merge remote-tracking branch 'origin/master' into gui-construct-runti…

a25ff29

…me-with-index

kianenigma approved these changes Sep 21, 2020

View reviewed changes

Merge remote-tracking branch 'origin/master' into gui-construct-runti…

384ed68

…me-with-index

cheme approved these changes Sep 22, 2020

View reviewed changes

"Update Substrate"

f8f5120

update substrate

14511be

gui1117 removed the A0-please_review Pull request needs code review. label Sep 22, 2020

gui1117 added A8-mergeoncegreen labels Sep 22, 2020

ghost merged commit 048c75c into master Sep 22, 2020

ghost deleted the gui-construct-runtime-with-index branch September 22, 2020 15:20

gui1117 changed the title ~~Companion: Handle construct_runtime breaking change.~~ Handle substrate construct_runtime change: break runtime Event encoding, change pallets error index Oct 12, 2020

gui1117 changed the title ~~Handle substrate construct_runtime change: break runtime Event encoding, change pallets error index~~ Handle substrate construct_runtime change: break runtime Event encoding, change some pallets error index Oct 12, 2020

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Handle substrate construct_runtime change: break runtime `Event` encoding, change some pallets error index #1692

Handle substrate construct_runtime change: break runtime `Event` encoding, change some pallets error index #1692

Uh oh!

gui1117 commented Sep 9, 2020 •

edited

Loading

Uh oh!

gui1117 commented Sep 9, 2020

Uh oh!

gui1117 commented Sep 18, 2020 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

kianenigma left a comment

Uh oh!

jacogr commented Sep 22, 2020

Uh oh!

cheme left a comment

Uh oh!

ghost commented Sep 22, 2020

Uh oh!

ghost commented Sep 22, 2020

Uh oh!

bkchr commented Sep 22, 2020

Uh oh!

ghost commented Sep 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Handle substrate construct_runtime change: break runtime Event encoding, change some pallets error index #1692

Handle substrate construct_runtime change: break runtime Event encoding, change some pallets error index #1692

Uh oh!

Conversation

gui1117 commented Sep 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Note for reviwer

Uh oh!

gui1117 commented Sep 9, 2020

Uh oh!

gui1117 commented Sep 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

polkadot master (with manually written line number in front of the line)

polkadot with this PR

kusama master (with manually written line number in front of the line)

Kusama with this PR:

Uh oh!

Uh oh!

Uh oh!

kianenigma left a comment

Choose a reason for hiding this comment

Uh oh!

jacogr commented Sep 22, 2020

Uh oh!

cheme left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Sep 22, 2020

Uh oh!

ghost commented Sep 22, 2020

Uh oh!

bkchr commented Sep 22, 2020

Uh oh!

ghost commented Sep 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Handle substrate construct_runtime change: break runtime `Event` encoding, change some pallets error index #1692

Handle substrate construct_runtime change: break runtime `Event` encoding, change some pallets error index #1692

gui1117 commented Sep 9, 2020 •

edited

Loading

gui1117 commented Sep 18, 2020 •

edited

Loading