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

pallet-xvm refactor follow-up #998

Merged
merged 9 commits into from
Aug 11, 2023
Merged

pallet-xvm refactor follow-up #998

merged 9 commits into from
Aug 11, 2023

Conversation

shaunxw
Copy link
Member

@shaunxw shaunxw commented Aug 9, 2023 •
edited
Loading

Pull Request Summary

  • Max weight limit config
  • Forbid XVM re-entrance
  • Along with minor code polish and improvements

Closes #986

Check list

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

@shaunxw shaunxw added runtime This PR/Issue is related to the topic “runtime”. XVM Related to XVM labels Aug 9, 2023
Dinonard
Dinonard reviewed Aug 9, 2023
View reviewed changes
pallets/xvm/src/lib.rs Outdated Show resolved Hide resolved
pallets/xvm/src/lib.rs Outdated Show resolved Hide resolved
@github-actions
Copy link

Code Coverage

Package Line Rate Branch Rate Health
pallets/ethereum-checked/src 48% 0% ❌
chain-extensions/pallet-assets/src 0% 0% ❌
chain-extensions/types/dapps-staking/src 0% 0% ❌
precompiles/utils/macro/src 0% 0% ❌
primitives/src 69% 0% ➖
chain-extensions/types/assets/src 0% 0% ❌
pallets/contracts-migration/src 0% 0% ❌
pallets/dapps-staking/src 81% 0% ✔
chain-extensions/types/xvm/src 0% 0% ❌
pallets/dapps-staking/src/pallet 85% 0% ✔
pallets/xvm/src 46% 0% ❌
precompiles/utils/src 71% 0% ➖
precompiles/xcm/src 84% 0% ✔
primitives/src/xcm 67% 0% ➖
precompiles/dapps-staking/src 93% 0% ✔
pallets/xc-asset-config/src 54% 0% ➖
precompiles/assets-erc20/src 76% 0% ➖
precompiles/xvm/src 83% 0% ✔
chain-extensions/dapps-staking/src 0% 0% ❌
pallets/block-reward/src 85% 0% ✔
chain-extensions/xvm/src 0% 0% ❌
pallets/collator-selection/src 69% 0% ➖
pallets/custom-signatures/src 51% 0% ➖
pallets/pallet-xcm/src 53% 0% ➖
precompiles/sr25519/src 79% 0% ➖
precompiles/substrate-ecdsa/src 78% 0% ➖
Summary 57% (2336 / 4131) 0% (0 / 0) ➖

Minimum allowed line rate is 50%

Dinonard
Dinonard approved these changes Aug 11, 2023
View reviewed changes
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!

ashutoshvarma
ashutoshvarma reviewed Aug 11, 2023
View reviewed changes
Copy link
Member

@ashutoshvarma ashutoshvarma left a comment

Choose a reason for hiding this comment

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

Minor comment, otherwise looks good

pallets/xvm/src/lib.rs Show resolved Hide resolved
@shaunxw shaunxw mentioned this pull request Aug 11, 2023
Closed
ashutoshvarma
ashutoshvarma approved these changes Aug 11, 2023
View reviewed changes
Copy link
Member

@ashutoshvarma ashutoshvarma left a comment

Choose a reason for hiding this comment

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

LGTM

@shaunxw shaunxw merged commit 1302aa9 into master Aug 11, 2023
@shaunxw shaunxw deleted the feat/xvm-refactor-follow-up branch August 11, 2023 08:10
PierreOssun pushed a commit that referenced this pull request Aug 15, 2023
@shaunxw @PierreOssun
pallet-xvm refactor follow-up (#998)
855053a 
* pallet-xvm max weight limit config.

* Fix runtime tests.

* Forbid XVM re-entrancy.

* XVM Re-entrance tests.

* Naming consistency.

* XVM precompile: respect caller's gas limit setting.

* Remove max weight limit config from pallet-xvm.

* Remove unused XVM call error.

* Add tests for XVM precompile gas limit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashutoshvarma ashutoshvarma ashutoshvarma approved these changes

@Dinonard Dinonard Dinonard approved these changes

@gitofdeepanshu gitofdeepanshu Awaiting requested review from gitofdeepanshu

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

Successfully merging this pull request may close these issues.

Forbid XVM re-entrancy
3 participants
@shaunxw @Dinonard @ashutoshvarma