backport: _to-insert-list_ external signer

[knst]

Blocked by #6019

What was done?

Extra backports to improve support of External signer
This backports probably are not mandatory to make it works, but it improvements for CI, tests, and user experience.

How Has This Been Tested?

N/A

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:

• feat(wallet): external signer (hardware signer) #6019 - feat(wallet): external signer (hardware signer)

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

[coderabbitai]

Walkthrough

This pull request adds external signer support to Dash Core. It replaces Boost.Process configuration with an --enable-external-signer option, introduces an ExternalSigner class and ExternalSignerScriptPubKeyMan, integrates external signing into wallet descriptor setup and PSBT flows, adds RPCs (enumeratesigners, walletdisplayaddress), exposes UI controls for external signer configuration and wallet creation, and includes mocks and functional tests for valid/invalid/multi-signer scenarios. Numerous build, docs, and CI updates enable and advertise the feature.

Sequence Diagram(s)

mermaid
sequenceDiagram
autonumber
participant UI as Qt UI
participant Node as Dash Node
participant Wallet as Wallet
participant Signer as External Signer (script)
Note over UI,Node: Wallet creation with external signer
UI->>Node: createwallet(external_signer=true)
Node->>Wallet: create with WALLET_FLAG_EXTERNAL_SIGNER
Wallet->>Signer: ExternalSigner::Enumerate("--signer")
Signer-->>Wallet: [fingerprint,name] (JSON)
Wallet-->>UI: creation result/status

mermaid
sequenceDiagram
autonumber
participant UI as Send Dialog
participant Wallet as Wallet
participant Signer as External Signer (script)
participant RPC as RunCommandParseJSON
Note over UI,Wallet: PSBT-based external signing flow
UI->>Wallet: create PSBT (sign=false)
Wallet->>UI: PSBT
UI->>Signer: signtransaction --stdin --fingerprint (PSBT)
Signer->>RPC: execute signer command
RPC-->>Signer: returns JSON {psbt: ""}
Signer-->>UI: signed PSBT
UI->>Wallet: finalize and broadcast (if complete)
Wallet-->>Network: broadcast tx

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check	❓ Inconclusive	The PR title "backport: to-insert-list external signer" attempts to convey the main purpose (external signer support backports), which aligns with the changeset that is entirely focused on external signer functionality across build, test, wallet, and UI components. However, the presence of "to-insert-list" appears to be placeholder or incomplete text rather than a meaningful descriptor, making the title unclear and suggesting unfinished authoring. The actual subject matter (external signer) is clearly related to the changes, but the vague placeholder element undermines the clarity and specificity expected of a PR title.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	The PR description clearly states that the changes consist of "extra backports to improve support of External signer" and identifies them as improvements for CI, tests, and user experience. This description directly relates to the changeset, which comprises comprehensive external signer support enhancements spanning configure scripts, build system, core implementation, wallet integration, Qt GUI, and test infrastructure. Although the description is relatively high-level and does not enumerate every modification, it meaningfully communicates the purpose and scope of the changes.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

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

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 08ca6d1 and a142e5b.

📒 Files selected for processing (3)

