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

Co #5064: Use runtime dependant weights #1076

Merged
merged 4 commits into from
Mar 11, 2022
Merged

Co #5064: Use runtime dependant weights #1076

merged 4 commits into from
Mar 11, 2022

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Mar 9, 2022

Companion for paritytech/polkadot#5064

  • Use the weights from polkadot-runtime instead of polkadot-runtime-common since the weights are now not common anymore but depend on the runtime.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez added A3-inprogress B0-silent Changes should not be mentioned in any release notes labels Mar 9, 2022
@ggwpez
Copy link
Member Author

ggwpez commented Mar 10, 2022

bot rebase

@paritytech-processbot
Copy link

Rebasing

parity-processbot and others added 2 commits March 10, 2022 12:51
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot
Copy link

Merge cancelled due to error. Error: Checks failed for 9f9350d

@ggwpez
Copy link
Member Author

ggwpez commented Mar 11, 2022

bot rebase

@paritytech-processbot
Copy link

Rebasing

@gavofyork gavofyork merged commit 984d9fa into master Mar 11, 2022
@gavofyork gavofyork deleted the oty-move-weights branch March 11, 2022 17:26
@ggwpez ggwpez mentioned this pull request Mar 11, 2022
@@ -58,7 +58,7 @@ kusama-runtime-constants = { git = "https://github.com/paritytech/polkadot", def
pallet-xcm = { git = "https://github.com/paritytech/polkadot", default-features = false, branch = "master" }
polkadot-core-primitives = { git = "https://github.com/paritytech/polkadot", default-features = false, branch = "master" }
polkadot-parachain = { git = "https://github.com/paritytech/polkadot", default-features = false, branch = "master" }
polkadot-runtime-common = { git = "https://github.com/paritytech/polkadot", default-features = false, branch = "master" }
polkadot-runtime = { git = "https://github.com/paritytech/polkadot", default-features = false, branch = "master" }
Copy link
Member

Choose a reason for hiding this comment

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

Wait wait. Sorry that I'm late to the party. But, this is canvas-kusama. Why did you even removed the runtime-common?

Because the RocksDbWeight is now different per runtime?

It is really bad that include here the entire runtime into another runtime. Especially as RocksDbWeight should be defined here in each runtime and should not be taken from Polkadot.

Copy link
Member Author

Choose a reason for hiding this comment

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

The polkadot-runtime-common does not contain the RocksDbWeight now anymore, I did not think that it is a bad idea to include one runtime into another.

So I should rather also copy the weights into all the parachain configs as well? Makes much more sense when I write it out like this 🤦

Copy link
Member

Choose a reason for hiding this comment

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

Good :D

Copy link
Member

Choose a reason for hiding this comment

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

I mean you now automatically generate this weight? So, this weight could also be generated for the parachains?

Copy link
Member Author

Choose a reason for hiding this comment

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

Theoretically it should work. I just never tried to run the collators myself.
Also not sure if their CLI uses the command yet.

So I fixed the parachain weights in paritytech/polkadot#5091 and #1081, but I'm a bit tired now, so hope this all makes sense.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that looks like a good start. However, every substrate based chain should generate these weights on their own. The same applies to all the other parachains. So, you should make sure that they also can generate these weights. It makes no sense to only create them for polkadot.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes sure that was the long term plan. For now this just fixes the dependency on the Polkadot runtime.
In the future the Storage and Block/Extrinsic weights should all be generated by the new bench bot.
Can we merge the mentioned MRs or do you need something else for that? I was under the impression that this should be fixes ASAP.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, yeah I will review them now :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
B0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants