backport: 0.25 batch 587 #6802
backport: 0.25 batch 587 #6802
Conversation
Co-authored-by: fanquake <[email protected]>
Co-authored-by: Claude Code <[email protected]> Co-authored-by: Claude <[email protected]>
…ubuntu qt5 build Co-authored-by: fanquake <[email protected]>
Co-authored-by: fanquake <[email protected]>
Co-authored-by: Ryan Ofsky <[email protected]>
Co-authored-by: Fabian Jahr <[email protected]>
Co-authored-by: fanquake <[email protected]>
Co-authored-by: Claude Code <[email protected]>
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
WalkthroughThis set of changes updates scripts, documentation, and source files across several areas. The Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ 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. 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)
ci/test/04_install.sh (1)
64-66: Wildcardsafe.directoryweakens Git trust model – consider scopingSetting
git config --global --add safe.directory "*"disables Git’s owner-check for every path inside the container. While the CI job runs in a controlled environment, narrowing the rule to the mounted source directory achieves the same goal with less blast-radius:
-$DOCKER_CI_CMD_PREFIX_ROOT git config --global --add safe.directory "*" +$DOCKER_CI_CMD_PREFIX_ROOT git config --global --add safe.directory "$BASE_ROOT_DIR"(or the precise path that may differ after
rsync).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
ci/dash/lint-tidy.sh(1 hunks)ci/test/04_install.sh(1 hunks)doc/build-unix.md(1 hunks)src/bitcoin-cli.cpp(1 hunks)src/index/base.cpp(1 hunks)src/test/argsman_tests.cpp(2 hunks)test/fuzz/test_runner.py(11 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
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/bitcoin-cli.cppsrc/test/argsman_tests.cppsrc/index/base.cpp
doc/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the doc directory (documentation)
Files:
doc/build-unix.md
ci/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the ci directory (continuous integration)
Files:
ci/test/04_install.shci/dash/lint-tidy.sh
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/argsman_tests.cpp
🧠 Learnings (15)
📓 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: UdjinM6
PR: dashpay/dash#6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.
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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : 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
Learnt from: kwvg
PR: dashpay/dash#6754
File: contrib/containers/guix/docker-compose.yml:18-19
Timestamp: 2025-07-14T10:11:05.011Z
Learning: In the Guix build process for Dash Core, the `guix.sigs` directory requires write access as signatures are written to it during the build process, and `dash-detached-sigs` may be updated with `git pull` operations, so both directories need rw permissions in the Docker volume mounts.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to 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
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
📚 Learning: in dash core, error() is used (instead of state.invalid()) to report critical failures that require ...
Learnt from: kwvg
PR: dashpay/dash#6530
File: src/validation.cpp:4514-4516
Timestamp: 2025-01-14T08:40:00.994Z
Learning: In Dash Core, error() is used (instead of state.Invalid()) to report critical failures that require immediate user intervention, particularly for database/disk errors and data corruption scenarios. This is an established pattern seen in validation.cpp.
Applied to files:
src/bitcoin-cli.cpp
📚 Learning: dash core uses bitcoin_ prefix for header guards as the standard convention, inherited from bitcoin ...
Learnt from: kwvg
PR: dashpay/dash#6761
File: src/chainlock/signing.h:5-6
Timestamp: 2025-07-23T09:30:34.631Z
Learning: Dash Core uses BITCOIN_ prefix for header guards as the standard convention, inherited from Bitcoin Core. Only a few BLS-specific files in src/bls/ use DASH_ prefix. The vast majority of files (385+) use BITCOIN_ prefix.
Applied to files:
src/bitcoin-cli.cpp
📚 Learning: in dash masternode proregtx validation, platform ports (platformhttpport and platformp2pport) are ma...
Learnt from: kwvg
PR: dashpay/dash#6665
File: src/evo/deterministicmns.cpp:1284-1287
Timestamp: 2025-06-16T17:59:55.669Z
Learning: In Dash masternode ProRegTx validation, platform ports (platformHTTPPort and platformP2PPort) are mandatory and must be non-zero, while netInfo (ipAndPort) is optional. This means that even if an empty netInfo returns 0 from GetPrimary().GetPort(), it won't cause false positives in port duplication checks since platform ports cannot be 0.
Applied to files:
src/bitcoin-cli.cpp
📚 Learning: the dash core deploy container runs a single process and doesn't require complex init systems, servi...
Learnt from: PastaPastaPasta
PR: dashpay/dash#6564
File: contrib/containers/deploy/Dockerfile:1-1
Timestamp: 2025-02-11T19:25:05.487Z
Learning: The Dash Core deploy container runs a single process and doesn't require complex init systems, service supervision, or system utilities that Phusion baseimage provides, making vanilla Ubuntu a more suitable base image choice.
Applied to files:
doc/build-unix.mdci/test/04_install.sh
📚 Learning: in the guix build process for dash core, the `guix.sigs` directory requires write access as signatur...
Learnt from: kwvg
PR: dashpay/dash#6754
File: contrib/containers/guix/docker-compose.yml:18-19
Timestamp: 2025-07-14T10:11:05.011Z
Learning: In the Guix build process for Dash Core, the `guix.sigs` directory requires write access as signatures are written to it during the build process, and `dash-detached-sigs` may be updated with `git pull` operations, so both directories need rw permissions in the Docker volume mounts.
Applied to files:
doc/build-unix.mdci/test/04_install.shci/dash/lint-tidy.sh
📚 Learning: the deploy container for dash core is designed to be minimal, requiring only basic container functio...
Learnt from: PastaPastaPasta
PR: dashpay/dash#6564
File: contrib/containers/deploy/Dockerfile:1-1
Timestamp: 2025-02-11T19:25:05.487Z
Learning: The deploy container for Dash Core is designed to be minimal, requiring only basic container functionality without complex init systems or service management.
Applied to files:
doc/build-unix.md
📚 Learning: applies to ci/** : unless specifically prompted, avoid making changes to the ci directory (continuou...
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to ci/** : Unless specifically prompted, avoid making changes to the ci directory (continuous integration)
Applied to files:
ci/test/04_install.shci/dash/lint-tidy.sh
📚 Learning: applies to .github/** : unless specifically prompted, avoid making changes to .github (github workfl...
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to .github/** : Unless specifically prompted, avoid making changes to .github (GitHub workflows and configs)
Applied to files:
ci/test/04_install.sh
📚 Learning: always run linting before commits using test/lint/all-lint.py...
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Always run linting before commits using test/lint/all-lint.py
Applied to files:
ci/dash/lint-tidy.sh
📚 Learning: code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded...
Learnt from: kwvg
PR: dashpay/dash#6529
File: src/rpc/governance.cpp:1074-1089
Timestamp: 2025-02-14T15:15:58.165Z
Learning: Code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded from clang-format suggestions as they are intentionally formatted manually for better readability.
Applied to files:
ci/dash/lint-tidy.sh
📚 Learning: applies to src/**/*.{cpp,h,cc,cxx,hpp} : dash core c++ codebase must be written in c++20 and require...
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to 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
Applied to files:
ci/dash/lint-tidy.sh
📚 Learning: in the test framework consolidation pr (#6718), user kwvg prefers to limit functional changes to tho...
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.
Applied to files:
src/test/argsman_tests.cpp
📚 Learning: applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : unit tests for c++ code should b...
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : 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
Applied to files:
src/test/argsman_tests.cpp
📚 Learning: applies to src/fuzz/**/*.{cpp,h,cc,cxx,hpp} : fuzzing harnesses should be placed in src/fuzz/...
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/fuzz/**/*.{cpp,h,cc,cxx,hpp} : Fuzzing harnesses should be placed in src/fuzz/
Applied to files:
test/fuzz/test_runner.py
🔇 Additional comments (12)
src/index/base.cpp (1)
150-158: Synchronisation flag now set only after successful Commit – good fixMoving
m_synced = trueto afterCommit()prevents the index from advertising a synced state before the last DB write has succeeded, eliminating a brief window where other threads could rely on an out-of-date persisted state.doc/build-unix.md (1)
107-111: Update to Qt package list looks correctReplacing the runtime libraries with the umbrella dev package (
qtbase5-dev) is all that’s needed for building; the change aligns with current Debian/Ubuntu package naming.src/test/argsman_tests.cpp (2)
904-909: Comment typo fixed – no functional impactThe updated hint now references the correct test name; nothing else to flag.
1007-1012: Analogous comment fix acknowledgedSame minor documentation correction for the
ChainMergetest section.src/bitcoin-cli.cpp (1)
831-834: LGTM! Improved error message formatting and user guidance.The enhanced error message provides better formatting with explicit newlines and helpful guidance by suggesting
dash-cli -help. This improves the user experience without affecting the underlying error handling logic.ci/dash/lint-tidy.sh (1)
15-19: LGTM! Enhanced error handling and reporting.The improved approach captures
run-clang-tidyoutput to a file and provides better error reporting by showing errors with context and a clear failure message. This makes CI failures easier to diagnose and debug.test/fuzz/test_runner.py (6)
9-9: LGTM! Modern path handling with pathlib.Adding the
Pathimport improves path handling throughout the script, which is consistent with modern Python best practices.
45-49: LGTM! Useful configuration option for empty corpus handling.The new
--empty_min_timeargument provides valuable control over fuzzing behavior when corpus directories are empty, improving the script's flexibility.
85-85: LGTM! Consistent path object usage.Converting
corpus_dirto aPathobject enables better path manipulation and is consistent with the pathlib import.
102-103: LGTM! Configurable fuzz binary path.Using the
DASHFUZZenvironment variable makes the script more flexible while maintaining backward compatibility with a sensible default path.
106-107: LGTM! Consistent function signature refactoring.The systematic update of function signatures to use
fuzz_bininstead ofbuild_dirmakes the interfaces more explicit about their requirements and supports the configurable binary path feature.Also applies to: 172-173, 183-184, 193-195, 199-199, 238-238, 276-276
279-291: LGTM! Well-implemented empty corpus handling logic.The conditional fuzzing logic properly handles empty corpus directories using time-based fuzzing when
empty_min_timeis set, while maintaining the traditionalruns=1approach for non-empty directories. The use ofPath.iterdir()for empty directory detection is clean and efficient.
…A_MAX Co-authored-by: fanquake <[email protected]>
There was a problem hiding this comment.
CI failures are unrelated, should go away on rebase/merge (https://github.com/UdjinM6/dash/actions/runs/16738506802)
utACK d04979f
4 participants
Issue being fixed or feature implemented
batch of backports
What was done?
How Has This Been Tested?
Breaking Changes
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.