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

EVM: Implement estimate_gas #947

Merged
merged 1 commit into from
Sep 29, 2023
Merged

EVM: Implement estimate_gas #947

merged 1 commit into from
Sep 29, 2023

Conversation

orkunkilic
Copy link
Contributor

Description

This PR implements estimate_gas endpoint which estimates the gas used by transaction. It is a copy of reth's code but with small differences to be compatible with our module system.

This also adds gas estimation feature to eth_sendTransaction endpoint.

Linked Issues

Testing

Describe how these changes were tested. If you've added new features, have you added unit tests?
Integration tests are added.

Docs

Describe where this code is documented. If it changes a documented interface, have the docs been updated?

@orkunkilic orkunkilic marked this pull request as draft September 28, 2023 10:51
@orkunkilic
Copy link
Contributor Author

My main concern on this PR is that it copies evm_db a lot because it performs several evm inspect runs. This will happen outside the zk context and it is also the same with Reth so I guess it is not a big problem but please share your thoughts.

@codecov
Copy link

codecov bot commented Sep 28, 2023 •
edited
Loading

Codecov Report

Merging #947 (503eb7b) into nightly (e481346) will decrease coverage by 0.7%.
The diff coverage is 0.0%.

❗ Current head 503eb7b differs from pull request most recent head 666b70f. Consider uploading reports for the commit 666b70f to get more accurate results

Files Coverage Δ
...e-system/module-implementations/sov-evm/src/lib.rs 83.3% <ø> (ø)
...system/module-implementations/sov-evm/src/query.rs 0.0% <0.0%> (ø)

... and 2 files with indirect coverage changes

@bkolad
Copy link
Member

bkolad commented Sep 28, 2023 •
edited
Loading

My main concern on this PR is that it copies evm_db a lot because it performs several evm inspect runs. This will happen outside the zk context and it is also the same with Reth so I guess it is not a big problem but please share your thoughts.

Yes this is fine, we have a cache in the WrokingSet so it won't be that have on the db.

@orkunkilic orkunkilic requested a review from bkolad September 28, 2023 12:03
@orkunkilic orkunkilic marked this pull request as ready for review September 28, 2023 12:03
@bkolad bkolad enabled auto-merge September 28, 2023 18:31
auto-merge was automatically disabled September 29, 2023 09:18

Head branch was pushed to by a user without write access

@bkolad bkolad enabled auto-merge September 29, 2023 09:20
@bkolad bkolad added this pull request to the merge queue Sep 29, 2023
Merged via the queue into Sovereign-Labs:nightly with commit 91c3423 Sep 29, 2023
@bkolad bkolad deleted the orkun/estimate-gas branch September 29, 2023 10:14
dubbelosix pushed a commit that referenced this pull request Sep 29, 2023
@orkunkilic @dubbelosix
estimate-gas signed (#947)
b696d80
dubbelosix pushed a commit that referenced this pull request Sep 29, 2023
@orkunkilic @dubbelosix
estimate-gas signed (#947)
a514a5b
github-merge-queue bot pushed a commit that referenced this pull request Sep 29, 2023
@dubbelosix @neysofu
@bkolad @preston-evans98 @dependabot @vlopes11 @orkunkilic @citizen-stig
offchain processing macro and example for sov-nft-module (#939)
47650f1 
* offchain processing

* BlockScout integration fixes (#917)

* Fixes

* retesteth config

Signed-off-by: Filippo Costa <[email protected]>

---------

Signed-off-by: Filippo Costa <[email protected]>

* modules_api: simplify public key definition (#918)

* simplify publi key definition

* private key

* `EVM`: Add missing docs (#914)

* rename pending_block to block_env

* make evm internals private

* refactor query.rs

* AccountData doc

* query.rs docs

* fix exports

* add missing docs

* cargo fmt

* Add readme

* Update README.md

* Update README.md

* better docs

* Cleanup prover docs/logs (#919)

* Cleanup prever docs/logs

* fmt

* Bump markdown from 1.0.0-alpha.13 to 1.0.0-alpha.14 (#924)

Bumps [markdown](https://github.com/wooorm/markdown-rs) from 1.0.0-alpha.13 to 1.0.0-alpha.14.
- [Release notes](https://github.com/wooorm/markdown-rs/releases)
- [Commits](wooorm/markdown-rs@1.0.0-alpha.13...1.0.0-alpha.14)

---
updated-dependencies:
- dependency-name: markdown
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump rayon from 1.7.0 to 1.8.0 (#923)

Bumps [rayon](https://github.com/rayon-rs/rayon) from 1.7.0 to 1.8.0.
- [Changelog](https://github.com/rayon-rs/rayon/blob/master/RELEASES.md)
- [Commits](rayon-rs/rayon@rayon-core-v1.7.0...rayon-core-v1.8.0)

---
updated-dependencies:
- dependency-name: rayon
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump prost-build from 0.11.9 to 0.12.1 (#930)

Bumps [prost-build](https://github.com/tokio-rs/prost) from 0.11.9 to 0.12.1.
- [Release notes](https://github.com/tokio-rs/prost/releases)
- [Commits](tokio-rs/prost@v0.11.9...v0.12.1)

---
updated-dependencies:
- dependency-name: prost-build
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump parking_lot from 0.11.2 to 0.12.1 (#927)

Bumps [parking_lot](https://github.com/Amanieu/parking_lot) from 0.11.2 to 0.12.1.
- [Changelog](https://github.com/Amanieu/parking_lot/blob/master/CHANGELOG.md)
- [Commits](Amanieu/parking_lot@0.11.2...0.12.1)

---
updated-dependencies:
- dependency-name: parking_lot
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat: use json as manifest constants instead of toml (#922)

* feat: use json as manifest constants instead of toml

Currently, we parse the manifest file as TOML. However, we will use JSON
for the genesis format. For improved consistency, it is desirable to
have the constants manifest file with the same format.

* fix CI lints

* use parent as argument for error reporting

* refactor gas config parse to return declaration

* fix ci lints

* Bump tungstenite from 0.20.0 to 0.20.1 (#931)

Bumps [tungstenite](https://github.com/snapview/tungstenite-rs) from 0.20.0 to 0.20.1.
- [Changelog](https://github.com/snapview/tungstenite-rs/blob/master/CHANGELOG.md)
- [Commits](snapview/tungstenite-rs@v0.20.0...v0.20.1)

---
updated-dependencies:
- dependency-name: tungstenite
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* EVM: Implement eth_call (#921)

* implement eth_call

* implement tests and error handling for eth_call

* move errors inside evm crate

* cleanup result

* fix call env

* comment test differences

* rebase

* Revert "rebase"

This reverts commit 44e41b2.

* fix unused imports

* fix review notes

* use mix_hash as prevrandao

* improve test s

* cargo lock

* some fixes

* move common code into lib

* added issue to the nft script

* add nft cli binary to nft-utils

* Revert "add nft cli binary to nft-utils"

This reverts commit d443009.

* move nft-utils back to sovereign/nft-utils and move sql to sql.rs

* remove un-necessary deps in nft-utils

* Run tests with prover enabled.  (#932)

* Remove demo-rollup dep from demo-prover

* add methods to demo-rollup

* fix cargo hack

* fix cargo hack

* Cargo.toml

* install toolchain

* install toolchain

* modify workflow

* update cargo.toml

* rust.yaml

* Add guest-mock

* use real prover in bank tests

* native

* fix lint

* update yaml

* fic coverage

* ELF in CI

* update bench get_guest_options()

* update build.rs

* remove local from demo-rollup

* fix lint

* remove ide_setup

* Remove riscv32im-risc0-zkvm-elf

* fix: require `native` when applicable for all targets (#937)

* fix: require `native` when applicable for all targets

Prior to this commit, some checks of all-targets without the feature
`native` would break.

This commit introduces a fix for every workspace member to be consistent
with the feature set.

* change feature requirement to self-dev-dep

* update deps

* fix dupl celestia dependencies

* Remove default trait bound from Module (#941)

* Remove demo-rollup-local job from CI (#943)

* Delta's go first, remove `get` from StateCheckpoint (#953)

* Make genesis config serializable (#956)

* Make genesis config serializable

* Evm config serde

* Fix chain state integ test

* fix vec-setter

* Introduce: `PublicKeyHex` in `sov-modules-api` (#954)

* Add PublicKeyHex

* PubKeyHex impl

* Update DefaultPublicKey::from_str

* Add tests

* Remove println

* Add doc

* estimate-gas signed (#947)

* remove include and make it a module

* fix packages.yml

* EVM: Implement account related RPC (#958)

* implement account endpoints

* test account endpoints

* remove unnecessary conversions

* feature: gas meter (#795)

* feat: add gas meter to working set

This commit introduces `GasMeter`, encapsulated by `WorkingSet`.

It will allow the user to consume scalar gas from the working set, and
define arbitrary price parsed from a constants.json manifest file at
compilation. At each compilation, the `ModuleInfo` derive macro will
parse such file, and set the gas price configuration.

* fix lint fmt

* fix ci test expected error string

* update default context to 2 dimensions

* Read `accounts` genesis from a file. (#959)

* PrivateKeyHex in accounts

* Add test_config_serialization

* Add accounts.json

* fix ci

* fix CI

* cleanup

* Arbitrary PublicKeyHex

* PublicKeyHex impl

* fix a wrong merge conflict

---------

Signed-off-by: Filippo Costa <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Filippo Neysofu Costa <[email protected]>
Co-authored-by: Blazej Kolad <[email protected]>
Co-authored-by: Preston Evans <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Victor Lopes <[email protected]>
Co-authored-by: Orkun Mahir Kılıç <[email protected]>
Co-authored-by: Nikolai Golub <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bkolad bkolad bkolad approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

EVM: Implement gas limit estimation
2 participants
@orkunkilic @bkolad