fix(chain): Fake FinalizeBlock in PrepareProposal #1272
Conversation
WalkthroughThe changes primarily revolve around the refactoring of state management in the Ethereum core. The Changes
Poem
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
Configuration used: CodeRabbit UI
Files selected for processing (2)
- eth/core/chain.go (3 hunks)
- eth/core/chain_writer.go (5 hunks)
Additional comments: 7
eth/core/chain.go (3)
33-38: The import statement for the "pkg.berachain.dev/polaris/eth/core/state" package has been removed. This indicates that the "state" package is no longer used in this file. Ensure that this change does not break any functionality.
67-72: The "statedb" field of type "state.StateDB" has been removed from the "blockchain" struct. This suggests a change in how state management is handled. Make sure that the state management functionality is correctly implemented elsewhere.
125-130: The "statedb" field is no longer initialized in the "NewChain" function. This is consistent with the removal of the "statedb" field from the "blockchain" struct. Ensure that the state management functionality is correctly implemented elsewhere.
eth/core/chain_writer.go (4)
46-50: The
WriteGenesisBlockfunction now creates a newstate.StateDBobject and passes it toWriteBlockAndSetHead. This is a significant change as it means a new state is created for each block, which could improve the efficiency and accuracy of state management.63-84: The
InsertBlockAndSetHeadfunction creates a newstate.StateDBobject and passes it toProcessandValidateState. It also passes thestateobject toWriteBlockAndSetHead. This change is consistent with the new approach of creating a new state for each block.90-98: The
WriteBlockAndSetHeadfunction now takes astate.StateDBparameter and passes it towriteBlockWithState. This change is consistent with the new approach of creating a new state for each block.176-182: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [159-182]
The
writeBlockWithStatefunction now takes astate.StateDBparameter and uses it to commit the state changes. This change is consistent with the new approach of creating a new state for each block.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cosmos/x/evm/plugins/state/plugin.go (1 hunks)
Additional comments: 1
cosmos/x/evm/plugins/state/plugin.go (1)
- 140-146: The
Preparefunction has been updated to set thelatestQueryContextfield with a modifiedsdk.UnwrapSDKContextvalue. The modified value includes the addition ofWithKVGasConfigandWithTransientKVGasConfigmethods, which set the gas configuration for the context. Ensure that this change does not affect the functionality of thePreparefunction and that the gas configuration is correctly applied.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1272 +/- ##
=======================================
Coverage 48.73% 48.73%
=======================================
Files 84 84
Lines 4838 4838
=======================================
Hits 2358 2358
Misses 2306 2306
Partials 174 174 |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (2)
- cosmos/x/evm/plugins/state/plugin.go (2 hunks)
- eth/core/chain_writer.go (5 hunks)
Additional comments: 4
eth/core/chain_writer.go (4)
46-50: The
WriteGenesisBlockfunction now creates a newstate.StateDBobject and passes it toWriteBlockAndSetHead. This is a good practice as it ensures that each block has its own state, reducing the risk of state corruption.60-90: The
InsertBlockAndSetHeadfunction now creates a newstate.StateDBobject and passes it toProcessandValidateState. This is a good practice as it ensures that each block has its own state, reducing the risk of state corruption.95-104: The
WriteBlockAndSetHeadfunction now takes astate.StateDBparameter and passes it towriteBlockWithState. This is a good practice as it ensures that each block has its own state, reducing the risk of state corruption.163-169: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [163-188]
The
writeBlockWithStatefunction now takes astate.StateDBparameter and uses it to commit state changes. This is a good practice as it ensures that each block has its own state, reducing the risk of state corruption.
cosmos/x/evm/plugins/state/plugin.go
Outdated
| // Prepare sets up the context on the state plugin for use in JSON-RPC calls. | ||
| // Prepare implements `core.StatePlugin`. | ||
| func (p *plugin) Prepare(ctx context.Context) { | ||
| p.latestQueryContext = sdk.UnwrapSDKContext(ctx) | ||
| p.latestQueryContext, _ = sdk.UnwrapSDKContext(ctx). | ||
| WithKVGasConfig(storetypes.GasConfig{}). | ||
| WithTransientKVGasConfig(storetypes.GasConfig{}).CacheContext() | ||
| } |
There was a problem hiding this comment.
The error returned by CacheContext() is ignored. It's a good practice to handle all errors to prevent unexpected behavior.
- p.latestQueryContext, _ = sdk.UnwrapSDKContext(ctx).
+ p.latestQueryContext, err = sdk.UnwrapSDKContext(ctx).
+ if err != nil {
+ // handle error
+ }Commitable suggestion (Beta)
| // Prepare sets up the context on the state plugin for use in JSON-RPC calls. | |
| // Prepare implements `core.StatePlugin`. | |
| func (p *plugin) Prepare(ctx context.Context) { | |
| p.latestQueryContext = sdk.UnwrapSDKContext(ctx) | |
| p.latestQueryContext, _ = sdk.UnwrapSDKContext(ctx). | |
| WithKVGasConfig(storetypes.GasConfig{}). | |
| WithTransientKVGasConfig(storetypes.GasConfig{}).CacheContext() | |
| } | |
| // Prepare sets up the context on the state plugin for use in JSON-RPC calls. | |
| // Prepare implements `core.StatePlugin`. | |
| func (p *plugin) Prepare(ctx context.Context) { | |
| p.latestQueryContext, err := sdk.UnwrapSDKContext(ctx) | |
| if err != nil { | |
| // handle error | |
| } | |
| p.latestQueryContext = p.latestQueryContext. | |
| WithKVGasConfig(storetypes.GasConfig{}). | |
| WithTransientKVGasConfig(storetypes.GasConfig{}).CacheContext() | |
| } |
cosmos/x/evm/plugins/state/plugin.go
Outdated
| if p.latestQueryContext.MultiStore() == nil { | ||
| ctx = p.latestQueryContext.WithEventManager(sdk.NewEventManager()) | ||
| } else { | ||
| ctx, _ = p.latestQueryContext.CacheContext() | ||
| ctx, _ = p.latestQueryContext.WithEventManager(sdk.NewEventManager()).CacheContext() | ||
| } |
There was a problem hiding this comment.
The error returned by CacheContext() is ignored. It's a good practice to handle all errors to prevent unexpected behavior.
- ctx, _ = p.latestQueryContext.WithEventManager(sdk.NewEventManager()).CacheContext()
+ ctx, err = p.latestQueryContext.WithEventManager(sdk.NewEventManager()).CacheContext()
+ if err != nil {
+ // handle error
+ }Commitable suggestion (Beta)
| if p.latestQueryContext.MultiStore() == nil { | |
| ctx = p.latestQueryContext.WithEventManager(sdk.NewEventManager()) | |
| } else { | |
| ctx, _ = p.latestQueryContext.CacheContext() | |
| ctx, _ = p.latestQueryContext.WithEventManager(sdk.NewEventManager()).CacheContext() | |
| } | |
| if p.latestQueryContext.MultiStore() == nil { | |
| ctx = p.latestQueryContext.WithEventManager(sdk.NewEventManager()) | |
| } else { | |
| ctx, err := p.latestQueryContext.WithEventManager(sdk.NewEventManager()).CacheContext() | |
| if err != nil { | |
| // handle error | |
| } | |
| } |
itsdevbear
changed the title
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cosmos/miner/miner.go (2 hunks)
Additional comments: 2
cosmos/miner/miner.go (2)
87-89: Ensure that all calls to this function throughout the codebase have been updated to match the new signature.
97-103: The
PreBlockermethod is now called with a modified argument&abci.RequestFinalizeBlockthat includes fields from thereqargument. This change modifies the behavior of the function and requires a review.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cosmos/miner/miner.go (2 hunks)
Additional comments: 2
cosmos/miner/miner.go (2)
85-91: The
PrepareProposalfunction now takes an additional argumentreq *abci.RequestPrepareProposal. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.97-104: The
PreBlockermethod is now called with a modified argument&abci.RequestFinalizeBlockthat includes fields from thereqargument. This change modifies the behavior of the function by passing additional information to thePreBlockermethod. Ensure that thePreBlockermethod can handle these additional fields correctly.
Summary by CodeRabbit
Refactor:
eth/core/chain.go, indicating a refactoring of state management functionality.eth/core/chain_writer.goto create a newstate.StateDBobject withinWriteGenesisBlockandInsertBlockAndSetHeadfunctions, enhancing the modularity of state management.WriteBlockAndSetHeadandwriteBlockWithStatefunctions to accept astate.StateDBparameter, improving the flexibility of state changes commitment.These changes streamline the state management process, potentially improving the performance and maintainability of the code.