backport: bitcoin#24836, #25379, #25773, #25957, #25986, #26158, #26294, #26349, #26388, #26404, #26424, #26625, #26920, #27350

[knst]

What was done?

Regular backports from bitcoin core v25

How Has This Been Tested?

Run unit & functional tests

Breaking Changes

N/A

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[coderabbitai]

Walkthrough

This PR adds fast descriptor-wallet rescans using compact block filters, a regtest-only submitpackage RPC for submitting transaction packages, moves URL headers from util/url.h to common/url.h and updates Makefile entries, changes the BENCHMARK macro to accept a priority and marks many benchmarks HIGH, extends the Chain interface with block-filter query/match methods and implements them, adds BCLog::SCAN, adds DescriptorScriptPubKeyMan index-aware accessors, introduces INVALID_PACKAGE error, and refactors many functional tests to use MiniWallet and exercise package/fast-rescan flows.

Sequence Diagram(s)

sequenceDiagram
    participant Node as Node/Wallet
    participant ChainImpl as ChainImpl
    participant FilterIdx as BlockFilterIndex
    participant Filter as GCSFilter

    Note over Node,Filter: Block Filter Matching Request
    Node->>ChainImpl: blockFilterMatchesAny(filter_type, block_hash, element_set)
    ChainImpl->>FilterIdx: GetBlockFilterIndex(filter_type)
    alt Index exists
        FilterIdx-->>ChainImpl: BlockFilterIndex instance
        ChainImpl->>ChainImpl: LookupFilter(block_hash)
        alt Filter found
            ChainImpl->>Filter: MatchAny(element_set)
            Filter-->>ChainImpl: bool (match result)
            ChainImpl-->>Node: std::optional<bool> (match result)
        else Filter not found
            ChainImpl-->>Node: std::nullopt
        end
    else Index not available
        FilterIdx-->>ChainImpl: nullptr
        ChainImpl-->>Node: std::nullopt
    end

Loading

sequenceDiagram
    participant Wallet as Wallet::Rescan
    participant BlockFilter as FastWalletRescanFilter
    participant ChainImpl as ChainImpl
    participant Node as Node/Block

    Note over Wallet,Node: Fast Rescan with Block Filters (Descriptor Wallet)
    Wallet->>BlockFilter: Create (from DescriptorScriptPubKeyMans)
    BlockFilter-->>BlockFilter: Build GCS filter set
    
    loop For each block in range
        Wallet->>BlockFilter: UpdateIfNeeded() [detect descriptor top-ups]
        Wallet->>BlockFilter: MatchesBlock(block_hash)
        alt Filter indicates match
            BlockFilter-->>Wallet: true
            Wallet->>Node: Fetch and inspect block
            Node-->>Wallet: Block data & transactions
            Wallet->>Wallet: Process transactions
        else Filter indicates no match
            BlockFilter-->>Wallet: false
            Wallet->>Wallet: Skip block (update scan state)
        else Filter cannot determine
            BlockFilter-->>Wallet: std::nullopt
            Wallet->>Wallet: Log warning, inspect block as fallback
        end
    end
    
    Wallet-->>Wallet: Rescan complete

Loading

sequenceDiagram
    participant RPC as submitpackage RPC
    participant Validation as Validation/Parser
    participant Mempool as Mempool
    participant Net as Network/Broadcast

    Note over RPC,Net: submitpackage RPC Execution
    RPC->>RPC: Decode hex transactions array
    RPC->>Validation: Parse & validate each transaction
    loop For each transaction
        alt Valid
            Validation-->>RPC: Transaction object
        else Invalid
            Validation-->>RPC: Error (rejected)
            RPC->>RPC: Record failure reason
        end
    end
    
    RPC->>Mempool: Process package via ProcessNewPackage
    alt Package policy valid
        Mempool-->>RPC: per-tx acceptance/rejection
        loop For each accepted tx
            Mempool->>Net: Broadcast transaction
            Net-->>RPC: Broadcast acknowledged
        end
        RPC-->>Caller: tx-results + optional package-feerate
    else Package policy invalid
        Mempool-->>RPC: Package error
        RPC-->>Caller: Error response
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.89% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the pull request as a set of backports from Bitcoin Core v25, listing the specific issue numbers being backported.
Description check	✅ Passed	The description relates to the changeset by explaining what was done (regular backports from Bitcoin Core v25), how it was tested (unit & functional tests), and confirming no breaking changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

Fix all issues with AI Agents 🤖

In @test/functional/mempool_packages.py:
- Around line 231-239: The comparison currently uses identity instead of
equality: replace the identity check `if utxo['txid'] is parent_transaction`
with an equality check against the txid string (i.e., compare utxo['txid'] ==
parent_transaction) so tx_children is populated reliably when the utxo txid
matches parent_transaction; update the condition in the loop that builds
chain/tx_children accordingly.

In @test/functional/rpc_packages.py:
- Around line 268-293: In test_submit_child_with_parents, replace uses of the
internal integer tx.hash with the hex txid string package_txn["txid"]: change
the loop membership check and lookup against submitpackage_result["tx-results"]
to use package_txn["txid"] (both the assert and tx_result assignment), and
change the peer.wait_for_broadcast call to pass a list of package_txn["txid"]
strings instead of tx["tx"].hash so wait_for_broadcast receives hex txid
strings.
- Around line 295-323: In test_submit_cpfp the child transaction is being
referenced by tx_child["tx"].hash and wait_for_broadcast is passed integer
hashes, which is inconsistent with the parent lookups (tx_rich/tx_poor using
"txid") and P2PTxInvStore API; change the child lookup to use
submitpackage_result["tx-results"][tx_child["txid"]] and pass txid strings to
peer.wait_for_broadcast by using [tx["txid"] for tx in package_txns] so all
indexing and broadcast checking consistently use txid strings (update references
to tx_child, package_txns, submitpackage_result, and peer.wait_for_broadcast
accordingly).

🧹 Nitpick comments (4)

test/functional/wallet_listsinceblock.py (1)

404-416: Consider using next() for safer list filtering.

The test logic correctly verifies that OP_RETURN outputs don't include an address field. However, the list comprehension with [0] indexing could raise IndexError if no matching transaction is found.

🔎 Safer alternative using next()

-        op_ret_tx = [tx for tx in self.nodes[2].listsinceblock(blockhash=block_hash)["transactions"] if tx['txid'] == tx_id][0]
+        op_ret_tx = next(tx for tx in self.nodes[2].listsinceblock(blockhash=block_hash)["transactions"] if tx['txid'] == tx_id)

This will raise StopIteration with a clearer error message if the transaction isn't found, or you can provide a default value:

op_ret_tx = next((tx for tx in self.nodes[2].listsinceblock(blockhash=block_hash)["transactions"] if tx['txid'] == tx_id), None)
assert op_ret_tx is not None, "Transaction not found"

test/functional/wallet_listtransactions.py (1)

170-179: Consider using next() for safer list filtering.

The test correctly verifies that OP_RETURN outputs don't include an address field in listtransactions. However, using [0] on the filtered list could raise IndexError if no matching transaction is found.

🔎 Safer alternative using next()

-        op_ret_tx = [tx for tx in self.nodes[0].listtransactions() if tx['txid'] == tx_id][0]
+        op_ret_tx = next(tx for tx in self.nodes[0].listtransactions() if tx['txid'] == tx_id)

This will raise StopIteration with a clearer error message if the transaction isn't found, or you can provide a default value with explicit error handling:

op_ret_tx = next((tx for tx in self.nodes[0].listtransactions() if tx['txid'] == tx_id), None)
assert op_ret_tx is not None, "Transaction not found in listtransactions"

test/functional/wallet_fast_rescan.py (2)

47-47: Replace print() with self.log.info() for consistency.

This appears to be leftover debug output. All other logging in this test uses self.log.info().

🔎 Proposed fix

-        print(w.importdescriptors([{"desc": descsum_create(f"wpkh({fixed_key.privkey})"), "timestamp": "now"}]))
+        self.log.info(w.importdescriptors([{"desc": descsum_create(f"wpkh({fixed_key.privkey})"), "timestamp": "now"}]))

---

52-52: Remove extraneous f-string prefix.

Static analysis correctly identifies this f-string has no placeholders. This is a minor style issue.

🔎 Proposed fix

-        self.log.info(f"Create txs sending to end range address of each descriptor, triggering top-ups")
+        self.log.info("Create txs sending to end range address of each descriptor, triggering top-ups")

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e376079 and b21e0a6.

📒 Files selected for processing (81)

• ci/dash/lint-tidy.sh
• ci/test/00_setup_env_mac_native_arm64.sh
• ci/test/04_install.sh
• doc/descriptors.md
• src/Makefile.am
• src/bench/addrman.cpp
• src/bench/base58.cpp
• src/bench/bech32.cpp
• src/bench/bench.h
• src/bench/bip324_ecdh.cpp
• src/bench/block_assemble.cpp
• src/bench/bls.cpp
• src/bench/bls_dkg.cpp
• src/bench/bls_pubkey_agg.cpp
• src/bench/ccoins_caching.cpp
• src/bench/chacha20.cpp
• src/bench/checkblock.cpp
• src/bench/checkqueue.cpp
• src/bench/coin_selection.cpp
• src/bench/crypto_hash.cpp
• src/bench/duplicate_inputs.cpp
• src/bench/ecdsa.cpp
• src/bench/ellswift.cpp
• src/bench/examples.cpp
• src/bench/gcs_filter.cpp
• src/bench/hashpadding.cpp
• src/bench/load_external.cpp
• src/bench/lockedpool.cpp
• src/bench/logging.cpp
• src/bench/mempool_eviction.cpp
• src/bench/mempool_stress.cpp
• src/bench/merkle_root.cpp
• src/bench/peer_eviction.cpp
• src/bench/poly1305.cpp
• src/bench/pool.cpp
• src/bench/pow_hash.cpp
• src/bench/prevector.cpp
• src/bench/rollingbloom.cpp
• src/bench/rpc_blockchain.cpp
• src/bench/rpc_mempool.cpp
• src/bench/strencodings.cpp
• src/bench/string_cast.cpp
• src/bench/util_time.cpp
• src/bench/verify_script.cpp
• src/bench/wallet_balance.cpp
• src/bench/wallet_create.cpp
• src/bench/wallet_loading.cpp
• src/bitcoin-cli.cpp
• src/bitcoin-wallet.cpp
• src/bitcoind.cpp
• src/common/url.cpp
• src/common/url.h
• src/interfaces/chain.h
• src/logging.cpp
• src/logging.h
• src/node/interfaces.cpp
• src/qt/main.cpp
• src/rpc/client.cpp
• src/rpc/mempool.cpp
• src/test/fuzz/rpc.cpp
• src/test/fuzz/string.cpp
• src/test/util/setup_common.cpp
• src/util/error.cpp
• src/util/error.h
• src/wallet/rpc/backup.cpp
• src/wallet/rpc/transactions.cpp
• src/wallet/rpc/util.cpp
• src/wallet/rpc/wallet.cpp
• src/wallet/scriptpubkeyman.cpp
• src/wallet/scriptpubkeyman.h
• src/wallet/wallet.cpp
• test/functional/mempool_package_limits.py
• test/functional/mempool_packages.py
• test/functional/rpc_getblockfrompeer.py
• test/functional/rpc_packages.py
• test/functional/test_framework/util.py
• test/functional/test_framework/wallet.py
• test/functional/test_runner.py
• test/functional/wallet_fast_rescan.py
• test/functional/wallet_listsinceblock.py
• test/functional/wallet_listtransactions.py

💤 Files with no reviewable changes (1)

• test/functional/test_framework/util.py

🧰 Additional context used

📓 Path-based instructions (8)

src/**/*.{cpp,h,hpp,cc}

📄 CodeRabbit inference engine (CLAUDE.md)

Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1

Files:

• src/wallet/rpc/util.cpp
• src/bench/strencodings.cpp
• src/bench/bip324_ecdh.cpp
• src/bench/coin_selection.cpp
• src/bench/hashpadding.cpp
• src/bench/mempool_stress.cpp
• src/bench/merkle_root.cpp
• src/bench/bls_pubkey_agg.cpp
• src/common/url.h
• src/wallet/scriptpubkeyman.h
• src/bench/pool.cpp
• src/bench/wallet_create.cpp
• src/bench/duplicate_inputs.cpp
• src/bench/rollingbloom.cpp
• src/wallet/rpc/backup.cpp
• src/util/error.cpp
• src/bench/gcs_filter.cpp
• src/bench/load_external.cpp
• src/bench/logging.cpp
• src/bench/checkqueue.cpp
• src/test/fuzz/rpc.cpp
• src/bench/addrman.cpp
• src/bench/rpc_blockchain.cpp
• src/util/error.h
• src/bench/checkblock.cpp
• src/bench/ecdsa.cpp
• src/bench/mempool_eviction.cpp
• src/bench/bench.h
• src/bench/ccoins_caching.cpp
• src/interfaces/chain.h
• src/bench/pow_hash.cpp
• src/bench/wallet_loading.cpp
• src/bench/examples.cpp
• src/bench/bls_dkg.cpp
• src/bench/chacha20.cpp
• src/bench/string_cast.cpp
• src/logging.h
• src/bench/base58.cpp
• src/wallet/rpc/wallet.cpp
• src/bench/bls.cpp
• src/bench/poly1305.cpp
• src/bench/util_time.cpp
• src/bitcoin-wallet.cpp
• src/bench/lockedpool.cpp
• src/wallet/wallet.cpp
• src/logging.cpp
• src/wallet/rpc/transactions.cpp
• src/bench/rpc_mempool.cpp
• src/bench/wallet_balance.cpp
• src/test/fuzz/string.cpp
• src/bitcoin-cli.cpp
• src/node/interfaces.cpp
• src/wallet/scriptpubkeyman.cpp
• src/bench/block_assemble.cpp
• src/bitcoind.cpp
• src/bench/ellswift.cpp
• src/rpc/client.cpp
• src/common/url.cpp
• src/bench/prevector.cpp
• src/bench/verify_script.cpp
• src/bench/crypto_hash.cpp
• src/bench/bech32.cpp
• src/bench/peer_eviction.cpp
• src/rpc/mempool.cpp
• src/qt/main.cpp
• src/test/util/setup_common.cpp

src/wallet/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Wallet implementation must use Berkeley DB and SQLite

Files:

• src/wallet/rpc/util.cpp
• src/wallet/scriptpubkeyman.h
• src/wallet/rpc/backup.cpp
• src/wallet/rpc/wallet.cpp
• src/wallet/wallet.cpp
• src/wallet/rpc/transactions.cpp
• src/wallet/scriptpubkeyman.cpp

src/bench/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Performance benchmarks in src/bench/ must use the nanobench library

Files:

• src/bench/strencodings.cpp
• src/bench/bip324_ecdh.cpp
• src/bench/coin_selection.cpp
• src/bench/hashpadding.cpp
• src/bench/mempool_stress.cpp
• src/bench/merkle_root.cpp
• src/bench/bls_pubkey_agg.cpp
• src/bench/pool.cpp
• src/bench/wallet_create.cpp
• src/bench/duplicate_inputs.cpp
• src/bench/rollingbloom.cpp
• src/bench/gcs_filter.cpp
• src/bench/load_external.cpp
• src/bench/logging.cpp
• src/bench/checkqueue.cpp
• src/bench/addrman.cpp
• src/bench/rpc_blockchain.cpp
• src/bench/checkblock.cpp
• src/bench/ecdsa.cpp
• src/bench/mempool_eviction.cpp
• src/bench/bench.h
• src/bench/ccoins_caching.cpp
• src/bench/pow_hash.cpp
• src/bench/wallet_loading.cpp
• src/bench/examples.cpp
• src/bench/bls_dkg.cpp
• src/bench/chacha20.cpp
• src/bench/string_cast.cpp
• src/bench/base58.cpp
• src/bench/bls.cpp
• src/bench/poly1305.cpp
• src/bench/util_time.cpp
• src/bench/lockedpool.cpp
• src/bench/rpc_mempool.cpp
• src/bench/wallet_balance.cpp
• src/bench/block_assemble.cpp
• src/bench/ellswift.cpp
• src/bench/prevector.cpp
• src/bench/verify_script.cpp
• src/bench/crypto_hash.cpp
• src/bench/bech32.cpp
• src/bench/peer_eviction.cpp

{guix-build*,releases,**/guix-build*,releases/**,.github/**,depends/**,ci/**,contrib/**,doc/**}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not make changes to build system files (guix-build*), release artifacts, or avoid changes to .github, depends, ci, contrib, and doc directories unless specifically prompted

Files:

• ci/test/00_setup_env_mac_native_arm64.sh
• ci/test/04_install.sh
• doc/descriptors.md
• ci/dash/lint-tidy.sh

test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Functional tests in test/functional/ must be written in Python (minimum version specified in .python-version) and depend on dashd and dash-node

Files:

• test/functional/wallet_listtransactions.py
• test/functional/mempool_packages.py
• test/functional/wallet_fast_rescan.py
• test/functional/test_framework/wallet.py
• test/functional/mempool_package_limits.py
• test/functional/rpc_getblockfrompeer.py
• test/functional/rpc_packages.py
• test/functional/wallet_listsinceblock.py
• test/functional/test_runner.py

src/{test,wallet/test}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework

Files:

• src/test/fuzz/rpc.cpp
• src/test/fuzz/string.cpp
• src/test/util/setup_common.cpp

src/node/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

NodeContext must be extended with Dash-specific managers for each subsystem during initialization

Files:

• src/node/interfaces.cpp

src/qt/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

GUI implementation in src/qt/ must use Qt 5

Files:

• src/qt/main.cpp

🧠 Learnings (31)

📓 Common learnings

Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.

Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.

Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.

Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.

Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,txmempool}/**/*.{cpp,h} : Block validation and mempool handling must use extensions to Bitcoin Core mechanisms for special transaction validation and enhanced transaction relay

📚 Learning: 2025-08-08T07:01:47.332Z

Learnt from: knst
Repo: dashpay/dash PR: 6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.332Z
Learning: In src/wallet/rpc/wallet.cpp, the upgradetohd RPC now returns a UniValue string message (RPCResult::Type::STR) instead of a boolean, including guidance about mnemonic backup and null-character passphrase handling; functional tests have been updated to assert returned strings in several cases.

Applied to files:

• src/wallet/rpc/util.cpp
• src/bench/wallet_create.cpp
• src/wallet/rpc/backup.cpp
• src/bench/wallet_loading.cpp
• test/functional/mempool_packages.py
• src/wallet/rpc/wallet.cpp
• src/bitcoin-wallet.cpp
• src/wallet/rpc/transactions.cpp
• src/bitcoin-cli.cpp
• test/functional/rpc_packages.py
• src/rpc/client.cpp
• src/rpc/mempool.cpp

📚 Learning: 2025-10-25T07:08:51.918Z

Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: The univalue library (in src/univalue/) is no longer a vendored external dependency but is now part of the Bitcoin Core codebase and can be modified as needed during backports.

Applied to files:

• src/wallet/rpc/util.cpp
• src/common/url.h
• src/bitcoin-wallet.cpp
• src/bitcoind.cpp

📚 Learning: 2025-02-14T15:19:17.218Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6529
File: src/wallet/rpcwallet.cpp:3002-3003
Timestamp: 2025-02-14T15:19:17.218Z
Learning: The `GetWallet()` function calls in `src/wallet/rpcwallet.cpp` are properly validated with null checks that throw appropriate RPC errors, making additional validation unnecessary.

Applied to files:

• src/wallet/rpc/util.cpp
• src/wallet/rpc/wallet.cpp
• src/wallet/rpc/transactions.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/wallet/**/*.{cpp,h} : Wallet implementation must use Berkeley DB and SQLite

Applied to files:

• src/wallet/rpc/util.cpp
• src/bench/wallet_create.cpp
• src/bench/wallet_loading.cpp
• src/wallet/rpc/wallet.cpp
• src/bitcoin-wallet.cpp
• src/wallet/wallet.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/bench/**/*.{cpp,h} : Performance benchmarks in src/bench/ must use the nanobench library

Applied to files:

• src/bench/strencodings.cpp
• src/bench/coin_selection.cpp
• src/bench/hashpadding.cpp
• src/bench/mempool_stress.cpp
• src/bench/bls_pubkey_agg.cpp
• src/bench/pool.cpp
• src/bench/rollingbloom.cpp
• src/bench/gcs_filter.cpp
• src/bench/load_external.cpp
• src/bench/logging.cpp
• src/bench/checkqueue.cpp
• src/bench/addrman.cpp
• src/bench/rpc_blockchain.cpp
• src/bench/checkblock.cpp
• src/bench/ecdsa.cpp
• src/bench/mempool_eviction.cpp
• src/bench/bench.h
• src/bench/ccoins_caching.cpp
• src/bench/pow_hash.cpp
• src/bench/wallet_loading.cpp
• src/bench/examples.cpp
• src/bench/bls_dkg.cpp
• src/bench/chacha20.cpp
• src/bench/string_cast.cpp
• src/bench/base58.cpp
• src/bench/poly1305.cpp
• src/bench/util_time.cpp
• src/bench/lockedpool.cpp
• src/bench/wallet_balance.cpp
• src/bench/block_assemble.cpp
• src/bench/prevector.cpp
• src/bench/verify_script.cpp
• src/bench/crypto_hash.cpp
• src/bench/bech32.cpp
• src/bench/peer_eviction.cpp

📚 Learning: 2025-01-02T08:33:26.751Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6516
File: ci/test/00_setup_env_mac.sh:11-11
Timestamp: 2025-01-02T08:33:26.751Z
Learning: The removal of DMG support in the macOS packaging process eliminates the need for python-based scripts or python3-setuptools in the build environment. The PACKAGES variable in ci/test/00_setup_env_mac.sh is vestigial due to the use of a general-purpose Docker container defined in contrib/containers/ci/Dockerfile.

Applied to files:

• ci/test/00_setup_env_mac_native_arm64.sh
• ci/test/04_install.sh

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs

Applied to files:

• src/bench/coin_selection.cpp
• src/wallet/wallet.cpp
• src/node/interfaces.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features

Applied to files:

• src/bench/bls_pubkey_agg.cpp
• src/wallet/rpc/wallet.cpp
• src/bench/bls.cpp
• src/node/interfaces.cpp
• src/rpc/mempool.cpp
• src/test/util/setup_common.cpp

📚 Learning: 2025-12-22T15:42:48.595Z

Learnt from: kwvg
Repo: dashpay/dash PR: 7068
File: src/qt/guiutil_font.h:68-77
Timestamp: 2025-12-22T15:42:48.595Z
Learning: In C++/Qt codebases, use fail-fast asserts in setters to enforce invariants (e.g., ensuring internal maps contain necessary keys). Prefer assert() for programmer errors that should be caught in development and debugging, rather than defensive runtime checks with fallbacks. This helps catch invariant violations early during development. Apply to header and source files where invariant-driven state mutations occur, especially in setters like SetWeightBold/SetWeightNormal that assume established relationships or preconditions.

Applied to files:

• src/common/url.h
• src/wallet/scriptpubkeyman.h
• src/util/error.h
• src/bench/bench.h
• src/interfaces/chain.h
• src/logging.h

📚 Learning: 2025-12-17T13:58:26.891Z

Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:26.891Z
Learning: In Dash Qt wallet code, when leveraging upstream Bitcoin Core wallet capabilities (especially for operations like rescanning), prefer using the inherited signal-based mechanisms (e.g., ShowProgress signals) over adding explicit Qt progress dialogs to minimize upstream deviation and simplify future backports.

Applied to files:

• src/wallet/rpc/backup.cpp
• src/wallet/wallet.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,txmempool}/**/*.{cpp,h} : Block validation and mempool handling must use extensions to Bitcoin Core mechanisms for special transaction validation and enhanced transaction relay

Applied to files:

• src/bench/rpc_blockchain.cpp
• src/bench/checkblock.cpp
• src/Makefile.am
• src/interfaces/chain.h
• test/functional/mempool_packages.py
• src/wallet/rpc/wallet.cpp
• src/bitcoin-wallet.cpp
• src/wallet/wallet.cpp
• src/wallet/rpc/transactions.cpp
• src/bench/rpc_mempool.cpp
• src/bitcoin-cli.cpp
• src/node/interfaces.cpp
• ci/dash/lint-tidy.sh
• src/rpc/mempool.cpp

📚 Learning: 2025-09-02T07:34:28.226Z

Learnt from: knst
Repo: dashpay/dash PR: 6834
File: test/functional/wallet_mnemonicbits.py:50-51
Timestamp: 2025-09-02T07:34:28.226Z
Learning: CJ (CoinJoin) descriptors with derivation path "9'/1" are intentionally inactive in descriptor wallets, while regular internal/external descriptors with different derivation paths remain active.

Applied to files:

• doc/descriptors.md

📚 Learning: 2025-07-23T09:30:34.631Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.h:5-6
Timestamp: 2025-07-23T09:30:34.631Z
Learning: Dash Core uses BITCOIN_ prefix for header guards as the standard convention, inherited from Bitcoin Core. Only a few BLS-specific files in src/bls/ use DASH_ prefix. The vast majority of files (385+) use BITCOIN_ prefix.

Applied to files:

• src/Makefile.am
• src/logging.h
• src/bench/crypto_hash.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Dash extends Bitcoin Core through composition with minimal changes to the Bitcoin Core foundation

Applied to files:

• src/Makefile.am

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention

Applied to files:

• src/interfaces/chain.h
• src/rpc/mempool.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,consensus,net_processing}/**/*.{cpp,h} : ValidationInterface callbacks must be used for event-driven architecture to coordinate subsystems during block/transaction processing

Applied to files:

• src/interfaces/chain.h
• src/node/interfaces.cpp
• src/rpc/mempool.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files

Applied to files:

• src/interfaces/chain.h
• src/bitcoin-wallet.cpp
• src/bitcoind.cpp
• src/rpc/mempool.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h} : Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data

Applied to files:

• src/bench/wallet_loading.cpp
• src/node/interfaces.cpp
• src/bench/crypto_hash.cpp

📚 Learning: 2025-06-09T16:43:20.996Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

Applied to files:

• test/functional/mempool_packages.py
• test/functional/mempool_package_limits.py
• test/functional/rpc_packages.py

📚 Learning: 2025-06-20T23:32:16.225Z

Learnt from: kwvg
Repo: dashpay/dash PR: 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.

Applied to files:

• test/functional/mempool_packages.py

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)

Applied to files:

• src/bench/bls_dkg.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus

Applied to files:

• src/bench/bls_dkg.cpp

📚 Learning: 2025-11-04T18:23:28.175Z

Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/governance/classes.cpp:147-154
Timestamp: 2025-11-04T18:23:28.175Z
Learning: In src/governance/classes.cpp, CSuperblock::GetPaymentsLimit intentionally uses synthetic difficulty constants (nBits = 1 for mainnet, powLimit for networks with min difficulty) and simple height-based V20 activation checks instead of actual chain block data. This is by design because superblocks themselves are "synthetic" governance payment blocks, not regular mined blocks.

Applied to files:

• src/wallet/wallet.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/governance/**/*.{cpp,h} : Governance implementation must support governance objects (proposals, triggers, superblock management) and on-chain voting with tallying

Applied to files:

• src/node/interfaces.cpp

📚 Learning: 2025-12-01T18:13:21.314Z

Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 7018
File: test/lint/lint-github-actions.py:1-9
Timestamp: 2025-12-01T18:13:21.314Z
Learning: In the Dash repository, the file test/util/data/non-backported.txt should only include C++ files (.cpp or .h) because it is used for running clang-format. Other file types (such as Python files, .ui files, etc.) should not be added to this list.

Applied to files:

• ci/dash/lint-tidy.sh

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/**/*.{cpp,h,hpp,cc} : Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1

Applied to files:

• ci/dash/lint-tidy.sh

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{test,wallet/test}/**/*.{cpp,h} : Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework

Applied to files:

• ci/dash/lint-tidy.sh

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h} : Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures

Applied to files:

• src/bench/crypto_hash.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/qt/**/*.{cpp,h} : GUI implementation in src/qt/ must use Qt 5

Applied to files:

• src/qt/main.cpp

📚 Learning: 2025-12-17T13:58:19.813Z

Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:19.813Z
Learning: In Dash Qt wallet code, prefer using existing Qt signals/slots (e.g., ShowProgress signals) for long-running operations instead of adding explicit QProgressDialog dialogs. This minimizes upstream deviations and simplifies backports. Apply this pattern to wallet-related Qt components (e.g., operations like rescanning) and rely on signal-based progress updates rather than bespoke dialogs.

Applied to files:

• src/qt/main.cpp

🧬 Code graph analysis (8)

src/common/url.h (1)

src/common/url.cpp (2)

• urlDecode (12-22)
• urlDecode (12-12)

src/wallet/scriptpubkeyman.h (1)

src/wallet/scriptpubkeyman.cpp (6)

• GetScriptPubKeys (2516-2519)
• GetScriptPubKeys (2516-2516)
• GetScriptPubKeys (2521-2531)
• GetScriptPubKeys (2521-2521)
• GetEndRange (2533-2536)
• GetEndRange (2533-2533)

src/interfaces/chain.h (2)

src/node/interfaces.cpp (14)

• filter_type (1072-1075)
• filter_type (1072-1072)
• filter_type (1076-1085)
• filter_type (1076-1076)
• block_hash (1056-1062)
• block_hash (1056-1056)
• block_hash (1124-1134)
• block_hash (1124-1124)
• block_hash (1135-1143)
• block_hash (1135-1135)
• block_hash (1159-1163)
• block_hash (1159-1159)
• block_hash (1164-1182)
• block_hash (1164-1164)

src/wallet/wallet.cpp (2)

• block_hash (319-322)
• block_hash (319-319)

test/functional/mempool_packages.py (3)

test/functional/test_framework/test_framework.py (1)

• is_specified_wallet_compiled (1114-1120)

test/functional/test_framework/p2p.py (1)

• wait_for_broadcast (1019-1026)

test/functional/test_framework/messages.py (1)

• get_vsize (593-594)

test/functional/test_framework/wallet.py (1)

test/functional/test_framework/util.py (2)

• assert_greater_than_or_equal (82-84)
• assert_equal (69-74)

test/functional/mempool_package_limits.py (1)

test/functional/test_framework/wallet.py (6)

• MiniWallet (74-366)
• send_self_transfer_chain (357-366)
• create_self_transfer (315-335)
• create_self_transfer_multi (260-313)
• send_self_transfer (226-230)
• sendrawtransaction (337-340)

test/functional/rpc_packages.py (5)

test/functional/test_framework/messages.py (1)

• tx_from_hex (230-232)

test/functional/test_framework/p2p.py (2)

• P2PTxInvStore (1001-1026)
• wait_for_broadcast (1019-1026)

test/functional/test_framework/util.py (2)

• assert_fee_amount (44-52)
• assert_raises_rpc_error (132-148)

test/functional/test_framework/wallet.py (5)

• MiniWallet (74-366)
• create_self_transfer (315-335)
• create_self_transfer_chain (342-355)
• create_self_transfer_multi (260-313)
• get_utxo (192-212)

test/functional/test_framework/test_node.py (1)

• get_deterministic_priv_key (191-194)

src/rpc/mempool.cpp (2)

src/wallet/rpc/spend.cpp (1)

• tx (983-983)

src/core_write.cpp (2)

• ValueFromAmount (36-47)
• ValueFromAmount (36-36)

🪛 Cppcheck (2.19.0)

src/bench/bls_pubkey_agg.cpp

[error] 95-95: failed to evaluate #if condition, undefined function-like macro invocation

(syntaxError)

src/node/interfaces.cpp

[error] 22-22: failed to evaluate #if condition, undefined function-like macro invocation

(syntaxError)

src/bench/peer_eviction.cpp

[error] 148-148: Buffer is accessed out of bounds

(bufferAccessOutOfBounds)

src/test/util/setup_common.cpp

[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation

(syntaxError)

🪛 Flake8 (7.3.0)

test/functional/wallet_fast_rescan.py

[error] 52-52: f-string is missing placeholders

(F541)

test/functional/rpc_packages.py

[error] 51-51: continuation line over-indented for hanging indent

(E126)

---

[error] 56-56: continuation line under-indented for hanging indent

(E121)

---

[error] 101-101: continuation line under-indented for visual indent

(E128)

🪛 Ruff (0.14.10)

test/functional/wallet_listtransactions.py

177-177: Prefer next(...) over single element slice

Replace with next(...)

(RUF015)

test/functional/wallet_fast_rescan.py

52-52: f-string without any placeholders

Remove extraneous f prefix

(F541)

test/functional/test_framework/wallet.py

290-290: zip() without an explicit strict= parameter

Add explicit value for parameter strict=

(B905)

test/functional/rpc_packages.py

120-120: Consider [tx_high_fee["hex"], *self.independent_txns_hex] instead of concatenation

Replace with [tx_high_fee["hex"], *self.independent_txns_hex]

(RUF005)

---

197-197: Consider [*package_hex, child_tx['hex']] instead of concatenation

Replace with [*package_hex, child_tx['hex']]

(RUF005)

---

202-202: Consider [*package_hex, child_tx["hex"]] instead of concatenation

Replace with [*package_hex, child_tx["hex"]]

(RUF005)

---

245-245: Unnecessary key check before dictionary access

Replace with dict.get

(RUF019)

---

260-260: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)

test/functional/wallet_listsinceblock.py

413-413: Prefer next(...) over single element slice

Replace with next(...)

(RUF015)

⏰ 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: linux64_sqlite-build / Build source
• GitHub Check: linux64_fuzz-build / Build source
• GitHub Check: linux64-build / Build source
• GitHub Check: linux64_ubsan-build / Build source
• GitHub Check: mac-build / Build source
• GitHub Check: win64-build / Build source
• GitHub Check: arm-linux-build / Build source
• GitHub Check: linux64_nowallet-build / Build source

[coderabbitai]

Actionable comments posted: 6

🤖 Fix all issues with AI Agents

In @src/bench/bls_pubkey_agg.cpp:
- Around line 90-99: Each BENCHMARK macro invocation in this snippet is missing
the trailing semicolon; update every BENCHMARK line (e.g.,
BENCHMARK(BLS_PubKeyAggregate_Iterative_5, benchmark::PriorityLevel::HIGH),
BENCHMARK(BLS_PubKeyAggregate_Batch_5, ...),
BENCHMARK(BLS_PubKeyAggregate_Iterative_25, ...), etc.) to end with a semicolon
so the macros are treated as statements and match the style used elsewhere.

In @src/rpc/mempool.cpp:
- Around line 861-870: The loop declares num_submitted inside the for-loop but
never increments it, causing the error message to incorrectly report "0
transactions were broadcast successfully"; either move size_t num_submitted out
before the for-loop and increment it after each successful call to
BroadcastTransaction(node, tx, ...) (checking err == TransactionError::OK) so
the JSONRPCTransactionError message accurately reports the number of successful
broadcasts, or remove num_submitted entirely and change the thrown
JSONRPCTransactionError to a simpler message that omits the success count;
update references to num_submitted and the error string accordingly
(functions/idents: BroadcastTransaction, TransactionError::OK,
JSONRPCTransactionError, num_submitted, txns).

In @test/functional/mempool_packages.py:
- Around line 231-239: The loop that builds chain/tx_children compares
utxo['txid'] to parent_transaction using identity (`is`), which is incorrect for
string values; update the comparison in the for-loop so that utxo['txid'] ==
parent_transaction is used (ensuring string equality) so tx_children is
populated correctly when txids match; locate the comparison involving
utxo['txid'], parent_transaction and tx_children in the transaction-package loop
and replace the identity check with an equality check.

In @test/functional/rpc_packages.py:
- Around line 306-323: The child transaction lookup and broadcast checks use
integer .hash values instead of txid strings; update the child_result lookup to
use tx_child["txid"] when indexing submitpackage_result["tx-results"] (matching
rich_parent_result/poor_parent_result usage) and pass the list of txid strings
(e.g., [tx["txid"] for tx in package_txns]) to peer.wait_for_broadcast so it
expects txid strings rather than integer hashes.
- Around line 268-293: The code incorrectly uses the internal integer tx.hash
for lookups and broadcast waits; change the membership check, lookup key, and
the list passed to peer.wait_for_broadcast to use the txid hex string (e.g.
package_txn["txid"] or tx.get_txid()/txid string from the package_txn dict)
instead of tx.hash or integer hashes so submitpackage_result["tx-results"]
lookups succeed and P2PTxInvStore.wait_for_broadcast receives hex txid strings.

In @test/functional/wallet_fast_rescan.py:
- Line 52: The log call using self.log.info currently uses an unnecessary
f-string with no placeholders; update the string in the self.log.info(...) call
in test/functional/wallet_fast_rescan.py (the line calling "Create txs sending
to end range address of each descriptor, triggering top-ups") by removing the
leading f so it becomes a plain string literal.

🧹 Nitpick comments (3)

test/functional/wallet_listsinceblock.py (1)

44-44: LGTM! OP_RETURN test coverage is valuable.

The new test_op_return method correctly verifies that OP_RETURN transactions in listsinceblock responses do not contain an address field, ensuring proper handling of data-only outputs.

Optional: Consider using next() for safer list access

Line 413 uses a list comprehension with [0] indexing which could raise IndexError if the transaction is not found. The next() function with a default or explicit check would be more Pythonic:

-        op_ret_tx = [tx for tx in self.nodes[2].listsinceblock(blockhash=block_hash)["transactions"] if tx['txid'] == tx_id][0]
+        op_ret_tx = next((tx for tx in self.nodes[2].listsinceblock(blockhash=block_hash)["transactions"] if tx['txid'] == tx_id), None)
+        assert op_ret_tx is not None, f"Transaction {tx_id} not found in listsinceblock"

Also applies to: 404-416

test/functional/wallet_listtransactions.py (1)

170-179: Consider using next() for safer iteration.

The test logic correctly verifies that OP_RETURN outputs don't have an address field. However, Line 177 uses list comprehension with indexing that could raise IndexError if no matching transaction is found.

🔎 Suggested improvement using next()

-op_ret_tx = [tx for tx in self.nodes[0].listtransactions() if tx['txid'] == tx_id][0]
+op_ret_tx = next(tx for tx in self.nodes[0].listtransactions() if tx['txid'] == tx_id)

This provides clearer intent and will raise StopIteration (which can be caught or defaulted) rather than IndexError if the transaction isn't found.

test/functional/wallet_fast_rescan.py (1)

47-47: Replace print() with self.log.debug() or self.log.info()

Using print() is inconsistent with the test framework's logging conventions used elsewhere in this file (e.g., self.log.info() on lines 42, 52, 54, etc.).

Proposed fix

-        print(w.importdescriptors([{"desc": descsum_create(f"wpkh({fixed_key.privkey})"), "timestamp": "now"}]))
+        self.log.debug(w.importdescriptors([{"desc": descsum_create(f"wpkh({fixed_key.privkey})"), "timestamp": "now"}]))

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b21e0a6 and 50c42a2.

📒 Files selected for processing (82)

• ci/dash/lint-tidy.sh
• ci/test/00_setup_env_mac_native_arm64.sh
• ci/test/04_install.sh
• doc/descriptors.md
• doc/release-notes-25957.md
• src/Makefile.am
• src/bench/addrman.cpp
• src/bench/base58.cpp
• src/bench/bech32.cpp
• src/bench/bench.h
• src/bench/bip324_ecdh.cpp
• src/bench/block_assemble.cpp
• src/bench/bls.cpp
• src/bench/bls_dkg.cpp
• src/bench/bls_pubkey_agg.cpp
• src/bench/ccoins_caching.cpp
• src/bench/chacha20.cpp
• src/bench/checkblock.cpp
• src/bench/checkqueue.cpp
• src/bench/coin_selection.cpp
• src/bench/crypto_hash.cpp
• src/bench/duplicate_inputs.cpp
• src/bench/ecdsa.cpp
• src/bench/ellswift.cpp
• src/bench/examples.cpp
• src/bench/gcs_filter.cpp
• src/bench/hashpadding.cpp
• src/bench/load_external.cpp
• src/bench/lockedpool.cpp
• src/bench/logging.cpp
• src/bench/mempool_eviction.cpp
• src/bench/mempool_stress.cpp
• src/bench/merkle_root.cpp
• src/bench/peer_eviction.cpp
• src/bench/poly1305.cpp
• src/bench/pool.cpp
• src/bench/pow_hash.cpp
• src/bench/prevector.cpp
• src/bench/rollingbloom.cpp
• src/bench/rpc_blockchain.cpp
• src/bench/rpc_mempool.cpp
• src/bench/strencodings.cpp
• src/bench/string_cast.cpp
• src/bench/util_time.cpp
• src/bench/verify_script.cpp
• src/bench/wallet_balance.cpp
• src/bench/wallet_create.cpp
• src/bench/wallet_loading.cpp
• src/bitcoin-cli.cpp
• src/bitcoin-wallet.cpp
• src/bitcoind.cpp
• src/common/url.cpp
• src/common/url.h
• src/interfaces/chain.h
• src/logging.cpp
• src/logging.h
• src/node/interfaces.cpp
• src/qt/main.cpp
• src/rpc/client.cpp
• src/rpc/mempool.cpp
• src/test/fuzz/rpc.cpp
• src/test/fuzz/string.cpp
• src/test/util/setup_common.cpp
• src/util/error.cpp
• src/util/error.h
• src/wallet/rpc/backup.cpp
• src/wallet/rpc/transactions.cpp
• src/wallet/rpc/util.cpp
• src/wallet/rpc/wallet.cpp
• src/wallet/scriptpubkeyman.cpp
• src/wallet/scriptpubkeyman.h
• src/wallet/wallet.cpp
• test/functional/mempool_package_limits.py
• test/functional/mempool_packages.py
• test/functional/rpc_getblockfrompeer.py
• test/functional/rpc_packages.py
• test/functional/test_framework/util.py
• test/functional/test_framework/wallet.py
• test/functional/test_runner.py
• test/functional/wallet_fast_rescan.py
• test/functional/wallet_listsinceblock.py
• test/functional/wallet_listtransactions.py

💤 Files with no reviewable changes (1)

• test/functional/test_framework/util.py

✅ Files skipped from review due to trivial changes (1)

• doc/release-notes-25957.md

🚧 Files skipped from review as they are similar to previous changes (41)

• src/logging.h
• src/qt/main.cpp
• src/bench/util_time.cpp
• src/bench/bip324_ecdh.cpp
• src/bench/duplicate_inputs.cpp
• src/bench/ellswift.cpp
• src/wallet/rpc/util.cpp
• src/util/error.cpp
• src/wallet/rpc/wallet.cpp
• src/bench/strencodings.cpp
• src/bench/poly1305.cpp
• src/bitcoin-cli.cpp
• src/bench/rpc_blockchain.cpp
• src/bitcoin-wallet.cpp
• ci/test/04_install.sh
• src/bench/verify_script.cpp
• src/bench/rpc_mempool.cpp
• src/bitcoind.cpp
• src/bench/chacha20.cpp
• src/wallet/rpc/backup.cpp
• src/bench/pow_hash.cpp
• src/bench/mempool_eviction.cpp
• src/bench/block_assemble.cpp
• src/bench/load_external.cpp
• doc/descriptors.md
• src/bench/bls.cpp
• ci/test/00_setup_env_mac_native_arm64.sh
• src/wallet/wallet.cpp
• src/bench/merkle_root.cpp
• src/bench/prevector.cpp
• src/bench/string_cast.cpp
• src/bench/gcs_filter.cpp
• src/bench/bls_dkg.cpp
• src/bench/wallet_balance.cpp
• src/wallet/scriptpubkeyman.cpp
• src/bench/crypto_hash.cpp
• ci/dash/lint-tidy.sh
• src/test/fuzz/rpc.cpp
• src/wallet/rpc/transactions.cpp
• src/test/fuzz/string.cpp
• src/common/url.cpp

🧰 Additional context used

📓 Path-based instructions (6)

src/**/*.{cpp,h,hpp,cc}

📄 CodeRabbit inference engine (CLAUDE.md)

Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1

Files:

• src/bench/bech32.cpp
• src/bench/mempool_stress.cpp
• src/bench/ecdsa.cpp
• src/bench/base58.cpp
• src/bench/hashpadding.cpp
• src/bench/coin_selection.cpp
• src/bench/addrman.cpp
• src/wallet/scriptpubkeyman.h
• src/bench/pool.cpp
• src/interfaces/chain.h
• src/bench/wallet_loading.cpp
• src/bench/ccoins_caching.cpp
• src/bench/checkqueue.cpp
• src/bench/checkblock.cpp
• src/util/error.h
• src/bench/rollingbloom.cpp
• src/logging.cpp
• src/bench/lockedpool.cpp
• src/bench/bench.h
• src/common/url.h
• src/bench/examples.cpp
• src/rpc/mempool.cpp
• src/bench/wallet_create.cpp
• src/rpc/client.cpp
• src/bench/logging.cpp
• src/bench/bls_pubkey_agg.cpp
• src/bench/peer_eviction.cpp
• src/node/interfaces.cpp
• src/test/util/setup_common.cpp

src/bench/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Performance benchmarks in src/bench/ must use the nanobench library

Files:

• src/bench/bech32.cpp
• src/bench/mempool_stress.cpp
• src/bench/ecdsa.cpp
• src/bench/base58.cpp
• src/bench/hashpadding.cpp
• src/bench/coin_selection.cpp
• src/bench/addrman.cpp
• src/bench/pool.cpp
• src/bench/wallet_loading.cpp
• src/bench/ccoins_caching.cpp
• src/bench/checkqueue.cpp
• src/bench/checkblock.cpp
• src/bench/rollingbloom.cpp
• src/bench/lockedpool.cpp
• src/bench/bench.h
• src/bench/examples.cpp
• src/bench/wallet_create.cpp
• src/bench/logging.cpp
• src/bench/bls_pubkey_agg.cpp
• src/bench/peer_eviction.cpp

src/wallet/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Wallet implementation must use Berkeley DB and SQLite

Files:

• src/wallet/scriptpubkeyman.h

test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Functional tests in test/functional/ must be written in Python (minimum version specified in .python-version) and depend on dashd and dash-node

Files:

• test/functional/rpc_packages.py
• test/functional/mempool_packages.py
• test/functional/wallet_listsinceblock.py
• test/functional/wallet_listtransactions.py
• test/functional/wallet_fast_rescan.py
• test/functional/rpc_getblockfrompeer.py
• test/functional/test_runner.py
• test/functional/test_framework/wallet.py
• test/functional/mempool_package_limits.py

src/node/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

NodeContext must be extended with Dash-specific managers for each subsystem during initialization

Files:

• src/node/interfaces.cpp

src/{test,wallet/test}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework

Files:

• src/test/util/setup_common.cpp

🧠 Learnings (19)

📓 Common learnings

Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.

Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.

Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.

Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.

Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.

Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,txmempool}/**/*.{cpp,h} : Block validation and mempool handling must use extensions to Bitcoin Core mechanisms for special transaction validation and enhanced transaction relay

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/bench/**/*.{cpp,h} : Performance benchmarks in src/bench/ must use the nanobench library

Applied to files:

• src/bench/bech32.cpp
• src/bench/mempool_stress.cpp
• src/bench/ecdsa.cpp
• src/bench/base58.cpp
• src/bench/hashpadding.cpp
• src/bench/coin_selection.cpp
• src/bench/addrman.cpp
• src/bench/pool.cpp
• src/bench/wallet_loading.cpp
• src/bench/ccoins_caching.cpp
• src/bench/checkqueue.cpp
• src/bench/checkblock.cpp
• src/bench/rollingbloom.cpp
• src/bench/lockedpool.cpp
• src/bench/bench.h
• src/bench/examples.cpp
• src/bench/logging.cpp
• src/bench/bls_pubkey_agg.cpp
• src/bench/peer_eviction.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs

Applied to files:

• src/bench/coin_selection.cpp
• src/node/interfaces.cpp

📚 Learning: 2025-12-22T15:42:48.595Z

Learnt from: kwvg
Repo: dashpay/dash PR: 7068
File: src/qt/guiutil_font.h:68-77
Timestamp: 2025-12-22T15:42:48.595Z
Learning: In C++/Qt codebases, use fail-fast asserts in setters to enforce invariants (e.g., ensuring internal maps contain necessary keys). Prefer assert() for programmer errors that should be caught in development and debugging, rather than defensive runtime checks with fallbacks. This helps catch invariant violations early during development. Apply to header and source files where invariant-driven state mutations occur, especially in setters like SetWeightBold/SetWeightNormal that assume established relationships or preconditions.

Applied to files:

• src/wallet/scriptpubkeyman.h
• src/interfaces/chain.h
• src/util/error.h
• src/bench/bench.h
• src/common/url.h

📚 Learning: 2025-06-09T16:43:20.996Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

Applied to files:

• test/functional/rpc_packages.py
• test/functional/mempool_packages.py
• test/functional/mempool_package_limits.py

📚 Learning: 2025-08-08T07:01:47.332Z

Learnt from: knst
Repo: dashpay/dash PR: 6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.332Z
Learning: In src/wallet/rpc/wallet.cpp, the upgradetohd RPC now returns a UniValue string message (RPCResult::Type::STR) instead of a boolean, including guidance about mnemonic backup and null-character passphrase handling; functional tests have been updated to assert returned strings in several cases.

Applied to files:

• test/functional/rpc_packages.py
• src/bench/wallet_loading.cpp
• test/functional/mempool_packages.py
• src/rpc/mempool.cpp
• src/rpc/client.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,txmempool}/**/*.{cpp,h} : Block validation and mempool handling must use extensions to Bitcoin Core mechanisms for special transaction validation and enhanced transaction relay

Applied to files:

• src/interfaces/chain.h
• test/functional/mempool_packages.py
• src/bench/checkblock.cpp
• src/Makefile.am
• src/rpc/mempool.cpp
• src/node/interfaces.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,consensus,net_processing}/**/*.{cpp,h} : ValidationInterface callbacks must be used for event-driven architecture to coordinate subsystems during block/transaction processing

Applied to files:

• src/interfaces/chain.h
• src/rpc/mempool.cpp
• src/node/interfaces.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files

Applied to files:

• src/interfaces/chain.h
• src/rpc/mempool.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/wallet/**/*.{cpp,h} : Wallet implementation must use Berkeley DB and SQLite

Applied to files:

• src/bench/wallet_loading.cpp
• src/bench/wallet_create.cpp

📚 Learning: 2025-02-06T14:34:30.466Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.

Applied to files:

• test/functional/mempool_packages.py

📚 Learning: 2025-06-20T23:32:16.225Z

Learnt from: kwvg
Repo: dashpay/dash PR: 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.

Applied to files:

• test/functional/mempool_packages.py

📚 Learning: 2025-07-23T09:30:34.631Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.h:5-6
Timestamp: 2025-07-23T09:30:34.631Z
Learning: Dash Core uses BITCOIN_ prefix for header guards as the standard convention, inherited from Bitcoin Core. Only a few BLS-specific files in src/bls/ use DASH_ prefix. The vast majority of files (385+) use BITCOIN_ prefix.

Applied to files:

• src/Makefile.am

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Dash extends Bitcoin Core through composition with minimal changes to the Bitcoin Core foundation

Applied to files:

• src/Makefile.am

📚 Learning: 2025-10-25T07:08:51.918Z

Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: The univalue library (in src/univalue/) is no longer a vendored external dependency but is now part of the Bitcoin Core codebase and can be modified as needed during backports.

Applied to files:

• src/common/url.h

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features

Applied to files:

• src/rpc/mempool.cpp
• src/bench/bls_pubkey_agg.cpp
• src/node/interfaces.cpp
• src/test/util/setup_common.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention

Applied to files:

• src/rpc/mempool.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/governance/**/*.{cpp,h} : Governance implementation must support governance objects (proposals, triggers, superblock management) and on-chain voting with tallying

Applied to files:

• src/node/interfaces.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h} : Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data

Applied to files:

• src/node/interfaces.cpp

🧬 Code graph analysis (6)

src/wallet/scriptpubkeyman.h (1)

src/wallet/scriptpubkeyman.cpp (6)

• GetScriptPubKeys (2516-2519)
• GetScriptPubKeys (2516-2516)
• GetScriptPubKeys (2521-2531)
• GetScriptPubKeys (2521-2521)
• GetEndRange (2533-2536)
• GetEndRange (2533-2533)

src/interfaces/chain.h (2)

src/node/interfaces.cpp (14)

• filter_type (1072-1075)
• filter_type (1072-1072)
• filter_type (1076-1085)
• filter_type (1076-1076)
• block_hash (1056-1062)
• block_hash (1056-1056)
• block_hash (1124-1134)
• block_hash (1124-1124)
• block_hash (1135-1143)
• block_hash (1135-1135)
• block_hash (1159-1163)
• block_hash (1159-1159)
• block_hash (1164-1182)
• block_hash (1164-1164)

src/wallet/wallet.cpp (2)

• block_hash (319-322)
• block_hash (319-319)

test/functional/rpc_getblockfrompeer.py (2)

test/functional/test_framework/p2p.py (1)

• send_and_ping (750-752)

test/functional/test_framework/messages.py (1)

• msg_headers (2050-2068)

src/common/url.h (1)

src/common/url.cpp (2)

• urlDecode (12-22)
• urlDecode (12-12)

test/functional/test_framework/wallet.py (1)

test/functional/test_framework/util.py (2)

• assert_greater_than_or_equal (82-84)
• assert_equal (69-74)

src/node/interfaces.cpp (2)

src/wallet/wallet.cpp (2)

• block_hash (319-322)
• block_hash (319-319)

src/index/blockfilterindex.cpp (1)

• filter (274-274)

🪛 Cppcheck (2.19.0)

src/bench/bls_pubkey_agg.cpp

[error] 95-95: failed to evaluate #if condition, undefined function-like macro invocation

(syntaxError)

src/bench/peer_eviction.cpp

[error] 148-148: Buffer is accessed out of bounds

(bufferAccessOutOfBounds)

src/node/interfaces.cpp

[error] 22-22: failed to evaluate #if condition, undefined function-like macro invocation

(syntaxError)

src/test/util/setup_common.cpp

[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation

(syntaxError)

🪛 Flake8 (7.3.0)

test/functional/rpc_packages.py

[error] 51-51: continuation line over-indented for hanging indent

(E126)

---

[error] 56-56: continuation line under-indented for hanging indent

(E121)

---

[error] 101-101: continuation line under-indented for visual indent

(E128)

test/functional/wallet_fast_rescan.py

[error] 52-52: f-string is missing placeholders

(F541)

🪛 Ruff (0.14.10)

test/functional/rpc_packages.py

120-120: Consider [tx_high_fee["hex"], *self.independent_txns_hex] instead of concatenation

Replace with [tx_high_fee["hex"], *self.independent_txns_hex]

(RUF005)

---

197-197: Consider [*package_hex, child_tx['hex']] instead of concatenation

Replace with [*package_hex, child_tx['hex']]

(RUF005)

---

202-202: Consider [*package_hex, child_tx["hex"]] instead of concatenation

Replace with [*package_hex, child_tx["hex"]]

(RUF005)

---

245-245: Unnecessary key check before dictionary access

Replace with dict.get

(RUF019)

---

260-260: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)

test/functional/wallet_listsinceblock.py

413-413: Prefer next(...) over single element slice

Replace with next(...)

(RUF015)

test/functional/wallet_listtransactions.py

177-177: Prefer next(...) over single element slice

Replace with next(...)

(RUF015)

test/functional/wallet_fast_rescan.py

52-52: f-string without any placeholders

Remove extraneous f prefix

(F541)

test/functional/test_framework/wallet.py

290-290: zip() without an explicit strict= parameter

Add explicit value for parameter strict=

(B905)

🔇 Additional comments (46)

test/functional/rpc_getblockfrompeer.py (1)

95-95: Excellent fix for the race condition.

Replacing send_message with send_and_ping ensures the header is fully processed by node 1 before the test proceeds to call getblockfrompeer on line 104. This eliminates the intermittent timing-dependent failure where the RPC could execute before header processing completed.

src/wallet/scriptpubkeyman.h (1)

644-645: LGTM! Clean API additions for fast wallet rescan.

The new methods correctly extend the DescriptorScriptPubKeyMan API to support index-aware scriptPubKey retrieval and range queries, which are essential for the BIP157 block filter-based fast rescan feature. The implementations (verified in the .cpp file) properly use LOCK(cs_desc_man) for thread safety.

src/logging.cpp (2)

182-182: LGTM: SCAN log category descriptor added correctly.

The new log category descriptor follows the established pattern and is correctly positioned before the ALL entries.

---

298-299: LGTM: LogCategoryToStr case added correctly.

The switch case for SCAN is properly implemented and consistent with the LogCategories array entry, ensuring correct bidirectional conversion between the log flag and its string representation.

src/common/url.h (1)

5-6: LGTM! Header guard correctly updated for file relocation.

The header guard macro renaming from BITCOIN_UTIL_URL_H to BITCOIN_COMMON_URL_H correctly reflects the file's new location in the common/ directory.

Also applies to: 14-14

src/test/util/setup_common.cpp (1)

10-10: LGTM! Include path correctly updated.

The header include path is properly updated to reflect the relocation of URL utilities from util/url.h to common/url.h.

src/Makefile.am (3)

190-190: LGTM! Core header list correctly updated.

The addition of common/url.h to BITCOIN_CORE_H is appropriate for the URL utilities relocation from util/ to common/.

---

960-963: LGTM! Conditional compilation correctly configured.

The conditional inclusion of common/url.cpp under USE_LIBEVENT is correct since urlDecode uses evhttp_uridecode from libevent. The EVENT_CFLAGS addition ensures proper header visibility.

---

1084-1084: LGTM! Linkage dependency correctly added.

Adding LIBBITCOIN_COMMON to dash_cli_LDADD is necessary because dash-cli now depends on URL utilities from common/url.h which are compiled into libbitcoin_common.

src/interfaces/chain.h (1)

8-8: LGTM! Well-designed interface additions for block filter support.

The new interface methods are cleanly designed:

• hasBlockFilterIndex provides a simple availability check
• blockFilterMatchesAny appropriately uses std::optional<bool> to distinguish between "filter unavailable" (std::nullopt) and "no match" (false)

The documentation clearly explains the purpose and behavior of these methods for BIP 157 block filter-based fast rescans.

Also applies to: 143-149

src/node/interfaces.cpp (1)

9-9: LGTM! Correct and thread-safe implementation.

The implementation properly handles all edge cases:

• Returns std::nullopt when the block filter index is unavailable
• Uses WITH_LOCK(::cs_main, ...) to safely lookup the block index
• Returns std::nullopt when the block is not found or filter lookup fails
• Successfully returns the match result when all lookups succeed

Thread safety is correctly maintained for the block index access, and all error conditions are appropriately handled.

Also applies to: 22-22, 1072-1085

test/functional/test_runner.py (1)

163-163: LGTM!

The new test scripts rpc_packages.py and wallet_fast_rescan.py --descriptors are properly added to BASE_SCRIPTS. The placement in the ~30s section is appropriate for their expected runtime.

Also applies to: 170-170

test/functional/mempool_package_limits.py (3)

24-38: LGTM!

The check_package_limits decorator cleanly centralizes the common test pattern: verifying empty mempool, running the test to get package hex, checking that testmempoolaccept fails with package-mempool-limits, then clearing mempool and verifying the package passes. This reduces duplication across all decorated test methods.

---

46-62: LGTM!

The test setup correctly initializes MiniWallet, generates sufficient mature UTXOs (35 blocks + COINBASE_MATURITY for maturity), and appropriately restarts the node with -datacarriersize=100000 before the size-limit tests that require large OP_RETURN outputs.

---

64-131: LGTM!

The test methods are cleanly refactored to use MiniWallet's send_self_transfer_chain and create_self_transfer helpers. The test logic for chain limits remains intact while the code becomes more maintainable.

src/bench/examples.cpp (1)

21-21: LGTM!

The benchmark registration correctly adopts the new two-argument macro format with benchmark::PriorityLevel::HIGH, aligning with the PR's benchmark framework priority level support.

src/bench/bech32.cpp (1)

35-36: LGTM!

Both Bech32Encode and Bech32Decode benchmark registrations correctly updated with the explicit priority level parameter.

src/bench/checkqueue.cpp (1)

73-73: LGTM!

The benchmark registration correctly updated with the explicit priority level parameter.

src/bench/lockedpool.cpp (1)

42-42: LGTM!

The benchmark registration correctly updated with the explicit priority level parameter.

src/rpc/client.cpp (1)

142-142: LGTM! Correct parameter conversion entry for submitpackage RPC.

The entry correctly registers the "package" parameter at index 0 for the new regtest-only submitpackage RPC, enabling proper JSON parsing of the package argument. The format and placement are consistent with other RPC parameter conversion rules in the table.

src/bench/wallet_create.cpp (1)

52-53: LGTM! Benchmark priority levels correctly added.

The BENCHMARK registrations have been properly updated to include explicit benchmark::PriorityLevel::HIGH priority, consistent with the benchmark framework changes across the codebase. The benchmark logic remains unchanged.

src/bench/ccoins_caching.cpp (1)

53-53: LGTM! Benchmark priority level correctly added.

The BENCHMARK registration has been properly updated to include explicit benchmark::PriorityLevel::HIGH priority, consistent with the benchmark framework updates. No changes to the benchmark implementation itself.

src/util/error.h (1)

35-35: LGTM! New error enum value correctly added.

The INVALID_PACKAGE enum value is properly added to the TransactionError enum to support package validation errors in the new submitpackage RPC. Placement at the end of the enum maintains compatibility with existing code.

src/bench/peer_eviction.cpp (1)

144-155: LGTM! All benchmark priority levels correctly added.

All six BENCHMARK registrations (for 0/1/2/3-network eviction protection scenarios with varying candidate counts) have been properly updated to include explicit benchmark::PriorityLevel::HIGH priority, consistent with the benchmark framework changes. The benchmark implementations remain unchanged.

Note: The static analysis warning at line 148 is a false positive—the line contains only a BENCHMARK registration macro with no buffer access.

src/bench/wallet_loading.cpp (1)

121-121: LGTM! Benchmark priority levels added correctly.

The explicit priority level assignments to both WalletLoadingLegacy and WalletLoadingDescriptors benchmarks align with the framework enhancement to support prioritized benchmark execution.

Also applies to: 126-126

src/bench/pool.cpp (1)

48-49: LGTM! Benchmark priority levels added correctly.

Both pool allocator benchmarks now explicitly specify benchmark::PriorityLevel::HIGH, consistent with the framework enhancement for prioritized benchmark execution.

src/bench/coin_selection.cpp (1)

121-122: LGTM! Benchmark priority levels added correctly.

Both coin selection benchmarks (CoinSelection and BnBExhaustion) now explicitly specify benchmark::PriorityLevel::HIGH, aligning with the framework enhancement to support prioritized benchmark execution.

src/bench/rollingbloom.cpp (1)

35-36: LGTM! Benchmark priority levels added correctly.

Both rolling bloom filter benchmarks (RollingBloom and RollingBloomReset) now explicitly specify benchmark::PriorityLevel::HIGH, consistent with the framework enhancement for prioritized benchmark execution.

src/bench/hashpadding.cpp (1)

29-29: LGTM!

The benchmark registrations have been correctly updated to include explicit priority levels. This aligns with the benchmark framework enhancement across the codebase.

Also applies to: 47-47

src/bench/ecdsa.cpp (1)

87-89: LGTM!

All three benchmark registrations correctly updated with explicit priority levels.

src/bench/checkblock.cpp (1)

59-60: LGTM!

Both benchmark registrations correctly updated with explicit priority levels.

src/bench/mempool_stress.cpp (1)

115-116: LGTM!

Both benchmark registrations correctly updated with explicit priority levels.

src/bench/addrman.cpp (1)

170-175: LGTM: Benchmark priority levels added correctly.

All six benchmark registrations have been updated to include the explicit benchmark::PriorityLevel::HIGH parameter, consistent with the updated BENCHMARK macro signature in bench.h.

src/bench/base58.cpp (1)

53-55: LGTM: Benchmark priority levels added correctly.

All three benchmark registrations now include benchmark::PriorityLevel::HIGH, matching the updated macro signature.

src/rpc/mempool.cpp (4)

8-8: LGTM: Required include for regtest check.

The chainparams.h include is necessary for the Params().IsMockableChain() check in the new submitpackage RPC at line 807.

---

240-240: LGTM: Simplified vsize description.

The description was simplified from a detailed note about high-sigop transactions to a concise "Transaction size."

---

769-894: LGTM: Regtest-only submitpackage RPC implementation is well-structured.

The new submitpackage RPC is properly gated to regtest-only mode, includes comprehensive error handling for package validation results, and provides detailed transaction results. The implementation correctly:

• Validates the environment (regtest-only)
• Decodes and validates transaction inputs
• Processes the package through ProcessNewPackage
• Handles various error cases (PCKG_POLICY, PCKG_MEMPOOL_ERROR, PCKG_TX)
• Returns structured results with transaction fees and optional package feerate

---

908-908: LGTM: Correct registration as hidden RPC.

The submitpackage RPC is appropriately registered under the "hidden" category, consistent with its experimental regtest-only status.

src/bench/logging.cpp (1)

44-48: LGTM: Benchmark priority levels added correctly.

All five benchmark registrations now include benchmark::PriorityLevel::HIGH, consistent with the updated macro signature.

src/bench/bench.h (1)

79-80: LGTM: BENCHMARK macro signature updated to support priority levels.

The macro now correctly accepts two parameters (n, priority_level) and passes the priority level to the BenchRunner constructor. This foundational change is consistently applied across all benchmark registrations in the codebase.

test/functional/wallet_fast_rescan.py (1)

37-99: LGTM!

The test logic is well-structured, covering four rescan scenarios (fast/slow × active/non-active descriptors) and properly validating that all variants discover the same transaction set. The use of assert_debug_log to verify the correct rescan path is taken is a good practice.

test/functional/mempool_packages.py (1)

43-70: LGTM on wallet migration!

The refactoring to use MiniWallet for transaction creation is clean. The conditional is_specified_wallet_compiled() check for watch wallet creation properly handles cases where the wallet module isn't compiled in.

test/functional/test_framework/wallet.py (3)

96-106: LGTM on _bulk_tx implementation

The padding logic using OP_RETURN is correct. The weight bounds assertions appropriately account for the encoding overhead (up to 3 bytes for lower bound, +12 for length encoding on upper bound).

---

342-366: LGTM on chain creation methods

The create_self_transfer_chain and send_self_transfer_chain methods provide clean abstractions for creating dependent transaction chains, which simplifies the test code in mempool_packages.py and other tests.

---

315-335: LGTM on target_weight parameter addition

The target_weight parameter enables exact weight targeting for bulk transactions. The conditional vsize assertion bypass when target_weight is specified is correct since the padded transaction will have a different size.

test/functional/rpc_packages.py (1)

234-250: LGTM on assert_equal_package_results helper

This helper provides clean validation that submitpackage results are consistent with testmempoolaccept and getmempoolentry data. The conditional check for "allowed" key handles the case where a tx was already in the mempool.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Missing semicolons after BENCHMARK macro invocations.

The BENCHMARK macro invocations on lines 90-99 are missing trailing semicolons. Comparing with other benchmark files in this PR (e.g., examples.cpp line 21, bech32.cpp lines 35-36, checkqueue.cpp line 73), all use semicolons after the macro call.

🔎 Proposed fix

-BENCHMARK(BLS_PubKeyAggregate_Iterative_5, benchmark::PriorityLevel::HIGH)
-BENCHMARK(BLS_PubKeyAggregate_Batch_5, benchmark::PriorityLevel::HIGH)
-BENCHMARK(BLS_PubKeyAggregate_Iterative_25, benchmark::PriorityLevel::HIGH)
-BENCHMARK(BLS_PubKeyAggregate_Batch_25, benchmark::PriorityLevel::HIGH)
-BENCHMARK(BLS_PubKeyAggregate_Iterative_50, benchmark::PriorityLevel::HIGH)
-BENCHMARK(BLS_PubKeyAggregate_Batch_50, benchmark::PriorityLevel::HIGH)
-BENCHMARK(BLS_PubKeyAggregate_Iterative_100, benchmark::PriorityLevel::HIGH)
-BENCHMARK(BLS_PubKeyAggregate_Batch_100, benchmark::PriorityLevel::HIGH)
-BENCHMARK(BLS_PubKeyAggregate_Iterative_200, benchmark::PriorityLevel::HIGH)
-BENCHMARK(BLS_PubKeyAggregate_Batch_200, benchmark::PriorityLevel::HIGH)
+BENCHMARK(BLS_PubKeyAggregate_Iterative_5, benchmark::PriorityLevel::HIGH);
+BENCHMARK(BLS_PubKeyAggregate_Batch_5, benchmark::PriorityLevel::HIGH);
+BENCHMARK(BLS_PubKeyAggregate_Iterative_25, benchmark::PriorityLevel::HIGH);
+BENCHMARK(BLS_PubKeyAggregate_Batch_25, benchmark::PriorityLevel::HIGH);
+BENCHMARK(BLS_PubKeyAggregate_Iterative_50, benchmark::PriorityLevel::HIGH);
+BENCHMARK(BLS_PubKeyAggregate_Batch_50, benchmark::PriorityLevel::HIGH);
+BENCHMARK(BLS_PubKeyAggregate_Iterative_100, benchmark::PriorityLevel::HIGH);
+BENCHMARK(BLS_PubKeyAggregate_Batch_100, benchmark::PriorityLevel::HIGH);
+BENCHMARK(BLS_PubKeyAggregate_Iterative_200, benchmark::PriorityLevel::HIGH);
+BENCHMARK(BLS_PubKeyAggregate_Batch_200, benchmark::PriorityLevel::HIGH);

Note: The Cppcheck static analysis error is likely a false positive caused by its inability to resolve the macro expansion context.

📝 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.

Suggested change

	BENCHMARK(BLS_PubKeyAggregate_Iterative_5, benchmark::PriorityLevel::HIGH)
	BENCHMARK(BLS_PubKeyAggregate_Batch_5, benchmark::PriorityLevel::HIGH)
	BENCHMARK(BLS_PubKeyAggregate_Iterative_25, benchmark::PriorityLevel::HIGH)
	BENCHMARK(BLS_PubKeyAggregate_Batch_25, benchmark::PriorityLevel::HIGH)
	BENCHMARK(BLS_PubKeyAggregate_Iterative_50, benchmark::PriorityLevel::HIGH)
	BENCHMARK(BLS_PubKeyAggregate_Batch_50, benchmark::PriorityLevel::HIGH)
	BENCHMARK(BLS_PubKeyAggregate_Iterative_100, benchmark::PriorityLevel::HIGH)
	BENCHMARK(BLS_PubKeyAggregate_Batch_100, benchmark::PriorityLevel::HIGH)
	BENCHMARK(BLS_PubKeyAggregate_Iterative_200, benchmark::PriorityLevel::HIGH)
	BENCHMARK(BLS_PubKeyAggregate_Batch_200, benchmark::PriorityLevel::HIGH)
	BENCHMARK(BLS_PubKeyAggregate_Iterative_5, benchmark::PriorityLevel::HIGH);
	BENCHMARK(BLS_PubKeyAggregate_Batch_5, benchmark::PriorityLevel::HIGH);
	BENCHMARK(BLS_PubKeyAggregate_Iterative_25, benchmark::PriorityLevel::HIGH);
	BENCHMARK(BLS_PubKeyAggregate_Batch_25, benchmark::PriorityLevel::HIGH);
	BENCHMARK(BLS_PubKeyAggregate_Iterative_50, benchmark::PriorityLevel::HIGH);
	BENCHMARK(BLS_PubKeyAggregate_Batch_50, benchmark::PriorityLevel::HIGH);
	BENCHMARK(BLS_PubKeyAggregate_Iterative_100, benchmark::PriorityLevel::HIGH);
	BENCHMARK(BLS_PubKeyAggregate_Batch_100, benchmark::PriorityLevel::HIGH);
	BENCHMARK(BLS_PubKeyAggregate_Iterative_200, benchmark::PriorityLevel::HIGH);
	BENCHMARK(BLS_PubKeyAggregate_Batch_200, benchmark::PriorityLevel::HIGH);

🧰 Tools

🪛 Cppcheck (2.19.0)

[error] 95-95: failed to evaluate #if condition, undefined function-like macro invocation

(syntaxError)

🤖 Prompt for AI Agents

In @src/bench/bls_pubkey_agg.cpp around lines 90 - 99, Each BENCHMARK macro
invocation in this snippet is missing the trailing semicolon; update every
BENCHMARK line (e.g., BENCHMARK(BLS_PubKeyAggregate_Iterative_5,
benchmark::PriorityLevel::HIGH), BENCHMARK(BLS_PubKeyAggregate_Batch_5, ...),
BENCHMARK(BLS_PubKeyAggregate_Iterative_25, ...), etc.) to end with a semicolon
so the macros are treated as statements and match the style used elsewhere.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Use equality (==) instead of identity (is) for txid comparison

The comparison utxo['txid'] is parent_transaction uses identity check (is) instead of equality (==). For string comparison, this is unreliable and can cause tx_children to be empty or incomplete, breaking the descendant/spentby checks below.

Proposed fix

-            if utxo['txid'] is parent_transaction:
+            if utxo['txid'] == parent_transaction:
                 tx_children.append(txid)

📝 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.

Suggested change

	for _ in range(DEFAULT_DESCENDANT_LIMIT - 1):
	utxo = transaction_package.pop(0)
	(txid, sent_value) = chain_transaction(self.nodes[0], [utxo['txid']], [utxo['vout']], utxo['amount'], fee, 10)
	new_tx = self.wallet.send_self_transfer_multi(from_node=self.nodes[0], num_outputs=10, utxos_to_spend=[utxo])
	txid = new_tx["txid"]
	chain.append(txid)
	if utxo['txid'] is parent_transaction:
	tx_children.append(txid)
	for j in range(10):
	transaction_package.append({'txid': txid, 'vout': j, 'amount': sent_value})
	transaction_package.extend(new_tx["new_utxos"])
	for _ in range(DEFAULT_DESCENDANT_LIMIT - 1):
	utxo = transaction_package.pop(0)
	new_tx = self.wallet.send_self_transfer_multi(from_node=self.nodes[0], num_outputs=10, utxos_to_spend=[utxo])
	txid = new_tx["txid"]
	chain.append(txid)
	if utxo['txid'] == parent_transaction:
	tx_children.append(txid)
	transaction_package.extend(new_tx["new_utxos"])

🤖 Prompt for AI Agents

In @test/functional/mempool_packages.py around lines 231 - 239, The loop that
builds chain/tx_children compares utxo['txid'] to parent_transaction using
identity (`is`), which is incorrect for string values; update the comparison in
the for-loop so that utxo['txid'] == parent_transaction is used (ensuring string
equality) so tx_children is populated correctly when txids match; locate the
comparison involving utxo['txid'], parent_transaction and tx_children in the
transaction-package loop and replace the identity check with an equality check.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Use txid strings instead of tx.hash for result lookups and broadcast wait

There are two issues here:

1. submitpackage_result["tx-results"] is keyed by txid strings, but tx.hash is an internal integer. The membership check and lookup will fail.

2. P2PTxInvStore.wait_for_broadcast expects hex txid strings and calls int(tx, 16) on each element. Passing integer hashes will raise a TypeError.

Proposed fix

         # Check that each result is present, with the correct size and fees
         for package_txn in package_txns:
             tx = package_txn["tx"]
-            assert tx.hash in submitpackage_result["tx-results"]
-            tx_result = submitpackage_result["tx-results"][tx.hash]
+            assert package_txn["txid"] in submitpackage_result["tx-results"]
+            tx_result = submitpackage_result["tx-results"][package_txn["txid"]]
             assert_equal(tx_result, {
                 "txid": package_txn["txid"],
                 "size": tx.get_vsize(),
@@ -289,7 +289,7 @@
 
         # The node should announce each transaction. No guarantees for propagation.
         self.bump_mocktime(30)
-        peer.wait_for_broadcast([tx["tx"].hash for tx in package_txns])
+        peer.wait_for_broadcast([tx["txid"] for tx in package_txns])
         self.generate(node, 1)

🤖 Prompt for AI Agents

In @test/functional/rpc_packages.py around lines 268 - 293, The code incorrectly
uses the internal integer tx.hash for lookups and broadcast waits; change the
membership check, lookup key, and the list passed to peer.wait_for_broadcast to
use the txid hex string (e.g. package_txn["txid"] or tx.get_txid()/txid string
from the package_txn dict) instead of tx.hash or integer hashes so
submitpackage_result["tx-results"] lookups succeed and
P2PTxInvStore.wait_for_broadcast receives hex txid strings.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Same tx.hash vs txid issue in CPFP test

The child result lookup uses tx_child["tx"].hash (integer) instead of tx_child["txid"] (string), and wait_for_broadcast receives integer hashes instead of txid strings.

Proposed fix

         submitpackage_result = node.submitpackage([tx["hex"] for tx in package_txns])
 
         rich_parent_result = submitpackage_result["tx-results"][tx_rich["txid"]]
         poor_parent_result = submitpackage_result["tx-results"][tx_poor["txid"]]
-        child_result = submitpackage_result["tx-results"][tx_child["tx"].hash]
+        child_result = submitpackage_result["tx-results"][tx_child["txid"]]
         assert_equal(rich_parent_result["fees"]["base"], DEFAULT_FEE)
@@ -319,7 +319,7 @@
 
         # The node will broadcast each transaction, still abiding by its peer's fee filter
         self.bump_mocktime(30)
-        peer.wait_for_broadcast([tx["tx"].hash for tx in package_txns])
+        peer.wait_for_broadcast([tx["txid"] for tx in package_txns])
         self.generate(node, 1)

📝 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.

Suggested change

	submitpackage_result = node.submitpackage([tx["hex"] for tx in package_txns])
	rich_parent_result = submitpackage_result["tx-results"][tx_rich["txid"]]
	poor_parent_result = submitpackage_result["tx-results"][tx_poor["txid"]]
	child_result = submitpackage_result["tx-results"][tx_child["tx"].hash]
	assert_equal(rich_parent_result["fees"]["base"], DEFAULT_FEE)
	assert_equal(poor_parent_result["fees"]["base"], 0)
	assert_equal(child_result["fees"]["base"], DEFAULT_FEE)
	# Package feerate is calculated for the remaining transactions after deduplication and
	# individual submission. Since this package had a 0-fee parent, package feerate must have
	# been used and returned.
	assert "package-feerate" in submitpackage_result
	assert_fee_amount(DEFAULT_FEE, rich_parent_result["size"] + child_result["size"], submitpackage_result["package-feerate"])
	# The node will broadcast each transaction, still abiding by its peer's fee filter
	self.bump_mocktime(30)
	peer.wait_for_broadcast([tx["tx"].hash for tx in package_txns])
	self.generate(node, 1)
	submitpackage_result = node.submitpackage([tx["hex"] for tx in package_txns])
	rich_parent_result = submitpackage_result["tx-results"][tx_rich["txid"]]
	poor_parent_result = submitpackage_result["tx-results"][tx_poor["txid"]]
	child_result = submitpackage_result["tx-results"][tx_child["txid"]]
	assert_equal(rich_parent_result["fees"]["base"], DEFAULT_FEE)
	assert_equal(poor_parent_result["fees"]["base"], 0)
	assert_equal(child_result["fees"]["base"], DEFAULT_FEE)
	# Package feerate is calculated for the remaining transactions after deduplication and
	# individual submission. Since this package had a 0-fee parent, package feerate must have
	# been used and returned.
	assert "package-feerate" in submitpackage_result
	assert_fee_amount(DEFAULT_FEE, rich_parent_result["size"] + child_result["size"], submitpackage_result["package-feerate"])
	# The node will broadcast each transaction, still abiding by its peer's fee filter
	self.bump_mocktime(30)
	peer.wait_for_broadcast([tx["txid"] for tx in package_txns])
	self.generate(node, 1)

🤖 Prompt for AI Agents

In @test/functional/rpc_packages.py around lines 306 - 323, The child
transaction lookup and broadcast checks use integer .hash values instead of txid
strings; update the child_result lookup to use tx_child["txid"] when indexing
submitpackage_result["tx-results"] (matching
rich_parent_result/poor_parent_result usage) and pass the list of txid strings
(e.g., [tx["txid"] for tx in package_txns]) to peer.wait_for_broadcast so it
expects txid strings rather than integer hashes.

[github-actions]

This pull request has conflicts, please rebase.

[coderabbitai]

Actionable comments posted: 5

🤖 Fix all issues with AI agents

In @src/rpc/mempool.cpp:
- Around line 861-870: The variable num_submitted is declared inside the loop in
the transaction broadcast block and never incremented, so the error message
always reports 0; move the declaration of num_submitted outside the for loop (so
it accumulates across iterations), increment it whenever
BroadcastTransaction(node, tx, ...) returns TransactionError::OK, and then use
that accumulated num_submitted in the JSONRPCTransactionError message; update
references around BroadcastTransaction and the surrounding loop to ensure
num_submitted reflects the count of successfully broadcast transactions.

In @test/functional/mempool_packages.py:
- Around line 223-239: The loop that builds chained child transactions
incorrectly uses identity comparison when checking utxo['txid'] is
parent_transaction, which can miss matches; update that conditional in the
for-loop (where utxo, parent_transaction, tx_children, chain and
transaction_package are used) to use equality (==) instead of is so string txid
comparisons correctly detect children and populate tx_children for downstream
descendant/spentby checks.

In @test/functional/rpc_packages.py:
- Around line 295-323: In test_submit_cpfp, the child transaction is being
referenced by tx_child["tx"].hash and peer.wait_for_broadcast is given integer
hashes, but submitpackage_result["tx-results"] and P2PTxInvStore expect txid
strings; change the child lookup to use tx_child["txid"] (matching
rich_parent_result/poor_parent_result) and call peer.wait_for_broadcast with the
list [tx["txid"] for tx in package_txns] so both the submitpackage_result access
and the broadcast check consistently use txid strings.
- Around line 234-293: The test uses internal integer tx.hash values where
submitpackage_result and P2PTxInvStore expect hex txid strings; update all
lookups and waits in test_submit_child_with_parents to use the
package_txn["txid"] string (e.g., when asserting membership in
submitpackage_result["tx-results"], when fetching tx_result, and when calling
peer.wait_for_broadcast) instead of tx.hash; ensure any places that compare or
index by txid use the same string key from package_txn["txid"] so
submitpackage_result lookups and broadcast waits use the correct hex txid type.

In @test/functional/wallet_fast_rescan.py:
- Line 52: The log call uses an unnecessary f-string prefix in the message
inside the self.log.info call; remove the leading 'f' so the string becomes a
normal literal in the self.log.info(...) invocation (locate the call to
self.log.info that currently reads f"Create txs sending to end range address of
each descriptor, triggering top-ups" and change it to a plain string without the
f prefix).

🧹 Nitpick comments (1)

test/functional/wallet_fast_rescan.py (1)

23-50: Replace print() with self.log.debug() for consistent logging.

Line 47 uses print() to output the result of importdescriptors, which should use the test framework's logging instead.

Proposed fix

-        print(w.importdescriptors([{"desc": descsum_create(f"wpkh({fixed_key.privkey})"), "timestamp": "now"}]))
+        self.log.debug(w.importdescriptors([{"desc": descsum_create(f"wpkh({fixed_key.privkey})"), "timestamp": "now"}]))

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 50c42a2 and f5f95e6.

📒 Files selected for processing (82)

• ci/dash/lint-tidy.sh
• ci/test/00_setup_env_mac_native_arm64.sh
• ci/test/04_install.sh
• doc/descriptors.md
• doc/release-notes-25957.md
• src/Makefile.am
• src/bench/addrman.cpp
• src/bench/base58.cpp
• src/bench/bech32.cpp
• src/bench/bench.h
• src/bench/bip324_ecdh.cpp
• src/bench/block_assemble.cpp
• src/bench/bls.cpp
• src/bench/bls_dkg.cpp
• src/bench/bls_pubkey_agg.cpp
• src/bench/ccoins_caching.cpp
• src/bench/chacha20.cpp
• src/bench/checkblock.cpp
• src/bench/checkqueue.cpp
• src/bench/coin_selection.cpp
• src/bench/crypto_hash.cpp
• src/bench/duplicate_inputs.cpp
• src/bench/ecdsa.cpp
• src/bench/ellswift.cpp
• src/bench/examples.cpp
• src/bench/gcs_filter.cpp
• src/bench/hashpadding.cpp
• src/bench/load_external.cpp
• src/bench/lockedpool.cpp
• src/bench/logging.cpp
• src/bench/mempool_eviction.cpp
• src/bench/mempool_stress.cpp
• src/bench/merkle_root.cpp
• src/bench/peer_eviction.cpp
• src/bench/poly1305.cpp
• src/bench/pool.cpp
• src/bench/pow_hash.cpp
• src/bench/prevector.cpp
• src/bench/rollingbloom.cpp
• src/bench/rpc_blockchain.cpp
• src/bench/rpc_mempool.cpp
• src/bench/strencodings.cpp
• src/bench/string_cast.cpp
• src/bench/util_time.cpp
• src/bench/verify_script.cpp
• src/bench/wallet_balance.cpp
• src/bench/wallet_create.cpp
• src/bench/wallet_loading.cpp
• src/bitcoin-cli.cpp
• src/bitcoin-wallet.cpp
• src/bitcoind.cpp
• src/common/url.cpp
• src/common/url.h
• src/interfaces/chain.h
• src/logging.cpp
• src/logging.h
• src/node/interfaces.cpp
• src/qt/main.cpp
• src/rpc/client.cpp
• src/rpc/mempool.cpp
• src/test/fuzz/rpc.cpp
• src/test/fuzz/string.cpp
• src/test/util/setup_common.cpp
• src/util/error.cpp
• src/util/error.h
• src/wallet/rpc/backup.cpp
• src/wallet/rpc/transactions.cpp
• src/wallet/rpc/util.cpp
• src/wallet/rpc/wallet.cpp
• src/wallet/scriptpubkeyman.cpp
• src/wallet/scriptpubkeyman.h
• src/wallet/wallet.cpp
• test/functional/mempool_package_limits.py
• test/functional/mempool_packages.py
• test/functional/rpc_getblockfrompeer.py
• test/functional/rpc_packages.py
• test/functional/test_framework/util.py
• test/functional/test_framework/wallet.py
• test/functional/test_runner.py
• test/functional/wallet_fast_rescan.py
• test/functional/wallet_listsinceblock.py
• test/functional/wallet_listtransactions.py

💤 Files with no reviewable changes (1)

• test/functional/test_framework/util.py

✅ Files skipped from review due to trivial changes (1)

• doc/descriptors.md

🚧 Files skipped from review as they are similar to previous changes (35)

• doc/release-notes-25957.md
• src/wallet/scriptpubkeyman.cpp
• src/qt/main.cpp
• src/bench/util_time.cpp
• src/bench/lockedpool.cpp
• src/bitcoin-wallet.cpp
• ci/dash/lint-tidy.sh
• src/bench/poly1305.cpp
• src/bench/rpc_blockchain.cpp
• src/wallet/rpc/backup.cpp
• src/bench/strencodings.cpp
• src/common/url.h
• src/logging.h
• src/util/error.cpp
• src/wallet/rpc/wallet.cpp
• src/bench/chacha20.cpp
• src/bench/ellswift.cpp
• src/bench/bls.cpp
• src/bench/block_assemble.cpp
• src/bench/verify_script.cpp
• src/bench/checkqueue.cpp
• src/bench/examples.cpp
• src/bench/checkblock.cpp
• src/bench/rollingbloom.cpp
• src/bench/pool.cpp
• src/bench/addrman.cpp
• src/test/fuzz/rpc.cpp
• ci/test/04_install.sh
• src/bitcoind.cpp
• src/bitcoin-cli.cpp
• src/bench/duplicate_inputs.cpp
• src/logging.cpp
• src/wallet/rpc/util.cpp
• src/bench/hashpadding.cpp
• test/functional/test_runner.py

🧰 Additional context used

📓 Path-based instructions (7)

src/**/*.{cpp,h,hpp,cc}

📄 CodeRabbit inference engine (CLAUDE.md)

Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1

Files:

• src/bench/rpc_mempool.cpp
• src/bench/bench.h
• src/util/error.h
• src/bench/coin_selection.cpp
• src/bench/mempool_stress.cpp
• src/wallet/scriptpubkeyman.h
• src/bench/load_external.cpp
• src/interfaces/chain.h
• src/bench/base58.cpp
• src/bench/ccoins_caching.cpp
• src/bench/gcs_filter.cpp
• src/wallet/wallet.cpp
• src/bench/string_cast.cpp
• src/bench/bls_pubkey_agg.cpp
• src/bench/bech32.cpp
• src/test/fuzz/string.cpp
• src/common/url.cpp
• src/rpc/mempool.cpp
• src/bench/bip324_ecdh.cpp
• src/bench/wallet_create.cpp
• src/bench/logging.cpp
• src/bench/ecdsa.cpp
• src/bench/crypto_hash.cpp
• src/test/util/setup_common.cpp
• src/rpc/client.cpp
• src/bench/wallet_loading.cpp
• src/bench/wallet_balance.cpp
• src/bench/prevector.cpp
• src/wallet/rpc/transactions.cpp
• src/bench/bls_dkg.cpp
• src/bench/merkle_root.cpp
• src/bench/mempool_eviction.cpp
• src/bench/pow_hash.cpp
• src/bench/peer_eviction.cpp
• src/node/interfaces.cpp

src/bench/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Performance benchmarks in src/bench/ must use the nanobench library

Files:

• src/bench/rpc_mempool.cpp
• src/bench/bench.h
• src/bench/coin_selection.cpp
• src/bench/mempool_stress.cpp
• src/bench/load_external.cpp
• src/bench/base58.cpp
• src/bench/ccoins_caching.cpp
• src/bench/gcs_filter.cpp
• src/bench/string_cast.cpp
• src/bench/bls_pubkey_agg.cpp
• src/bench/bech32.cpp
• src/bench/bip324_ecdh.cpp
• src/bench/wallet_create.cpp
• src/bench/logging.cpp
• src/bench/ecdsa.cpp
• src/bench/crypto_hash.cpp
• src/bench/wallet_loading.cpp
• src/bench/wallet_balance.cpp
• src/bench/prevector.cpp
• src/bench/bls_dkg.cpp
• src/bench/merkle_root.cpp
• src/bench/mempool_eviction.cpp
• src/bench/pow_hash.cpp
• src/bench/peer_eviction.cpp

src/wallet/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Wallet implementation must use Berkeley DB and SQLite

Files:

• src/wallet/scriptpubkeyman.h
• src/wallet/wallet.cpp
• src/wallet/rpc/transactions.cpp

{guix-build*,releases,**/guix-build*,releases/**,.github/**,depends/**,ci/**,contrib/**,doc/**}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not make changes to build system files (guix-build*), release artifacts, or avoid changes to .github, depends, ci, contrib, and doc directories unless specifically prompted

Files:

• ci/test/00_setup_env_mac_native_arm64.sh

test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Functional tests in test/functional/ must be written in Python (minimum version specified in .python-version) and depend on dashd and dash-node

Files:

• test/functional/wallet_fast_rescan.py
• test/functional/test_framework/wallet.py
• test/functional/rpc_packages.py
• test/functional/mempool_packages.py
• test/functional/rpc_getblockfrompeer.py
• test/functional/mempool_package_limits.py
• test/functional/wallet_listsinceblock.py
• test/functional/wallet_listtransactions.py

src/{test,wallet/test}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework

Files:

• src/test/fuzz/string.cpp
• src/test/util/setup_common.cpp

src/node/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

NodeContext must be extended with Dash-specific managers for each subsystem during initialization

Files:

• src/node/interfaces.cpp

🧠 Learnings (27)

📓 Common learnings

Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.

Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.

Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.

Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.

Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.

Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.

Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:26.891Z
Learning: In Dash Qt wallet code, when leveraging upstream Bitcoin Core wallet capabilities (especially for operations like rescanning), prefer using the inherited signal-based mechanisms (e.g., ShowProgress signals) over adding explicit Qt progress dialogs to minimize upstream deviation and simplify future backports.

Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,txmempool}/**/*.{cpp,h} : Block validation and mempool handling must use extensions to Bitcoin Core mechanisms for special transaction validation and enhanced transaction relay

Learnt from: knst
Repo: dashpay/dash PR: 6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.332Z
Learning: In src/wallet/rpc/wallet.cpp, the upgradetohd RPC now returns a UniValue string message (RPCResult::Type::STR) instead of a boolean, including guidance about mnemonic backup and null-character passphrase handling; functional tests have been updated to assert returned strings in several cases.

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/bench/**/*.{cpp,h} : Performance benchmarks in src/bench/ must use the nanobench library

Applied to files:

• src/bench/rpc_mempool.cpp
• src/bench/bench.h
• src/bench/coin_selection.cpp
• src/bench/mempool_stress.cpp
• src/bench/load_external.cpp
• src/bench/base58.cpp
• src/bench/ccoins_caching.cpp
• src/bench/gcs_filter.cpp
• src/bench/string_cast.cpp
• src/bench/bls_pubkey_agg.cpp
• src/bench/bech32.cpp
• src/bench/logging.cpp
• src/bench/ecdsa.cpp
• src/bench/crypto_hash.cpp
• src/bench/wallet_loading.cpp
• src/bench/wallet_balance.cpp
• src/bench/prevector.cpp
• src/bench/merkle_root.cpp
• src/bench/mempool_eviction.cpp
• src/bench/pow_hash.cpp
• src/bench/peer_eviction.cpp

📚 Learning: 2025-12-22T15:42:48.595Z

Learnt from: kwvg
Repo: dashpay/dash PR: 7068
File: src/qt/guiutil_font.h:68-77
Timestamp: 2025-12-22T15:42:48.595Z
Learning: In C++/Qt codebases, use fail-fast asserts in setters to enforce invariants (e.g., ensuring internal maps contain necessary keys). Prefer assert() for programmer errors that should be caught in development and debugging, rather than defensive runtime checks with fallbacks. This helps catch invariant violations early during development. Apply to header and source files where invariant-driven state mutations occur, especially in setters like SetWeightBold/SetWeightNormal that assume established relationships or preconditions.

Applied to files:

• src/bench/bench.h
• src/util/error.h
• src/wallet/scriptpubkeyman.h
• src/interfaces/chain.h

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs

Applied to files:

• src/bench/coin_selection.cpp
• src/wallet/wallet.cpp
• src/node/interfaces.cpp

📚 Learning: 2025-01-02T08:33:26.751Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6516
File: ci/test/00_setup_env_mac.sh:11-11
Timestamp: 2025-01-02T08:33:26.751Z
Learning: The removal of DMG support in the macOS packaging process eliminates the need for python-based scripts or python3-setuptools in the build environment. The PACKAGES variable in ci/test/00_setup_env_mac.sh is vestigial due to the use of a general-purpose Docker container defined in contrib/containers/ci/Dockerfile.

Applied to files:

• ci/test/00_setup_env_mac_native_arm64.sh

📚 Learning: 2025-07-23T09:30:34.631Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.h:5-6
Timestamp: 2025-07-23T09:30:34.631Z
Learning: Dash Core uses BITCOIN_ prefix for header guards as the standard convention, inherited from Bitcoin Core. Only a few BLS-specific files in src/bls/ use DASH_ prefix. The vast majority of files (385+) use BITCOIN_ prefix.

Applied to files:

• src/Makefile.am
• src/bench/crypto_hash.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Dash extends Bitcoin Core through composition with minimal changes to the Bitcoin Core foundation

Applied to files:

• src/Makefile.am

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,txmempool}/**/*.{cpp,h} : Block validation and mempool handling must use extensions to Bitcoin Core mechanisms for special transaction validation and enhanced transaction relay

Applied to files:

• src/interfaces/chain.h
• src/wallet/wallet.cpp
• test/functional/mempool_packages.py
• src/rpc/mempool.cpp
• src/wallet/rpc/transactions.cpp
• src/node/interfaces.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,consensus,net_processing}/**/*.{cpp,h} : ValidationInterface callbacks must be used for event-driven architecture to coordinate subsystems during block/transaction processing

Applied to files:

• src/interfaces/chain.h
• src/rpc/mempool.cpp
• src/node/interfaces.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files

Applied to files:

• src/interfaces/chain.h
• src/rpc/mempool.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention

Applied to files:

• src/interfaces/chain.h
• src/rpc/mempool.cpp

📚 Learning: 2025-08-08T07:01:47.332Z

Learnt from: knst
Repo: dashpay/dash PR: 6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.332Z
Learning: In src/wallet/rpc/wallet.cpp, the upgradetohd RPC now returns a UniValue string message (RPCResult::Type::STR) instead of a boolean, including guidance about mnemonic backup and null-character passphrase handling; functional tests have been updated to assert returned strings in several cases.

Applied to files:

• test/functional/rpc_packages.py
• test/functional/mempool_packages.py
• src/rpc/mempool.cpp
• src/rpc/client.cpp
• src/bench/wallet_balance.cpp
• src/wallet/rpc/transactions.cpp

📚 Learning: 2025-06-09T16:43:20.996Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

Applied to files:

• test/functional/rpc_packages.py
• test/functional/mempool_packages.py
• test/functional/mempool_package_limits.py

📚 Learning: 2025-12-17T13:58:26.891Z

Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:26.891Z
Learning: In Dash Qt wallet code, when leveraging upstream Bitcoin Core wallet capabilities (especially for operations like rescanning), prefer using the inherited signal-based mechanisms (e.g., ShowProgress signals) over adding explicit Qt progress dialogs to minimize upstream deviation and simplify future backports.

Applied to files:

• src/wallet/wallet.cpp

📚 Learning: 2025-02-14T15:19:17.218Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6529
File: src/wallet/rpcwallet.cpp:3002-3003
Timestamp: 2025-02-14T15:19:17.218Z
Learning: The `GetWallet()` function calls in `src/wallet/rpcwallet.cpp` are properly validated with null checks that throw appropriate RPC errors, making additional validation unnecessary.

Applied to files:

• src/wallet/wallet.cpp
• src/wallet/rpc/transactions.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/wallet/**/*.{cpp,h} : Wallet implementation must use Berkeley DB and SQLite

Applied to files:

• src/wallet/wallet.cpp
• src/bench/wallet_create.cpp
• src/bench/wallet_loading.cpp

📚 Learning: 2025-11-04T18:23:28.175Z

Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/governance/classes.cpp:147-154
Timestamp: 2025-11-04T18:23:28.175Z
Learning: In src/governance/classes.cpp, CSuperblock::GetPaymentsLimit intentionally uses synthetic difficulty constants (nBits = 1 for mainnet, powLimit for networks with min difficulty) and simple height-based V20 activation checks instead of actual chain block data. This is by design because superblocks themselves are "synthetic" governance payment blocks, not regular mined blocks.

Applied to files:

• src/wallet/wallet.cpp

📚 Learning: 2025-06-20T23:32:16.225Z

Learnt from: kwvg
Repo: dashpay/dash PR: 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.

Applied to files:

• test/functional/mempool_packages.py

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features

Applied to files:

• src/bench/bls_pubkey_agg.cpp
• src/rpc/mempool.cpp
• src/test/util/setup_common.cpp
• src/node/interfaces.cpp

📚 Learning: 2025-02-14T15:15:58.165Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6529
File: src/rpc/governance.cpp:1074-1089
Timestamp: 2025-02-14T15:15:58.165Z
Learning: Code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded from clang-format suggestions as they are intentionally formatted manually for better readability.

Applied to files:

• src/bench/bls_pubkey_agg.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h} : Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data

Applied to files:

• src/bench/crypto_hash.cpp
• src/node/interfaces.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h} : Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures

Applied to files:

• src/bench/crypto_hash.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{test,wallet/test}/**/*.{cpp,h} : Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework

Applied to files:

• src/bench/wallet_balance.cpp

📚 Learning: 2025-02-06T14:34:30.466Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.

Applied to files:

• src/wallet/rpc/transactions.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)

Applied to files:

• src/bench/bls_dkg.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus

Applied to files:

• src/bench/bls_dkg.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/governance/**/*.{cpp,h} : Governance implementation must support governance objects (proposals, triggers, superblock management) and on-chain voting with tallying

Applied to files:

• src/node/interfaces.cpp

🧬 Code graph analysis (7)

src/wallet/scriptpubkeyman.h (1)

src/wallet/scriptpubkeyman.cpp (6)

• GetScriptPubKeys (2516-2519)
• GetScriptPubKeys (2516-2516)
• GetScriptPubKeys (2521-2531)
• GetScriptPubKeys (2521-2521)
• GetEndRange (2533-2536)
• GetEndRange (2533-2533)

test/functional/test_framework/wallet.py (2)

test/functional/test_framework/script.py (1)

• CScript (420-593)

test/functional/test_framework/util.py (1)

• assert_greater_than_or_equal (82-84)

test/functional/rpc_getblockfrompeer.py (2)

test/functional/test_framework/p2p.py (1)

• send_and_ping (750-752)

test/functional/test_framework/messages.py (1)

• msg_headers (2050-2068)

test/functional/mempool_package_limits.py (2)

test/functional/test_framework/util.py (1)

• assert_equal (69-74)

test/functional/test_framework/wallet.py (4)

• MiniWallet (73-363)
• create_self_transfer (312-332)
• create_self_transfer_multi (257-310)
• sendrawtransaction (334-337)

src/node/interfaces.cpp (2)

src/wallet/wallet.cpp (2)

• block_hash (319-322)
• block_hash (319-319)

src/index/blockfilterindex.cpp (1)

• filter (274-274)

test/functional/wallet_listsinceblock.py (1)

src/wallet/rpc/transactions.cpp (2)

• listsinceblock (554-692)
• listsinceblock (554-554)

test/functional/wallet_listtransactions.py (2)

test/functional/wallet_listsinceblock.py (1)

• test_op_return (404-415)

src/wallet/rpc/transactions.cpp (2)

• listtransactions (442-552)
• listtransactions (442-442)

🪛 Cppcheck (2.19.0)

src/bench/bls_pubkey_agg.cpp

[error] 95-95: failed to evaluate #if condition, undefined function-like macro invocation

(syntaxError)

src/test/util/setup_common.cpp

[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation

(syntaxError)

src/bench/peer_eviction.cpp

[error] 148-148: Buffer is accessed out of bounds

(bufferAccessOutOfBounds)

src/node/interfaces.cpp

[error] 22-22: failed to evaluate #if condition, undefined function-like macro invocation

(syntaxError)

🪛 Flake8 (7.3.0)

test/functional/wallet_fast_rescan.py

[error] 52-52: f-string is missing placeholders

(F541)

test/functional/rpc_packages.py

[error] 51-51: continuation line over-indented for hanging indent

(E126)

---

[error] 56-56: continuation line under-indented for hanging indent

(E121)

---

[error] 101-101: continuation line under-indented for visual indent

(E128)

🪛 Ruff (0.14.10)

test/functional/wallet_fast_rescan.py

52-52: f-string without any placeholders

Remove extraneous f prefix

(F541)

test/functional/test_framework/wallet.py

287-287: zip() without an explicit strict= parameter

Add explicit value for parameter strict=

(B905)

test/functional/rpc_packages.py

120-120: Consider [tx_high_fee["hex"], *self.independent_txns_hex] instead of concatenation

Replace with [tx_high_fee["hex"], *self.independent_txns_hex]

(RUF005)

---

197-197: Consider [*package_hex, child_tx['hex']] instead of concatenation

Replace with [*package_hex, child_tx['hex']]

(RUF005)

---

202-202: Consider [*package_hex, child_tx["hex"]] instead of concatenation

Replace with [*package_hex, child_tx["hex"]]

(RUF005)

---

245-245: Unnecessary key check before dictionary access

Replace with dict.get

(RUF019)

---

260-260: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)

test/functional/wallet_listsinceblock.py

413-413: Prefer next(...) over single element slice

Replace with next(...)

(RUF015)

test/functional/wallet_listtransactions.py

177-177: Prefer next(...) over single element slice

Replace with next(...)

(RUF015)

⏰ 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: Lint / Run linters
• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: linux64_nowallet-build / Build source
• GitHub Check: win64-build / Build source
• GitHub Check: linux64_ubsan-build / Build source
• GitHub Check: linux64_fuzz-build / Build source
• GitHub Check: linux64_sqlite-build / Build source
• GitHub Check: linux64-build / Build source
• GitHub Check: arm-linux-build / Build source
• GitHub Check: mac-build / Build source

🔇 Additional comments (62)

test/functional/rpc_getblockfrompeer.py (1)

95-95: LGTM: Proper synchronization added to fix intermittent test failure.

Replacing send_message with send_and_ping ensures the header is fully processed before the test attempts to fetch the block at line 104, eliminating the race condition that caused intermittent failures.

src/bench/wallet_create.cpp (1)

52-53: LGTM! Priority level correctly added to benchmark registrations.

The changes correctly add benchmark::PriorityLevel::HIGH to both wallet creation benchmarks, consistent with the PR's objective to add priority level support to the benchmark framework.

src/wallet/scriptpubkeyman.h (1)

644-645: LGTM! New accessor methods for descriptor wallet fast rescan.

The new GetScriptPubKeys(int32_t minimum_index) overload and GetEndRange() method correctly extend the DescriptorScriptPubKeyMan interface to support index-aware filtering of script public keys. The declarations are const-correct, use appropriate types (int32_t matching the ScriptPubKeyMap index type), and match their implementations in scriptpubkeyman.cpp which properly handle thread safety with cs_desc_man locking.

These additions enable the fast rescan feature to efficiently query descriptor ranges when using BIP157 block filters.

src/util/error.h (1)

35-35: LGTM! Clean enum addition for package validation.

The new INVALID_PACKAGE error type is correctly positioned and aligns with the PR's objective to add package submission support via the regtest-only submitpackage RPC.

src/bench/rpc_mempool.cpp (1)

42-42: LGTM! Benchmark priority registration updated correctly.

The explicit priority level addition aligns with the benchmark framework enhancement in this backport.

src/bench/bip324_ecdh.cpp (1)

51-51: LGTM! Benchmark priority registration updated correctly.

The explicit priority level addition aligns with the benchmark framework enhancement in this backport.

src/bench/coin_selection.cpp (1)

121-122: LGTM! Benchmark priority registrations updated correctly.

Both benchmark registrations now include the explicit priority level, consistent with the framework enhancement in this backport.

src/bench/wallet_balance.cpp (1)

58-61: LGTM! Benchmark priority registrations updated correctly.

All four benchmark registrations now include the explicit priority level, consistent with the framework enhancement in this backport.

src/bench/wallet_loading.cpp (2)

121-121: LGTM! Benchmark priority registration updated correctly.

The explicit priority level addition for the legacy wallet loading benchmark aligns with the framework enhancement in this backport.

---

126-126: LGTM! Benchmark priority registration updated correctly.

The explicit priority level addition for the descriptor wallet loading benchmark aligns with the framework enhancement in this backport.

src/bench/ccoins_caching.cpp (1)

53-53: LGTM! Benchmark priority level addition is correct.

The addition of benchmark::PriorityLevel::HIGH to the benchmark registration is consistent with the framework enhancement across the repository. The benchmark body remains unchanged, and this metadata allows the benchmark runner to prioritize execution appropriately.

src/bench/bech32.cpp (1)

35-36: LGTM! Benchmark priority level additions are correct.

Both benchmark registrations have been properly updated to include benchmark::PriorityLevel::HIGH. The benchmark implementations remain unchanged, and the priority metadata enables selective benchmark execution in the test suite.

src/bench/load_external.cpp (1)

63-63: LGTM! Benchmark priority level addition is correct.

The benchmark registration has been properly updated to include the priority parameter. The benchmark logic remains unchanged, and this addition aligns with the framework enhancement for prioritized benchmark execution.

src/bench/peer_eviction.cpp (1)

144-155: LGTM! All benchmark priority level additions are correct.

All six peer eviction benchmark registrations have been consistently updated to include benchmark::PriorityLevel::HIGH. The benchmark implementations remain unchanged, and these metadata additions enable the benchmark framework to selectively run high-priority benchmarks during make check.

src/bench/gcs_filter.cpp (1)

84-88: LGTM! All benchmark priority level additions are correct.

All five GCS filter benchmark registrations have been properly updated to include benchmark::PriorityLevel::HIGH. The benchmark implementations are unchanged, and these additions support the enhanced benchmark framework for prioritized execution.

src/bench/merkle_root.cpp (1)

26-26: LGTM!

The benchmark registration correctly adopts the new two-argument BENCHMARK macro with PriorityLevel::HIGH.

src/bench/ecdsa.cpp (1)

87-89: LGTM!

All three ECDSA benchmark registrations correctly updated with PriorityLevel::HIGH. The benchmark bodies remain unchanged and continue to use the nanobench library correctly.

src/bench/bls_dkg.cpp (1)

134-139: LGTM!

All four BLS DKG benchmark macro definitions correctly updated to include benchmark::PriorityLevel::HIGH in their internal BENCHMARK invocations. The macro structure and benchmark logic remain intact.

Also applies to: 152-157, 159-172, 174-187

src/bench/mempool_eviction.cpp (1)

125-125: LGTM!

The mempool eviction benchmark registration correctly updated with PriorityLevel::HIGH.

src/bench/mempool_stress.cpp (1)

115-116: LGTM!

Both mempool stress benchmarks (ComplexMemPool and MempoolCheck) correctly updated with PriorityLevel::HIGH.

src/bench/base58.cpp (1)

53-55: LGTM!

All three Base58 benchmarks correctly updated with PriorityLevel::HIGH.

src/bench/prevector.cpp (2)

136-146: LGTM!

The PREVECTOR_TEST macro correctly updated to register both Nontrivial and Trivial benchmark variants with PriorityLevel::HIGH.

---

153-154: LGTM!

Standalone prevector benchmarks (PrevectorAssign and PrevectorAssignTo) correctly updated with PriorityLevel::HIGH.

src/bench/string_cast.cpp (1)

70-75: LGTM!

The benchmark registrations correctly use the updated BENCHMARK macro signature with explicit benchmark::PriorityLevel::HIGH priority. The changes align with the benchmark framework enhancement in this backport.

src/bench/logging.cpp (1)

44-48: LGTM!

The logging benchmark registrations correctly adopt the new priority-level parameter. All five benchmarks are appropriately marked as HIGH priority.

src/bench/bench.h (1)

79-80: LGTM!

The BENCHMARK macro signature update correctly requires an explicit priority_level parameter. The macro expansion already includes the trailing semicolon, so invocation sites should omit it (as they correctly do throughout the codebase).

src/bench/crypto_hash.cpp (2)

261-285: LGTM!

All Bitcoin Core cryptographic hash benchmarks correctly adopt the new priority-level parameter with HIGH priority.

---

353-360: LGTM!

The Dash-specific DSHA256 benchmarks are consistently updated to match the new BENCHMARK macro signature.

src/bench/pow_hash.cpp (1)

265-359: LGTM! Consistent priority level update across all PoW benchmarks.

All 77 benchmark registrations have been correctly updated to include benchmark::PriorityLevel::HIGH, aligning with the new BENCHMARK macro signature. The changes are mechanical, consistent, and appropriate for these critical PoW hash function benchmarks.

ci/test/00_setup_env_mac_native_arm64.sh (3)

10-10: LGTM: Correct ARM64 architecture target.

The HOST triplet correctly specifies the ARM64 architecture for native macOS builds.

---

11-11: The lief package removal is safe and intentional for ARM64 macOS native builds.

The lief package is not required for the ARM64 native macOS build configuration. Security tests (RUN_SECURITY_TESTS) default to false and are not explicitly enabled in the ARM64 setup, so the test-security-check tools (which require lief) will not run. The lief package is installed separately in the ci/lint phase for static analysis checks, not in the test phase. This change matches the pattern used for regular macOS builds, which also do not include lief in PIP_PACKAGES.

---

13-13: Do not modify ci/ directory files without explicit justification. Per coding guidelines, changes to ci/ directories should be avoided unless specifically prompted. If this change is intentional, clarify the business requirement and whether it aligns with upstream Bitcoin Core's macOS ARM64 configuration.

The --with-miniupnpc and --with-natpmp additions are valid for native ARM64 macOS builds, but they belong in the core build configuration, not CI setup files. Verify this configuration matches the upstream Bitcoin Core macOS ARM64 CI setup before approving.

src/test/fuzz/string.cpp (1)

7-7: LGTM! Header path updated correctly.

The include path change from <util/url.h> to <common/url.h> is consistent with the broader refactoring to move URL utilities to the common directory.

src/Makefile.am (3)

190-190: LGTM! Header moved to common directory.

The addition of common/url.h to BITCOIN_CORE_H correctly reflects the relocation of URL utilities from the util directory to the common directory.

---

960-963: LGTM! Proper handling of libevent dependency.

The conditional compilation under USE_LIBEVENT correctly:

• Adds EVENT_CFLAGS to libbitcoin_common_a_CPPFLAGS (required because common/url.cpp includes event2/http.h)
• Adds common/url.cpp to libbitcoin_common_a_SOURCES

This ensures the URL implementation is only compiled when libevent is available.

---

1084-1084: LGTM! Correct linkage for dash-cli.

Adding LIBBITCOIN_COMMON to dash_cli_LDADD is necessary because dash-cli uses URL functions that are now part of the common library.

src/common/url.cpp (1)

5-5: LGTM! Self-referential header path updated.

The include path correctly references the new location of the header file in the common directory.

src/test/util/setup_common.cpp (1)

10-10: LGTM! Header path updated correctly.

The include path change from <util/url.h> to <common/url.h> is consistent with the URL utilities relocation.

Note: The cppcheck error about "undefined function-like macro invocation" appears to be a false positive related to conditional compilation directives in the broader codebase.

src/rpc/client.cpp (1)

142-142: LGTM! Parameter conversion added for new RPC.

The addition of the submitpackage RPC parameter conversion entry is correctly formatted and consistent with the existing pattern. This enables proper JSON parsing for the package parameter (index 0) when calling the regtest-only submitpackage RPC.

test/functional/wallet_listsinceblock.py (2)

44-44: LGTM!

The integration of test_op_return into the test run flow is correct and follows the existing pattern of other test method invocations.

---

403-415: LGTM!

The new test_op_return method correctly tests that OP_RETURN transactions don't include an address field in the listsinceblock response, which aligns with the RPC changes making the address field optional for outputs without addresses (e.g., OP_RETURN null data).

Note: Static analysis suggests using next(...) instead of the list comprehension with [0], but this is backported code matching upstream Bitcoin Core style.

test/functional/wallet_listtransactions.py (2)

107-107: LGTM!

Correctly integrates the OP_RETURN test into the test flow after the parameter validity tests.

---

170-179: LGTM!

The test_op_return method correctly verifies that OP_RETURN outputs don't include an address field in listtransactions results. This provides complementary coverage to the parallel test in wallet_listsinceblock.py.

Note: Static analysis suggests using next(...) instead of the list comprehension with [0], but this is backported code matching upstream Bitcoin Core style.

src/wallet/rpc/transactions.cpp (3)

460-461: LGTM!

The address field is correctly marked as optional=true with updated documentation explaining it's omitted for move transactions or outputs without addresses (e.g., OP_RETURN null data). This matches the functional test coverage added in wallet_listtransactions.py.

---

575-575: LGTM!

The address field is correctly marked as optional=true with updated documentation for listsinceblock, consistent with the listtransactions changes.

---

850-852: LGTM!

Good documentation update for rescanblockchain that informs users about the faster descriptor wallet rescan capability when block filters are available via -blockfilterindex=1.

src/interfaces/chain.h (3)

8-8: LGTM!

Required include for BlockFilterType and GCSFilter::ElementSet types used in the new interface methods.

---

143-148: LGTM!

Well-designed Chain interface extensions for block filter support:

• hasBlockFilterIndex provides a simple availability check
• blockFilterMatchesAny returns std::optional<bool> which correctly handles three states: filter unavailable (nullopt), no match (false), or match (true)

These methods enable the fast descriptor wallet rescan feature.

---

158-158: LGTM!

Minor formatting adjustment.

src/node/interfaces.cpp (2)

9-9: LGTM!

Required includes for the block filter index implementation - blockfilter.h for BlockFilter and BlockFilterType, and index/blockfilterindex.h for GetBlockFilterIndex() and BlockFilterIndex.

Also applies to: 22-22

---

1072-1085: LGTM!

Solid implementation of the Chain interface methods:

• hasBlockFilterIndex: Simple and correct null check
• blockFilterMatchesAny: Properly handles all failure cases by returning std::nullopt (missing index, missing block, failed filter lookup), and returns the actual match result otherwise

The locking pattern with WITH_LOCK(::cs_main, ...) for the block index lookup is correct.

src/rpc/mempool.cpp (5)

8-8: LGTM!

Required include for Params().IsMockableChain() used in the regtest-only check for submitpackage.

---

240-240: LGTM!

Minor documentation refinement for the vsize field description.

---

769-860: LGTM!

The submitpackage RPC implementation is well-structured:

• Correctly restricted to regtest via Params().IsMockableChain()
• Proper input validation for package size limits
• Comprehensive error handling for all PackageValidationResult cases
• Correct use of ProcessNewPackage with test_accept=false for actual submission

The help text appropriately warns that this is experimental and that individual transaction broadcast may not propagate packages effectively until package relay is implemented.

---

871-894: LGTM!

The result construction correctly builds the response with transaction results including txid, size, and fees for valid/mempool-entry transactions, plus optional package-feerate when available.

---

908-908: LGTM!

Correctly registers submitpackage as a hidden command, appropriate for a regtest-only experimental RPC.

test/functional/wallet_fast_rescan.py (3)

1-21: LGTM!

Good test file structure with clear constants defining the test parameters. The docstring accurately describes the test purpose - verifying that fast rescan using block filters correctly handles top-ups and finds the same transactions as the slow variant.

---

53-99: LGTM!

Excellent test coverage for fast rescan functionality:

• Tests four scenarios: fast/slow × backup restore/non-active descriptors
• Uses assert_debug_log to verify the correct rescan variant is used
• Validates that all rescan methods find exactly NUM_DESCRIPTORS * NUM_BLOCKS transactions
• Confirms fast and slow variants produce identical sorted transaction lists

The test correctly exercises the top-up mechanism by sending to end-range addresses.

---

100-102: LGTM!

Standard test entry point.

src/wallet/wallet.cpp (1)

9-10: Fast rescan filter wiring and block-filter–based ScanForWalletTransactions look correct

The new FastWalletRescanFilter class and its usage in ScanForWalletTransactions are consistent and self‑contained:

• Filter construction only for non‑legacy wallets when a BASIC_FILTER index is available, matching the intended descriptor‑wallet optimization.
• Descriptor SPK managers are located via GetAllScriptPubKeyMans() / GetScriptPubKeyMan(id) and safely downcast; non‑HD managers are handled once at construction without range tracking.
• Per‑block logic cleanly gates fetch_block based on blockFilterMatchesAny and updates last_scanned_block/height whether the block was actually read or safely skipped.
• Reorg handling, failure reporting (last_failed_block / ScanResult::FAILURE), and progress saving (WriteBestBlock via locator) remain correct and are only triggered after blocks are confirmed active.
• AttachChain still invokes ScanForWalletTransactions in the same way, so existing rescan semantics are preserved, with the filter path as a pure speedup.

This matches the upstream design and doesn’t appear to affect Dash‑specific wallet behavior.

Also applies to: 285-341, 1886-2011, 3414-3424

test/functional/mempool_package_limits.py (1)

9-15: Decorator and MiniWallet-based package limit tests look consistent

The new check_package_limits decorator plus MiniWallet-based transaction generation cleanly restructures these tests:

• Each subtest starts with an empty mempool, creates the intended mempool+package topology, then confirms testmempoolaccept fails with package-mempool-limits before mining and re-checking successful acceptance.
• MiniWallet is initialized with sufficient mature UTXOs and is only used to send the mempool transactions; returned hex payloads are exclusively used as the package under test.
• The refactors preserve the previous scenarios (chains, A/V/Y-shaped graphs, bushy trees, size-based cases) while centralizing the common assertions.

No functional issues stand out here; behavior should match the upstream tests once MiniWallet’s _bulk_tx is correct.

If you haven’t already, it’s worth running test/functional/mempool_package_limits.py after these changes to confirm the decorator and MiniWallet integration behave as expected.

Also applies to: 24-38, 46-63, 64-77, 95-132, 134-171, 173-210, 212-246, 248-277, 279-311, 313-345

test/functional/test_framework/wallet.py (1)

29-32: MiniWallet padding and chain helpers are wired correctly; please verify _bulk_tx sizing in your test run

• sign_tx now iterates over tx.vin rather than by index and still assigns fixed-size scriptSigs per input for RAW modes; behaviour is equivalent and simpler.
• create_self_transfer_multi / create_self_transfer properly propagate target_weight and return a "fee" field consistent with inputs_value_total - outputs_value_total.
• _bulk_tx pads transactions using an OP_RETURN output and asserts the final tx.get_weight() falls within a small window above target_weight, which is then exercised by mempool size–limit tests.
• New create_self_transfer_chain and send_self_transfer_chain helpers correctly build linear dependency chains and are used by the updated functional tests.

Given the interplay between target_weight and CTransaction.get_weight(), it’s worth re-running the size‑sensitive tests (notably mempool_package_limits.py) to ensure the padding assertions in _bulk_tx hold with Dash’s test framework implementation. No code changes are needed here if the behaviour matches upstream.

Also applies to: 42-47, 95-106, 145-163, 251-255, 257-307, 312-333, 339-364

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Unused variable num_submitted always reports 0 in error message.

The variable num_submitted is declared inside the loop at line 862, initialized to 0, and never incremented. The error message will always report "0 transactions were broadcast successfully" even if multiple transactions were successfully broadcast before one fails.

🤖 Prompt for AI Agents

In @src/rpc/mempool.cpp around lines 861 - 870, The variable num_submitted is
declared inside the loop in the transaction broadcast block and never
incremented, so the error message always reports 0; move the declaration of
num_submitted outside the for loop (so it accumulates across iterations),
increment it whenever BroadcastTransaction(node, tx, ...) returns
TransactionError::OK, and then use that accumulated num_submitted in the
JSONRPCTransactionError message; update references around BroadcastTransaction
and the surrounding loop to ensure num_submitted reflects the count of
successfully broadcast transactions.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Fix is vs == when matching UTXO txids to the parent transaction

utxo['txid'] is parent_transaction is using identity comparison on Python strings. This is unreliable and can cause children not to be associated with their parent correctly, breaking the descendant/spentby checks.

Use equality instead:

Proposed fix

-            if utxo['txid'] is parent_transaction:
+            if utxo['txid'] == parent_transaction:
                 tx_children.append(txid)

📝 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.

Suggested change

	tx_children = []
	# First create one parent tx with 10 children
	(txid, sent_value) = chain_transaction(self.nodes[0], [txid], [vout], value, fee, 10)
	parent_transaction = txid
	for i in range(10):
	transaction_package.append({'txid': txid, 'vout': i, 'amount': sent_value})
	tx_with_children = self.wallet.send_self_transfer_multi(from_node=self.nodes[0], num_outputs=10)
	parent_transaction = tx_with_children["txid"]
	transaction_package = tx_with_children["new_utxos"]
	# Sign and send up to MAX_DESCENDANT transactions chained off the parent tx
	chain = [] # save sent txs for the purpose of checking node1's mempool later (see below)
	for _ in range(DEFAULT_DESCENDANT_LIMIT - 1):
	utxo = transaction_package.pop(0)
	(txid, sent_value) = chain_transaction(self.nodes[0], [utxo['txid']], [utxo['vout']], utxo['amount'], fee, 10)
	new_tx = self.wallet.send_self_transfer_multi(from_node=self.nodes[0], num_outputs=10, utxos_to_spend=[utxo])
	txid = new_tx["txid"]
	chain.append(txid)
	if utxo['txid'] is parent_transaction:
	tx_children.append(txid)
	for j in range(10):
	transaction_package.append({'txid': txid, 'vout': j, 'amount': sent_value})
	transaction_package.extend(new_tx["new_utxos"])
	tx_children = []
	# First create one parent tx with 10 children
	tx_with_children = self.wallet.send_self_transfer_multi(from_node=self.nodes[0], num_outputs=10)
	parent_transaction = tx_with_children["txid"]
	transaction_package = tx_with_children["new_utxos"]
	# Sign and send up to MAX_DESCENDANT transactions chained off the parent tx
	chain = [] # save sent txs for the purpose of checking node1's mempool later (see below)
	for _ in range(DEFAULT_DESCENDANT_LIMIT - 1):
	utxo = transaction_package.pop(0)
	new_tx = self.wallet.send_self_transfer_multi(from_node=self.nodes[0], num_outputs=10, utxos_to_spend=[utxo])
	txid = new_tx["txid"]
	chain.append(txid)
	if utxo['txid'] == parent_transaction:
	tx_children.append(txid)
	transaction_package.extend(new_tx["new_utxos"])

🤖 Prompt for AI Agents

In @test/functional/mempool_packages.py around lines 223 - 239, The loop that
builds chained child transactions incorrectly uses identity comparison when
checking utxo['txid'] is parent_transaction, which can miss matches; update that
conditional in the for-loop (where utxo, parent_transaction, tx_children, chain
and transaction_package are used) to use equality (==) instead of is so string
txid comparisons correctly detect children and populate tx_children for
downstream descendant/spentby checks.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Use txid strings instead of tx.hash for submitpackage results and broadcast waits

In test_submit_child_with_parents:

• submitpackage_result["tx-results"] is keyed by txid strings, not the internal integer tx.hash.
• P2PTxInvStore.wait_for_broadcast expects hex txid strings and currently receives integer hashes.

This will cause incorrect lookups and can raise type errors. Index and wait using txid fields instead:

Proposed fix

-        for package_txn in package_txns:
-            tx = package_txn["tx"]
-            assert tx.hash in submitpackage_result["tx-results"]
-            tx_result = submitpackage_result["tx-results"][tx.hash]
+        for package_txn in package_txns:
+            tx = package_txn["tx"]
+            assert package_txn["txid"] in submitpackage_result["tx-results"]
+            tx_result = submitpackage_result["tx-results"][package_txn["txid"]]
@@
-        self.bump_mocktime(30)
-        peer.wait_for_broadcast([tx["tx"].hash for tx in package_txns])
+        self.bump_mocktime(30)
+        peer.wait_for_broadcast([tx["txid"] for tx in package_txns])

🧰 Tools

🪛 Ruff (0.14.10)

245-245: Unnecessary key check before dictionary access

Replace with dict.get

(RUF019)

---

260-260: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)

🤖 Prompt for AI Agents

In @test/functional/rpc_packages.py around lines 234 - 293, The test uses
internal integer tx.hash values where submitpackage_result and P2PTxInvStore
expect hex txid strings; update all lookups and waits in
test_submit_child_with_parents to use the package_txn["txid"] string (e.g., when
asserting membership in submitpackage_result["tx-results"], when fetching
tx_result, and when calling peer.wait_for_broadcast) instead of tx.hash; ensure
any places that compare or index by txid use the same string key from
package_txn["txid"] so submitpackage_result lookups and broadcast waits use the
correct hex txid type.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Align CPFP submitpackage test with txid-based keys and broadcasts

In test_submit_cpfp:

• The child result is indexed with tx_child["tx"].hash, but tx-results is keyed by txid strings.
• peer.wait_for_broadcast is again called with integer hashes.

To be consistent with the parent lookups and the P2PTxInvStore API, this should use txid strings:

Proposed fix

-        rich_parent_result = submitpackage_result["tx-results"][tx_rich["txid"]]
-        poor_parent_result = submitpackage_result["tx-results"][tx_poor["txid"]]
-        child_result = submitpackage_result["tx-results"][tx_child["tx"].hash]
+        rich_parent_result = submitpackage_result["tx-results"][tx_rich["txid"]]
+        poor_parent_result = submitpackage_result["tx-results"][tx_poor["txid"]]
+        child_result = submitpackage_result["tx-results"][tx_child["txid"]]
@@
-        self.bump_mocktime(30)
-        peer.wait_for_broadcast([tx["tx"].hash for tx in package_txns])
+        self.bump_mocktime(30)
+        peer.wait_for_broadcast([tx["txid"] for tx in package_txns])

This keeps all indexing and broadcast checks consistently based on txid strings.

🤖 Prompt for AI Agents

In @test/functional/rpc_packages.py around lines 295 - 323, In test_submit_cpfp,
the child transaction is being referenced by tx_child["tx"].hash and
peer.wait_for_broadcast is given integer hashes, but
submitpackage_result["tx-results"] and P2PTxInvStore expect txid strings; change
the child lookup to use tx_child["txid"] (matching
rich_parent_result/poor_parent_result) and call peer.wait_for_broadcast with the
list [tx["txid"] for tx in package_txns] so both the submitpackage_result access
and the broadcast check consistently use txid strings.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Remove extraneous f prefix from string literal.

This f-string has no placeholders, as flagged by static analysis.

🧰 Tools

🪛 Flake8 (7.3.0)

[error] 52-52: f-string is missing placeholders

(F541)

🪛 Ruff (0.14.10)

52-52: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 Prompt for AI Agents

In @test/functional/wallet_fast_rescan.py at line 52, The log call uses an
unnecessary f-string prefix in the message inside the self.log.info call; remove
the leading 'f' so the string becomes a normal literal in the self.log.info(...)
invocation (locate the call to self.log.info that currently reads f"Create txs
sending to end range address of each descriptor, triggering top-ups" and change
it to a plain string without the f prefix).