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

tests: Enable EEST blockchainTests #238

Merged
merged 34 commits into from
Feb 12, 2025
Merged

Conversation

Mdaiki0730
Copy link
Contributor

@Mdaiki0730 Mdaiki0730 commented Jan 28, 2025

Proposed changes

Modify the eest blockchain test flow to run it on all forks (except 2537 and 7702). This time, enable state and storage checks, and comment out header validation.

Test flow

  1. Calculate genesis from json
  2. Compare the genesis block hash and state root with the expected in json
  3. Create a Kaia blockchain object and insert blocks created from RLP data.
  4. After inserting the block, compare the expected state (post state) in the JSON with the actual state.
  5. Validate the part of header of the inserted block

Changes

  • ./tests
    • Add a test engine that reproduces Ethereum gas fees and rewards.
    • Process to decode Eth Block RLP to Kaia TX
    • Change IsPrecompiledContractAddress to a global variable
    • Divide useEthGasPrice into a json modification part and a calculation part
    • Divide useEthMiningReward into a json modification part and a calculation part
  • ./blockchain
    • Add a call to BeforeApplyMessage in blockchain.ApplyTransaction
    • Add a function to chain maker to add Tx to a block regardless of the occurrence of an error.

Impact to the main logic

  • blockchain.ApplyTransaction
    • If the engine implements the BeforeApplyMessage method, it will be called immediately before ApplyMessage with blockchain.ApplyTransaction. Existing engines do not implement this method and so are not affected.
  • IsPrecompiledContractAddress
    • Some tx that use IsPrecompiledContractAddress for validation will provide an additional rules argument to use it.
    • If you write a process to update this during main code execution, it could cause problems with your validation logic.

Types of changes

Please put an x in the boxes related to your change.

  • Bugfix
  • New feature or enhancement
  • Others

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • 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

Related issues

  • Add Ethereum compatibility test #152
    • Add blockchainTests from ethereum/execution-spec-test for new EIPs if it's needed
    • Add blockchainTest from ethereum/execution-spec-test for old EIPs if it's needed (except 2537 and 7702)

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

Mdaiki0730 and others added 26 commits January 16, 2025 19:30
test: Enable Shanghai and Cancun EIPs blockchain test by Cancun and Prague forks
* Enable eip 2935 on blockchain EEST

* Fix cyclic import

* Fix for lint
* Enable old forks other than frontier and byzantium

* Restore all old fork tests
* Remove gxhash global variable

* Remove unused code
test: Remove some main code modifications in EETS
@ulbqb ulbqb mentioned this pull request Jan 29, 2025
20 tasks
@Mdaiki0730 Mdaiki0730 marked this pull request as ready for review January 29, 2025 03:43
@ulbqb ulbqb requested a review from tnasu January 30, 2025 10:23
@blukat29 blukat29 changed the title test: Enable blockchain EEST tests: Enable EEST blockchainTests Jan 31, 2025
@blukat29
Copy link
Contributor

blukat29 commented Feb 3, 2025

Let's merge it after rc.3.

@blukat29 blukat29 added do not merge Do not merge just yet and removed do not merge Do not merge just yet labels Feb 3, 2025
@Mdaiki0730
Copy link
Contributor Author

I'm planning to merge dev once after #246 is merged. (To incorporate the eest update and genesis changes)

@hyeonLewis
Copy link
Contributor

Please rebase on dev for CI fix https://app.circleci.com/pipelines/github/kaiachain/kaia/1915/workflows/33fd30c6-9855-48bc-902b-a81c999af375/jobs/10398

@ulbqb ulbqb self-requested a review February 12, 2025 02:25
@Mdaiki0730
Copy link
Contributor Author

Mdaiki0730 commented Feb 12, 2025

I'll make PR such as #250 after this PR will be merged.
Please check the points excluding it.

@Mdaiki0730 Mdaiki0730 merged commit ab7bdcf into dev Feb 12, 2025
15 checks passed
@Mdaiki0730 Mdaiki0730 deleted the feat/eets-blockchain-test branch February 12, 2025 08:05
@github-actions github-actions bot locked and limited conversation to collaborators Feb 12, 2025
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.

7 participants