• src/script/interpreter.h (1 hunks)
• src/script/sign.cpp (1 hunks)
• src/script/sign.h (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

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/script/interpreter.h
• src/script/sign.h
• src/script/sign.cpp

🧬 Code graph analysis (1)

src/script/sign.h (1)

src/script/sign.cpp (1)

• DUMMY_CHECKER (364-364)

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

🔇 Additional comments (6)

src/script/interpreter.h (1)

195-198: Verified: const-correctness improvement is safe and correct.

All call sites are compatible. The only instantiation found (in src/script/sign.cpp:227) passes a non-const BaseSignatureChecker&, which safely binds to the new const BaseSignatureChecker& parameter. This is a standard C++ conversion that maintains backward compatibility while improving const-correctness.

src/script/sign.h (1)

53-54: LGTM!

The extern declaration and comment are clear. This properly exposes DUMMY_CHECKER with the same pattern used for the other dummy creators, maintaining consistency across the API.

src/script/sign.cpp (4)

346-353: LGTM!

The DummySignatureChecker is correctly placed in an anonymous namespace and implements the expected dummy behavior (accepting all signatures). The use of final and override keywords follows modern C++ best practices.

---

355-355: LGTM!

The const reference bound to a temporary is a valid C++ pattern. The temporary's lifetime is extended to static storage duration, ensuring it lives for the entire program. This approach maintains polymorphism while avoiding heap allocation.

---

357-382: LGTM!

The DummySignatureCreator correctly constructs valid DER-encoded signatures. The encoding structure is accurate:

• Produces 71-byte signatures for (32, 32) configuration
• Produces 72-byte signatures for (33, 32) configuration

The implementation properly handles the DER SEQUENCE structure with INTEGER components for r and s values, and appends the sighash byte.

---

384-385: LGTM!

Both signature creator definitions follow the same valid const-reference-to-temporary pattern. The configurations produce the expected signature sizes:

• DUMMY_SIGNATURE_CREATOR (32, 32) → 71 bytes
• DUMMY_MAXIMUM_SIGNATURE_CREATOR (33, 32) → 72 bytes

---

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

test/functional/wallet_abandonconflict.py (1)

139-139: Use wallet RPC handle for consistency.

This line uses self.nodes[0].listtransactions() instead of alice.listtransactions(), which is inconsistent with the refactoring pattern applied throughout the rest of the file. While functionally equivalent (both access the default wallet), using the alice handle maintains consistency with the PR's objective to switch all wallet operations to wallet RPC handles.

Apply this diff:

-        txAB1_listtransactions = [d for d in self.nodes[0].listtransactions() if d['txid'] == txAB1]
+        txAB1_listtransactions = [d for d in alice.listtransactions() if d['txid'] == txAB1]

src/wallet/scriptpubkeyman.cpp (1)

1624-1649: Avoid pushing invalid entries when pool read fails.

Only append to result when ReadPool succeeds; otherwise you may return default-initialized CKeyPool.

-    while (it != std::end(*setKeyPool)) {
+    while (it != std::end(*setKeyPool)) {
         const int64_t& index = *(it);
         if (index > keypool_id) break; // set*KeyPool is ordered

         CKeyPool keypool;
-        if (batch.ReadPool(index, keypool)) { //TODO: This should be unnecessary
-            m_pool_key_to_index.erase(keypool.vchPubKey.GetID());
-        }
+        if (batch.ReadPool(index, keypool)) { // TODO: Should be unnecessary
+            m_pool_key_to_index.erase(keypool.vchPubKey.GetID());
+        }
         batch.ErasePool(index);
         WalletLogPrintf("keypool index %d removed\n", index);
         it = setKeyPool->erase(it);
-        result.push_back(std::move(keypool));
+        if (keypool.vchPubKey.IsValid()) {
+            result.push_back(std::move(keypool));
+        }
     }

src/qt/sendcoinsdialog.cpp (1)

796-809: External signer balance is always shown as 0.

You change the label but never assign a balance for this branch. Show watch-only (typical for external signer wallets) or regular balance.

-        if (model->wallet().hasExternalSigner()) {
-            ui->labelBalanceName->setText(tr("External balance:"));
+        if (model->wallet().hasExternalSigner()) {
+            balance = balances.watch_only_balance;
+            ui->labelBalanceName->setText(tr("External balance:"));

🧹 Nitpick comments (35)

test/functional/wallet_listtransactions.py (5)

21-24: num_nodes=3: OK, but keep restart args consistent.

Bumping to 3 is justified for cross‑node wallet checks. When you restart later, pass self.extra_args to preserve initial node config.

Apply this small change at the later restart:

-        self.start_nodes()
+        self.start_nodes(extra_args=self.extra_args)

---

113-118: Avoid hard‑coding wallet.dat paths; use RPC‑based cloning.

Directly copying wallet.dat is brittle (backend differences, walletdir layout like “wallets/”, multiwallet). Prefer RPCs:

• dumpwallet on node 0 and importwallet on node 2 (no stop/start needed), or
• backupwallet while running, then copy the backup file.

This removes assumptions about self.chain/default_wallet_name and survives backend changes.

Example replacement:

-        self.nodes[0].keypoolrefill(1000)
-        self.stop_nodes()
-        wallet0 = os.path.join(self.nodes[0].datadir, self.chain, self.default_wallet_name, "wallet.dat")
-        wallet2 = os.path.join(self.nodes[2].datadir, self.chain, self.default_wallet_name, "wallet.dat")
-        shutil.copyfile(wallet0, wallet2)
-        self.start_nodes()
+        self.nodes[0].keypoolrefill(1000)
+        dump_file = os.path.join(self.nodes[0].datadir, "w0_dump.txt")
+        self.nodes[0].dumpwallet(dump_file)
+        # Ensure wallet 2 is fresh and imports all keys/labels from node 0
+        self.nodes[2].importwallet(dump_file)

If you must keep file‑copy semantics, derive the wallet path via RPC or node attributes (e.g., walletdir) rather than constructing it manually.

---

118-123: Reconnect is fine; add sync for determinism.

After restarting/reconnecting, call sync_* to avoid races before subsequent sends/mines and comparisons.

         self.connect_nodes(0, 1)
         self.connect_nodes(1, 2)
         self.connect_nodes(2, 0)
+        self.sync_all()

---

128-140: Add chain syncs after mining to avoid flakiness across nodes.

generate() mines on a single node; peers may lag briefly. Sync before comparing listtransactions to ensure both nodes see the same chain.

         self.nodes[1].sendtoaddress(addr3, "0.001")
         self.generate(self.nodes[1], 1)
+        self.sync_blocks()
@@
         self.nodes[1].sendtoaddress(addr2, "0.001")
         self.generate(self.nodes[1], 1)
+        self.sync_blocks()
@@
         self.nodes[0].sendtoaddress(addr1, "0.001")
         self.generate(self.nodes[0], 1)
+        self.sync_blocks()

---

145-152: Strengthen normalization sort key to avoid duplicate‑txid ordering differences.

listtransactions can emit multiple entries with the same txid (e.g., send‑to‑self). Sorting only by txid may keep per‑txid order different across nodes, causing spurious mismatches. Include category and vout in the sort key.

-        def normalize_list(txs):
+        def normalize_list(txs):
             for tx in txs:
                 tx.pop('label', None)
                 tx.pop('time', None)
                 tx.pop('timereceived', None)
-            txs.sort(key=lambda x: x['txid'])
+            txs.sort(key=lambda x: (x['txid'], x.get('category', ''), x.get('vout', -1)))

src/wallet/init.cpp (1)

70-72: Hide -signer when feature is not compiled for consistent CLI UX.

Mirror existing patterns (e.g., -unsafesqlitesync) to avoid unknown-option noise.

 #ifdef ENABLE_EXTERNAL_SIGNER
-    argsman.AddArg("-signer=<cmd>", "External signing tool, see doc/external-signer.md", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
+    argsman.AddArg("-signer=<cmd>", "External signing tool, see doc/external-signer.md", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
+#else
+    argsman.AddHiddenArgs({"-signer"});
 #endif

src/qt/forms/createwalletdialog.ui (2)

112-121: Text/style polish and availability gating.

• Use title case to match help/category naming: “External Signer”.
• Ensure the checkbox is hidden/disabled when external signer is not compiled or no signers are detected.

-         <string>External signer</string>
+         <string>External Signer</string>

If not already handled in CreateWalletDialog code, I can add the visibility/enablement toggle based on signer enumeration.

---

155-155: Tab order looks fine; consider adding descriptor checkbox too (if missing).

If descriptor_checkbox isn’t already in tabstops elsewhere, add it for keyboard navigation consistency.

src/qt/walletmodel.h (1)

144-144: Consider passing string parameter by const reference.

The method signature takes std::string sAddress by value. For efficiency, consider using const std::string& sAddress to avoid unnecessary string copies, especially since the implementation only needs to read the value.

Apply this diff if you'd like to optimize the parameter passing:

-    bool displayAddress(std::string sAddress);
+    bool displayAddress(const std::string& sAddress);

Also update the implementation in src/qt/walletmodel.cpp accordingly.

src/qt/walletmodel.cpp (2)

582-592: Consider using this as the parent widget for the error dialog.

Passing nullptr as the parent to QMessageBox::critical can lead to suboptimal behavior: the dialog may not be modal to the wallet window, may appear on the wrong screen in multi-monitor setups, and lacks proper window ownership. Consider passing this or another appropriate parent widget instead.

Apply this diff:

-        QMessageBox::critical(nullptr, tr("Can't display address"), e.what());
+        QMessageBox::critical(/* appropriate parent widget */, tr("Can't display address"), e.what());

Note: You may need to refactor this method to accept a parent widget parameter, or use a pattern that ensures proper parent widget handling.

---

582-582: Parameter efficiency: pass by const reference.

The parameter sAddress is passed by value, which creates an unnecessary copy. Consider passing by const std::string& instead.

Apply this diff:

-bool WalletModel::displayAddress(std::string sAddress)
+bool WalletModel::displayAddress(const std::string& sAddress)

src/qt/walletcontroller.cpp (1)

296-306: Consider using a translatable string for the multi-signer error message.

The error message on line 303 uses QString::fromStdString with a hardcoded English string, while other error messages in the same flow (like line 300) use Qt's tr() for translation.

Apply this diff to make the error message translatable and consistent with the rest of the codebase:

     if (signers.size() > 1) {
-        QMessageBox::critical(nullptr, tr("Too many external signers found"), QString::fromStdString("More than one external signer found. Please connect only one at a time."));
+        QMessageBox::critical(nullptr, tr("Too many external signers found"), tr("More than one external signer found. Please connect only one at a time."));
         signers.clear();
     }

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

2041-2047: CoinJoin detection via string search is brittle

Parsing the descriptor string to find "/'/'/4'/0'" is fragile. Prefer a structured check (e.g., store derivation purpose/path metadata in WalletDescriptor or a helper that decodes derivation steps) to avoid false positives if formatting changes or keys are reordered.

src/interfaces/node.h (1)

160-169: Make ExternalSigner interface const-correct and defaulted

getName is an accessor; mark it const. Also default the virtual destructor for clarity.

Apply:

-class ExternalSigner
+class ExternalSigner
 {
 public:
-    virtual ~ExternalSigner() {};
+    virtual ~ExternalSigner() = default;
 
     //! Get signer display name
-    virtual std::string getName() = 0;
+    virtual std::string getName() const = 0;
 };

doc/external-signer.md (1)

1-171: Tighten Markdown (fences, prompts, style) and mention build flag

• Add language identifiers (sh, json).
• Drop leading “$ ” prompts or include sample outputs per MD014.
• Minor grammar: “In order to be” → “To be”, hyphenate “BIP44/49/84-compatible”.
• Document that builds must enable external signer support (configure with --enable-external-signer) if that’s the gating.

Apply sample edits:

-```sh
-$ dashd -signer=../HWI/hwi.py
-```
+```sh
+dashd -signer=../HWI/hwi.py
+```

-```
-$ dash-cli enumeratesigners
+```json
+{
+  "signers": [
+    { "fingerprint": "c8df832a" }
+  ]
+}
+```

-```sh
-$ dash-cli createwallet "hww" true true "" true true true
-```
+```sh
+dash-cli createwallet "hww" true true "" true true true
+```

-``` 
-$ <cmd> enumerate
+```sh
+<cmd> enumerate
 [
   { "fingerprint": "00000000" }
 ]

+Note: build with --enable-external-signer to enable this feature.

Also fix “immedidately” → “immediately” if you keep that note in code comments elsewhere (see createwalletdialog.cpp).

</blockquote></details>
<details>
<summary>src/qt/createwalletdialog.cpp (2)</summary><blockquote>

`49-65`: **External signer toggle: align blank-wallet semantics and fix typo**

- You set blank_wallet_checkbox to checked when toggled on (Line 64), implying watch-only wallets should be “blank”. In setSigners() you later force it unchecked (see below), which is inconsistent. Decide the intended default and make both paths consistent.
- Typo in comment: “immedidately” → “immediately”.



Would you like me to update both spots to a single policy (e.g., blank=false because descriptors are imported immediately)?

---

`115-134`: **setSigners(): minor polish and consistency**

- Consistency: As noted above, this forces blank_wallet_checkbox unchecked even though the “toggled” path sets it checked. Pick one behavior.
- UX: When no signers are found (else branch), consider re-enabling the other checkboxes back to their defaults to avoid a partially locked UI if setSigners() is called after prior toggling.
- API: You call signers[0]->getName(); with the proposed const qualifier, this still works.


```diff
-        ui->blank_wallet_checkbox->setEnabled(false);
-        ui->blank_wallet_checkbox->setChecked(false);
+        ui->blank_wallet_checkbox->setEnabled(false);
+        ui->blank_wallet_checkbox->setChecked(true); // or false, but keep consistent with the toggle handler
@@
     } else {
         ui->external_signer_checkbox->setEnabled(false);
+        // Optionally restore defaults:
+        ui->encrypt_wallet_checkbox->setEnabled(true);
+        ui->blank_wallet_checkbox->setEnabled(true);
+        ui->disable_privkeys_checkbox->setEnabled(true);
+        ui->descriptor_checkbox->setEnabled(true);
     }

If you prefer blank=false, update the toggle handler accordingly.

src/node/interfaces.cpp (3)

530-551: Make listExternalSigners resilient (catch enumerate errors, guard empty cmd).

Avoid throwing across the Node interface. Catch exceptions from ExternalSigner::Enumerate and return an empty list; also use command.empty() for clarity.

Apply:

     std::vector<std::unique_ptr<interfaces::ExternalSigner>> listExternalSigners() override
     {
 #ifdef ENABLE_EXTERNAL_SIGNER
-        std::vector<ExternalSigner> signers = {};
-        const std::string command = gArgs.GetArg("-signer", "");
-        if (command == "") return {};
-        ExternalSigner::Enumerate(command, signers, Params().NetworkIDString());
+        std::vector<ExternalSigner> signers;
+        const std::string command = gArgs.GetArg("-signer", "");
+        if (command.empty()) return {};
+        try {
+            ExternalSigner::Enumerate(command, signers, Params().NetworkIDString());
+        } catch (const std::exception& e) {
+            // Log and degrade gracefully
+            LogPrintf("listExternalSigners: enumerate failed: %s\n", e.what());
+            return {};
+        }
         std::vector<std::unique_ptr<interfaces::ExternalSigner>> result;
-        for (auto& signer : signers) {
+        result.reserve(signers.size());
+        for (auto& signer : signers) {
             result.emplace_back(std::make_unique<ExternalSignerImpl>(std::move(signer)));
         }
         return result;
 #else

---

17-17: Optional: include only when enabled.

Wrap the header include with ENABLE_EXTERNAL_SIGNER to avoid unnecessary coupling when the feature is off.

-#include <external_signer.h>
+#ifdef ENABLE_EXTERNAL_SIGNER
+#include <external_signer.h>
+#endif

---

402-411: Minor: keep adaptor minimal.

Consider storing just the name instead of copying the whole ::ExternalSigner, unless future methods are planned.

-class ExternalSignerImpl : public interfaces::ExternalSigner
-{
-public:
-    ExternalSignerImpl(::ExternalSigner signer) : m_signer(std::move(signer)) {}
-    std::string getName() override { return m_signer.m_name; }
-private:
-    ::ExternalSigner m_signer;
-};
+class ExternalSignerImpl : public interfaces::ExternalSigner {
+public:
+    explicit ExternalSignerImpl(std::string name) : m_name(std::move(name)) {}
+    std::string getName() override { return m_name; }
+private:
+    std::string m_name;
+};

And in the loop:

- result.emplace_back(std::make_unique<ExternalSignerImpl>(std::move(signer)));
+ result.emplace_back(std::make_unique<ExternalSignerImpl>(std::move(signer.m_name)));

src/external_signer.h (1)

27-35: API polish for performance and const-correctness.

• Remove top-level const on by-value return.
• Pass chain by const reference in ctor.
• Make GetDescriptors const.

-    const std::string NetworkArg() const;
+    std::string NetworkArg() const;
@@
-    ExternalSigner(const std::string& command, const std::string chain, const std::string& fingerprint, const std::string name);
+    ExternalSigner(const std::string& command, const std::string& chain, const std::string& fingerprint, const std::string& name);
@@
-    UniValue GetDescriptors(const int account);
+    UniValue GetDescriptors(int account) const;

Also update implementations:

-const std::string ExternalSigner::NetworkArg() const
+std::string ExternalSigner::NetworkArg() const
 {
     return " --chain " + m_chain;
 }
@@
-ExternalSigner::ExternalSigner(const std::string& command, const std::string chain, const std::string& fingerprint, const std::string name)
+ExternalSigner::ExternalSigner(const std::string& command, const std::string& chain, const std::string& fingerprint, const std::string& name)
   : m_command(command), m_chain(chain), m_fingerprint(fingerprint), m_name(name) {}
@@
-UniValue ExternalSigner::GetDescriptors(const int account)
+UniValue ExternalSigner::GetDescriptors(int account) const

Also applies to: 54-59

src/wallet/external_signer_scriptpubkeyman.cpp (1)

64-86: Use wallet logging instead of stderr on signing failure.

Direct writes to stderr bypass logging. Prefer LogPrintf/WalletLogPrintf.

-    if(!GetExternalSigner().SignTransaction(psbt, strFailReason)) {
-        tfm::format(std::cerr, "Failed to sign: %s\n", strFailReason);
+    if (!GetExternalSigner().SignTransaction(psbt, strFailReason)) {
+        LogPrintf("%s: external signer failed: %s\n", __func__, strFailReason);
         return TransactionError::EXTERNAL_SIGNER_FAILED;
     }

Add:

+#include <logging.h>

near the includes.

test/functional/rpc_signer.py (2)

51-54: Fix Flake8 E128 under-indented continuations.

Reformat the four multi-line assert_raises_rpc_error calls.

-        assert_raises_rpc_error(-1, 'Error: restart dashd with -signer=<cmd>',
-            self.nodes[0].enumeratesigners
-        )
+        assert_raises_rpc_error(
+            -1,
+            'Error: restart dashd with -signer=<cmd>',
+            self.nodes[0].enumeratesigners,
+        )

-        assert_raises_rpc_error(-1, 'execve failed: No such file or directory',
-            self.nodes[3].enumeratesigners
-        )
+        assert_raises_rpc_error(
+            -1,
+            'execve failed: No such file or directory',
+            self.nodes[3].enumeratesigners,
+        )

-        assert_raises_rpc_error(-1, 'RunCommandParseJSON error',
-            self.nodes[1].enumeratesigners
-        )
+        assert_raises_rpc_error(
+            -1,
+            'RunCommandParseJSON error',
+            self.nodes[1].enumeratesigners,
+        )

-        assert_raises_rpc_error(-1, 'fingerprint not found',
-            self.nodes[1].enumeratesigners
-        )
+        assert_raises_rpc_error(
+            -1,
+            'fingerprint not found',
+            self.nodes[1].enumeratesigners,
+        )

Also applies to: 56-58, 62-64, 68-70

---

55-58: Harden missing-signer assertion across OSes.

The exact message “execve failed: No such file or directory” is Linux-specific and will differ on Windows/macOS. Match a less brittle substring or accept multiple options (e.g., “No such file or directory” or “The system cannot find the file specified.”).

src/qt/sendcoinsdialog.cpp (1)

503-518: Optional: sanitize suggested filename for PSBT saves.

Labels/addresses may contain characters invalid for filenames. Consider replacing forbidden characters (/:*?"<>| etc.) or falling back to a safe default.

src/wallet/wallet.h (1)

30-31: Guard external_signer include for non-signer builds.

Protect include behind ENABLE_EXTERNAL_SIGNER to avoid unnecessary coupling.

-#include <external_signer.h>
+#ifdef ENABLE_EXTERNAL_SIGNER
+#include <external_signer.h>
+#endif

test/functional/wallet_signer.py (5)

113-116: Fix flake8 E128/E124: align continuation and closing paren.

-        assert_raises_rpc_error(-1, 'RunCommandParseJSON error',
-            hww.walletdisplayaddress, address1
-        )
+        assert_raises_rpc_error(
+            -1,
+            'RunCommandParseJSON error',
+            hww.walletdisplayaddress,
+            address1,
+        )

---

128-141: Fix flake8 E122: indent the second descriptor dict.

-        result = mock_wallet.importdescriptors([{
+        result = mock_wallet.importdescriptors([
+            {
             "desc": "pkh([00000001/84'/1'/0']tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/0/*)#6k3x80k9",
             "timestamp": 0,
             "range": [0,1],
             "internal": False,
             "active": True
-        },
-        {
+            },
+            {
             "desc": "pkh([00000001/84'/1'/0']tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/*)#tz5866xa",
             "timestamp": 0,
             "range": [0, 0],
             "internal": True,
             "active": True
-        }])
+            },
+        ])

---

179-180: Prefer assert_equal for clearer failures.

-        assert(hww.testmempoolaccept([mock_tx])[0]["allowed"])
+        assert_equal(hww.testmempoolaccept([mock_tx])[0]["allowed"], True)

---

21-26: Make signer path robust on Windows and paths with spaces.
Use sys.executable and quoting to avoid shell issues.

+import sys
+import shlex
...
-        if platform.system() == "Windows":
-            return "py " + path
-        else:
-            return path
+        if platform.system() == "Windows":
+            return f'"{sys.executable}" {shlex.quote(path)}'
+        return path

Please confirm CI on Windows still finds Python via sys.executable.

Also applies to: 29-34, 36-40

---

100-106: Reduce brittleness of exact address assertion.
Hardcoding the full address risks future derivation/encoding changes. Consider asserting validity and keypath properties instead.

-        assert_equal(address3, "yg8pCsowDbY7PbJMthTi4xkkmhNfcNNdHV")
+        assert_equal(address_info['hdkeypath'].startswith("m/44'/1'/0'/0/"), True)
+        assert_equal(address_info['iswitness'], False)

If the exact address is intentionally fixed by the mock, feel free to keep it.

src/wallet/scriptpubkeyman.h (1)

604-608: SetupDescriptor public hook — consider nodiscard.
Return value indicates success; marking [[nodiscard]] would prevent accidental ignore.

-    bool SetupDescriptor(std::unique_ptr<Descriptor>desc);
+    [[nodiscard]] bool SetupDescriptor(std::unique_ptr<Descriptor> desc);

src/wallet/wallet.cpp (3)

3731-3741: Avoid reusing SignatureData across SPKMs.
Stateful CanProvide() may mutate sigdata; reinitialize per iteration.

 std::set<ScriptPubKeyMan*> CWallet::GetScriptPubKeyMans(const CScript& script) const
 {
     std::set<ScriptPubKeyMan*> spk_mans;
-    SignatureData sigdata;
     for (const auto& spk_man_pair : m_spk_managers) {
-        if (spk_man_pair.second->CanProvide(script, sigdata)) {
+        SignatureData sigdata;
+        if (spk_man_pair.second->CanProvide(script, sigdata)) {
             spk_mans.insert(spk_man_pair.second.get());
         }
     }
     return spk_mans;
 }

---

3869-3897: External signer descriptor ingestion — minor hardening.
Consider logging/skipping descriptors with missing/unsupported output types; current continue silently drops them.

-                if (!desc->GetOutputType()) {
-                    continue;
-                }
+                if (!desc->GetOutputType()) {
+                    WalletLogPrintf("%s: skipping descriptor without output type: %s\n", __func__, desc_str);
+                    continue;
+                }

---

3965-3987: Remove unused variable and assert in IsInternalScriptPubKeyMan.
The output type is fetched but not used; trim dead code.

-    const auto& type = desc_spk_man->GetWalletDescriptor().descriptor->GetOutputType();
-    assert(type.has_value());
-
     return GetScriptPubKeyMan(/*internal=*/true) == desc_spk_man;

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e5ce1f0 and 08ca6d1.

📒 Files selected for processing (80)

• ci/dash/build_src.sh (1 hunks)
• ci/test/00_setup_env_arm.sh (1 hunks)
• ci/test/00_setup_env_mac.sh (1 hunks)
• ci/test/00_setup_env_mac_native_x86_64.sh (1 hunks)
• ci/test/00_setup_env_native_asan.sh (1 hunks)
• ci/test/00_setup_env_native_fuzz.sh (1 hunks)
• ci/test/00_setup_env_native_multiprocess.sh (1 hunks)
• ci/test/00_setup_env_native_nowallet.sh (1 hunks)
• ci/test/00_setup_env_native_qt5.sh (1 hunks)
• ci/test/00_setup_env_native_tsan.sh (1 hunks)
• ci/test/00_setup_env_s390x.sh (1 hunks)
• ci/test/00_setup_env_win64.sh (1 hunks)
• configure.ac (8 hunks)
• doc/Doxyfile.in (1 hunks)
• doc/build-openbsd.md (1 hunks)
• doc/build-osx.md (1 hunks)
• doc/dependencies.md (1 hunks)
• doc/external-signer.md (1 hunks)
• src/Makefile.am (5 hunks)
• src/Makefile.test.include (1 hunks)
• src/dummywallet.cpp (1 hunks)
• src/external_signer.cpp (1 hunks)
• src/external_signer.h (1 hunks)
• src/interfaces/node.h (2 hunks)
• src/interfaces/wallet.h (2 hunks)
• src/node/interfaces.cpp (3 hunks)
• src/qt/createwalletdialog.cpp (4 hunks)
• src/qt/createwalletdialog.h (2 hunks)
• src/qt/forms/createwalletdialog.ui (2 hunks)
• src/qt/forms/optionsdialog.ui (1 hunks)
• src/qt/forms/receiverequestdialog.ui (1 hunks)
• src/qt/optionsdialog.cpp (3 hunks)
• src/qt/optionsmodel.cpp (3 hunks)
• src/qt/optionsmodel.h (1 hunks)
• src/qt/receiverequestdialog.cpp (1 hunks)
• src/qt/sendcoinsdialog.cpp (8 hunks)
• src/qt/sendcoinsdialog.h (2 hunks)
• src/qt/walletcontroller.cpp (4 hunks)
• src/qt/walletmodel.cpp (2 hunks)
• src/qt/walletmodel.h (1 hunks)
• src/qt/walletmodeltransaction.cpp (1 hunks)
• src/qt/walletmodeltransaction.h (1 hunks)
• src/rpc/client.cpp (1 hunks)
• src/rpc/external_signer.cpp (1 hunks)
• src/rpc/register.h (2 hunks)
• src/test/system_tests.cpp (3 hunks)
• src/util/error.cpp (1 hunks)
• src/util/error.h (1 hunks)
• src/util/system.cpp (3 hunks)
• src/util/system.h (0 hunks)
• src/wallet/external_signer_scriptpubkeyman.cpp (1 hunks)
• src/wallet/external_signer_scriptpubkeyman.h (1 hunks)
• src/wallet/init.cpp (1 hunks)
• src/wallet/interfaces.cpp (2 hunks)
• src/wallet/rpc/addresses.cpp (2 hunks)
• src/wallet/rpc/backup.cpp (2 hunks)
• src/wallet/rpc/spend.cpp (8 hunks)
• src/wallet/rpc/wallet.cpp (6 hunks)
• src/wallet/scriptpubkeyman.cpp (5 hunks)
• src/wallet/scriptpubkeyman.h (6 hunks)
• src/wallet/spend.cpp (2 hunks)
• src/wallet/wallet.cpp (13 hunks)
• src/wallet/wallet.h (8 hunks)
• src/wallet/walletdb.cpp (1 hunks)
• src/wallet/walletdb.h (1 hunks)
• src/wallet/walletutil.h (1 hunks)
• test/config.ini.in (1 hunks)
• test/functional/mocks/invalid_signer.py (1 hunks)
• test/functional/mocks/multi_signers.py (1 hunks)
• test/functional/mocks/signer.py (1 hunks)
• test/functional/rpc_fundrawtransaction.py (1 hunks)
• test/functional/rpc_help.py (1 hunks)
• test/functional/rpc_signer.py (1 hunks)
• test/functional/test_framework/test_framework.py (1 hunks)
• test/functional/test_framework/test_node.py (1 hunks)
• test/functional/test_runner.py (1 hunks)
• test/functional/wallet_abandonconflict.py (5 hunks)
• test/functional/wallet_listtransactions.py (2 hunks)
• test/functional/wallet_signer.py (1 hunks)
• test/lint/spelling.ignore-words.txt (1 hunks)

💤 Files with no reviewable changes (1)

• src/util/system.h

🧰 Additional context used

📓 Path-based instructions (5)

ci/**

📄 CodeRabbit inference engine (CLAUDE.md)

Unless specifically prompted, avoid making changes to the ci directory (continuous integration)

Files:

• ci/test/00_setup_env_native_asan.sh
• ci/test/00_setup_env_s390x.sh
• ci/test/00_setup_env_win64.sh
• ci/test/00_setup_env_native_multiprocess.sh
• ci/test/00_setup_env_mac_native_x86_64.sh
• ci/test/00_setup_env_arm.sh
• ci/test/00_setup_env_native_nowallet.sh
• ci/test/00_setup_env_mac.sh
• ci/test/00_setup_env_native_fuzz.sh
• ci/test/00_setup_env_native_qt5.sh
• ci/dash/build_src.sh
• ci/test/00_setup_env_native_tsan.sh

doc/**

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• doc/build-openbsd.md
• doc/Doxyfile.in
• doc/external-signer.md
• doc/dependencies.md
• doc/build-osx.md

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/util/error.cpp
• src/qt/optionsmodel.h
• src/qt/createwalletdialog.h
• src/util/system.cpp
• src/wallet/external_signer_scriptpubkeyman.cpp
• src/qt/optionsmodel.cpp
• src/wallet/walletdb.h
• src/qt/sendcoinsdialog.cpp
• src/qt/walletmodel.h
• src/qt/receiverequestdialog.cpp
• src/qt/createwalletdialog.cpp
• src/wallet/rpc/addresses.cpp
• src/rpc/external_signer.cpp
• src/test/system_tests.cpp
• src/qt/sendcoinsdialog.h
• src/wallet/rpc/wallet.cpp
• src/util/error.h
• src/external_signer.h
• src/wallet/wallet.h
• src/qt/optionsdialog.cpp
• src/rpc/register.h
• src/wallet/rpc/backup.cpp
• src/wallet/spend.cpp
• src/qt/walletmodeltransaction.cpp
• src/wallet/scriptpubkeyman.cpp
• src/qt/walletmodeltransaction.h
• src/wallet/walletutil.h
• src/qt/walletcontroller.cpp
• src/wallet/rpc/spend.cpp
• src/interfaces/node.h
• src/wallet/walletdb.cpp
• src/wallet/scriptpubkeyman.h
• src/dummywallet.cpp
• src/interfaces/wallet.h
• src/wallet/init.cpp
• src/external_signer.cpp
• src/wallet/interfaces.cpp
• src/wallet/wallet.cpp
• src/node/interfaces.cpp
• src/rpc/client.cpp
• src/wallet/external_signer_scriptpubkeyman.h
• src/qt/walletmodel.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/test/system_tests.cpp

test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Functional tests should be written in Python and placed in test/functional/

Files:

• test/functional/mocks/invalid_signer.py
• test/functional/wallet_abandonconflict.py
• test/functional/rpc_help.py
• test/functional/rpc_signer.py
• test/functional/test_runner.py
• test/functional/rpc_fundrawtransaction.py
• test/functional/test_framework/test_node.py
• test/functional/mocks/signer.py
• test/functional/test_framework/test_framework.py
• test/functional/wallet_listtransactions.py
• test/functional/wallet_signer.py
• test/functional/mocks/multi_signers.py

🧬 Code graph analysis (39)

src/util/error.cpp (1)

src/util/translation.h (1)

• Untranslated (48-48)

src/qt/createwalletdialog.h (2)

src/interfaces/node.h (2)

• interfaces (48-411)
• ExternalSigner (161-168)

src/qt/createwalletdialog.cpp (12)

• setSigners (115-134)
• setSigners (115-115)
• isEncryptWalletChecked (141-144)
• isEncryptWalletChecked (141-141)
• isDisablePrivateKeysChecked (146-149)
• isDisablePrivateKeysChecked (146-146)
• isDescriptorWalletChecked (156-159)
• isDescriptorWalletChecked (156-156)
• isExternalSignerChecked (161-164)
• isExternalSignerChecked (161-161)
• CreateWalletDialog (17-108)
• CreateWalletDialog (110-113)

src/util/system.cpp (1)

src/node/interfaces.cpp (1)

• ENABLE_EXTERNAL_SIGNER (530-551)

src/wallet/external_signer_scriptpubkeyman.cpp (2)

src/wallet/wallet.cpp (6)

• DisplayAddress (2533-2545)
• DisplayAddress (2533-2533)
• GetSolvingProvider (3751-3755)
• GetSolvingProvider (3751-3751)
• GetSolvingProvider (3757-3765)
• GetSolvingProvider (3757-3757)

src/external_signer.cpp (4)

• Enumerate (24-60)
• Enumerate (24-24)
• DisplayAddress (62-65)
• DisplayAddress (62-62)

src/qt/sendcoinsdialog.cpp (2)

src/qt/guiutil.cpp (4)

• setClipboard (1653-1660)
• setClipboard (1653-1653)
• getSaveFileName (521-559)
• getSaveFileName (521-523)

src/qt/bitcoinunits.cpp (4)

• formatWithUnit (125-128)
• formatWithUnit (125-125)
• format (80-114)
• format (80-80)

src/qt/walletmodel.h (1)

src/qt/walletmodel.cpp (2)

• displayAddress (582-592)
• displayAddress (582-582)

src/qt/createwalletdialog.cpp (2)

src/node/interfaces.cpp (1)

• ENABLE_EXTERNAL_SIGNER (530-551)

src/qt/createwalletdialog.h (1)

• CreateWalletDialog (24-44)

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

src/wallet/interfaces.cpp (18)

• LOCK (249-258)
• LOCK (259-262)
• LOCK (290-294)
• LOCK (295-299)
• LOCK (352-360)
• LOCK (478-490)
• dest (209-213)
• dest (209-209)
• dest (222-225)
• dest (222-222)
• dest (226-229)
• dest (226-226)
• dest (230-248)
• dest (230-233)
• dest (263-267)
• dest (263-263)
• dest (268-272)
• dest (268-268)

src/wallet/scriptpubkeyman.h (1)

• wallet (28-253)

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

• GetWalletForJSONRPCRequest (55-77)
• GetWalletForJSONRPCRequest (55-55)

src/rpc/external_signer.cpp (2)

src/node/interfaces.cpp (1)

• ENABLE_EXTERNAL_SIGNER (530-551)

src/external_signer.cpp (2)

• Enumerate (24-60)
• Enumerate (24-24)

src/test/system_tests.cpp (2)

src/node/interfaces.cpp (1)

• ENABLE_EXTERNAL_SIGNER (530-551)

src/util/system.cpp (2)

• RunCommandParseJSON (1401-1438)
• RunCommandParseJSON (1401-1401)

src/qt/sendcoinsdialog.h (1)

src/qt/sendcoinsdialog.cpp (6)

• presentPSBT (489-533)
• presentPSBT (489-489)
• processSendCoinsReturn (820-859)
• processSendCoinsReturn (820-820)
• signWithExternalSigner (535-561)
• signWithExternalSigner (535-535)

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

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

• walletdisplayaddress (674-712)
• walletdisplayaddress (674-674)

src/node/interfaces.cpp (1)

• ENABLE_EXTERNAL_SIGNER (530-551)

src/external_signer.h (1)

src/external_signer.cpp (11)

• ExternalSigner (17-17)
• NetworkArg (19-22)
• NetworkArg (19-19)
• Enumerate (24-60)
• Enumerate (24-24)
• DisplayAddress (62-65)
• DisplayAddress (62-62)
• GetDescriptors (67-70)
• GetDescriptors (67-67)
• SignTransaction (72-117)
• SignTransaction (72-72)

src/wallet/wallet.h (5)

src/wallet/wallet.cpp (19)

• DisplayAddress (2533-2545)
• DisplayAddress (2533-2533)
• FillPSBT (2060-2109)
• FillPSBT (2060-2060)
• IsMine (1449-1453)
• IsMine (1449-1449)
• IsMine (1455-1459)
• IsMine (1455-1455)
• IsMine (1461-1469)
• IsMine (1461-1461)
• IsMine (1471-1478)
• IsMine (1471-1471)
• IsMine (1480-1491)
• IsMine (1480-1480)
• outpoint (1082-1082)
• GetScriptPubKeyMans (3731-3741)
• GetScriptPubKeyMans (3731-3731)
• IsInternalScriptPubKeyMan (3965-3987)
• IsInternalScriptPubKeyMan (3965-3965)

src/wallet/external_signer_scriptpubkeyman.cpp (4)

• DisplayAddress (53-62)
• DisplayAddress (53-53)
• FillPSBT (65-86)
• FillPSBT (65-65)

src/external_signer.cpp (2)

• DisplayAddress (62-65)
• DisplayAddress (62-62)

src/wallet/interfaces.cpp (21)

• dest (209-213)
• dest (209-209)
• dest (222-225)
• dest (222-222)
• dest (226-229)
• dest (226-226)
• dest (230-248)
• dest (230-233)
• dest (263-267)
• dest (263-263)
• dest (268-272)
• dest (268-268)
• outpoint (397-397)
• outpoint (397-397)
• outpoint (398-398)
• outpoint (398-398)
• script (188-195)
• script (188-188)
• script (204-208)
• script (204-204)
• spk_man (214-221)

src/wallet/scriptpubkeyman.h (5)

• ScriptPubKeyMan (170-233)
• ScriptPubKeyMan (178-178)
• bool (185-185)
• bool (230-230)
• bool (236-236)

src/qt/optionsdialog.cpp (1)

src/node/interfaces.cpp (1)

• ENABLE_EXTERNAL_SIGNER (530-551)

test/functional/mocks/invalid_signer.py (2)

test/functional/mocks/signer.py (3)

• perform_pre_checks (11-18)
• enumerate (20-21)
• getdescriptors (23-37)

test/functional/mocks/multi_signers.py (1)

• enumerate (10-12)

test/functional/wallet_abandonconflict.py (4)

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

• get_wallet_rpc (363-369)

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

• sync_mempools (848-874)
• disconnect_nodes (744-778)

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

• assert_raises_rpc_error (132-148)

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

• abandontransaction (803-842)
• abandontransaction (803-803)
• gettransaction (694-801)
• gettransaction (694-694)
• listsinceblock (554-692)
• listsinceblock (554-554)
• listtransactions (442-552)
• listtransactions (442-442)

src/rpc/register.h (2)

src/rpc/external_signer.cpp (2)

• RegisterSignerRPCCommands (61-69)
• RegisterSignerRPCCommands (61-61)

src/node/interfaces.cpp (1)

• ENABLE_EXTERNAL_SIGNER (530-551)

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

src/wallet/scriptpubkeyman.h (1)

• wallet (28-253)

src/wallet/spend.cpp (2)

src/interfaces/wallet.h (2)

• wallet (39-46)
• wallet (389-389)

src/node/interfaces.cpp (2)

• coins (998-998)
• coins (998-998)

test/functional/rpc_help.py (2)

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

• is_external_signer_compiled (1115-1117)
• is_zmq_compiled (1135-1137)

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

• assert_equal (69-74)

test/functional/rpc_signer.py (4)

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

• skip_if_no_external_signer (1106-1109)

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

• assert_equal (69-74)
• assert_raises_rpc_error (132-148)

test/functional/wallet_signer.py (5)

• mock_signer_path (21-26)
• set_test_params (42-48)
• skip_test_if_missing_module (50-52)
• set_mock_result (54-56)
• clear_mock_result (58-59)

src/rpc/external_signer.cpp (2)

• enumeratesigners (16-59)
• enumeratesigners (16-16)

src/qt/walletmodeltransaction.h (1)

src/qt/walletmodeltransaction.cpp (2)

• setWtx (29-32)
• setWtx (29-29)

test/functional/rpc_fundrawtransaction.py (2)

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

• assert_raises_rpc_error (132-148)

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

• fundrawtransaction (554-644)
• fundrawtransaction (554-554)

src/interfaces/node.h (2)

src/external_signer.cpp (1)

• ExternalSigner (17-17)

src/external_signer.h (1)

• ExternalSigner (18-64)

src/wallet/walletdb.cpp (1)

src/node/interfaces.cpp (1)

• ENABLE_EXTERNAL_SIGNER (530-551)

src/wallet/scriptpubkeyman.h (3)

src/wallet/scriptpubkeyman.cpp (6)

• MarkUnusedAddresses (305-340)
• MarkUnusedAddresses (305-305)
• MarkUnusedAddresses (2019-2045)
• MarkUnusedAddresses (2019-2019)
• MarkReserveKeysAsUsed (1624-1649)
• MarkReserveKeysAsUsed (1624-1624)

src/wallet/walletutil.h (1)

• WalletDescriptor (70-103)

src/wallet/external_signer_scriptpubkeyman.cpp (2)

• SetupDescriptor (17-40)
• SetupDescriptor (17-17)

src/interfaces/wallet.h (2)

src/qt/walletmodel.cpp (2)

• displayAddress (582-592)
• displayAddress (582-582)

src/wallet/interfaces.cpp (12)

• dest (209-213)
• dest (209-209)
• dest (222-225)
• dest (222-222)
• dest (226-229)
• dest (226-226)
• dest (230-248)
• dest (230-233)
• dest (263-267)
• dest (263-263)
• dest (268-272)
• dest (268-268)

src/wallet/init.cpp (2)

src/node/interfaces.cpp (1)

• ENABLE_EXTERNAL_SIGNER (530-551)

src/dummywallet.cpp (1)

• argsman (31-31)

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

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

• createwallet (615-713)
• createwallet (615-615)

src/external_signer.cpp (4)

src/external_signer.h (1)

• ExternalSigner (18-64)

src/util/system.cpp (2)

• RunCommandParseJSON (1401-1438)
• RunCommandParseJSON (1401-1401)

src/wallet/wallet.cpp (6)

• DisplayAddress (2533-2545)
• DisplayAddress (2533-2533)
• SignTransaction (2026-2043)
• SignTransaction (2026-2026)
• SignTransaction (2045-2058)
• SignTransaction (2045-2045)

src/wallet/external_signer_scriptpubkeyman.cpp (2)

• DisplayAddress (53-62)
• DisplayAddress (53-53)

test/functional/mocks/signer.py (2)

test/functional/mocks/invalid_signer.py (3)

• perform_pre_checks (11-18)
• enumerate (20-21)
• getdescriptors (23-39)

test/functional/mocks/multi_signers.py (1)

• enumerate (10-12)

src/wallet/wallet.cpp (5)

src/wallet/interfaces.cpp (26)

• txout (463-467)
• txout (463-463)
• txout (473-477)
• txout (473-473)
• dest (209-213)
• dest (209-209)
• dest (222-225)
• dest (222-222)
• dest (226-229)
• dest (226-226)
• dest (230-248)
• dest (230-233)
• dest (263-267)
• dest (263-263)
• dest (268-272)
• dest (268-268)
• script (188-195)
• script (188-188)
• script (204-208)
• script (204-204)
• LOCK (249-258)
• LOCK (259-262)
• LOCK (290-294)
• LOCK (295-299)
• LOCK (352-360)
• LOCK (478-490)

src/wallet/scriptpubkeyman.cpp (9)

• IsMine (165-177)
• IsMine (165-165)
• IsMine (179-183)
• IsMine (179-179)
• IsMine (1828-1835)
• IsMine (1828-1828)
• IsMine (1837-1841)
• IsMine (1837-1837)
• id (1684-1684)

src/wallet/scriptpubkeyman.h (2)

• IsMine (180-180)
• IsMine (181-181)

src/wallet/external_signer_scriptpubkeyman.cpp (4)

• DisplayAddress (53-62)
• DisplayAddress (53-53)
• GetExternalSigner (42-51)
• GetExternalSigner (42-42)

src/external_signer.cpp (2)

• DisplayAddress (62-65)
• DisplayAddress (62-62)

src/node/interfaces.cpp (1)

src/external_signer.cpp (2)

• Enumerate (24-60)
• Enumerate (24-24)

src/wallet/external_signer_scriptpubkeyman.h (4)

src/wallet/scriptpubkeyman.h (6)

• wallet (28-253)
• DescriptorScriptPubKeyMan (528-650)
• WalletStorage (34-246)
• bool (185-185)
• bool (230-230)
• bool (236-236)

src/wallet/external_signer_scriptpubkeyman.cpp (8)

• SetupDescriptor (17-40)
• SetupDescriptor (17-17)
• GetExternalSigner (42-51)
• GetExternalSigner (42-42)
• DisplayAddress (53-62)
• DisplayAddress (53-53)
• FillPSBT (65-86)
• FillPSBT (65-65)

src/wallet/wallet.cpp (4)

• DisplayAddress (2533-2545)
• DisplayAddress (2533-2533)
• FillPSBT (2060-2109)
• FillPSBT (2060-2060)

src/wallet/scriptpubkeyman.cpp (4)

• FillPSBT (743-787)
• FillPSBT (743-743)
• FillPSBT (2314-2381)
• FillPSBT (2314-2314)

test/functional/wallet_listtransactions.py (4)

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

• stop_nodes (684-692)
• start_nodes (660-678)

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

• assert_equal (69-74)
• assert_raises_rpc_error (132-148)

src/wallet/rpc/addresses.cpp (6)

• keypoolrefill (263-305)
• keypoolrefill (263-263)
• getnewaddress (17-56)
• getnewaddress (17-17)
• getaddressinfo (418-563)
• getaddressinfo (418-418)

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

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

test/functional/wallet_signer.py (6)

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

• set_test_params (415-417)
• set_test_params (1473-1475)
• skip_test_if_missing_module (423-425)
• skip_test_if_missing_module (1477-1478)
• skip_if_no_external_signer (1106-1109)
• run_test (494-496)
• run_test (1480-1482)
• restart_node (694-697)
• is_bdb_compiled (1147-1149)

test/functional/rpc_signer.py (6)

• mock_signer_path (21-26)
• set_test_params (28-36)
• skip_test_if_missing_module (38-39)
• set_mock_result (41-43)
• clear_mock_result (45-46)
• run_test (48-73)

src/wallet/rpc/wallet.cpp (14)

• res (591-591)
• createwallet (615-713)
• createwallet (615-615)
• getwalletinfo (152-276)
• getwalletinfo (152-152)
• getnewaddress (1098-1098)
• getaddressinfo (1097-1097)
• result (308-308)
• result (766-766)
• result (1088-1088)
• walletdisplayaddress (1108-1108)
• walletcreatefundedpsbt (1151-1151)
• walletprocesspsbt (1150-1150)
• send (1149-1149)

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

• createwallet (884-889)
• get_wallet_rpc (363-369)

src/wallet/rpc/addresses.cpp (10)

• getnewaddress (17-56)
• getnewaddress (17-17)
• getaddressinfo (418-563)
• getaddressinfo (418-418)
• result (254-254)
• result (707-707)
• walletdisplayaddress (674-712)
• walletdisplayaddress (674-674)
• dest (382-382)
• dest (382-382)

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

• result (636-636)
• result (730-730)
• result (903-903)
• result (998-998)
• result (1127-1127)
• walletcreatefundedpsbt (1009-1134)
• walletcreatefundedpsbt (1009-1009)
• walletprocesspsbt (930-1007)
• walletprocesspsbt (930-930)
• send (737-928)
• send (737-737)

test/functional/mocks/multi_signers.py (2)

test/functional/mocks/invalid_signer.py (1)

• enumerate (20-21)

test/functional/mocks/signer.py (1)

• enumerate (20-21)

src/qt/walletmodel.cpp (1)

src/wallet/interfaces.cpp (28)

• dest (209-213)
• dest (209-209)
• dest (222-225)
• dest (222-222)
• dest (226-229)
• dest (226-226)
• dest (230-248)
• dest (230-233)
• dest (263-267)
• dest (263-263)
• dest (268-272)
• dest (268-268)
• m_wallet (157-160)
• m_wallet (165-165)
• m_wallet (174-174)
• m_wallet (175-175)
• m_wallet (181-181)
• m_wallet (182-182)
• m_wallet (442-445)
• m_wallet (446-449)
• m_wallet (521-521)
• m_wallet (522-522)
• m_wallet (523-523)
• m_wallet (524-524)
• m_wallet (525-525)
• m_wallet (526-526)
• m_wallet (531-531)
• m_wallet (591-591)

🪛 Flake8 (7.3.0)

test/functional/wallet_abandonconflict.py

[error] 79-79: block comment should start with '# '

(E265)

---

[error] 120-120: test for membership should be 'not in'

(E713)

test/functional/rpc_signer.py

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

(E128)

---

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

(E128)

---

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

(E128)

---

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

(E128)

test/functional/mocks/signer.py

[error] 96-96: indentation is not a multiple of 4

(E111)

test/functional/wallet_signer.py

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

(E128)

---

[error] 116-116: closing bracket does not match visual indentation

(E124)

---

[error] 135-135: continuation line missing indentation or outdented

(E122)

test/functional/mocks/multi_signers.py

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

(E128)

🪛 LanguageTool

doc/external-signer.md

[style] ~70-~70: Consider a more concise word here.
Context: ...ue, "txid": } ``` ## Signer API In order to be compatible with Dash Core any signer...

(IN_ORDER_TO_PREMIUM)

---

[grammar] ~167-~167: Use a hyphen to join words.
Context: ... support address types, in a BIP44/49/84 compatible manner. The `walletdisplayad...

(QB_NEW_EN_HYPHEN)

doc/build-osx.md

[grammar] ~87-~87: Ensure spelling is correct
Context: ...descriptor wallets. macOS ships with a useable sqlite package, meaning you don't nee...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🪛 markdownlint-cli2 (0.18.1)

doc/external-signer.md

14-14: Dollar signs used before commands without showing output

(MD014, commands-show-output)

---

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

(MD040, fenced-code-language)

---

40-40: Dollar signs used before commands without showing output

(MD014, commands-show-output)

---

48-48: Dollar signs used before commands without showing output

(MD014, commands-show-output)

---

49-49: Dollar signs used before commands without showing output

(MD014, commands-show-output)

---

59-59: Dollar signs used before commands without showing output

(MD014, commands-show-output)

---

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

(MD040, fenced-code-language)

---

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

(MD040, fenced-code-language)

---

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

(MD040, fenced-code-language)

---

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

(MD040, fenced-code-language)

---

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

(MD040, fenced-code-language)

---

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

(MD040, fenced-code-language)

doc/build-osx.md

83-83: Heading levels should only increment by one level at a time
Expected: h5; Actual: h6

(MD001, heading-increment)

🪛 Ruff (0.14.1)

test/functional/mocks/invalid_signer.py

20-20: Unused function argument: args

(ARG001)

test/functional/wallet_abandonconflict.py

120-120: Test for membership should be not in

Convert to not in

(E713)

test/functional/mocks/signer.py

20-20: Unused function argument: args

(ARG001)

test/functional/test_framework/test_framework.py

1109-1109: Avoid specifying long messages outside the exception class

(TRY003)

test/functional/mocks/multi_signers.py

10-10: Unused function argument: args

(ARG001)

⏰ 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). (1)

• GitHub Check: Build container / Build container

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Fix the malformed configure flag (three dashes instead of two).

The configure flag has three leading dashes (---enable-external-signer) instead of the standard two dashes (--enable-external-signer). This syntax error will cause the configure script to either fail or silently ignore the flag, preventing external signer support from being enabled.

Apply this diff:

-BITCOIN_CONFIG_ALL="---enable-external-signer -disable-dependency-tracking --prefix=$DEPENDS_DIR/$HOST --bindir=$BASE_OUTDIR/bin --libdir=$BASE_OUTDIR/lib"
+BITCOIN_CONFIG_ALL="--enable-external-signer --disable-dependency-tracking --prefix=$DEPENDS_DIR/$HOST --bindir=$BASE_OUTDIR/bin --libdir=$BASE_OUTDIR/lib"

Note: Also fixed -disable-dependency-tracking to use double dashes for consistency.

📝 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

	BITCOIN_CONFIG_ALL="---enable-external-signer -disable-dependency-tracking --prefix=$DEPENDS_DIR/$HOST --bindir=$BASE_OUTDIR/bin --libdir=$BASE_OUTDIR/lib"
	BITCOIN_CONFIG_ALL="--enable-external-signer --disable-dependency-tracking --prefix=$DEPENDS_DIR/$HOST --bindir=$BASE_OUTDIR/bin --libdir=$BASE_OUTDIR/lib"

🤖 Prompt for AI Agents

In ci/dash/build_src.sh around line 36, the configure flags are malformed:
'---enable-external-signer' uses three dashes and '-disable-dependency-tracking'
uses a single dash; update them to the standard GNU-style flags by replacing
'---enable-external-signer' with '--enable-external-signer' and
'-disable-dependency-tracking' with '--disable-dependency-tracking' so the
configure script correctly recognizes both options.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Link probe uses PTHREAD_CFLAGS instead of PTHREAD_LIBS.

This can cause link failures on platforms requiring -lpthread at link time. Use PTHREAD_LIBS for LDFLAGS.

-  dnl Boost 1.73 and older require the following workaround.
-  LDFLAGS="$LDFLAGS $PTHREAD_CFLAGS"
+  dnl Boost 1.73 and older require the following workaround.
+  LDFLAGS="$LDFLAGS $PTHREAD_LIBS"

📝 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

	LDFLAGS="$LDFLAGS $PTHREAD_CFLAGS"
	AC_LINK_IFELSE([AC_LANG_PROGRAM([[
	LDFLAGS="$LDFLAGS $PTHREAD_LIBS"
	AC_LINK_IFELSE([AC_LANG_PROGRAM([[

🤖 Prompt for AI Agents

In configure.ac around lines 1589-1590, LDFLAGS is being appended with
PTHREAD_CFLAGS (compiler flags) instead of PTHREAD_LIBS (linker flags); change
the append to use $PTHREAD_LIBS so the link probe gets the proper linker flags
(e.g., -lpthread) — replace usage of PTHREAD_CFLAGS in the LDFLAGS assignment
with PTHREAD_LIBS and ensure quotes/spacing match the surrounding code.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Avoid shadowing and unnecessary copies in enumerate loop.

Use const references and distinct names for clarity.

-    for (UniValue signer : result.getValues()) {
+    for (const UniValue& entry : result.getValues()) {
@@
-        const UniValue& error = signer.find_value("error");
+        const UniValue& error = entry.find_value("error");
@@
-        const UniValue& fingerprint = signer.find_value("fingerprint");
+        const UniValue& fingerprint = entry.find_value("fingerprint");
@@
-        for (const ExternalSigner& signer : signers) {
-            if (signer.m_fingerprint.compare(fingerprintStr) == 0) duplicate = true;
+        for (const ExternalSigner& existing : signers) {
+            if (existing.m_fingerprint == fingerprintStr) {
+                duplicate = true;
+                break;
+            }
         }
@@
-        const UniValue& model_field = signer.find_value("model");
+        const UniValue& model_field = entry.find_value("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

	for (UniValue signer : result.getValues()) {
	// Check for error
	const UniValue& error = signer.find_value("error");
	if (!error.isNull()) {
	if (!error.isStr()) {
	throw std::runtime_error(strprintf("'%s' error", command));
	}
	throw std::runtime_error(strprintf("'%s' error: %s", command, error.getValStr()));
	}
	// Check if fingerprint is present
	const UniValue& fingerprint = signer.find_value("fingerprint");
	if (fingerprint.isNull()) {
	throw std::runtime_error(strprintf("'%s' received invalid response, missing signer fingerprint", command));
	}
	const std::string fingerprintStr = fingerprint.get_str();
	// Skip duplicate signer
	bool duplicate = false;
	for (const ExternalSigner& signer : signers) {
	if (signer.m_fingerprint.compare(fingerprintStr) == 0) duplicate = true;
	}
	if (duplicate) break;
	std::string name = "";
	const UniValue& model_field = signer.find_value("model");
	if (model_field.isStr() && model_field.getValStr() != "") {
	name += model_field.getValStr();
	}
	signers.push_back(ExternalSigner(command, chain, fingerprintStr, name));
	}
	for (const UniValue& entry : result.getValues()) {
	// Check for error
	const UniValue& error = entry.find_value("error");
	if (!error.isNull()) {
	if (!error.isStr()) {
	throw std::runtime_error(strprintf("'%s' error", command));
	}
	throw std::runtime_error(strprintf("'%s' error: %s", command, error.getValStr()));
	}
	// Check if fingerprint is present
	const UniValue& fingerprint = entry.find_value("fingerprint");
	if (fingerprint.isNull()) {
	throw std::runtime_error(strprintf("'%s' received invalid response, missing signer fingerprint", command));
	}
	const std::string fingerprintStr = fingerprint.get_str();
	// Skip duplicate signer
	bool duplicate = false;
	for (const ExternalSigner& existing : signers) {
	if (existing.m_fingerprint == fingerprintStr) {
	duplicate = true;
	break;
	}
	}
	if (duplicate) break;
	std::string name = "";
	const UniValue& model_field = entry.find_value("model");
	if (model_field.isStr() && model_field.getValStr() != "") {
	name += model_field.getValStr();
	}
	signers.push_back(ExternalSigner(command, chain, fingerprintStr, name));
	}

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Duplicate handling aborts enumeration prematurely.

Using break on duplicate fingerprint stops processing remaining entries. Use continue to skip only the duplicate.

-        if (duplicate) break;
+        if (duplicate) continue;

📝 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

	bool duplicate = false;
	for (const ExternalSigner& signer : signers) {
	if (signer.m_fingerprint.compare(fingerprintStr) == 0) duplicate = true;
	}
	if (duplicate) break;
	std::string name = "";
	bool duplicate = false;
	for (const ExternalSigner& signer : signers) {
	if (signer.m_fingerprint.compare(fingerprintStr) == 0) duplicate = true;
	}
	if (duplicate) continue;
	std::string name = "";

🤖 Prompt for AI Agents

In src/external_signer.cpp around lines 47 to 52, the loop uses break when a
duplicate fingerprint is found which aborts the entire enumeration; change the
control flow to skip only the current entry by replacing the break with continue
so the loop keeps processing remaining signers, ensuring duplicate remains
detection stays the same and no other entries are prematurely skipped.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Don’t include full PSBT in error strings.

The current error leaks base64 PSBT, which is sensitive. Keep the message minimal.

-        error = "Signer fingerprint " + m_fingerprint + " does not match any of the inputs:\n" + EncodeBase64(ssTx.str());
+        error = "Signer fingerprint " + m_fingerprint + " does not match any PSBT input.";

🤖 Prompt for AI Agents

In src/external_signer.cpp around lines 87 to 90, the error string currently
appends the full base64 PSBT (sensitive data); remove the
EncodeBase64(ssTx.str()) portion and replace the message with a minimal,
non-sensitive description (for example: indicate the signer fingerprint
mismatched and optionally include non-sensitive metadata such as the PSBT input
count or PSBT txid/hash if available), then return false as before.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix the indentation on line 96.

Line 96 uses 3 spaces instead of a multiple of 4, violating PEP 8 style guidelines.

Apply this diff:

 if not sys.stdin.isatty():
     buffer = sys.stdin.read()
     if buffer and buffer.rstrip() != "":
-       sys.argv.extend(buffer.rstrip().split(" "))
+        sys.argv.extend(buffer.rstrip().split(" "))

📝 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

	if not sys.stdin.isatty():
	buffer = sys.stdin.read()
	if buffer and buffer.rstrip() != "":
	sys.argv.extend(buffer.rstrip().split(" "))
	if not sys.stdin.isatty():
	buffer = sys.stdin.read()
	if buffer and buffer.rstrip() != "":
	sys.argv.extend(buffer.rstrip().split(" "))

🧰 Tools

🪛 Flake8 (7.3.0)

[error] 96-96: indentation is not a multiple of 4

(E111)

🤖 Prompt for AI Agents

In test/functional/mocks/signer.py around lines 93 to 96, the third line in the
if-block (currently using 3-space indentation) violates PEP 8; re-indent that
line to use a multiple of 4 spaces (align it with the other block lines, i.e.,
replace the 3-space indent with 4 spaces) so the if-block uses consistent
4-space indentation.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Make the category comparison truly order‑insensitive.

Sort titles as well, not just components.

-        assert_equal(titles, sorted(components))
+        assert_equal(sorted(titles), sorted(components))

🤖 Prompt for AI Agents

In test/functional/rpc_help.py around lines 108 to 115, the comparison only
sorts the components list but not titles, which makes the assertion
order-sensitive; update the assertion to compare both lists in a deterministic
order by sorting titles as well (e.g., assert_equal(sorted(titles),
sorted(components))) so the comparison is truly order-insensitive.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix comment formatting.

The comment should have a space after the # symbol per PEP 8.

Apply this diff:

-        #Create a child tx spending AB1 and C
+        # Create a child tx spending AB1 and C

📝 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

	#Create a child tx spending AB1 and C
	# Create a child tx spending AB1 and C

🧰 Tools

🪛 Flake8 (7.3.0)

[error] 79-79: block comment should start with '# '

(E265)

🤖 Prompt for AI Agents

In test/functional/wallet_abandonconflict.py around line 79, the inline comment
"#Create a child tx spending AB1 and C" does not follow PEP 8; add a space after
the '#' so it reads "# Create a child tx spending AB1 and C". Update only the
comment text to include the leading space and keep the rest of the line
unchanged.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Use idiomatic membership test.

Replace not txABC2 in with txABC2 not in for better readability per PEP 8.

Apply this diff:

-        assert not txABC2 in [utxo["txid"] for utxo in alice.listunspent(0)]
+        assert txABC2 not in [utxo["txid"] for utxo in alice.listunspent(0)]

📝 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

	assert not txABC2 in [utxo["txid"] for utxo in alice.listunspent(0)]
	assert txABC2 not in [utxo["txid"] for utxo in alice.listunspent(0)]

🧰 Tools

🪛 Flake8 (7.3.0)

[error] 120-120: test for membership should be 'not in'

(E713)

🪛 Ruff (0.14.1)

120-120: Test for membership should be not in

Convert to not in

(E713)

🤖 Prompt for AI Agents

In test/functional/wallet_abandonconflict.py around line 120, replace the
non-idiomatic membership test "not txABC2 in ..." with the PEP 8 preferred
"txABC2 not in ...": update the assertion to use "txABC2 not in [utxo['txid']
for utxo in alice.listunspent(0)]" so the test reads more clearly and follows
Python style guidelines.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Incorrect RPC usage: Dash getnewaddress does not accept address_type; bech32/p2sh‑segwit are unsupported.

Dash Core getnewaddress only takes an optional label and returns base58 addresses; there is no address_type argument (and Dash does not use bech32). This test will fail with “too many parameters” or invalid type. Update to call getnewaddress with just the label.

-        addr1 = self.nodes[0].getnewaddress("pizza1", 'legacy')
-        addr2 = self.nodes[0].getnewaddress("pizza2", 'p2sh-segwit')
-        addr3 = self.nodes[0].getnewaddress("pizza3", 'bech32')
+        addr1 = self.nodes[0].getnewaddress("pizza1")
+        addr2 = self.nodes[0].getnewaddress("pizza2")
+        addr3 = self.nodes[0].getnewaddress("pizza3")

Evidence: Dash Core wallet RPC docs list only a single label parameter and base58 result; no address_type is documented. Also, the provided src/wallet/rpc/addresses.cpp snippet shows a single parameter “label”. (docs.dash.org)

---

🏁 Script executed:

#!/bin/bash
# Verify Dash's RPC signature in-tree to avoid relying on assumptions.
rg -nP 'RPCHelpMan\s+getnewaddress\s*\(\)' -n -C3 --no-heading src | sed -n '1,120p'

Length of output: 1266

---

I need to see the full getnewaddress() function implementation to verify the parameter signature.

---

🏁 Script executed:

```shell
#!/bin/bash
# Extract the full getnewaddress() function definition to see all parameters
sed -n '17,100p' src/wallet/rpc/addresses.cpp

Length of output: 3069

---

Fix incorrect getnewaddress RPC calls: remove address_type parameter.

The Dash RPC getnewaddress() function definition (src/wallet/rpc/addresses.cpp:17-56) accepts only a single optional label parameter. The test code incorrectly passes a second address_type argument ('legacy', 'p2sh-segwit', 'bech32'), which Dash does not support and will cause the RPC call to fail.

-        addr1 = self.nodes[0].getnewaddress("pizza1", 'legacy')
-        addr2 = self.nodes[0].getnewaddress("pizza2", 'p2sh-segwit')
-        addr3 = self.nodes[0].getnewaddress("pizza3", 'bech32')
+        addr1 = self.nodes[0].getnewaddress("pizza1")
+        addr2 = self.nodes[0].getnewaddress("pizza2")
+        addr3 = self.nodes[0].getnewaddress("pizza3")

📝 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

	addr1 = self.nodes[0].getnewaddress("pizza1", 'legacy')
	addr2 = self.nodes[0].getnewaddress("pizza2", 'p2sh-segwit')
	addr3 = self.nodes[0].getnewaddress("pizza3", 'bech32')
	addr1 = self.nodes[0].getnewaddress("pizza1")
	addr2 = self.nodes[0].getnewaddress("pizza2")
	addr3 = self.nodes[0].getnewaddress("pizza3")

🤖 Prompt for AI Agents

In test/functional/wallet_listtransactions.py around lines 124 to 126, the calls
to getnewaddress pass an unsupported second address_type argument ('legacy',
'p2sh-segwit', 'bech32'); Dash's getnewaddress RPC only accepts an optional
label. Remove the second argument from each call so each invocation is just
getnewaddress("label") (e.g., getnewaddress("pizza1")), leaving label usage
intact and avoiding the invalid parameter that causes the RPC to fail.