backport: bitcoin#19426, #24652, #25078, #25090, #25095, #25178, bitcoin-core/gui#577, bitcoin-core/gui#589, bitcoin-core/gui#593

[knst]

What was done?

Regular backports from Bitcoin Core v24; see commits

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 (for repository code-owners and collaborators only)

[github-actions]

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

• backport: bitcoin#22235, #23635, #23662, #24147, #24152, #24538, #24786, #24796, #24906, #25043, #25067 #6871 - backport: bitcoin#22235, #23635, #23662, #24147, #24152, #24538, #24786, #24796, #24906, #25043, #25067

Please coordinate with the authors of these PRs to avoid merge conflicts.

[coderabbitai]

Walkthrough

Documentation updated: CONTRIBUTING governance wording and README phrasing; doc/build-openbsd.md rewritten for OpenBSD 7.1 with expanded GUI and descriptor-wallet instructions and consolidated dependency steps. Core signing API changed: MutableTransactionSignatureCreator and related constructors now take a CMutableTransaction by reference instead of by pointer, with corresponding call-site updates across signing, RPC, bitcoin-tx, psbt, tests, and fuzz. Qt GUI changes: AddressBook filter made Qt5/Qt6-aware using regular-expression APIs, coin control tristate flag switched to ItemIsAutoTristate, a few Qt includes added, and RPCConsole refactored to use a member RPCExecutor with queued invocation and adjusted signals/slots. gettxout now uses uint32_t for the vout index.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title Check	⚠️ Warning	The title enumerates many upstream PR numbers and fails to concisely summarize the primary change as a backport of Bitcoin Core v24 updates. Listing individual PR references adds unnecessary noise rather than communicating the content of the backported changes. This reduces readability and undermines quick comprehension of the PR’s purpose.	Please shorten the title to a concise summary such as “Backport Bitcoin Core v24 changes” so that it clearly conveys the PR’s purpose without listing every upstream PR number.
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	The description clearly states that this PR contains regular backports from Bitcoin Core v24 and explains that unit and functional tests were run, making it directly relevant to the changeset.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

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: 5

🧹 Nitpick comments (2)

doc/dash-conf.md (1)

66-74: Consider adding language identifier to code block.

The documentation clearly explains how to generate and use the dash.conf file. However, the code block on lines 71-74 should specify bash as the language identifier for better syntax highlighting and accessibility.

Apply this diff:

-```
+```bash
 # example copy command for linux user
 cp share/examples/dash.conf ~/.dashcore

As per coding guidelines.

</blockquote></details>
<details>
<summary>contrib/devtools/gen-dash-conf.sh (1)</summary><blockquote>

`1-80`: **LGTM with observation about complexity.**

The script effectively generates a dash.conf by parsing `dashd --help` output. The overall structure is sound:
- Proper validation of the binary
- Clear warning for dirty builds
- Well-formatted output with header and footer

However, the sed/awk pipeline (lines 49-60) is quite complex with multiple chained transformations. While this appears functionally correct, consider adding inline comments explaining each transformation step for future maintainability, especially:
- Line 54: Prefix conversion
- Line 56: Boolean option detection
- Line 57: Option grouping logic




To improve maintainability, consider documenting the pipeline:

```diff
 # parse the output from dashd --help
 # adding newlines is a bit funky to ensure portability for BSD
 # see here for more details: https://stackoverflow.com/a/24575385
 ${DASHD} --help \
-    | sed '1,/Print this help message and exit/d' \
-    | sed -E 's/^[[:space:]]{2}\-/#/' \
-    | sed -E 's/^[[:space:]]{7}/# /' \
-    | sed -E '/[=[:space:]]/!s/#.*$/&=1/' \
-    | awk '/^#[a-z]/{x=$0;next}{if (NF==0) print x"\n",x="";else print}' \
-    | sed 's,\(^[[:upper:]].*\)\:$,\
+    | sed '1,/Print this help message and exit/d' \    # Remove header lines before options
+    | sed -E 's/^[[:space:]]{2}\-/#/' \                # Comment out option lines
+    | sed -E 's/^[[:space:]]{7}/# /' \                 # Comment out description lines
+    | sed -E '/[=[:space:]]/!s/#.*$/&=1/' \            # Add =1 for boolean options
+    | awk '/^#[a-z]/{x=$0;next}{if (NF==0) print x"\n",x="";else print}' \  # Group options with descriptions
+    | sed 's,\(^[[:upper:]].*\)\:$,\                   # Format section headers
 ### \1,' \
     | sed 's/[[:space:]]*$//' >> "${EXAMPLE_CONF_FILE}"

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d2a6c34 and 9ac4eb6.

📒 Files selected for processing (25)

• CONTRIBUTING.md (2 hunks)
• README.md (1 hunks)
• contrib/debian/examples/dash.conf (1 hunks)
• contrib/devtools/README.md (1 hunks)
• contrib/devtools/gen-dash-conf.sh (1 hunks)
• contrib/guix/libexec/build.sh (1 hunks)
• doc/build-openbsd.md (1 hunks)
• doc/dash-conf.md (1 hunks)
• doc/release-process.md (1 hunks)
• src/bitcoin-tx.cpp (1 hunks)
• src/init.cpp (1 hunks)
• src/psbt.cpp (2 hunks)
• src/qt/addressbookpage.cpp (2 hunks)
• src/qt/bitcoingui.cpp (1 hunks)
• src/qt/coincontroldialog.cpp (1 hunks)
• src/qt/guiutil.cpp (1 hunks)
• src/qt/rpcconsole.cpp (3 hunks)
• src/qt/rpcconsole.h (4 hunks)
• src/qt/test/addressbooktests.cpp (3 hunks)
• src/rpc/blockchain.cpp (1 hunks)
• src/rpc/rawtransaction.cpp (1 hunks)
• src/script/sign.cpp (4 hunks)
• src/script/sign.h (2 hunks)
• src/test/fuzz/script_sign.cpp (1 hunks)
• src/test/script_tests.cpp (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

doc/**

📄 CodeRabbit inference engine (CLAUDE.md)

Unless specifically prompted, avoid making changes to the doc directory (documentation)

Files:

• doc/release-process.md
• doc/dash-conf.md
• doc/build-openbsd.md

contrib/**

📄 CodeRabbit inference engine (CLAUDE.md)

Unless specifically prompted, avoid making changes to the contrib directory (contributed scripts)

Files:

• contrib/guix/libexec/build.sh
• contrib/devtools/README.md
• contrib/devtools/gen-dash-conf.sh
• contrib/debian/examples/dash.conf

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

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction

Files:

• src/init.cpp
• src/qt/guiutil.cpp
• src/qt/coincontroldialog.cpp
• src/qt/rpcconsole.cpp
• src/bitcoin-tx.cpp
• src/psbt.cpp
• src/script/sign.h
• src/qt/bitcoingui.cpp
• src/qt/addressbookpage.cpp
• src/test/script_tests.cpp
• src/test/fuzz/script_sign.cpp
• src/script/sign.cpp
• src/rpc/blockchain.cpp
• src/rpc/rawtransaction.cpp
• src/qt/test/addressbooktests.cpp
• src/qt/rpcconsole.h

src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests

Files:

• src/test/script_tests.cpp
• src/test/fuzz/script_sign.cpp
• src/qt/test/addressbooktests.cpp

🧠 Learnings (1)

📓 Common learnings

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

🧬 Code graph analysis (7)

src/qt/rpcconsole.cpp (1)

src/qt/rpcconsole.h (2)

• message (126-126)
• m_is_executing (204-219)

src/bitcoin-tx.cpp (1)

src/script/sign.cpp (4)

• ProduceSignature (189-218)
• ProduceSignature (189-189)
• MutableTransactionSignatureCreator (20-24)
• MutableTransactionSignatureCreator (26-32)

src/psbt.cpp (1)

src/script/sign.cpp (1)

• creator (327-327)

src/script/sign.h (1)

src/script/sign.cpp (2)

• MutableTransactionSignatureCreator (20-24)
• MutableTransactionSignatureCreator (26-32)

src/test/script_tests.cpp (1)

src/script/sign.cpp (4)

• ProduceSignature (189-218)
• ProduceSignature (189-189)
• MutableTransactionSignatureCreator (20-24)
• MutableTransactionSignatureCreator (26-32)

src/rpc/rawtransaction.cpp (1)

src/script/sign.cpp (4)

• ProduceSignature (189-218)
• ProduceSignature (189-189)
• MutableTransactionSignatureCreator (20-24)
• MutableTransactionSignatureCreator (26-32)

src/qt/rpcconsole.h (2)

src/qt/rpcconsole.cpp (5)

• RPCExecutor (97-99)
• addWallet (823-844)
• addWallet (823-823)
• removeWallet (846-865)
• removeWallet (846-846)

src/qt/bitcoingui.cpp (4)

• addWallet (944-970)
• addWallet (944-944)
• removeWallet (972-990)
• removeWallet (972-972)

🪛 markdownlint-cli2 (0.18.1)

doc/dash-conf.md

71-71: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Lint / Run linters
• GitHub Check: x86_64-pc-linux-gnu_nowallet / Build depends

🔇 Additional comments (23)

src/rpc/blockchain.cpp (1)

1301-1301: LGTM! Type safety improvement.

The change to explicitly use uint32_t and aggregate initialization is correct:

• Vout indices are semantically unsigned 32-bit integers
• Aggregate initialization {} prevents narrowing conversions
• More concise than using an intermediate variable

src/qt/coincontroldialog.cpp (1)

641-641: LGTM! Improved tri-state management.

Changing from Qt::ItemIsTristate to Qt::ItemIsAutoTristate enables Qt to automatically manage the parent item's tri-state based on its children's check states, eliminating the need for manual state synchronization.

contrib/debian/examples/dash.conf (1)

1-1: LGTM!

The placeholder clearly directs users to the generation instructions. This change aligns well with the new dash.conf generation workflow introduced by the companion script contrib/devtools/gen-dash-conf.sh.

src/init.cpp (1)

524-524: LGTM!

The enhanced help text provides better clarity about the -conf option's behavior, specifically that it's read-only, prefixes relative paths with the datadir location, and is only usable from the command line.

CONTRIBUTING.md (2)

13-15: LGTM!

The updated governance language reflects a more distributed maintenance model by removing references to a single lead maintainer and clarifying the role of repository maintainers. The addition of "moderation" to the responsibilities list is also appropriate.

---

274-274: LGTM!

Simplified and clearer language that aligns with the governance updates earlier in the document.

doc/release-process.md (1)

4-6: LGTM!

The branch references are correctly updated from master to develop for consistency with the repository's branch structure.

contrib/guix/libexec/build.sh (1)

358-360: LGTM!

The addition correctly integrates the example dash.conf into the distribution process. The comment appropriately notes that the file may be a stub if gen-dash-conf.sh hasn't been run, which helps maintainers understand the expected state.

README.md (2)

13-17: Updated overview reads well.

Clearer explanation of networking, wallet, and GUI capabilities helps newcomers. Looks good.

---

22-26: Strong summary.

Concise description of Dash’s properties and the role of Dash Core. No issues spotted.

src/bitcoin-tx.cpp (1)

626-633: Signature creator call updated correctly

Passing mergedTx by reference aligns with the new lifetime-bound constructor and keeps the signing path intact. Nicely done.

src/test/script_tests.cpp (1)

1087-1094: Test adapts cleanly to reference-based creator

The test now builds MutableTransactionSignatureCreator with an lvalue tx, matching the ref-based API without altering semantics. Looks good.

src/psbt.cpp (2)

202-212: PSBT output updater matches new lifetime contract

Binding creator to the CMutableTransaction& ensures the checker sees the current tx while remaining within the new API. All good.

---

260-272: Input signing path stays intact

Switching to the reference-based constructor while still threading txdata keeps signature production behavior unchanged.

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

100-119: Fuzz target conforms to ref-based signature creator

The fuzz case now supplies the mutable transaction by reference, so the creator’s internal checker keeps working with live state. Looks good.

src/rpc/rawtransaction.cpp (1)

898-905: RPC combine-path aligned with ref ctor

Using mergedTx directly satisfies the new constructor contract while continuing to sign in-place. No issues spotted.

src/script/sign.h (1)

36-51: Header refactor tightens lifetime guarantees

Storing m_txto as a reference and marking the ctor LIFETIMEBOUND makes the ownership contract explicit without changing behavior. Include addition is spot on.

src/script/sign.cpp (1)

20-44: Implementation mirrors header semantics

Constructors now capture the transaction by reference and thread it through the checker and SignatureHash, preserving signing semantics while enforcing lifetime safety.

src/qt/test/addressbooktests.cpp (5)

26-26: LGTM!

The QLineEdit include is necessary for the new search functionality testing.

---

102-108: LGTM!

The expansion from one to two test addresses improves coverage and is implemented correctly. Using std::ignore for the unused destination is appropriate.

---

159-162: LGTM!

The first address submission correctly uses the new variable and updates expected counts. The "io" label prefix enables later wildcard search testing.

---

164-167: LGTM!

The second address submission is correctly implemented with appropriate assertions for the updated address book state.

---

169-205: LGTM! Comprehensive search testing.

The search functionality tests are thorough and well-structured, covering:

• Exact label matches
• Partial label matches
• Wildcard patterns (? for single character, * for any sequence)
• Full address searches
• Filter clearing

The test logic correctly verifies that only sending addresses appear in the sending tab and that filtering works on both labels and addresses.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Correct the SQLite install command.

Line 39’s pkg_add install sqlite3 will fail because OpenBSD’s pkg_add does not accept the install subcommand; it should be just pkg_add sqlite3. Please adjust to the plain form so the instructions work as written.

-pkg_add install sqlite3
+pkg_add sqlite3

📝 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

	pkg_add install sqlite3
	```
	pkg_add sqlite3

🤖 Prompt for AI Agents

In doc/build-openbsd.md around lines 39 to 40, the install command shown uses an
incorrect subcommand ("pkg_add install sqlite3"); replace it with the correct
OpenBSD usage ("pkg_add sqlite3") so the command will run successfully—update
the line to remove the "install" token and keep the rest of the doc consistent.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Avoid capturing this in queued worker lambda

QMetaObject::invokeMethod queues this lambda onto the worker thread. If the console gets torn down before the event runs, the queued lambda will still dereference this->m_executor, leading to a use-after-free crash. Capture the executor pointer by value (or a QPointer) outside the lambda instead of capturing this.

-    QMetaObject::invokeMethod(m_executor, [this, cmd, wallet_model] {
-        m_executor->request(cmd, wallet_model);
-    });
+    RPCExecutor* const executor = m_executor;
+    if (!executor) {
+        return;
+    }
+    QMetaObject::invokeMethod(executor, [executor, cmd, wallet_model] {
+        executor->request(cmd, wallet_model);
+    });

📝 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

	QMetaObject::invokeMethod(m_executor, [this, cmd, wallet_model] {
	m_executor->request(cmd, wallet_model);
	});
	RPCExecutor* const executor = m_executor;
	if (!executor) {
	return;
	}
	QMetaObject::invokeMethod(executor, [executor, cmd, wallet_model] {
	executor->request(cmd, wallet_model);
	});

🤖 Prompt for AI Agents

In src/qt/rpcconsole.cpp around lines 1196-1199, the queued lambda captures this
and may dereference m_executor after the console is destroyed; instead capture
the executor pointer (or a QPointer) by value outside the lambda and use that
inside the lambda, e.g. store auto executor = m_executor (or QPointer<...>
executor = m_executor) and then call QMetaObject::invokeMethod with a lambda
capturing executor, cmd and wallet_model by value and checking executor for null
before calling executor->request(...).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix typo in comment.

"wilcard" should be "wildcard".

Apply this diff to fix the typo:

-    // Check wilcard "?".
+    // Check wildcard "?".

📝 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

	// Check wilcard "?".
	// Check wildcard "?".

🤖 Prompt for AI Agents

In src/qt/test/addressbooktests.cpp around line 180, there's a comment typo
"wilcard" — change it to "wildcard" so the comment reads "Check wildcard \"?\"."
replacing the misspelled word only.

[knst]

to be fixed by bitcoin#25200 someday, out-of-scope of PR

[coderabbitai]

@knst, understood! I see this aligns with the Dash repository's practice of keeping typos and minor issues in backported code as-is to reduce merge conflicts in future backports from Bitcoin Core. Since bitcoin#25200 will address this upstream, it makes sense to wait for that fix to be backported rather than creating a divergence now.

Thank you for the context!

---

🧠 Learnings used

Learnt from: knst
PR: dashpay/dash#6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.442Z
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
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.

[UdjinM6]

24652: missing gmp

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (2)

doc/build-openbsd.md (1)

39-40: Fix the sqlite install command.

pkg_add install sqlite3 is invalid on OpenBSD; the correct invocation is pkg_add sqlite3. Please update the command so the instructions work as written.

-pkg_add install sqlite3
+pkg_add sqlite3

src/qt/rpcconsole.cpp (1)

1196-1198: Acknowledge existing critical issue: use-after-free risk in queued lambda.

The existing review comment on lines 1196-1199 correctly identifies that capturing this in the queued lambda can lead to a use-after-free crash if the console is destroyed before the lambda executes. The suggested fix—capturing the executor pointer by value outside the lambda and checking for null—is the right approach.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8a087a2 and 30b4e9d.

📒 Files selected for processing (8)

• doc/build-openbsd.md (1 hunks)
• src/qt/addressbookpage.cpp (2 hunks)
• src/qt/bitcoingui.cpp (1 hunks)
• src/qt/coincontroldialog.cpp (1 hunks)
• src/qt/guiutil.cpp (1 hunks)
• src/qt/rpcconsole.cpp (3 hunks)
• src/qt/rpcconsole.h (4 hunks)
• src/qt/test/addressbooktests.cpp (3 hunks)

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

• src/qt/addressbookpage.cpp
• src/qt/bitcoingui.cpp
• src/qt/guiutil.cpp

🧰 Additional context used

📓 Path-based instructions (3)

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

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction

Files:

• src/qt/rpcconsole.h
• src/qt/coincontroldialog.cpp
• src/qt/rpcconsole.cpp
• src/qt/test/addressbooktests.cpp

src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests

Files:

• src/qt/test/addressbooktests.cpp

doc/**

📄 CodeRabbit inference engine (CLAUDE.md)

Unless specifically prompted, avoid making changes to the doc directory (documentation)

Files:

• doc/build-openbsd.md

🧠 Learnings (3)

📓 Common learnings

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

📚 Learning: 2025-10-05T20:38:28.457Z

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

Applied to files:

• src/qt/test/addressbooktests.cpp

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

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

Applied to files:

• src/qt/test/addressbooktests.cpp

🔇 Additional comments (7)

src/qt/coincontroldialog.cpp (1)

641-641: LGTM! Improved tristate checkbox behavior.

The change from Qt::ItemIsTristate to Qt::ItemIsAutoTristate enables Qt's automatic tristate management for parent items in tree mode. This allows parent checkboxes to automatically reflect the state of their children (checked when all children are checked, unchecked when none are, partially checked when some are), eliminating the need for manual state management.

src/qt/rpcconsole.cpp (2)

177-189: LGTM! Wallet context initialization and change handling are correctly implemented.

The unconditional initialization of wallet_model to nullptr at line 177 ensures the variable is always defined, even in non-wallet builds, for use later at line 1196. The wallet context change detection (lines 181-188) appropriately tracks and communicates to the user when the active wallet changes.

---

1240-1261: LGTM! Executor lifecycle management follows Qt best practices.

The executor is correctly created on the heap (line 1240) and moved to the worker thread (line 1241). Signal connections use proper context objects:

• Line 1244: this as context ensures automatic disconnection if RPCConsole is destroyed.
• Line 1253: deleteLater ensures executor cleanup when thread finishes.
• Line 1258: m_executor as context ensures the lambda only runs if the executor is alive.

These patterns correctly prevent use-after-free in the signal/slot connections.

src/qt/rpcconsole.h (4)

8-10: LGTM! Conditional config include follows standard build system practices.

The conditional inclusion of the generated config header is correctly guarded by HAVE_CONFIG_H.

---

25-25: LGTM! Forward declaration reduces compilation dependencies.

Using a forward declaration for RPCExecutor is good practice since the header only needs to declare a pointer member. This avoids unnecessary header inclusion and reduces compilation time.

---

60-63: LGTM! Wallet method correctly guarded by feature flag.

Wrapping addWallet (and removeWallet) in #ifdef ENABLE_WALLET is correct—these methods should only be available when wallet support is compiled in.

---

202-202: LGTM! Member declaration properly initialized.

The m_executor member is correctly declared and initialized to nullptr using brace initialization, following modern C++ practices.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Remove the git clone command from the root-only dependency block.

This block is explicitly meant to be run as root to install packages, but it now includes git clone …, causing users to clone the repo as root and then clone again in Step 2. Drop the clone from Step 1 so only the dependency installs run with elevated privileges while the cloning remains in Step 2.

🤖 Prompt for AI Agents

In doc/build-openbsd.md around lines 13 to 18, the root-only dependency
installation block includes a git clone command which causes the repository to
be cloned as root and duplicated in Step 2; remove the `git clone` line from
this root/elevated-privilege block so it contains only pkg_add package
installation commands, leaving the repository cloning to the separate Step 2 for
non-root execution.

[UdjinM6]

utACK 30b4e9d