refactor: fully decouple evm from evmd #229

zsystm · 2025-06-18T17:30:37Z

Description

This PR finalizes the decoupling between the evm and evmd packages by ensuring a strict one-way dependency from evmd → evm, and eliminating all reverse dependencies.

Key Changes

Moved shared logic: Files previously located in evmd but used by evm were moved into appropriate evm subpackages.
Relocated evmd-specific tests: tests that directly rely on evmd internals and not using Network harness which is used from integraion tests were moved into the evmd package.
Interface-based refactoring: Tests that unnecessarily depended on evmd were updated to use the EvmApp interface, allowing them to remain within evm while preserving abstraction.
Updated Makefile: Adjusted build and install targets to reflect the relocation of cmd/ under evmd.

Motivation

Enforces a clean architectural boundary between evm (library) and evmd (application).
Enables evm to be consumed independently by downstream chains.
Reduces coupling, simplifies maintainability, and aligns with long-term modularization goals.

Closes: #211

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

tackled an existing issue or discussed with a team member
left instructions on how to review the changes
targeted the main branch

Reviewers Checklist

All items are required.
Please add a note if the item is not applicable
and please add your handle next to the items reviewed
if you only reviewed selected items.

I have...

added a relevant changelog entry to the Unreleased section in CHANGELOG.md
confirmed all author checklist items have been addressed
confirmed that this PR does not change production code
reviewed content
tested instructions (if applicable)
confirmed all CI checks have passed

- Make gas limit configurable for testing lower gas limit scenario - Get ibctesting.AppCreator when create coordinator, so other applications can leverage this test suits. - Make GenerateContractCallArgs as separate function. This doens't have to be a member method of TxFactory implementation. un-necessary dependency. - SendEvmTx now supports contract creation.

