Skip to content

Authorize upgrade tests for testnet runtimes + execute_as_governance refactor#7656

Merged
bkontur merged 9 commits intomasterfrom
bko-authorize-upgrade-tests
Feb 26, 2025
Merged

Authorize upgrade tests for testnet runtimes + execute_as_governance refactor#7656
bkontur merged 9 commits intomasterfrom
bko-authorize-upgrade-tests

Conversation

@bkontur
Copy link
Copy Markdown
Contributor

@bkontur bkontur commented Feb 21, 2025

Relates to: #7541
Relates to: #7566

This PR contains improved test cases that rely on the governance location as preparation for AHM to capture the state as it is.
It introduces execute_as_governance_call, which can be configured with various governance location setups instead of the hard-coded Location::parent().

Additionally, it adds a test for authorize_upgrade to all SP testnets.

TODO

  • rewrite all tests using RuntimeHelper::<Runtime>::execute_as_governance (because it is using hard-coded Location::parent()) to use RuntimeHelper::<Runtime>::execute_as_governance_call

Follow-up

  • add similar test for westend-runtime
  • add test that ensure xcm-executor adds ClearOrigin before all side-effect sent to different chain

@bkontur bkontur added T10-tests This PR/Issue is related to tests. T14-system_parachains This PR/Issue is related to system parachains. labels Feb 21, 2025
@bkontur bkontur self-assigned this Feb 21, 2025
@bkontur
Copy link
Copy Markdown
Contributor Author

bkontur commented Feb 26, 2025

/cmd prdoc --audience runtime_dev --bump patch

@paritytech-workflow-stopper
Copy link
Copy Markdown

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/13539961010
Failed job name: fmt

@bkontur
Copy link
Copy Markdown
Contributor Author

bkontur commented Feb 26, 2025

/cmd fmt

@bkontur bkontur changed the title Authorize upgrade tests for testnet runtimes Authorize upgrade tests for testnet runtimes + execute_as_governance refactor Feb 26, 2025
@bkontur
Copy link
Copy Markdown
Contributor Author

bkontur commented Feb 26, 2025

/cmd prdoc --audience runtime_dev --bump patch

@github-actions
Copy link
Copy Markdown
Contributor

Command "prdoc --audience runtime_dev --bump patch" has failed ❌! See logs here

@bkontur
Copy link
Copy Markdown
Contributor Author

bkontur commented Feb 26, 2025

/cmd prdoc --audience runtime_dev --bump patch --force

@bkontur bkontur enabled auto-merge February 26, 2025 11:02
@bkontur bkontur added this pull request to the merge queue Feb 26, 2025
parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::<
Runtime,
RuntimeOrigin,
>(GovernanceOrigin::Location(Location::new(1, Parachain(ASSET_HUB_ID)))),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just minor comment: maybe it would be good idea to reuse existing config parameter AssetHubLocation, so for example GovernanceOrigin::Location(AssetHubLocation::get())

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@karolk91 hmm, good point, I didn't realize this, I just copy this test to all runtimes, I didn't realize that AssetHubLocation exists for some runtimes. The idea here is that now assert_err for AssetHub now, but within AHM, this assert_err should fail and should be changed to assert_ok, because the governance will be moved also to the AH.

I would suggest to replace Location::new(1, Parachain(ASSET_HUB_ID) with AssetHubLocation::get() later when we will come to this:
#7626
#7623
#7566


/// Enum representing governance origin/location.
#[derive(Clone)]
pub enum GovernanceOrigin<RuntimeOrigin> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please ignore if it doesn't make sense, asking more for a learning purpose - I was wondering if RuntimeOrigin isn't already a type that encompasses various origins like Xcm, System/signed ? And if you could just use RuntimeOrigin instead of wrapping different possibilites into enum

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

RuntimeOrigin here is just a generic type; it could also be simply RO or O or OriginWhateverName.

The actual RuntimeOrigin is provided at the runtime level, e.g., coretime_westend_runtime::RuntimeOrigin. This real type is constructed by the construct_runtime! macro from all the configured origins (as you said) defined in the runtime.

Merged via the queue into master with commit e9be92d Feb 26, 2025
264 of 271 checks passed
@bkontur bkontur deleted the bko-authorize-upgrade-tests branch February 26, 2025 12:37
@paritytech-cmd-bot-polkadot-sdk
Copy link
Copy Markdown
Contributor

Created backport PR for stable2407:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-7656-to-stable2407
git worktree add --checkout .worktree/backport-7656-to-stable2407 backport-7656-to-stable2407
cd .worktree/backport-7656-to-stable2407
git reset --hard HEAD^
git cherry-pick -x e9be92d66f6b61c5cfbd2b56456e2205b478b04c
git push --force-with-lease

@paritytech-cmd-bot-polkadot-sdk
Copy link
Copy Markdown
Contributor

Created backport PR for stable2409:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-7656-to-stable2409
git worktree add --checkout .worktree/backport-7656-to-stable2409 backport-7656-to-stable2409
cd .worktree/backport-7656-to-stable2409
git reset --hard HEAD^
git cherry-pick -x e9be92d66f6b61c5cfbd2b56456e2205b478b04c
git push --force-with-lease

@paritytech-cmd-bot-polkadot-sdk
Copy link
Copy Markdown
Contributor

Created backport PR for stable2412:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-7656-to-stable2412
git worktree add --checkout .worktree/backport-7656-to-stable2412 backport-7656-to-stable2412
cd .worktree/backport-7656-to-stable2412
git reset --hard HEAD^
git cherry-pick -x e9be92d66f6b61c5cfbd2b56456e2205b478b04c
git push --force-with-lease

@paritytech-cmd-bot-polkadot-sdk
Copy link
Copy Markdown
Contributor

Created backport PR for stable2503:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-7656-to-stable2503
git worktree add --checkout .worktree/backport-7656-to-stable2503 backport-7656-to-stable2503
cd .worktree/backport-7656-to-stable2503
git reset --hard HEAD^
git cherry-pick -x e9be92d66f6b61c5cfbd2b56456e2205b478b04c
git push --force-with-lease

bkontur added a commit that referenced this pull request Mar 2, 2025
…` refactor (#7656)

Relates to: #7541
Relates to: #7566

This PR contains improved test cases that rely on the governance
location as preparation for AHM to capture the state as it is.
It introduces `execute_as_governance_call`, which can be configured with
various governance location setups instead of the hard-coded
`Location::parent()`.

Additionally, it adds a test for `authorize_upgrade` to all SP testnets.


## TODO
- [x] rewrite all tests using
`RuntimeHelper::<Runtime>::execute_as_governance` (because it is using
hard-coded `Location::parent()`) to use
`RuntimeHelper::<Runtime>::execute_as_governance_call`

## Follow-up
- [ ] add similar test for westend-runtime
- [ ] add test that ensure xcm-executor adds `ClearOrigin` before all
side-effect sent to different chain

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
(cherry picked from commit e9be92d)

# Conflicts:
#	cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs
EgorPopelyaev pushed a commit that referenced this pull request Mar 3, 2025
Backport #7656 into `stable2503` from bkontur.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

Co-authored-by: Branislav Kontur <bkontur@gmail.com>
@bkontur bkontur moved this to Done in @bkontur's board Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T10-tests This PR/Issue is related to tests. T14-system_parachains This PR/Issue is related to system parachains.

Projects

Status: Done
Status: Done
Status: Done
Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants