feat: upgrade to Polkadot v0.9.29 #416

wischli · 2022-09-21T14:31:07Z

fixes KILTProtocol/ticket#2200

Upgrades to latest Polkadot v0.9.29
Updates weights to v1.5 (a one dimensional struct instead of u64), supersedes feat: add weights v2 #403
Changes execution order of pallets in construct_runtime! macro because our former trait AllPalletsReversedWithSystemFirst is now deprecated. To the best of my knowledge, only the staking related pallets have a dependent order which I reversed to replicate the current behaviour. Thanks to the pallet indices, there should be no side effects.

Note worthy Substrate PRs

Runtime State Test + Integration with try-runtime paritytech/substrate#10174

This removes the --state-cache parameter and adds the new --trie-cache-size cli parameter. Parachain operators should now be able to just drop --state-cache 0 and are not required to add the --trie-cache-size as the cache is enabled by default.

Checklist:

I have verified that the code works
- No panics! (checked arithmetic ops, no indexing array[3] use get(3), ...)
I have verified that the code is easy to understand
- If not, I have left a well-balanced amount of inline comments
I have left the code in a better state
I have documented the changes (where applicable)

…0.9.29

wischli · 2022-09-22T11:45:00Z

runtimes/spiritnet/src/lib.rs

 		AuraExt: cumulus_pallet_aura_ext = 24,
+		Aura: pallet_aura = 23,
+		Session: pallet_session = 22,
+		ParachainStaking: parachain_staking = 21,
+		Authorship: pallet_authorship::{Pallet, Call, Storage} = 20,


Weirdly, when running benchmarks, an error is thrown with this config under AllPalletsWithSystem:
Thread 'main' panicked at 'AuRa authorities empty, maybe wrong order in `construct_runtime!`?', /home/willi/.cargo/git/checkouts/cumulus-59522f43471fa161/35dd14f/pallets/aura-ext/src/lib.rs:96

When reverting the order (:= REV = Authorship -> Staking -> Session -> Aura -> AuraExt), the benchmarks work. That does not make sense though, as we would then run everything in reverse order compared to our current execution. Moreover, there was a user on Discord having problems with REV and AllPalletsWithSystem. The issue there was that session keys were not propagated in --dev environment.

Need to investigate locally...

After intensive testing, I can say that Aura -> Session -> Staking -> Authorship is fine. However, AuraExt needs to be after Aura because the latter sets the authorities which are queried by AuraExt. Should hopefully be fixed now, see 134d428

ntn-x2 · 2022-10-04T08:18:01Z

@wischli maybe we can wait to see if paritytech/substrate#12351 gets backported into 0.9.29, so that we can push the fix with the next runtime upgrade.

ntn-x2 · 2022-10-04T08:36:43Z

@wischli the fix has been backported now. So we should update dependencies and test that the new origin fix works.

….9.29

wischli · 2022-10-05T08:55:29Z

@wischli the fix has been backported now. So we should update dependencies and test that the new origin fix works.

Should be fixed in 68056c6

…0.9.29

ntn-x2

I had few comments that I just noted were pending.

pallets/parachain-staking/src/lib.rs

runtimes/common/src/constants.rs

runtimes/common/src/xcm_config.rs

runtimes/peregrine/src/lib.rs

rpc/did/runtime-api/Cargo.toml

runtimes/spiritnet/src/lib.rs

support/src/relay.rs

ntn-x2 · 2022-10-05T09:03:40Z

runtimes/clone/src/lib.rs

