Skip to content

Conversation

qdm12
Copy link
Contributor

@qdm12 qdm12 commented Apr 3, 2025

Why this should be merged

Critical fixes only from review on #1501

Minor changes can be applied in #1552

How this works

How this was tested

Need to be documented?

Need to update RELEASES.md?

@qdm12 qdm12 force-pushed the qdm12/libevm/final-changes-2 branch from 5c294ec to 6c36ad4 Compare April 3, 2025 08:59
@qdm12 qdm12 changed the title fix(all): critical fixes to libevm branch fix(all): critical fixes to the libevm branch Apr 3, 2025
@qdm12 qdm12 mentioned this pull request Apr 3, 2025
@qdm12 qdm12 force-pushed the qdm12/libevm/final-changes-2 branch from 490753a to 73326c0 Compare April 3, 2025 09:49
@qdm12 qdm12 added the libevm label Apr 3, 2025
@qdm12 qdm12 marked this pull request as ready for review April 3, 2025 10:03
@qdm12 qdm12 requested review from a team, ceyonur and darioush as code owners April 3, 2025 10:03
Copy link
Contributor

@ARR4N ARR4N left a comment

Choose a reason for hiding this comment

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

If these are critical then they need tests, specifically ones that:

  1. Confirm that the ETH upgrades have been applied; and
  2. Verify a ChainConfig with invalid FeeConfig.

@qdm12 qdm12 requested a review from ARR4N April 8, 2025 15:24
@qdm12
Copy link
Contributor Author

qdm12 commented Apr 8, 2025

Tests added:

  • ChainConfig Description() test TestChainConfigDescription
  • ChainConfig Verify test TestChainConfigVerify (Verify coverage, does not test in-depth unlike other existing tests such as precompile ones)
  • Genesis params.SetEthUpgrades test stolen from coreth TestGenesisEthUpgrades and adapted with small changes, with the aim of having small diffs between the 2 repositories.

Copy link
Contributor

@ARR4N ARR4N left a comment

Choose a reason for hiding this comment

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

Approving with the caveat of the comments I've made. This will still need Cey's review as a code owner.

@qdm12 qdm12 merged commit 220454e into libevm Apr 15, 2025
11 checks passed
@qdm12 qdm12 deleted the qdm12/libevm/final-changes-2 branch April 15, 2025 18:19
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.

5 participants