backport: Merge bitcoin#28618, 28639, 28320, 27829 #7073
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
vijaydasmp
changed the title
vijaydasmp
changed the title
vijaydasmp
force-pushed
the
Jan_2026_1
branch
from
5261435 to
fd8c56d
Compare
WalkthroughThis PR removes the Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (1)src/**/*.{cpp,h,hpp,cc}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-08-08T07:01:47.332ZApplied to files:
⏰ 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). (10)
🔇 Additional comments (1)
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: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
doc/design/assumeutxo.md(1 hunks)src/node/utxo_snapshot.h(1 hunks)src/rpc/blockchain.cpp(2 hunks)src/rpc/rawtransaction.cpp(1 hunks)src/wallet/rpc/spend.cpp(2 hunks)src/wallet/transaction.cpp(1 hunks)src/wallet/transaction.h(1 hunks)test/get_previous_releases.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
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/node/utxo_snapshot.hsrc/rpc/rawtransaction.cppsrc/rpc/blockchain.cppsrc/wallet/transaction.cppsrc/wallet/rpc/spend.cppsrc/wallet/transaction.h
src/node/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
NodeContext must be extended with Dash-specific managers for each subsystem during initialization
Files:
src/node/utxo_snapshot.h
{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/design/assumeutxo.md
src/wallet/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Wallet implementation must use Berkeley DB and SQLite
Files:
src/wallet/transaction.cppsrc/wallet/rpc/spend.cppsrc/wallet/transaction.h
🧠 Learnings (5)
📓 Common learnings
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: 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: 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: UdjinM6
Repo: dashpay/dash PR: 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: 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: 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: 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.
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
📚 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/rawtransaction.cppsrc/rpc/blockchain.cppsrc/wallet/rpc/spend.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/{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/blockchain.cppsrc/wallet/transaction.h
📚 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/evo/evodb/**/*.{cpp,h} : Evolution Database (CEvoDb) must handle masternode snapshots, quorum state, governance objects with efficient differential updates for masternode lists
Applied to files:
src/rpc/blockchain.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:
src/wallet/transaction.h
🧬 Code graph analysis (2)
src/node/utxo_snapshot.h (1)
src/kernel/coinstats.h (1)
coins_count(42-42)
src/wallet/transaction.h (1)
src/wallet/transaction.cpp (2)
CopyFrom(28-31)CopyFrom(28-28)
🪛 LanguageTool
doc/design/assumeutxo.md
[grammar] ~43-~43: Ensure spelling is correct
Context: ...t hash in the source code. The utility scriptt ./contrib/devtools/utxo_snapshot.sh m...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ 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). (10)
- GitHub Check: linux64_multiprocess-build / Build source
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
🔇 Additional comments (8)
test/get_previous_releases.py (1)
3-3: RISC‑V support wiring matches existing release artifactsThe updated copyright range and the new
'riscv64-*-linux*' -> 'riscv64-linux-gnu'mapping are consistent with the existingdashcore-*-riscv64-linux-gnu.tar.gzentries inSHA256_SUMS, so--download-binarywill correctly pick the RISC‑V binaries for riscv64 Linux hosts.Also applies to: 270-279
src/rpc/rawtransaction.cpp (1)
201-203: Clarifiedoutputshelp text aligns with existing validationThe new wording for
outputs(key uniqueness, single'data'output, and at least one output required) matches howConstructTransactionalready interprets this parameter, so this is a pure documentation improvement with no behavioral change.src/wallet/rpc/spend.cpp (1)
806-809: Wallet RPCoutputsdocs now consistent with rawtransaction RPCsThe updated
outputsdescriptions insendandwalletcreatefundedpsbt(key-value pairs, unique keys, single'data'output, non-empty set) bring these wallet RPCs in line withcreaterawtransaction/createpsbthelp text and existing behavior, without altering logic.Also applies to: 1101-1104
src/wallet/transaction.cpp (1)
28-31:CWalletTx::CopyFromcorrectly delegates to default copy assignmentUsing
*this = _tx;cleanly relies on the defaulted copy assignment semantics introduced in the header, soCopyFromprovides the explicit-copy API without introducing new behavior or risks.src/node/utxo_snapshot.h (1)
26-34: SnapshotMetadata ctor & serialization are internally consistentThe constructor now takes just
base_blockhashandcoins_countandSERIALIZE_METHODSmatches by serializing onlym_base_blockhashandm_coins_count, so the in-memory and on-disk metadata formats stay aligned. Callers usingSnapshotMetadatamust now pass only these two fields, which matches the new usage inCreateUTXOSnapshot.src/rpc/blockchain.cpp (1)
2775-2803: UTXO snapshot metadata and dumptxoutset result remain coherent
SnapshotMetadatais now instantiated with{tip->GetBlockHash(), maybe_stats->coins_count}, matching the revised constructor and on-disk header format, while the RPC result still exposesnchaintxviatip->nChainTx. That cleanly separates what’s stored in the snapshot header from what’s reported to RPC users without introducing inconsistencies.src/wallet/transaction.h (1)
320-329: LGTM: Well-designed copy semantics refactoring.The restructuring to use private defaulted copy operations with a public explicit
CopyFromhelper follows C++ best practices. This pattern prevents accidental copies (e.g., inmapWallet) while allowing intentional copying when needed. The implementation correctly delegates to the defaulted copy assignment operator.doc/design/assumeutxo.md (1)
6-42: LGTM: Documentation improvements enhance clarity.The new sections for loading snapshots, pruning considerations, index behavior, and snapshot generation provide valuable guidance for users working with assumeutxo functionality. The structured approach with dedicated subsections improves readability.
| to create a snapshot on one node that you wish to load on another node. | ||
| It can also be used to verify the hardcoded snapshot hash in the source code. | ||
|
|
||
| The utility scriptt `./contrib/devtools/utxo_snapshot.sh` may |
There was a problem hiding this comment.
Fix typo in "scriptt".
The word "scriptt" should be "script". However, if this typo exists in the upstream Bitcoin Core PR being backported, it should be preserved to reduce merge conflicts in future backports.
🔎 Proposed fix if not from upstream
-The utility scriptt `./contrib/devtools/utxo_snapshot.sh` may
+The utility script `./contrib/devtools/utxo_snapshot.sh` mayBased on learnings, typos in backported code should be kept as-is to maintain consistency with upstream Bitcoin Core.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The utility scriptt `./contrib/devtools/utxo_snapshot.sh` may | |
| The utility script `./contrib/devtools/utxo_snapshot.sh` may |
🧰 Tools
🪛 LanguageTool
[grammar] ~43-~43: Ensure spelling is correct
Context: ...t hash in the source code. The utility scriptt ./contrib/devtools/utxo_snapshot.sh m...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In doc/design/assumeutxo.md around line 43, fix the typo "scriptt" to "script"
unless this exact typo exists in the upstream Bitcoin Core PR being backported —
in that case leave it unchanged to avoid future merge conflicts; if you change
it, update only the word to "script" and run a quick spellcheck to ensure no
other similar typos were introduced.
03f8208 doc: assumeutxo prune and index notes (Sjors Provoost) Pull request description: Based on recent comments on bitcoin#27596. ACKs for top commit: pablomartin4btc: re ACK 03f8208 ryanofsky: ACK 03f8208. Nice changes, these seem like very helpful notes Tree-SHA512: fe651b49f4d667400a3655899f27a96dd1eaf67cf9215fb35db5f44fb8c0313e7d541518be6791fec93392df24b909793f3886adb808e53228ed2a291165639d
…tadata constructor, fix test, add test fafde92 test: Check snapshot file with wrong number of coins (MarcoFalke) faa90f6 refactor: Remove unused nchaintx from SnapshotMetadata constructor (MarcoFalke) Pull request description: See commit messages ACKs for top commit: Sjors: utACK fafde92 theStack: ACK fafde92 Tree-SHA512: 9ed2720b50d1c0938f30543ba143e1a4c6af3a0ff166f8b3eb452e1d99ddee6e3443a4c99f77efe94b8c3eb2feff984bf5259807ee8085e1e0e1e0d1de98227e
vijaydasmp
force-pushed
the
Jan_2026_1
branch
from
fd8c56d to
aee639a
Compare
2222e15 test: Support riscv64 in get_previous_releases.py (MarcoFalke) Pull request description: To test: `test/get_previous_releases.py -b -t /tmp/prev_releases v0.18.1` On master: `Not sure which binary to download for riscv64-unknown-linux-gnu` Here: (pass) ACKs for top commit: fanquake: ACK 2222e15 Tree-SHA512: 18dc9a6c65f78adb5f7fc09e57db34c6b544071cb7bb3fa2846c86a23202e37d6ea1c5aca9acc1c2040b7d2b97bb93840a8a949f81f71fe6f01c395d2894739d
27b168b Update help text for spend and rawtransaction rpcs (Michael Tidwell) Pull request description: The "data" field without outputs was marked as "required" in the help docs when using bitcoin-cli. This field when left off worked as an intended optional OP_RETURN. closes bitcoin#27828. Motivation: It is hard to understand that "data" is actually optional for commands like `createpsbt` and `walletcreatefundedpsbt`. ACKs for top commit: achow101: ACK 27b168b Sjors: tACK 27b168b Tree-SHA512: 235e7ed4af69880880c04015b3f7de72c8f31ae035485c4c64c483e282948f3ea3f1eef16f15e260a1aaf21114150713516ba6a99967ccad9ecd91ff67cb0450
vijaydasmp
force-pushed
the
Jan_2026_1
branch
from
aee639a to
27563d3
Compare
vijaydasmp
changed the title
5 participants
Bitcoin Backports