Skip to content

proxy types: refactor whitelist of calls#646

Merged
fellowship-merge-bot[bot] merged 18 commits into
polkadot-fellows:mainfrom
gui1117:gui-refactor
Aug 18, 2025
Merged

proxy types: refactor whitelist of calls#646
fellowship-merge-bot[bot] merged 18 commits into
polkadot-fellows:mainfrom
gui1117:gui-refactor

Conversation

@gui1117
Copy link
Copy Markdown
Contributor

@gui1117 gui1117 commented Mar 27, 2025

Simply refactor the whitelist of calls in proxy types.
Whitelist is a better practice than blacklist in this context.

Copy link
Copy Markdown
Contributor

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

I would not add pallets to the whitelist that have no callable functions for users.

Also this pr requires a CHANGELOG entry.

Comment on lines +550 to +555
RuntimeCall::Aura(_) |
RuntimeCall::AuraExt(_) |
RuntimeCall::XcmpQueue(_) |
RuntimeCall::CumulusXcm(_) |
RuntimeCall::ToPolkadotXcmRouter(_) |
RuntimeCall::MessageQueue(_) |
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.

There is no functionality for users.

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.

Yes done: 10a4767

But I didn't look extremely carefully for each pallet but I think I didn't miss any.

RuntimeCall::Multisig(_) |
RuntimeCall::Proxy(_) |
RuntimeCall::RemoteProxyRelayChain(_) |
RuntimeCall::StateTrieMigration(_)
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.

Suggested change
RuntimeCall::StateTrieMigration(_)

@bkchr
Copy link
Copy Markdown
Contributor

bkchr commented Mar 31, 2025

@gui1117 CI is not happy.

@github-actions
Copy link
Copy Markdown

Review required! Latest push from author must always be reviewed

@gui1117
Copy link
Copy Markdown
Contributor Author

gui1117 commented Apr 1, 2025

Yes sorry fixed.

ecosystem tests seems hanging, I will look into it later.

EDIT: some timeout error, let's re-run

Comment thread CHANGELOG.md Outdated
Comment on lines +516 to +520
RuntimeCall::Broker(pallet_broker::Call::claim_revenue { .. }) |
RuntimeCall::Broker(pallet_broker::Call::drop_region { .. }) |
RuntimeCall::Broker(pallet_broker::Call::drop_contribution { .. }) |
RuntimeCall::Broker(pallet_broker::Call::drop_history { .. }) |
RuntimeCall::Broker(pallet_broker::Call::drop_renewal { .. }) |
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.

Besides this one, the other calls should not be required.

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.

yes ok, partition could be added, but I kept it removed. done 48b9837

RuntimeCall::Broker(pallet_broker::Call::set_lease { .. }) |
RuntimeCall::Broker(pallet_broker::Call::start_sales { .. }) |
RuntimeCall::Broker(pallet_broker::Call::partition { .. }) |
RuntimeCall::Broker(pallet_broker::Call::claim_revenue { .. }) |
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.

Same here

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.

yes done 48b9837

Comment on lines +200 to +207
RuntimeCall::EncointerScheduler(_) |
RuntimeCall::EncointerCeremonies(_) |
RuntimeCall::EncointerCommunities(_) |
RuntimeCall::EncointerBazaar(_) |
RuntimeCall::EncointerReputationCommitments(_) |
RuntimeCall::EncointerFaucet(_) |
RuntimeCall::EncointerDemocracy(_) |
RuntimeCall::EncointerTreasuries(_)
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.

@clangenb could you verify these?

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.

Yes. looks good to me!

Co-authored-by: Bastian Köcher <git@kchr.de>
@github-actions github-actions Bot requested a review from bkchr April 16, 2025 01:06
@bkchr bkchr requested a review from ggwpez April 28, 2025 19:54
Comment on lines +200 to +207
RuntimeCall::EncointerScheduler(_) |
RuntimeCall::EncointerCeremonies(_) |
RuntimeCall::EncointerCommunities(_) |
RuntimeCall::EncointerBazaar(_) |
RuntimeCall::EncointerReputationCommitments(_) |
RuntimeCall::EncointerFaucet(_) |
RuntimeCall::EncointerDemocracy(_) |
RuntimeCall::EncointerTreasuries(_)
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.

Yes. looks good to me!

Comment thread CHANGELOG.md Outdated
- Disable MBM migrations for all runtimes for check-migrations CI ([polkadot-fellows/runtimes/pull/590](https://github.com/polkadot-fellows/runtimes/pull/590))
- chain-spec-generator supports conditional building (`--no-default-features --features <runtime>` or `--no-default-features --features all-runtimes` or
`--no-default-features --features all-polkadot` or `--no-default-features --features all-kusama`)([polkadot-fellows/runtimes/pull/637](https://github.com/polkadot-fellows/runtimes/pull/637))
- Proxy type `NonTranfer`: Use a whitelist of calls and remove calls to pallets with not useful calls ([polkadot-fellows/runtimes/pull/646](https://github.com/polkadot-fellows/runtimes/pull/646))
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 move this to its right place in the changelog (under "unreleased")

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.

yes done: 81b6fb6

@acatangiu
Copy link
Copy Markdown
Contributor

and please a more descriptive title/description for the PR 😄

@gui1117 gui1117 changed the title Refactor a bit proxy types: refactor whitelist of calls Aug 18, 2025
@acatangiu
Copy link
Copy Markdown
Contributor

/merge

@fellowship-merge-bot
Copy link
Copy Markdown
Contributor

Enabled auto-merge in Pull Request

Available commands
  • /merge: Enables auto-merge for Pull Request
  • /merge cancel: Cancels auto-merge for Pull Request
  • /merge help: Shows this menu

For more information see the documentation

@fellowship-merge-bot fellowship-merge-bot Bot enabled auto-merge (squash) August 18, 2025 10:29
@fellowship-merge-bot fellowship-merge-bot Bot merged commit 1759908 into polkadot-fellows:main Aug 18, 2025
52 of 62 checks passed
match self {
ProxyType::Any => true,
ProxyType::NonTransfer => !matches!(
ProxyType::NonTransfer => matches!(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This conflicts with the changes in the call filter of the asset hub migration... can we please pause doing refactors until AHM is done.

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.

yes, you're right, sorry about that Oli!

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.

5 participants