Merge bitcoin/bitcoin#25730: RPC: listunspent, add "include immature coinbase" flag #1233
Conversation
…" flag fa84df1 scripted-diff: wallet: rename AvailableCoinsParams members to snake_case (furszy) 61c2265 wallet: group AvailableCoins filtering parameters in a single struct (furszy) f0f6a35 RPC: listunspent, add "include immature coinbase" flag (furszy) Pull request description: Simple PR; adds a "include_immature_coinbase" flag to `listunspent` to include the immature coinbase UTXOs on the response. Requested by bitcoin#25728. ACKs for top commit: danielabrozzoni: reACK fa84df1 achow101: ACK fa84df1 aureleoules: reACK fa84df1 kouloumos: reACK fa84df1 theStack: Code-review ACK fa84df1 Tree-SHA512: 0f3544cb8cfd0378a5c74594480f78e9e919c6cfb73a83e0f3112f8a0132a9147cf846f999eab522cea9ef5bd3ffd60690ea2ca367dde457b0554d7f38aec792
WalkthroughThis pull request introduces a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
doc/release-notes-25730.md(1 hunks)src/wallet/rpc/coins.cpp(6 hunks)src/wallet/spend.cpp(7 hunks)src/wallet/spend.h(1 hunks)test/functional/wallet_balance.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
doc/**
📄 CodeRabbit inference engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the doc directory (documentation)
Files:
doc/release-notes-25730.md
**
⚙️ CodeRabbit configuration file
**: # CodeRabbit AI Review Instructions for Dash BackportsYour Role
You are reviewing Bitcoin Core backports to Dash Core. Your ONLY job is to validate that the Dash commit faithfully represents the original Bitcoin commit with minimal, necessary adaptations.
Critical Validation Rules
1. File Operations Must Match (AUTO-REJECT if violated)
- If Bitcoin modifies an existing file → Dash MUST modify (not create new)
- If Bitcoin creates a new file → Dash creates
- If Bitcoin deletes a file → Dash deletes
- Common failure: Bitcoin modifies keys.txt, Dash creates new file with 58 keys
2. Size Ratio Check (80-150% of Bitcoin)
- Count functional lines changed (exclude comments/whitespace)
- Dash changes should be 80-150% of Bitcoin's size
- Red flag: 2-line Bitcoin fix becoming 150+ lines in Dash
3. No Scope Creep
- Reject if you see: "TODO:", "FIXME:", "while we're here", "also fix"
- No unrelated refactoring or style changes
- Only Bitcoin's intended changes + minimal Dash adaptations
4. Bitcoin-Specific Code Detection
- Auto-reject witness/segwit code:
msg_wtxidrelay,MSG_WTX, witness imports- Auto-reject RBF (replace-by-fee) functionality
- Note: PSBT is supported in Dash (don't flag)
5. Mandatory Adaptations Only
bitcoin→dashin strings/pathsBitcoin→Dashin user-facing text- Port numbers: 8332→9998 (RPC), 8333→9999 (P2P)
- Hardcoded test values specific to Dash
- No other changes unless absolutely required
6. Completeness Check
- All files changed in Bitcoin must be present
- Extra files need clear justification (Dash-specific compatibility)
- Missing files = incomplete backport
Review Process
- First: Check file operations match exactly
- Second: Calculate size ratio
- Third: Scan for scope creep patterns
- Fourth: Detect Bitcoin-specific code
- Fifth: Verify all changes are minimal adaptations
Output Format
VALIDATION: [PASS/FAIL] File Operatio...
Files:
doc/release-notes-25730.mdtest/functional/wallet_balance.pysrc/wallet/rpc/coins.cppsrc/wallet/spend.cppsrc/wallet/spend.h
test/functional/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Functional tests should be placed in test/functional/ and written in Python
Files:
test/functional/wallet_balance.py
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
C++20 codebase should be placed under src/
Files:
src/wallet/rpc/coins.cppsrc/wallet/spend.cppsrc/wallet/spend.h
🧠 Learnings (4)
📓 Common learnings
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-31T01:14:55.631Z
Learning: DashCoreAutoGuix successfully completed a complex Bitcoin Core backport (PR #29412) for block mutation detection by implementing the IsBlockMutated function, adding net processing integration, creating comprehensive unit tests, and properly adapting all Bitcoin-specific witness code for Dash compatibility. The backport maintains full security functionality while respecting Dash's non-witness transaction architecture.
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-28T20:34:29.061Z
Learning: During Dash backport verification of Bitcoin Core commit 06d469c26b, scope creep was detected when additional pruning test functionality was added to interface_usdt_utxocache.py beyond what was in the original Bitcoin commit. The fix involved removing the extra test block while maintaining the core compiler flag fixes for USDT compilation errors.
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-28T22:03:12.364Z
Learning: During multiple verification attempts of Bitcoin Core commit 06d469c26b backport to Dash PR #566, DashCoreAutoGuix consistently identified scope creep in interface_usdt_utxocache.py where additional pruning test functionality was added beyond the original Bitcoin commit. The user provided comprehensive fixes including both scope creep removal and missing mempool test file additions, but couldn't push due to authentication restrictions. The scope creep fix was identified as the priority to resolve CI failures.
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-28T19:54:21.426Z
Learning: In Dash backports from Bitcoin Core, including necessary compilation fixes (such as API compatibility changes like UniValue get_int() → getInt<int>()) alongside the core backport is standard and expected practice. These compatibility fixes ensure the backported code compiles in Dash's evolved codebase while preserving Bitcoin's original functionality and intent.
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-29T21:29:32.827Z
Learning: DashCoreAutoGuix successfully fixed scope creep in Bitcoin Core commit fcdb39d3ee backport by removing the parse test case from src/test/uint256_tests.cpp that was not part of the original Bitcoin commit. The fix was implemented in commit 16748115ce and verified through range-diff analysis.
📚 Learning: 2025-07-30T14:45:15.700Z
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-30T14:45:15.700Z
Learning: The `add_wallet_options` method is not available in Dash's test framework and should be removed when backporting Bitcoin Core tests that use this method, as demonstrated in wallet_rescan_unconfirmed.py.
Applied to files:
test/functional/wallet_balance.py
📚 Learning: 2025-07-28T20:34:29.061Z
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-28T20:34:29.061Z
Learning: During Dash backport verification of Bitcoin Core commit 06d469c26b, scope creep was detected when additional pruning test functionality was added to interface_usdt_utxocache.py beyond what was in the original Bitcoin commit. The fix involved removing the extra test block while maintaining the core compiler flag fixes for USDT compilation errors.
Applied to files:
test/functional/wallet_balance.py
📚 Learning: 2025-07-30T14:45:15.700Z
Learnt from: DashCoreAutoGuix
Repo: DashCoreAutoGuix/dash PR: 0
File: :0-0
Timestamp: 2025-07-30T14:45:15.700Z
Learning: In Dash backports from Bitcoin Core test files, the `address_to_scriptpubkey` function should be imported from `test_framework.wallet` instead of `test_framework.address` as the import location differs between Bitcoin and Dash test frameworks.
Applied to files:
test/functional/wallet_balance.py
🧬 Code graph analysis (3)
test/functional/wallet_balance.py (1)
src/wallet/rpc/coins.cpp (2)
listunspent(505-736)listunspent(505-505)
src/wallet/rpc/coins.cpp (1)
src/wallet/spend.cpp (2)
AvailableCoinsListUnspent(265-269)AvailableCoinsListUnspent(265-265)
src/wallet/spend.h (1)
src/wallet/spend.cpp (4)
AvailableCoins(100-263)AvailableCoins(100-103)AvailableCoinsListUnspent(265-269)AvailableCoinsListUnspent(265-265)
🪛 Cppcheck (2.18.0)
src/wallet/rpc/coins.cpp
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
src/wallet/spend.cpp
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
🪛 markdownlint-cli2 (0.18.1)
doc/release-notes-25730.md
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
6-6: Files should end with a single newline character
(MD047, single-trailing-newline)
⏰ 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 container / Build container
- GitHub Check: Build slim container / Build container
🔇 Additional comments (15)
test/functional/wallet_balance.py (1)
81-92: LGTM! Test coverage for include_immature_coinbase is appropriate.The tests correctly verify:
- Immature coinbase UTXOs are included when
include_immature_coinbase=True- Immature coinbase UTXOs are excluded when
include_immature_coinbase=False- Behavior differences between descriptor and legacy wallets are properly handled
The test logic aligns with the RPC implementation in
src/wallet/rpc/coins.cpp.src/wallet/spend.h (2)
58-71: LGTM! CoinFilterParams struct is well-designed.The struct properly consolidates the coin filtering parameters with sensible defaults:
min_amount{1}- filters out zero-value outputs by defaultmax_amount{MAX_MONEY}- no upper limit by defaultmin_sum_amount{MAX_MONEY}- return all UTXOs by default (won't trigger early return)max_count{0}- no count limit by defaultonly_spendable{true}- conservative default for transaction creationinclude_immature_coinbase{false}- excludes immature coinbases by default (safe)
76-85: LGTM! Function signatures properly updated.The updated signatures for
AvailableCoinsandAvailableCoinsListUnspentcorrectly use the newCoinFilterParamsparameter with default values, maintaining backward compatibility for existing call sites.Note:
AvailableCoinsListUnspenttakesCoinFilterParamsby value (not const ref) to allow internal modification ofonly_spendablein the implementation, which is the correct design choice.src/wallet/rpc/coins.cpp (4)
530-530: LGTM! New RPC option properly documented.The
include_immature_coinbaseoption is correctly added to thequery_optionsobject with appropriate type (BOOL), default value (false), and description.
602-603: Correct initialization of CoinFilterParams for listunspent.Setting
filter_coins.min_amount = 0explicitly overrides the default value of1fromCoinFilterParams. This is intentional sincelistunspentshould display all UTXOs including those with zero value (unless the user specifies a minimum), while the default of1inCoinFilterParamsis for transaction creation contexts.
610-649: LGTM! Filter options correctly integrated with Dash-specific coinType preserved.The implementation:
- Validates all option keys including the new
include_immature_coinbase- Populates
CoinFilterParamsfields from query options- Preserves Dash's
coinTypefiltering (lines 637-645)- Properly handles the new
include_immature_coinbaseoption (lines 647-649)
664-666: LGTM! Updated call to AvailableCoinsListUnspent.The function call correctly passes the populated
filter_coinsparameter toAvailableCoinsListUnspent, completing the integration ofCoinFilterParamsin the listunspent RPC.src/wallet/spend.cpp (8)
100-103: LGTM! AvailableCoins signature correctly updated.The function signature properly accepts
const CoinFilterParams& paramswith a default empty parameter, maintaining backward compatibility.
122-123: LGTM! Immature coinbase filtering using CoinFilterParams.The check correctly uses
params.include_immature_coinbaseto control whether immature coinbase UTXOs are included in the result.
181-182: LGTM! Amount filtering using CoinFilterParams.The value filtering correctly uses
params.min_amountandparams.max_amountto filter outputs by their value.
212-212: LGTM! Spendable filtering using CoinFilterParams.The
params.only_spendableflag correctly controls whether non-spendable outputs are filtered out.
249-258: LGTM! Early return conditions using CoinFilterParams.The early return logic correctly uses:
params.min_sum_amountto stop when the target sum is reachedparams.max_countto limit the number of UTXOs returned
265-269: LGTM! AvailableCoinsListUnspent wrapper correctly implemented.The wrapper:
- Takes
CoinFilterParamsby value to allow modification- Sets
only_spendable = falsesince this is for listing, not transaction creation- Passes through to
AvailableCoinswithfeerate = std::nullopt
271-275: LGTM! GetAvailableBalance simplified.The function now uses the default
CoinFilterParams{}implicitly when callingAvailableCoins, which is correct since it needs all spendable coins with default filtering.
888-888: LGTM! CreateTransactionInternal uses default CoinFilterParams.The call to
AvailableCoinsin transaction creation correctly uses the defaultCoinFilterParams{}(implicitly), which hasonly_spendable = trueand appropriate defaults for transaction funding.
| RPC Wallet | ||
| ---------- | ||
|
|
||
| - RPC `listunspent` now has a new argument `include_immature_coinbase` | ||
| to include coinbase UTXOs that don't meet the minimum spendability | ||
| depth requirement (which before were silently skipped). (#25730) No newline at end of file |
There was a problem hiding this comment.
Minor formatting issues detected by linter.
The release notes content is correct. However, address these markdown issues:
- Line 1: Add a top-level heading (e.g.,
# Release Notes) - Line 6: Add a trailing newline at end of file
Apply this diff:
+# Release Notes
+
RPC Wallet
----------
- RPC `listunspent` now has a new argument `include_immature_coinbase`
to include coinbase UTXOs that don't meet the minimum spendability
- depth requirement (which before were silently skipped). (#25730)
+ depth requirement (which before were silently skipped). (#25730)
+📝 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.
| RPC Wallet | |
| ---------- | |
| - RPC `listunspent` now has a new argument `include_immature_coinbase` | |
| to include coinbase UTXOs that don't meet the minimum spendability | |
| depth requirement (which before were silently skipped). (#25730) | |
| # Release Notes | |
| RPC Wallet | |
| ---------- | |
| - RPC `listunspent` now has a new argument `include_immature_coinbase` | |
| to include coinbase UTXOs that don't meet the minimum spendability | |
| depth requirement (which before were silently skipped). (#25730) | |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
6-6: Files should end with a single newline character
(MD047, single-trailing-newline)
🤖 Prompt for AI Agents
In doc/release-notes-25730.md around lines 1 to 6, the linter flagged minor
Markdown formatting issues: add a top-level heading at the top of the file (for
example “# Release Notes”) and ensure the file ends with a trailing newline;
update the first line to include the heading and append a final newline
character at EOF so the file passes linting.
Backports bitcoin#25730
Original commit: f0c646f
Summary
CoinFilterParamsstruct to group AvailableCoins filtering parametersinclude_immature_coinbaseflag tolistunspentRPC to include immature coinbase UTXOs in the responseDash-specific notes
src/bench/wallet_create_tx.cppnot modified (file doesn't exist in Dash)src/wallet/rpc/spend.cppnot modified (Bitcoin's change was insidesendall()which Dash doesn't have)coinTypefiltering option in listunspent🤖 Generated with Claude Code
Summary by CodeRabbit
include_immature_coinbaseparameter (default: false). When enabled, the method includes coinbase UTXOs that have not yet reached the minimum confirmations required for spending, which were previously excluded from results. This provides greater flexibility in UTXO selection.✏️ Tip: You can customize this high-level summary in your review settings.