perf(comet): Eject the comet mempool on ReCheckTx to keep it clean
#1237
Conversation
Rate Limit Exceeded@itsdevbear has exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 0 minutes and 15 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, convert this PR to a draft and then mark it as ready for review again to re-trigger the review. Alternatively, you can push new commits to this PR. We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per repository. WalkthroughThe changes introduce a new Changes
TipsChat with CodeRabbit Bot (
|
itsdevbear
force-pushed
the
reject-recheck-tx
branch
from
866a7c5 to
14d4925
Compare
itsdevbear
force-pushed
the
reject-recheck-tx
branch
from
7df90ec to
41dcf94
Compare
![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 (2)
- cosmos/lib/ante/eject.go (1 hunks)
- cosmos/runtime/runtime.go (3 hunks)
Files skipped from review due to trivial changes (1)
- cosmos/lib/ante/eject.go
Additional comments (Suppressed): 3
cosmos/runtime/runtime.go (3)
32-38: The new import statement for the
antepackage is added. Ensure that the package is available at the specified location and that it is compatible with the current version of the application.97-103: The
Buildfunction of thePolarisstruct now sets theWrappedTxPoolfield using thetxpool.Newfunction with an additional argumentp.ExecutionLayer.Backend(). Ensure that this change does not break the existing functionality and that theBackendmethod of theExecutionLayerstruct returns a value compatible with thetxpool.Newfunction.- p.WrappedTxPool = txpool.New(p.TxPool()) + p.WrappedTxPool = txpool.New(p.TxPool(), p.ExecutionLayer.Backend())
- 110-114: The
SetAnteHandlerfunction of theCosmosAppinterface is now called with an instance ofantelib.NewAnteHandler(), replacing the previousnilvalue. Ensure that theNewAnteHandlerfunction from theantepackage returns a value compatible with theSetAnteHandlerfunction and that this change does not break the existing functionality.- app.SetAnteHandler(nil) + app.SetAnteHandler(antelib.NewAnteHandler())
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1237 +/- ##
=======================================
Coverage 49.23% 49.23%
=======================================
Files 77 77
Lines 4613 4613
=======================================
Hits 2271 2271
Misses 2179 2179
Partials 163 163
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Files selected for processing (5)
- contracts/scripts/DeployAndCallERC20.sol (1 hunks)
- cosmos/lib/ante/eject.go (1 hunks)
- cosmos/runtime/runtime.go (2 hunks)
- eth/eth.go (2 hunks)
- eth/polar/backend.go (1 hunks)
Files skipped from review due to trivial changes (1)
- eth/eth.go
Additional comments (Suppressed): 7
contracts/scripts/DeployAndCallERC20.sol (1)
- 39-42: The loop iteration count has been increased from 66 to 10066, and the
quantityvariable is incremented by 50000 in each iteration. This will significantly increase the number of tokens minted and the gas cost of therunfunction. Ensure that this is the intended behavior and that the contract has enough gas to complete the operation.- for (uint256 i = 0; i < 66; i++) { + for (uint256 i = 0; i < 10066; i++) {eth/polar/backend.go (2)
219-221: No changes have been made to the
Blockchainmethod. It continues to return the blockchain associated with thePolarisinstance.223-230: The new
IncludedInBlockmethod checks if a transaction, identified by its hash, has been included in a block. It retrieves the transaction lookup entry (tle) for the given hash from the blockchain. If thetleisnil, it means the transaction does not exist in the blockchain, so it returnsfalse. Otherwise, it checks if theBlockHashfield of thetleis not an empty hash, which would indicate that the transaction has been included in a block. This method provides a way to verify the inclusion of a transaction in a block, which can be useful in various scenarios, such as confirming transaction finality.cosmos/runtime/runtime.go (2)
35-38: The import statement for the
antelibpackage has been added. Ensure that the package is available and accessible in the project's dependencies.108-113: The
SetAnteHandlerfunction is no longer being set tonil. Instead, it's being set toantelib.NewAnteHandler(). This change implies that the AnteHandler is now required for the application. Make sure that this change is intentional and that theNewAnteHandlerfunction from theantelibpackage is correctly implemented and tested.- app.SetAnteHandler(nil) + app.SetAnteHandler(antelib.NewAnteHandler())cosmos/lib/ante/eject.go (2)
29-35: The
NewAnteHandlerfunction creates a chain ofsdk.AnteDecoratorinstances and returns ansdk.AnteHandler. It currently only includesEjectOnRecheckTxDecoratorin the chain. If more decorators are to be added in the future, they should be appended to theanteDecoratorsslice in the order they should be executed.39-45: The
AnteHandlemethod ofEjectOnRecheckTxDecoratorchecks if the transaction context is a recheck and returns an error if it is. This prevents rechecked transactions from being processed further. If the transaction is not a recheck, it calls the nextAnteHandlerin the chain. Ensure that this behavior is intended and that it doesn't interfere with the normal operation of the application.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Files selected for processing (5)
- contracts/scripts/DeployAndCallERC20.sol (1 hunks)
- cosmos/lib/ante/eject.go (1 hunks)
- cosmos/runtime/runtime.go (2 hunks)
- eth/eth.go (2 hunks)
- eth/polar/backend.go (1 hunks)
Files skipped from review due to trivial changes (1)
- eth/eth.go
Additional comments (Suppressed): 4
cosmos/lib/ante/eject.go (1)
- 39-45: The
AnteHandlemethod ofEjectOnRecheckTxDecoratorchecks if the transaction context is a recheck and returns an error if it is. This effectively ejects the transaction from the mempool during a recheck. Ensure that this behavior is intended and that it won't cause any unwanted side effects, such as valid transactions being ejected during a recheck.contracts/scripts/DeployAndCallERC20.sol (1)
- 39-42: The loop iteration count has been increased from 66 to 10066. This will significantly increase the number of tokens minted and the time taken to execute the
run()function. Ensure that this change is intentional and that the system can handle this increased load.cosmos/runtime/runtime.go (2)
32-38: The import statement for the
antelibpackage has been added. This package is used to create a new AnteHandler in theBuildfunction. Ensure that the package is correctly imported and available in the project.108-114: The
SetAnteHandlerfunction is now being called with an instance ofantelib.NewAnteHandler(), instead ofnil. This change introduces a new AnteHandler that will handle the transaction validation and processing. Make sure that theNewAnteHandlerfunction from theantelibpackage is correctly implemented and that it returns an AnteHandler that is compatible with theSetAnteHandlerfunction.- app.SetAnteHandler(nil) + app.SetAnteHandler(antelib.NewAnteHandler())
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Files selected for processing (2)
- cosmos/lib/ante/eject.go (1 hunks)
- cosmos/runtime/runtime.go (2 hunks)
Additional comments (Suppressed): 4
cosmos/lib/ante/eject.go (2)
29-35: The
NewAnteHandlerfunction creates a newsdk.AnteHandlerinterface with a chain ofsdk.AnteDecoratorinstances. It currently only includesEjectOnRecheckTxDecorator. If more decorators are to be added in the future, ensure they are added in the correct order as the order of decorators can affect the execution flow.39-47: The
AnteHandlemethod ofEjectOnRecheckTxDecoratorreturns an error if the transaction context is a recheck. This is a good practice as it prevents unnecessary processing of transactions that are being rechecked, potentially improving performance.cosmos/runtime/runtime.go (2)
33-38: The import statement for the
antelibpackage has been added. This package contains the newAnteHandlerfunction that will be used to set theAnteHandlerfor the application.108-114: The
SetAnteHandlerfunction is now being called with an instance ofantelib.NewAnteHandler(). This is a change from the previous version where it was set tonil. This change is part of the effort to eject the comet mempool onReCheckTxto keep it clean and improve performance. However, it's important to ensure that theNewAnteHandlerfunction from theantelibpackage is correctly implemented and that it doesn't introduce any unexpected behavior or errors.- app.SetAnteHandler(nil) + app.SetAnteHandler(antelib.NewAnteHandler())
Summary by CodeRabbit
These changes aim to improve the user experience by increasing the system's reliability and efficiency.