Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade polkadot v0.9.16 #567

Merged
merged 3 commits into from
Feb 14, 2022
Merged

Upgrade polkadot v0.9.16 #567

merged 3 commits into from
Feb 14, 2022

Conversation

akru
Copy link
Member

@akru akru commented Feb 14, 2022

Pull Request Summary

Describe what changes this pull request makes to the repository

Check list

  • contains breaking changes
  • adds new feature
  • modifies existing feature (bug fix or improvements)
  • updated spec version
  • relies on other tasks
  • documentation changes
  • tests and/or benchmarks are included
  • changed API client type definition or chain metadata

This pull request makes the following changes:

Adds

  • (ex: Add feature A)

Fixes

  • (ex: Fix validation function)

Changes

  • (ex: Change document B)

To-dos

*Feel free to remove this section if it's not applicable

  • (ex: add user list)

@akru akru requested a review from Dinonard February 14, 2022 12:38
Copy link
Member

@Dinonard Dinonard left a comment

Choose a reason for hiding this comment

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

I can find time today to create the tags in astar-frame if needed. Let's avoid using the branch directly.

pallet-custom-signatures = { git = "https://github.com/AstarNetwork/astar-frame", tag = "pallet-custom-signatures-4.3.0/polkadot-0.9.13", default-features = false }
pallet-dapps-staking = { git = "https://github.com/AstarNetwork/astar-frame", tag = "pallet-dapps-staking-1.1.2/polkadot-0.9.13", default-features = false }
pallet-vesting = { git = "https://github.com/AstarNetwork/astar-frame", tag = "pallet-vesting-4.0.0-dev/polkadot-0.9.13", default-features = false }
pallet-block-reward = { git = "https://github.com/AstarNetwork/astar-frame", branch = "polkadot-v0.9.16", default-features = false }
Copy link
Member

Choose a reason for hiding this comment

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

I really think we shouldn't do this and should go with tags instead.
This will become the new default branch and we have fairly often deliveries there.

Copy link
Member Author

Choose a reason for hiding this comment

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

As I can see now, tags isn't so convenient as branches, you can't fork tag easily to provide new polkadot adjusted version.

Other point is to have default branch, if some should be fixed let's use dedicated branches. In situation with tags the new polkadot release is hell.

Copy link
Member

Choose a reason for hiding this comment

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

We don't need to fork each tag separately - only what's needed.
The majority of pallets we use just take from the default - only Astar dapps-staking is special.
So when doing a polkadot upgrade, we could just:

  • create a new branch for the new polkadot version and make this the new default branch
  • create just a single new tag polkadot-a.b.c for this latest version
    • once we update a pallet, we can create the new tag as previously agreed
  • update runtimes to use this tag UNLESS they require a pallet version that differs from the latest default branch

It's not just about fixes though - the tags are useful for situations like we'll have when I merge individual claim.
Initially only Shibuya will use it - then Shiden - then Astar. I can work freely on the default branch in astar-frame as long as I know that Shiden and Astar don't depend on any branch and they use dapps-staking-x.y.z./polkadot-a.b.c. instead.

@@ -2,14 +2,14 @@
//! Autogenerated weights for `pallet_dapps_staking`
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
//! DATE: 2021-10-18, STEPS: `20`, REPEAT: 10, LOW RANGE: `[]`, HIGH RANGE: `[]`
Copy link
Member

Choose a reason for hiding this comment

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

Not really important but it would be more correct to use the ones from pallet - those were measured on dev chain.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's update it after, please write TODO somewhere.

@akru
Copy link
Member Author

akru commented Feb 14, 2022

@Dinonard let's discuss tag/branch question on dedicated meeting, may be tomorrow. We can fix in any patch release.

pub struct WeightInfo<T>(PhantomData<T>);
impl<T: frame_system::Config> pallet_dapps_staking::WeightInfo for WeightInfo<T> {
// Storage: DappsStaking RegisteredDevelopers (r:1 w:1)
// Storage: DappsStaking RegisteredDapps (r:1 w:1)
// Storage: DappsStaking PreApprovalIsEnabled (r:1 w:0)
fn register() -> Weight {
(63_974_000 as Weight)
(39_675_000 as Weight)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why the block weights changed on the local? I can see the weights are the same as

and this was also executed with the shibuya-dev flag, but just curious why the auto-gen file was assigned to the local runtime 🤔 https://github.com/AstarNetwork/Astar/pull/567/files#diff-3949041c500f115558c821562657ee76c628c7d0c3d8e2a7a82ea34e63eff8c8R12

Copy link
Contributor

@hoonsubin hoonsubin left a comment

Choose a reason for hiding this comment

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

Small questions, but I think it LGTM

@akru akru merged commit d38011e into master Feb 14, 2022
@akru akru deleted the feature/polkadot-v0.9.16 branch February 14, 2022 14:08
@shunsukew shunsukew mentioned this pull request Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants