chore(app): Setup app better. #1224
Conversation
WalkthroughThis changeset primarily focuses on refactoring and reorganizing the Polaris runtime and EVM Keeper setup. It includes the removal of deprecated methods, addition of new functions, and modifications to existing ones. The changes aim to improve the code's maintainability, modularity, and performance. Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (3)
- cosmos/runtime/runtime.go (4 hunks)
- e2e/testapp/app.go (7 hunks)
- e2e/testapp/polard/cmd/root.go (2 hunks)
Files skipped from review due to trivial changes (1)
- e2e/testapp/polard/cmd/root.go
Additional comments (Suppressed): 12
cosmos/runtime/runtime.go (4)
31-44: The import statements have been updated. The
engineppackage has been removed and theconfig,core, andethlogpackages have been added. Ensure that these changes do not break any dependencies and that the new packages are used correctly in the code.54-87: The
Newfunction has been introduced to create a new Polaris runtime. It creates a new Geth networking stack and initializes a new Polaris instance with this networking stack. The function also sets up a logger for the Polaris Geth module. Ensure that theNewfunction is used correctly throughout the codebase and that the logger is functioning as expected.99-104: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [89-103]
The
Setupfunction has been modified. The call toEVMKeeper.SetupPrecompileshas been removed, and thePolaris.Initmethod is now called to set up the Polaris struct. TheWrappedMinerfield is initialized with a new miner, and thePrepareProposalmethod of theWrappedMinerfield is set as the prepare proposal function for the base app. Ensure that the removal ofEVMKeeper.SetupPrecompilesdoes not affect the functionality of the application and that the new setup process works as expected.
- 119-125: The
RegisterServicesfunction has been updated. The call toEVMKeeper.Host.GetEnginePlugin().(enginep.Plugin).Start(clientContext)has been removed, and the services are now registered with the Polaris struct directly. Ensure that the removal of the engine plugin start does not affect the functionality of the application and that the services are registered correctly.e2e/testapp/app.go (8)
57-62: The new import statements for
evmkeeperandengineppackages are introduced. Ensure these packages are available and correctly imported.74-80: The
SimAppstruct now includes aPolarisfield. This change might affect the usage ofSimAppin other parts of the codebase. Please verify.95-99: The
EVMKeeperfield is added to theSimAppstruct. Ensure that this field is correctly initialized and used.121-134: The
appinitialization has been modified. Thepolarisfield is no longer initialized here. Ensure that this change does not affect the application's behavior.180-209: The
app.Polarisfield is now initialized using thepolarruntime.Newfunction. Theapp.EVMKeeperis also set up here. Ensure that these changes are correct and do not introduce any issues.224-230: The
app.Polaris.LoadLastStatefunction call has been updated to use the newapp.Polarisfield. Ensure that this change is correct.274-279: The
app.EVMKeeper.Host.GetEnginePlugin().(enginep.Plugin).Start(apiSvr.ClientCtx)line starts the engine plugin. Also, theapp.Polaris.Initfunction call has been updated to use the newapp.Polarisfield. Ensure that these changes are correct.282-287: The
Closemethod now usesapp.Polaris. Ensure that this change is correct and does not introduce any issues.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1224 +/- ##
==========================================
+ Coverage 48.68% 48.90% +0.21%
==========================================
Files 78 78
Lines 4667 4644 -23
==========================================
- Hits 2272 2271 -1
+ Misses 2232 2210 -22
Partials 163 163
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (35)
- contracts/bindings/cosmos/lib/cosmos_types.abigen.go (1 hunks)
- contracts/bindings/testing/consume_gas.abigen.go (1 hunks)
- contracts/bindings/testing/distribution_testing_helper.abigen.go (1 hunks)
- contracts/bindings/testing/governance/governance_wrapper.abigen.go (1 hunks)
- contracts/bindings/testing/liquid_staking.abigen.go (1 hunks)
- contracts/bindings/testing/precompile_constructor.abigen.go (1 hunks)
- contracts/bindings/testing/solmate_erc20.abigen.go (1 hunks)
- cosmos/config/mocks/app_options.go (1 hunks)
- cosmos/txpool/mocks/geth_tx_pool.go (1 hunks)
- cosmos/txpool/mocks/lifecycle.go (1 hunks)
- cosmos/txpool/mocks/sdk_tx.go (1 hunks)
- cosmos/txpool/mocks/subscription.go (1 hunks)
- cosmos/txpool/mocks/tx_broadcaster.go (1 hunks)
- cosmos/txpool/mocks/tx_serializer.go (1 hunks)
- cosmos/txpool/mocks/tx_sub_provider.go (1 hunks)
- cosmos/x/evm/genesis_test.go (1 hunks)
- cosmos/x/evm/keeper/genesis.go (1 hunks)
- cosmos/x/evm/keeper/host.go (6 hunks)
- cosmos/x/evm/plugins/historical/historical_data.go (1 hunks)
- cosmos/x/evm/plugins/historical/plugin.go (3 hunks)
- cosmos/x/evm/plugins/historical/plugin_test.go (2 hunks)
- eth/core/chain.go (5 hunks)
- eth/core/chain_writer.go (1 hunks)
- eth/core/host.go (2 hunks)
- eth/core/state/journal/mocks/accesslist.go (1 hunks)
- eth/core/state/journal/mocks/log.go (1 hunks)
- eth/core/state/journal/mocks/refund.go (1 hunks)
- eth/core/state/journal/mocks/self_destruct_state_plugin.go (1 hunks)
- eth/core/state/journal/mocks/self_destructs.go (1 hunks)
- eth/core/state/journal/mocks/transient_storage.go (1 hunks)
- eth/core/state/mocks/plugin.go (1 hunks)
- eth/core/state/mocks/polar_state_db.go (1 hunks)
- eth/core/state/mocks/precompile_plugin.go (1 hunks)
- eth/core/types/transaction.rlpgen.go (1 hunks)
- eth/polar/backend.go (2 hunks)
Files not summarized due to errors (2)
- contracts/bindings/testing/governance/governance_wrapper.abigen.go: Error: Message exceeds token limit
- contracts/bindings/testing/liquid_staking.abigen.go: Error: Message exceeds token limit
Files skipped from review due to trivial changes (27)
- contracts/bindings/cosmos/lib/cosmos_types.abigen.go
- contracts/bindings/testing/distribution_testing_helper.abigen.go
- contracts/bindings/testing/precompile_constructor.abigen.go
- contracts/bindings/testing/solmate_erc20.abigen.go
- cosmos/config/mocks/app_options.go
- cosmos/txpool/mocks/geth_tx_pool.go
- cosmos/txpool/mocks/lifecycle.go
- cosmos/txpool/mocks/sdk_tx.go
- cosmos/txpool/mocks/subscription.go
- cosmos/txpool/mocks/tx_broadcaster.go
- cosmos/txpool/mocks/tx_serializer.go
- cosmos/txpool/mocks/tx_sub_provider.go
- eth/core/chain.go
- eth/core/chain_writer.go
- eth/core/host.go
- eth/core/state/journal/mocks/accesslist.go
- eth/core/state/journal/mocks/log.go
- eth/core/state/journal/mocks/refund.go
- eth/core/state/journal/mocks/self_destruct_state_plugin.go
- eth/core/state/journal/mocks/self_destructs.go
- eth/core/state/journal/mocks/transient_storage.go
- eth/core/state/mocks/plugin.go
- eth/core/state/mocks/polar_state_db.go
- eth/core/state/mocks/precompile_plugin.go
- eth/core/types/transaction.rlpgen.go
- contracts/bindings/testing/governance/governance_wrapper.abigen.go (diff too large)
- contracts/bindings/testing/liquid_staking.abigen.go (diff too large)
Additional comments (Suppressed): 16
cosmos/x/evm/genesis_test.go (1)
- 93-97: The
core.NewChainfunction now requires an additional argumentparams.DefaultChainConfig. Ensure that this change doesn't break any existing calls to this function and that theparams.DefaultChainConfigis the correct configuration to use in this context.cosmos/x/evm/plugins/historical/historical_data.go (1)
- 188-188: The function
DeriveReceiptsFromBlocknow usesp.chainConfiginstead ofp.cp.ChainConfig(). Ensure thatp.chainConfigis correctly initialized and updated whenever necessary, and that it is thread-safe if accessed concurrently.contracts/bindings/testing/consume_gas.abigen.go (1)
- 34-36: The
Binfield, which represents the compiled bytecode of the contract, has been updated. This indicates that the contract's code has been modified. Ensure that these changes are intentional and that they have been thoroughly tested. Also, verify that the ABI (Application Binary Interface) still correctly describes the contract's interface after the changes.cosmos/x/evm/keeper/genesis.go (1)
- 30-35: The removal of the line
genState.Config = k.Host.GetConfigurationPlugin().ChainConfig()suggests that the chain configuration is no longer being set in theInitGenesisfunction. Ensure that the chain configuration is being set elsewhere in the codebase to avoid potential issues.- genState.Config = k.Host.GetConfigurationPlugin().ChainConfig()cosmos/x/evm/plugins/historical/plugin_test.go (2)
34-40: The import of the
paramspackage is new in this hunk. Ensure that this package is used in the subsequent code and that it is necessary for the new implementation.50-56: The
NewPluginfunction now takesparams.DefaultChainConfigas the first argument instead ofcp(ConfigurationPlugin). Make sure that this change is reflected in all calls toNewPluginthroughout the codebase. Also, verify thatparams.DefaultChainConfigprovides the necessary configuration for the plugin.- p = utils.MustGetAs[*plugin](NewPlugin(cp, bp, nil, testutil.EvmKey)) + p = utils.MustGetAs[*plugin](NewPlugin(params.DefaultChainConfig, bp, nil, testutil.EvmKey))cosmos/x/evm/plugins/historical/plugin.go (2)
43-47: The
cpfield has been replaced withchainConfigof type*params.ChainConfig. Ensure that all references tocpin the codebase have been updated tochainConfigand that the type change does not introduce any issues.54-62: The
NewPluginfunction now requires a*params.ChainConfigargument instead of acore.ConfigurationPlugin. Ensure that all calls toNewPluginhave been updated to match the new signature.- func NewPlugin(cp core.ConfigurationPlugin, bp core.BlockPlugin, _ storetypes.StoreKey, storekey storetypes.StoreKey) Plugin { + func NewPlugin(chainConfig *params.ChainConfig, bp core.BlockPlugin, _ storetypes.StoreKey, storekey storetypes.StoreKey) Plugin {eth/polar/backend.go (2)
109-115: The
core.NewChainfunction now requires aChainConfigargument. Ensure that theconfig.Chainfield is properly initialized before this call. Also, verify that all calls tocore.NewChainthroughout the codebase have been updated to match the new signature.156-159: The
miner.Newfunction now usespl.config.Chaininstead ofpl.host.GetConfigurationPlugin().ChainConfig(). Ensure that thepl.config.Chainfield is equivalent to the previousChainConfigobtained from the configuration plugin. If not, this could lead to unexpected behavior in the miner.cosmos/x/evm/keeper/host.go (6)
28-33: The import of the
configurationplugin has been removed. Ensure that this plugin is no longer needed and that its functionality has been replaced or is no longer required.43-48: The
configuration.Pluginhas been removed from theHoststruct. This change is consistent with the removal of theconfigurationplugin import. Ensure that this does not affect the functionality of theHoststruct.70-75: The
configuration.Pluginis no longer being initialized in theHoststruct. This is consistent with the removal of theconfigurationplugin import and its removal from theHoststruct. Ensure that this does not affect the functionality of theHoststruct.78-84: The
historical.Pluginis now being initialized with&cfg.Polar.Chaininstead ofh.cp. This change is consistent with the removal of theconfigurationplugin. Ensure that&cfg.Polar.Chainprovides the same or equivalent functionality ash.cp.105-110: The
GetConfigurationPluginfunction has been removed. This is consistent with the removal of theconfigurationplugin. Ensure that this function is no longer needed and that its functionality has been replaced or is no longer required.125-129: The
configuration.Pluginhas been removed from the return value of theGetAllPluginsfunction. This is consistent with the removal of theconfigurationplugin. Ensure that this does not affect the functionality of theGetAllPluginsfunction.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (9)
- cosmos/runtime/runtime.go (3 hunks)
- cosmos/x/evm/keeper/host.go (5 hunks)
- cosmos/x/evm/plugins/block/header.go (2 hunks)
- cosmos/x/evm/plugins/block/header_test.go (1 hunks)
- cosmos/x/evm/plugins/block/plugin.go (2 hunks)
- eth/core/chain.go (6 hunks)
- eth/core/chain_resources.go (2 hunks)
- eth/crypto/imported.go (1 hunks)
- eth/params/imported.go (1 hunks)
Files skipped from review due to trivial changes (3)
- cosmos/x/evm/plugins/block/header.go
- eth/core/chain.go
- eth/params/imported.go
Additional comments (Suppressed): 16
eth/core/chain_resources.go (2)
26-30: The import statement for "pkg.berachain.dev/polaris/eth/params" has been added. Ensure that this new import does not introduce any conflicts with existing code and is used appropriately in the subsequent code.
35-39: A new method
Config() *params.ChainConfighas been added to theChainResourcesinterface. Make sure all structs that implement this interface are updated to include this new method.+ Config() *params.ChainConfigcosmos/x/evm/plugins/block/plugin.go (2)
37-41: The
SetQueryContextFnmethod has been removed from thePlugininterface. Ensure that this change does not affect any other parts of the codebase that were using this method.53-62: The
NewPluginfunction now takes an additional parameterqfnwhich is a function that returns another function for querying block headers. This replaces theSetQueryContextFnmethod that was previously part of thePlugininterface. Ensure that all calls toNewPluginhave been updated to match the new signature.- func NewPlugin(storekey storetypes.StoreKey, sk StakingKeeper) Plugin { + func NewPlugin( + storekey storetypes.StoreKey, sk StakingKeeper, + qfn func() func(height int64, prove bool) (sdk.Context, error), + ) Plugin {cosmos/x/evm/plugins/block/header_test.go (1)
- 52-53: The
SetQueryContextFnfunction has been removed and replaced with agetQueryContextfield in thepluginstruct. Ensure that this change does not affect the functionality of the plugin and that all calls toSetQueryContextFnhave been updated.eth/crypto/imported.go (1)
- 25-39: The new hunk removes several functions that were previously exposed by the
cryptopackage. Ensure that these functions are not used elsewhere in the codebase, or that appropriate replacements have been provided. The removed functions are:SigToPub,Ecrecover,CreateAddress,UnmarshalPubkey,CompressPubkey,DecompressPubkey,DigestLength,EthSign,FromECDSA,ValidateSignatureValues,SignatureLength,ToECDSA,VerifySignature,FromECDSAPub.cosmos/x/evm/keeper/host.go (5)
29-33: The import statement for the
configurationplugin has been removed. Ensure that this plugin is no longer needed in the codebase.43-55: The
Hoststruct has been modified. Thecpfield (configuration plugin) andstoreKeyfield have been removed. Ensure that these fields are not used elsewhere in the codebase.68-86: The
NewHostfunction has been updated. Theconfigurationplugin andstoreKeyare no longer used in the creation of theHoststruct. Thestateplugin'sSetQueryContextFnis now set here. Ensure that these changes do not affect the functionality of theHoststruct.104-115: The
GetConfigurationPluginfunction has been removed. Ensure that this function is not called elsewhere in the codebase.126-130: The
GetAllPluginsfunction no longer returns theconfigurationplugin. Ensure that this change does not affect the functionality of theHoststruct.cosmos/runtime/runtime.go (5)
45-51: The
EVMKeeperfield has been removed from thePolarisstruct. Ensure that all references toEVMKeeperin the codebase have been updated or removed accordingly.55-83: The
Newfunction has been introduced to create a newPolarisruntime. It seems to handle the creation of a new Polaris instance with a networking stack and logging. However, it doesn't handle the initialization ofWrappedMinerandWrappedTxPoolwhich were previously part of thePolarisstruct. Make sure these components are initialized elsewhere in the codebase.86-88: The
Setupfunction no longer callsSetupPrecompileson theEVMKeeper. If this is intentional, ensure that precompiles are set up elsewhere in the codebase.95-98: The
Setupfunction no longer sets the blockchain for theEVMKeeper. If this is intentional, ensure that the blockchain is set elsewhere in the codebase.118-121: The
RegisterServicesfunction no longer requires aclientContextargument and no longer starts the engine plugin. If this is intentional, ensure that the engine plugin is started elsewhere in the codebase and that the removal of theclientContextargument does not affect the registration of services.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Files selected for processing (4)
- cosmos/runtime/runtime.go (2 hunks)
- cosmos/x/evm/keeper/genesis.go (1 hunks)
- cosmos/x/evm/keeper/keeper.go (3 hunks)
- e2e/testapp/app.go (7 hunks)
Files skipped from review due to trivial changes (2)
- cosmos/runtime/runtime.go
- cosmos/x/evm/keeper/genesis.go
Additional comments (Suppressed): 7
cosmos/x/evm/keeper/keeper.go (2)
35-46: The
Blockchaininterface has been updated to include a new methodConfig() *params.ChainConfig. Ensure that all implementations of this interface have been updated to include this method.84-87: The
Setupmethod has been added to theKeeperstruct. This method sets up the blockchain and precompiles. Ensure that this method is called before using theKeeperinstance.e2e/testapp/app.go (5)
57-61: The import
evmkeeper "pkg.berachain.dev/polaris/cosmos/x/evm/keeper"has been added. Ensure that this package is available and accessible.73-79: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [73-98]
The
SimAppstruct has been modified. The*polarruntime.Polarisfield has been added and thepolarisfield has been replaced withEVMKeeper *evmkeeper.Keeper. Make sure that all references to these fields in the codebase are updated accordingly.
120-133: The
appinitialization inNewPolarisApp()function has been changed. Thepolarisfield is no longer initialized here. Also, thedepinject.Provide()function no longer includespolarruntime.ProvidePolarisRuntime. Ensure that these changes do not affect the application's behavior.179-200: The
app.Polarisandapp.EVMKeeperfields are now being initialized and set up within theNewPolarisApp()function. Theapp.SetAnteHandler(nil)line has been removed. Make sure that the AnteHandler is set elsewhere if needed.212-218: The
LoadLastState()method is now called onapp.Polarisinstead ofapp.polaris. Ensure that thePolarisfield is correctly initialized before this call.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Files selected for processing (3)
- cosmos/runtime/runtime.go (2 hunks)
- cosmos/x/evm/keeper/host.go (5 hunks)
- eth/polar/backend.go (3 hunks)
Additional comments (Suppressed): 9
eth/polar/backend.go (2)
109-115: The
core.NewChainfunction now takes an additional parameter&config.Chain. Ensure that this change doesn't break the function's expected behavior and that theconfig.Chainis correctly initialized before this call.152-153: The
miner.Newfunction now takes&pl.config.Chaininstead ofpl.host.GetConfigurationPlugin().ChainConfig(). Ensure thatpl.config.Chainis equivalent topl.host.GetConfigurationPlugin().ChainConfig()and that this change doesn't affect the function's expected behavior.cosmos/x/evm/keeper/host.go (5)
28-33: The
configurationplugin import has been removed. Ensure that this does not affect any dependencies or functionality that relied on it.43-55: The
configuration.Pluginfield has been removed from theHoststruct. This change should be verified across the codebase to ensure that no other parts of the code are trying to access this field.65-85: The
historical.NewPluginfunction call has been updated to pass the&cfg.Polar.Chainparameter instead ofh.cp. Ensure that this change is compatible with the function's signature and expected behavior.103-114: The
GetConfigurationPluginmethod has been removed from theHoststruct. This change should be verified across the codebase to ensure that no other parts of the code are trying to access this method.125-129: The
configurationplugin has been removed from theGetAllPluginsmethod. Ensure that this does not affect any dependencies or functionality that relied on it.cosmos/runtime/runtime.go (2)
90-106: The
Setupmethod has been updated to initialize theWrappedTxPoolandWrappedMinerfields and set them as the mempool and prepare proposal function for the base app respectively. It also calls theSetupmethod of theEVMKeeperinterface. TheEVMKeeperfield has been removed from thePolarisstruct and is now passed as a parameter to theSetupmethod. Ensure that all calls to this method throughout the codebase have been updated to match the new signature.125-128: The
RegisterServicesmethod no longer starts the engine plugin. Ensure that the engine plugin is started elsewhere in the codebase, or if it's no longer needed, that its removal doesn't cause any issues.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (1)
- cosmos/x/evm/keeper/processor.go (2 hunks)
Additional comments (Suppressed): 2
cosmos/x/evm/keeper/processor.go (2)
35-45: The variable
envelopehas been moved from the old hunk's line 50 to the new hunk's line 41. This change is acceptable as it doesn't affect the logic of the code. However, it's important to ensure that theenvelopevariable is not used before its declaration and initialization.50-60: The error handling for the
envelope.UnmarshalJSON(msg.Data)andengine.ExecutableDataToBlock(*envelope.ExecutionPayload, nil, nil)functions has been modified. In the new hunk, the error is directly checked in the if condition, which makes the code more concise and readable. This is a good practice as it reduces the number of lines of code and improves readability.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (13)
- cosmos/runtime/logging.go (1 hunks)
- cosmos/runtime/runtime.go (3 hunks)
- cosmos/x/evm/keeper/host.go (5 hunks)
- cosmos/x/evm/keeper/keeper.go (3 hunks)
- cosmos/x/evm/plugins/block/plugin.go (2 hunks)
- cosmos/x/evm/plugins/precompile/plugin_test.go (1 hunks)
- cosmos/x/evm/plugins/state/genesis_test.go (1 hunks)
- cosmos/x/evm/plugins/state/plugin.go (6 hunks)
- cosmos/x/evm/plugins/state/plugin_benchmark_test.go (1 hunks)
- cosmos/x/evm/plugins/state/plugin_test.go (1 hunks)
- e2e/testapp/app.go (7 hunks)
- eth/core/host.go (2 hunks)
- eth/core/mock/block_plugin.mock.go (7 hunks)
Files skipped from review due to trivial changes (4)
- cosmos/runtime/logging.go
- cosmos/x/evm/keeper/host.go
- eth/core/host.go
- eth/core/mock/block_plugin.mock.go
Additional comments (Suppressed): 32
cosmos/x/evm/plugins/state/plugin_benchmark_test.go (1)
- 41-46: The function signature of
state.NewPluginhas changed. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the twonilarguments passed tostate.NewPluginin line 43 are intentional and won't cause any issues.cosmos/x/evm/plugins/precompile/plugin_test.go (1)
- 131-134: The
state.NewPluginfunction call has been updated with an additionalnilargument. Ensure that this change is intentional and that the function signature has been updated to accept this additional argument. If the function signature has not been updated, this will cause a runtime error.- return state.NewPlugin( - nil, nil, nil, - ) + return state.NewPlugin( + nil, nil, nil, nil, + )cosmos/x/evm/plugins/state/genesis_test.go (1)
- 47-53: The
NewPluginfunction call has been modified to passnilinstead of&mockPLF{}. Ensure that this change does not affect the functionality of thestate.Plugininstance. If&mockPLF{}was providing any necessary setup or configuration for the plugin, this change could cause issues.- sp = state.NewPlugin(ak, testutil.EvmKey, &mockPLF{}) + sp = state.NewPlugin(ak, testutil.EvmKey, nil, &mockPLF{})cosmos/x/evm/plugins/block/plugin.go (3)
34-37: The
SetQueryContextFnmethod has been removed from thePlugininterface. Ensure that this change does not break any existing functionality that relied on this method.50-58: The
NewPluginfunction now takes an additional parameterqfnwhich is a function that returns another function. This is a significant change and should be verified for correctness and compatibility with existing code. Also, ensure that theqfnfunction is properly defined and used in the codebase.47-48: The
pluginstruct now includes agetQueryContextfield. This field seems to replace theSetQueryContextFnmethod that was removed from thePlugininterface. Ensure that this field is properly initialized and used in the codebase.cosmos/x/evm/plugins/state/plugin_test.go (1)
- 51-55: The
NewPluginfunction call has been modified to remove theSetQueryContextFnfunction and add anilargument. Ensure that this change does not affect the functionality of thestateplugin and that all calls toNewPluginthroughout the codebase have been updated to match the new signature.- sp = state.NewPlugin(ak, testutil.EvmKey, &mockPLF{}) + sp = state.NewPlugin(ak, testutil.EvmKey, nil, &mockPLF{})cosmos/x/evm/keeper/keeper.go (4)
35-39: The import statement for
"pkg.berachain.dev/polaris/eth/params"has been added. Ensure that this package is available and accessible in the project's dependencies.43-43: The
Config()method has been added to theBlockchaininterface. Make sure all implementations of this interface have been updated to include this method.84-87: The
Setupmethod has been added to theKeeperstruct. This method sets up theKeeperinstance with a givenBlockchaininstance and sets up precompiles. Ensure that error handling is properly done in theSetupPrecompilesmethod.89-91: The
StartEnginePluginmethod has been added to theKeeperstruct. This method starts the engine plugin with a given context. Ensure that theStartmethod of the engine plugin handles errors properly and that the context passed is valid.cosmos/x/evm/plugins/state/plugin.go (6)
55-60: The
SetQueryContextFnmethod has been removed from thePlugininterface. Ensure that this change does not affect any other parts of the codebase that rely on this method.105-112: The
getQueryContextfield has been renamed toqfn. This change should not affect the functionality of the code, but make sure to update any references to this field in the rest of the codebase.121-134: The
NewPluginfunction now takes an additional parameterqfn. This change will require updates to all calls toNewPluginthroughout the codebase to include this new parameter.524-535: The
SetQueryContextFnmethod has been removed and its functionality has been integrated into theStateAtBlockNumbermethod. This change simplifies the code by removing the need for a separate method to set the query context function. However, ensure that this change does not affect any other parts of the codebase that rely on theSetQueryContextFnmethod.546-559: The
StateAtBlockNumbermethod now callsNewPluginwith theqfnparameter. This change aligns with the updated signature of theNewPluginfunction.567-573: The
Clonemethod now callsNewPluginwith theqfnparameter. This change aligns with the updated signature of theNewPluginfunction.cosmos/runtime/runtime.go (8)
34-43: The import statements have been updated to remove the
engineppackage and add theconfigandcorepackages. Ensure that these changes do not break any dependencies in the code.45-49: A new interface
EVMKeeperhas been defined. This interface is used in theSetupmethod of thePolarisstruct. Ensure that theSetupmethod of theEVMKeeperinterface is implemented wherever this interface is used.52-60: The
Polarisstruct has been updated. TheEVMKeeperfield has been removed and theWrappedMinerandWrappedTxPoolfields have been added. Ensure that these changes do not break any dependencies in the code.64-76: A new function
Newhas been added to create a newPolarisinstance. This function takes aconfig.Config, alog.Logger, and acore.PolarisHostChainas arguments. It returns a pointer to aPolarisinstance. Ensure that this function is used correctly throughout the codebase.82-97: The
Setupmethod of thePolarisstruct has been updated. TheEVMKeeperfield is no longer used in this method. Instead, anEVMKeeperinterface is passed as an argument. TheSetupmethod of theEVMKeeperinterface is called in this method. Ensure that these changes do not break any dependencies in the code.102-104: The
Initmethod of thePolarisstruct has been updated. TheEVMKeeperfield is no longer used in this method. Ensure that these changes do not break any dependencies in the code.114-123: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [120-130]
The
RegisterServicesfunction has been modified. TheEVMKeeperfield is no longer used in this function. Ensure that these changes do not break any dependencies in the code.
- 135-137: The
LoadLastStatemethod of thePolarisstruct has not been changed. This method loads the last state of the Polaris blockchain.e2e/testapp/app.go (7)
83-87: The
SimAppstruct now includes aPolarisfield. This change seems to be part of a larger refactoring effort to separate the Polaris EVM runtime from the rest of the application. Ensure that all references to thePolarisfield in theSimAppstruct have been updated accordingly.105-107: The
SimAppstruct now includes anEVMKeeperfield. This field seems to be part of the effort to separate the Polaris EVM runtime from the rest of the application. Ensure that all references to theEVMKeeperfield in theSimAppstruct have been updated accordingly.121-132: The
NewPolarisAppfunction has been modified. Theappvariable is now initialized without thepolarisfield. This change is likely part of the effort to separate the Polaris EVM runtime from the rest of the application. Ensure that all calls to theNewPolarisAppfunction have been updated accordingly.178-195: The
NewPolarisAppfunction now initializes thePolarisfield of theappstruct and calls theSetupmethod ofapp.Polaris. This change is likely part of the effort to separate the Polaris EVM runtime from the rest of the application. Ensure that all calls to theNewPolarisAppfunction have been updated accordingly.213-217: The
LoadLastStatemethod is now called on thePolarisfield of theappstruct. This change is likely part of the effort to separate the Polaris EVM runtime from the rest of the application. Ensure that all calls to theLoadLastStatemethod have been updated accordingly.262-266: The
StartEnginePluginmethod is now called on theEVMKeeperfield of theappstruct, and theInitmethod is now called on thePolarisfield of theappstruct. These changes are likely part of the effort to separate the Polaris EVM runtime from the rest of the application. Ensure that all calls to theStartEnginePluginandInitmethods have been updated accordingly.270-274: The
Closemethod now checks if thePolarisfield of theappstruct is notnilbefore calling itsClosemethod. This change is likely part of the effort to separate the Polaris EVM runtime from the rest of the application. Ensure that all calls to theClosemethod have been updated accordingly.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (10)
- cosmos/runtime/runtime.go (3 hunks)
- cosmos/x/evm/depinject.go (2 hunks)
- cosmos/x/evm/genesis_test.go (5 hunks)
- cosmos/x/evm/interfaces.go (2 hunks)
- cosmos/x/evm/keeper/host.go (4 hunks)
- cosmos/x/evm/keeper/keeper.go (4 hunks)
- cosmos/x/evm/plugins/block/header_test.go (1 hunks)
- cosmos/x/evm/plugins/block/plugin.go (2 hunks)
- cosmos/x/evm/plugins/historical/plugin.go (3 hunks)
- e2e/testapp/app.go (7 hunks)
Files skipped from review due to trivial changes (5)
- cosmos/x/evm/depinject.go
- cosmos/x/evm/genesis_test.go
- cosmos/x/evm/interfaces.go
- cosmos/x/evm/keeper/host.go
- cosmos/x/evm/plugins/block/plugin.go
Additional comments (Suppressed): 21
cosmos/x/evm/plugins/historical/plugin.go (3)
35-35: The comment "Plugin is the interface that must be implemented by the plugin." seems to be misplaced or outdated as there is no interface definition in the provided hunk. Please verify if this comment is still relevant or needs to be updated.
42-49: The
cpfield (core.ConfigurationPlugin) has been replaced withchainConfig(*params.ChainConfig). Ensure that this change doesn't affect the functionality of the plugin, as thecpfield was previously used to get the current chain configuration.53-63: The
NewPluginfunction signature has been updated. Thecpparameter (core.ConfigurationPlugin) has been replaced withchainConfig(*params.ChainConfig). Ensure that all calls toNewPluginhave been updated to match the new signature.cosmos/x/evm/plugins/block/header_test.go (1)
- 45-54: The
BeforeEachfunction has been modified. TheSetupMinimalKeepersfunction call has been removed and replaced with a function that returnsmockQueryContext. Ensure that this change does not affect the tests and that themockQueryContextfunction provides the necessary context for the tests.- _, _, _, sk := testutil.SetupMinimalKeepers(log.NewTestLogger(GinkgoT())) - p = utils.MustGetAs[*plugin](NewPlugin(testutil.EvmKey, sk)) - p.SetQueryContextFn( - func() func(height int64, prove bool) (sdk.Context, error) { return mockQueryContext }) + p = utils.MustGetAs[*plugin](NewPlugin(testutil.EvmKey, + func() func(height int64, prove bool) (sdk.Context, error) { return mockQueryContext }))cosmos/x/evm/keeper/keeper.go (5)
40-45: The
Config()method has been added to theBlockchaininterface. Ensure that all implementations of this interface have been updated to include this method.58-63: The
sk block.StakingKeeperparameter has been removed from theNewKeeperfunction. Verify that this change does not affect any calls to this function and that theStakingKeeperis no longer needed.68-73: The
skparameter has been removed from thehost.Newfunction call. Ensure that theStakingKeeperis no longer needed in thehost.Newfunction.81-84: The
Setupmethod has been added to theKeeperstruct. This method sets up the keeper with the provided blockchain and callsSetupPrecompiles. Ensure that this method is called appropriately in the codebase.86-88: The
StartEnginePluginmethod has been added to theKeeperstruct. This method starts the engine plugin. Ensure that this method is called appropriately in the codebase.cosmos/runtime/runtime.go (5)
67-80: The
Newfunction has been introduced to initialize thePolarisstruct. It creates a new Polaris runtime from the provided dependencies. It's a good practice to have a separate function for initialization. However, ensure that all instances where thePolarisstruct is being initialized have been updated to use this new function.82-100: The
Setupfunction has been updated. It now takes anEVMKeeperas an argument and calls itsSetupmethod. This change improves the modularity of the code by allowing different implementations of theEVMKeeperinterface to be used. However, ensure that all calls to theSetupfunction have been updated to pass anEVMKeeperargument.114-129: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [106-120]
The
Initfunction has been updated. It now takes anEVMKeeperas an argument and calls itsStartEnginePluginmethod. This change improves the modularity of the code by allowing different implementations of theEVMKeeperinterface to be used. However, ensure that all calls to theInitfunction have been updated to pass anEVMKeeperargument.
- 114-129: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [126-136]
The
RegisterServicesfunction has been updated. It no longer starts the engine plugin. This change improves the modularity of the code by allowing the engine plugin to be started elsewhere. However, ensure that the engine plugin is still being started somewhere in the code.
- 138-143: No significant changes in the
LoadLastStatefunction. The function signature and implementation remain the same.e2e/testapp/app.go (7)
83-87: The
SimAppstruct now embeds thePolarisstruct from thepolarruntimepackage. This change allows direct access to thePolarisstruct's methods and fields from aSimAppinstance. Ensure that this change does not introduce any naming conflicts or unexpected behavior.102-107: The
EVMKeeperfield has been added to theSimAppstruct. This field is a pointer to anevmkeeper.Keeperinstance, which is used to interact with the EVM (Ethereum Virtual Machine). Ensure that this field is properly initialized before use to prevent nil pointer dereferences.119-132: The
appvariable in theNewPolarisAppfunction no longer initializes thePolarisfield. This field is now initialized later in the function. Ensure that this change does not affect the order of operations or introduce any issues.178-199: The
Polarisfield of theSimAppstruct is now initialized using thepolarruntime.Newfunction, which takes the application's configuration, logger, and EVM host as arguments. TheSetupmethod of thePolarisinstance is then called with theBaseAppandEVMKeeperas arguments. Ensure that these changes align with the intended setup process for the Polaris runtime.211-217: The
LoadLastStatemethod of thePolarisinstance is now called with the application's commit multi-store and the last block height as arguments. Ensure that these changes align with the intended state loading process for the Polaris EVM.258-264: The
Initmethod of thePolarisinstance is now called with the application's client context, logger, and EVM keeper as arguments. Ensure that these changes align with the intended initialization process for the Polaris EVM.266-271: The
Closemethod of theSimAppstruct now checks if thePolarisfield is not nil before calling itsClosemethod. This change prevents a potential nil pointer dereference if thePolarisfield is not initialized. Good practice for error handling.
Summary by CodeRabbit