feat(runtime): Polaris Runtime Module #1215
Conversation
WalkthroughThe changes primarily revolve around the integration of the Polaris EVM into the existing system. This includes the creation of a new 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: 2
Files ignored due to filter (2)
- e2e/testapp/go.mod
- go.work.sum
Files selected for processing (9)
- cosmos/runtime/depinject.go (1 hunks)
- cosmos/runtime/runtime.go (1 hunks)
- cosmos/x/evm/genesis_test.go (3 hunks)
- cosmos/x/evm/keeper/genesis.go (2 hunks)
- cosmos/x/evm/keeper/host.go (5 hunks)
- cosmos/x/evm/keeper/keeper.go (2 hunks)
- cosmos/x/evm/keeper/processor.go (1 hunks)
- e2e/testapp/app.go (7 hunks)
- e2e/testapp/polard/cmd/root.go (2 hunks)
Files skipped from review due to trivial changes (3)
- cosmos/x/evm/keeper/genesis.go
- cosmos/x/evm/keeper/host.go
- cosmos/x/evm/keeper/processor.go
Additional comments (Suppressed): 19
cosmos/runtime/runtime.go (5)
44-53: The
Polarisstruct is defined with a pointer topolar.Polaris,EVMKeeper,mmandmp. Ensure that these fields are properly initialized before use to avoid nil pointer dereference errors.55-77: The
Setupfunction initializes thePolarisstruct and its components. It also sets up the precompile contracts and the blockchain. Ensure that theSetupfunction is called before any other operations on thePolarisstruct to avoid runtime errors.79-92: The
Initfunction initializes the services and registers them with Polaris. It is important to ensure that theInitfunction is called afterSetupand before the services are used to avoid runtime errors.94-109: The
RegisterServicesfunction registers services with the EVM networking stack and starts them. Ensure that the services are properly initialized before they are registered and started.111-117: The
LoadLastStatefunction loads the last state of the blockchain. It is important to ensure that the blockchain is properly initialized before this function is called to avoid runtime errors.e2e/testapp/polard/cmd/root.go (2)
64-69: The import
polarruntime "pkg.berachain.dev/polaris/cosmos/runtime"has been added. Ensure that the package exists and is accessible. Also, verify that the functions and types from this package are used correctly in the code.91-97: The
polarruntime.ProvidePolarisRuntimefunction is now being provided as a dependency. Ensure that this function is correctly implemented and that it provides the expected functionality. Also, verify that it does not introduce any breaking changes or unexpected behavior.cosmos/x/evm/genesis_test.go (3)
33-40: The import of
"github.com/ethereum/go-ethereum/consensus/beacon"is new. Ensure that this package is used appropriately in the code and that it doesn't introduce any breaking changes or security vulnerabilities.92-101: The
k.SetBlockchain(core.NewChain(k.Host, beacon.NewFaker()))is a new addition. Ensure that thebeacon.NewFaker()is suitable for this context and doesn't introduce any unexpected behavior. Also, verify that theSetBlockchainmethod is correctly implemented.118-145: The calls to
k.Host.GetBlockPlugin()andk.Host.GetStatePlugin()have replaced the previous calls tok.Polaris().Host().GetBlockPlugin()andk.Polaris().Host().GetStatePlugin(). Ensure that these changes are intentional and that they don't break any existing functionality. Also, verify that theHostmethod is correctly implemented.cosmos/x/evm/keeper/keeper.go (2)
21-52: The
Keeperstruct has been refactored to include achainfield of typeBlockchainand aSetBlockchainmethod. Thechainfield replaces thepolarisfield from the old hunk. Ensure that all references topolarisin the codebase have been updated tochain. Also, theHostfield is now a pointer, which could potentially introduce null pointer exceptions if not handled properly. Make sure to check for null before dereferencingHost.71-88: The
NewKeeperfunction no longer initializes apolarisinstance and does not handle any errors during initialization. This could potentially lead to runtime errors if thechainfield is not properly set before it's used. Consider adding error handling or checks to ensure thatchainis properly initialized before use.The
Loggerfunction remains unchanged and is approved.e2e/testapp/app.go (7)
42-47: The import of
sdk "github.com/cosmos/cosmos-sdk/types"has been removed. Ensure that this does not affect any part of the code that might be using it.53-60: The import of
"github.com/ethereum/go-ethereum/node","pkg.berachain.dev/polaris/cosmos/lib/tx","pkg.berachain.dev/polaris/cosmos/miner","pkg.berachain.dev/polaris/cosmos/txpool","pkg.berachain.dev/polaris/cosmos/x/evm/keeper", and"pkg.berachain.dev/polaris/cosmos/x/evm/types"have been removed. Ensure that this does not affect any part of the code that might be using them.90-96: The
EVMKeeper,mm(miner), andmp(mempool) fields have been replaced with a singlepolarisfield of type*polarruntime.Polaris. Ensure that all references to the removed fields have been updated accordingly.118-139: The
appis now initialized withpolaris: &polarruntime.Polaris{}. TheProvidePolarisRuntimefunction is now provided todepinject.Provide. Ensure that this does not affect the initialization of the application.178-200: The
EVMKeeperis now accessed throughapp.polaris.EVMKeeper. TheSetupPrecompilesandInitmethods have been replaced withapp.polaris.Setup(app.BaseApp). TheSetMempoolandSetPrepareProposalmethods have been removed. Ensure that these changes do not affect the functionality of the application.206-220: The
LoadLastStatemethod is now called onapp.polarisinstead ofapp.EVMKeeper.Polaris().Blockchain(). Ensure that this does not affect the loading of the last state.259-272: The
Initmethod is now called onapp.polarisinstead ofapp.mmandapp.mp. TheRegisterServicesmethod has been removed. TheClosemethod is now called onapp.polarisinstead ofapp.EVMKeeper.Polaris(). Ensure that these changes do not affect the initialization and closing of the application.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1215 +/- ##
==========================================
+ Coverage 48.61% 48.68% +0.06%
==========================================
Files 77 78 +1
Lines 4669 4667 -2
==========================================
+ Hits 2270 2272 +2
+ Misses 2236 2232 -4
Partials 163 163
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Files ignored due to filter (10)
- cosmos/go.mod
- cosmos/go.sum
- e2e/localnet/go.mod
- e2e/localnet/go.sum
- e2e/precompile/go.mod
- e2e/precompile/go.sum
- e2e/testapp/go.sum
- eth/go.mod
- eth/go.sum
- go.work.sum
Files selected for processing (1)
- cosmos/runtime/runtime.go (1 hunks)
Additional comments (Suppressed): 1
cosmos/runtime/runtime.go (1)
- 55-77: The
Setupmethod is well structured and handles errors properly. However, it's not clear whyp.EVMKeeper.SetBlockchain(p.Blockchain())is marked for deprecation. If it's not needed, consider removing it. If it's needed for now but will be removed in the future, provide a comment explaining why.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Files selected for processing (10)
- contracts/scripts/DeployAndCallERC20.sol (1 hunks)
- cosmos/runtime/depinject.go (1 hunks)
- cosmos/x/evm/keeper/abci.go (1 hunks)
- cosmos/x/evm/module.go (3 hunks)
- cosmos/x/evm/plugins/state/plugin.go (5 hunks)
- e2e/testapp/app_config.go (1 hunks)
- eth/core/chain_resources.go (1 hunks)
- eth/log/imported.go (1 hunks)
- eth/polar/backend.go (1 hunks)
- eth/polar/config.go (3 hunks)
Files skipped from review due to trivial changes (3)
- eth/core/chain_resources.go
- eth/log/imported.go
- eth/polar/config.go
Additional comments (Suppressed): 11
eth/polar/backend.go (1)
- 134-140: The new hunk introduces a safety message that warns the user if they are running with development configurations. This is a good practice as it helps prevent accidental use of development configurations in a production environment. However, ensure that the
SafetyMessagefunction does not halt the execution or throw an error that is not handled.cosmos/x/evm/module.go (2)
43-50: The
AppModulestruct now implements theappmodule.HasPrepareCheckStateinterface. Ensure that thePrepareCheckStatemethod is correctly implemented in theAppModulestruct and that it behaves as expected.128-130: The
PrepareCheckStatemethod is added to theAppModulestruct. This method calls thePrepareCheckStatemethod of thekeeperfield of theAppModulestruct. Ensure that thePrepareCheckStatemethod of thekeeperis correctly implemented and that it behaves as expected.+func (am AppModule) PrepareCheckState(ctx context.Context) error { + return am.keeper.PrepareCheckState(ctx) +}e2e/testapp/app_config.go (1)
- 126-128: The
evmtypes.ModuleNamehas been added to thePrepareCheckStaterslist. This change implies that the EVM module will now be checked before each block is processed. Ensure that the EVM module'sPrepareCheckStatefunction is implemented correctly and that it does not introduce any performance issues due to additional checks.contracts/scripts/DeployAndCallERC20.sol (2)
27-46: The contract
DeployAndCallERC20has been significantly modified. TheSolmateERC20contract is no longer being used, and instead, a new contractConsumeGasis being instantiated. The loop now runs 10 times instead of 66, and theconsumeGasmethod of theConsumeGascontract is being called instead of themintmethod of theSolmateERC20contract. Ensure that these changes are intentional and that they do not break any dependencies or expectations in the rest of the codebase.38-38: The
ConsumeGascontract is being instantiated without any constructor arguments. If theConsumeGascontract requires any arguments for its constructor, this will result in a compilation error. Please verify if any arguments are needed.cosmos/x/evm/plugins/state/plugin.go (5)
115-120: The
latestStatefield has been added to thepluginstruct. This field is of typesdk.Contextand is used to store the latest context state. This change could affect the behavior of the plugin, especially in methods that rely on the context state. Ensure that thelatestStatefield is properly initialized and updated throughout the plugin's lifecycle.141-145: The
Preparemethod has been modified. The new implementation assigns the unwrapped context tolatestState. This change could affect the behavior of the plugin, especially in methods that rely on the context state. Ensure that thelatestStatefield is properly initialized and updated throughout the plugin's lifecycle.- p.ctx = sdk.UnwrapSDKContext(ctx). - WithKVGasConfig(storetypes.GasConfig{}). - WithTransientKVGasConfig(storetypes.GasConfig{}) + p.latestState = sdk.UnwrapSDKContext(ctx)
199-201: The
Finalizemethod has been added to thepluginstruct. This method calls theFinalizemethod on theControllerfield of thepluginstruct. Ensure that theControllerfield is properly initialized before this method is called to avoid a nil pointer dereference.544-550: The
ifcondition has been updated to compareint64Numberwithp.latestState.BlockHeight()instead ofp.ctx.BlockHeight(). This change could affect the behavior of the plugin, especially in methods that rely on the context state. Ensure that thelatestStatefield is properly initialized and updated throughout the plugin's lifecycle.- if int64Number >= p.ctx.BlockHeight() { + if int64Number >= p.latestState.BlockHeight() {
- 563-565: The
ifcondition has been updated to checkp.latestState.MultiStore()instead ofp.ctx.MultiStore(). This change could affect the behavior of the plugin, especially in methods that rely on the context state. Ensure that thelatestStatefield is properly initialized and updated throughout the plugin's lifecycle.- if p.ctx.MultiStore() != nil { + if p.latestState.MultiStore() != nil {
itsdevbear
force-pushed
the
polaris-runtime
branch
from
15b1648 to
e2f8f06
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 5
Files ignored due to filter (12)
- contracts/go.sum
- cosmos/go.mod
- cosmos/go.sum
- e2e/localnet/go.mod
- e2e/localnet/go.sum
- e2e/precompile/go.mod
- e2e/precompile/go.sum
- e2e/testapp/go.mod
- e2e/testapp/go.sum
- eth/go.mod
- eth/go.sum
- go.work.sum
Files selected for processing (7)
- contracts/scripts/DeployAndCallERC20.sol (1 hunks)
- contracts/scripts/DeployAndConsumeGas.sol (1 hunks)
- cosmos/x/evm/keeper/abci.go (1 hunks)
- cosmos/x/evm/keeper/keeper.go (2 hunks)
- cosmos/x/evm/keeper/processor.go (1 hunks)
- cosmos/x/evm/module.go (3 hunks)
- e2e/testapp/app_config.go (2 hunks)
Files skipped from review due to trivial changes (2)
- contracts/scripts/DeployAndCallERC20.sol
- cosmos/x/evm/keeper/abci.go
Additional comments (Suppressed): 12
e2e/testapp/app_config.go (2)
123-131: The
evmtypes.ModuleNamehas been added to thePrepareCheckStaterslist. Ensure that the EVM module is prepared to handle check state operations and that this change doesn't introduce any unexpected behavior.143-148: The
evmtypes.ModuleNamehas been added to thePrecommiterslist. Make sure that the EVM module is ready to handle precommit operations and that this change doesn't introduce any unexpected behavior.cosmos/x/evm/module.go (3)
21-28: No significant changes in the import section. The context package has been added, which is used in the new methods.
43-51: The
AppModulestruct now implements two additional interfaces:appmodule.HasPrepareCheckStateandappmodule.HasPrecommit. Ensure that theAppModulestruct has thePrepareCheckStateandPrecommitmethods as required by these interfaces.125-135: Two new methods
PrepareCheckStateandPrecommithave been added to theAppModulestruct. These methods call the corresponding methods on thekeeperfield of theAppModulestruct. Ensure that thekeeperfield has these methods and that they are implemented correctly.cosmos/x/evm/keeper/keeper.go (5)
39-43: The
Blockchaininterface is introduced here. Ensure that all methods required by this interface are implemented by any struct that uses it. Also, consider adding comments to describe what each method in the interface does.45-53: The
Keeperstruct has been refactored. Thepolarisfield has been replaced with achainfield of typeBlockchain. TheHostfield is now a pointer. Ensure that these changes do not break any existing functionality that relies on theKeeperstruct.72-78: The
NewKeeperfunction has been updated to reflect the changes in theKeeperstruct. Ensure that all calls to this function have been updated accordingly.82-84: The
SetBlockchainmethod has been added to theKeeperstruct. This method allows setting thechainfield of theKeeperstruct. Ensure that this method is called before any operations that require thechainfield.87-89: The
Loggermethod remains unchanged. It returns a module-specific logger.cosmos/x/evm/keeper/processor.go (2)
41-41: The TODO comment suggests a potential simplification of the gas consumption logic. If the block gas limit is known and fixed, consuming it upfront could simplify the code and improve performance by avoiding the need to track and consume gas usage during block execution. However, this would also remove the ability to refund unused gas. Consider the trade-offs and decide whether to implement this change.
63-63: The comment suggests that the
PreparePluginsmethod should be moved to theBlockchainstruct. If this method is more related to the blockchain than to theKeeperstruct, moving it could improve the code's organization and clarity. However, this would require changes to theBlockchaininterface and potentially to other parts of the codebase. Consider the impact of this change on the overall system architecture and maintainability.
Summary by CodeRabbit
runtimethat provides a functionProvidePolarisRuntimefor creating a new Polaris runtime.Keeperstruct in thecosmos/x/evm/keeperpackage to include achainfield of typeBlockchainand removed several methods.PrecommitandPrepareCheckStatefunctions to theKeeperstruct in thecosmos/x/evm/keeperpackage.AppModulestruct in thecosmos/x/evmpackage to includePrepareCheckStateandPrecommitmethods.DeployAndConsumeGasin thecontracts/scriptsdirectory that consumes gas in a loop.MakeAppConfigfunction in thee2e/testapppackage to includeevmtypes.ModuleNamein thePrepareCheckStatersandPrecommiterslists.eth/polar/config.gofile.