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

Open
wants to merge 28 commits into
base: dev
Choose a base branch
from

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 EOA check and CodeFormat for SCA when setting genesis code in Prague fork.
    • 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
31 tasks
@Mdaiki0730 Mdaiki0730 marked this pull request as ready for review January 29, 2025 03:43
@Mdaiki0730 Mdaiki0730 requested a review from ian0371 as a code owner January 29, 2025 03:43
@Mdaiki0730 Mdaiki0730 requested review from shiki-tak and ulbqb January 29, 2025 03:59
@ulbqb
Copy link
Contributor

ulbqb commented Jan 29, 2025

Please describe impact to the main logic (IsPrecompiledContractAddress and BeforeApplyMessage).

@Mdaiki0730
Copy link
Contributor Author

Please describe impact to the main logic (IsPrecompiledContractAddress and BeforeApplyMessage).

@ulbqb I added it to PR's description, please check it!

@ulbqb
Copy link
Contributor

ulbqb commented Jan 30, 2025

@Mdaiki0730
How about describing it in another section? It's not that all modification affects the main logic.

@Mdaiki0730
Copy link
Contributor Author

That's good. Changed it to new section.

@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
blockchain/vm/evm.go Outdated Show resolved Hide resolved
return nil
}

// validateStorage validates storage while considering the difference between Kana and Ethereum.
Copy link
Contributor

@shiki-tak shiki-tak Jan 31, 2025

Choose a reason for hiding this comment

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

nit: What is Kana ? Kaia ?

Mdaiki0730 and others added 2 commits January 31, 2025 11:30
Co-authored-by: Lewis <lewis.kim@kaia.io>
@blukat29
Copy link
Contributor

blukat29 commented Feb 3, 2025

Let's merge it after rc.3.

@blukat29 blukat29 added the do not merge Do not merge just yet label Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Do not merge just yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants