backport: Merge bitcoin#228164, (partial) 28291, 28164, 27888, 28168, (partial) gui#771, gui#742

[vijaydasmp]

Bitcoin backports

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[vijaydasmp]

Hello @knst @UdjinM6 @PastaPastaPasta , requesting review

[coderabbitai]

Walkthrough

Adds CConnman::GetMappedAS and uses it to populate mapped AS fields in node stats and connection logs; tightens Qt GuiMain command-line token validation and exposes BITCOIN_IPC_PREFIX; changes PSBTOperationsDialog::renderTransaction return type from std::string to QString and updates UI usage; consolidates RPC JSON parsing by removing ParseNonRFCJSONValue and adding a unified Parse helper; updates fuzz and unit tests to use UniValue::read; introduces StrFormatInternalBug and STR_INTERNAL_BUG macro; adds an internal fee-consistency check in wallet spending logic; adjusts multiple tests and adds a Qt translation lint script.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.38% 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 explicitly references multiple Bitcoin PR numbers being backported (#228164, 28291, 28164, 27888, 28168) and GUI PRs (gui#771, gui#742), which directly matches the changeset containing networking, RPC, wallet, and Qt GUI modifications across multiple systems.
Description check	✅ Passed	The description 'Bitcoin backports' is directly related to the changeset, which contains multiple backports from Bitcoin Core as evidenced by the PR title and the nature of changes across net, RPC, wallet, and utility modules.

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

test/lint/lint-qt-translation.py (1)

13-19: LGTM! The linting logic is correct.

The script correctly uses git grep to find translatable strings with leading whitespace. The pattern tr("[[:space:]] appropriately matches the intended cases, and the error reporting is clear.

Regarding the static analysis hint (S607): This is a false positive. Using git from PATH is standard practice for development/CI linting scripts, and there's no security risk in this context.

Optional improvement: You could add basic error handling for cases where git is unavailable, though this is not critical for a linting script that runs in CI:

try:
    result = subprocess.run(['git', 'grep', '-e', 'tr("[[:space:]]', '--', 'src/qt'], 
                          stdout=subprocess.PIPE, text=True, check=False)
    tr_strings = result.stdout
except FileNotFoundError:
    print("Error: git command not found")
    sys.exit(1)

src/net_processing.cpp (1)

3954-3957: Consider moving mapped_as suffix to the end of the “New peer connected” log

Right now the format string is "New %s %s %s peer connected: ..." and the first %s is the ", mapped_as=%d" suffix, so logs read like:

New , mapped_as=123 outbound full-relay IPv4 peer connected: ...

which is slightly odd and differs from the upstream layout where mapped_as is appended at the end of the line.

If you want the log to stay closer to upstream and be more readable, you could reorder the arguments and tweak the format string like this:

-            const auto mapped_as{m_connman.GetMappedAS(pfrom.addr)};
-            LogPrintf("New %s %s %s peer connected: version: %d, blocks=%d, peer=%d%s\n",
-                      (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : ""),
-                      pfrom.ConnectionTypeAsString(),
-                      TransportTypeAsString(pfrom.m_transport->GetInfo().transport_type),
-                      pfrom.nVersion.load(), peer->m_starting_height,
-                      pfrom.GetId(), (fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToStringAddrPort()) : ""));
+            const auto mapped_as{m_connman.GetMappedAS(pfrom.addr)};
+            LogPrintf("New %s %s peer connected: version: %d, blocks=%d, peer=%d%s%s\n",
+                      pfrom.ConnectionTypeAsString(),
+                      TransportTypeAsString(pfrom.m_transport->GetInfo().transport_type),
+                      pfrom.nVersion.load(), peer->m_starting_height,
+                      pfrom.GetId(),
+                      (fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToStringAddrPort()) : ""),
+                      (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : ""));

This is purely cosmetic, so fine to defer if you’d prefer an exact backport. Based on learnings, this would better mirror the upstream log shape without changing behavior.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1c3afb and 0342c026adacb07e123b134e6c9de7fd0f7761fe.

📒 Files selected for processing (19)

