refactor(config): remove evmAppOptions entirely #661

vladjdk · 2025-09-24T21:14:02Z

closes: #617

todos for this PR before close:

migration guide addition
changelog entry

consolidates these constants into one file in /config

the test was using a duplicate eips multiplier value instead of the global one moved to config

…idate into /config

…config-refactor

app options is removed from new app creation. however, we're getting a recovered panic on *restarting* the chain due to prepareproposal running before preblocker: +0x120\ngithub.202132.xyz/cosmos/cosmos-sdk/baseapp.(*BaseApp).PrepareProposal.func1()\n\tgithub.202132.xyz/cosmos/[email protected]/baseapp/abci.go:433 +0x38\npanic({0x1061c8160?, 0x10912faa0?})\n\truntime/panic.go:783 +0x120\ngithub.202132.xyz/cosmos/evm/x/vm/types.GetEVMCoinDecimals(...)\n\tgithub.202132.xyz/cosmos/[email protected]/x/vm/types/denom_config.go:61\ngithub.202132.xyz/cosmos/evm/x/vm/types.ConvertAmountTo18DecimalsLegacy(...)\n\tgithub.202132.xyz/cosmos/[email protected]/x/vm/types/scaling.go:17\ngithub.202132.xyz/cosmos/evm/x/vm/wrappers.FeeMarketWrapper.GetBaseFee({{_, _}}, {{0x10698a810, 0x109225540}, {0x1069b51c0, 0x14001f4c200}, {{0x0, 0x0}, {0x16dc0f689, 0x4}, ...}, ...})\n\tgithub.202132.xyz/cosmos/[email protected]/x/vm/wrappers/feemarket.go:38 +0x68\ngithub.202132.xyz/cosmos/evm/x/vm/keeper.Keeper.GetBaseFee({{0x1069b4890, 0x14001eddeb0}, {0x10695e0b0, 0x14003560450}, {0x10695e240, 0x14003560490}, 0x140035637a0, {0x1400189d3e0, 0x14, 0x20}, ...}, ...)\n\tgithub.202132.xyz/cosmos/[email protected]/x/vm/keeper/keeper.go:355 +0xe4\ngithub.202132.xyz/cosmos/evm/mempool.(*ExperimentalEVMMempool).getIterators(0x14003cbd180, {0x10698a848?, 0x14002c39c08?}, {0x109225540, 0x0, 0x0})\n\tgithub.202132.xyz/cosmos/[email protected]/mempool/mempool.go:439 +0x20c\ngithub.202132.xyz/cosmos/evm/mempool.(*ExperimentalEVMMempool).SelectBy(0x14003cbd180, {0x10698a848?, 0x14002c39c08?}, {0x109225540?, 0x14000085ce8?, 0x104cce034?}, 0x140044cd540)\n\tgithub.202132.xyz/cosmos/[email protected]/mempool/mempool.go:370 +0xe0\ngithub.202132.xyz/cosmos/cosmos-sdk/types/mempool.SelectBy({0x10698a848?, 0x14002c39c08?}, {0x10698b108?, 0x14003cbd180?}, {0x109225540?, 0x0?, 0x0?}, 0x140044cd540)\n\tgithub.202132.xyz/cosmos/[email protected]/types/mempool/mempool.go:58 +0xa0\ngithub.202132.xyz/cosmos/evm/evmd.NewExampleApp.(*DefaultProposalHandler).PrepareProposalHandler.func3({{0x10698a810, 0x109225540}, {0x1069b51c0, 0x14001f4c200}, {{0x0, 0x0}, {0x16dc0f689, 0x4}, 0xd, {0x3507e5b0, ...}, ...}, ...}, ...)\n\tgithub.202132.xyz/cosmos/[email protected]/baseapp/abci_utils.go:294 +0x250\ngithub.202132.xyz/cosmos/cosmos-sdk/baseapp.(*BaseApp).PrepareProposal(0x14002d32488, 0x140015f42c0)\n\tgithub.202132.xyz/cosmos/[email protected]/baseapp/abci.go:439

the mempool needed the global variable, but that wasn't set, so we set it in the vm keeper storage and use it from there in the mempool everything else still uses the global variable, but we now have a foundation to move it over to the state storage

we were getting the 6 decimal balance when we were operating in 18-balance territory. now that we have a separation in the ibc tests, we should check balances accordingly

