feat(baseapp): add basic support in sdk for block-stm #24458
Conversation
Ironbird - launch a networkTo use Ironbird, you can use the following commands:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
baseapp/abci.go
Outdated
|
|
||
| // GasMeter must be set after we get a context with updated consensus params. | ||
| gasMeter := app.getBlockGasMeter(app.finalizeBlockState.Context()) | ||
| app.finalizeBlockState.SetContext(app.finalizeBlockState.Context().WithBlockGasMeter(gasMeter)) | ||
| app.finalizeBlockState.SetContext( | ||
| app.finalizeBlockState.Context(). | ||
| WithBlockGasMeter(gasMeter). | ||
| WithTxCount(len(req.Txs)), | ||
| ) |
There was a problem hiding this comment.
Change potentially affects state.
Call sequence:
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).internalFinalizeBlock (baseapp/abci.go:705)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).FinalizeBlock (baseapp/abci.go:869)
thomas-nguy
force-pushed
the
thomas/block-stm
branch
from
ec25c23 to
0100b57
Compare
📝 WalkthroughWalkthroughThis update refactors how transactions are executed within the BaseApp. A new method ( Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant BA as BaseApp (Finalize)
participant ET as executeTxs
participant CTX as Context Modifier
participant DT as deliverTxWithMultiStore
Client->>BA: Submit block (with transactions)
BA->>CTX: Create/update context with block gas meter and txCount
BA->>ET: Call executeTxs(txs)
loop For each transaction
ET->>CTX: Update context with txIndex and msgIndex
ET->>DT: Execute transaction (deliverTxWithMultiStore)
DT-->>ET: Return execution result or error
end
ET-->>BA: Return array of tx execution results
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
baseapp/txexecutor.go (1)
11-16: Good definition of the TxExecutor function type.The TxExecutor type is well-designed to facilitate parallel transaction execution. It takes appropriate parameters (context, block size, store, and a function for individual tx execution) and returns transaction results with error handling. This cleanly encapsulates the execution logic and allows for pluggable execution strategies.
Consider adding a doc comment that explains the intended use case for this function type, particularly noting that it's designed for parallel execution strategies like block-stm. This would make the purpose clearer for developers.
+// TxExecutor defines a function type that can be used to execute transactions, potentially in parallel. +// It is designed to support block-stm or other parallel execution strategies. +// The function takes a context, block size, multi-store, and a function to deliver individual transactions, +// and returns the results of all transaction executions along with any error encountered. type TxExecutor func( ctx context.Context, blockSize int, cms types.MultiStore, deliverTxWithMultiStore func(int, types.MultiStore) *abci.ExecTxResult, ) ([]*abci.ExecTxResult, error)baseapp/baseapp.go (1)
667-678: Updated context creation to include transaction index.The
getContextForTxmethod now accepts and uses a txIndex parameter, initializing the context with the proper transaction index information viaWithMsgIndex.The method sets
WithMsgIndexusing thetxIndexparameter. While this works functionally, it would be clearer to use bothWithTxIndexandWithMsgIndexto properly initialize all relevant fields:ctx := modeState.Context(). WithTxBytes(txBytes). WithGasMeter(storetypes.NewInfiniteGasMeter()). - WithMsgIndex(txIndex) + WithTxIndex(txIndex). + WithMsgIndex(-1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
baseapp/abci.go(3 hunks)baseapp/baseapp.go(7 hunks)baseapp/genesis.go(1 hunks)baseapp/options.go(2 hunks)baseapp/test_helpers.go(1 hunks)baseapp/txexecutor.go(1 hunks)types/context.go(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
baseapp/txexecutor.go (2)
types/context.go (1)
Context(40-73)store/types/store.go (1)
MultiStore(115-147)
baseapp/abci.go (3)
types/context.go (1)
Context(40-73)baseapp/baseapp.go (1)
BaseApp(63-203)store/types/store.go (1)
MultiStore(115-147)
baseapp/test_helpers.go (2)
baseapp/baseapp.go (1)
BaseApp(63-203)types/context.go (1)
Context(40-73)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (15)
baseapp/options.go (2)
127-130: The function implementation looks good.This function allows setting a custom transaction executor for the BaseApp, which is essential for the block-stm support being added. The implementation follows the established options pattern used throughout this file.
412-415: Good addition of the direct setter method.This method provides a convenient way to set the transaction executor directly on a BaseApp instance, complementing the functional option defined above. This follows the same pattern as other setter methods in this file.
baseapp/genesis.go (1)
16-16: Update to match new deliverTx signature.The change appropriately adds the required txIndex parameter (with -1 value) to align with the updated deliverTx method signature. Using -1 as the index is logical here since genesis transactions are not part of a standard block.
baseapp/test_helpers.go (2)
80-80: Correctly updated for new context creation signature.The change properly adds the required txIndex parameter (with -1 value) to align with the updated getContextForTx method signature. The -1 value appropriately indicates this context is not for a specific transaction in a block.
84-84: Correctly updated for new context creation signature.Similar to the above change, this properly adds the required txIndex parameter (with -1 value) to the getContextForTx call for CheckTx context creation.
baseapp/abci.go (3)
761-765: Added transaction count to the execution context.This enhancement allows access to the total number of transactions in the current block from within the context. Good addition that will be useful for transaction execution optimizations in block-stm.
805-809: Refactored transaction execution into a separate method.The transaction execution logic has been centralized into
executeTxs(), which improves code organization and enables optional parallel execution via custom transaction executors.
839-876: Good implementation of transaction execution with parallel execution support.The new
executeTxsmethod provides two execution paths:
- Using a custom executor when
txExecutoris set, allowing parallel execution- Falling back to sequential execution with proper error handling
The method also includes proper cancellation checks which is important for long-running operations.
baseapp/baseapp.go (3)
201-202: Added optional transaction executor for parallel execution.The new
txExecutorfield provides a hook for alternative transaction execution strategies, which is essential for block-stm parallel transaction processing. The comment clearly explains its purpose.
763-767: Introduced MultiStore variant of deliverTx.Good refactoring that provides flexibility for transaction processing. The original
deliverTxnow delegates todeliverTxWithMultiStorewith a nil multistore parameter, maintaining backward compatibility while supporting the new parallel execution capability.
843-852: Enhanced runTx to support custom MultiStore.The transaction executor can now provide a custom MultiStore instance for transaction execution, which is essential for parallel transaction processing with block-stm. This allows each transaction to operate on its own isolated multistore view.
types/context.go (4)
69-72: Added transaction tracking fields to Context.New fields added to track:
txIndex: Current transaction index in the blockmsgIndex: Current message index within a transactiontxCount: Total transaction count in the blockblockGasUsed: Total gas used in the blockThe comments clearly explain the purpose and initialization of each field, which is excellent.
101-104: Added accessor methods for the new transaction tracking fields.These methods follow the established pattern for field access in the Context struct, maintaining consistency with the codebase.
151-152: Initialized transaction and message indices to -1.The default value of -1 indicates that a context is not associated with a specific transaction or message in a finalize block context, which is consistent with the field documentation.
332-350: Added methods to modify transaction tracking fields.These methods follow the immutable context pattern used throughout the SDK, creating a new Context with the updated field values. The implementation is correct and consistent with other similar methods.
📝 WalkthroughWalkthroughThe changes introduce a new TxExecutor option in the baseapp module to support parallel transaction execution. The BaseApp now enhances its transaction processing flow by refactoring inline transaction processing into a dedicated Changes
Sequence Diagram(s)sequenceDiagram
participant BA as BaseApp
participant IFB as internalFinalizeBlock
participant ET as executeTxs
participant RT as runTxWithMultiStore
participant CTX as Context
participant TE as TxExecutor
BA->>IFB: Begin block finalization
IFB->>CTX: Create Context (blockGasUsed, txCount)
IFB->>ET: Invoke executeTxs(txs)
ET->>RT: Process each transaction with txIndex
RT->>CTX: Update Context with txIndex & msgIndex
alt Custom Executor Set
RT->>TE: Execute transaction in parallel
end
RT-->>ET: Return transaction result
ET-->>IFB: Return executed transactions
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
baseapp/genesis.go (1)
16-16: Use a dedicated constant instead of-1to improve clarity.
This magic number might cause confusion and reduce readability. Consider defining a named constant (e.g.,NoTxIndex = -1) or adding a comment for better clarity.- res := ba.deliverTx(tx, -1) + const NoTxIndex = -1 + res := ba.deliverTx(tx, NoTxIndex)baseapp/abci.go (1)
759-765: Mention usage ofTxCountin documentation or tests.
You’re now settingTxCountin the context, which may be crucial for tracking or analytics. Consider adding a brief note in the function’s docstring or ensuring it's tested.baseapp/test_helpers.go (1)
80-80: Clarify the semantics of the-1index.
Using-1for the transaction index can cause confusion. Define a named constant or comment to indicate its purpose (e.g., not in a standard block flow). This small change improves readability.-func (app *BaseApp) GetContextForFinalizeBlock(txBytes []byte) sdk.Context { - return app.getContextForTx(execModeFinalize, txBytes, -1) +const NoTxIndex = -1 +func (app *BaseApp) GetContextForFinalizeBlock(txBytes []byte) sdk.Context { + return app.getContextForTx(execModeFinalize, txBytes, NoTxIndex) } -func (app *BaseApp) GetContextForCheckTx(txBytes []byte) sdk.Context { - return app.getContextForTx(execModeCheck, txBytes, -1) +func (app *BaseApp) GetContextForCheckTx(txBytes []byte) sdk.Context { + return app.getContextForTx(execModeCheck, txBytes, NoTxIndex) }Also applies to: 84-84
baseapp/baseapp.go (2)
200-203: Enhance documentation for new fields.
Adding a short doc comment clarifying the interaction betweendisableBlockGasMeterand the newtxExecutorwould help future maintainers understand their roles.
763-765: Update function comments to reflect the newtxIndexparameter.
Ensure the docstring fordeliverTxhighlights that an explicit transaction index is now expected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CHANGELOG.md(1 hunks)baseapp/abci.go(3 hunks)baseapp/baseapp.go(7 hunks)baseapp/genesis.go(1 hunks)baseapp/options.go(2 hunks)baseapp/test_helpers.go(1 hunks)baseapp/txexecutor.go(1 hunks)types/context.go(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
baseapp/txexecutor.go (2)
types/context.go (1)
Context(40-73)store/types/store.go (1)
MultiStore(115-147)
baseapp/abci.go (3)
types/context.go (1)
Context(40-73)baseapp/baseapp.go (1)
BaseApp(63-203)store/types/store.go (1)
MultiStore(115-147)
baseapp/test_helpers.go (2)
baseapp/baseapp.go (1)
BaseApp(63-203)types/context.go (1)
Context(40-73)
baseapp/options.go (2)
baseapp/txexecutor.go (1)
TxExecutor(11-16)baseapp/baseapp.go (1)
BaseApp(63-203)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (12)
baseapp/txexecutor.go (1)
1-16: Well-structured and clean introduction of TxExecutorThe new
TxExecutortype is a well-designed function type that will serve as the foundation for parallel transaction execution. The signature captures all necessary components for transaction execution including context, block size, multi-store, and a delivery function.baseapp/options.go (2)
127-130: Clean implementation of functional option patternGood implementation of the functional option pattern consistent with other option setters in the file.
412-415: Direct setter method complements the functional optionProviding a direct method on BaseApp follows the established pattern in the codebase and allows for flexibility in how the option is set.
types/context.go (4)
68-72: Well-documented new context fields for block-stmThe new fields are clearly documented with their purpose and default values, which is important for maintainability.
101-104: Clean accessor methods for new fieldsThe accessor methods follow the established pattern in the codebase.
151-152: Proper initialization of indexesSetting default values of -1 for txIndex and msgIndex in the constructor is appropriate, indicating that they are not within a finalized block context by default.
332-350: Clean setter methods for new fieldsThe setter methods follow the established pattern for immutable context in the codebase, returning a new context with the updated field.
CHANGELOG.md (1)
63-63: Clear and concise changelog entryThe changelog entry accurately summarizes the changes made in this PR, mentioning both the new TxExecutor option and the context fields added to support parallel execution.
baseapp/abci.go (2)
805-806: Good encapsulation of transaction execution flow.
Encapsulating transactions intoexecuteTxsmakes the code more modular and easier to maintain. The current error-handling approach looks appropriate.
839-876: Add tests for both parallel and fallback paths.
This newexecuteTxsmethod introduces optional parallel execution viatxExecutor. Consider adding or expanding unit tests covering both whentxExecutoris set and when it isnil, to ensure correctness and stability.Would you like assistance in generating a sample test suite script that checks both code paths?
baseapp/baseapp.go (2)
767-802: Validate multi-store concurrency and partial failures.
This newdeliverTxWithMultiStorecan be used in parallel contexts. Make sure to guard against partial writes or race conditions. You may want to confirm thread safety or coordinate withtxExecutorusage.Do you want a script to check for references to multi-store concurrency in your codebase?
667-667:✅ Verification successful
Confirm feasibility of using an infinite gas meter here.
Replacing the previous meter with anInfiniteGasMetercould mask potential gas constraint issues. Verify that you handle gas consumption correctly elsewhere or justify why infinite gas is acceptable in this path.Also applies to: 677-678
🏁 Script executed:
#!/bin/bash # Look for references to getContextForTx usage to see if an infinite meter is always used or if there's alternate logic: rg -A 5 "getContextForTx"Length of output: 2933
Infinite Gas Meter Usage Confirmed
After verifying the usage of
getContextForTxin multiple areas (including documentation and test helpers), we can see that while an infinite gas meter is initialized in this context, the actual gas consumption is enforced later in the transaction lifecycle. In particular, the documentation (e.g., in advanced/00-baseapp.md) clearly indicates that responsibility for gas management is deferred—via functions invoked upon returning fromRunTx—which guarantees that gas consumption is properly accounted for regardless of an infinite meter at context creation.
- Key Points:
- The infinite gas meter is used temporarily during the creation of the transaction context.
- Deferred functions and subsequent transaction processing (e.g., through the anteHandler and finalization in
RunTx) ensure gas consumption is properly checked and errors are thrown if necessary.- Similar usage patterns in tests and documentation suggest that this design choice is intentional and safe, even for code at lines 677-678.
Based on these findings, using an
InfiniteGasMeterin this context is acceptable as long as the later gas checks remain robust.
thomas-nguy
force-pushed
the
thomas/block-stm
branch
3 times, most recently
from
a6e1bbe to
d12f491
Compare
thomas-nguy
force-pushed
the
thomas/block-stm
branch
2 times, most recently
from
43bed18 to
fd7d907
Compare
thomas-nguy
force-pushed
the
thomas/block-stm
branch
6 times, most recently
from
bd2dfa3 to
ed84926
Compare
thomas-nguy
force-pushed
the
thomas/block-stm
branch
2 times, most recently
from
1f840e8 to
8dd8556
Compare
thomas-nguy
force-pushed
the
thomas/block-stm
branch
from
8dd8556 to
d22064a
Compare
| // and execute successfully. An error is returned otherwise. | ||
| // both txbytes and the decoded tx are passed to runTx to avoid the state machine encoding the tx and decoding the transaction twice | ||
| // passing the decoded tx to runTX is optional, it will be decoded if the tx is nil | ||
| func (app *BaseApp) RunTx(mode execMode, txBytes []byte, tx sdk.Tx, txMultiStore storetypes.MultiStore, incarnationCache map[string]any) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []abci.Event, err error) { |
There was a problem hiding this comment.
Change visibility to public so that it can be accessible for "custom" executor implementation
| @@ -769,7 +971,7 @@ func (app *BaseApp) beginBlock(_ *abci.RequestFinalizeBlock) (sdk.BeginBlock, er | |||
| return resp, nil | |||
| } | |||
|
|
|||
| func (app *BaseApp) deliverTx(tx []byte) *abci.ExecTxResult { | |||
| func (app *BaseApp) deliverTx(tx []byte, txMultiStore storetypes.MultiStore, incarnationCache map[string]any) *abci.ExecTxResult { | |||
There was a problem hiding this comment.
Added few more parameters needed for performance optimisations
| return ms.MultiStore.GetKVStore(key) | ||
| } | ||
|
|
||
| func (ms msWrapper) GetObjKVStore(key storetypes.StoreKey) storetypes.ObjKVStore { |
There was a problem hiding this comment.
This PR is dependent on ObjectKV store support https://github.com/cosmos/cosmos-sdk/pull/24159/files#diff-15077278eab514ef0a8d38600c68580c7f629965200a045b64788ad1407d20beR131
Description
Initial PR to add basic support in sdk for block-stm:
Executorfor custom execution logicDefaultExecutorandBlockSTMExecutorAuthor Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
Refactor