• src/net.cpp (2 hunks)
• src/net.h (1 hunks)
• src/net_processing.cpp (2 hunks)
• src/qt/bitcoin.cpp (1 hunks)
• src/qt/paymentserver.h (1 hunks)
• src/qt/psbtoperationsdialog.cpp (3 hunks)
• src/qt/psbtoperationsdialog.h (2 hunks)
• src/qt/sendcoinsdialog.cpp (1 hunks)
• src/rpc/client.cpp (3 hunks)
• src/rpc/client.h (0 hunks)
• src/test/fuzz/parse_univalue.cpp (1 hunks)
• src/test/fuzz/string.cpp (0 hunks)
• src/test/rpc_tests.cpp (1 hunks)
• src/univalue/test/object.cpp (1 hunks)
• src/util/check.cpp (1 hunks)
• src/util/check.h (1 hunks)
• test/functional/rpc_fundrawtransaction.py (0 hunks)
• test/functional/rpc_misc.py (1 hunks)
• test/lint/lint-qt-translation.py (1 hunks)

💤 Files with no reviewable changes (3)

• test/functional/rpc_fundrawtransaction.py
• src/rpc/client.h
• src/test/fuzz/string.cpp

🧰 Additional context used

📓 Path-based instructions (5)

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/net_processing.cpp
• src/qt/sendcoinsdialog.cpp
• src/net.cpp
• src/test/rpc_tests.cpp
• src/qt/paymentserver.h
• src/qt/bitcoin.cpp
• src/qt/psbtoperationsdialog.h
• src/util/check.h
• src/rpc/client.cpp
• src/qt/psbtoperationsdialog.cpp
• src/univalue/test/object.cpp
• src/util/check.cpp
• src/test/fuzz/parse_univalue.cpp
• src/net.h

test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• test/functional/rpc_misc.py

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

📄 CodeRabbit inference engine (CLAUDE.md)

GUI implementation in src/qt/ must use Qt 5

Files:

• src/qt/sendcoinsdialog.cpp
• src/qt/paymentserver.h
• src/qt/bitcoin.cpp
• src/qt/psbtoperationsdialog.h
• src/qt/psbtoperationsdialog.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/test/rpc_tests.cpp
• src/test/fuzz/parse_univalue.cpp

src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue,crypto/{ctaes,x11}}/**

📄 CodeRabbit inference engine (CLAUDE.md)

Do not make changes to vendored dependencies: src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue} or src/crypto/{ctaes,x11}

Files:

• src/univalue/test/object.cpp

🧠 Learnings (6)

📓 Common learnings

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

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

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

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

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

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

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

Learnt from: 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-06-09T16:43:20.996Z

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

Applied to files:

• test/functional/rpc_misc.py

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

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

Applied to files:

• test/functional/rpc_misc.py
• src/qt/sendcoinsdialog.cpp
• src/test/rpc_tests.cpp
• src/qt/bitcoin.cpp
• src/rpc/client.cpp
• src/univalue/test/object.cpp
• src/test/fuzz/parse_univalue.cpp

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

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

Applied to files:

• src/test/rpc_tests.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/test/rpc_tests.cpp

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

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

Applied to files:

• src/test/fuzz/parse_univalue.cpp

🧬 Code graph analysis (6)

src/test/rpc_tests.cpp (2)

src/rpc/util.cpp (2)

• AmountFromValue (85-95)
• AmountFromValue (85-85)

src/bitcoin-tx.cpp (2)

• AmountFromValue (510-520)
• AmountFromValue (510-510)

src/qt/bitcoin.cpp (1)

src/qt/paymentserver.cpp (1)

• arg (79-79)

src/qt/psbtoperationsdialog.h (1)

src/qt/psbtoperationsdialog.cpp (2)

• renderTransaction (175-223)
• renderTransaction (175-175)

src/util/check.h (1)

src/util/check.cpp (3)

• StrFormatInternalBug (17-23)
• StrFormatInternalBug (17-17)
• NonFatalCheckError (25-28)

src/util/check.cpp (1)

src/util/check.h (2)

• NonFatalCheckError (15-19)
• NonFatalCheckError (28-29)

src/net.h (1)

src/net.cpp (4)

• GetMappedAS (4462-4465)
• GetMappedAS (4462-4462)
• addr (2884-2884)
• addr (3336-3336)

🪛 Ruff (0.14.7)

test/lint/lint-qt-translation.py

14-14: Starting a process with a partial executable path

(S607)

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

🔇 Additional comments (18)

src/qt/psbtoperationsdialog.h (1)

9-10: Public interface now correctly exposes QString-based renderTransaction

Including <QString> and changing renderTransaction to return QString keeps the header consistent with the implementation and with the Qt UI usage (no redundant std::string conversions anymore). This also aligns with the upstream Bitcoin backport pattern while remaining minimal in scope.
Based on learnings, this is an appropriate, fidelity-preserving backport change.

Also applies to: 48-51

src/qt/psbtoperationsdialog.cpp (1)

171-173: QString-based transaction rendering is consistent and type-safe

Switching updateTransactionDisplay and renderTransaction over to QString (with the local bullet_point helper) removes std::string↔QString churn and keeps all UI text assembly in Qt types. The control flow and fee/amount logic remain unchanged from the prior implementation, so behavior should be preserved while improving integration with the Qt widgets.
Based on learnings, this matches the intended upstream backport without introducing extra deviations.

Also applies to: 175-223

test/lint/lint-qt-translation.py (2)

1-11: LGTM!

The shebang, copyright notice, license reference, and imports are all appropriate for this linting script.

---

22-23: LGTM!

Standard Python main guard is correctly implemented.

test/functional/rpc_misc.py (1)

28-32: LGTM!

The test assertion correctly reflects the updated internal bug message formatting from the backported changes to StrFormatInternalBug in src/util/check.cpp.

src/qt/paymentserver.h (1)

57-58: LGTM!

The new extern declaration for BITCOIN_IPC_PREFIX is properly placed and provides a clean way to expose the IPC prefix constant for use in command-line parsing and IPC-related validation.

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

25-32: LGTM!

The refactoring from exception-driven parsing (ParseNonRFCJSONValue with try/catch) to the boolean-based uv.read() approach is cleaner and aligns with the PR's consolidation of JSON parsing paths. The control flow correctly handles parse failures by setting valid = false and returning an empty UniValue.

src/univalue/test/object.cpp (1)

415-441: LGTM!

Comprehensive test coverage expansion for JSON parsing validation. The tests properly verify:

• Valid inputs with correct value extraction (numbers, booleans, arrays, objects, whitespace handling)
• Invalid inputs are correctly rejected (missing leading zeros, garbage characters, invalid key formats)
• Non-JSON strings like addresses are properly rejected

Based on learnings, the BTC address test strings are retained from upstream backport.

src/qt/sendcoinsdialog.cpp (1)

346-349: LGTM!

The change from string concatenation to tr("%1 from wallet '%2'").arg(...) improves localization support by allowing translators to reorder placeholders for different languages where word order may vary. This follows Qt best practices for translatable strings.

src/rpc/client.cpp (2)

280-286: LGTM! Clean unification of JSON parsing.

The new Parse() helper consolidates JSON parsing with clear error reporting. The implementation correctly throws on invalid JSON with a descriptive error message.

---

315-315: LGTM! Consistent replacement of deprecated parsing function.

Both ArgToUniValue overloads now use the unified Parse() function. The transformation pipeline (MaybeUnquoteString → Parse) remains correct.

Also applies to: 324-324

src/test/rpc_tests.cpp (1)

288-288: LGTM! Appropriate test for stricter JSON parsing.

The test correctly validates that monetary values require a leading zero before the decimal point. This aligns with the unified parsing behavior introduced in src/rpc/client.cpp.

src/util/check.h (1)

13-13: LGTM! Standard internal bug formatting utilities.

The new StrFormatInternalBug() function and STR_INTERNAL_BUG macro provide consistent formatting for internal bug reports. The implementation correctly captures source location information.

Also applies to: 21-21

src/net.h (1)

1486-1486: LGTM! Clean exposure of AS mapping functionality.

The new GetMappedAS() method appropriately exposes the autonomous system mapping capability. The delegation to m_netgroupman.GetMappedAS(addr) is the correct implementation pattern.

src/qt/bitcoin.cpp (1)

555-581: Verify command-line validation logic handles edge cases correctly.

The new validation enforces important security constraints:

1. Non-option tokens are rejected (except BIP-21 URIs when wallet enabled)
2. Options cannot follow BIP-21 payment URIs

The logic appears sound, but please verify:

• Multiple BIP-21 URIs without intervening options are handled correctly
• Empty strings and special characters in argv are handled safely
• Related test coverage exists for these edge cases

src/net.cpp (1)

4462-4479: Encapsulation of AS mapping via GetMappedAS looks correct

The new CConnman::GetMappedAS helper is a simple, const delegation to m_netgroupman and GetNodeStats now uses it instead of touching m_netgroupman directly. This preserves behavior while tightening encapsulation and keeps the call site clean. No issues spotted.

src/util/check.cpp (1)

17-26: New StrFormatInternalBug helper and constructor delegation are sound

The extracted StrFormatInternalBug correctly formats all parameters, and NonFatalCheckError now cleanly delegates to it via the std::runtime_error base ctor. This centralizes internal-bug message formatting without changing behavior in error reporting. Looks good.

src/net_processing.cpp (1)

3914-3918: Mapped AS logging in VERSION handler looks good

Using m_connman.GetMappedAS(pfrom.addr) once and conditionally appending ", mapped_as=%d" to the existing receive version message log is consistent and matches the upstream pattern; no functional risk spotted. Based on learnings, this keeps the backport aligned with the original change.

[knst]

several comments to fix

[knst]

re-order it a bit

            LogPrintf("New %s %s peer connected: version: %d, blocks=%d, peer=%d%s%s\n",
                      pfrom.ConnectionTypeAsString(),
                      TransportTypeAsString(pfrom.m_transport->GetInfo().transport_type),
                      pfrom.nVersion.load(), peer->m_starting_height,
                      pfrom.GetId(), (fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToStringAddrPort()) : ""),
                      (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : ""));

[knst]

should dashify it s/bitcoin-qt/dash-qt/g

[knst]

26611: add this one 59e4354
it has been missing since bitcoin#22686

[knst]

28291: partial because it is depends on bitcoin#28123

[knst]

utACK 55773b22eeb28a9ea6f88422c1b9e70c467f1b50

CI failed

[knst]

CI fails

[vijaydasmp]

Hello @knst @PastaPastaPasta @UdjinM6 requesting review

[UdjinM6]

gui771: missing changes in src/qt/walletcontroller.cpp, should be partial

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d1da8cc and e141f44.

📒 Files selected for processing (20)

• src/net.cpp
• src/net.h
• src/net_processing.cpp
• src/qt/bitcoin.cpp
• src/qt/paymentserver.h
• src/qt/psbtoperationsdialog.cpp
• src/qt/psbtoperationsdialog.h
• src/qt/sendcoinsdialog.cpp
• src/rpc/client.cpp
• src/rpc/client.h
• src/test/fuzz/parse_univalue.cpp
• src/test/fuzz/string.cpp
• src/test/rpc_tests.cpp
• src/univalue/test/object.cpp
• src/util/check.cpp
• src/util/check.h
• src/wallet/spend.cpp
• test/functional/rpc_fundrawtransaction.py
• test/functional/rpc_misc.py
• test/lint/lint-qt-translation.py

💤 Files with no reviewable changes (3)

• src/test/fuzz/string.cpp
• test/functional/rpc_fundrawtransaction.py
• src/rpc/client.h

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

• src/wallet/spend.cpp
• src/rpc/client.cpp
• src/util/check.h
• src/qt/bitcoin.cpp
• src/qt/paymentserver.h
• src/net_processing.cpp
• src/univalue/test/object.cpp
• src/qt/sendcoinsdialog.cpp

🧰 Additional context used

📓 Path-based instructions (4)

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/test/rpc_tests.cpp
• src/net.cpp
• src/qt/psbtoperationsdialog.h
• src/test/fuzz/parse_univalue.cpp
• src/util/check.cpp
• src/net.h
• src/qt/psbtoperationsdialog.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/test/rpc_tests.cpp
• src/test/fuzz/parse_univalue.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

GUI implementation in src/qt/ must use Qt 5

Files:

• src/qt/psbtoperationsdialog.h
• src/qt/psbtoperationsdialog.cpp

test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• test/functional/rpc_misc.py

🧠 Learnings (8)

📓 Common learnings

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

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

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

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

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

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

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

Learnt from: 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.

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

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

📚 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/test/rpc_tests.cpp
• src/test/fuzz/parse_univalue.cpp
• test/functional/rpc_misc.py

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

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

Applied to files:

• src/test/rpc_tests.cpp

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

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

Applied to files:

• src/test/rpc_tests.cpp
• test/functional/rpc_misc.py

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

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

Applied to files:

• src/qt/psbtoperationsdialog.h

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

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

Applied to files:

• src/qt/psbtoperationsdialog.h
• src/net.h

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

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

Applied to files:

• src/test/fuzz/parse_univalue.cpp

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

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

Applied to files:

• src/qt/psbtoperationsdialog.cpp

🧬 Code graph analysis (4)

src/test/rpc_tests.cpp (2)

src/bitcoin-tx.cpp (2)

• AmountFromValue (507-517)
• AmountFromValue (507-507)

src/rpc/util.cpp (2)

• AmountFromValue (85-95)
• AmountFromValue (85-85)

src/qt/psbtoperationsdialog.h (1)

src/qt/psbtoperationsdialog.cpp (2)

• renderTransaction (175-223)
• renderTransaction (175-175)

src/util/check.cpp (1)

src/util/check.h (2)

• NonFatalCheckError (15-19)
• NonFatalCheckError (28-29)

src/net.h (1)

src/net.cpp (4)

• GetMappedAS (4462-4465)
• GetMappedAS (4462-4462)
• addr (2884-2884)
• addr (3336-3336)

🪛 Ruff (0.14.10)

test/lint/lint-qt-translation.py

14-14: Starting a process with a partial executable path

(S607)

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

• GitHub Check: Build slim container / Create multi-arch manifest
• GitHub Check: Build container / Build container (amd64)
• GitHub Check: Build container / Build container (arm64)

🔇 Additional comments (9)

src/util/check.cpp (2)

17-23: LGTM! Clean extraction of internal bug formatting logic.

The new StrFormatInternalBug function centralizes the formatting of internal bug messages. The parameter count matches the format specifiers, and the implementation is straightforward.

---

25-26: LGTM! Constructor delegation is clean.

The NonFatalCheckError constructor now correctly delegates message formatting to StrFormatInternalBug, which improves code reusability.

test/functional/rpc_misc.py (1)

30-30: LGTM! Test expectation updated to match new internal bug message format.

The expected error message correctly reflects the updated formatting from StrFormatInternalBug, which no longer includes extra quotes around the expression.

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

26-28: LGTM! Parsing logic correctly updated.

The transition from exception-based ParseNonRFCJSONValue to boolean-return UniValue::read() is properly implemented. The lambda correctly handles both success (returns parsed uv) and failure (sets valid = false and returns empty UniValue, triggering early exit at line 30-32). The logic is sound and consistent with the unified parsing approach described in the PR objectives.

src/net.h (1)

1462-1464: GetMappedAS accessor looks correct and well‑placed

The new GetMappedAS(const CNetAddr&) const declaration is const‑correct, matches the definition in src/net.cpp, and cleanly exposes existing NetGroupManager functionality through CConnman without altering behavior.

src/net.cpp (1)

4462-4465: Mapped‑AS helper and stats wiring preserve existing behavior

CConnman::GetMappedAS simply forwards to m_netgroupman.GetMappedAS(addr), and GetNodeStats now uses this accessor to fill stats.m_mapped_as; this centralizes netgroup lookups in CConnman without changing semantics or threading characteristics.

Also applies to: 4473-4479

test/lint/lint-qt-translation.py (1)

1-23: Qt translation linter is simple and fit for purpose

The script correctly flags tr("... strings in src/qt that start with whitespace and fails the run when such instances exist. Using subprocess.run(['git', ...]) with git resolved via PATH is acceptable for this internal lint tool, and the Ruff S607 warning can be safely ignored to keep the backport aligned with upstream.

src/qt/psbtoperationsdialog.h (1)

9-10: Switching renderTransaction to QString matches Qt usage and implementation

Including <QString> and changing renderTransaction to return QString aligns the header with the updated implementation, removes the need for intermediate std::string conversions in updateTransactionDisplay, and keeps the change scoped to this dialog’s internal API.

Also applies to: 48-52

src/qt/psbtoperationsdialog.cpp (1)

170-223: QString‑based renderTransaction implementation is coherent and UI‑friendly

Updating updateTransactionDisplay to consume a QString and reworking renderTransaction to build a Qt string (including per‑output lines, wallet “own address” notes, fee/total section, and unsigned‑input warning) keeps all formatting on the Qt side, preserves existing semantics, and adds a useful ownership hint without introducing new edge‑case risks.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Fix compilation error: missing ValueFromString wrapper.

The test passes a string literal ".19e-6" directly to AmountFromValue, but AmountFromValue expects a const UniValue& parameter. Line 287 shows the correct pattern using ValueFromString() to wrap the string.

🔎 Proposed fix

-    BOOST_CHECK_EXCEPTION(AmountFromValue(".19e-6"), UniValue, HasJSON(R"({"code":-3,"message":"Invalid amount"})")); //should fail, no leading 0
+    BOOST_CHECK_EXCEPTION(AmountFromValue(ValueFromString(".19e-6")), UniValue, HasJSON(R"({"code":-3,"message":"Invalid amount"})")); //should fail, no leading 0

🤖 Prompt for AI Agents

In src/test/rpc_tests.cpp around line 288, the test passes the string literal
".19e-6" directly to AmountFromValue which expects a const UniValue&; wrap the
literal with ValueFromString(".19e-6") so the call becomes
AmountFromValue(ValueFromString(".19e-6")) to match the pattern used on the
previous line and fix the compilation error.

[UdjinM6]

LGTM e141f44, pls rebase to fix CI