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

test: Add Shanghai tests by ethereum/execution-spec-tests #129

Merged
merged 57 commits into from
Nov 25, 2024

Conversation

ulbqb
Copy link
Contributor

@ulbqb ulbqb commented Nov 12, 2024

Proposed changes

This PR adds tests by ethereum/execution-spec-tests. Currently, this supports testing only Shanghai EIPs against each forks (only Cancun and Prague). I'm going to add tests for other forks in other PRs.

Simulating Ethereum behavior is needed because of the difference of spec between Kaia and Ethereum to use ethereum/execution-spec-tests as is. This simulate following behavior:

  • Gas price - Simulate Ethereum gas prices and set the values ​​directly on the txContext
  • Intrinsic gas - Ethereum’s intrinsic gas after Istanbul is the same as Kaia’s Prague
  • Op code gas - Change constant gas for specific op code.
  • Mining reward - Simualte Ethereum mining reward and set the value directly to the state db.
  • State root - Fetch all data from the state db and set the data to a new state db as LegacyAccount

Main code modifications:

  • Add SetLegacyAccountForTest to StateDB
  • Add relaxPrecompileRangeForTest
  • Add ChangeGasCostForTest

Remarks:

  • TestExecutionSpecState should be tested separately because of changing global variable (relaxPrecompileRangeForTest)

related: #152

Types of changes

  • Bugfix
  • New feature or enhancement
  • Others

Checklist

  • I have read the CONTRIBUTING GUIDELINES doc
  • I have read the CLA and signed by comment I have read the CLA Document and I hereby sign the CLA in first time contribute
  • Lint and unit tests pass locally with my changes ($ make test)
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Copy link

github-actions bot commented Nov 12, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@ulbqb ulbqb force-pushed the eip-7702-exec-tests branch from 002866f to e62559e Compare November 12, 2024 14:29
@ulbqb ulbqb changed the title test: Add tests using ethereum/execution-spec-tests test: Add tests by ethereum/execution-spec-tests Nov 12, 2024
@ulbqb ulbqb marked this pull request as ready for review November 15, 2024 14:45
@ulbqb ulbqb requested review from hyunsooda and blukat29 November 15, 2024 14:46
@ulbqb
Copy link
Contributor Author

ulbqb commented Nov 18, 2024

Update

  • Enable to change opcode gas cost only during tests (useEthOpCodeGas) - This is needed if Kaia adopts different gas cost of opcode from Ethereum such as EXTCODEHASH.
  • Enable to running node as only Cancun and Prague only during tests - It's meaningless to test Kaia as old forks from now. And Randao, Kip103, and Kaia forks are run as Cancun.

Makefile Outdated
@@ -28,10 +28,12 @@ abigen:
@echo "Run \"$(BIN)/abigen\" to launch abigen."

test:
$(GORUN) build/ci.go test
$(GORUN) build/ci.go test -skip ^TestExecutionSpecState$$
Copy link
Contributor

Choose a reason for hiding this comment

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

@blukat29 t.Parallel() seems not much effective and shows no difference in my local. What do you think?

tests/state_test_util.go Outdated Show resolved Hide resolved
@ulbqb ulbqb requested a review from hyunsooda November 19, 2024 09:44
tests/state_test_util.go Outdated Show resolved Hide resolved
@ulbqb ulbqb requested a review from hyunsooda November 20, 2024 04:24
@ulbqb ulbqb merged commit 229fd15 into kaiachain:dev Nov 25, 2024
11 checks passed
@ulbqb ulbqb deleted the eip-7702-exec-tests branch November 25, 2024 06:01
@github-actions github-actions bot locked and limited conversation to collaborators Nov 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants