Skip to content

Conversation

@levonpetrosyan93
Copy link
Contributor

No description provided.

Added a check to ensure that the startNumber does not exceed the available elements in the tags vector. This prevents potential out-of-bounds access and improves the robustness of the function.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 29, 2026

Walkthrough

The PR modifies getusedcoinstagstxhashes in src/rpc/misc.cpp to handle an edge case by clamping the starting index. Instead of iterating from the beginning and filtering in-loop, the loop now starts at tags.begin() + skip where skip = min(startNumber, tags.size()), eliminating unnecessary iterations.

Changes

Cohort / File(s) Summary
Index clamping optimization
src/rpc/misc.cpp
Edge case handling for starting index in getusedcoinstagstxhashes; clamps startNumber to available range and restructures loop to skip elements from the start rather than filter during iteration.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • firoorg/firo#1773: Also modifies getusedcoinstagstxhashes in src/rpc/misc.cpp with refactoring to use GetSpendsMobile and vector-based tag storage.

Suggested reviewers

  • navidR
  • psolstice

Poem

🐰 A starting index, once wild and free,
Now gently clamped to what it should be—
No loops that wander where none should roam,
Just hop to the right place, straight to home! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is missing entirely, but the repository requires a mandatory 'PR intention' section explaining what the PR does and what issue it solves. Add a PR description with at least the mandatory 'PR intention' section explaining what edge case is being handled and why this fix is necessary.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: handling edge cases in the getusedcoinstagstxhashes function, which aligns with the code modifications shown in the summary.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 getusedcoinstagstxhashes_fix

Tip

🧪 Unit Test Generation v2 is now available!

We have significantly improved our unit test generation capabilities.

To enable: Add this to your .coderabbit.yaml configuration:

reviews:
  finishing_touches:
    unit_tests:
      enabled: true

Try it out by using the @coderabbitai generate unit tests command on your code files or under ✨ Finishing Touches on the walkthrough!

Have feedback? Share your thoughts on our Discord thread!


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.

❤️ Share

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

@coderabbitai coderabbitai bot requested review from navidR and psolstice January 29, 2026 23:55
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/rpc/misc.cpp`:
- Around line 1602-1609: The code casts startNumber to size_t causing huge
values for negative inputs; before casting, guard against negatives by clamping
startNumber to zero (or return an RPC error) and then compute skip from the
non-negative value; e.g., check if startNumber < 0 and set a signed temporary to
0 (or reject) and then set size_t skip =
static_cast<size_t>(that_non_negative_value) before comparing with tags.size();
update the logic around startNumber, skip and the for-loop that uses tags to use
this clamped/validated value.
- Around line 1602-1609: The pagination logic in getusedcoinstagstxhashes is
inverted: instead of skipping the last startNumber elements like
getusedcoinstags, it currently skips the first ones; update the loop to use the
same end-based pagination used by getusedcoinstags (refer to the logic at
getusedcoinstags) by replacing the current skip/iteration approach with the
check used there (e.g., inside the loop use if (cmp::less((tags.size() - i - 1),
startNumber)) continue;) so the method skips newest tags and returns the same
slice semantics as getusedcoinstags.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants