fix: clamp gobject list-prepared count#7382
Conversation
The previous arithmetic computed `vecObjects.begin() + std::max<int>(0, vecObjects.size() - nCount)`, mixing size_t with int64_t. When nCount exceeded vecObjects.size(), the unsigned subtraction wrapped to a huge value before being converted to int, producing an invalid iterator offset. Clamp nCount to the vector size with a typed std::min and use `end() - clamped` instead so count=0 returns nothing, very large or out-of-range counts return all entries, and negative counts still raise the existing RPC error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughIn Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
✅ Review complete (commit 380cb13) |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Small, focused fix for an unsigned arithmetic wraparound in gobject list-prepared. The clamp via std::min<int64_t>(nCount, vecObjects.size()) followed by end() - clamped is correct given the prior nCount < 0 guard, and the functional test now covers oversized and INT64_MAX counts. All four agent reports converge on no findings; verified against the head source.
Note: GitHub does not allow me to submit APPROVE on my own PR, so this is posted as a COMMENT while preserving the verified non-blocking state.
3 participants
Issue being fixed or feature implemented
The wallet
gobject list-preparedRPC accepted acountparameter but computedthe iterator start with mixed signed and unsigned arithmetic. Oversized counts
could wrap before iterator arithmetic and produce an invalid iterator.
What was done?
Clamp the requested count to the number of prepared governance objects before
computing the start iterator, then iterate from
end() - clamped_count.Added functional test coverage for oversized and very large count values.
How Has This Been Tested?
Ran:
git diff --check upstream/develop...HEAD code-review dashpay/dash upstream/develop 380cb1304adffb28bc3689df38921727d86b610d "Clamp gobject list-prepared count before computing vector iterators to avoid out-of-range iterator arithmetic"The pre-PR review gate returned
ship.I did not run the functional test locally because this worktree is not configured
or built with
dashd.Breaking Changes
None.
Checklist: