fix: Infinite loop#464
Conversation
WalkthroughThe Stellar client's block pagination logic was modified to detect cursor stalls by tracking the previous cursor value instead of comparing against the start_block. An accompanying regression test verifies pagination completes correctly when the initial cursor matches the start_block. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.16.4)src/services/blockchain/clients/stellar/client.rs┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m tests/integration/blockchain/clients/stellar/client.rs┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m Comment Tip You can generate walkthrough in a markdown collapsible section to save space.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/services/blockchain/clients/stellar/client.rs (1)
293-298: Consider adding stall detection toget_transactionsandget_eventsfor consistency.While these methods didn't have the
cursor == start_blockbug, they could still loop indefinitely if a server misbehaves and returns the same cursor repeatedly with non-empty results. Adding the sameprevious_cursorpattern would provide defensive consistency across all pagination methods.This is optional and can be deferred if you'd prefer to keep this PR focused on the specific bug.
Also applies to: 434-439
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/blockchain/clients/stellar/client.rs` around lines 293 - 298, Add stall-detection in get_transactions and get_events by tracking a previous_cursor variable alongside cursor and breaking the pagination loop if response_body["result"]["cursor"] equals previous_cursor (i.e., cursor did not advance) to avoid infinite loops when the server returns the same cursor with non-empty results; update the loop in both functions to set previous_cursor = cursor before assigning cursor = response_body["result"]["cursor"] and check equality after assignment to break out defensively.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/services/blockchain/clients/stellar/client.rs`:
- Around line 293-298: Add stall-detection in get_transactions and get_events by
tracking a previous_cursor variable alongside cursor and breaking the pagination
loop if response_body["result"]["cursor"] equals previous_cursor (i.e., cursor
did not advance) to avoid infinite loops when the server returns the same cursor
with non-empty results; update the loop in both functions to set previous_cursor
= cursor before assigning cursor = response_body["result"]["cursor"] and check
equality after assignment to break out defensively.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f7eab22d-9938-42f1-8272-ae9f65b03019
📒 Files selected for processing (2)
src/services/blockchain/clients/stellar/client.rstests/integration/blockchain/clients/stellar/client.rs
There was a problem hiding this comment.
Pull request overview
This PR adjusts Stellar get_blocks pagination termination logic to avoid incorrect early termination/infinite-loop behavior and adds a regression integration test around cursor handling.
Changes:
- Track the previous pagination cursor in
StellarClient::get_blocksand stop when the cursor stops advancing. - Remove the prior termination condition that treated
cursor == start_blockas end-of-range. - Add an integration regression test ensuring pagination continues even when the first page cursor equals
start_block.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/services/blockchain/clients/stellar/client.rs |
Updates get_blocks pagination break conditions by tracking previous_cursor. |
tests/integration/blockchain/clients/stellar/client.rs |
Adds a regression integration test covering the cursor == start_block pagination case. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #464 +/- ##
=====================================
Coverage 95.5% 95.5%
=====================================
Files 97 97
Lines 31586 31586
=====================================
Hits 30186 30186
Misses 1400 1400
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
Testing Process
Checklist
Note
If you are using Monitor in your stack, consider adding your team or organization to our list of Monitor Users in the Wild!
Summary by CodeRabbit
Bug Fixes
Tests