Proper Precompile Gas Consumption #337

notlesh · 2021-03-31T19:43:23Z

This PR modifies our precompiles to charge for gas in the same manner as mainnet Ethereum does. This means adopting the same case-by-case calculations for gas.

In addition, it also pulls in some test vectors I found for various precompiles. These test vectors (I refer to them as "Ethereum Consensus tests" sometimes, perhaps incorrectly) test expected input and output for a given precompile as well as expected gas costs in some cases. There are many more similar test vectors out there (example).

There are also numerous fixes for logic errors in some of the precompiles. If this PR is unwelcome, I suggest we still pull in these fixes.

frame/evm/precompile/blake2/src/lib.rs

notlesh

This test failure seems intermittent:

  Frontier RPC (EthFilterApi)
web3-shh package will be deprecated in version 1.3.5 and will no longer be supported.
web3-bzz package will be deprecated in version 1.3.5 and will no longer be supported.
    1) should return response for raw Log filter request.


  0 passing (3s)
  1 failing

  1) Frontier RPC (EthFilterApi)
       should return response for raw Log filter request.:
     TypeError: Cannot read property 'logs' of null
      at Context.<anonymous> (tests/test-filter-api.ts:124:18)
      at step (tests/test-filter-api.ts:33:23)
      at Object.next (tests/test-filter-api.ts:14:53)
      at fulfilled (tests/test-filter-api.ts:5:58)
      at processTicksAndRejections (internal/process/task_queues.js:97:5)

On my own local machine, I ran this test in a loop and it took about 20 runs before it failed. I suspect that it is unrelated to this PR.

frame/evm/precompile/blake2/src/lib.rs

sorpaas · 2021-04-20T13:33:25Z

frame/evm/Cargo.toml

