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.

Sign up for GitHub

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

Jump to bottom

feat: dispatch precompile filter #1022

Merged
merged 16 commits into from
Sep 27, 2023
Merged

feat: dispatch precompile filter #1022

merged 16 commits into from
Sep 27, 2023

Conversation

ashutoshvarma
Copy link
Member

@ashutoshvarma ashutoshvarma commented Sep 12, 2023 •
edited by gitofdeepanshu
Loading

Pull Request Summary
Add filter to disable non-whitelisted calls in dispatch precompile

Check list

  • added or updated unit tests
  • updated Astar official documentation
  • added OnRuntimeUpgrade hook for precompile revert code registration
  • updated spec version
  • updated semver

@ashutoshvarma ashutoshvarma added shiden related to shiden runtime astar Related to Astar shibuya related to shibuya runtime This PR/Issue is related to the topic “runtime”. labels Sep 12, 2023
@gitofdeepanshu gitofdeepanshu changed the title feat: disable all in dispatch precompile feat: dispatch precompile filter Sep 19, 2023
@gitofdeepanshu gitofdeepanshu marked this pull request as ready for review September 19, 2023 08:29
Dinonard
Dinonard reviewed Sep 19, 2023
View reviewed changes
runtime/astar/src/lib.rs Outdated
impl InstanceFilter<RuntimeCall> for DispatchPrecompileFilter {
fn filter(&self, c: &RuntimeCall) -> bool {
match c {
RuntimeCall::Utility(pallet_utility::Call::batch { calls }) => {
Copy link
Member

Choose a reason for hiding this comment

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

There's also batch_all.

runtime/astar/src/lib.rs Outdated Show resolved Hide resolved
primitives/src/precompiles.rs
traits::InstanceFilter,
};
use pallet_evm_precompile_dispatch::DispatchValidateT;

Copy link
Member

Choose a reason for hiding this comment

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

Please add Rustdoc to public structs.

primitives/src/precompiles.rs Outdated Show resolved Hide resolved
tests/integration/src/lib.rs Show resolved Hide resolved
tests/integration/src/dispatch_filter.rs Outdated Show resolved Hide resolved
shaunxw
shaunxw reviewed Sep 22, 2023
View reviewed changes
Copy link
Member

@shaunxw shaunxw left a comment

Choose a reason for hiding this comment

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

The implementation looks good to me. Some suggestions about code style and docs.

primitives/src/precompiles.rs Outdated Show resolved Hide resolved
primitives/src/precompiles.rs Outdated Show resolved Hide resolved
runtime/astar/src/precompiles.rs Outdated Show resolved Hide resolved
runtime/astar/src/precompiles.rs Outdated Show resolved Hide resolved
tests/integration/src/lib.rs Outdated Show resolved Hide resolved
shaunxw
shaunxw previously approved these changes Sep 27, 2023
View reviewed changes
@github-actions
Copy link

Code Coverage

Package Line Rate Branch Rate Health
chain-extensions/pallet-assets/src 0% 0% ❌
chain-extensions/types/dapps-staking/src 0% 0% ❌
pallets/dapps-staking/src 81% 0% ✔
chain-extensions/xvm/src 0% 0% ❌
pallets/xvm/src 40% 0% ❌
chain-extensions/types/assets/src 0% 0% ❌
pallets/block-reward/src 85% 0% ✔
pallets/dapps-staking/src/pallet 85% 0% ✔
precompiles/batch/src 80% 0% ✔
precompiles/sr25519/src 79% 0% ➖
pallets/ethereum-checked/src 48% 0% ❌
precompiles/dapps-staking/src 93% 0% ✔
precompiles/substrate-ecdsa/src 78% 0% ➖
pallets/collator-selection/src 69% 0% ➖
precompiles/utils/macro/src 0% 0% ❌
precompiles/assets-erc20/src 76% 0% ➖
precompiles/utils/src 68% 0% ➖
precompiles/xcm/src 84% 0% ✔
precompiles/xvm/src 75% 0% ➖
primitives/src 62% 0% ➖
precompiles/utils/src/testing 62% 0% ➖
pallets/pallet-xcm/src 53% 0% ➖
pallets/xc-asset-config/src 53% 0% ➖
pallets/unified-accounts/src 80% 0% ✔
chain-extensions/types/xvm/src 0% 0% ❌
primitives/src/xcm 66% 0% ➖
pallets/contracts-migration/src 0% 0% ❌
chain-extensions/dapps-staking/src 0% 0% ❌
Summary 58% (2618 / 4545) 0% (0 / 0) ➖

Minimum allowed line rate is 50%

@gitofdeepanshu gitofdeepanshu merged commit b6a9cf5 into master Sep 27, 2023
8 checks passed
@gitofdeepanshu gitofdeepanshu deleted the feat/dispatch-block-all branch September 27, 2023 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gitofdeepanshu gitofdeepanshu gitofdeepanshu left review comments

@shaunxw shaunxw shaunxw approved these changes

@Dinonard Dinonard Dinonard approved these changes

Assignees
No one assigned
Labels
astar Related to Astar runtime This PR/Issue is related to the topic “runtime”. shibuya related to shibuya shiden related to shiden runtime
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ashutoshvarma @Dinonard @shaunxw @gitofdeepanshu