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

uplift to stable2407 #1372

Merged
merged 12 commits into from
Oct 23, 2024
Merged

uplift to stable2407 #1372

merged 12 commits into from
Oct 23, 2024

Conversation

vedhavyas
Copy link
Contributor

@vedhavyas vedhavyas commented Oct 16, 2024

Pull Request Summary
PR uplifts to stable 2407.
Commit one uplifts the project
Commit two bumps the versions

Check list

  • added or updated unit tests
  • updated Astar official documentation
  • added OnRuntimeUpgrade hook for precompile revert code registration
  • added benchmarks & weights for any modified runtime logics.

@vedhavyas vedhavyas added astar Related to Astar runtime This PR/Issue is related to the topic “runtime”. client This PR/Issue is related to the topic “client”. labels Oct 16, 2024
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.

For the treasury changes, we also need to update the docs and inform the Subsquare team to update the UI logic.

bin/collator/Cargo.toml Outdated Show resolved Hide resolved
pallets/xc-asset-config/src/tests.rs Outdated Show resolved Hide resolved
runtime/astar/src/lib.rs Outdated Show resolved Hide resolved
runtime/local/src/lib.rs Outdated Show resolved Hide resolved
runtime/shibuya/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor Author

@vedhavyas vedhavyas left a comment

Choose a reason for hiding this comment

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

New treasury changes requires the origin to return the Max amount to spend at a time and I have set the Max to Max Balance.

As for the Payout timeout, I have set 3 days for shibuya and 30 minutes for Local. After this period, the spends are expired

For the treasury changes, we also need to update the docs and inform the Subsquare team to update the UI logic.

@Dinonard How do I start doing that ?

runtime/shibuya/src/lib.rs Outdated Show resolved Hide resolved
ipapandinas
ipapandinas previously approved these changes Oct 17, 2024
Copy link
Contributor

@ipapandinas ipapandinas left a comment

Choose a reason for hiding this comment

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

LGTM

bin/collator/src/parachain/service.rs Show resolved Hide resolved
Copy link
Contributor

@ipapandinas ipapandinas left a comment

Choose a reason for hiding this comment

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

It's missing xcmp-queue v5 migrations as Ermal noticed

@vedhavyas
Copy link
Contributor Author

It's missing xcmp-queue v5 migrations as Ermal noticed

yes I'min the process of adding those :)

@vedhavyas
Copy link
Contributor Author

@ipapandinas Due to this recent patch to stable2407 paritytech/polkadot-sdk#5913
I had to adjust xcm dry tests. FYI since you seems to have written them

ipapandinas
ipapandinas previously approved these changes Oct 21, 2024
Copy link
Contributor

@ipapandinas ipapandinas left a comment

Choose a reason for hiding this comment

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

nice catch on the XCM dry-run patch! those tests are purely integration tests to ensure that the runtime API is correctly implemented

now that you've reversed the treasury updates, the changes are lighter, so I approve the uplift ✅

Good job! 👏

@vedhavyas
Copy link
Contributor Author

Pallet treasury is forked from https://github.com/AstarNetwork/polkadot-sdk/tree/stable2407 after applying necessary reverts so that we fork the upto date version of pallet

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.

Looks good now!

Just one question about the placement - would it be more appropriate for the vendor folder given we already have there reused crates from other projects?

Copy link

Code Coverage

Package Line Rate Branch Rate Health
pallets/vesting-mbm/src 89% 0%
precompiles/dapp-staking/src/test 0% 0%
chain-extensions/types/unified-accounts/src 0% 0%
pallets/inflation/src 89% 0%
pallets/ethereum-checked/src 74% 0%
precompiles/assets-erc20/src 78% 0%
pallets/xc-asset-config/src 50% 0%
chain-extensions/unified-accounts/src 0% 0%
chain-extensions/types/assets/src 0% 0%
pallets/dapp-staking/rpc/runtime-api/src 0% 0%
pallets/collective-proxy/src 86% 0%
precompiles/sr25519/src 64% 0%
pallets/dapp-staking/src 83% 0%
pallets/static-price-provider/src 85% 0%
precompiles/dispatch-lockdrop/src 86% 0%
pallets/dapp-staking/src/benchmarking 98% 0%
pallets/unified-accounts/src 86% 0%
primitives/src 57% 0%
pallets/price-aggregator/src 82% 0%
precompiles/substrate-ecdsa/src 74% 0%
pallets/dapp-staking/src/test 0% 0%
pallets/dynamic-evm-base-fee/src 89% 0%
pallets/astar-xcm-benchmarks/src/fungible 100% 0%
pallets/astar-xcm-benchmarks/src/generic 100% 0%
chain-extensions/pallet-assets/src 56% 0%
precompiles/xcm/src 71% 0%
precompiles/dapp-staking/src 90% 0%
precompiles/unified-accounts/src 100% 0%
pallets/collator-selection/src 92% 0%
pallets/astar-xcm-benchmarks/src 86% 0%
primitives/src/xcm 65% 0%
Summary 79% (3730 / 4738) 0% (0 / 0)

Minimum allowed line rate is 50%

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.

LGTM!

@Dinonard
Copy link
Member

/runtime-upgrade-test shibuya

Copy link

Runtime upgrade test is scheduled at https://github.com/AstarNetwork/Astar/actions/runs/11478975444.
Please wait for a while.
Runtime: shibuya
Branch: uplift-stable2407
SHA: f2cfc25

@vedhavyas
Copy link
Contributor Author

@Dinonard are we expected to run these tests now? Looks like they failed due to the specVersion check which we decided to rollback in this PR :)

@Dinonard
Copy link
Member

@Dinonard no, it's fine, I triggered the wrong one (wanted to run the migration check).

But the check can also be configured to ignore the spec version check, if needed.

@vedhavyas
Copy link
Contributor Author

is this good to be merged or should we merge after the testing on shinjoku ?

@Dinonard
Copy link
Member

You can merge it, but we need to test on Shinjuku and Shibuya before the actual release.

@vedhavyas
Copy link
Contributor Author

yeah, I have asked for access. Will test there and merge after
Though I believe for try-runtime, we need to update the specVersions

What was process for previous uplifts. What was the versions updated ?

@Dinonard
Copy link
Member

We don't update versions when doing the uplifts, it's done in a separate PR before making the release.

So you can merge this PR, and follow-it up with tests on the Shinjuku & Shibuya.
We can proceed with the release next week.

@vedhavyas vedhavyas merged commit 50ed03d into master Oct 23, 2024
11 checks passed
@vedhavyas vedhavyas deleted the uplift-stable2407 branch October 23, 2024 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astar Related to Astar client This PR/Issue is related to the topic “client”. runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants