feat(abci): Utilize geth miner to build blocks
#1165
Conversation
![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: 4
Files ignored due to filter (1)
- e2e/testapp/go.mod
Files selected for processing (5)
- cosmos/miner/miner.go (1 hunks)
- cosmos/txpool/handler.go (1 hunks)
- cosmos/x/evm/keeper/keeper.go (1 hunks)
- e2e/testapp/app.go (4 hunks)
- eth/polar/polaris.go (7 hunks)
Files skipped from review due to trivial changes (1)
- cosmos/x/evm/keeper/keeper.go
Additional comments (Suppressed): 13
e2e/testapp/app.go (4)
59-67: The import
"pkg.berachain.dev/polaris/cosmos/miner"is new and replaces the old"pkg.berachain.dev/polaris/cosmos/abci/prepare". Ensure that all dependencies are correctly managed and that this change does not break any existing functionality.103-109: The
SimAppstruct now includes a*miner.Minerfield, replacing the previous*prepare.Handlerfield. Make sure that all references to these fields in the codebase have been updated accordingly.201-210: The
NewPolarisAppfunction has been updated to initialize theMinerandMempoolinstances and set them as the prepare proposal handler for the base app. This replaces the previous setup of thePreparehandler. Verify that this change aligns with the intended application logic and doesn't introduce any unexpected behavior.323-328: The initialization of services has been updated to include the
Minerinstance, replacing the previousPreparehandler. Ensure that this change is reflected throughout the codebase and doesn't cause any issues with service initialization.cosmos/miner/miner.go (4)
42-46: The
Minerstruct embedsminer.Minerand implements thebaseapp.TxSelectorinterface. It includes a serializer for transactions and a payload that is currently being processed. Ensure that the serializer and payload are properly initialized before use to avoid nil pointer dereference errors.61-70: The
PrepareProposalmethod builds a block of transactions and prepares it for proposal. The error handling here is good, but consider adding more context to the returned error usingfmt.Errorforerrors.Wrapfor easier debugging.75-80: The
buildBlockmethod submits a payload for building and waits for the transactions to resolve from the underlying worker. The use ofdeferto resetm.currentPayloadis a good practice to clean up after the function execution.82-100: The
submitPayloadForBuildingmethod submits a payload for building. It logs the process and handles errors appropriately. However, the TODO comment on line 89 indicates that the payload needs to be properly filled in. Ensure this is addressed before merging.eth/polar/polaris.go (5)
30-44: The new hunk introduces additional imports, including
github.com/ethereum/go-ethereum/minerandpkg.berachain.dev/polaris/eth/consensus. Ensure these packages are used in the code and that they are compatible with the existing codebase.76-98: The
Polarisstruct has been modified to include a new fieldbeaconof typeconsensus.Engine, and theminerfield has been changed fromminer.Minerto*miner.Miner. Thespminerfield of typeoldminer.Minerhas also been added. Make sure all references to these fields in the codebase have been updated accordingly.132-138: The
Init()function now initializespl.spminerinstead ofpl.miner. This change should be reflected whereverInit()is called.152-157: The miner is now initialized with additional parameters including
mux,pl.beacon, andpl.IsLocalBlock. Ensure these parameters are correctly set up before this call.215-238: New getter methods
SPMiner(),MinerChain(), andIsLocalBlock()have been introduced. Verify their usage throughout the codebase.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 6
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.mod
- e2e/testapp/go.sum
- eth/go.mod
- eth/go.sum
Files selected for processing (10)
- cosmos/miner/miner.go (1 hunks)
- cosmos/miner/miner_test.go (1 hunks)
- cosmos/x/evm/keeper/processor.go (1 hunks)
- e2e/testapp/app.go (4 hunks)
- eth/core/chain_reader.go (3 hunks)
- eth/core/chain_writer.go (1 hunks)
- eth/miner/miner.go (1 hunks)
- eth/polar/backend.go (4 hunks)
- eth/polar/mining.go (2 hunks)
- eth/polar/polaris.go (7 hunks)
Files skipped from review due to trivial changes (2)
- eth/core/chain_reader.go
- eth/miner/miner.go
Additional comments (Suppressed): 18
eth/core/chain_writer.go (2)
30-35: The
ChainWriterinterface has been extended with a new methodWriteBlockAndSetHead. Ensure that all implementations of this interface have been updated to include this new method.44-44: The
InsertBlockfunction signature remains unchanged. No action required.cosmos/miner/miner.go (1)
- 42-46: The
Minerstruct is implementing thebaseapp.TxSelectorinterface but it seems like the required methods for this interface are not implemented in the struct. Please ensure that all necessary methods are implemented.eth/polar/mining.go (2)
29-35: The
Preparemethod now uses thespminerobject instead of theminerobject. Ensure that thespminer.Preparemethod has the same behavior as theminer.Preparemethod to avoid unexpected results.40-49: The
ProcessTransactionandFinalizemethods have been updated to use thespminerobject instead of theminerobject. Verify that thespminer.ProcessTransactionandspminer.Finalizemethods behave similarly to their counterparts in theminerobject to maintain consistency.e2e/testapp/app.go (4)
56-64: The import of the package
"pkg.berachain.dev/polaris/cosmos/miner"is new, while the package"pkg.berachain.dev/polaris/cosmos/abci/prepare"has been removed. Ensure that all dependencies on thepreparepackage have been properly refactored to use theminerpackage instead.98-105: The
SimAppstruct now includes aMinerobject (mm) and no longer includes aPreparehandler (pp). This change aligns with the PR summary's mention of replacing calls from theminerobject with those from thespminerobject in thePrepare,ProcessTransaction, andFinalizefunctions. Make sure that this doesn't break any existing functionality that relied on thePreparehandler.198-204: The
Minerobject is being initialized and itsPrepareProposalmethod is being set as the app's prepare proposal function. This replaces the previousPreparehandler. Ensure that theMiner'sPrepareProposalmethod provides equivalent or improved functionality compared to the oldPreparehandler.311-315: The
MinerandMempoolobjects are being initialized with a serializer. This replaces the initialization of thePreparehandler. As before, ensure that this change doesn't negatively impact any functionality that relied on thePreparehandler.cosmos/x/evm/keeper/processor.go (1)
- 46-46: The
Miner().ProcessTransaction()function call has been replaced withSPMiner().ProcessTransaction(). Ensure that the newSPMinerobject and itsProcessTransactionmethod are correctly implemented and tested. Also, verify that all instances ofMiner().ProcessTransaction()in the codebase have been updated toSPMiner().ProcessTransaction().eth/polar/backend.go (4)
229-236: The new hunk has removed the handling of
rpc.PendingBlockNumbercase where it used to return the current block from the blockchain. This might be a breaking change if there are parts of the codebase that rely on this behavior. Please verify.293-303: Similar to the previous comment, the handling of
rpc.PendingBlockNumberinStateAndHeaderByNumberfunction has been changed to return the pending state and header from the miner instead of resolving the block number and returning its state. Ensure that this change is propagated throughout the codebase.362-368: The method
PendingBlockAndReceiptsnow retrieves the pending block and receipts from the miner instead of the blockchain. If other parts of the codebase expect this method to return the pending block and receipts from the blockchain, this could introduce bugs. Please confirm.546-548: The
SubscribePendingLogsEventmethod now subscribes to the miner's pending logs instead of the blockchain's. If there are subscribers expecting logs from the blockchain, this could lead to unexpected behavior. Please check.eth/polar/polaris.go (4)
30-44: The import paths have been updated to include new packages and rename the "miner" package to "oldminer". Ensure that these changes are reflected throughout the codebase and that all dependencies are correctly managed.
85-100: The
Polarisstruct has been modified to include a new fieldspminerof typeoldminer.Minerand the existingminerfield's type has been changed fromminer.Minerto*miner.Miner. Also, a new fieldbeaconof typeconsensus.Enginehas been added. Make sure that these changes do not break any existing functionality or introduce any null pointer dereferences.150-161: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [134-161]
The
Init()function now initializesspminerwitholdminer.New(pl)instead ofminer.New(pl). Themineris also being initialized differently, with additional parameters passed tominer.New(). Ensure that these changes are compatible with the rest of the codebase and that the necessary updates have been made where these functions are called.
- 214-240: New getter methods
SPMiner(),MinerChain(), andIsLocalBlock()have been added to thePolarisstruct. The return type ofMiner()has been changed fromminer.Minerto*miner.Miner. Verify that these changes are consistent with how these methods are used in the rest of the codebase.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 7
Files ignored due to filter (2)
- e2e/testapp/go.sum
- go.work.sum
Files selected for processing (11)
- cosmos/x/evm/keeper/host.go (1 hunks)
- e2e/testapp/app.go (4 hunks)
- eth/consensus/blank_engine.go (1 hunks)
- eth/core/chain.go (2 hunks)
- eth/core/chain_writer.go (1 hunks)
- eth/core/state/imported.go (1 hunks)
- eth/core/state/statedb.go (1 hunks)
- eth/core/types/imported.go (1 hunks)
- eth/miner/miner.go (2 hunks)
- eth/polar/backend.go (4 hunks)
- eth/polar/polaris.go (7 hunks)
Files skipped from review due to trivial changes (2)
- eth/core/state/statedb.go
- eth/core/types/imported.go
Additional comments (Suppressed): 24
eth/miner/miner.go (2)
68-69: The comment suggests renaming the
minerstruct or deprecating it. Ensure that this TODO is addressed or tracked in your project management tool to avoid leaving loose ends in the codebase.125-131: The
Preparemethod has been modified to reset the state plugin before preparing other plugins. This change could potentially affect the behavior of thePreparemethod, especially if any of the other plugins depend on the state of thespplugin before it was reset. Verify that this change does not introduce any unintended side effects.eth/core/chain_writer.go (1)
- 30-35: The
ChainWriterinterface has been extended with a new methodWriteBlockAndSetHead. Ensure that all implementations of this interface in the codebase have been updated to include this new method.e2e/testapp/app.go (4)
59-64: The import of the
minerpackage and removal of thepreparepackage aligns with the PR summary. Ensure that all dependencies are correctly managed.98-105: The
Minerstruct is replacing theHandlerstruct from thepreparepackage. Make sure that theMinerstruct implements all necessary methods and functionalities previously provided by theHandlerstruct.200-206: The
PrepareProposalmethod of theMinerstruct is now being used instead of thePrepareProposalmethod of theHandlerstruct from thepreparepackage. Verify that the new method provides the same functionality and doesn't introduce any breaking changes.313-317: Initialization of the
MinerandMempoolstructs has replaced the initialization of theHandlerstruct from thepreparepackage. Confirm that this change doesn't affect the application's behavior.eth/core/chain.go (2)
23-32: The import of
"math/big"is new in this hunk. Ensure that it's used in the code not shown in these hunks.120-129: The
currentBlockfield of theblockchainstruct is now being initialized with a new block having number and base fee as 0, instead ofnil. This change could have implications on how the current block is handled elsewhere in the codebase. Make sure all such instances are properly handled to accommodate this change.- bc.currentBlock.Store(nil) + bc.currentBlock.Store( + types.NewBlock(&types.Header{Number: big.NewInt(0), + BaseFee: big.NewInt(0)}, nil, nil, nil, trie.NewStackTrie(nil)))eth/consensus/blank_engine.go (4)
38-41: The
Authorfunction is returning an empty address. If this is intentional, ensure that the calling code can handle an empty address without causing any issues.75-76: The
Finalizefunction does nothing. If this is intentional, ensure that the calling code doesn't expect any side effects from this function.79-84: The
FinalizeAndAssemblefunction creates a new block but doesn't use thewithdrawalsparameter. If this is intentional, ensure that the calling code doesn't expect the withdrawals to be included in the block.87-92: The
Sealfunction doesn't actually seal the block (the.seal()call is commented out). If this is intentional, ensure that the calling code doesn't expect the block to be sealed.cosmos/x/evm/keeper/host.go (1)
- 106-116: The new hunk is identical to the old one, no changes have been made. Ensure this is intended.
eth/polar/backend.go (4)
229-236: The new hunk has replaced the previous mechanism of handling pending blocks with a new one. The old code commented out the miner's handling of pending blocks and used the current block as a fallback. The new code directly uses the miner's pending block. Ensure that the
miner.PendingBlock()function is implemented correctly and handles all edge cases.292-303: Similar to the previous comment, the new hunk has replaced the previous mechanism of handling pending states with a new one. The old code commented out the miner's handling of pending states. The new code directly uses the miner's pending state. Ensure that the
miner.Pending()function is implemented correctly and handles all edge cases.364-368: The method
PendingBlockAndReceipts()now retrieves the pending block and associated receipts from the miner instead of the blockchain. This change aligns with the overall shift towards using the miner for handling pending transactions and blocks. Ensure that theminer.PendingBlockAndReceipts()function is implemented correctly and handles all edge cases.546-548: The method
SubscribePendingLogsEvent()now subscribes to the miner's pending logs event instead of the blockchain's. This change aligns with the overall shift towards using the miner for handling pending transactions and blocks. Ensure that theminer.SubscribePendingLogs(ch)function is implemented correctly and handles all edge cases.eth/polar/polaris.go (6)
30-44: The new hunk introduces additional imports, including the
consensuspackage and theoldminerpackage. Ensure that these packages are correctly implemented and used in the codebase.85-100: The
Polarisstruct has been modified to include a new fieldbeaconof typeconsensus.Engineandspminerof typeoldminer.Miner. Make sure that these fields are properly initialized and used throughout the codebase.113-117: A new
beaconfield is being initialized with&consensus.DummyEngine{}. If this is a placeholder, ensure it's replaced with an actual consensus engine before production use.134-138: The
Init()function now initializesspminerwitholdminer.New(pl). Ensure that this change doesn't affect the functionality of the miner.153-156: The
mineris now initialized with additional parametersmuxandpl.beacon. Verify that these parameters are correctly passed and handled in theminer.New()function.217-240: New methods
SPMiner(),MinerChain(), andIsLocalBlock()have been added to thePolarisstruct. Ensure that they are correctly implemented and used.
itsdevbear
force-pushed
the
dev/miner-episode-1
branch
from
acb959b to
98a35ef
Compare
|
@ocnc @calbera @BrickBera let's get this guy on devnet by EOD if possible 😤😤😤 |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Files ignored due to filter (2)
- e2e/testapp/go.mod
- go.work.sum
Files selected for processing (10)
- e2e/testapp/app.go (6 hunks)
- e2e/testapp/export.go (1 hunks)
- e2e/testapp/helpers.go (1 hunks)
- eth/consensus/dummy_eth_one.go (1 hunks)
- eth/core/chain_writer.go (1 hunks)
- eth/core/state/statedb.go (1 hunks)
- eth/core/types/imported.go (1 hunks)
- eth/miner/miner.go (1 hunks)
- eth/polar/backend.go (4 hunks)
- eth/polar/polaris.go (7 hunks)
Files skipped from review due to trivial changes (3)
- eth/core/state/statedb.go
- eth/core/types/imported.go
- eth/miner/miner.go
Additional comments (Suppressed): 24
eth/core/chain_writer.go (3)
30-35: The
ChainWriterinterface has been extended with a new methodWriteBlockAndSetHead. Ensure that all implementations of this interface have been updated to include this new method.33-34: The function signature for
WriteBlockAndSetHeadhas changed, adding two new parameters:state state.StateDBandemitHeadEvent bool. Make sure all calls to this function have been updated accordingly.44-44: The
InsertBlockfunction remains unchanged in its functionality. However, ensure that the changes in import statements (line 26-27) do not affect its behavior.eth/consensus/dummy_eth_one.go (3)
44-47: The
Authorfunction returns a zero-value address. If this is intentional, ensure that the calling code can handle this value correctly.82-82: The
Finalizefunction does nothing and has no return statement. If this is intentional, ensure that the calling code can handle this correctly.88-90: The
FinalizeAndAssemblefunction creates a new block with a nil trie. If this is intentional, ensure that the calling code can handle this correctly.e2e/testapp/export.go (1)
- 223-227: The method to get the store key has been changed from
GetKey()toUnsafeFindStoreKey(). Ensure that this change doesn't introduce any security vulnerabilities or data inconsistencies. The new method might not perform the same checks as the old one.- store := ctx.KVStore(app.GetKey(stakingtypes.StoreKey)) + store := ctx.KVStore(app.UnsafeFindStoreKey(stakingtypes.StoreKey))e2e/testapp/helpers.go (1)
- 61-61: The method call
app.interfaceRegistryhas been changed to a property accessapp.InterfaceRegistry. Ensure that theInterfaceRegistryis now a public field in theappstruct and not a method. If it's still a method, this change will cause a runtime error.- app.InterfaceRegistry(), + app.interfaceRegistry,e2e/testapp/app.go (6)
56-64: The import statements have been updated to include the new
minerpackage and remove thepreparepackage. Ensure that all dependencies are correctly managed and that there are no lingering references to thepreparepackage in the codebase.98-105: The
SimAppstruct has been updated to include aMinerobject instead of aPreparehandler. This change aligns with the introduction of the newminerpackage. Make sure that all instances ofSimAppare updated accordingly.200-215: The setup for the
MinerandMempoolobjects has been updated. ThePrepareProposalmethod is now called from theMinerobject instead of thePreparehandler. Ensure that this change does not affect the functionality of the application.252-257: Several methods (
AppCodec,InterfaceRegistry,TxConfig,GetKey) have been removed from theSimAppstruct. If these methods were used elsewhere in the codebase, ensure those references are updated or removed as necessary.283-289: The initialization of services has been updated to initialize the
Minerobject instead of thePreparehandler. Ensure that this change does not affect the functionality of the application.299-301: Several functions (
GetMaccPerms,BlockedAddresses) have been removed from the file. If these functions were used elsewhere in the codebase, ensure those references are updated or removed as necessary.eth/polar/backend.go (4)
229-236: The new hunk has removed the call to
b.polar.blockchain.GetBlock(header.Hash(), header.Number.Uint64())in the case ofrpc.PendingBlockNumber. Ensure that this change does not affect the expected behavior of the function. The old hunk used to return the current block when the pending block was requested, but now it returns the actual pending block from the miner.293-303: Similar to the previous comment, the new hunk has changed the behavior for
rpc.PendingBlockNumbercase. Previously, the function would resolve the block number and return its state regardless of whether the block number was pending or not. Now, it correctly returns the pending state from the miner if the block number is pending. Make sure all calls to this function are updated to handle this new behavior.362-368: The function
PendingBlockAndReceipts()now retrieves the pending block and receipts from the miner instead of the blockchain. This change aligns with the introduction of the new miner package and seems appropriate given the function's purpose. However, ensure that the miner'sPendingBlockAndReceipts()function behaves as expected and that any side effects are handled appropriately.546-548: The subscription method for pending logs events has been updated to use the miner instead of the blockchain. This change is consistent with the introduction of the new miner package. Ensure that the miner's
SubscribePendingLogsfunction behaves as expected and that any side effects are handled appropriately.eth/polar/polaris.go (6)
30-44: The import statements have been updated to include new packages and reflect changes in the codebase. Ensure that these packages are available and correctly imported.
85-100: The
Polarisstruct has been updated with a new fieldspminerof typeoldminer.Minerandmineris now a pointer tominer.Miner. Also, a new fieldbeaconof typeconsensus.Enginehas been added. Make sure all references to these fields in the codebase are updated accordingly.113-117: A new field
beaconis being initialized with&consensus.DummyEthOne{}in theNewWithNetworkingStackfunction. Verify if this dummy consensus engine is intended for testing or if it should be replaced with a real implementation in production.134-140: The
Initmethod of thePolarisstruct has been updated. The miner is now being initialized asoldminer.New(pl), stored inpl.spminer. Ensure that this change is reflected whereverInitis called.150-161: The
minerfield of thePolarisstruct is now being initialized withminer.New(pl, &pl.cfg.Miner, pl.host.GetConfigurationPlugin().ChainConfig(), mux, pl.beacon, pl.IsLocalBlock). This is a significant change from the previous version and may affect how the miner operates. Ensure that this new initialization is correct and does not introduce any issues.214-240: New methods
SPMiner(),MinerChain(), andIsLocalBlock(_ *types.Header)have been added to thePolarisstruct. Also, theMiner()method now returns a pointer tominer.Miner. Make sure these changes are reflected wherever these methods are called.
itsdevbear
force-pushed
the
dev/miner-episode-1
branch
from
1dbbfd9 to
5d03e96
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 6
Files ignored due to filter (2)
- e2e/testapp/go.mod
- go.work.sum
Files selected for processing (10)
- e2e/testapp/app.go (6 hunks)
- e2e/testapp/export.go (1 hunks)
- e2e/testapp/helpers.go (1 hunks)
- eth/consensus/dummy_eth_one.go (1 hunks)
- eth/core/chain_writer.go (1 hunks)
- eth/core/state/statedb.go (1 hunks)
- eth/core/types/imported.go (1 hunks)
- eth/miner/miner.go (1 hunks)
- eth/polar/backend.go (4 hunks)
- eth/polar/polaris.go (7 hunks)
Files skipped from review due to trivial changes (3)
- eth/core/types/imported.go
- eth/miner/miner.go
- eth/polar/backend.go
Additional comments (Suppressed): 21
e2e/testapp/helpers.go (1)
- 61-61: The method call
app.interfaceRegistryhas been changed to a property accessapp.InterfaceRegistry(). Ensure that theinterfaceRegistryis correctly initialized and accessible at this point in the code. If it's not, consider reverting back to the method call.- app.InterfaceRegistry(), + app.interfaceRegistry,eth/core/state/statedb.go (2)
64-64: The comment
// to match geth naminghas been updated to//nolint:revive // to match geth naming. This seems to be a directive for the linter to ignore this line. Ensure that this is the intended behavior and that it doesn't introduce any linting issues in other parts of your codebase.66-69: No significant changes have been made to the
PolarStateDBinterface. The comment has been slightly modified but the functionality remains the same.eth/core/chain_writer.go (1)
- 31-35: The
ChainWriterinterface has been extended with a new methodWriteBlockAndSetHead. Ensure that all implementations of this interface have been updated to include this new method.e2e/testapp/export.go (1)
- 225-225: The method to get the store key has been changed from
GetKey()toUnsafeFindStoreKey(). Ensure that this change does not introduce any security vulnerabilities or unexpected behavior, as the new method might bypass certain checks or validations present in the old method.- store := ctx.KVStore(app.GetKey(stakingtypes.StoreKey)) + store := ctx.KVStore(app.UnsafeFindStoreKey(stakingtypes.StoreKey))eth/consensus/dummy_eth_one.go (4)
44-47: The
Authorfunction returns an empty address. If this is intentional, ensure that the calling code can handle an empty address without causing any issues.81-82: The
Finalizefunction does nothing. If this is intentional, ensure that the calling code can handle this correctly.84-90: In the
FinalizeAndAssemblefunction, a new block is created with atrie.NewStackTrie(nil). Ensure that the calling code can handle a block with a nil trie without causing any issues.92-98: In the
Sealfunction, the block is "sealed" by simply assigning it tosealedBlock. If this is intentional, ensure that the calling code can handle this correctly.e2e/testapp/app.go (6)
57-64: The import statement for the "miner" package has been added, and the "prepare" package is no longer imported. Ensure that all dependencies are correctly managed and that there are no unused imports or missing packages.
99-105: The
ppfield (of type*prepare.Handler) in theSimAppstruct has been replaced withmm(of type*miner.Miner). Make sure that this change does not affect other parts of the codebase that rely on theppfield.200-215: The
PrepareProposalmethod from theminerobject is now being used instead of the one from theprepareobject. Also, thetxConfig.SignModeHandler()call has been changed toapp.txConfig.SignModeHandler(). Verify that these changes are intended and do not introduce any unexpected behavior.252-257: Several methods (
AppCodec,InterfaceRegistry,TxConfig,GetKey) have been removed from theSimAppstruct. If these methods were used elsewhere in the codebase, those references will need to be updated or removed.283-289: The
Initmethod is now called on theminerobject instead of theprepareobject. Ensure that this change is intentional and doesn't cause any issues.297-301: The
Closemethod of theBaseAppis now being returned if thePolarisobject is nil. Previously,nilwas returned directly. This change could potentially alter the behavior of the application when closing, so it should be verified.eth/polar/polaris.go (6)
34-44: The import statements have been updated to include new packages and rename existing ones. Ensure that these changes do not break any dependencies or cause conflicts.
85-100: The
Polarisstruct has been updated with a new fieldspminerof typeoldminer.Minerandbeaconof typeconsensus.Engine. Make sure these changes are reflected wherever this struct is used.113-117: A new consensus engine
DummyEthOnefrom theconsensuspackage is being used. Verify if this mock implementation is intended for testing purposes only or if it's meant for production use.134-140: The
Init()function now initializesspminerinstead ofminer. This change should be reflected in all calls to this function.150-161: The
mineris now initialized with additional parametersmuxandpl.beacon. Ensure that these changes are compatible with theminer.New()function signature.214-240: New methods
SPMiner(),MinerChain(), andIsLocalBlock()have been added. If these methods are part of an interface thatPolarisimplements, ensure that they are implemented correctly.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Files ignored due to filter (1)
- go.work.sum
Files selected for processing (1)
- cosmos/miner/miner.go (1 hunks)
Additional comments (Suppressed): 1
cosmos/miner/miner.go (1)
- 104-105: These fields are marked as "todo". Ensure that these placeholders are replaced with actual values before merging the code.
Summary by CodeRabbit
Polarisstruct to use the newspminerobject, reflecting a change in the mining mechanism.Broadcasterinterface toTxBroadcasterand updated related functions and structures accordingly for better clarity.Polarisstruct includingSPMiner(),MinerChain(), andIsLocalBlock()to enhance block management capabilities.eth_getBlockByNumberRPC call to correctly represent data size and withdrawals.backend.go, improving code organization.app.InterfaceRegistry()withapp.interfaceRegistryandctx.KVStorewithctx.UnsafeFindStoreKeyfor improved code efficiency.