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

chore(upgrade): polkadot v1.10 to v1.2.0 #1982

Merged
merged 1 commit into from
May 29, 2024

Conversation

enddynayn
Copy link
Collaborator

@enddynayn enddynayn commented May 22, 2024

Goal

Update polkadot from v1.1.0 to v1.2.0.

Polkadot release notes

Lazy Migration
paritytech/polkadot-sdk#1363

Closes #1718

@enddynayn enddynayn requested a review from wilwade as a code owner May 22, 2024 22:10
@enddynayn enddynayn marked this pull request as draft May 22, 2024 22:11
@enddynayn enddynayn force-pushed the chore/upgrade-polkadot-v1.1.0-to-polkadot-v1.2.0 branch from 0353c31 to 3fa64ba Compare May 22, 2024 22:58
@github-actions github-actions bot added metadata-changed Metadata has changed since the latest full release metadata-version-not-incremented Metadata has changed since the latest full release, but the version has not been incremented and removed metadata-changed Metadata has changed since the latest full release metadata-version-not-incremented Metadata has changed since the latest full release, but the version has not been incremented labels May 22, 2024
Copy link

codecov bot commented May 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

@github-actions github-actions bot added metadata-changed Metadata has changed since the latest full release metadata-version-not-incremented Metadata has changed since the latest full release, but the version has not been incremented labels May 23, 2024
@enddynayn enddynayn force-pushed the chore/upgrade-polkadot-v1.1.0-to-polkadot-v1.2.0 branch from c998734 to f4d0615 Compare May 23, 2024 17:43
@github-actions github-actions bot removed metadata-changed Metadata has changed since the latest full release metadata-version-not-incremented Metadata has changed since the latest full release, but the version has not been incremented labels May 23, 2024
@enddynayn enddynayn force-pushed the chore/upgrade-polkadot-v1.1.0-to-polkadot-v1.2.0 branch from f4d0615 to 1cbb9de Compare May 23, 2024 17:55
@github-actions github-actions bot added metadata-changed Metadata has changed since the latest full release metadata-version-not-incremented Metadata has changed since the latest full release, but the version has not been incremented labels May 23, 2024
@enddynayn enddynayn force-pushed the chore/upgrade-polkadot-v1.1.0-to-polkadot-v1.2.0 branch from 1cbb9de to 669d90b Compare May 23, 2024 21:01
@github-actions github-actions bot removed metadata-changed Metadata has changed since the latest full release metadata-version-not-incremented Metadata has changed since the latest full release, but the version has not been incremented labels May 23, 2024
@enddynayn enddynayn force-pushed the chore/upgrade-polkadot-v1.1.0-to-polkadot-v1.2.0 branch 3 times, most recently from 4208fe8 to 5c119a3 Compare May 23, 2024 21:37
@github-actions github-actions bot added the metadata-changed Metadata has changed since the latest full release label May 23, 2024
@enddynayn enddynayn force-pushed the chore/upgrade-polkadot-v1.1.0-to-polkadot-v1.2.0 branch from c686fce to 7de683c Compare May 23, 2024 23:51
@github-actions github-actions bot removed the metadata-changed Metadata has changed since the latest full release label May 23, 2024
@enddynayn enddynayn force-pushed the chore/upgrade-polkadot-v1.1.0-to-polkadot-v1.2.0 branch from 7de683c to 9663fa3 Compare May 23, 2024 23:53
@enddynayn enddynayn marked this pull request as ready for review May 23, 2024 23:55
@github-actions github-actions bot added the metadata-changed Metadata has changed since the latest full release label May 24, 2024
@@ -646,7 +649,7 @@ impl pallet_balances::Config for Runtime {
type WeightInfo = weights::pallet_balances::SubstrateWeight<Runtime>;
type MaxReserves = BalancesMaxReserves;
type ReserveIdentifier = [u8; 8];
type MaxHolds = ConstU32<0>;
type MaxHolds = ConstU32<1>;
Copy link
Collaborator

@wilwade wilwade May 24, 2024

Choose a reason for hiding this comment

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

Curious why this change. I thought we didn't want to allow holds as they are going away

Although perhaps it is needed to not break the preimage pallet while upgrading?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great question. I also had to review this part. The introduction of MaxHolds is part of the process of deprecating the Currency trait. To clarify, a hold is similar to the deprecated reserve idea, reducing the free balance in your account. Previously, holds were set to zero because no pallet was utilizing them. However, with the Preimage pallet now using holds, as a result, we increased the MaxHold by one. You can see how holds are utilized in the new associated type called Consideration:

impl pallet_preimage::Config for Runtime {
         ...
	type Consideration = HoldConsideration<
		AccountId,
		Balances,
		PreimageHoldReason,
		LinearStoragePrice<PreimageBaseDeposit, PreimageByteDeposit, Balance>,
	>;
}

This type incorporates the balances pallet, which implements the MutateHold trait. Within the Preimage pallet, the method T::Consideration::new(...) is called, invoking the balances pallet to create a hold.

Copy link
Collaborator

@wilwade wilwade left a comment

Choose a reason for hiding this comment

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

One question about holds, but otherwise looks like a nice simple upgrade.

  • Read through the changes

@enddynayn enddynayn force-pushed the chore/upgrade-polkadot-v1.1.0-to-polkadot-v1.2.0 branch from 0a96653 to 074b894 Compare May 28, 2024 16:33
@github-actions github-actions bot added metadata-changed Metadata has changed since the latest full release and removed metadata-changed Metadata has changed since the latest full release labels May 28, 2024
Upgrade Polkadot v1.1.0 to v1.2.0.
[Polkadot release notes](https://forum.polkadot.network/t/polkadot-release-analysis-v1-2-0/4451)

Lazy Migration
paritytech/polkadot-sdk#1363

Update weights.

issue-1718
@enddynayn enddynayn force-pushed the chore/upgrade-polkadot-v1.1.0-to-polkadot-v1.2.0 branch from 074b894 to 7b2f70c Compare May 28, 2024 20:42
@github-actions github-actions bot added metadata-changed Metadata has changed since the latest full release and removed metadata-changed Metadata has changed since the latest full release labels May 28, 2024
Comment on lines -45 to +48
// Minimum execution time: 13_175_000 picoseconds.
Weight::from_parts(11_629_620, 2809)
// Standard Error: 9_686
.saturating_add(Weight::from_parts(3_219_955, 0).saturating_mul(b.into()))
// Minimum execution time: 13_026_000 picoseconds.
Weight::from_parts(11_528_896, 2809)
// Standard Error: 11_636
.saturating_add(Weight::from_parts(3_232_614, 0).saturating_mul(b.into()))
Copy link
Collaborator

@claireolmstead claireolmstead May 28, 2024

Choose a reason for hiding this comment

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

How do you know what numbers to change all of these values to? They all seem somewhat arbitrary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These numbers are automatically generated when we run benchmarks against the extrinsics we create. For example, this benchmark tests the performance of the messages pallet. When the benchmarks are executed, they run the specified code and provide measurements of execution time and resource usage. These metrics help us understand the computational cost of transactions and determine appropriate fees.

Copy link
Collaborator

@shannonwells shannonwells left a comment

Choose a reason for hiding this comment

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

  • I reviewed Cargo.lock to check that the update SHA matches the Polkadot release SHA
  • I checked that new weights were not significantly change from before - around 2 microseconds except where noted.
  • I reviewed other code changes - mostly the switch to transferAllowDeath and the note about MaxHolds.

.saturating_add(Weight::from_parts(2_305, 0).saturating_mul(s.into()))
.saturating_add(T::DbWeight::get().reads(1_u64))
.saturating_add(T::DbWeight::get().writes(2_u64))
// Minimum execution time: 45_175_000 picoseconds.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Noting that preimage pallet weights are a lot higher (10+ microseconds some of them) due to extra reads/writes

Copy link
Collaborator

@mattheworris mattheworris left a comment

Choose a reason for hiding this comment

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

  • Read through the changes
  • Read through the release notes and release analysis
  • Executed e2e tests (All passed)

Suggestion: Consider adding https://github.com/ggwpez/substrate-scripts/blob/master/migrate-preimage-lazy.py to our scripts, I have found this helpful for running the scripts in the past.

@enddynayn enddynayn enabled auto-merge (squash) May 29, 2024 00:05
Copy link
Collaborator

@wilwade wilwade left a comment

Choose a reason for hiding this comment

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

Ship it!

  • Reviewed changes
  • Ran locally
  • Ran e2e tests locally

@enddynayn enddynayn merged commit 81f29ea into main May 29, 2024
28 checks passed
@enddynayn enddynayn deleted the chore/upgrade-polkadot-v1.1.0-to-polkadot-v1.2.0 branch May 29, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
metadata-changed Metadata has changed since the latest full release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to Polkadot Release polkadot-v1.2.0
5 participants