[CP-686] feat(baseapp): introduce txResults post hook

[kakysha]

so app can plug in any transformer of tx results (like EVM transformer for fixing tx and log indexes)

Summary by CodeRabbit

Release Notes

• New Features
  • Added transaction results post-processing capability, allowing modifications to transaction outcomes after block execution. This enables enhancements such as log corrections and improved data indexing.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

baseapp/options.go (1)

415-421: Add documentation for the new public API.

Both functions are exported but lack godoc comments. Consider documenting:

• The purpose of the post-hook (e.g., "allows transformation of transaction results after block execution")
• When it's invoked (after all transactions in a block are executed)
• Example use cases (as mentioned in the PR: EVM transformer for fixing transaction and log indexes)
• Any constraints or expectations for the hook implementation

baseapp/baseapp.go (1)

50-50: Add documentation for the public type.

The exported TxResultsPostHook type lacks a godoc comment. Document its purpose, usage, and any expectations for implementations (e.g., should preserve result count, when it's invoked, etc.).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 94abda2 and a56a899.

📒 Files selected for processing (3)

• baseapp/abci.go (1 hunks)
• baseapp/baseapp.go (2 hunks)
• baseapp/options.go (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

baseapp/options.go (1)

baseapp/baseapp.go (2)

• BaseApp (69-213)
• TxResultsPostHook (50-50)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)

• GitHub Check: tests (00)
• GitHub Check: tests (02)
• GitHub Check: tests (01)
• GitHub Check: tests (03)
• GitHub Check: test-sim-nondeterminism
• GitHub Check: liveness-test
• GitHub Check: golangci-lint
• GitHub Check: test-simapp
• GitHub Check: test-e2e
• GitHub Check: test-integration
• GitHub Check: build (amd64)
• GitHub Check: build (arm64)
• GitHub Check: build (arm)
• GitHub Check: Analyze
• GitHub Check: Gosec
• GitHub Check: dependency-review

🔇 Additional comments (2)

baseapp/abci.go (1)

881-883: Verify hook behavior in implementations.

The hook replaces the entire txResults slice with no validation. Ensure that hook implementations:

• Return a non-nil slice matching the original transaction count
• Don't inadvertently corrupt or drop transaction results

While this flexibility enables transformations like reordering or enriching results, misuse could cause mismatches between the number of transactions and results.

baseapp/baseapp.go (1)

212-212: LGTM!

The field is appropriately unexported and placed at the end of the struct, minimizing impact on existing code. The nil zero value integrates cleanly with the nil-check in the hook invocation.

[github-actions]

@kakysha your pull request is missing a changelog!

[coderabbitai]

Walkthrough

This PR introduces a post-processing hook mechanism for transaction results in the Cosmos baseapp. A new TxResultsPostHook function type is defined, stored as a field in BaseApp, and invoked after transaction execution in executeTxs to transform the collected transaction results. Configuration is provided via setter methods.

Changes

Cohort / File(s)	Summary
Transaction Results Post-Hook baseapp/abci.go, baseapp/baseapp.go, baseapp/options.go	Introduced TxResultsPostHook type for transaction result post-processing; added txResultsPostHook field to BaseApp; integrated hook invocation in executeTxs after result collection; added setter method and functional option to configure the hook

Sequence Diagram

sequenceDiagram
    participant ABCI as ABCI Layer
    participant ExecuteTxs as executeTxs
    participant Hook as txResultsPostHook
    
    ABCI->>ExecuteTxs: Execute transactions
    ExecuteTxs->>ExecuteTxs: Collect per-tx results
    alt Hook configured
        ExecuteTxs->>Hook: Transform txResults
        Hook-->>ExecuteTxs: Modified results
    else Hook not set
        ExecuteTxs->>ExecuteTxs: Skip hook (nil)
    end
    ExecuteTxs-->>ABCI: Return results

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Straightforward extension point pattern with consistent implementation across three files
• Minimal logic complexity; primarily additive changes (type definition, field addition, method stubs)
• No control flow restructuring or complex interdependencies

Poem

🐰 twitches whiskers thoughtfully
A hook to catch results in flight,
Post-processing magic, shining bright!
Transactions dance, then transform true,
The baseapp's newest rendezvous! ✨

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title 'CP-686 feat(baseapp): introduce txResults post hook' clearly and accurately reflects the main changes in the changeset. The title explicitly mentions the introduction of a txResults post hook in the baseapp package, which aligns with the core modifications: adding the TxResultsPostHook type, the txResultsPostHook field to BaseApp, and the SetTxResultsPostHook methods. The title is concise, specific, and directly communicates the primary purpose of the changes without being vague or misleading.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat-txresults-post-hook

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

baseapp/options.go (1)

420-422: Add sealed check for consistency.

Most setter methods in this file check app.sealed before allowing modifications to prevent modification after initialization.

Apply this diff to add the sealed check:

 func (app *BaseApp) SetTxResultsPostHook(hookFn TxResultsPostHook) {
+	if app.sealed {
+		panic("SetTxResultsPostHook() on sealed BaseApp")
+	}
+
 	app.txResultsPostHook = hookFn
 }

🧹 Nitpick comments (1)

baseapp/options.go (1)

429-431: Consider calling the method instead of direct field assignment.

The functional option directly assigns the field, bypassing the public method SetTxResultsPostHook. For consistency and to ensure any checks added to the method (like the sealed check) are applied, consider calling the method instead.

Apply this diff:

 func SetTxResultsPostHook(hookFn TxResultsPostHook) func(*BaseApp) {
-	return func(app *BaseApp) { app.txResultsPostHook = hookFn }
+	return func(app *BaseApp) { app.SetTxResultsPostHook(hookFn) }
 }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 94abda2 and f5c8455.

📒 Files selected for processing (3)

• baseapp/abci.go (1 hunks)
• baseapp/baseapp.go (2 hunks)
• baseapp/options.go (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

baseapp/options.go (1)

baseapp/baseapp.go (2)

• BaseApp (69-217)
• TxResultsPostHook (50-50)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)

• GitHub Check: tests (00)
• GitHub Check: tests (02)
• GitHub Check: tests (03)
• GitHub Check: tests (01)
• GitHub Check: test-simapp
• GitHub Check: test-integration
• GitHub Check: test-sim-nondeterminism
• GitHub Check: liveness-test
• GitHub Check: test-e2e
• GitHub Check: Gosec
• GitHub Check: Analyze
• GitHub Check: golangci-lint
• GitHub Check: build (arm64)
• GitHub Check: build (amd64)
• GitHub Check: build (arm)

🔇 Additional comments (3)

baseapp/abci.go (1)

880-884: LGTM! Post-hook invocation is correctly implemented.

The nil check ensures safe invocation, and the placement after transaction execution is appropriate for post-processing the results.

baseapp/baseapp.go (2)

49-50: LGTM! Type definition is clean and appropriate.

The function signature properly supports post-processing transaction results by taking and returning the same type, enabling transformations without requiring side effects.

---

212-216: LGTM! Field declaration with clear documentation.

The field is properly declared with comprehensive documentation explaining the use case for post-processing transaction results, particularly for EVM log indexing.