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

Update to Polkadot SDK 1.6.0 #510

Open
wants to merge 40 commits into
base: main
Choose a base branch
from

Conversation

ebma
Copy link
Member

@ebma ebma commented Nov 27, 2024

TODO

  • Update the declared dependency versions (mostly done). cargo check does not work yet.
  • Add code changes to adjust to breaking changes
  • (Optional) Group everything in a global workspace file. I wanted to do this last so we can first focus on making everything work in general before dealing with the errors that potentially arise only due to mistakes with the migration to the global workspace.
  • Run try-runtime CLI tests, to verify the storage version updates.
  • Verify new configuration parameters chosen for pallets, necessary after the upgrade.
  • Replace the forked repositories (bifrost, zenlink) that avoid using crates.io with forks from pendulum-chain.

Important Note

Package runtime-integration-tests will not be updated on this PR, as it requires further dependency refactor that potentially consist of moving all polkadot dependencies to it's crates-io definition (mostly due to the removal of asset-hub runtimes from polkadot-sdk).

Closes #505.

@ebma ebma linked an issue Nov 27, 2024 that may be closed by this pull request
@ebma ebma changed the title [WIP505 update to polkadot sdk 160 pendulum [WIP] Update to Polkadot SDK 1.6.0 Nov 27, 2024
@ebma ebma marked this pull request as draft November 27, 2024 12:45
@gianfra-t gianfra-t marked this pull request as ready for review December 11, 2024 16:56
@gianfra-t gianfra-t requested a review from a team December 11, 2024 16:56
@@ -468,8 +475,9 @@ impl pallet_balances::Config for Runtime {
type ReserveIdentifier = ReserveIdentifier;
type FreezeIdentifier = ();
type MaxFreezes = ();
type MaxHolds = ConstU32<1>;
type MaxHolds = ConstU32<2>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Otherwise, we get an error MaxHolds should be greater than or equal to the number of hold reasons: 1 < 2, which is an internal check of the pallet.
Apparently the RuntimeHoldReason has 2 variants.

@@ -719,6 +745,7 @@ parameter_types! {
pub const Burn: Permill = Permill::from_percent(0);
pub const TreasuryPalletId: PalletId = PalletId(*b"py/trsry");
pub const MaxApprovals: u32 = 100;
pub const PayoutSpendPeriod: BlockNumber = 30 * DAYS;
Copy link
Contributor

Choose a reason for hiding this comment

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

I selected this arbitrarily. We might want to change.

@@ -1005,6 +1042,8 @@ parameter_types! {
pub const MaxSubAccounts: u32 = 100;
pub const MaxAdditionalFields: u32 = 100;
pub const MaxRegistrars: u32 = 20;
pub const ByteDeposit: Balance = MILLIUNIT;
Copy link
Contributor

Choose a reason for hiding this comment

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

Also arbitrary and we may tweak to our business req.

}

impl cumulus_pallet_dmp_queue::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
type XcmExecutor = XcmExecutor<XcmConfig>;
type ExecuteOverweightOrigin = EnsureRoot<AccountId>;
type DmpSink = frame_support::traits::EnqueueWithOrigin<MessageQueue, RelayOrigin>;
Copy link
Contributor

@gianfra-t gianfra-t Dec 11, 2024

Choose a reason for hiding this comment

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

The pallet is deprecated, but we keep it to until it drains it's messages (to the replacement), assuming there are some after the update.

More info on the original PR.

}

use parachains_common::message_queue::NarrowOriginToSibling;
impl pallet_message_queue::Config for Runtime {
Copy link
Contributor

Choose a reason for hiding this comment

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

This general queue pallet can work as dmp, xcmp, etc. We use it now as a replacement for the deprecated dmp_queue pallet.

@gianfra-t gianfra-t changed the title [WIP] Update to Polkadot SDK 1.6.0 Update to Polkadot SDK 1.6.0 Dec 12, 2024
@ebma
Copy link
Member Author

ebma commented Dec 20, 2024

@gianfra-t can you please also configure and perform a benchmark for the parachain-staking pallet? I saw just now that we are still using the default weights that were measured on a different parachain, see here. Instead we should measure different weights specific to each of our runtimes as some of the weights take into account constants which can differ for each of them.

@@ -24,7 +24,7 @@
// --repeat
// 5
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we should adhere to the 'best practices' used in the docs for benchmarking and have it repeat not 5 but 20 times.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I changed it because it takes ages with 20 times. It already takes like 30 minutes with 5. And then I remembered that was the reason we disabled it from the script to run all benchmarks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah okay 😅 fair enough.

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.

Update to Polkadot-sdk 1.6.0 - Pendulum
2 participants