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

Fix pyspec hive tests #2621

Merged
merged 9 commits into from
Apr 10, 2023
Merged

Fix pyspec hive tests #2621

merged 9 commits into from
Apr 10, 2023

Conversation

jochem-brouwer
Copy link
Member

@jochem-brouwer jochem-brouwer commented Apr 5, 2023

This PR fixes the pyspec hive tests. These tests are tests from the execution-spec-tests repo and re-execute these tests, but now using the engine API of the client.

No consensus bugs were found. However, some missing features and some annoying stuff has been fixed in this PR: (most of those are edge cases, except genesis files where nonce is nonzero for some accounts)

  • When importing genesis state (for instance, from a Geth genesis), also read the nonce (if available)
    • This caused some pyspec tests not to run, because the provided state after putting this genesis has a different state root than what the genesis block actually points to (which results in client reporting: parent block not executed yet)
  • Ensures that if genesis blocks have very large (larger than Number.MAX_SAFE_INTEGER) gasLimit or difficulty, this is parsed right by using BigInt
  • Ensures in Trie that if we request if the empty state root (so 0 accounts, not even empty ones) is available, it returns true (this value is not in the DB). Fixes a test which starts with empty genesis allocation.
  • Fixes setting up the genesis trie db in VM where, if you put empty accounts, this account is actually in the touched list and thus deleted on the first tx run (only exception of course if this tx creates a non-empty account)
  • Updates engine-only RPC to only require the strict methods which should be available by current spec
  • Fixes a nasty instance where you create a private key file which ends with a newline (private key import then thinks private key is 33 bytes instead of expected 32 bytes). Now client removes all newlines from the private key file.

Before this PR: fail 93/94. Current: fail 0/94

To run:

  • Ensure you have updated hive to the latest version
  • From hive: go run hive.go --docker.output --client ethereumjs --sim ethereum/pyspec

@codecov
Copy link

codecov bot commented Apr 5, 2023

Codecov Report

❗ No coverage uploaded for pull request base (develop-v7@a42c10e). Click here to learn what that means.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
ethash ∅ <0.00%> (?)
evm 79.48% <0.00%> (?)
tx 94.45% <0.00%> (?)

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

@jochem-brouwer jochem-brouwer changed the title Client: update exposed eth methods in case EngineOnly RPC is requested Client: update exposed eth methods in case EngineOnly RPC is requested + fix pyspec hive tests Apr 5, 2023
@jochem-brouwer
Copy link
Member Author

Current status:

[b8fa5a84] failing tests:
[b8fa5a84] ethereumjs/withdrawals/withdrawals/use_value_in_contract/000/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create2_opcode/000/max_size_zeros/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create2_opcode/001/max_size_ones/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create_opcode/009/49121_bytes/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create_opcode/004/empty/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create_opcode/005/single_byte/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/withdrawals/withdrawals/use_value_in_tx/001/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create2_opcode/006/32_bytes/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create2_opcode/007/33_bytes/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create_opcode/001/max_size_ones/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create_opcode/000/max_size_zeros/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create2_opcode/004/empty/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create2_opcode/009/49121_bytes/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create2_opcode/003/over_limit_ones/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create_opcode/002/over_limit_zeros/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create_opcode/007/33_bytes/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create_opcode/003/over_limit_ones/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create_opcode/006/32_bytes/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create_opcode/008/49120_bytes/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create2_opcode/005/single_byte/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create2_opcode/008/49120_bytes/shanghai: payload status mismatch
[b8fa5a84] ethereumjs/eips/eip3860/initcode_limit_create2_opcode/002/over_limit_zeros/shanghai: payload status mismatch
INFO[04-06|00:44:58] API: test ended                          suite=0 test=1 pass=true
INFO[04-06|00:44:58] API: suite ended                         suite=0
INFO[04-06|00:44:59] simulation ethereum/pyspec finished      suites=1 tests=94 failed=22

Will continue tomorrow.

@jochem-brouwer
Copy link
Member Author

Current failing:

go run hive.go --docker.output --client ethereumjs --sim ethereum/pyspec --sim.limit "pyspec/eips/eip3860/initcode_limit_create2_opcode/002/over_limit_zeros/shanghai"

Fix by 7cc4c0f ensures the genesis has the correct state root. However, it now complains that "block 0 is not executed", I think this is because these trie nodes of genesis are not written to disk? So VM trie has no access to it (parent state root (e.g. genesis state root) unknown to VM). (Also this is somewhat a big problem, it thus seems we cannot init a custom chain client if we provide genesis state..?)

@jochem-brouwer
Copy link
Member Author

At commit 77287ba:

[b7413058] failing tests:
[b7413058] ethereumjs/withdrawals/withdrawals/use_value_in_contract/000/shanghai: payload status mismatch
[b7413058] ethereumjs/withdrawals/withdrawals/use_value_in_tx/001/shanghai: payload status mismatch

Only 2 failing.

@jochem-brouwer jochem-brouwer marked this pull request as ready for review April 10, 2023 18:27
@jochem-brouwer jochem-brouwer changed the title Client: update exposed eth methods in case EngineOnly RPC is requested + fix pyspec hive tests Fix pyspec hive tests Apr 10, 2023
Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

LTGM - I ran the hive tests and they all passed as well.

@acolytec3 acolytec3 merged commit ec2494c into develop-v7 Apr 10, 2023
@acolytec3 acolytec3 deleted the update-engine-only-methods branch April 10, 2023 23:55
g11tech pushed a commit that referenced this pull request Apr 13, 2023
* client: update engine-only rpc methods to be exposed by spec

* common: add support for geth configs with gasLimit/difficulty > Number.MAX_SAFE_INTEGER

* client: ensure if private key is unlocked, remove any accidentally added newlines from file [no ci]

* blockchain: support genesis nonce [no ci]

* vm/eei: also read nonce from genesis [no ci]

* vm/eei: ensure empty accounts at genesis are not marked as touched [no ci]

* trie: ensure checkRoot returns true in case root of empty trie is provided

* vm: fix tests

---------

Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants