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] Dry-run function #5749

Merged
merged 37 commits into from
Apr 29, 2024
Merged

[EVM] Dry-run function #5749

merged 37 commits into from
Apr 29, 2024

Conversation

devbugging
Copy link
Contributor

@devbugging devbugging commented Apr 22, 2024

Closes: #5603

🍄 API Added Function

A new function for dry-running was added to the EVM Core contract:

    /// Simulates running unsigned RLP-encoded transaction using
    /// the from address as the signer.
    /// The transaction state changes are not persisted.
    /// This is useful for gas estimation or calling view contract functions.
    access(all)
    fun dryRun(tx: [UInt8], from: EVMAddress): Result {
        return InternalEVM.dryRun(
            tx: tx,
            from: from.bytes,
        ) as! Result
    }

The function takes the unsigned RLP encoded transaction and from address and dry-run a transaction to calculate used gas and result which is reported back to the caller.

This will be used for gas estimation and calling contract values by EVM gateway.

@devbugging devbugging self-assigned this Apr 22, 2024
@devbugging devbugging changed the title [EVM] Gas estimation [EVM] Dry-run function Apr 23, 2024
@devbugging devbugging marked this pull request as ready for review April 24, 2024 16:52
@devbugging devbugging enabled auto-merge April 24, 2024 17:45
@codecov-commenter
Copy link

codecov-commenter commented Apr 25, 2024

Codecov Report

Attention: Patch coverage is 47.82609% with 36 lines in your changes are missing coverage. Please review.

Project coverage is 55.69%. Comparing base (55bb6fe) to head (16e75d0).
Report is 21 commits behind head on master.

Files Patch % Lines
fvm/evm/emulator/emulator.go 0.00% 14 Missing ⚠️
fvm/evm/handler/handler.go 48.14% 7 Missing and 7 partials ⚠️
fvm/evm/stdlib/contract.go 71.42% 4 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5749      +/-   ##
==========================================
+ Coverage   55.67%   55.69%   +0.01%     
==========================================
  Files        1094     1094              
  Lines       85576    85645      +69     
==========================================
+ Hits        47648    47696      +48     
- Misses      33321    33334      +13     
- Partials     4607     4615       +8     
Flag Coverage Δ
unittests 55.69% <47.82%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Gregor G added 7 commits April 25, 2024 19:59
This commit includes the addition of a new method named DryRunTransaction in the emulator. This method performs a dry run of an unsigned transaction without persisting any state changes. The functionality also considers the 'from' address as the signer given that the transaction is not signed.
The dry run methods in the ContractHandler have been significantly refactored to simplify their parameters and improve their clarity. Specifically, the transaction is now passed as an RLP-encoded byte array instead of separate parameters for each part of the transaction. This streamlines the function definition and the calls to it and relies more on standard transaction representation and less on custom data structures.
The `dryRun` function in the EVM contracts has been simplified for easier usage. This new implementation takes a transaction and a from-address as arguments, instead of separate entities like `from`, `to`, `gasLimit`, `value`, and `data`. The changes provide a neater interface and potentially reduce errors stemming from incorrect or ambiguous inputs.
@devbugging devbugging requested review from ramtinms and m-Peter April 25, 2024 18:48
Copy link
Contributor

@ramtinms ramtinms left a comment

Choose a reason for hiding this comment

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

Looks good to me just some minor comments

Gregor G added 4 commits April 26, 2024 14:41
# Conflicts:
#	fvm/evm/handler/handler_test.go
#	fvm/evm/stdlib/contract.cdc
#	fvm/evm/stdlib/contract.go
#	fvm/evm/stdlib/contract_test.go
#	fvm/evm/testutils/emulator.go
#	fvm/evm/types/emulator.go
#	fvm/evm/types/handler.go
Copy link
Collaborator

@m-Peter m-Peter left a comment

Choose a reason for hiding this comment

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

LGTM!

@devbugging devbugging added this pull request to the merge queue Apr 26, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 26, 2024
@devbugging devbugging enabled auto-merge April 26, 2024 16:38
@devbugging devbugging added this pull request to the merge queue Apr 29, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 29, 2024
@devbugging devbugging added this pull request to the merge queue Apr 29, 2024
Merged via the queue into master with commit c20f91f Apr 29, 2024
55 checks passed
@devbugging devbugging deleted the gregor/evm/gas-estimation branch April 29, 2024 14:03
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.

[Flow EVM] Gas estimation
5 participants