tests/integration/x/vm/keeper_test_suite.go

tests/integration/x/vm/test_grpc_query.go

…ad/remove-app-options

tests/integration/x/vm/keeper_test_suite.go

 	configurator.ResetTestConfig()
-	err := configurator.
-		WithChainConfig(chainConfig).
+	err := evmtypes.SetChainConfig(chainConfig)


tests/integration/x/vm/test_grpc_query.go

 				configurator.ResetTestConfig()
-				err := configurator.
-					WithChainConfig(chainConfig).
+				err := types.SetChainConfig(chainConfig)


The best way to fix this problem is to remove the unreachable statement. Since CodeQL reports the line err := types.SetChainConfig(chainConfig) as unreachable, the fix is to delete this line, as it cannot execute in the current control flow. The edit should be constrained to the region starting on line 1607 within the function-literal block for the test case titled "pass - nil Base Fee when london hardfork not activated". No other changes are necessary if no code depends on the value of err further down in this block (verified here).

tests/integration/x/vm/test_grpc_query.go

 			s.Require().NoError(s.Network.NextBlock())
 			configurator := types.NewEVMConfigurator()
 			configurator.ResetTestConfig()
+			err = types.SetChainConfig(chainConfig)


To fix the unreachable statement, we should ensure that test cleanup/configuration code (such as resetting global configuration with SetChainConfig and configurator.Configure()) is reliably executed regardless of whether earlier Require() assertions pass or fail. The best way is to move these statements into a defer block at the start of the subtest invocation function. This way, cleanup is run even if an assertion fails.

Specifically, inside the subtest closure (the argument to s.Run(tc.name, func() {...})), move the configurator reset logic and the SetChainConfig call into a single defer block placed at the very beginning of the closure. This requires no new imports, only restructuring of existing code.

cloudgray

Overall, LGTM — I just left a few minor comments and questions.

It’s nice and clean now that CoinInfo is being stored in the Keeper.

evmd/cmd/evmd/cmd/root.go

evmd/app.go

testutil/ibc/chain.go

x/precisebank/module.go

x/vm/types/config.go

x/vm/types/config_testing.go

x/vm/types/denom.go

This reverts commit cbbaa44.

vladjdk added 23 commits September 23, 2025 17:10

move tests to remove circular dependency

ac47fa4

remove ante2

463ad28

Merge branch 'vlad/remove-evmd-dep' into vlad/config-refactor

73f5395

wip: move all config files out of evmd

c3dc99e

wip: add todos

9b9dd66

reset config to fix test

dc32dbb

fix evmd tests by adding resets to the evmappoptions

019f3d8

remove testutil/config constants

3aff36b

consolidates these constants into one file in /config

fix eips test

567764f

the test was using a duplicate eips multiplier value instead of the global one moved to config

remove all configs except for genesis from testutil/config and consol…

91d432c

…idate into /config

lints and merge main

2b13bcf

Merge branch 'main' into vlad/config-refactor

6bee431

changelogchangelogchangelogchangelogchangelog

dda4aac

Merge remote-tracking branch 'origin/vlad/config-refactor' into vlad/…

e050fc1

…config-refactor

lints

94fc72a

remove debugging artifact

87904e3

artifact removal 2

bedbd6d

fix unit tests

799db42

wip: fix ibc testing

5bd1add

delete all evmappoptions from tests

f16467c

fix ibc precompile integration test balance getters

b9fb2a7

we were getting the 6 decimal balance when we were operating in 18-balance territory. now that we have a separation in the ibc tests, we should check balances accordingly

vladjdk force-pushed the vlad/remove-app-options branch from 1b76be4 to b9fb2a7 Compare September 25, 2025 19:05

vladjdk added 2 commits September 25, 2025 22:06

Merge remote-tracking branch 'origin/main' into vlad/remove-app-options

77a30c2

fix all evmd tests

71b7722

github-advanced-security bot found potential problems Sep 26, 2025

View reviewed changes

tests/integration/x/vm/keeper_test_suite.go Fixed Show fixed Hide fixed

tests/integration/x/vm/test_grpc_query.go Fixed Show fixed Hide fixed

tests/integration/x/vm/test_grpc_query.go Fixed Show fixed Hide fixed

vladjdk added 3 commits September 26, 2025 14:11

fix precisebank keeper test

f8c0743

fix smore tests

de569ea

lol

0eeb6ec

vladjdk added 2 commits October 8, 2025 13:03

Merge remote-tracking branch 'origin/vlad/remove-app-options' into vl…

6efae1b

…ad/remove-app-options

Remove test* denoms and replace with default*

bb572eb

vladjdk force-pushed the vlad/remove-app-options branch from c6e2c56 to bb572eb Compare October 8, 2025 17:37

vladjdk mentioned this pull request Oct 8, 2025

Standardize Bond Denom and Base Denom #693

Open

vladjdk added 8 commits October 8, 2025 13:43

extract var

af5caaa

match vars

e8ead26

remove chain config from configurator

cc38dfb

undo uint8 change

2d283fa

comment fixes

97e8c38

Merge remote-tracking branch 'origin/main' into vlad/remove-app-options

51e5e51

Auto stash before merge of "vlad/remove-app-options" and "origin/main"

0d932c0

set chain config in vm integ tests

a16bba8

github-advanced-security bot found potential problems Oct 8, 2025

View reviewed changes

vladjdk added 2 commits October 8, 2025 14:23

lint

949698c

fix error on test

1a51a6f

cloudgray reviewed Oct 9, 2025

View reviewed changes

vladjdk added 6 commits October 9, 2025 11:34

fix denoms for ibc chain

cbbaa44

Revert "fix denoms for ibc chain"

0ed773a

This reverts commit cbbaa44.

use app options chain id instead of passing param

0b44e5b

lints

e56e942

revert make race

8d966ff

Merge branch 'main' into vlad/remove-app-options

303f101

almk-dev mentioned this pull request Oct 9, 2025

refactor(config): Expose mempool configuration flags and move mempool app configuration to helper #698

Open

3 tasks

vladjdk enabled auto-merge October 9, 2025 15:47

almk-dev approved these changes Oct 9, 2025

View reviewed changes

vladjdk added this pull request to the merge queue Oct 9, 2025

Merged via the queue into main with commit 18163be Oct 9, 2025
23 checks passed

vladjdk deleted the vlad/remove-app-options branch October 9, 2025 16:09

almk-dev restored the vlad/remove-app-options branch October 9, 2025 18:24

@@ -1604,7 +1604,6 @@
             				configurator := types.NewEVMConfigurator()
             				configurator.ResetTestConfig()
-            				err := types.SetChainConfig(chainConfig)
             				s.Require().NoError(err)
             				err = configurator.
             					WithEVMCoinInfo(testconstants.ExampleChainCoinInfo[testconstants.ExampleChainID]).

@@ -1642,6 +1642,17 @@
             	for _, tc := range testCases {
             		s.Run(tc.name, func() {
+            			// Always run configurator reset/SetChainConfig on subtest exit
+            			defer func() {
+            				configurator := types.NewEVMConfigurator()
+            				configurator.ResetTestConfig()
+            				err := types.SetChainConfig(chainConfig)
+            				s.Require().NoError(err)
+            				err = configurator.
+            					WithEVMCoinInfo(coinInfo).
+            					Configure()
+            				s.Require().NoError(err)
+            			}()
             			// Set necessary params
             			tc.setParams()
             			// Get the expected response
@@ -1659,14 +1670,6 @@
             				s.Require().Error(err)
             			}
             			s.Require().NoError(s.Network.NextBlock())
-            			configurator := types.NewEVMConfigurator()
-            			configurator.ResetTestConfig()
-            			err = types.SetChainConfig(chainConfig)
-            			s.Require().NoError(err)
-            			err = configurator.
-            				WithEVMCoinInfo(coinInfo).
-            				Configure()
-            			s.Require().NoError(err)
             		})
             	}
             }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(config): remove evmAppOptions entirely #661

refactor(config): remove evmAppOptions entirely #661

vladjdk commented Sep 24, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check warning

Copilot Autofix

Check warning

Copilot Autofix

Check warning

Copilot Autofix

cloudgray left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

refactor(config): remove evmAppOptions entirely #661

refactor(config): remove evmAppOptions entirely #661

Conversation

vladjdk commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check warning

Copilot Autofix

Check warning

Copilot Autofix

Check warning

Copilot Autofix

cloudgray left a comment

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 commented Sep 24, 2025 •

edited

Loading