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.

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

Evm gas estimation #3299

Merged
merged 66 commits into from
Nov 2, 2020
Merged

Evm gas estimation #3299

merged 66 commits into from
Nov 2, 2020

Conversation

ailisp
Copy link
Member

@ailisp ailisp commented Sep 9, 2020

Mostly done, comments by @ilblackdragon isn't addressed, some clean ups needed. Didn't have a proper way to pre-cap evm gas limit from near gas limit, but this should be possible. Please review regarding how number is calculated. From my machine, it's a very stable number that change 3% or less.

  • figure out why gas usage is negative
  • add evm runner in runtime-params-estimator, measure deploy_code and function call cost from runtime-param-estimator (Y), gas_used (X) of different contracts
    • deploy_code
    • function call
    • call precompiled function
  • Calculate near cost of any evm deploy/function call operation in near, with possibly call precompiled function
  • cap evm prepaid gas calculated from prepaid near gas, so evm won't use more gas than corresponding prepaid near gas
  • address Evgeny's comment
  • stablization cost counting numbers in different runs

@gitpod-io
Copy link

gitpod-io bot commented Sep 9, 2020

@ailisp ailisp changed the base branch from master to evm-precompile September 9, 2020 01:15
rust-toolchain Outdated Show resolved Hide resolved
runtime/near-vm-errors/src/lib.rs Outdated Show resolved Hide resolved
runtime/near-evm-runner/tests/utils.rs Show resolved Hide resolved
runtime/near-evm-runner/tests/standard_ops.rs Outdated Show resolved Hide resolved
runtime/near-evm-runner/src/lib.rs Show resolved Hide resolved
runtime/near-evm-runner/src/lib.rs Show resolved Hide resolved
runtime/near-evm-runner/src/evm_state.rs Outdated Show resolved Hide resolved
}
Err(VMLogicError::EvmError(err)) => {
(None, Some(VMError::FunctionCallError(FunctionCallError::EvmError(err))))
(Some(outcome), None, context.evm_gas_counter.used_gas)
Copy link
Member

Choose a reason for hiding this comment

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

Hm, I'm curious how is this done in other places? Seems like this is wrong level where evm gas should be surfaced.
And the normal gas is in the VMOutcome

Copy link
Member Author

@ailisp ailisp Sep 15, 2020

Choose a reason for hiding this comment

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

run_evm is only called at two places, one is actual call evm in vm runner:
image

where i'm going to convert evm gas and add into near gas (it could be also add somewhere else, in that case, this return arg is ignored)

Another is in runtime_param_estimator:
image

where we're only interested in evm_gas used, and are measuring number of cpu instructions and calculate a relationship between evm_gas used vs cpu instructions (cost).

how is this done in other places?

In near gas estimator, there is a feature costs_counting turn on measure mode and it's turned off in production, turned on in param estimator. but it's not necessary here, because evm crate will always calculate you evm gas used, and it's not expensive to return one more argument. In the case you don't need, you can ignore it.

@ailisp ailisp marked this pull request as ready for review September 18, 2020 00:11
@ailisp
Copy link
Member Author

ailisp commented Oct 21, 2020

@ilblackdragon @nearmax @evgenykuzyakov done in finalizeing numbers, volatility, 200K accounts: #3495 #3396 #3502, and updated the numbers, I think it's ready, please review again

@evgenykuzyakov for counting action receipt cost i had a new approach now. by inspecting every step of txn outcome/receipt, gas_used, gas_burnt, I found:

  • process_block: txn send to evm account, action receipt created, gas_used and gas_burnt is correctly count to action receipt cost for the wrapped FunctionCallAction.
  • process_block_until_no_receipts: actual place run_evm is invoked, produce no receipt and one outcome of evm execution result. cost of this part should only count evm execution.
    When run in a real node, runtime.apply did is same as above, first sent txn to evm account and in this step, gas_counter has count gas_used & gas_burnt with action receipt cost, then second step process_receipt would trigger run_evm with remaining gas from first step. It's same as a cross account function call of a wasm contract. Therefore I only measure process_block_until_no_receipts for evm execution part and in real node runtime will deduct total cost correctly

@ailisp ailisp merged commit e350e29 into evm-precompile Nov 2, 2020
@ailisp ailisp deleted the evm-gas-est branch November 2, 2020 22:01
artob added a commit that referenced this pull request Nov 17, 2020
The regression was due to an invalid zombieAttack.bin imported
in e350e29 (#3299). Hereby reverted to the original from cc4ec1f.

This also removes an annoying build warning introduced in 8a45140.
artob added a commit that referenced this pull request Nov 17, 2020
The regression was due to an invalid zombieAttack.bin imported
in e350e29 (#3299). Hereby reverted to the original from cc4ec1f.

This also removes an annoying build warning introduced in 8a45140.
evgenykuzyakov pushed a commit that referenced this pull request Nov 17, 2020
This fixes #3611. The regression was due to an invalid `zombieAttack.bin` imported in e350e29 (#3299). Hereby reverted to the original from cc4ec1f.

Additionally, this also removes an annoying build warning inadvertently introduced in 8a45140 (#3595).
evgenykuzyakov pushed a commit that referenced this pull request Dec 3, 2020
Implements near/NEPs#106

- [x] PR for Ethereum chain_id
- [x] Return deterministic errors
- [x] evm accounts are EVM.
- [x] Create sub-account from evm are removed and we don't plan to support it originally.
- [x] Integrate gas usage #3299 
- [x] Meta transaction calls are currently disabled
- <s>Precompile in EVM to call outside to NEAR</s> Moved to separate issue
- [x] Protocol version upgrade to 42 from which `evm` accounts activate.
- [ ] Add a check that the `evm` account doesn't have access keys.

Reviews
----
- [ ] @bowenwang1996 please review changes around genesis/protocol, so we don't leak this feature accidentally.
- [ ] @nearmax if you want to take a second look at runtime changes.
- [ ] @frol do we need to do something from the RPC/indexer perspective?
- [ ] @ilblackdragon In case you want to take a look as well.

Test plan
----

* Set of Solidity contracts that can be run inside EVM (see runtime/near-evm-runner/tests/contracts)
* Interact with CryptoZombies contract via runtime & RPC test
* <s>Nightly test that will leverage full e2e tooling to deploy & test a full fledged contract(s)</s> Moved to separate issue
* http://nayduck.eastus.cloudapp.azure.com:3000/#/run/764
* To the moon
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.

4 participants