@@ -197,8 +196,8 @@ impl pallet_transaction_payment::Config for Runtime {
 }

 parameter_types! {
-	pub const ReservedXcmpWeight: Weight = constants::MAXIMUM_BLOCK_WEIGHT / 4;
-	pub const ReservedDmpWeight: Weight = constants::MAXIMUM_BLOCK_WEIGHT / 4;
+	pub const ReservedXcmpWeight: Weight = constants::MAXIMUM_BLOCK_WEIGHT.saturating_div(4);


Can we not saturate here but fail since these are constants?

We could but I wanted to stay in sync with Parity, see Cumulus

I've seen few PRs where people have pointed out that compilation time should rather panic than silently do something like saturating. Can't find one easily now though. I personally think it would be better not to saturate.

Overflow doesn't panic tho do they? And these are not "consts" in a sense of rust "consts". This gets expanded to a function that returns constants::MAXIMUM_BLOCK_WEIGHT / 4. So the compiler might optimize that, but he doesn't need to.

The only overflow that can happen while using div is when dividing with -1 anyways and that's not possible since this is a u64. So I actually might also remove the saturating_div.

I suppose the reason for the saturated div here is that the compiler does not accept just dividing, e.g. it throws cannot call non-const operator in constants for constants::MAXIMUM_BLOCK_WEIGHT / 4.

I am unhappy with panicking here. But if you prefer, we can do that.

aha interesting! So it's actually a const function. Didn't look close enough then.

It is a constant hence using a simple / should be enough, as any invalid case would be caught at compilation time (when compiled in release mode, any overflow or division by zero is also caught). A saturating division would, potentially, saturate without giving any notice. I don't see why we would want to replace a simple division with a saturating one. Or am I missing something else?

I also don't see a strong case for the saturating_div here.

Reposting from above:

[... the compiler] throws cannot call non-const operator in constants for constants::MAXIMUM_BLOCK_WEIGHT / 4.

The issue is that division is currently not implemented for Weight, on purpose.

Oh sorry. Yes I totally didn't get that 🙈! Weight is not a u64 any more and they don't implement that for const. But saturating_div

I was blind but now i can see!

weichweich · 2022-10-05T10:20:39Z

runtimes/standalone/src/lib.rs

+	#[cfg(not(feature = "disable-runtime-api"))]
 	apis: RUNTIME_API_VERSIONS,
+	#[cfg(feature = "disable-runtime-api")]
+	apis: sp_version::create_apis_vec![[]],


interesting. Do you know why this was added?

I honestly did not think beyond "yeah maybe we want to be able to disable the runtime API for some testing at some point" since this is part of all relaychain runtimes. However, that has been the case for a long time which I overlooked. So I am perfectly fine with reverting these changes.

@weichweich Should I remove it? Else I would like to merge.

It's standalone so keep it in. We might find a use for it someday. 😂

Its also in the other runtimes 🙈

In that case i wouldn't want something in our spiritnet runtime if we don't have a use case for it. 😰

Removed it for all runtimes in 2560d7a

weichweich

LGTM

The saturating thing should be resolved so that everyone is happy and I would like to know why the disable-runtime-api feature was added to the standalone.

* chore: bump deps * wip: migrate pallets and runtimes * chore: bump more versions * wip: fix clients * fix: runtime tomls * style: ignore clippy in default weights * fix: client pt2 * fix: runtimes, clippy * feat: switch pallet execution order * tests: merge delegation from wf-2168-polkadot-v0.9.28 * chore: bump deps to polkadot-v0.9.29 * fix: weights 1.5 for new stuff * fix: cp mistake * fix: update weight templates * fix: pallet order instruction * chore: bump deps * feat: add backported batch fix * refactor: same serde versions * fix: UnitWeightCost type * fix: CI regex for versions * fix: remove disable-runtime-api feat

The order of the pallets during runtime construction has changed. see KILTprotocol/kilt-node#416

* first iteration: fixes the development-runtime package. * fixed some errors in foucoco runtime. Not yet finished. * new iteration, fixing `foucoco-runtime` * set network to None * fix foucoco runtime and update spacewalk tag * bring back `FungiblesAdapter` * fix amplitude-runtime * fix pendulum-runtime * fix node * cleanup some test build errors * add .lock * fix test build issues * fix test build issues of parachain-staking. The order of the pallets during runtime construction has changed. see KILTprotocol/kilt-node#416 * #242 (comment), #242 (comment), #242 (comment), #242 (comment) * partially fixed test integration test cases * fixed test integration test cases * remove extern crate core * revert comment println * tried to fix the benchmark, but to no avail. * fix benchmark and added weights * add cargo lock * #242 (comment), #242 (comment), * change spec versions * revert version

wischli added 13 commits September 6, 2022 15:00

chore: bump deps

34ca624

wip: migrate pallets and runtimes

7160779

chore: bump more versions

be04423

wip: fix clients

a24e19a

fix: runtime tomls

2f04b88

style: ignore clippy in default weights

a18bca5

fix: client pt2

9c48ae3

fix: runtimes, clippy

89ba1e5

feat: switch pallet execution order

6d6aad3

tests: merge delegation from wf-2168-polkadot-v0.9.28

a4cb4ed

Merge remote-tracking branch 'origin/develop' into wf-2020-polkadot-v…

1ab83db

…0.9.29

chore: bump deps to polkadot-v0.9.29

d542a18

fix: weights 1.5 for new stuff

9df8e97

wischli self-assigned this Sep 21, 2022

wischli added 3 commits September 21, 2022 16:32

Merge branch 'develop' into wf-2020-polkadot-v0.9.29

36649f6

fix: cp mistake

d45b845

fix: update weight templates

f49dd24

wischli commented Sep 22, 2022

View reviewed changes

wischli added 2 commits September 22, 2022 17:22

fix: pallet order instruction

134d428

chore: bump deps

b5e01bd

wischli requested review from weichweich and ntn-x2 September 23, 2022 09:18

feat: add backported batch fix

68056c6

wischli added the next release This PR is required for the next release. label Oct 5, 2022

wischli added this to the 1.7.5 milestone Oct 5, 2022

wischli changed the base branch from develop to master October 5, 2022 08:54

Merge remote-tracking branch 'origin/master' into wf-2020-polkadot-v0…

be7c214

….9.29

wischli changed the base branch from master to develop October 5, 2022 08:58

Merge remote-tracking branch 'origin/develop' into wf-2020-polkadot-v…

80bba47

…0.9.29

ntn-x2 reviewed Oct 5, 2022

View reviewed changes

refactor: same serde versions

909317a

weichweich reviewed Oct 5, 2022

View reviewed changes

weichweich approved these changes Oct 5, 2022

View reviewed changes

wischli added 3 commits October 5, 2022 13:08

fix: UnitWeightCost type

6aa4b19

fix: CI regex for versions

061b1ab

fix: remove disable-runtime-api feat

2560d7a

wischli enabled auto-merge (squash) October 6, 2022 08:19

wischli merged commit 93fb3ab into develop Oct 6, 2022

wischli deleted the wf-2020-polkadot-v0.9.29 branch October 6, 2022 08:29

wischli mentioned this pull request Oct 7, 2022

feat: add weights v2 #403

Closed

6 tasks

wischli mentioned this pull request Oct 18, 2022

feat: release v1.7.5 #424

Merged

5 tasks

b-yap added a commit to pendulum-chain/pendulum that referenced this pull request Jun 12, 2023

fix test build issues of parachain-staking.

2a0227b

The order of the pallets during runtime construction has changed. see KILTprotocol/kilt-node#416

b-yap mentioned this pull request Jun 12, 2023

upgrade to version polkadot-v0.9.40 pendulum-chain/pendulum#242

Merged

5 tasks

b-yap added a commit to pendulum-chain/pendulum that referenced this pull request Jun 12, 2023

fix test build issues of parachain-staking.

19dceb0

The order of the pallets during runtime construction has changed. see KILTprotocol/kilt-node#416

b-yap added a commit to pendulum-chain/pendulum that referenced this pull request Jul 5, 2023

fix test build issues of parachain-staking.

8e8d1d6

The order of the pallets during runtime construction has changed. see KILTprotocol/kilt-node#416

b-yap added a commit to pendulum-chain/pendulum that referenced this pull request Jul 6, 2023

fix test build issues of parachain-staking.

7e3aa86

The order of the pallets during runtime construction has changed. see KILTprotocol/kilt-node#416

b-yap added a commit to pendulum-chain/pendulum that referenced this pull request Jul 6, 2023

fix test build issues of parachain-staking.

39761da

The order of the pallets during runtime construction has changed. see KILTprotocol/kilt-node#416

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: upgrade to Polkadot v0.9.29 #416

feat: upgrade to Polkadot v0.9.29 #416

wischli commented Sep 21, 2022 •

edited

Loading

wischli Sep 22, 2022

wischli Sep 23, 2022

ntn-x2 commented Oct 4, 2022

ntn-x2 commented Oct 4, 2022

wischli commented Oct 5, 2022

ntn-x2 left a comment

ntn-x2 Oct 5, 2022

wischli Oct 5, 2022

ntn-x2 Oct 5, 2022

weichweich Oct 5, 2022

wischli Oct 5, 2022

weichweich Oct 5, 2022

ntn-x2 Oct 5, 2022

weichweich Oct 5, 2022

wischli Oct 5, 2022 •

edited

Loading

weichweich Oct 5, 2022

weichweich Oct 5, 2022

wischli Oct 5, 2022

wischli Oct 6, 2022

weichweich Oct 6, 2022

wischli Oct 6, 2022

weichweich Oct 6, 2022 •

edited

Loading

wischli Oct 6, 2022

weichweich left a comment

feat: upgrade to Polkadot v0.9.29 #416

feat: upgrade to Polkadot v0.9.29 #416

Conversation

wischli commented Sep 21, 2022 • edited Loading

fixes KILTProtocol/ticket#2200

Note worthy Substrate PRs

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntn-x2 commented Oct 4, 2022

ntn-x2 commented Oct 4, 2022

wischli commented Oct 5, 2022

ntn-x2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wischli Oct 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weichweich Oct 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weichweich left a comment

Choose a reason for hiding this comment

wischli commented Sep 21, 2022 •

edited

Loading

wischli Oct 5, 2022 •

edited

Loading

weichweich Oct 6, 2022 •

edited

Loading