fix(katana-rpc): event idx not respecting cursor's block idx #2578
Conversation
WalkthroughOhayo, sensei! This pull request introduces enhancements to the Changes
Assessment against linked issues
Possibly related PRs
🪧 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
Documentation and Community
|
![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
🧹 Outside diff range and nitpick comments (2)
crates/katana/rpc/rpc/src/utils/events.rs (2)
279-295: Ohayo! The documentation is much clearer now, sensei!The new documentation nicely explains the function's purpose and parameters. However, consider adding information about possible error cases to make it even more complete.
Add error cases documentation:
/// Returns a cursor if it couldn't include all the events of the current transaction because /// the buffer is already full. Otherwise, if it is able to include all the transactions, /// returns None. +/// +/// # Errors +/// +/// Returns `Error` if: +/// * The provider fails to retrieve transaction data +/// * The cursor is invalid
Line range hint
322-332: Ohayo! Consider a small optimization, sensei!The event index tracking logic is correct, but we could optimize the memory usage by avoiding the intermediate Vec collection when we know the exact capacity needed.
Consider this optimization:
- .collect::<Vec<_>>(); + .collect::<Vec<_>>() { + let mut filtered = Vec::with_capacity(total_can_take); + filtered.extend(iter); + filtered + };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- crates/katana/rpc/rpc/src/utils/events.rs (6 hunks)
🔇 Additional comments (3)
crates/katana/rpc/rpc/src/utils/events.rs (3)
73-73: Ohayo! Nice documentation improvement, sensei!The updated comment better clarifies that the PartialCursor points to a specific event within the context of a transaction, which helps prevent confusion about its scope.
Line range hint
1-370: Ohayo! Let's verify the complete fix, sensei!The changes look solid and properly address the syncing issues mentioned in #2574. To ensure the fix is complete, we should verify:
- The cursor properly maintains state across block boundaries
- Edge cases are handled correctly when events span multiple blocks
Let's verify the complete fix:
180-198: Ohayo! This fix looks solid, sensei!The improved cursor logic correctly handles event index determination by:
- Properly checking if we're at the cursor's exact position (block and transaction)
- Correctly handling the case where the cursor points to an event index that's either:
- Less than total events (continue processing from that index)
- Equal/Greater than total events (skip to next transaction)
This change directly addresses the core issue where event idx wasn't respecting the cursor's block idx.
Let's verify the fix with this script:
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2578 +/- ##
==========================================
- Coverage 69.58% 69.47% -0.12%
==========================================
Files 401 401
Lines 50745 50817 +72
==========================================
- Hits 35312 35306 -6
- Misses 15433 15511 +78 ☔ View full report in Codecov by Sentry. |
resolves #2574
Summary by CodeRabbit
Documentation
PartialCursorstruct to improve readability.fetch_tx_eventsfunction.Refactor
fetch_pending_eventsandfetch_events_at_blocksfunctions to improve cursor handling.