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 polkadot-sdk to stable2407 #648

Merged
merged 13 commits into from
Aug 20, 2024
Merged

Update polkadot-sdk to stable2407 #648

merged 13 commits into from
Aug 20, 2024

Conversation

tmpolaczyk
Copy link
Contributor

@tmpolaczyk tmpolaczyk commented Aug 6, 2024

Main changes:

NativeElseWasmExecutor has been deprecated, but we will keep using it for now. Removing it implies not being able to debug runtimes using breakpoints anymore.

cumulus-pallet-dmp-queue has been deprecated, it is safe to remove so we removed it. No need to kill pallet storage because it is automatically done in on_idle when migration completes.

pallet_treasury: proposeSpend and approveSpend have been removed. Instead of ApproveOrigin = Root and SpendOrigin = Never, we now have SpendOrigin = Root. In tests, users can no longer propose treasury spends, only root can spend. So I removed most of the treasury tests and added some new ones, please review that (test/suites/common-tanssi/pallet-treasury/test_pallet_treasury.ts). Info about the removal: paritytech/polkadot-sdk#138

Changed branch format in polkadot-sdk. Instead of "release-polkadot-v1.11.0", the release branch is now just called "stable2407". Our fork branch is called "tanssi-polkadot-stable2407" instead of "tanssi-polkadot-v1.11.0". I had to fix the download-polkadot.sh script, maybe other tools will break. More info:

https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-stable2407
https://github.com/paritytech/polkadot-sdk/blob/master/docs/RELEASE.md

@tmpolaczyk tmpolaczyk added breaking Needs to be mentioned in breaking changes B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Aug 7, 2024
Copy link
Contributor

github-actions bot commented Aug 7, 2024

Coverage Report

(master)

@@                  Coverage Diff                   @@
##           master   upgrade-stable2407      +/-   ##
======================================================
+ Coverage   66.19%               66.49%   +0.30%     
- Files         264                  261       -3     
- Lines       45952                45854      -98     
======================================================
+ Hits        30414                30490      +76     
- Misses      15538                15364     -174     
Files Changed Coverage
/client/node-common/src/service.rs 48.31% (+0.07%) 🔼
/container-chains/nodes/frontier/src/service.rs 79.57% (+0.07%) 🔼
/container-chains/nodes/simple/src/service.rs 86.52% (+0.09%) 🔼
/container-chains/runtime-templates/frontier/build.rs 100.00% (+100.00%) 🔼
/container-chains/runtime-templates/frontier/src/lib.rs 58.56% (-0.07%) 🔽
/container-chains/runtime-templates/simple/build.rs 100.00% (+100.00%) 🔼
/container-chains/runtime-templates/simple/src/lib.rs 87.23% (+15.85%) 🔼
/node/src/service.rs 23.62% (+0.15%) 🔼
/runtime/dancebox/build.rs 100.00% (+100.00%) 🔼
/runtime/dancebox/src/lib.rs 88.74% (-0.07%) 🔽
/runtime/dancebox/src/weights/pallet_balances.rs 18.18% (-3.51%) 🔽
/runtime/dancebox/src/weights/pallet_treasury.rs 26.47% (-3.43%) 🔽
/runtime/flashbox/build.rs 100.00% (+100.00%) 🔼
/solo-chains/runtime/starlight/build.rs 100.00% (+100.00%) 🔼
/solo-chains/runtime/starlight/src/lib.rs 24.67% (+0.04%) 🔼

Coverage generated Tue Aug 20 14:26:26 UTC 2024

@tmpolaczyk tmpolaczyk marked this pull request as ready for review August 7, 2024 13:10
@Agusrodri
Copy link
Contributor

Agusrodri commented Aug 16, 2024

cumulus-pallet-dmp-queue has been deprecated, it is safe to remove so we removed it. TODO: need migration to kill storage?

We don't need an extra migration to kill storage of the dmp-queue pallet as the lazy migration present inside on_idle has been already completed.

If you check the chain state of the different networks (Dancebox, Flashbox), you will notice that the MigrationStatus storage returns Completed, and in that case the pallet can be safely removed.

Agusrodri
Agusrodri previously approved these changes Aug 16, 2024
Copy link
Contributor

@Agusrodri Agusrodri left a comment

Choose a reason for hiding this comment

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

The overall changes (specially the treasury ones and related TS tests) look good to me.

@tmpolaczyk
Copy link
Contributor Author

We don't need an extra migration to kill storage of the dmp-queue pallet as the lazy migration present inside on_idle has been already completed.

Ok that's very smart, thanks for checking @Agusrodri

Copy link
Collaborator

@girazoki girazoki left a comment

Choose a reason for hiding this comment

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

LGTM!

@tmpolaczyk tmpolaczyk merged commit 0153f6c into master Aug 20, 2024
37 checks passed
@tmpolaczyk tmpolaczyk deleted the upgrade-stable2407 branch August 20, 2024 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants