[STAB-32] Push CheckTx concurrency into protocol #1030
Conversation
WalkthroughThe update involves modifications to the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![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 ignored due to path filters (2)
protocol/go.modis excluded by:!**/*.modprotocol/go.sumis excluded by:!**/*.sum
Files selected for processing (1)
- protocol/app/ante.go (2 hunks)
Additional comments: 2
protocol/app/ante.go (2)
- 5-5: The addition of the
baseapppackage fromgithub.com/cosmos/cosmos-sdk/baseappis correctly implemented. This import is necessary for the new concurrency control mechanism being introduced.- 72-72: The integration of
baseapp.NewLockAndCacheContextAnteDecorator()into theNewAnteDecoratorChainfunction is a significant change aimed at enhancing concurrency control within the protocol. This decorator is expected to manage locking mechanisms more efficiently, aligning with the PR's objectives. However, ensure that the placement of this decorator does not inadvertently affect the execution order of other decorators, as the order in which ante decorators are executed can significantly impact transaction processing logic.Ensure thorough testing, particularly focusing on concurrency aspects, to validate that the new locking mechanism behaves as expected without introducing regressions.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- protocol/app/ante_whitebox_test.go (1 hunks)
Additional comments: 1
protocol/app/ante_whitebox_test.go (1)
- 80-80: The addition of
"baseapp.lockAndCacheContextDecorator"to theexpectedDecoratorTypesslice aligns with the PR's objective to enhance concurrency control by integrating a new locking mechanism closer to the CLOB ante decorator. This change is crucial for ensuring that the test reflects the updated ante handler chain order, which now includes thelockAndCacheContextDecorator. It's important to verify that this new decorator is correctly implemented and integrated within the actual ante handler chain to maintain the integrity of transaction processing and concurrency control.
lcwik
force-pushed
the
lukestab32
branch
2 times, most recently
from
19e4784 to
7407527
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
protocol/go.modis excluded by:!**/*.modprotocol/go.sumis excluded by:!**/*.sum
Files selected for processing (2)
- protocol/app/ante.go (2 hunks)
- protocol/app/ante_whitebox_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- protocol/app/ante.go
- protocol/app/ante_whitebox_test.go
protocol/go.mod
Outdated
| @@ -411,7 +411,7 @@ replace ( | |||
| // Use dYdX fork of CometBFT | |||
| github.com/cometbft/cometbft => github.com/dydxprotocol/cometbft v0.38.3-0.20231204185009-5e6c4b6d67b8 | |||
| // Use dYdX fork of Cosmos SDK | |||
| github.com/cosmos/cosmos-sdk => github.com/dydxprotocol/cosmos-sdk v0.50.4-0.20240126201553-f1da3a9865bb | |||
There was a problem hiding this comment.
Please don't forget to change
There was a problem hiding this comment.
Thanks for the reminder.
This pushes locking slightly closer to the CLOB ante decorator hopefully allowing for protocol to control parallelism more finely in the future.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
protocol/go.modis excluded by:!**/*.modprotocol/go.sumis excluded by:!**/*.sum
Files selected for processing (2)
- protocol/app/ante.go (2 hunks)
- protocol/app/ante_whitebox_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- protocol/app/ante.go
- protocol/app/ante_whitebox_test.go
Changelist
[STAB-32] Push CheckTx concurrency into protocol
This pushes locking slightly closer to the CLOB ante decorator hopefully allowing for protocol to control parallelism more finely in the future.
Test Plan
Tests were added in the Cosmos SDK fork PR and there is this existing concurrency test that is of note.
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.