Previous ibc test suite uses different bond denom ("stake) with EVM denom ("aatom"). Even though it could be different technically, but we need to make sure everything works correctly with default config (bond denom == evm denom). Wit this commit, we can make sure IBC and ICS20 transfers with default config should work well. Updated test util for writing test cases more easily.

Currently, Arguments.Copy in geth panics if there are type mismatch. Returning error instead makes more sense

- Moved shared test/data files from evmd to evm to eliminate evm → evmd dependency - Relocated evmd-bound tests that directly depend on App implementation into evmd/ - Refactored remaining evmd-dependent tests to use EvmApp interface instead - Updated Makefile to reflect cmd relocation under evmd/

zsystm · 2025-06-19T00:43:14Z

@vladjdk
This PR is currently based on the #219 because that work already includes relevant refactoring (e.g., using EvmApp instead of evmd in IBC test setup). I wanted to avoid duplicating those changes.

I'll mark the PR as Ready for Review once #219 is merged.

…-evmd-and-evm-dependency # Conflicts: # evmd/go.mod # evmd/go.sum # evmd/tests/ibc/helper.go # evmd/tests/integration/create_app.go # go.mod # tests/integration/precompiles/ics20/test_integration.go # tests/integration/precompiles/ics20/test_query.go # tests/integration/precompiles/ics20/test_setup.go # tests/integration/precompiles/ics20/test_tx.go

Variables in evmd_config.go are defined for serving evmd

vladjdk

Overall, I would prefer to have app-specific configs (and the ante, somewhat) contained within EVMD. If we need to duplicate them for testing, we should do that. I mostly don't feel comfortable having the configs and options as dependencies that people could use within Cosmos EVM. They likely won't be entirely conforming to the specs each chain have.

vladjdk · 2025-06-19T18:03:21Z

config/chain_id.go

I would prefer that we have two copies of these (evm_app_options, constants, config, activators). One in the test files, and one in evmd. The reason is that these are app-specific config options and it should be clear that any chain implementation needs to implement their own. If we include it in EVMD, then evmd-specific configs will be available via the package, which is confusing imo

Totally agree — that's a great point, thanks for highlighting it.
I've placed the test-related configs under testutil/config, and the ones used by evmd under cmd/evmd/config.
This should help prevent any confusion for client chains integrating with evm.

vladjdk · 2025-06-19T18:04:14Z

config/evmd_config.go

Likewise, an evmd config shouldn't be in the cosmos/evm files

Absolutely — I’ve moved the evmd-specific configs out of cosmos/evm and into cmd/evmd/config, where they belong.
This separation should help maintain a clean boundary between the core evm package and app-specific configurations.

vladjdk · 2025-06-19T18:04:37Z

evmd/test_helpers.go

 	appOptions[server.FlagInvCheckPeriod] = invCheckPeriod

-	app := NewExampleApp(log.NewNopLogger(), db, nil, true, appOptions, evmChainID, EvmAppOptions, baseapp.SetChainID(chainID))
+	app := NewExampleApp(log.NewNopLogger(), db, nil, true, appOptions, evmChainID, evmconfig.EvmAppOptions, baseapp.SetChainID(chainID))


App options should be within the evmd files

Yes — I’ve moved the app options into cmd/evmd/config as suggested.
Thanks again for pointing this out!

vladjdk · 2025-06-19T19:03:08Z

ante/ante.go

I think these ante handlers should also remain in evmd - they're custom to every chain and I don't think they should be set as a default

Good point — I’ve moved the ante handlers under evmd/ante since they’re specific to the app and shouldn’t be treated as defaults. Thanks for the suggestion!

…arts Moved evmd-specific app configurations under the evmd/ directory to improve modularity and maintain clearer separation between evmd and other components. Introduced corresponding configuration files under testutil/ to support test applications, since relocating the original files required new equivalents for testing purposes.

tests/integration/eip712/test_eip712.go

tests/integration/precompiles/bech32/test_bech32.go

tests/integration/precompiles/bech32/test_methods.go

tests/integration/precompiles/distribution/test_event.go

vladjdk

lgtm, nice work

* tests: add ics20 precompile tests and refactor test utils - Make gas limit configurable for testing lower gas limit scenario - Get ibctesting.AppCreator when create coordinator, so other applications can leverage this test suits. - Make GenerateContractCallArgs as separate function. This doens't have to be a member method of TxFactory implementation. un-necessary dependency. - SendEvmTx now supports contract creation. * fix ibc test suite and ics20 tx vulnerability Previous ibc test suite uses different bond denom ("stake) with EVM denom ("aatom"). Even though it could be different technically, but we need to make sure everything works correctly with default config (bond denom == evm denom). Wit this commit, we can make sure IBC and ICS20 transfers with default config should work well. Updated test util for writing test cases more easily. * add revert test case * fix lint * implement safeCopyInputs and add query test case Currently, Arguments.Copy in geth panics if there are type mismatch. Returning error instead makes more sense * refactor: decouple evm from evmd and relocate evmd-specific tests - Moved shared test/data files from evmd to evm to eliminate evm → evmd dependency - Relocated evmd-bound tests that directly depend on App implementation into evmd/ - Refactored remaining evmd-dependent tests to use EvmApp interface instead - Updated Makefile to reflect cmd relocation under evmd/ * fix ci and update test-helper's config path * resolve conflicts after merge upstream * nit: changed filename Variables in evmd_config.go are defined for serving evmd * chore: update importing package name * Refactor: relocate evmd-specific configurations and add test counterparts Moved evmd-specific app configurations under the evmd/ directory to improve modularity and maintain clearer separation between evmd and other components. Introduced corresponding configuration files under testutil/ to support test applications, since relocating the original files required new equivalents for testing purposes. * Refactor: relocate app specific ante handler * fix solidity test config path * chore: update import package name

zsystm added 7 commits June 16, 2025 23:41

add revert test case

563623a

fix lint

dd1401e

implement safeCopyInputs and add query test case

65997cc

Currently, Arguments.Copy in geth panics if there are type mismatch. Returning error instead makes more sense

fix ci and update test-helper's config path

f7bd682

zsystm force-pushed the refactor/decoupling-evmd-and-evm-dependency branch from e8d99b7 to f7bd682 Compare June 18, 2025 19:07

zsystm added 4 commits June 20, 2025 01:06

resolve conflicts after merge upstream

ca195e0

nit: changed filename

4704a1f

Variables in evmd_config.go are defined for serving evmd

chore: update importing package name

34bfcc3

zsystm force-pushed the refactor/decoupling-evmd-and-evm-dependency branch from dc33d3d to 34bfcc3 Compare June 19, 2025 16:22

zsystm marked this pull request as ready for review June 19, 2025 16:35

zsystm requested a review from vladjdk June 19, 2025 16:35

vladjdk requested changes Jun 19, 2025

View reviewed changes

zsystm force-pushed the refactor/decoupling-evmd-and-evm-dependency branch from af0de7a to f46992e Compare June 20, 2025 10:45

github-advanced-security bot found potential problems Jun 20, 2025

View reviewed changes

zsystm added 2 commits June 20, 2025 20:00

Refactor: relocate app specific ante handler

7d9b1e7

fix solidity test config path

322afeb

zsystm force-pushed the refactor/decoupling-evmd-and-evm-dependency branch from 30dd79d to 322afeb Compare June 20, 2025 12:43

chore: update import package name

c86da46

zsystm requested a review from vladjdk June 20, 2025 13:14

vladjdk approved these changes Jun 23, 2025

View reviewed changes

vladjdk merged commit fb7e407 into cosmos:main Jun 23, 2025
17 checks passed

refactor: fully decouple evm from evmd #229

refactor: fully decouple evm from evmd #229

Uh oh!

Conversation

zsystm commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Key Changes

Motivation

Author Checklist

Reviewers Checklist

Uh oh!

zsystm commented Jun 19, 2025

Uh oh!

vladjdk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vladjdk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zsystm commented Jun 18, 2025 •

edited

Loading