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

XVM gas handling prototype #94

Merged
merged 11 commits into from
Sep 21, 2022
Merged

Conversation

Dinonard
Copy link
Member

@Dinonard Dinonard commented Sep 13, 2022

Pull Request Summary

The purpose of this PR is to provide a prototype implementation for gas handling in XVM.

Description

When executing a contract within the context of XVM, different VMs and gas handling schemas will be present.
E.g. the most basic example is EVM and Partity's WASM - gas vs. native weight. Each unit of these measures represents
a different concept, although similar. However, both are encapsulated by substrate's native way of handling weight (or gas).

So it's natural to keep the handling of gas in XVM context tied to the native weight system.

Current Solution

We rely on native weight handling for XVM gas handling.

To take an example when EVM contract calls WASM contract:

  • EVM contract call is made by a user who sets gas_limit
  • during EVM contract execution, some gas is consumed and a XVM call is made towards a WASM contract
  • XVM precompile handles the call - it reads the remaining available gas, converts it to native weight and passes it on as max_weight via an XVM call
  • XVM pallet detects which VM is being called and delegates call handling to it, also passing the max_weight value to WASM handler
  • WASM handler executes the call, limiting it by max_weight, and in the return value returns how much weight was actually spent
  • weight of the XVM call is updated (it can only be REDUCED compared to the initial setting)
  • EVM precompile handler detects the real weight used for the XVM call, converts it to gas and refunds unused gas if needed

Same goes for WASM --> EVM direction.

Open Issues, Questions & Suggestions

  1. We should remove Async VM until we actually need to implement it.
  2. Using wrapped Call isn't good enough if we want to have a return value..
  3. Weight in the XVM call should be subtracted by XVM execution cost itself. Requires proper benchmarking.
  4. We should limit XVM call depth. Probably a good starting value is 1 or 2.
  5. XvmContext needs to be revised - imo, contract shouldn't care about limiting gas consumption - it's implicit from the call. I'm not even sure what exactly should be passed via it.

Check list

  • Receive feedback from team (1)
  • Apply first feedback
  • TBD

@Dinonard Dinonard force-pushed the feature/xvm-gas-handling branch from c4c8899 to 7543e31 Compare September 13, 2022 14:00
frame/pallet-xvm/src/lib.rs Outdated Show resolved Hide resolved
frame/pallet-xvm/src/lib.rs Show resolved Hide resolved
frame/pallet-xvm/src/lib.rs Outdated Show resolved Hide resolved
@Dinonard Dinonard force-pushed the feature/xvm-gas-handling branch from f637668 to 61a6f1a Compare September 19, 2022 14:07
@Dinonard Dinonard changed the base branch from polkadot-v0.9.28 to feature/xvm-pallet-v2 September 19, 2022 14:09
Copy link
Contributor

@0x7CFE 0x7CFE left a comment

Choose a reason for hiding this comment

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

more grumbles

@@ -21,5 +21,5 @@ members = [
"precompiles/xcm",
"precompiles/xvm",

"contracts/*",
# "contracts/*",
Copy link
Contributor

Choose a reason for hiding this comment

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

Unintentionally left edit?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a comment.

It seems that after latest updates, compilation with cargo check fails.

cargo contract check should work but it also fails for me with another error so I'll have to check that in more detail.

Perhaps this also indicates that we shouldn't keep contracts under root .toml file 🤔

frame/pallet-xvm/src/pallet/mod.rs Outdated Show resolved Hide resolved
frame/pallet-xvm/src/pallet/mod.rs Outdated Show resolved Hide resolved
@Dinonard Dinonard marked this pull request as ready for review September 20, 2022 13:43
@github-actions
Copy link

Code Coverage

Package Line Rate Branch Rate Health
frame/custom-signatures/src 81% 0%
precompiles/utils/src/data 72% 0%
contracts/xvm/src 0% 0%
chain-extensions/types/xvm/src 0% 0%
chain-extensions/trait/src 0% 0%
frame/pallet-xvm/src/pallet 39% 0%
frame/block-reward/src 96% 0%
frame/pallet-xcm/src 81% 0%
frame/vesting/src 87% 0%
precompiles/substrate-ecdsa/src 78% 0%
precompiles/utils/src 84% 0%
primitives/xcm/src 88% 0%
chain-extensions/impls/dapps-staking/src 0% 0%
frame/collator-selection/src 89% 0%
precompiles/assets-erc20/src 91% 0%
precompiles/sr25519/src 78% 0%
precompiles/xcm/src 80% 0%
precompiles/utils/macro/tests 100% 0%
chain-extensions/types/dapps-staking/src 0% 0%
frame/dapps-staking/src 93% 0%
frame/pallet-xvm/src 14% 0%
frame/xc-asset-config/src 88% 0%
precompiles/utils/macro/src 0% 0%
chain-extensions/impls/xvm/src 0% 0%
frame/dapps-staking/src/pallet 85% 0%
precompiles/dapps-staking/src 95% 0%
precompiles/xvm/src 73% 0%
Summary 84% (8418 / 9982) 0% (0 / 0)

Minimum allowed line rate is 60%

@Dinonard Dinonard merged commit d142d4b into feature/xvm-pallet-v2 Sep 21, 2022
@Dinonard Dinonard deleted the feature/xvm-gas-handling branch September 21, 2022 07:36
akru added a commit that referenced this pull request Oct 25, 2022
* Init commit

* Small fixes

* Chain extension & precompile modifications

* TODO handling

* Traces, improved gas tracking, fixed issues

* Doc to placeholder weight

* Doc & typos

* Change base

* Fix UT

* Update TODOs, cleanup

* Fix contract issues
akru added a commit that referenced this pull request Nov 8, 2022
* XVM interface refactoring

* Switch precompiles & chain-extension to XVMv2

* XVM gas handling prototype (#94)

* Init commit

* Small fixes

* Chain extension & precompile modifications

* TODO handling

* Traces, improved gas tracking, fixed issues

* Doc to placeholder weight

* Doc & typos

* Change base

* Fix UT

* Update TODOs, cleanup

* Fix contract issues

* Remove chain-extension-trait package (#109)

* Remove chain-extension-trait package

* Dapps Staking CE optimizations

* Dapps Staking CE optimizations

* Rename dapps-staking CE in general way

* Derive Default for chain extension structs

* Implement Default trait for CEs

* Fix typo

* XVM types cargo deps fix

* Build fixes

* Compilation fixes

* Fix formatting

* Fix formatting

* Fix compilation issues

* Fix cargo fmt

* Fix clippy errors

* Fix weight estimation
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants