fix: make clang-tidy happy

[UdjinM6]

Issue being fixed or feature implemented

clang-tidy complains in develop https://github.com/dashpay/dash/actions/runs/19766627745/job/56641081222

#6980 follow-up

What was done?

Apply missing changes

How Has This Been Tested?

https://github.com/UdjinM6/dash/actions/runs/19767082677/job/56642583627

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]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[coderabbitai]

Walkthrough

Two files were modified to optimize memory usage by replacing value copies with const references. In src/external_signer.cpp, a range-based loop over result.getValues() now iterates by const reference, and a fingerprint string extraction now binds to a const reference instead of creating a copy. In src/wallet/wallet.cpp, a local variable holding a descriptor string was changed from a non-const std::string to a const reference. The underlying logic remains unchanged across both files.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Verify const reference bindings are safe and don't extend beyond the scope of temporary objects
• Confirm the referenced strings remain valid for the duration of their usage in each scope
• Check that the optimization doesn't inadvertently change behavior in edge cases (e.g., if get_str() returns a temporary or if getValStr() has side effects)

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

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.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly indicates the PR addresses clang-tidy warnings, which aligns with the specific code changes optimizing variable references and reducing unnecessary copies.
Description check	✅ Passed	The description directly relates to the changeset by referencing a clang-tidy issue, linking to a follow-up PR, and explaining that missing changes are being applied to resolve warnings.

✨ 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 7ba3f42 and 851697f.

📒 Files selected for processing (2)

• src/external_signer.cpp (2 hunks)
• src/wallet/wallet.cpp (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/wallet/wallet.cpp
• src/external_signer.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

Wallet implementation must use Berkeley DB and SQLite

Files:

• src/wallet/wallet.cpp

🧠 Learnings (4)

📓 Common learnings

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

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

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

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

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

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

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

Applied to files:

• src/wallet/wallet.cpp

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

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

Applied to files:

• src/wallet/wallet.cpp

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

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

Applied to files:

• src/wallet/wallet.cpp

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

• GitHub Check: linux64_multiprocess-build / Build source
• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: x86_64-apple-darwin / Build depends
• GitHub Check: x86_64-pc-linux-gnu_nowallet / Build depends
• GitHub Check: x86_64-w64-mingw32 / Build depends
• GitHub Check: x86_64-pc-linux-gnu / Build depends
• GitHub Check: arm-linux-gnueabihf / Build depends

🔇 Additional comments (2)

src/wallet/wallet.cpp (1)

3875-3887: Avoiding descriptor string copy looks safe and idiomatic

Binding desc_str as a const std::string& to desc_val.getValStr() within the loop avoids an extra copy while keeping the lifetime safely tied to the owning UniValue. Usage is local to the loop body, so there’s no dangling risk. Semantics remain unchanged.

src/external_signer.cpp (1)

31-58: Const-reference usage here cleanly removes copies without changing behavior

Iterating result.getValues() by const UniValue& and binding fingerprintStr as a const std::string& both eliminate avoidable allocations/copies while keeping references valid for the loop scope. The enumerate logic (error handling, duplicate filtering, signer construction) is unchanged.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

• Provide your own instructions using the high_level_summary_instructions setting.
• Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
• Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
4. 📊 Contributor Summary — Include a Markdown table showing contributions:
   | Contributor | Lines Added | Lines Removed | Files Changed |
5. ✔️ Additional Notes — Add any extra reviewer context.
   Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

---

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

[knst]

utACK 851697f