@@ -23,7 +23,7 @@ sp-core = { version = "3.0.0", default-features = false, git = "https://github.c
 sp-runtime = { version = "3.0.0", default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "frontier" }
 sp-std = { version = "3.0.0", default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "frontier" }
 sp-io = { version = "3.0.0", default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "frontier" }
-fp-evm = { version = "1.0.0", default-features = false, path = "../../primitives/evm" }
+fp-evm = { version = "1.0.1-dev", default-features = false, path = "../../primitives/evm" }


Please also update pallet-evm to 3.0.1-dev

frame/evm/precompile/dispatch/Cargo.toml

frame/evm/precompile/ed25519/Cargo.toml

sorpaas · 2021-04-20T13:35:13Z

frame/evm/precompile/sha3fips/Cargo.toml

 description = "SHA3 FIPS202 precompile for EVM pallet."

 [dependencies]
 sp-core = { version = "3.0.0", default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "frontier" }
 sp-io = { version = "3.0.0", default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "frontier" }
-fp-evm = { version = "1.0.0", default-features = false, path = "../../../../primitives/evm" }
+fp-evm = { version = "1.0.1-dev", default-features = false, path = "../../../../primitives/evm" }


Update pallet-evm-precompile-sha3fips to 1.0.1-dev

sorpaas · 2021-04-20T13:35:59Z

frame/evm/test-vector-support/Cargo.toml

@@ -0,0 +1,26 @@
+[package]
+name = "pallet-evm-test-vector-support"
+version = "1.0.1-dev"


Suggested change

version = "1.0.1-dev"

version = "1.0.0-dev"

Because this crate has not yet been released at all!

primitives/rpc/Cargo.toml

primitives/storage/Cargo.toml

sorpaas · 2021-04-20T13:39:38Z

The majority of the required version bumps remaining comes from the fact that fp-evm is updated from 1.0.0 to 1.0.1-dev. That is okay. But alternatively, since there's no noticeable changes at all to fp-evm, you can also keep it at 1.0.0.

notlesh · 2021-04-21T20:23:56Z

@sorpaas I believe I took care of the version changes you requested, but feel free to be pedantic if not.

Also, I read up on https://github.com/paritytech/frontier/#versioning for next time 👍

primitives/storage/Cargo.toml

sorpaas · 2021-04-22T14:37:02Z

client/consensus/Cargo.toml

@@ -17,7 +17,7 @@ sc-client-api = { version = "3.0.0", git = "https://github.com/paritytech/substr
 sp-block-builder = { version = "3.0.0", git = "https://github.com/paritytech/substrate.git", branch = "frontier" }
 sp-inherents = { version = "3.0.0", git = "https://github.com/paritytech/substrate.git", branch = "frontier" }
 fp-consensus = { version = "1.0.0", path = "../../primitives/consensus" }
-fp-rpc = { version = "1.0.0", path = "../../primitives/rpc" }
+fp-rpc = { version = "1.0.1-dev", path = "../../primitives/rpc" }


All bumps should be recursive so fc-consensus needs to be bumped!

Done in 565ea71. And I think that should cover it: every crate that was modified got a bump with -dev unless it was already at -dev. The exception is the node template and runtime, which appear to not be versioned. Hope I got it right this time.

sorpaas · 2021-04-22T16:02:00Z

Please pull master!

sorpaas · 2021-04-22T20:35:25Z

@notlesh Still has conflict with master!

notlesh · 2021-04-22T21:53:02Z

@notlesh Still has conflict with master!

Odd, I pushed a new commit (which merges master), but this PR doesn't seem to update. You can see it on PureStake's repo: https://github.com/PureStake/frontier/commits/notlesh-precompile-gas . Commit is b29dd13.

sorpaas · 2021-04-22T21:53:55Z

Hmm let me try reopen this PR.

notlesh · 2021-04-22T21:55:12Z

I see the commit now

sorpaas

Thank you!

* First pass at Modexp precompile gas calculation * Incremental WIP adding modexp-eip2565.json tests * Include Modexp in precompile set * Working impl for ethereum test cases * Cleanup * Add Ethereum consensus tests in Modexp unit tests * Return gas cost from Modexp::execute() * Add (not very DRY) ethereum tests for Blake2 precompile * Fixes for Blake2F precompile * Implement EIP1108 gas costs for bn128 precompiles * Make precompile consensus test generic * Move test_precompile_consensus_tests to fp_evm * Clean up precompile testdata * Improve test_precompile_consensus_tests error handling * Change ECRecover to return "" on error; add related tests * Add clarifying comment * Add sha256 and ripem160 precompile test vectors * Add bn128 test vectors * Remove non-existent mod declaration * Implement Blake2 gas cost * Remove ecrecover tests that expect bytes 33-63 to matter * editorconfig * Remove version specification from local crate references * Remove modexp_eip2565 ts-tests (now redundant) * Use 'core::cmp::max' * Resolve compilation warnings * Handle target_gas properly * Revert from_ne_bytes -> from_le_bytes with comment about the logic * Bump precompile versions * Use 1.1.0 instead of 1.0.1 for new precompile versions Co-authored-by: Wei Tang <[email protected]> * Bump fp-evm to 1.0.1-dev * Reflect new fp-evm version in crates which depend on it * Remove unused log dependency in modexp precompile * Move eth "consensus tests" utility to its own crate * Modify remaining precompile tests to use pallet_evm_test_vector_support * Leave primitives/evm/src/precompile.r untouched * Add missing files * Use 1.0.1-dev as version for pallet-evm-test-vector-support * Chain 'std' for 'fp-evm' * Specify paritytech/frontier as the repo in all Cargo.toml files * Bump versions * Bump fp-storage to 1.0.1-dev * Bump fp-rpc to 1.0.1-dev * Bump fc-consensus to 1.0.1-dev Co-authored-by: Wei Tang <[email protected]>

notlesh added 9 commits March 31, 2021 10:22

First pass at Modexp precompile gas calculation

323703a

Incremental WIP adding modexp-eip2565.json tests

e15510e

Include Modexp in precompile set

4190349

Working impl for ethereum test cases

32bd18f

Cleanup

8741c70

Add Ethereum consensus tests in Modexp unit tests

173c1eb

Return gas cost from Modexp::execute()

9f38a16

Add (not very DRY) ethereum tests for Blake2 precompile

33dc225

Fixes for Blake2F precompile

2b9f4df

notlesh commented Apr 6, 2021

View reviewed changes

frame/evm/precompile/blake2/src/lib.rs Show resolved Hide resolved

notlesh added 16 commits April 6, 2021 14:48

Implement EIP1108 gas costs for bn128 precompiles

1328579

Make precompile consensus test generic

2d39fd0

Move test_precompile_consensus_tests to fp_evm

2402503

Clean up precompile testdata

907349c

Improve test_precompile_consensus_tests error handling

df397b7

Change ECRecover to return "" on error; add related tests

13d291f

Add clarifying comment

60b2dde

Add sha256 and ripem160 precompile test vectors

32cd435

Add bn128 test vectors

cecc514

Remove non-existent mod declaration

354f01e

Implement Blake2 gas cost

3c90389

Remove ecrecover tests that expect bytes 33-63 to matter

af60d3e

Merge branch 'master' into notlesh-precompile-gas

32abb10

editorconfig

197996e

Remove version specification from local crate references

344f850

Remove modexp_eip2565 ts-tests (now redundant)

b71086c

notlesh commented Apr 9, 2021

View reviewed changes

notlesh marked this pull request as ready for review April 9, 2021 16:47

notlesh requested a review from sorpaas as a code owner April 9, 2021 16:47

sorpaas reviewed Apr 9, 2021

View reviewed changes

frame/evm/precompile/blake2/src/lib.rs Outdated Show resolved Hide resolved

Merge branch 'master' into notlesh-precompile-gas

1898b3f

sorpaas reviewed Apr 20, 2021

View reviewed changes

frame/evm/precompile/dispatch/Cargo.toml Show resolved Hide resolved

sorpaas reviewed Apr 20, 2021

View reviewed changes

frame/evm/precompile/ed25519/Cargo.toml Show resolved Hide resolved

sorpaas reviewed Apr 20, 2021

View reviewed changes

primitives/rpc/Cargo.toml Show resolved Hide resolved

sorpaas reviewed Apr 20, 2021

View reviewed changes

primitives/storage/Cargo.toml Show resolved Hide resolved

Bump versions

132933a

sorpaas reviewed Apr 22, 2021

View reviewed changes

primitives/storage/Cargo.toml Show resolved Hide resolved

notlesh added 2 commits April 22, 2021 08:27

Bump fp-storage to 1.0.1-dev

69d35b5

Bump fp-rpc to 1.0.1-dev

c3a27de

sorpaas reviewed Apr 22, 2021

View reviewed changes

Bump fc-consensus to 1.0.1-dev

565ea71

Merge branch 'master' into notlesh-precompile-gas

c173557

Merge branch 'master' into notlesh-precompile-gas

b29dd13

notlesh changed the base branch from master to legacy April 22, 2021 21:53

sorpaas closed this Apr 22, 2021

sorpaas reopened this Apr 22, 2021

notlesh changed the base branch from legacy to master April 22, 2021 21:54

sorpaas approved these changes Apr 22, 2021

View reviewed changes

sorpaas merged commit 961e992 into polkadot-evm:master Apr 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proper Precompile Gas Consumption #337

Proper Precompile Gas Consumption #337

notlesh commented Mar 31, 2021 •

edited

Loading

notlesh left a comment •

edited

Loading

sorpaas Apr 20, 2021

sorpaas Apr 20, 2021

sorpaas Apr 20, 2021

sorpaas Apr 20, 2021

sorpaas commented Apr 20, 2021

notlesh commented Apr 21, 2021

sorpaas Apr 22, 2021

notlesh Apr 22, 2021

sorpaas commented Apr 22, 2021

sorpaas commented Apr 22, 2021

notlesh commented Apr 22, 2021

sorpaas commented Apr 22, 2021

notlesh commented Apr 22, 2021

sorpaas left a comment

Proper Precompile Gas Consumption #337

Proper Precompile Gas Consumption #337

Conversation

notlesh commented Mar 31, 2021 • edited Loading

notlesh left a comment • edited Loading

Choose a reason for hiding this comment

sorpaas Apr 20, 2021

Choose a reason for hiding this comment

sorpaas Apr 20, 2021

Choose a reason for hiding this comment

sorpaas Apr 20, 2021

Choose a reason for hiding this comment

sorpaas Apr 20, 2021

Choose a reason for hiding this comment

sorpaas commented Apr 20, 2021

notlesh commented Apr 21, 2021

sorpaas Apr 22, 2021

Choose a reason for hiding this comment

notlesh Apr 22, 2021

Choose a reason for hiding this comment

sorpaas commented Apr 22, 2021

sorpaas commented Apr 22, 2021

notlesh commented Apr 22, 2021

sorpaas commented Apr 22, 2021

notlesh commented Apr 22, 2021

sorpaas left a comment

Choose a reason for hiding this comment

notlesh commented Mar 31, 2021 •

edited

Loading

notlesh left a comment •

edited

Loading