Skip to content

Conversation

@levonpetrosyan93
Copy link
Contributor

No description provided.

…ieval

Updated the getusedcoinstagstxhashes function to replace the use of GetSpends with GetSpendsMobile, changing the data structure from an unordered_map to a vector of pairs for improved performance and compatibility.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 28, 2026

Walkthrough

Reworked the getusedcoinstagstxhashes RPC in src/rpc/misc.cpp to require -mobile, replace unordered_map tag storage with a vector of pairs, and use GetSpendsMobile() instead of GetSpends(); iteration and serialization updated to the new vector layout.

Changes

Cohort / File(s) Summary
RPC mobile tag handling
src/rpc/misc.cpp
Added runtime guard requiring -mobile; replaced unordered_map tag storage with std::vector<std::pair<...>>; switched spends source from GetSpends() to GetSpendsMobile() and updated iteration/serialization to use the vector-based tag structure.

Sequence Diagram(s)

(omitted — change is localized and does not introduce a multi-component sequential flow requiring visualization)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • psolstice

Poem

🐰 I swapped the map for tidy rows,
Mobile-only now the code goes,
Spends hop in a different lane,
Tags lined up — neat on the plain,
A tiny change, a joyful pose! 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is empty despite the repository template requiring a mandatory 'PR intention' section explaining what the PR does and what issue it solves. Add a description following the template: explain that this PR refactors getusedcoinstagstxhashes to use GetSpendsMobile with a -mobile flag guard, switches to vector-based tag storage, and clarify the issue being addressed.
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 directly and clearly describes the main refactoring change: using GetSpendsMobile instead of the previous approach in getusedcoinstagstxhashes.

✏️ 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

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 a review from psolstice January 28, 2026 13:14
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: 1

🤖 Fix all issues with AI agents
In `@src/rpc/misc.cpp`:
- Around line 1589-1596: Add the same -mobile flag guard used by other mobile
RPCs to the getusedcoinstagstxhashes RPC: at the start of the
getusedcoinstagstxhashes function check GetBoolArg("-mobile", false) and if
false throw std::runtime_error("Please rerun Firo with -mobile "); do this
before accessing spark::CSparkState::GetState() and calling
GetSpendsMobile()/GetSpendTxIds() to match the behavior in zagot functions like
getanonymityset/getsparkanonymityset and the RPC help text.

navidR
navidR previously approved these changes Jan 28, 2026
@reubenyap
Copy link
Member

bugbot run

psolstice
psolstice previously approved these changes Jan 28, 2026
Added a check in the getusedcoinstagstxhashes function to ensure that the application is run with the -mobile flag. If the flag is not set, an error is thrown prompting the user to rerun with the appropriate argument.
@levonpetrosyan93 levonpetrosyan93 dismissed stale reviews from psolstice and navidR via a478906 January 28, 2026 14:11
@coderabbitai coderabbitai bot requested a review from psolstice January 28, 2026 14:12
@reubenyap reubenyap merged commit 3bd0730 into master Jan 28, 2026
23 of 26 checks passed
@reubenyap reubenyap deleted the getusedcoinstagstxhashes_fix branch January 28, 2026 18:29
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.

5 participants