backport: bitcoin#25670, #22270, #22150, #20938, #20939, #20913, bitcoin-core/gui#409, partial: bitcoin#18267, #19937, #19993, #20014, #20035, #20830, #20917, #20923, #21384, #24424, #24559, #25625 (DNM signet) #7054
Conversation
knst
changed the title
|
WalkthroughThis pull request adds a new bitcoin-util/dash-util command-line binary implementing a multithreaded "grind" command to brute-force a proof-of-work nonce for a provided block header, and wires that binary into the build, packaging, manpage generation, and test infrastructure. It expands chain support to include devnet across init, help text, docs, tests, and GUI styling, and introduces two consensus fields (nMinimumChainWork, defaultAssumeValid) with chainparams default-initialized. The changeset also adds PSBT serialization/test helpers, updates many docs, adjusts ArgsManager command registration (dropping the category parameter), and extends functional test framework hooks for the new binary. Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![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/rpc/mining.cpp (1)
795-796: LGTM!The refactoring consolidates the
consensusParamsdeclaration, eliminating redundancy and improving code clarity. The earlier declaration ensures the reference is available for all subsequent uses (UpdateTime, version bits operations, and payment block checks) without changing functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between a1c3afb and 3b143f506a70f25219ec5ee6a4b29698027608af.
📒 Files selected for processing (35)
.gitignore(1 hunks)Makefile.am(3 hunks)configure.ac(7 hunks)contrib/devtools/gen-manpages.py(1 hunks)doc/REST-interface.md(1 hunks)doc/dash-conf.md(1 hunks)doc/developer-notes.md(2 hunks)doc/man/Makefile.am(1 hunks)doc/man/bitcoin-util.1(1 hunks)doc/release-process.md(3 hunks)src/Makefile.am(2 hunks)src/bitcoin-cli.cpp(1 hunks)src/bitcoin-tx.cpp(1 hunks)src/bitcoin-util-res.rc(1 hunks)src/bitcoin-util.cpp(1 hunks)src/bitcoin-wallet.cpp(2 hunks)src/bitcoind.cpp(1 hunks)src/chainparams.cpp(1 hunks)src/chainparamsbase.cpp(1 hunks)src/consensus/params.h(1 hunks)src/qt/bitcoin.cpp(1 hunks)src/qt/networkstyle.cpp(1 hunks)src/qt/walletcontroller.cpp(1 hunks)src/rpc/mining.cpp(2 hunks)src/test/argsman_tests.cpp(2 hunks)src/test/pow_tests.cpp(1 hunks)src/util/system.cpp(3 hunks)src/util/system.h(1 hunks)test/config.ini.in(1 hunks)test/functional/feature_nulldummy.py(1 hunks)test/functional/rpc_psbt.py(2 hunks)test/functional/test_framework/messages.py(1 hunks)test/functional/test_framework/psbt.py(1 hunks)test/functional/test_framework/test_framework.py(2 hunks)test/util/data/bitcoin-util-test.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
src/**/*.{cpp,h,hpp,cc}
📄 CodeRabbit inference engine (CLAUDE.md)
Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1
Files:
src/test/argsman_tests.cppsrc/qt/networkstyle.cppsrc/bitcoin-tx.cppsrc/qt/walletcontroller.cppsrc/consensus/params.hsrc/test/pow_tests.cppsrc/chainparams.cppsrc/bitcoin-wallet.cppsrc/chainparamsbase.cppsrc/util/system.hsrc/bitcoin-cli.cppsrc/rpc/mining.cppsrc/bitcoind.cppsrc/qt/bitcoin.cppsrc/bitcoin-util.cppsrc/util/system.cpp
src/{test,wallet/test}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework
Files:
src/test/argsman_tests.cppsrc/test/pow_tests.cpp
src/qt/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
GUI implementation in src/qt/ must use Qt 5
Files:
src/qt/networkstyle.cppsrc/qt/walletcontroller.cppsrc/qt/bitcoin.cpp
test/functional/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Functional tests in test/functional/ must be written in Python (minimum version specified in .python-version) and depend on dashd and dash-node
Files:
test/functional/feature_nulldummy.pytest/functional/test_framework/test_framework.pytest/functional/test_framework/messages.pytest/functional/rpc_psbt.pytest/functional/test_framework/psbt.py
{guix-build*,releases,**/guix-build*,releases/**,.github/**,depends/**,ci/**,contrib/**,doc/**}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not make changes to build system files (guix-build*), release artifacts, or avoid changes to .github, depends, ci, contrib, and doc directories unless specifically prompted
Files:
doc/man/bitcoin-util.1doc/REST-interface.mddoc/developer-notes.mdcontrib/devtools/gen-manpages.pydoc/release-process.mddoc/man/Makefile.amdoc/dash-conf.md
src/{validation,consensus,net_processing}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
ValidationInterface callbacks must be used for event-driven architecture to coordinate subsystems during block/transaction processing
Files:
src/consensus/params.h
🧠 Learnings (24)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 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: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.
Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.
Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Learnt from: kwvg
Repo: dashpay/dash 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.
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Dash extends Bitcoin Core through composition with minimal changes to the Bitcoin Core foundation
📚 Learning: 2025-08-11T17:16:36.654Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.
Applied to files:
.gitignorecontrib/devtools/gen-manpages.pydoc/release-process.mdtest/util/data/bitcoin-util-test.jsonsrc/Makefile.amconfigure.ac
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue,crypto/{ctaes,x11}}/** : Do not make changes to vendored dependencies: src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue} or src/crypto/{ctaes,x11}
Applied to files:
.gitignorecontrib/devtools/gen-manpages.py
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h} : Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data
Applied to files:
.gitignorecontrib/devtools/gen-manpages.pysrc/Makefile.amconfigure.ac
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Dash extends Bitcoin Core through composition with minimal changes to the Bitcoin Core foundation
Applied to files:
.gitignorecontrib/devtools/gen-manpages.pysrc/Makefile.amconfigure.ac
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h} : Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures
Applied to files:
.gitignorecontrib/devtools/gen-manpages.pysrc/Makefile.am
📚 Learning: 2025-07-23T09:30:34.631Z
Learnt from: kwvg
Repo: dashpay/dash PR: 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:
.gitignorecontrib/devtools/gen-manpages.pysrc/Makefile.amconfigure.ac
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/**/*.{cpp,h,hpp,cc} : Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1
Applied to files:
.gitignorecontrib/devtools/gen-manpages.pysrc/Makefile.am
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/**/*.{cpp,h} : NodeContext must be extended with Dash-specific managers for each subsystem during initialization
Applied to files:
.gitignore
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to test/functional/**/*.py : Functional tests in test/functional/ must be written in Python (minimum version specified in .python-version) and depend on dashd and dash-node
Applied to files:
.gitignore
📚 Learning: 2025-12-01T18:13:21.314Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 7018
File: test/lint/lint-github-actions.py:1-9
Timestamp: 2025-12-01T18:13:21.314Z
Learning: In the Dash repository, the file test/util/data/non-backported.txt should only include C++ files (.cpp or .h) because it is used for running clang-format. Other file types (such as Python files, .ui files, etc.) should not be added to this list.
Applied to files:
.gitignorecontrib/devtools/gen-manpages.py
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,txmempool}/**/*.{cpp,h} : Block validation and mempool handling must use extensions to Bitcoin Core mechanisms for special transaction validation and enhanced transaction relay
Applied to files:
src/bitcoin-tx.cppsrc/test/pow_tests.cppsrc/rpc/mining.cppsrc/bitcoind.cpp
📚 Learning: 2025-02-06T14:34:30.466Z
Learnt from: kwvg
Repo: dashpay/dash PR: 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.
Applied to files:
src/bitcoin-tx.cppsrc/bitcoind.cpp
📚 Learning: 2025-08-08T07:01:47.332Z
Learnt from: knst
Repo: dashpay/dash PR: 6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.332Z
Learning: In src/wallet/rpc/wallet.cpp, the upgradetohd RPC now returns a UniValue string message (RPCResult::Type::STR) instead of a boolean, including guidance about mnemonic backup and null-character passphrase handling; functional tests have been updated to assert returned strings in several cases.
Applied to files:
src/qt/walletcontroller.cppsrc/bitcoin-wallet.cppsrc/chainparamsbase.cppsrc/rpc/mining.cppsrc/bitcoind.cppsrc/qt/bitcoin.cpp
📚 Learning: 2025-11-04T18:23:28.175Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/governance/classes.cpp:147-154
Timestamp: 2025-11-04T18:23:28.175Z
Learning: In src/governance/classes.cpp, CSuperblock::GetPaymentsLimit intentionally uses synthetic difficulty constants (nBits = 1 for mainnet, powLimit for networks with min difficulty) and simple height-based V20 activation checks instead of actual chain block data. This is by design because superblocks themselves are "synthetic" governance payment blocks, not regular mined blocks.
Applied to files:
src/consensus/params.hsrc/chainparams.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/wallet/**/*.{cpp,h} : Wallet implementation must use Berkeley DB and SQLite
Applied to files:
test/config.ini.in
📚 Learning: 2025-06-09T16:43:20.996Z
Learnt from: kwvg
Repo: dashpay/dash 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.
Applied to files:
src/test/pow_tests.cpptest/functional/rpc_psbt.py
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{test,wallet/test}/**/*.{cpp,h} : Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework
Applied to files:
src/test/pow_tests.cpp
📚 Learning: 2025-11-04T18:24:27.241Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.
Applied to files:
src/chainparams.cppdoc/release-process.md
📚 Learning: 2025-06-06T11:53:09.094Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6665
File: src/evo/providertx.h:82-82
Timestamp: 2025-06-06T11:53:09.094Z
Learning: In ProTx serialization code (SERIALIZE_METHODS), version checks should use hardcoded maximum flags (/*is_basic_scheme_active=*/true, /*is_extended_addr=*/true) rather than deployment-based flags. This is because serialization code should be able to deserialize any structurally valid ProTx up to the maximum version the code knows how to handle, regardless of current consensus validity. Validation code, not serialization code, is responsible for checking whether a ProTx version is consensus-valid based on deployment status.
Applied to files:
src/chainparams.cpp
📚 Learning: 2025-02-14T15:19:17.218Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6529
File: src/wallet/rpcwallet.cpp:3002-3003
Timestamp: 2025-02-14T15:19:17.218Z
Learning: The `GetWallet()` function calls in `src/wallet/rpcwallet.cpp` are properly validated with null checks that throw appropriate RPC errors, making additional validation unnecessary.
Applied to files:
src/bitcoin-wallet.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files
Applied to files:
doc/release-process.md
📚 Learning: 2025-10-05T20:38:28.457Z
Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.
Applied to files:
doc/release-process.md
📚 Learning: 2025-11-17T19:11:39.159Z
Learnt from: knst
Repo: dashpay/dash PR: 6019
File: test/functional/mocks/signer.py:52-52
Timestamp: 2025-11-17T19:11:39.159Z
Learning: In Dash, address prefixes follow this pattern: addresses starting with 'y' are used for test networks (regtest, devnet, testnet), while addresses starting with 'X' are used for mainnet. This is the opposite of some other cryptocurrencies' conventions.
Applied to files:
doc/dash-conf.md
🧬 Code graph analysis (8)
test/functional/feature_nulldummy.py (1)
test/functional/test_framework/test_framework.py (1)
BitcoinTestFramework(101-1144)
src/test/pow_tests.cpp (1)
src/test/argsman_tests.cpp (2)
args(68-73)args(68-68)
src/bitcoin-wallet.cpp (1)
src/dummywallet.cpp (1)
argsman(31-31)
src/util/system.h (1)
src/util/system.cpp (2)
AddCommand(694-704)AddCommand(694-694)
test/functional/test_framework/messages.py (1)
test/functional/test_framework/psbt.py (2)
deserialize(73-84)deserialize(105-112)
test/functional/rpc_psbt.py (3)
test/functional/test_framework/messages.py (18)
COutPoint(453-471)CTransaction(527-602)CTxIn(474-501)CTxOut(504-524)serialize(262-266)serialize(322-333)serialize(419-423)serialize(443-447)serialize(464-468)serialize(491-496)serialize(515-519)serialize(563-572)serialize(643-651)serialize(689-693)serialize(801-813)serialize(926-930)serialize(960-969)serialize(1053-1059)test/functional/test_framework/psbt.py (5)
PSBT(96-131)PSBTMap(67-94)serialize(86-94)serialize(114-124)to_base64(126-127)test/functional/test_framework/util.py (3)
assert_approx(32-41)find_output(582-591)random_bytes(314-316)
src/bitcoin-util.cpp (2)
src/util/system.cpp (8)
SetupHelpOptions(843-847)SetupHelpOptions(843-843)HelpRequested(838-841)HelpRequested(838-838)SetupEnvironment(1417-1442)SetupEnvironment(1417-1417)PrintExceptionContinue(864-869)PrintExceptionContinue(864-864)src/chain.h (1)
nBits(190-190)
test/functional/test_framework/psbt.py (1)
test/functional/test_framework/messages.py (36)
CTransaction(527-602)deser_string(119-121)from_binary(236-246)ser_compact_size(95-105)deserialize(258-260)deserialize(310-320)deserialize(415-417)deserialize(439-441)deserialize(460-462)deserialize(485-489)deserialize(511-513)deserialize(551-561)deserialize(633-641)deserialize(685-687)deserialize(785-799)deserialize(921-924)deserialize(949-958)deserialize(1047-1051)deserialize(1088-1090)deserialize(1110-1113)serialize(262-266)serialize(322-333)serialize(419-423)serialize(443-447)serialize(464-468)serialize(491-496)serialize(515-519)serialize(563-572)serialize(643-651)serialize(689-693)serialize(801-813)serialize(926-930)serialize(960-969)serialize(1053-1059)serialize(1092-1096)serialize(1115-1120)
⏰ 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). (2)
- GitHub Check: linux64_multiprocess-test / Test source
- GitHub Check: linux64_tsan-test / Test source
🔇 Additional comments (41)
test/functional/feature_nulldummy.py (1)
49-49: LGTM!Cosmetic formatting change that maintains the correct PEP 8 convention of two blank lines between top-level definitions. No functional impact.
.gitignore (1)
15-15: Ignore rule for dash-util binary is appropriateAdding
src/dash-utilalongside other built binaries keeps the tree clean and matches existing patterns. No issues.src/bitcoin-cli.cpp (1)
180-186: Clarified chain-settings comment matches existing behaviorThe new comment correctly documents that
BaseParams()-dependent calls are only valid afterSelectBaseParams(gArgs.GetChainName()); no behavioral change. Looks good.src/bitcoind.cpp (1)
164-169: Chain settings comment in AppInit is accurateThe updated comment correctly describes the
SelectParams(args.GetChainName())precondition forParams()usage; no functional impact.src/test/argsman_tests.cpp (1)
758-764: DEVNET inclusion in ArgsMerge matrix and updated checksum are consistentExtending
ForEachMergeSetupto iterate over{MAIN, TESTNET, DEVNET}for bothsectionandnetworkis coherent with adding DEVNET-aware settings behavior, and the newout_sha_hexgolden value reflects the expanded combination space. As long as this hash was regenerated from a fresh run ofutil_ArgsMerge, the regression test remains valid.If not already done on this branch, please re-run
./test_dash --run_test=argsman_tests/util_ArgsMerge(optionally withARGS_MERGE_TEST_OUTset) to confirm the checksum matches the current behavior.Also applies to: 910-914
doc/REST-interface.md (1)
6-7: REST port documentation now covers all networksListing explicit default REST/RPC ports for mainnet (9998), testnet (19998), devnet (19788), and regtest (19898) improves clarity. Assuming these match the values in chainparams for each network, this is correct.
Please double-check these port numbers against the current
GetDefaultPort()/ RPC port definitions inchainparams*.cppto keep the doc in sync if those ever change.src/bitcoin-tx.cpp (1)
93-99: Clarified chain-settings comment in AppInitRawTxThe comment now correctly notes that
Params()usage must followSelectParams(gArgs.GetChainName()). Behavior is unchanged and consistent with bitcoind/bitcoin-cli.test/functional/test_framework/messages.py (1)
235-247: from_binary helper aligns with existing deserialize patterns
from_binary(cls, stream)cleanly supports bothBytesIO-style streams and rawbytes, and the “consume-all” assertion for byte inputs is useful in tests. Current callers (PSBTMap,CTransactionin psbt.py) satisfy the nullary-ctor +.deserialize(f)contract, so this integrates well.src/util/system.h (1)
427-427: Simplified AddCommand API matches implementationChanging
ArgsManager::AddCommandto take just(cmd, help)and implicitly register underOptionsCategory::COMMANDSaligns with thesystem.cppimplementation and centralizes command categorization. This should be source-compatible after updating all call sites.src/test/pow_tests.cpp (1)
184-235: Chainparams sanity helper and tests look correctThe new
sanity_check_chainparams()helper and the fourChainParams_*_sanitytests correctly enforce basic consensus invariants (genesis hash, timespan/spacing divisibility, powLimit range, overflow guard), with an intentional, documented devnet exception. This matches upstream behavior and should help catch misconfigured params without introducing new risk.src/bitcoin-wallet.cpp (1)
44-49: Wallet tool command registration and chain comment are consistent with new args API
AddCommandcalls have been updated to the new(cmd, help)signature and the nearby comment now correctly refers to generic chain settings. No behavioral changes or issues spotted.Also applies to: 90-92
test/functional/rpc_psbt.py (1)
13-27: PSBT test extensions correctly exercise preimage decoding and PSBT compatibilityThe new imports and PSBT-focused tests are well-structured:
- PSBTs are constructed via the new
test_framework.psbthelpers with consistentvin/voutand map counts.- Per-input RIPEMD160/SHA256/HASH160/HASH256 preimage checks validate that
decodepsbtsurfaces preimages on the correct inputs and under the expected keys.- The combine test properly distinguishes incompatible PSBTs (different serialized transactions) from compatible, identical ones.
No correctness or flakiness concerns found.
Also applies to: 33-35, 523-563
src/util/system.cpp (1)
289-295: ArgsManager devnet wiring and command handling are coherent and thread‑safe
- DEVNET is now treated as a first-class config section, avoiding false “unrecognized section” reports.
AddCommandcorrectly centralizes command registration underOptionsCategory::COMMANDSwithcs_argslocking and duplicate detection.- Updated
GetChainName()logic cleanly enforces that only one of-regtest,-testnet,-devnet(treated as presence, not boolean) or-chainmay be set, and returnsDEVNETwhen-devnetis present, which matches the new-devnet=<name>semantics.No issues identified.
Also applies to: 694-704, 1104-1133
test/functional/test_framework/psbt.py (1)
1-131: PSBT helper module correctly models (de)serialization for testsThe new
test_framework.psbtmodule provides a faithful PSBT representation:
- Constants cover the standard PSBT global/input/output key types.
PSBTMap’s map encoding/decoding matches BIP174 (varint length, key/value pairs, 0-length key terminator, 1-byte keys normalized to ints).PSBTenforces a valid unsigned transaction in the global map and keeps input/output maps aligned withCTransactionvin/vout counts.This slots cleanly into the existing test framework and is suitable for the new PSBT tests.
src/chainparamsbase.cpp (1)
20-25: -chain help text now correctly includes devnetThe updated
-chainhelp string accurately listsdevnetalongsidemain,test, andregtest, consistent with the supported chains. No behavioral impact.doc/man/bitcoin-util.1 (1)
1-5: Placeholder dash-util man page is acceptableThe new
bitcoin-util.1stub correctly documentsdash-utilas a placeholder and points to contrib/devtools for regeneration after releases. This is consistent with existing manpage workflows.doc/man/Makefile.am (1)
19-21: Manpage build wiring for dash-util is consistent with other binariesConditionally adding
bitcoin-util.1todist_man1_MANSunderBUILD_BITCOIN_UTILmirrors the pattern used for other binaries and correctly ties manpage distribution to the new utility.doc/dash-conf.md (1)
34-34: LGTM!The documentation correctly reflects the addition of devnet support, listing
[devnet]alongside the existing network section headers.src/qt/bitcoin.cpp (1)
615-615: LGTM!The comment update generalizes the description appropriately, moving from specific command-line flags to the broader concept of "chain settings."
test/config.ini.in (1)
22-22: LGTM!The addition of the
ENABLE_BITCOIN_UTILconfiguration flag follows the established pattern and properly integrates the new dash-util component into the test configuration.src/qt/walletcontroller.cpp (1)
452-457: LGTM!The change improves UI consistency by using a static title ("Load Wallets") while keeping the dynamic label ("Loading wallets…"). This pattern aligns with other wallet activities like
CreateWalletActivityandOpenWalletActivity.src/qt/networkstyle.cpp (1)
28-28: LGTM!The color parameter changes provide visual distinction for devnet (orange hue) from testnet (cyan hue), making it easier for users to identify which network they're connected to in the UI.
contrib/devtools/gen-manpages.py (1)
15-15: LGTM!Enabling manpage generation for
dash-utilproperly integrates the new utility into the documentation build process.doc/developer-notes.md (2)
18-18: LGTM!Table of contents accurately reflects the expanded section covering devnet alongside testnet and regtest.
401-409: LGTM!The documentation updates properly explain the devnet option and when developers should use it versus testnet or regtest. The guidance is clear and helpful.
src/rpc/mining.cpp (1)
449-449: LGTM!The documentation correctly reflects that the
chainfield can now return "devnet" in addition to main, test, and regtest.src/consensus/params.h (1)
174-177: Addition of nMinimumChainWork/defaultAssumeValid fields is soundThe new consensus fields are typed consistently, placed logically next to the difficulty interval, and are initialized for each network in
src/chainparams.cpp. No correctness or ABI concerns here.src/bitcoin-util-res.rc (1)
1-35: dash-util VERSIONINFO resource mirrors existing pattern and is correctThe VERSIONINFO block uses the usual client version macros, language/translation tuple, and consistent product/file metadata (
dash-util). This should compile cleanly under windres and embed the expected version info.test/util/data/bitcoin-util-test.json (1)
2-31: dash-util CLI error-path test vectors are well-formedThe new
./dash-utilcases cover invalid commands and missing/invalid grind headers, with JSON structure and expectedreturn_code/error_txtfields matching the existing dash-tx test style. Nothing problematic here.test/functional/test_framework/test_framework.py (1)
273-278: bitcoin-util/dash-util wiring and skip helpers integrate cleanlyThe new
dash-utilbinary mapping inset_binary_paths, plusskip_if_no_bitcoin_util()andis_bitcoin_util_compiled(), follow the existing CLI/wallet-tool patterns and rely on theENABLE_BITCOIN_UTILconfig flag as expected. No behavioral or compatibility issues spotted.Also applies to: 1074-1077, 1126-1128
Makefile.am (1)
23-32: BITCOIN_UTIL_BIN target and dist wiring are consistent with existing binariesDefining
BITCOIN_UTIL_BIN, adding its FORCE rule, including it in the Windows strip/install step, and listingbitcoin-util-test.jsonplus the util test runner inEXTRA_DISTall mirror patterns used for the other binaries and test data. This should integrate the new utility cleanly into builds and releases.Also applies to: 76-86, 143-166, 252-300
src/chainparams.cpp (1)
822-823: Regtest nMinimumChainWork/defaultAssumeValid reset to zero is appropriateSetting both values to
uint256{}for regtest disables IBD/assumevalid shortcuts on the local test chain, which aligns with typical regtest behavior and avoids any consensus impact on public networks.doc/release-process.md (1)
15-18: Release-process updates for chainparams and size/tx stats look coherentThe added guidance around maintaining
nMinimumChainWork/defaultAssumeValid,m_assumed_blockchain_size/m_assumed_chain_state_size, andchainTxDatalines up with how these fields are used inchainparams.cpp, and the GiB-based size instructions for mainnet/testnet are clear and actionable.Also applies to: 32-38, 290-297
configure.ac (1)
37-37: LGTM - Build configuration for dash-util is properly integrated.The autoconf plumbing for the new
dash-utilbinary follows the established patterns for other utility targets (dash-cli,dash-tx,dash-wallet). The variable naming, conditional logic, and substitution are all consistent with the existing infrastructure.Also applies to: 766-771, 1774-1776, 1991-1991
src/Makefile.am (3)
1127-1127: Missing$(PTHREAD_FLAGS)in LDFLAGS for a multithreaded binary.The
dash-utilbinary usesstd::threadfor parallel nonce grinding (seebitcoin-util.cpplines 122-129), but unlikedash-clianddash-tx, the LDFLAGS are missing$(PTHREAD_FLAGS). This could cause linker errors on platforms where pthread linkage isn't implicit.-dash_util_LDFLAGS = $(LDFLAGS_WRAP_EXCEPTIONS) $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +dash_util_LDFLAGS = $(LDFLAGS_WRAP_EXCEPTIONS) $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(PTHREAD_FLAGS)Please verify this builds correctly on all target platforms, or confirm if this matches upstream Bitcoin Core's configuration.
147-149: Build conditional block properly integrated.The
BUILD_BITCOIN_UTILconditional follows the established pattern used by other utility binaries in the Makefile.
1133-1140: Verify Dash-specific library dependencies.Comparing with
dash_tx_LDADD, thedash_util_LDADDis missing$(LIBDASHBLS)and$(GMP_LIBS). SinceLIBBITCOIN_COMMONincludeschainparams.cppwhich may have Dash-specific dependencies (BLS for masternodes/LLMQs), verify whether this causes undefined reference errors at link time.src/bitcoin-util.cpp (4)
80-104: Thread coordination is correctly implemented.The
grind_taskfunction properly handles multithreaded nonce grinding:
- The atomic
found.exchange(true)on line 96 ensures only one thread succeeds in claiming the solution and writing toheader_orig.nNonce- Each thread works on its own stride (offset + step*n) to avoid duplicate work
- Early exit via the
foundflag check in the outer loop prevents unnecessary computation once a solution is found
106-139: Grind command implementation looks correct.The function properly:
- Validates input argument count
- Decodes the hex block header with error handling
- Spawns worker threads based on hardware concurrency
- Joins all threads before checking results
- Serializes the result back to hex on success
151-163: Main initialization follows established patterns.The initialization sequence correctly:
- Uses
gArgsreference for argument management- Calls
SetupEnvironment()for platform-specific setup- Handles exceptions with
PrintExceptionContinue- Returns early on help/version or initialization failure
1-27: New utility file structure is appropriate.The includes and global setup follow the patterns established by other utility binaries (
bitcoin-cli.cpp,bitcoin-tx.cpp). TheG_TRANSLATION_FUNnull initialization is standard for utilities that don't require translation support.
BACKPORT NOTE: tiny code unifications +extra unit tests +changed style (color) of icon for devnets 8258c4c test: some sanity checks for consensus logic (Anthony Towns) e47ad37 test: basic signet tests (Karl-Johan Alm) 4c189ab test: add small signet fuzzer (practicalswift) ec9b25d test: signet network selection tests (Karl-Johan Alm) 3efe298 signet: hard-coded parameters for Signet Global Network VI (2020-09-07) (Karl-Johan Alm) c7898bc qt: update QT to support signet network (Karl-Johan Alm) a8de47a consensus: add signet validation (Karl-Johan Alm) e8990f1 add signet chain and accompanying parameters (Karl-Johan Alm) 404682b add signet basic support (signet.cpp) (Karl-Johan Alm) a2147d7 validation: move GetWitnessCommitmentIndex to consensus/validation (Karl-Johan Alm) Pull request description: This PR is a part of BIP-325 (https://github.com/bitcoin/bips/blob/master/bip-0325.mediawiki), and is a sub-PR of bitcoin#16411. * Signet consensus (this) * Signet RPC tools (pending) * Signet utility scripts (contrib/signet) (pending) ACKs for top commit: jonatack: re-ACK 8258c4c per `git diff dbeea65 8258c4c`, only change since last review is updated `-signet*` config option naming. fjahr: re-ACK 8258c4c laanwj: ACK 8258c4c MarcoFalke: Approach ACK 8258c4c 🌵 Tree-SHA512: 5d158add96755910837feafa8214e13695b769a6aec3a2da753cf672618bef377fac43b0f4b772a87b25dd9f0c1c9b29f2789785d7a7d47a155cdcf48f7c975d
BACKPORT NOTE: It have various refactorings for existing code facaf9e doc: Document signet BIP (MarcoFalke) faf0a26 doc: Update comments for new chain settings (-signet and -chain=signet) (MarcoFalke) fae0548 fuzz: Remove needless guard (MarcoFalke) 77771a0 refactor: Remove SignetTxs::m_valid and use optional instead (MarcoFalke) fa2ad5d test: Run signet test even when wallet was not compiled (MarcoFalke) Pull request description: Some doc and test fixups for bitcoin#18267 ACKs for top commit: ajtowns: ACK facaf9e -- code review only dr-orlovsky: Reviewed & ACK bitcoin@facaf9e kallewoof: Code review ACK facaf9e Tree-SHA512: 8085027c488d84bb4bddccba78bd2d4c5af0d8e2644ee72265f1f30fa8c83f61a961d9da2c796f2940e69682291cbee7b1028b6a6ce123ad9134c0ebbf4723b0
…to show a default port
b3972bc doc: Mention signet in -help output (Hennadii Stepanov) Pull request description: ``` $ src/bitcoind -help | grep -A 4 -e '-chain=' | head -8 -chain=<chain> Use the chain <chain> (default: main). Allowed values: main, test, signet, regtest -signet Use the signet chain. Equivalent to -chain=signet. Note that the network is defined by the -signetchallenge parameter ``` ``` $ src/bitcoind -help | grep -A 3 -e '-port=' -port=<port> Listen for connections on <port> (default: 8333, testnet: 18333 signet: 38333, regtest: 18444) ``` ``` $ src/bitcoind -help | grep -A 3 -e '-rpcport=' -rpcport=<port> Listen for JSON-RPC connections on <port> (default: 8332, testnet: 18332, signet: 38332, regtest: 18443) ``` ACKs for top commit: jonatack: ACK b3972bc kallewoof: ACK b3972bc ajtowns: ACK b3972bc - skimmed code only, looks fine to me Tree-SHA512: 66c59cdc3c19e8f8a02d3f3f992ff1db06769df63244d4af62629e18aaf4a12b3b7e75e4a0b9f616033cdc4415da046053ba36fede8be145b2dc695b2aa69a02
…tion fa723e3 Initialize default-initialized uint256 consensus params to zero explicitly (MarcoFalke) fa729cd doc: Move assumed-values doxygen comments to header (MarcoFalke) fa64892 signet: Fix uninitialized read in validation (MarcoFalke) Pull request description: ACKs for top commit: practicalswift: re-ACK fa723e3: patch still looks correct kallewoof: ReACK fa723e3 theStack: re-ACK fa723e3 🍐 Tree-SHA512: db562bcc15af23bbcbf485f0bbf7564c64c144a4368230fd7682e8861d9500f6f5240351e31c560140df43b2e8456eafd9d27d1e8dd682b20afcc279a39dc329
ee701a9 doc: update developer notes for signet (Jon Atack) Pull request description: Preview with working anchor links: https://github.com/jonatack/bitcoin/blob/add-signet-to-developer-notes/doc/developer-notes.md ACKs for top commit: 0xB10C: ACK ee701a9 michaelfolkson: ACK ee701a9 MarcoFalke: ACK ee701a9 Tree-SHA512: 12f0a81a701d9fd48e1f314e7a66b865309fdcb8f139323410d35c0cfb093d23a2f899249d1b0a025078894cf4789ed7221ae42804a56ab704a8b9ad35077bac
595a34d contrib/signet: Document miner script in README.md (Anthony Towns) ff7dbdc contrib/signet: Add script for generating a signet chain (Anthony Towns) 13762bc Add bitcoin-util command line utility (Anthony Towns) 95d5d5e rpc: allow getblocktemplate for test chains when unconnected or in IBD (Anthony Towns) 81c54de rpc: update getblocktemplate with signet rule, include signet_challenge (Anthony Towns) Pull request description: Adds `contrib/signet/miner` for mining signet blocks. Adds `bitcoin-util` cli utility, with the idea being it can provide bitcoin related functionality that does not rely on the ability to access a running node. Only subcommand currently is "grind" which takes a hex-encoded header and grinds its nonce until its nBits is satisfied. Updates `getblocktemplate` to include `signet_challenge` field, and makes `getblocktemplate` require the signet rule when invoked on the signet change. Removes connectivity and IBD checks from `getblocktemplate` when applied to a test chain (regtest, testnet, signet). ACKs for top commit: laanwj: code review ACK 595a34d Tree-SHA512: 8d43297710fdc1edc58acd9b53e1bd1671e5724f7097b40ab73653715dc8becc70534c4496cbba9290f4dd6538a7a3d5830eb85f83391ea31a3bb5b9d3378cc3
…her symbols
/usr/bin/ld: libbitcoin_util.a(libbitcoin_util_a-stacktraces.o): in function `__wrap___cxa_free_exception':
/DASH/src/stacktraces.cpp:608:(.text+0x1f74): undefined reference to `__real___cxa_free_exception'
/usr/bin/ld: libbitcoin_util.a(libbitcoin_util_a-stacktraces.o): in function `__wrap___cxa_allocate_exception':
/DASH/src/stacktraces.cpp:599:(.text+0x325d): undefined reference to `__real___cxa_allocate_exception'
/usr/bin/ld: libbitcoin_util.a(libbitcoin_util_a-stacktraces.o): in function `__wrap___assert_fail':
/DASH/src/stacktraces.cpp:664:(.text+0x6a3d): undefined reference to `__real___assert_fail'
collect2: error: ld returned 1 exit status
/usr/bin/ld: libbitcoin_util.a(libbitcoin_util_a-stacktraces.o): in function `GetStackFrameInfos(std::vector<unsigned long, std::allocator<unsigned long> > const&)':
/DASH/src/stacktraces.cpp:355:(.text+0x259b): undefined reference to `backtrace_pcinfo'
/usr/bin/ld: /DASH/src/stacktraces.cpp:138:(.text+0x262a): undefined reference to `backtrace_create_state'
bc99ae7 scripted-diff: Fix typo in stub manual pages (Wladimir J. van der Laan) b5e93f8 doc: Add manual page generation for bitcoin-util (Wladimir J. van der Laan) Pull request description: - Add `-version` option to `bitcoin-util` - Add `bitcoin-util` call to `gen-manpages.sh` - Add stub manual page `bitcoin-util.1` - Add install of `bitcoin-util.1` to build system ACKs for top commit: fanquake: ACK bc99ae7 Tree-SHA512: 948df66c62bbca1cf6da26845dfa63f8f5d036a3d5744add468dd1ce7f442c123d7b0db7011c2e8e3ee6539fd391c7ee2c21b706ec81b21b02821c9501cd077d
… network name lists fc726e0 doc, rpc: add missing signet mentions in network name lists (Sebastian Falbesoner) Pull request description: This small PR adds a few missing mentions of signet w.r.t. chain enumerations: - RPC `getblockchaininfo`: result description for `"chain"` - RPC `getmininginfo`: result description for `"chain"` - REST interface documentation: - default ports listing for each chain - `"chain"` description for `chaininfo` endpoint result The instances were identified via `git grep -i "main.*test.*reg"`. ACKs for top commit: ajtowns: ACK fc726e0 -- quick code review only benthecarman: ACK fc726e0 Tree-SHA512: 62cdc6ef74fa10db75cc04b9eaf7367183f726b3fee3d21fdf741b3816669dd21508735e89da389ddac980f49773ab229263748d1399553375fefe4526361846
…oin-util c061800 build: fix RELOC_SECTION security check for bitcoin-util (fanquake) Pull request description: The binutils we use for gitian builds strips the reloc section from Windows binaries, which breaks ASLR. As a temporary workaround, export main(). This is the same workaround as bitcoin#18702 (bitcoin-cli), and will fix the currently failing security check: ```bash + make -j1 -C src check-security make: Entering directory '/home/ubuntu/build/bitcoin/distsrc-x86_64-w64-mingw32/src' Checking binary security... bitcoin-util.exe: failed RELOC_SECTION make: *** [check-security] Error 1 ``` Relevant upstream issue: https://sourceware.org/bugzilla/show_bug.cgi?id=19011 ACKs for top commit: dongcarl: ACK c061800 laanwj: ACK c061800 Tree-SHA512: a1a4da0b2cddfc377190b9044a04f42a859ca79f11ce2c2ab4c3d066a2786c34d5446d75f8eec634f308d2d3349ebbd7c9f76dcaebeeb28e471c829851592367
…g for riscv 54ce4fa build: improve macro for testing -latomic requirement (fanquake) 2c010b9 add std::atomic include to bitcoin-util.cpp (fanquake) Pull request description: Since the merge of bitcoin#19937, riscv builds have been failing, due to a link issue with [`std::atomic_exchange`](https://en.cppreference.com/w/cpp/atomic/atomic_exchange) in `bitcoin-util`: ```bash CXXLD bitcoin-util bitcoin_util-bitcoin-util.o: In function `grind_task': /home/ubuntu/build/bitcoin/distsrc-riscv64-linux-gnu/src/bitcoin-util.cpp:98: undefined reference to `__atomic_exchange_1' collect2: error: ld returned 1 exit status ``` We have a [macro](https://github.com/bitcoin/bitcoin/blob/master/build-aux/m4/l_atomic.m4) that tries to determine when `-latomic` is required, however it doesn't quite work well enough, as it's currently determining it isn't needed: ```bash ./autogen.sh ./configure --prefix=/home/ubuntu/bitcoin/depends/riscv64-linux-gnu ... checking whether std::atomic can be used without link library... yes ``` This PR adds a call to `std::atomic_exchange` to the macro, which will get us properly linked against `-latomic` on riscv: ```bash checking whether std::atomic can be used without link library... no checking whether std::atomic needs -latomic... yes ``` Also adds an `<atomic>` include to `bitcoin-util.cpp`. ACKs for top commit: laanwj: Tested ACK 54ce4fa Tree-SHA512: 963c875097ee96b131163ae8109bcf8fecf4451d20faa2f3d223f9938ea3d8d1ed5604e12ad82c2b4b1c605fd293a9b6b08fefc00dd3e68d09c49e95029c6f50
…ation 4a28510 doc: add signet to doc/bitcoin-conf.md (Jon Atack) 21b6a23 doc: add signet to share/examples/bitcoin.conf (Jon Atack) Pull request description: ACKs for top commit: jarolrod: ACK 4a28510 🥃 kristapsk: ACK 4a28510 Tree-SHA512: 2c2aa58ce5316cf986a1f3b7638b42d3939a482c38becb346d6aeab2887e57adf5c0e961de33f7721c89936881769e76b46504445428737ee810d21f4d9fb1a4
fa2b6c6 test: Remove unused node from feature_nulldummy (MarcoFalke) Pull request description: This is confusing and might even slow down the test. This reverts a change that was added a year ago in d438d60 and then the need for it was removed by 95d5d5e six months ago. Top commit has no ACKs. Tree-SHA512: 9a86792e9a634cf7bbd4e7a21b1acdfc3baba1b1962fe2b9b73848436d10351d2326dca01313c097ba2342dde7207add73e731d053c0bfa888a5d8f2b233a7cf
b3c712c contrib/signet/miner: remove debug code (Anthony Towns) 297e351 bitcoin-util: use AddCommand / GetCommand (Anthony Towns) b6d493f contrib/signet/README.md: Update miner description (Anthony Towns) e665438 contrib/signet/miner: Automatic timestamp for first block (Anthony Towns) a383ce5 contrib/signet/miner: --grind-cmd is required for calibrate (Anthony Towns) 1a45cd2 contrib/signet: Fix typos (Anthony Towns) Pull request description: Followups from bitcoin#19937 ACKs for top commit: laanwj: Code review ACK b3c712c Tree-SHA512: a1003f9ee3697438114b60872b50f4300c8b52f0d58551566eb61c421d787525807ae75be205dcab2c24358cd568f53260120880109a9d728773405ff987596f
fa4017e refactor: Pass grind args vector as const reference (MarcoFalke) fa08bc2 Remove gArgs from AppInitUtil (MarcoFalke) fa751a4 Remove unused OptionsCategory arg from AddCommand (MarcoFalke) fa3c1ee Remove unused includes from bitcoin-util (MarcoFalke) fa30492 test: Add bitcoin-util tests (MarcoFalke) fa831e7 build_msvc: Add bitcoin-util.exe (MarcoFalke) Pull request description: bitcoin-util has no tests See https://marcofalke.github.io/btc_cov/total.coverage/src/bitcoin-util.cpp.gcov.html (Coverage report showing 0%) ACKs for top commit: klementtan: Code review and tested ACK fa4017e theStack: Tested ACK fa4017e jamesob: reACK fa4017e ([`jamesob/ackr/22270.1.MarcoFalke.test_add_bitcoin_util_te`](https://github.com/jamesob/bitcoin/tree/ackr/22270.1.MarcoFalke.test_add_bitcoin_util_te)) Tree-SHA512: 68e2791239bc48d28fbb6394155c39ea0357a96ec7e4896ca579feeef1a803657165a0ef9fa3cf6e2a381e5b0ca0dafa1b594158303a04997db784201d8dd66d
01bff8f qt: Fix WalletControllerActivity progress dialog title (Shashwat) Pull request description: Throughout the GUI, the title of the window, tells about the purpose of the window. This was not true for the title of wallet loading wallet. This PR fixes this issue by renaming the wallet loading window title to 'Open Wallet' Changes introduced in this PR (Runned Bitcoin-GUI on signet network) |Master|PR| |---|---| ||| ACKs for top commit: jarolrod: ACK 01bff8f hebasto: ACK 01bff8f, tested on Linux Mint 20.2 (Qt 5.12.8). Tree-SHA512: cd21c40752eb1c0afb5ec61b8a40e900bc3aa05749963f7957ece6024e4957f5bb37e0eb4f95aac488f5e08aea51fe13b023b05d8302a08c88dcc6790410ba64
038d2a6 test: add test for signet miner script (Sebastian Falbesoner) 449b96e test: add `is_bitcoin_util_compiled` helper (Sebastian Falbesoner) dde33ec test: determine path to `bitcoin-util` in test framework (Sebastian Falbesoner) Pull request description: This PR adds a very basic test for the signet miner script (contrib/signet/miner). ~~It was based on bitcoin#24553 (merged by now) which fixes a bug (and was also the motivation to write this test).~~ The test roughly follows the steps from https://en.bitcoin.it/wiki/Signet#Custom_Signet, except that the challenge key-pair is created solely with the test framework. Calibration is also skipped, the difficulty is simply set to the first mainnet target `0x1d00ffff` (see also https://bitcoin.stackexchange.com/a/57186). ACKs for top commit: laanwj: re-ACK 038d2a6 Tree-SHA512: 150698a3c0cda3679661b47688e3b932c9761e777fdd284776b867b485db6a8895960177bd02a53f838a4c9b9bbe6a9beea8d7a5b14825b38e4e43b3177821b3
BACKPORT NOTE: Missing changes are "signet" only 74743ad Clarify in release process how to update defaultAssumeValid/nMinimumChainWork (Jon Atack) 415345d Release process: use 4096 blocks and getbestblockhash for getchaintxstats (Jon Atack) fe048f7 Specify in release process which chains need to be updated (Jon Atack) 5841476 Reorganize release process chainparams section to reduce repetition (Jon Atack) e8f8448 Clarify release process overhead note to be more actionable (Jon Atack) e538ead Release process: exclude huge files for mainnet m_assumed_blockchain_size (laanwj) b4d2d74 Release process: specify blockchain/chain_state units, reduce repetition (Jon Atack) 318655c Add missing references to signet in the release process (Jon Atack) Pull request description: Release process updates, fixes and clarifications regarding updating the chainparams: - add missing references to signet - specify specify blockchain/chainstate units, reduce repetition - exclude huge files for m_assumed_blockchain_size on mainnet - rewrite overhead note to be more actionable - reorganize the chainparams section to reduce repetition - specify which chains need to be updated - use 4096 blocks and getbestblockhash for getchaintxstats - clarify how to update defaultAssumeValid and nMinimumChainWork ACKs for top commit: laanwj: ACK 74743ad brunoerg: re-ACK 74743ad Tree-SHA512: 7fc092be739f63c5d8404add2dcbcd0c570b680ff0ab36a9b5a774b2e930717beebaa6c867ab6db6795b3c234d9016ab1ae905a78d6ea6610140a59930c43029
…r-input preimage types
Missing changes are in
contrib/signet/miner
test/functional/feature_taproot.py
71a751f test: add test for decoding PSBT with per-input preimage types (Sebastian Falbesoner)
faf4337 refactor: move helper `random_bytes` to util library (Sebastian Falbesoner)
fdc1ca3 test: add constants for PSBT key types (BIP 174) (Sebastian Falbesoner)
1b035c0 refactor: move PSBT(Map) helpers from signet miner to test framework (Sebastian Falbesoner)
7c0dfec refactor: move `from_binary` helper from signet miner to test framework (Sebastian Falbesoner)
597a4b3 scripted-diff: rename `FromBinary` helper to `from_binary` (signet miner) (Sebastian Falbesoner)
Pull request description:
This PR adds missing test coverage for the `decodepsbt` RPC in the case that a PSBT with on of the per-input preimage types (`PSBT_IN_RIPEMD160`, `PSBT_IN_SHA256`, `PSBT_IN_HASH160`, `PSBT_IN_HASH256`; see [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#Specification)) is passed. As preparation, the first four commits move the already existing helpers for (de)serialization of PSBTs and PSBTMaps from the signet miner to the test framework (in a new module `psbt.py`), which should be quite useful for further tests to easily create PSBTs.
ACKs for top commit:
achow101:
ACK 71a751f
Tree-SHA512: 04f2671612d94029da2ac8dc15ff93c4c8fcb73fe0b8cf5970509208564df1f5e32319b53ae998dd6e544d37637a9b75609f27a3685da51f603f6ed0555635fb
…txs fails 4e616d2 test: check that combining PSBTs with different txs fails (Sebastian Falbesoner) 2a428c7 test: support passing PSBTMaps directly to PSBT ctor (Sebastian Falbesoner) Pull request description: This PR adds missing test coverage for the `combinepsbt` RPC, in the case of combining two PSBTs with different transactions: https://github.com/bitcoin/bitcoin/blob/b8067cd435059eedb580975afc62c4e7a6f27321/src/psbt.cpp#L24-L27 The calling function `CombinePSBTs` checks for the false return value and then returns the transaction error string `PSBT_MISMATCH`: https://github.com/bitcoin/bitcoin/blob/b8067cd435059eedb580975afc62c4e7a6f27321/src/psbt.cpp#L433-L435 https://github.com/bitcoin/bitcoin/blob/b8067cd435059eedb580975afc62c4e7a6f27321/src/util/error.cpp#L30-L31 ACKs for top commit: instagibbs: reACK bitcoin@4e616d2 achow101: ACK 4e616d2 Tree-SHA512: 45b2b224b13b44ad69ae62e4bc20f74cab32770cf8127b026ec47a7520f7253148fdbf1fad612afece59e45a6738bef9a351ae87ea98dc83d095cc78f6db0318
Changes:
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
doc/release-process.md (1)
15-22: Clarify cross-references in chainparams update notes (and optional Dash-specific wording tweak)The new checklists correctly spell out which RPCs to use for updating
nMinimumChainWork,defaultAssumeValid,m_assumed_blockchain_size,m_assumed_chain_state_size, andchainTxDatafor mainnet/testnet, which is very helpful. Two small documentation issues are worth tightening up:
In the “Before every minor and major release” section, the bullet for
nMinimumChainWork/defaultAssumeValidrefers to “the previous step” and towindow_final_block_height above, but there is no such step or variable defined earlier in that section. If the intent is to tie these heights to thegetchaintxstatswindow used when updatingchainTxData(described later under “Before every major release”), it would be clearer to reference that section explicitly or rephrase to “using the same height you used when computingchainTxData(see the ‘Before every major release’ section below)”.In the major-release bullet for
chainTxData, the parenthetical “nBlocks of 4096 (28 days)” is inherited from Bitcoin’s ~10‑minute blocks. For Dash’s ~2.5‑minute target, 4096 blocks correspond to roughly 7 days. The procedure is still correct (the important part is using a fixed 4096‑block window), but you may want to either drop the “28 days” note or adjust it to the Dash timeframe to avoid confusion for release engineers.The new “How to calculate
m_assumed_blockchain_sizeandm_assumed_chain_state_size” subsection, with explicit directory lists for mainnet/testnet and an overhead note, looks good as-is.Based on learnings, this is a Dash-specific doc, so clarifying text here should not conflict with upstream backports.
Also applies to: 33-38, 285-300
🧹 Nitpick comments (4)
contrib/tracing/README.md (1)
179-192: Devnet wording and example usage look correctThe updated example text and
./src/dashd -devnet -reindexinvocation are consistent with Dash’s devnet usage; no further changes needed here. I’d keep the markdown style as-is to stay close to upstream unless you explicitly want to address markdownlint findings in a follow‑up doc‑only PR.contrib/devtools/gen-manpages.py (1)
10-17: Add dash-util to BINARIES and re-indent list to satisfy flake8Including
'src/dash-util'inBINARIESis correct so thathelp2mangenerates a manpage for the new binary. The new line currently triggersE122(“continuation line missing indentation”) in flake8 because the list items are not indented relative toBINARIES = [.If you want to keep flake8 clean on this change, consider re-indenting the whole list once, for example:
-BINARIES = [ -'src/dashd', -'src/dash-cli', -'src/dash-tx', -'src/dash-wallet', -'src/dash-util', -'src/qt/dash-qt', -] +BINARIES = [ + 'src/dashd', + 'src/dash-cli', + 'src/dash-tx', + 'src/dash-wallet', + 'src/dash-util', + 'src/qt/dash-qt', +]This keeps behavior identical while aligning with PEP 8 and clearing the lint on the newly added line.
doc/developer-notes.md (1)
18-18: Devnet/testnet/regtest guidance is accurate and consistentThe updated TOC entry and section correctly describe when to use
-testnet,-devnet, and-regtest, and align with how these networks behave in Dash. I’d leave the surrounding markdown structure unchanged here to keep the doc close to upstream; any markdownlint cleanups are better handled in a dedicated docs PR rather than a backport.Based on learnings, backported docs should stay as close to upstream Bitcoin text as practical.
Also applies to: 401-409
src/test/versionbits_tests.cpp (1)
427-427: Unusual comment artifact before DEVNET.The comment
/*CBaseChainParams::DEVNET,=*/immediately beforeCBaseChainParams::DEVNETappears to be an editing artifact or leftover. This doesn't affect functionality but is confusing.Consider cleaning up this line:
- for (const auto& chain_name : {CBaseChainParams::MAIN, CBaseChainParams::TESTNET, /*CBaseChainParams::DEVNET,=*/CBaseChainParams::DEVNET, CBaseChainParams::REGTEST}) { + for (const auto& chain_name : {CBaseChainParams::MAIN, CBaseChainParams::TESTNET, CBaseChainParams::DEVNET, CBaseChainParams::REGTEST}) {Based on learnings, if this matches the upstream backport exactly, it may be intentional to keep for merge conflict reduction.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 3b143f506a70f25219ec5ee6a4b29698027608af and ade733b.
📒 Files selected for processing (40)
.gitignore(1 hunks)Makefile.am(3 hunks)configure.ac(7 hunks)contrib/devtools/gen-manpages.py(1 hunks)contrib/tracing/README.md(1 hunks)doc/REST-interface.md(1 hunks)doc/dash-conf.md(1 hunks)doc/developer-notes.md(2 hunks)doc/man/Makefile.am(1 hunks)doc/man/bitcoin-util.1(1 hunks)doc/release-process.md(3 hunks)src/Makefile.am(2 hunks)src/bitcoin-cli.cpp(3 hunks)src/bitcoin-tx.cpp(1 hunks)src/bitcoin-util-res.rc(1 hunks)src/bitcoin-util.cpp(1 hunks)src/bitcoin-wallet.cpp(2 hunks)src/bitcoind.cpp(1 hunks)src/chainparams.cpp(2 hunks)src/chainparamsbase.cpp(1 hunks)src/consensus/params.h(1 hunks)src/external_signer.h(2 hunks)src/init.cpp(6 hunks)src/qt/bitcoin.cpp(1 hunks)src/qt/networkstyle.cpp(1 hunks)src/qt/walletcontroller.cpp(1 hunks)src/rpc/mining.cpp(2 hunks)src/test/argsman_tests.cpp(2 hunks)src/test/pow_tests.cpp(1 hunks)src/test/versionbits_tests.cpp(1 hunks)src/util/system.cpp(3 hunks)src/util/system.h(1 hunks)test/config.ini.in(1 hunks)test/functional/feature_nulldummy.py(1 hunks)test/functional/rpc_psbt.py(2 hunks)test/functional/test_framework/messages.py(1 hunks)test/functional/test_framework/psbt.py(1 hunks)test/functional/test_framework/test_framework.py(2 hunks)test/functional/test_framework/v2_p2p.py(1 hunks)test/util/data/bitcoin-util-test.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/external_signer.h
- test/functional/test_framework/v2_p2p.py
🚧 Files skipped from review as they are similar to previous changes (18)
- src/qt/bitcoin.cpp
- src/chainparamsbase.cpp
- .gitignore
- doc/man/bitcoin-util.1
- src/test/pow_tests.cpp
- src/chainparams.cpp
- src/bitcoind.cpp
- doc/REST-interface.md
- src/test/argsman_tests.cpp
- doc/dash-conf.md
- src/consensus/params.h
- src/bitcoin-util-res.rc
- src/Makefile.am
- src/bitcoin-tx.cpp
- test/util/data/bitcoin-util-test.json
- test/functional/feature_nulldummy.py
- test/config.ini.in
- src/util/system.cpp
🧰 Additional context used
📓 Path-based instructions (5)
{guix-build*,releases,**/guix-build*,releases/**,.github/**,depends/**,ci/**,contrib/**,doc/**}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not make changes to build system files (guix-build*), release artifacts, or avoid changes to .github, depends, ci, contrib, and doc directories unless specifically prompted
Files:
doc/man/Makefile.amcontrib/devtools/gen-manpages.pydoc/release-process.mdcontrib/tracing/README.mddoc/developer-notes.md
src/**/*.{cpp,h,hpp,cc}
📄 CodeRabbit inference engine (CLAUDE.md)
Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1
Files:
src/rpc/mining.cppsrc/bitcoin-wallet.cppsrc/init.cppsrc/util/system.hsrc/test/versionbits_tests.cppsrc/bitcoin-util.cppsrc/qt/walletcontroller.cppsrc/bitcoin-cli.cppsrc/qt/networkstyle.cpp
test/functional/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Functional tests in test/functional/ must be written in Python (minimum version specified in .python-version) and depend on dashd and dash-node
Files:
test/functional/rpc_psbt.pytest/functional/test_framework/psbt.pytest/functional/test_framework/messages.pytest/functional/test_framework/test_framework.py
src/{test,wallet/test}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework
Files:
src/test/versionbits_tests.cpp
src/qt/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
GUI implementation in src/qt/ must use Qt 5
Files:
src/qt/walletcontroller.cppsrc/qt/networkstyle.cpp
🧠 Learnings (15)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 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: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.
Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.
Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.
Learnt from: kwvg
Repo: dashpay/dash 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.
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Dash extends Bitcoin Core through composition with minimal changes to the Bitcoin Core foundation
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,txmempool}/**/*.{cpp,h} : Block validation and mempool handling must use extensions to Bitcoin Core mechanisms for special transaction validation and enhanced transaction relay
Applied to files:
src/rpc/mining.cpp
📚 Learning: 2025-02-06T14:34:30.466Z
Learnt from: kwvg
Repo: dashpay/dash PR: 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.
Applied to files:
src/rpc/mining.cppdoc/release-process.md
📚 Learning: 2025-06-09T16:43:20.996Z
Learnt from: kwvg
Repo: dashpay/dash 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.
Applied to files:
src/rpc/mining.cpptest/functional/rpc_psbt.pytest/functional/test_framework/test_framework.py
📚 Learning: 2025-08-08T07:01:47.332Z
Learnt from: knst
Repo: dashpay/dash PR: 6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.332Z
Learning: In src/wallet/rpc/wallet.cpp, the upgradetohd RPC now returns a UniValue string message (RPCResult::Type::STR) instead of a boolean, including guidance about mnemonic backup and null-character passphrase handling; functional tests have been updated to assert returned strings in several cases.
Applied to files:
src/rpc/mining.cppsrc/bitcoin-wallet.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h} : Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data
Applied to files:
contrib/devtools/gen-manpages.pyconfigure.ac
📚 Learning: 2025-08-11T17:16:36.654Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.
Applied to files:
contrib/devtools/gen-manpages.pydoc/release-process.mdconfigure.ac
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h} : Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures
Applied to files:
contrib/devtools/gen-manpages.py
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Dash extends Bitcoin Core through composition with minimal changes to the Bitcoin Core foundation
Applied to files:
contrib/devtools/gen-manpages.pyconfigure.ac
📚 Learning: 2025-07-23T09:30:34.631Z
Learnt from: kwvg
Repo: dashpay/dash PR: 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:
contrib/devtools/gen-manpages.pyconfigure.ac
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/**/*.{cpp,h,hpp,cc} : Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1
Applied to files:
contrib/devtools/gen-manpages.py
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue,crypto/{ctaes,x11}}/** : Do not make changes to vendored dependencies: src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue} or src/crypto/{ctaes,x11}
Applied to files:
contrib/devtools/gen-manpages.py
📚 Learning: 2025-12-01T18:13:21.314Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 7018
File: test/lint/lint-github-actions.py:1-9
Timestamp: 2025-12-01T18:13:21.314Z
Learning: In the Dash repository, the file test/util/data/non-backported.txt should only include C++ files (.cpp or .h) because it is used for running clang-format. Other file types (such as Python files, .ui files, etc.) should not be added to this list.
Applied to files:
contrib/devtools/gen-manpages.py
📚 Learning: 2025-10-05T20:38:28.457Z
Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.
Applied to files:
doc/release-process.md
📚 Learning: 2025-11-04T18:24:27.241Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.
Applied to files:
doc/release-process.md
🧬 Code graph analysis (7)
src/init.cpp (1)
src/chainparamsbase.cpp (2)
CreateBaseChainParams(39-51)CreateBaseChainParams(39-39)
test/functional/rpc_psbt.py (2)
test/functional/test_framework/messages.py (18)
COutPoint(453-471)CTransaction(527-602)CTxIn(474-501)CTxOut(504-524)serialize(262-266)serialize(322-333)serialize(419-423)serialize(443-447)serialize(464-468)serialize(491-496)serialize(515-519)serialize(563-572)serialize(643-651)serialize(689-693)serialize(801-813)serialize(926-930)serialize(960-969)serialize(1053-1059)test/functional/test_framework/psbt.py (5)
PSBT(96-131)PSBTMap(67-94)serialize(86-94)serialize(114-124)to_base64(126-127)
src/util/system.h (1)
src/util/system.cpp (2)
AddCommand(694-704)AddCommand(694-694)
src/bitcoin-util.cpp (2)
src/util/system.cpp (8)
SetupHelpOptions(843-847)SetupHelpOptions(843-843)HelpRequested(838-841)HelpRequested(838-838)SetupEnvironment(1416-1441)SetupEnvironment(1416-1416)PrintExceptionContinue(864-869)PrintExceptionContinue(864-864)src/chain.h (1)
nBits(190-190)
src/bitcoin-cli.cpp (1)
src/chainparamsbase.cpp (2)
CreateBaseChainParams(39-51)CreateBaseChainParams(39-39)
test/functional/test_framework/psbt.py (1)
test/functional/test_framework/messages.py (34)
CTransaction(527-602)deser_string(119-121)deserialize(258-260)deserialize(310-320)deserialize(415-417)deserialize(439-441)deserialize(460-462)deserialize(485-489)deserialize(511-513)deserialize(551-561)deserialize(633-641)deserialize(685-687)deserialize(785-799)deserialize(921-924)deserialize(949-958)deserialize(1047-1051)deserialize(1088-1090)deserialize(1110-1113)serialize(262-266)serialize(322-333)serialize(419-423)serialize(443-447)serialize(464-468)serialize(491-496)serialize(515-519)serialize(563-572)serialize(643-651)serialize(689-693)serialize(801-813)serialize(926-930)serialize(960-969)serialize(1053-1059)serialize(1092-1096)serialize(1115-1120)
test/functional/test_framework/messages.py (1)
test/functional/test_framework/psbt.py (2)
deserialize(73-84)deserialize(105-112)
🪛 Cppcheck (2.18.0)
src/bitcoin-util.cpp
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
🪛 Flake8 (7.3.0)
contrib/devtools/gen-manpages.py
[error] 15-15: continuation line missing indentation or outdented
(E122)
🪛 LanguageTool
doc/release-process.md
[style] ~18-~18: ‘taking into account’ might be wordy. Consider a shorter alternative.
Context: ...block height comment with that height), taking into account the following: - The selected value m...
(EN_WORDINESS_PREMIUM_TAKING_INTO_ACCOUNT)
🪛 markdownlint-cli2 (0.18.1)
contrib/tracing/README.md
183-183: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
184-184: Dollar signs used before commands without showing output
(MD014, commands-show-output)
190-190: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
191-191: Dollar signs used before commands without showing output
(MD014, commands-show-output)
doc/developer-notes.md
18-18: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
401-401: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
🪛 Ruff (0.14.8)
test/functional/rpc_psbt.py
548-548: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
test/functional/test_framework/psbt.py
123-123: Consider [self.g, *self.i, *self.o] instead of concatenation
Replace with [self.g, *self.i, *self.o]
(RUF005)
test/functional/test_framework/test_framework.py
1077-1077: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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). (2)
- GitHub Check: Build slim container / Build container
- GitHub Check: Build container / Build container
🔇 Additional comments (30)
src/bitcoin-wallet.cpp (2)
44-49: AddCommand call-site migration matches new API and upstream behaviorSwitching these registrations to
AddCommand(cmd, help)aligns with the updatedArgsManager::AddCommandsignature and the implementation that always routes commands into theCOMMANDScategory. Command names here are valid per the implementation preconditions (no leading-, no=). Looks good and consistent with the backport intent.
90-92: Chain settings comment is now accurate for generalized chain selectionThe updated comment correctly describes this block as the point where chain settings are established before
Params()can be used, without hardcoding-testnet/-regtest. This better matches the generalized chain handling (including devnet/signet) while preserving semantics.src/util/system.h (1)
427-427: AddCommand declaration matches simplified implementation and preserves semanticsThe
ArgsManager::AddCommanddeclaration now matches the two-argument implementation that internally inserts into theCOMMANDScategory. This simplifies the public API without changing command registration behavior and is consistent with the related call-site updates.src/bitcoin-cli.cpp (1)
75-79: Devnet integration in CLI args and netinfo banner is consistentUsing
CreateBaseChainParams(CBaseChainParams::DEVNET)to derive the devnet RPC port for-rpcporthelp, and extendingChainToString()to return" devnet"when appropriate, are both consistent with the existing main/testnet/regtest patterns. The clarifying comment beforeSelectBaseParams()is also accurate.Also applies to: 99-99, 438-443
src/rpc/mining.cpp (1)
447-451: getmininginfo and getblocktemplate updates correctly account for devnetIncluding
devnetin the"chain"field description matches the actual values returned, and hoistingconsensusParamsto a single binding reused byUpdateTimeand the versionbits logic preserves behavior while simplifying the code.Also applies to: 795-797
src/init.cpp (6)
527-531: LGTM: DEVNET base and chain params initialization.The additions correctly follow the established pattern for MAIN, TESTNET, and REGTEST, initializing both base params and chain params for DEVNET.
545-545: Help text update for devnet looks correct.The
-assumevalidhelp string now includes devnet's default, matching the pattern used for testnet.
568-568: Help text for-minimumchainworkcorrectly includes devnet.Follows the same pattern as
-assumevalidand other network-specific defaults.
607-607: LGTM:-bindhelp includes devnet onion service target port.Correctly extends the existing format to include devnet alongside testnet and regtest.
638-638: LGTM:-porthelp includes devnet default port.The pattern is consistent with other network-specific port documentation.
811-811: LGTM:-rpcporthelp includes devnet RPC port.Completes the devnet port documentation across all relevant arguments.
src/bitcoin-util.cpp (4)
29-38: LGTM: Utility argument setup.Clean setup following the established pattern with help options, version flag, command registration, and chain params.
80-104: Thread-safe nonce grinding implementation looks correct.The implementation properly handles:
- Early validation of target (line 85)
- Atomic found flag with exchange to prevent race conditions (line 96)
- Stride-based iteration to distribute work across threads
- Periodic checking of the found flag to allow early termination
106-139: LGTM: Grind orchestration function.Correctly spawns threads based on hardware concurrency, joins all threads, and handles both success and failure cases. The use of
std::reffor the header and found flag is appropriate for thread parameter passing.
147-190: LGTM: Main entry point with proper error handling.The entry point correctly:
- Sets up environment
- Handles initialization failures with appropriate exception handling
- Validates command presence
- Routes to the grind command
- Outputs to stdout on success, stderr on failure
doc/man/Makefile.am (1)
19-21: LGTM: Manpage conditional for bitcoin-util.Correctly follows the established pattern for conditional manpage inclusion.
src/qt/networkstyle.cpp (1)
28-28: Visual differentiation for devnet icon color.The color transformation parameters changed from
(190, 20)to(35, 15). This produces a distinct hue compared to testnet(190, 20), providing better visual differentiation between the networks in the UI.src/qt/walletcontroller.cpp (1)
452-457: LGTM: Progress dialog with distinct title and label.The change provides a clear window title "Load Wallets" separate from the descriptive label "Loading wallets…", improving the user experience. The translator comments follow Qt's i18n best practices.
test/functional/test_framework/messages.py (1)
235-247: Binary deserialization helper is consistent and safe
from_binarycleanly mirrorsfrom_hex, correctly handles both bytes and streams, and enforces full consumption only for the bytes case, which matches howPSBTandPSBTMapuse it. No issues found.test/functional/test_framework/test_framework.py (1)
1074-1078: New bitcoin-util test hooks match existing patterns
skip_if_no_bitcoin_utilandis_bitcoin_util_compiledmirror the existing wallet/CLI helpers, correctly key offENABLE_BITCOIN_UTIL, and integrate cleanly into the test-skipping mechanism. Looks good as-is.Also applies to: 1126-1129
Makefile.am (1)
28-28: dash-util wiring in top-level Makefile is consistent
BITCOIN_UTIL_BINis defined, built, and included in the Windows installer using the same pattern as the other binaries (daemon/CLI/tx/wallet). This matches the existing build structure and upstream backport style.Also applies to: 85-85, 155-156
test/functional/rpc_psbt.py (1)
13-27: Additional PSBT coverage is well-formed and aligned with helpersThe new imports and tests correctly leverage
CTransaction/COutPoint/CTxIn/CTxOutand thePSBT/PSBTMaphelpers to:
- Build a synthetic PSBT with per-input RIPEMD160/SHA256/HASH160/HASH256 preimages and verify
decodepsbtexposes them under the expected*_preimagesmaps.- Verify
combinepsbtrejects PSBTs whose unsigned transactions differ and accepts combining identical PSBTs.Construction of keys (
type_byte + hash) and the global unsigned transaction map matches the PSBT format used intest_framework/psbt.py, and the assertions are precise without altering node behavior.Also applies to: 33-35, 523-563
configure.ac (5)
32-41: dash-util binary name wiring looks consistentDefining
BITCOIN_UTIL_NAME=dash-utiland exporting it viaAC_SUBST(BITCOIN_UTIL_NAME)aligns with the existing pattern for the other binaries and cleanly exposes the name to Makefiles and packaging scripts. No issues spotted.Also applies to: 1986-1991
742-747: New--enable-util-utilflag and utils default behave as expectedThe updated
--with-utilshelp text and the newAC_ARG_ENABLE([util-util])correctly mirror the existingutil-cli,util-tx, andutil-walletflags.build_bitcoin_utildefaults tobuild_bitcoin_utils, so--with-utils=nocleanly disables all utils unless explicitly re-enabled per‑tool. Initialization order is sound and won’t leavebuild_bitcoin_utilunset.Also applies to: 766-770
1434-1453: Fuzz-only build correctly disables dash-utilExtending the
enable_fuzzbranch to setbuild_bitcoin_util=nokeeps dash-util consistent with the other binaries that are disabled in fuzz‑only builds, avoiding accidental compilation of extra targets in that mode.
1762-1776: New BUILD_BITCOIN_UTIL conditional is wired correctlyThe added
AC_MSG_CHECKING([whether to build dash-util])andAM_CONDITIONAL([BUILD_BITCOIN_UTIL], [test $build_bitcoin_util = "yes"])follow the same pattern as the other build conditionals and tie neatly into the earlierbuild_bitcoin_utilflag. Looks good.
1931-1933: No-target guard correctly accounts for dash-utilIncluding
build_bitcoin_utilin the “No targets!” check and extending the literal"nonononononononono"to cover the extra variable keeps the guard accurate and prevents mis-reporting when dash-util is the only enabled binary.test/functional/test_framework/psbt.py (3)
16-65: PSBT type constant definitions look complete and correctThe global, per-input, and per-output PSBT type constants match the expected PSBT/PSBTv2 assignments and provide a clear, centralized enum surface for tests. No issues here.
67-95: PSBTMap (de)serialization matches PSBT map structure
PSBTMap.deserializeandserializecorrectly implement PSBT map encoding:
- Use of
deser_string/ser_compact_sizematches the variable-length key/value encoding.- Converting single-byte keys to integers while leaving longer keys as bytes reflects PSBT’s “type + keydata” scheme without over-parsing.
- Uniqueness enforcement on the full key prevents duplicate entries within a map.
This is appropriate for the functional test framework and aligns with upstream behaviour.
96-131: PSBT class integrates cleanly with CTransaction and from_binaryThe PSBT wrapper correctly:
- Verifies the
b"psbt\xff"magic.- Reads the global map, enforces presence of the unsigned tx (key
0), and deserializes it viaCTransaction.- Deserializes exactly one PSBTMap per vin/vout entry and, on serialization, re-parses the tx from
g.map[0]and asserts input/output counts match.The base64 helpers are thin, straightforward wrappers. Overall the implementation is sound and suitable for tests, and it remains aligned with the upstream Bitcoin test framework helper, so no refactors are needed here. (Based on learnings, backported code is best kept identical unless there is a clear bug.)
||){kind=link}
7 participants
Issue being fixed or feature implemented
What is SigNet?
https://github.com/bitcoin/bips/blob/master/bip-0325.mediawiki
Dash Core has a feature "devnets" that covers most cases of signet.
How signet is better than regtest?
It's available for test in real networks (private or publics, not only localhost as regtest).
Devnet covers this case.
How signet is better than testnet?
How signet is different with devnets?
Motivation
As thephez mentioned:
And it is the only benefit.
If anyone present can show just and lawful cause why devnets and signets should be joined together in matrimony, let them speak now or forever hold their peace
What was done?
Multiple backports that contains signet's related refactorings, tests improvements, bitcoin-util [dash-util] tool etc - everything except SigNet feature itself.
How Has This Been Tested?
Run unit & functional tests
dash-utilhas been tested:Breaking Changes
N/A
Checklist: