feat: add data max size and emit deposit hook events #133
Conversation
WalkthroughThis pull request introduces modifications across multiple files in the x/opchild and x/ophost modules. The changes primarily focus on enhancing event handling for token deposits, adding a new test for deposit hook events, and implementing data validation for token deposit messages. The modifications include updating the message handling to capture and emit events, creating a new test to verify event emission, adding a new error type, and introducing a data length constraint for token deposit messages. Changes
Sequence DiagramsequenceDiagram
participant MsgServer
participant Handler
participant EventManager
participant CacheContext
MsgServer->>Handler: Execute message
Handler-->>MsgServer: Return result with events
MsgServer->>CacheContext: Capture context
CacheContext->>EventManager: Emit events from handler result
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #133 +/- ##
==========================================
- Coverage 49.40% 49.38% -0.03%
==========================================
Files 57 57
Lines 4271 4275 +4
==========================================
+ Hits 2110 2111 +1
- Misses 1723 1726 +3
Partials 438 438
|
![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)
x/opchild/keeper/deposit_test.go (1)
1-65: Add negative test scenarios for invalid dataCurrently, no test ensures
FinalizeTokenDepositfails due to an excessively largeDatapayload. Adding a negative test scenario that triggersErrInvalidDatawill help confirm that the system correctly rejects deposits exceeding the maximum data length.Would you like help drafting or extending your test coverage for oversize data inputs?
x/ophost/types/tx.go (1)
25-25: Consider making MaxDataLength configurableA hard-coded limit of 10KB may be acceptable for now, but if you anticipate changing it in the future, it might be beneficial to store this value in module parameters to simplify future adjustments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
x/opchild/keeper/deposit.go(1 hunks)x/opchild/keeper/deposit_test.go(1 hunks)x/ophost/types/error.go(1 hunks)x/ophost/types/tx.go(2 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
x/ophost/types/tx.go
[warning] 199-201: x/ophost/types/tx.go#L199-L201
Added lines #L199 - L201 were not covered by tests
🔇 Additional comments (3)
x/ophost/types/error.go (1)
25-25: Thanks for introducing a new error codeNo immediate issues observed. The code addition aligns with the module’s error handling pattern.
x/ophost/types/tx.go (1)
199-201: Add coverage for the invalid data conditionThe static analysis (codecov) indicates that these lines are untested. Ensure you create tests that provide a
msg.Dataarray exceedingMaxDataLength, confirming thatErrInvalidDatais correctly triggered.Let me know if you’d like a helper script or a code snippet to integrate such a test.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 199-201: x/ophost/types/tx.go#L199-L201
Added lines #L199 - L201 were not covered by testsx/opchild/keeper/deposit.go (1)
61-68: Event emission is correctly handledCapturing the
resand emitting its events is an excellent enhancement, ensuring deposit-related actions are properly tracked.
Description
add MsgInitiateTokenDeposit msg size checker and emit deposit hook events.
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 changeReviewers 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.
I have...
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements
The updates focus on improving system reliability and data validation in the token deposit and withdrawal workflows.