feat(baseapp): add option to discard events #24440
Conversation
Ironbird - launch a networkTo use Ironbird, you can use the following commands:
|
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThe pull request updates how event managers are instantiated and managed within the BaseApp framework. The BaseApp methods now invoke a new method from the message service router to create event managers instead of directly calling the default constructor. A new discarding event manager implementation has been added to handle cases when event emissions should be suppressed. Additionally, a boolean flag and a corresponding setter method allow toggling event emission behavior, ensuring that the appropriate event manager is used during transaction execution and block finalization. Changes
Sequence Diagram(s)sequenceDiagram
participant BA as BaseApp
participant MSR as MsgServiceRouter
participant SEM as Standard EventManager
participant DEM as DiscardingEventManager
BA->>MSR: Call newEventManager()
alt disableEventEmission = true
MSR->>DEM: Return discardingEventManager instance
else
MSR->>SEM: Return standard sdk.NewEventManager instance
end
BA-->>MSR: Use returned event manager in context (preBlock/runTx)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🧰 Additional context used🧬 Code Graph Analysis (3)baseapp/options.go (1)
baseapp/msg_service_router.go (2)
baseapp/discard_event_mgr.go (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (8)
✨ 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 (
|
|
do we have an issue for the app.toml follow up? |
baseapp/msg_service_router.go
Outdated
| routes map[string]MsgServiceHandler | ||
| hybridHandlers map[string]func(ctx context.Context, req, resp protoiface.MessageV1) error | ||
| circuitBreaker CircuitBreaker | ||
| interfaceRegistry codectypes.InterfaceRegistry |
There was a problem hiding this comment.
We write a test that demonstrates how this new option works:
- events are emitted when the field is false
- events are not emitted when true
by running some SDK message on the router
There was a problem hiding this comment.
I adapted the existing FinalizeBlock test to check both cases: f130f81. Does that seem sufficient or should we aim to cover more possible cases?
|
@aljo242 I've addressed your comments and also handled a few cases for emit emission that I originally missed. Let me know if you see anything else that needs to be addressed! |
aaronc
changed the title
Not yet. We should add one. I looked quickly and maybe I'm missing something, but I'm not seeing a straightforward way to do this without adding it specifically to each app.go. |
|
Blocking until we can perform ironbird tests against this code |
aljo242
added
the
Cryostasis
Description
This adds a
BaseAppoption to totally disable event emission. A follow-up to this PR would be adding an app.toml setting to configure this.Author 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