Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Whitelist pallet preimage provider upgrade #12834

Merged
merged 6 commits into from
Dec 6, 2022
Merged

Whitelist pallet preimage provider upgrade #12834

merged 6 commits into from
Dec 6, 2022

Conversation

muharem
Copy link
Contributor

@muharem muharem commented Dec 3, 2022 •
edited
Loading

Attempt to solve #12821

polkadot companion: paritytech/polkadot#6392

@muharem muharem added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Dec 3, 2022
@muharem muharem requested review from ggwpez and gavofyork December 4, 2022 00:32
gavofyork
gavofyork approved these changes Dec 5, 2022
View reviewed changes
Copy link
Member

@gavofyork gavofyork 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.

@muharem muharem requested a review from bkchr December 5, 2022 14:51
@muharem muharem mentioned this pull request Dec 5, 2022
Merged
ggwpez
ggwpez approved these changes Dec 5, 2022
View reviewed changes
frame/whitelist/src/benchmarking.rs Outdated
@@ -40,7 +37,7 @@ benchmarks! {
"call not whitelisted"
);
ensure!(
T::PreimageProvider::preimage_requested(&call_hash),
<T::Preimages as QueryPreimage>::is_requested(&call_hash),
Copy link
Member

Choose a reason for hiding this comment

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

nit

Suggested change
<T::Preimages as QueryPreimage>::is_requested(&call_hash),
T::Preimages::is_requested(&call_hash),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated it everywhere.

frame/whitelist/src/lib.rs Show resolved Hide resolved
frame/whitelist/src/lib.rs Outdated
WhitelistedCallDispatched { call_hash: T::Hash, result: DispatchResultWithPostInfo },
CallWhitelisted { call_hash: Hash },
WhitelistedCallRemoved { call_hash: Hash },
WhitelistedCallDispatched { call_hash: Hash, result: DispatchResultWithPostInfo },
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you can change these to H256 to easier spot that it is not the standard T::Hash.

Copy link
Member

Choose a reason for hiding this comment

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

Could also be preimages::Hash as PreimageHash

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

frame/whitelist/src/lib.rs Outdated
@@ -114,12 +114,12 @@ pub mod pallet {
}

#[pallet::storage]
pub type WhitelistedCall<T: Config> = StorageMap<_, Twox64Concat, T::Hash, (), OptionQuery>;
pub type WhitelistedCall<T: Config> = StorageMap<_, Twox64Concat, Hash, (), OptionQuery>;
Copy link
Member

@ggwpez ggwpez Dec 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I think we always use Hash = H256 but it would need a migration if some chains use something different.
Probably noteworthy for the change-log.

Copy link
Member

Choose a reason for hiding this comment

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

I very much doubt any are.

@ggwpez
Copy link
Member

ggwpez commented Dec 5, 2022

/cmd queue -c bench-bot $ pallet dev pallet_whitelist

@command-bot
Copy link

command-bot bot commented Dec 5, 2022 •
edited
Loading

@ggwpez https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2120335 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_whitelist. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment /cmd cancel 6-3cab59be-80a0-48bf-8748-ca7ed32be994 to cancel this command or /cmd cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Dec 5, 2022

@ggwpez Command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_whitelist has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2120335 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2120335/artifacts/download.

@muharem
Copy link
Contributor Author

muharem commented Dec 6, 2022

will wait for the companion to be approved, before merging.

@gavofyork gavofyork merged commit 44fbbd9 into master Dec 6, 2022
@gavofyork gavofyork deleted the muharem-whitelist-preimage-upgrade branch December 6, 2022 14:51
@muharem muharem mentioned this pull request Dec 12, 2022
Closed
@AurevoirXavier AurevoirXavier mentioned this pull request Jan 10, 2023
Closed
@kacperzuk-neti kacperzuk-neti mentioned this pull request Feb 14, 2023
Merged
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
@muharem @ltfschoen
Whitelist pallet preimage provider upgrade (paritytech#12834)
0957aa0 
* whitelist preimage provider upgrade

* rustdocs unresolved link error fix

* ".git/.scripts/bench-bot.sh" pallet dev pallet_whitelist

* PreimageHash alias, remove type annotation

Co-authored-by: command-bot <>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@muharem @ark0f
Whitelist pallet preimage provider upgrade (paritytech#12834)
b14ecbd 
* whitelist preimage provider upgrade

* rustdocs unresolved link error fix

* ".git/.scripts/bench-bot.sh" pallet dev pallet_whitelist

* PreimageHash alias, remove type annotation

Co-authored-by: command-bot <>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gavofyork gavofyork gavofyork approved these changes

@ggwpez ggwpez ggwpez approved these changes

@bkchr bkchr Awaiting requested review from bkchr

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@muharem @ggwpez @gavofyork