fix: resolve logical conflict between dash#6691 and dash#6775, compile error #6783
Conversation
dash#6775 updates UniValue, which changed the syntax for fetching integers, which created a divergence of expected behavior from dash#6691
kwvg
changed the title
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
WalkthroughThe test case Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (1)
src/test/llmq_commitment_tests.cpp (1)
145-146: Potential signed/unsigned mismatch – consider aligning the extracted JSON type withCountSigners()/CountValidMembers()
getInt<int>()returns a signedint, whereasCountSigners()andCountValidMembers()return an unsigned type (size_t). The implicit signed/unsigned conversion inBOOST_CHECK_EQUALcompiles but can trigger compiler warnings and masks overflow corner-cases. Prefer extracting the value as an unsigned integer (or explicitly casting the counters) to keep the comparison type-safe.- BOOST_CHECK_EQUAL(json["signersCount"].getInt<int>(), commitment.CountSigners()); - BOOST_CHECK_EQUAL(json["validMembersCount"].getInt<int>(), commitment.CountValidMembers()); + BOOST_CHECK_EQUAL(json["signersCount"].getInt<unsigned int>(), commitment.CountSigners()); + BOOST_CHECK_EQUAL(json["validMembersCount"].getInt<unsigned int>(), commitment.CountValidMembers());Alternatively, cast the counters to
intif you are certain the counts will always fit into a signed 32-bit range.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/llmq_commitment_tests.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/test/llmq_commitment_tests.cpp
src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Files:
src/test/llmq_commitment_tests.cpp
🧠 Learnings (2)
📓 Common learnings
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#6752
File: src/wallet/load.cpp:164-164
Timestamp: 2025-07-17T15:48:29.418Z
Learning: In Dash Core, kwvg prefers using assert() statements to document expected behavior in code paths, especially when the called function also has internal asserts. This is used as a defensive programming pattern rather than requiring conditional error handling.
src/test/llmq_commitment_tests.cpp (1)
Learnt from: kwvg
PR: #6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: linux64_tsan-test / Test source
kwvg
changed the title
3 participants
Additional Information
dash#6691 and dash#6775 were merged into
developin that order, neither conflicting with the other. dash#6775 updated the UniValue subtree before it was unsubtree'd and subsequent improvements were backported. To enable this, a syntax change was required which replacedget_int()withgetInt<int>(), which, the code introduced in dash#6691 didn't use as it was merged before dash#6775.As it was new code, this was not registered as a merge conflict but this logical conflict caused
developto fail (build). This pull request remedies that issue.Breaking Changes
None expected
Checklist