backport: merge bitcoin#17211, #23188, #24859, #20640, #24649, #25118, #25481, #24699, #25790, partial bitcoin#23201, #11403 (wallet backports: part 7) #6755
Conversation
kwvg
force-pushed
the
wbps_p7
branch
2 times, most recently
from
7194128 to
770b087
Compare
WalkthroughThis set of changes introduces comprehensive enhancements and refactoring to the wallet's coin selection, transaction creation, and input handling, particularly to support external (non-wallet) inputs and richer coin control capabilities. The transaction creation API is refactored to return a structured result instead of using multiple output parameters, and now uses an explicit constant for random change output positioning. Coin control is extended to track external inputs, their metadata, and associated solving data, with new methods for managing them. Transaction size estimation and dummy signing functions are updated to work with external inputs and coin control context. The wallet's associative containers are switched to salted hash-based unordered maps for improved security and performance. Several RPC commands and tests are updated to support and verify the new ability to fund and sign transactions involving external inputs, requiring explicit solving data. Output type handling is made more explicit and robust, and new test cases are added to ensure correct behavior for these extended features. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
test/functional/wallet_createwallet.py (1)
29-32: Good test addition but fix line continuation indentation.The test correctly validates that creating a wallet with invalid parameters (passphrase while private keys are disabled) raises the expected RPC error. However, there's a formatting issue with the line continuation indentation.
Apply this diff to fix the indentation:
assert_raises_rpc_error(-4, "Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled.", - self.nodes[0].createwallet, wallet_name='w0', descriptors=True, disable_private_keys=True, passphrase="passphrase") + self.nodes[0].createwallet, wallet_name='w0', descriptors=True, disable_private_keys=True, passphrase="passphrase")src/wallet/rpc/spend.cpp (2)
458-505: Comprehensive solving_data parsing with proper validation.The implementation correctly validates and parses pubkeys, scripts, and descriptors with appropriate error handling. The automatic generation of P2PK scripts for pubkeys is a nice touch for usability.
One minor suggestion: Consider extracting this parsing logic into a separate helper function since it's duplicated in multiple RPC methods.
794-813: Consistent implementation of solving_data support.The send RPC follows the same pattern as fundrawtransaction for solving_data support.
Note: The solving_data parsing code appears to be duplicated. Consider extracting it to a shared helper function.
src/wallet/spend.cpp (1)
688-697: Excellent refactoring to use std::optional return type.The change from output parameters to a structured optional result significantly improves the API clarity and error handling consistency.
src/wallet/coincontrol.h (2)
12-13: Consider removing potentially unused includes.The includes for
script/keyorigin.hand<algorithm>don't appear to be directly used in this header file. If they're not required by the implementation or as transitive dependencies, consider removing them to reduce compilation dependencies.Also applies to: 17-19
132-148: Consider the assertion in GetInputWeight().The implementation is correct. The assertion in
GetInputWeight()provides fail-fast behavior for programming errors, which is appropriate since callers should checkHasInputWeight()first.Consider documenting the precondition in a comment:
int64_t GetInputWeight(const COutPoint& outpoint) const { + // Precondition: HasInputWeight(outpoint) must be true auto it = m_input_weights.find(outpoint); assert(it != m_input_weights.end()); return it->second; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
src/bench/coin_selection.cpp(1 hunks)src/coinjoin/util.cpp(5 hunks)src/interfaces/wallet.h(2 hunks)src/outputtype.cpp(1 hunks)src/outputtype.h(1 hunks)src/rpc/evo.cpp(2 hunks)src/rpc/util.cpp(1 hunks)src/wallet/coincontrol.h(6 hunks)src/wallet/coinjoin.cpp(1 hunks)src/wallet/interfaces.cpp(2 hunks)src/wallet/rpc/addresses.cpp(3 hunks)src/wallet/rpc/spend.cpp(12 hunks)src/wallet/scriptpubkeyman.cpp(1 hunks)src/wallet/scriptpubkeyman.h(1 hunks)src/wallet/spend.cpp(22 hunks)src/wallet/spend.h(2 hunks)src/wallet/test/coinjoin_tests.cpp(3 hunks)src/wallet/test/coinselector_tests.cpp(4 hunks)src/wallet/test/spend_tests.cpp(1 hunks)src/wallet/test/wallet_tests.cpp(8 hunks)src/wallet/wallet.cpp(13 hunks)src/wallet/wallet.h(7 hunks)test/functional/rpc_fundrawtransaction.py(5 hunks)test/functional/rpc_psbt.py(2 hunks)test/functional/wallet_createwallet.py(1 hunks)test/functional/wallet_send.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{cpp,h,cc,cxx,hpp}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
src/bench/**/*.{cpp,h,cc,cxx,hpp}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
test/functional/**/*.py
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (23)
📓 Common learnings
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Favor extension over modification of the Bitcoin Core foundation
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
src/bench/coin_selection.cpp (2)
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: kwvg
PR: dashpay/dash#6530
File: src/validation.cpp:360-362
Timestamp: 2025-01-14T08:37:16.955Z
Learning: The UpdateTransactionsFromBlock() method in txmempool.cpp takes parameters in the order: vHashUpdate, ancestor_size_limit, ancestor_count_limit. The size limit comes before the count limit.
src/rpc/util.cpp (1)
Learnt from: kwvg
PR: dashpay/dash#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.
src/wallet/test/spend_tests.cpp (4)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests should be implemented in src/test/, src/wallet/test/, and src/qt/test/ using Boost::Test or Qt 5 for GUI tests
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#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.
src/coinjoin/util.cpp (4)
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#6530
File: src/validation.cpp:360-362
Timestamp: 2025-01-14T08:37:16.955Z
Learning: The UpdateTransactionsFromBlock() method in txmempool.cpp takes parameters in the order: vHashUpdate, ancestor_size_limit, ancestor_count_limit. The size limit comes before the count limit.
Learnt from: kwvg
PR: dashpay/dash#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.
src/wallet/scriptpubkeyman.cpp (1)
Learnt from: kwvg
PR: dashpay/dash#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.
src/wallet/test/coinjoin_tests.cpp (4)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests should be implemented in src/test/, src/wallet/test/, and src/qt/test/ using Boost::Test or Qt 5 for GUI tests
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#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.
test/functional/wallet_send.py (2)
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#6726
File: test/functional/rpc_createmultisig.py:120-120
Timestamp: 2025-06-20T23:32:16.225Z
Learning: In the rpc_createmultisig.py test, the checkbalances() function correctly excludes 9 blocks from the balance calculation: 8 blocks from do_multisig() calls (2 blocks per call × 4 calls) plus 1 block from checkbalances() itself.
test/functional/rpc_psbt.py (3)
Learnt from: kwvg
PR: dashpay/dash#6726
File: test/functional/rpc_createmultisig.py:120-120
Timestamp: 2025-06-20T23:32:16.225Z
Learning: In the rpc_createmultisig.py test, the checkbalances() function correctly excludes 9 blocks from the balance calculation: 8 blocks from do_multisig() calls (2 blocks per call × 4 calls) plus 1 block from checkbalances() itself.
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#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.
test/functional/wallet_createwallet.py (3)
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: kwvg
PR: dashpay/dash#6726
File: test/functional/rpc_createmultisig.py:120-120
Timestamp: 2025-06-20T23:32:16.225Z
Learning: In the rpc_createmultisig.py test, the checkbalances() function correctly excludes 9 blocks from the balance calculation: 8 blocks from do_multisig() calls (2 blocks per call × 4 calls) plus 1 block from checkbalances() itself.
src/wallet/test/wallet_tests.cpp (6)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests should be implemented in src/test/, src/wallet/test/, and src/qt/test/ using Boost::Test or Qt 5 for GUI tests
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#6726
File: test/functional/rpc_createmultisig.py:120-120
Timestamp: 2025-06-20T23:32:16.225Z
Learning: In the rpc_createmultisig.py test, the checkbalances() function correctly excludes 9 blocks from the balance calculation: 8 blocks from do_multisig() calls (2 blocks per call × 4 calls) plus 1 block from checkbalances() itself.
Learnt from: kwvg
PR: dashpay/dash#6530
File: src/validation.cpp:360-362
Timestamp: 2025-01-14T08:37:16.955Z
Learning: The UpdateTransactionsFromBlock() method in txmempool.cpp takes parameters in the order: vHashUpdate, ancestor_size_limit, ancestor_count_limit. The size limit comes before the count limit.
test/functional/rpc_fundrawtransaction.py (2)
Learnt from: kwvg
PR: dashpay/dash#6726
File: test/functional/rpc_createmultisig.py:120-120
Timestamp: 2025-06-20T23:32:16.225Z
Learning: In the rpc_createmultisig.py test, the checkbalances() function correctly excludes 9 blocks from the balance calculation: 8 blocks from do_multisig() calls (2 blocks per call × 4 calls) plus 1 block from checkbalances() itself.
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
src/wallet/coincontrol.h (1)
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
src/wallet/rpc/spend.cpp (3)
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: kwvg
PR: dashpay/dash#6530
File: src/validation.cpp:360-362
Timestamp: 2025-01-14T08:37:16.955Z
Learning: The UpdateTransactionsFromBlock() method in txmempool.cpp takes parameters in the order: vHashUpdate, ancestor_size_limit, ancestor_count_limit. The size limit comes before the count limit.
src/wallet/wallet.h (4)
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests should be implemented in src/test/, src/wallet/test/, and src/qt/test/ using Boost::Test or Qt 5 for GUI tests
Learnt from: kwvg
PR: dashpay/dash#6532
File: src/test/fuzz/netaddress.cpp:83-84
Timestamp: 2025-01-14T09:06:19.717Z
Learning: In fuzzer harness tests, CServiceHash can be used with both default constructor (CServiceHash()) and parameterized constructor (CServiceHash(salt_k0, salt_k1)) to test different variants. The usage pattern CServiceHash()(service) and CServiceHash(0, 0)(service) is valid and intentional in such tests.
src/interfaces/wallet.h (2)
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
src/wallet/test/coinselector_tests.cpp (5)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests should be implemented in src/test/, src/wallet/test/, and src/qt/test/ using Boost::Test or Qt 5 for GUI tests
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/**/*.{cpp,h,cc,cxx,hpp} : Dash Core codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
src/wallet/rpc/addresses.cpp (1)
Learnt from: kwvg
PR: dashpay/dash#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.
src/wallet/spend.h (4)
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: kwvg
PR: dashpay/dash#6530
File: src/validation.cpp:360-362
Timestamp: 2025-01-14T08:37:16.955Z
Learning: The UpdateTransactionsFromBlock() method in txmempool.cpp takes parameters in the order: vHashUpdate, ancestor_size_limit, ancestor_count_limit. The size limit comes before the count limit.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
src/wallet/interfaces.cpp (3)
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
src/rpc/evo.cpp (7)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#6729
File: src/evo/deterministicmns.cpp:1313-1316
Timestamp: 2025-07-09T15:02:26.899Z
Learning: In Dash's masternode transaction validation, `IsVersionChangeValid()` is only called by transaction types that update existing masternode entries (like `ProUpServTx`, `ProUpRegTx`, `ProUpRevTx`), not by `ProRegTx` which creates new entries. This means validation logic in `IsVersionChangeValid()` only applies to the subset of transaction types that actually call it, not all masternode transaction types.
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: knst
PR: dashpay/dash#6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions `MigrateDBIfNeeded` and `MigrateDBIfNeeded2` in `src/evo/deterministicmns.cpp` are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
src/wallet/wallet.cpp (5)
undefined
<retrieved_learning>
Learnt from: kwvg
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: kwvg
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: kwvg
PR: #6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
</retrieved_learning>
<retrieved_learning>
Learnt from: kwvg
PR: #6729
File: src/evo/deterministicmns.cpp:1313-1316
Timestamp: 2025-07-09T15:02:26.899Z
Learning: In Dash's masternode transaction validation, IsVersionChangeValid() is only called by transaction types that update existing masternode entries (like ProUpServTx, ProUpRegTx, ProUpRevTx), not by ProRegTx which creates new entries. This means validation logic in IsVersionChangeValid() only applies to the subset of transaction types that actually call it, not all masternode transaction types.
</retrieved_learning>
<retrieved_learning>
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests should be implemented in src/test/, src/wallet/test/, and src/qt/test/ using Boost::Test or Qt 5 for GUI tests
</retrieved_learning>
src/wallet/spend.cpp (4)
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
Learnt from: kwvg
PR: dashpay/dash#6530
File: src/validation.cpp:360-362
Timestamp: 2025-01-14T08:37:16.955Z
Learning: The UpdateTransactionsFromBlock() method in txmempool.cpp takes parameters in the order: vHashUpdate, ancestor_size_limit, ancestor_count_limit. The size limit comes before the count limit.
🧬 Code Graph Analysis (6)
src/rpc/util.cpp (2)
src/outputtype.h (1)
AddAndGetDestinationForScript(40-40)src/outputtype.cpp (2)
AddAndGetDestinationForScript(60-73)AddAndGetDestinationForScript(60-60)
src/coinjoin/util.cpp (4)
src/wallet/rpc/coins.cpp (1)
coinControl(606-606)src/wallet/spend.cpp (6)
CalculateMaximumSignedTxSize(48-55)CalculateMaximumSignedTxSize(48-48)CalculateMaximumSignedTxSize(57-78)CalculateMaximumSignedTxSize(57-57)CreateTransaction(1023-1073)CreateTransaction(1023-1031)src/wallet/spend.h (3)
CalculateMaximumSignedTxSize(26-26)CalculateMaximumSignedTxSize(27-27)CreateTransaction(113-113)src/coinjoin/util.h (1)
strResult(122-122)
src/wallet/coinjoin.cpp (4)
src/wallet/spend.cpp (1)
outpoint(130-130)src/wallet/wallet.h (6)
outpoint(294-294)outpoint(545-545)outpoint(547-547)outpoint(551-551)outpoint(552-552)outpoint(554-554)src/wallet/wallet.cpp (1)
outpoint(1054-1054)src/wallet/coincontrol.h (2)
outpoint(96-104)outpoint(96-96)
test/functional/wallet_send.py (5)
test/functional/test_framework/test_framework.py (1)
BitcoinTestFramework(101-1132)test/functional/test_framework/util.py (1)
assert_raises_rpc_error(128-144)test/functional/test_framework/wallet_util.py (1)
bytes_to_wif(82-85)test/functional/test_framework/test_node.py (1)
get_wallet_rpc(360-366)src/wallet/rpc/spend.cpp (2)
walletprocesspsbt(946-1023)walletprocesspsbt(946-946)
test/functional/wallet_createwallet.py (1)
test/functional/test_framework/util.py (1)
assert_raises_rpc_error(128-144)
src/interfaces/wallet.h (5)
src/wallet/wallet.h (16)
a(245-248)a(245-245)tx(303-303)tx(322-322)tx(328-328)tx(332-332)tx(620-620)tx(622-622)tx(644-644)tx(653-653)tx(655-655)tx(695-695)tx(792-792)tx(794-794)tx(795-795)tx(824-824)src/node/miner.h (4)
a(85-88)a(85-85)a(103-109)a(103-103)src/coinjoin/client.h (4)
a(60-63)a(60-60)a(64-67)a(64-64)src/qt/transactiontablemodel.cpp (6)
a(46-49)a(46-46)a(50-53)a(50-50)a(54-57)a(54-54)src/wallet/interfaces.cpp (2)
tx(304-310)tx(304-306)
🪛 Flake8 (7.2.0)
test/functional/wallet_createwallet.py
[error] 32-32: continuation line under-indented for visual indent
(E128)
⏰ 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: win64-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
🔇 Additional comments (91)
src/wallet/coinjoin.cpp (1)
152-152: LGTM - Good modernization of iterator declaration.The change from explicit iterator type to
autoimproves code readability while maintaining the same functionality and const-correctness. This aligns with modern C++ best practices.src/wallet/scriptpubkeyman.h (1)
528-529: LGTM - Good performance optimization with caching.The addition of
m_map_signing_providerscache is a sensible performance optimization to avoid regeneratingFlatSigningProviderinstances. The use ofmutableis appropriate here as it allows the cache to be updated in const contexts without changing the logical state of the object.src/bench/coin_selection.cpp (1)
59-60: LGTM - Updated to use current wallet API.The change from
GetTxSpendSizetoCalculateMaximumSignedInputSizealigns with the broader wallet refactoring. The benchmark now correctly uses the current wallet API for input size estimation. The use ofnullptrfor coin control is appropriate for the benchmark context.src/rpc/util.cpp (1)
293-293: LGTM - Explicit output type specification improves clarity.The addition of
OutputType::LEGACYparameter explicitly specifies that multisig destinations should use the legacy output type. This improves code clarity and ensures compatibility with the new output type system.src/interfaces/wallet.h (3)
65-65: LGTM: Adding RANDOM_CHANGE_POSITION using declarationThe addition of
using wallet::RANDOM_CHANGE_POSITION;aligns with the wallet transaction creation refactoring described in the PR objectives.
433-433: LGTM: Well-implemented comparison operatorThe
operator<implementation correctly comparesWalletTxobjects by their transaction hash (tx->GetHash()), which provides a consistent ordering for thestd::setcontainer. This is appropriate since transaction hashes are unique identifiers.
198-198: Interface change fully propagated: allgetWalletTxs()implementations now usestd::set<WalletTx>No remaining references to
std::vector<WalletTx>oremplace_backforWalletTxwere found in:
- src/interfaces/wallet.h
- src/wallet/interfaces.cpp
- src/qt/transactiontablemodel.cpp
No further updates required.
src/rpc/evo.cpp (2)
65-65: LGTM: Consistent RANDOM_CHANGE_POSITION usageThe addition of
using wallet::RANDOM_CHANGE_POSITION;is consistent with the wallet transaction creation refactoring.
290-296: LGTM: Proper migration to new CreateTransaction APIThe update correctly migrates from the old
CreateTransactionAPI with output parameters to the new API that returns an optionalCreatedTransactionResult. The implementation properly:
- Uses
RANDOM_CHANGE_POSITIONfor the change position parameter- Sets the
signflag totrue- Checks for failure with
!txrand throws appropriate error- Extracts the transaction from the result with
txr->txThis aligns with the wallet transaction creation refactoring described in the PR objectives.
src/wallet/scriptpubkeyman.cpp (1)
2165-2179: Approve the signing provider caching optimization.The caching mechanism is well-implemented and addresses a performance optimization by avoiding redundant re-derivation of signing data for the same index. The implementation correctly:
- Checks for existing cached providers before performing expensive operations
- Merges cached data with the output provider
- Caches the result for future use
- Maintains thread safety with the existing lock assertion
The optimization is particularly beneficial for repeated signing or fee estimation operations that rely on descriptor expansions.
src/wallet/test/spend_tests.cpp (1)
34-39: Approve the test updates for the new CreateTransaction API.The test correctly adapts to the new
CreateTransactionAPI that returnsstd::optional<CreatedTransactionResult>. The changes properly:
- Use
std::optionalto handle the optional result- Check for success using
has_value()- Access transaction and fee through the structured result
- Use
RANDOM_CHANGE_POSITIONconstant for better code clarityThe test maintains the same validation logic while adapting to the improved API design.
src/wallet/test/coinselector_tests.cpp (3)
91-92: LGTM! Correct usage of the updated transaction size calculation API.The change from
GetTxSpendSizetoCalculateMaximumSignedInputSizealigns with the refactored transaction size estimation functions. Passingnullptrforcoin_controlis appropriate here since these are wallet-owned inputs.
337-337: LGTM! Consistent member variable naming.The rename from
fAllowOtherInputstom_allow_other_inputsfollows proper naming conventions with them_prefix for member variables.Also applies to: 394-394
921-966: Excellent test coverage for minimum input selection behavior!This Dash-specific test validates the correct behavior when
fRequireAllInputsis false - ensuring only the minimum necessary inputs are selected to meet the target amount. The test effectively covers the edge case where the target (25 COIN) can be met exactly with a subset of the available inputs (9 + 16 = 25).src/wallet/test/coinjoin_tests.cpp (2)
167-167: LGTM! Good use of auto type deduction.Using
decltypeimproves maintainability by automatically adapting to any future type changes inmapWallet.
185-185: LGTM! Correct adaptation to the new transaction creation API.The changes properly handle the new
std::optional<CreatedTransactionResult>return type and use theRANDOM_CHANGE_POSITIONconstant instead of the magic number-1. The error handling withhas_value()check is appropriate.Also applies to: 196-201, 208-208
src/coinjoin/util.cpp (4)
22-22: LGTM! Proper namespace import.Good practice to import the constant with a using declaration rather than repeating the namespace qualification.
131-131: LGTM! Consistent member variable naming.The rename to
m_allow_other_inputsfollows the updated coin control API.
151-151: LGTM! Correct parameter for transaction size calculation.Passing
nullptrforcoin_controlis appropriate here since we're calculating the size for wallet-owned inputs only.
270-270: LGTM! Proper adaptation to the new transaction creation API.The code correctly:
- Uses
RANDOM_CHANGE_POSITIONconstant instead of magic number-1- Handles the
std::optional<CreatedTransactionResult>return type- Extracts transaction, fee, and change position from the result structure
- Maintains the original validation logic for change output presence
Also applies to: 286-290, 298-298, 304-304
test/functional/rpc_psbt.py (2)
11-12: LGTM! Required imports for external input testing.The new imports support the test's key generation and descriptor creation functionality.
Also applies to: 20-20
467-503: Excellent test coverage for external input funding!This test comprehensively validates the new external input functionality:
- Tests the expected "Insufficient funds" error when solving data is not provided
- Validates both solving data formats (explicit pubkeys/scripts and descriptors)
- Verifies the complete signing workflow with external inputs
- Uses a clever
sh(pkh())descriptor that's non-standard but signableThe test ensures that external inputs can only be used when proper solving data is provided, which is crucial for security.
test/functional/wallet_send.py (5)
12-12: LGTM: Import addition for key generationThe import of
ECKeyis necessary for generating private keys in the new external outputs test scenario.
20-20: LGTM: Import addition for WIF conversionThe import of
bytes_to_wifis needed to convert the generated private key to Wallet Import Format for the test.
40-40: LGTM: Parameter addition for solving dataThe addition of the
solving_dataparameter to thetest_sendmethod properly extends the test framework to support external input testing scenarios.
93-94: LGTM: Solving data option handlingThe conditional addition of
solving_datato the options dictionary follows the same pattern as other optional parameters in the method.
465-505: LGTM: Comprehensive external outputs testThis test scenario effectively validates the new external input functionality:
- Creates a non-standard but signable descriptor (
sh(pkh(privkey)))- Tests that sending without solving data fails with "Insufficient funds"
- Verifies that providing solving data (both pubkeys/scripts and descriptors) enables successful transaction creation
- Confirms that both wallets can properly sign and finalize the PSBT
The test covers the key use case mentioned in the PR objectives where external inputs require explicit solving data.
src/wallet/rpc/addresses.cpp (3)
344-378: LGTM: Well-structured method for script processingThe
ProcessSubScriptmethod effectively encapsulates the logic for analyzing redeem scripts:
- Correctly extracts script type and hex representation
- Handles embedded address extraction with proper recursive description
- Promotes embedded pubkey to top-level for API consistency
- Properly handles multisig scripts when no embedded address is found
- Follows consistent naming conventions and error handling patterns
The method improves code maintainability by eliminating duplication between different script handling paths.
400-400: LGTM: Proper refactoring to use new methodThe refactoring correctly replaces the inline logic with a call to the new
ProcessSubScriptmethod, maintaining the same functionality while improving code organization.
450-454: LGTM: Clear documentation for embedded fieldThe documentation accurately describes the new
embeddedfield functionality, explaining that it provides detailed information about addresses embedded in P2SH scripts while excluding metadata and wallet relation fields.src/wallet/interfaces.cpp (2)
296-303: LGTM: Proper adaptation to new transaction creation APIThe refactoring correctly adapts to the new
CreateTransactionAPI:
- Uses
std::optional<CreatedTransactionResult>return type- Properly extracts
feeandchange_posfrom the result structure- Returns empty
CTransactionRefon failure (when!txr)- Maintains the same interface contract for callers
The change aligns with the PR objective to use structured results instead of multiple output parameters.
341-349: LGTM: Container type change with proper usageThe return type change from
std::vector<WalletTx>tostd::set<WalletTx>is correctly implemented:
- Container declaration updated to
std::set<WalletTx>- Uses
emplace()instead ofpush_back()for set insertion- Maintains the same functional behavior for callers
This change likely improves performance by providing automatic deduplication and ordering of wallet transactions.
test/functional/rpc_fundrawtransaction.py (5)
11-11: LGTM: Import addition is appropriate.The
ECKeyimport is correctly added to support the new test method's key generation functionality.
24-24: LGTM: Import addition is appropriate.The
bytes_to_wifimport is correctly added to support WIF conversion in the new test method.
133-133: LGTM: Test method integration is correct.The
test_external_inputs()call is appropriately added to the test sequence.
944-991: LGTM: Comprehensive test coverage for external inputs functionality.The test method thoroughly covers:
- Key generation and wallet creation
- Descriptor import and address derivation
- Error conditions with insufficient funds and malformed solving data
- Successful funding with proper solving data (both pubkeys/scripts and descriptors)
- Signing workflow verification
- Proper cleanup
The test follows good practices and provides excellent coverage for the external inputs feature.
1026-1026: LGTM: Proper cleanup addition.The wallet unload ensures proper cleanup after the
test_include_unsafetest, which is consistent with other test methods.src/outputtype.h (5)
12-14: LGTM: Appropriate includes for new functionality.The added includes (
<array>,<string>,<vector>) are necessary for the new declarations and follow standard C++ practices.
21-21: LGTM: Well-defined constant array declaration.The
OUTPUT_TYPESconstant array provides a clean way to enumerate all supported output types for iteration and validation purposes.
23-24: LGTM: Clean function declarations for output type parsing.The
ParseOutputTypeandFormatOutputTypefunctions provide a standard interface for string conversion, following the established pattern in the codebase.
26-33: LGTM: Well-documented destination management functions.The function declarations for
GetDestinationForKeyandGetAllDestinationsForKeyare properly documented with clear parameter descriptions and usage notes. The[[nodiscard]]attribute onGetDestinationForKeyis appropriate for preventing accidental result discarding.
40-40: LGTM: Consistent API extension.The addition of the
OutputTypeparameter toAddAndGetDestinationForScriptis a logical extension that maintains backward compatibility while enabling explicit output type specification.src/wallet/test/wallet_tests.cpp (5)
560-564: LGTM: Proper adaptation to new CreateTransaction API.The test correctly adapts to the new
CreateTransactionAPI that returnsstd::optional<CreatedTransactionResult>instead of using output parameters. The change from checking a boolean return value to checkingtxr.has_value()and extracting the transaction withtxr->txis appropriate.
871-881: LGTM: Consistent variable naming and proper wallet spend checks.The changes properly update the variable naming (
prev_txinstead of inline usage) and correctly use the transaction reference inHasWalletSpend()calls. The logic remains functionally equivalent while improving readability.
1040-1094: LGTM: Comprehensive CreateTransaction API adaptation.The test infrastructure correctly adapts to the new
CreateTransactionAPI:
- Properly handles the
std::optional<CreatedTransactionResult>return type- Correctly extracts transaction and change position from the result
- Updates change position comparisons to use
RANDOM_CHANGE_POSITIONconstant- Maintains all existing test logic and assertions
1110-1120: LGTM: Consistent API adaptation in GetCoins helper.The
GetCoinshelper function properly adapts to the new API pattern, maintaining consistency with other test methods. The variable initialization and extraction logic is correct.
1450-1458: LGTM: Proper handling of CreateTransaction results in conflict test.The test correctly adapts to the new API for both transaction creation calls, properly extracting transactions and using them in subsequent operations. The error handling and result verification remain intact.
src/outputtype.cpp (7)
8-17: LGTM: Appropriate includes for output type functionality.The added includes are necessary for the new output type functionality:
pubkey.hforCPubKeyusagescript/script.handscript/sign.hfor script handlingutil/vector.hfor theVectorutility functionstringfor string operations
18-22: LGTM: Well-defined constants and output type array.The string constants provide clear, consistent naming for output types, and the
OUTPUT_TYPESarray provides a convenient way to iterate over all available output types. The static constants are appropriately scoped.
23-33: LGTM: Robust output type parsing function.The
ParseOutputTypefunction correctly:
- Uses string comparison to identify output types
- Properly sets the output parameter on success
- Returns appropriate boolean values for success/failure
- Handles both
LEGACYandUNKNOWNtypes
35-42: LGTM: Consistent formatting function with proper error handling.The
FormatOutputTypefunction correctly:
- Uses a switch statement without default case to enable compiler warnings
- Returns string references to avoid unnecessary copying
- Includes an assertion for unreachable code as a safety measure
- Handles both defined output types
44-51: LGTM: Appropriate destination generation for different output types.The
GetDestinationForKeyfunction correctly:
- Returns
PKHashfor legacy output type, which is appropriate for P2PKH addresses- Intentionally asserts on
UNKNOWNtype since it should never be used in destination generation- Uses switch without default case for compiler warnings
- Follows the established pattern of the codebase
53-58: LGTM: Simple and correct implementation for getting all destinations.The
GetAllDestinationsForKeyfunction correctly:
- Creates a
PKHashdestination for the given key- Uses
Vectorutility to create the return vector- Handles the current Dash Core limitation to P2PKH (legacy) addresses only
- Returns a vector containing the single destination type
60-73: LGTM: Proper script destination handling with output type awareness.The
AddAndGetDestinationForScriptfunction correctly:
- Adds the script to the keystore as before
- Creates a
ScriptHashfrom the script- Handles the
LEGACYoutput type by adding the P2SH script to the keystore- Properly asserts on
UNKNOWNtype since it shouldn't be used for script destinations- Maintains the existing behavior while adding output type awareness
src/wallet/wallet.cpp (14)
306-312: LGTM! The validation correctly prevents invalid wallet configuration.The check appropriately rejects wallet creation with a passphrase when private keys are disabled, since passphrases are only used to encrypt private keys.
434-434: Good modernization to useautofor iterator.This improves code readability while maintaining type safety.
548-548: Consistent use ofautofor iterator declaration.
566-577: Improved method to check spends across all transaction outputs.The refactored method now accepts a
CTransactionRefand checks all outputs in a single call, which is more efficient and less error-prone than requiring callers to check each output individually.
639-639: Consistent iterator modernization.
1178-1184: Clean refactoring to use range-based loop.The code is more readable and idiomatic with the range-based for loop over
equal_rangeresults.
1245-1252: Consistent refactoring pattern applied.Same improvement as in
AbandonTransaction- cleaner range-based loop.
1388-1388: Auto iterator usage maintained consistently.
1548-1563: Well-designed refactoring of dummy signing logic.The function is now more flexible as a free function and correctly uses max signatures for watch-only or external inputs to ensure sufficient fee calculation. The comments clearly explain the rationale.
1566-1583: Correct implementation supporting external inputs.The method properly attempts signing with the wallet's provider first, then falls back to the external provider from coin control, enabling support for external inputs as described in the PR objectives.
1971-1971: Iterator modernization maintained.
3398-3398: Consistent auto usage for iterators.
3853-3853: Improved logging with proper type formatting.Using
FormatOutputTypeprovides better abstraction and consistency compared to hardcoded strings.
3871-3871: Consistent logging improvement applied.Same formatting improvements as in
LoadActiveScriptPubKeyMan.src/wallet/coincontrol.h (6)
75-76: Well-designed external input handling infrastructure.The addition of
m_external_providerand associated methods provides a clean interface for managing external inputs that require solving data for fee estimation. The encapsulation of external outputs in a private map with public accessor methods follows good design practices.Also applies to: 91-116
74-75: LGTM!The addition of
m_external_provideris well-documented and provides necessary functionality for handling external inputs' signing data.
91-105: LGTM!The external output query methods are well-implemented with proper const-correctness and clear return semantics.
111-116: LGTM!The
SelectExternal()method correctly maintains consistency between the selected set and external outputs map.
162-164: LGTM!The private member variables are well-designed with appropriate types and access control. The accompanying public methods provide proper encapsulation.
49-52: No remaining references tofAllowOtherInputs
- Verified that
fAllowOtherInputsdoes not appear anywhere in the codebase.- Confirmed
m_allow_other_inputsis used consistently in:
• src/wallet/coincontrol.h
• src/wallet/spend.cpp
• src/wallet/rpc/spend.cpp
• src/wallet/test/coinselector_tests.cpp
• src/coinjoin/util.cppThe rename is complete and all usages have been updated.
src/wallet/rpc/spend.cpp (2)
70-74: Correctly adapted to new CreateTransaction API.The changes properly handle the new
optional<CreatedTransactionResult>return type while maintaining the same error handling behavior.
538-541: Clear documentation for external input requirements.The help text properly documents that external inputs must be in the UTXO set and require solving data, with a well-structured explanation of the solving_data parameter format.
Also applies to: 572-591
src/wallet/wallet.h (4)
292-293: Good security improvement using salted hashers.Switching to
unordered_map/multimapwith salted hashers (SaltedOutpointHasher,SaltedTxidHasher) prevents hash collision attacks while likely improving performance. Since the code doesn't appear to rely on the ordering properties ofstd::map, this is a safe change.Also applies to: 474-474
703-710: Improved API flexibility with CCoinControl parameter.Replacing the boolean
use_max_sigparameter withCCoinControl*provides more flexibility for controlling signing behavior while maintaining backward compatibility through the default nullptr parameter.
1111-1111: Good refactoring to support external input signing.Moving
DummySignInputto a free function in the wallet namespace allows dummy signing of external inputs without requiring a wallet instance, which aligns with the external input support added throughout this PR.
824-824: Confirm HasWalletSpend implementation correctness
The function insrc/wallet/wallet.cppusestx->GetHash()and iterates overtx->voutto construct eachCOutPoint, checking againstmapTxSpends—exactly as expected for the newCTransactionRefsignature. No further changes are needed.src/wallet/spend.h (4)
16-17: Good practice: Named constant replaces magic number.Defining
RANDOM_CHANGE_POSITIONas -1 improves code readability and makes the special value's purpose clear throughout the codebase.
19-27: Improved size calculation APIs with CCoinControl support.The updated signatures replacing boolean parameters with
CCoinControl*provide better control over signature size assumptions, particularly important for accurate fee estimation with external inputs.
98-107: Clean API design with structured result type.The
CreatedTransactionResultstruct is a significant improvement over the previous design with multiple output parameters. It makes the API cleaner, safer, and easier to use.
111-113: Excellent API modernization for CreateTransaction.Returning
std::optional<CreatedTransactionResult>is a much cleaner design than the previous boolean return with multiple output parameters. It clearly indicates success/failure and encapsulates all results in a type-safe manner.src/wallet/spend.cpp (7)
11-11: LGTM!The include is necessary for the new
SigningProviderparameter in the refactored functions.
31-45: Well-structured overloads for external input support.The new overloads properly extend the existing functionality to support external inputs with coin control. The helper function provides a clean interface for wallet-based calls.
48-78: Robust handling of wallet and external inputs.The refactored functions correctly handle inputs from both the wallet and coin control, with proper bounds checking and error handling.
168-169: Correct implementation of coin control filtering.The logic properly respects the
m_allow_other_inputsflag when coin control has selected outputs.
455-494: Comprehensive handling of preset inputs including external ones.The refactored logic correctly processes both wallet and external inputs, with proper validation and error handling for unsolvable outputs.
1045-1072: Smart optimization with avoid partial spends fallback.The implementation correctly attempts grouped coin selection as an optimization, with proper fee comparison and change destination reuse to avoid BIP44 index issues.
1096-1113: Proper integration of external UTXO handling.The implementation correctly fetches UTXO data for external inputs and distinguishes between wallet and external outputs for coin control selection.
| //! If true, the selection process can add extra unselected inputs from the wallet | ||
| //! while requires all selected inputs be used | ||
| bool m_allow_other_inputs = false; | ||
| //! If false, only include as many inputs as necessary to fulfill a coin selection request. Only usable together with m_allow_other_inputs |
There was a problem hiding this comment.
Breaking change: Inverted logic in member variable rename requires careful migration.
The rename from fAllowOtherInputs to m_allow_other_inputs appears to maintain the same logical meaning (true = allow other inputs), which is good. However, this is a breaking API change that requires all existing code using CCoinControl to be updated.
Ensure all code that constructs or uses CCoinControl objects has been updated to use the new member name, particularly in:
- RPC implementations
- Tests
- Any external code that might depend on this API
🤖 Prompt for AI Agents
In src/wallet/coincontrol.h around lines 49 to 52, the member variable
fAllowOtherInputs was renamed to m_allow_other_inputs with the same logical
meaning, causing a breaking API change. To fix this, update all code that
constructs or uses CCoinControl objects to replace any usage of
fAllowOtherInputs with m_allow_other_inputs. This includes RPC implementations,
test files, and any external code depending on this API, ensuring consistent and
correct references to the renamed member variable.
| int64_t GetInputWeight(const COutPoint& outpoint) const | ||
| { | ||
| auto it = m_input_weights.find(outpoint); | ||
| assert(it != m_input_weights.end()); | ||
| return it->second; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider returning an optional instead of asserting in GetInputWeight.
The assert in GetInputWeight will crash the program if the weight doesn't exist. While the caller should check HasInputWeight first, this creates a fragile API that could lead to crashes in production.
Consider making the API safer:
-int64_t GetInputWeight(const COutPoint& outpoint) const
-{
- auto it = m_input_weights.find(outpoint);
- assert(it != m_input_weights.end());
- return it->second;
-}
+std::optional<int64_t> GetInputWeight(const COutPoint& outpoint) const
+{
+ auto it = m_input_weights.find(outpoint);
+ if (it != m_input_weights.end()) {
+ return it->second;
+ }
+ return std::nullopt;
+}📝 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.
| int64_t GetInputWeight(const COutPoint& outpoint) const | |
| { | |
| auto it = m_input_weights.find(outpoint); | |
| assert(it != m_input_weights.end()); | |
| return it->second; | |
| } | |
| std::optional<int64_t> GetInputWeight(const COutPoint& outpoint) const | |
| { | |
| auto it = m_input_weights.find(outpoint); | |
| if (it != m_input_weights.end()) { | |
| return it->second; | |
| } | |
| return std::nullopt; | |
| } |
🤖 Prompt for AI Agents
In src/wallet/coincontrol.h around lines 142 to 147, the GetInputWeight method
uses an assert to check if the input weight exists, which can crash the program
if the weight is missing. To make the API safer, change the return type to
std::optional<int64_t> and return std::nullopt when the input weight is not
found instead of asserting. This avoids crashes and forces callers to handle the
missing case explicitly.
We didn't need this as we don't support SegWit but we should be aligned with upstream regardless, it will help with the next step.
…sbt to take external inputs
* CWallet::CreateTransaction() creates its own `ReserveDestination` through CreateTransactionInternal() and doesn't need one. * CWallet::CommitTransaction() submits an already-formed transaction and does not need keypool reservation.
…onal struct We deviate from upstream by consolidating our RANDOM_CHANGE_POSITION definition to wallet/spend.h. This is because we use it far more than upstream and continuous redefinition would therefore be more bothersome for us. This should be a temporary deviation though as upstream eventually replaces change_pos with an std::optional, which should realign us with them. But until then, let's treat this as an opportunity to consolidate usage.
Our current code runs AddInput() on every iteration of preset_inputs, which is incorrect as it results in accumulation of duplicate values but this went unnoticed since m_selected_inputs is a std::set and it handled de-duplication for us. This is an implementation detail and we should not rely on it. Added a unit test to avoid potential regression.
We don't fully embrace ignoring vCoins as it would be a departure from earlier behavior. Instead, we will only give it preference if Dash-specific conditions are met but default to upstream behavior if they aren't.
…uplicated operations
includes: - 940a219 (only changes to `src/wallet/wallet.cpp`, some functions) We already introduced outputtype.{cpp,h} source files in ed88ba7 but we didn't port over everything as we didn't need it at the time but the next commit does, while the changes were introduced in wallet/wallet.{cpp,h}, we will be impl. them in outputtype.{cpp,h}
This does little for us now but realignment opens up the ability to use different output types in future versions by retaining the skeleton.
Adapted from f5649db (bitcoin#25734) but modified to account for missing backports. Completing them currently out of scope but enum entry already exists, so must be defined for completeness' sake.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/functional/wallet_createwallet.py (1)
29-32: Fix indentation and validate test logic.The test correctly verifies that providing a passphrase while disabling private keys raises the expected RPC error. The test also validates that this failure doesn't prevent subsequent wallet creation with the same name.
Fix the indentation issue flagged by static analysis:
- assert_raises_rpc_error(-4, "Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled.", - self.nodes[0].createwallet, wallet_name='w0', descriptors=True, disable_private_keys=True, passphrase="passphrase") + assert_raises_rpc_error(-4, "Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled.", + self.nodes[0].createwallet, wallet_name='w0', descriptors=True, disable_private_keys=True, passphrase="passphrase")Also note the extra space in
wallet_name='w0', descriptors=True- consider removing the double space for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
src/bench/coin_selection.cpp(1 hunks)src/coinjoin/util.cpp(5 hunks)src/interfaces/wallet.h(2 hunks)src/outputtype.cpp(1 hunks)src/outputtype.h(1 hunks)src/rpc/evo.cpp(2 hunks)src/rpc/util.cpp(1 hunks)src/wallet/coincontrol.h(6 hunks)src/wallet/coinjoin.cpp(1 hunks)src/wallet/interfaces.cpp(2 hunks)src/wallet/rpc/addresses.cpp(3 hunks)src/wallet/rpc/spend.cpp(12 hunks)src/wallet/scriptpubkeyman.cpp(1 hunks)src/wallet/scriptpubkeyman.h(1 hunks)src/wallet/spend.cpp(22 hunks)src/wallet/spend.h(2 hunks)src/wallet/test/coinjoin_tests.cpp(3 hunks)src/wallet/test/coinselector_tests.cpp(4 hunks)src/wallet/test/spend_tests.cpp(1 hunks)src/wallet/test/wallet_tests.cpp(8 hunks)src/wallet/wallet.cpp(13 hunks)src/wallet/wallet.h(7 hunks)test/functional/rpc_fundrawtransaction.py(5 hunks)test/functional/rpc_psbt.py(2 hunks)test/functional/wallet_createwallet.py(1 hunks)test/functional/wallet_send.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (25)
- src/wallet/scriptpubkeyman.h
- src/wallet/coinjoin.cpp
- src/interfaces/wallet.h
- src/coinjoin/util.cpp
- src/rpc/util.cpp
- src/bench/coin_selection.cpp
- src/wallet/test/spend_tests.cpp
- src/wallet/test/coinjoin_tests.cpp
- src/wallet/scriptpubkeyman.cpp
- src/wallet/test/coinselector_tests.cpp
- test/functional/wallet_send.py
- src/wallet/rpc/addresses.cpp
- test/functional/rpc_psbt.py
- src/rpc/evo.cpp
- src/outputtype.h
- test/functional/rpc_fundrawtransaction.py
- src/wallet/test/wallet_tests.cpp
- src/wallet/interfaces.cpp
- src/wallet/coincontrol.h
- src/wallet/rpc/spend.cpp
- src/wallet/wallet.cpp
- src/outputtype.cpp
- src/wallet/spend.h
- src/wallet/wallet.h
- src/wallet/spend.cpp
🧰 Additional context used
📓 Path-based instructions (1)
test/functional/**/*.py
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Favor extension over modification of the Bitcoin Core foundation
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to src/evo/specialtx.h : Special transactions must use payload extensions as implemented in src/evo/specialtx.h
test/functional/wallet_createwallet.py (3)
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#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.
Learnt from: kwvg
PR: dashpay/dash#6726
File: test/functional/rpc_createmultisig.py:120-120
Timestamp: 2025-06-20T23:32:16.225Z
Learning: In the rpc_createmultisig.py test, the checkbalances() function correctly excludes 9 blocks from the balance calculation: 8 blocks from do_multisig() calls (2 blocks per call × 4 calls) plus 1 block from checkbalances() itself.
🧬 Code Graph Analysis (1)
test/functional/wallet_createwallet.py (1)
test/functional/test_framework/util.py (1)
assert_raises_rpc_error(132-148)
🪛 Flake8 (7.2.0)
test/functional/wallet_createwallet.py
[error] 32-32: continuation line under-indented for visual indent
(E128)
⏰ 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). (7)
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: x86_64-apple-darwin / Build depends
- GitHub Check: x86_64-pc-linux-gnu_multiprocess / Build depends
- GitHub Check: Lint / Run linters
- GitHub Check: x86_64-pc-linux-gnu / Build depends
…uments ead7dd0 fix: resolve compile error due to change in function arguments (Kittywhiskers Van Gogh) Pull request description: ## Additional Information [dash#6570](#6570) and [dash#6755](#6755) were merged into `develop` in that order, neither conflicting with the other. Unfortunately, as [dash#6570](#6570) updated the arguments list for `SetupDescriptorScriptPubKeyMans()`, [dash#6755](#6755) introduced new usage based on the older argument list and as it was new code, it was not registered as a conflict but has caused `develop` to fail ([build](https://github.com/dashpay/dash/actions/runs/16336328705)). This pull request remedies that issue. ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: knst: utACK ead7dd0 UdjinM6: utACK ead7dd0 Tree-SHA512: 74ef89ca8295a02e6d144a2a3b312604c7446b24b176084da8c88d042b06ff794a5cb5a699ae4ae7386856e8778d1e5a42a338e0f3c813b84de7c6f0e9aae084
3 participants
Additional Information
The
embeddedfield was introduced in bitcoin#11403 (3eaa003) for adding RPC awareness for P2SH (Legacy) SegWit addresses. While it is primarily used for SegWit, it can be used in non-SegWit contexts (e.g. atypical descriptor usage).As Dash Core does not support SegWit, this was not implemented and the leftover reference to
embeddedwas removed fromgetaddressinfoin dash#4675 (ec162d7).But, bitcoin#17211 engages in this exact type of atypical descriptor usage in
rpc_psbt.py, embedding a P2PKH address (which is valid in Dash Core) in a P2SH (source). The exact scenario that was considered outside the scope ofgetaddressinfobut now causes a test failure (see below).Test failure:
This has since been remedied.
Since bitcoin#16208 (78fdc99), neither
CreateTransaction()norCommitTransaction()have consumed aReserveDestination(thenCReserveKey) but they have still persisted despite serving no purpose.CreateTransaction(), throughCreateTransactionInternal()generates its ownReserveDestination(source) andCommitTransaction()has no use for it.This has since been remedied.
When backporting bitcoin#20640, upstream took the decision to define
RANDOM_CHANGE_POSITIONat every location it was used, while unclear why this is the case, this is somewhat feasible for them. Dash Core, courtesy of CoinJoin, references the value significantly more often. So, we deviate from upstream and defineRANDOM_CHANGE_POSITIONinwallet/spend.hto consolidate definitions.Dash-specific code has been updated to use
RANDOM_CHANGE_POSITIONwhen possible to specify what the magic number (-1) is supposed to signify.This deviation from upstream will resolve itself when backporting 758501b from bitcoin#25273, which does away with
RANDOM_CHANGE_POSITIONaltogether.When translating
!fRequireAllInputsbehavior (introduced in dash#2581) for bitcoin#22019 (c5038c1), it engaged in erroneous behavior of adding the whole set ofpreset_inputstoresultwhile iterating instead of after (source). This results in unwanted accumulative behavior but did not surface asSelectionResultinternally uses astd::set(source), which meant accumulative behavior was mitigated by deduplication.But this is an implementation detail and should not be relied for correctness of malformed code. To remedy this, the code has been reworked to call
SelectionResult::AddInput()afternTargetValueis met and a test,minimum_inputs_testhas been introduced tocoinselector_teststo validate that:!fRequireAllInputswill result in only consuming the minimum required set of coin to matchnTargetValueandGetSelectedValue()doesn't count the same coin multiple times)To preserve the expected behavior of
!m_allow_other_inputs && !fRequireAllInputs, despite the goal of bitcoin#25118 to reducevCoinsusage, we retain it only for the Dash-specific condition, falling back to upstream behavior that condition isn't met or ifnTargetValueis not satisfied.While Dash only supports P2PKH as the primary address type (a.k.a.
OutputType::LEGACY), upstream supports multiple with the introduction of SegWit and introduced helper functions to track those multiple address types. They were introduced in bitcoin#11403 and as Dash doesn't implement SegWit, it wasn't implemented.Though, despite that, the
outputtype.{cpp,h}source files were introduced when backporting bitcoin#17261 (ed88ba7) to fulfil that backport. As there are backports that may require those helper functions (e.g. bitcoin#25790), they were implemented from bitcoin#11403 but directly applied tooutputtype.{cpp,h}.OutputType::UNKNOWNexists as a valid entry indevelop(source), the helper functions were modified to account for this valid enum value courtesy of f5649db from bitcoin#25734. Liberties were taken to ensure the smallest set of changes needed were implemented.Breaking Changes
None expected.
How Has This Been Tested?
A full CoinJoin session run on 7194128
Checklist