feat: better docs for dash specific wallet RPCs supporting null character #6805
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
WalkthroughThis change updates three areas of the codebase. In Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📥 CommitsReviewing files that changed from the base of the PR and between 1873776040533c6a004f19bac92c4eddce219e4e and 960afcf. 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (1)test/functional/**/*.py📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-06-09T16:43:20.996ZApplied to files:
📚 Learning: 2025-02-14T15:19:17.218ZApplied to files:
🧬 Code Graph Analysis (1)test/functional/wallet_upgradewallet.py (2)
⏰ 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). (4)
🔇 Additional comments (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/wallet/rpc/wallet.cpp (1)
492-502: Unify reference style to issue link for consistencyElsewhere you used “bitcoin#27067”; here it says “see bitcoin#27067”. Consider aligning the reference for consistency.
- // Check if the passphrase has a null character (see #27067 for details) + // Check if the passphrase has a null character (see bitcoin#27067 for details)test/functional/wallet_upgradetohd.py (2)
246-247: Avoid brittle equality on user-facing messageSmall future-proofing: assert the prefix rather than full equality so minor wording tweaks don’t break the test.
- assert_equal(ret, "Make sure that you have backup of your mnemonic.") + assert ret.startswith("Make sure that you have backup of your mnemonic.")
256-257: Relax strict equality to resilient assertionsMake the test robust to minor text edits while still validating the key guidance.
- assert_equal(ret, "Make sure that you have backup of your mnemonic. Your mnemonic passphrase contains a null character (ie - a zero byte). If the passphrase was created with a version of this software prior to 23.0, please try again with only the characters up to — but not including — the first null character. If this is successful, please set a new passphrase to avoid this issue in the future.") + assert ret.startswith("Make sure that you have backup of your mnemonic.") + assert "contains a null character" in ret
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between d4202b5 and 1873776040533c6a004f19bac92c4eddce219e4e.
📒 Files selected for processing (3)
src/wallet/rpc/encrypt.cpp(2 hunks)src/wallet/rpc/wallet.cpp(5 hunks)test/functional/wallet_upgradetohd.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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/wallet/rpc/encrypt.cppsrc/wallet/rpc/wallet.cpp
test/functional/**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
Functional tests should be written in Python and placed in test/functional/
Files:
test/functional/wallet_upgradetohd.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: PastaPastaPasta
PR: dashpay/dash#6804
File: src/qt/proposalwizard.cpp:0-0
Timestamp: 2025-08-08T04:30:37.944Z
Learning: In the Dash codebase, direct RPC calling has been removed in recent commits, making suggestions about RPC command validation and error handling for direct RPC calls obsolete. The ProposalWizard and related components no longer use direct RPC calling patterns.
Learnt from: kwvg
PR: dashpay/dash#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: kwvg
PR: dashpay/dash#6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Learnt from: kwvg
PR: dashpay/dash#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: kwvg
PR: dashpay/dash#6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.
Learnt from: UdjinM6
PR: dashpay/dash#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.
📚 Learning: 2025-02-06T14:34:30.466Z
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Applied to files:
src/wallet/rpc/encrypt.cpp
📚 Learning: 2025-02-14T15:19:17.218Z
Learnt from: kwvg
PR: dashpay/dash#6529
File: src/wallet/rpcwallet.cpp:3002-3003
Timestamp: 2025-02-14T15:19:17.218Z
Learning: The `GetWallet()` function calls in `src/wallet/rpcwallet.cpp` are properly validated with null checks that throw appropriate RPC errors, making additional validation unnecessary.
Applied to files:
src/wallet/rpc/encrypt.cpptest/functional/wallet_upgradetohd.pysrc/wallet/rpc/wallet.cpp
📚 Learning: 2025-06-09T16:43:20.996Z
Learnt from: kwvg
PR: dashpay/dash#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/wallet/rpc/encrypt.cpptest/functional/wallet_upgradetohd.py
🧬 Code Graph Analysis (1)
src/wallet/rpc/wallet.cpp (2)
src/wallet/interfaces.cpp (4)
wallet_passphrase(156-159)wallet_passphrase(156-156)wallet_passphrase(162-162)wallet_passphrase(162-162)src/rpc/request.cpp (2)
JSONRPCError(56-62)JSONRPCError(56-56)
⏰ 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). (5)
- GitHub Check: x86_64-w64-mingw32 / Build depends
- GitHub Check: x86_64-pc-linux-gnu_multiprocess / Build depends
- GitHub Check: Lint / Run linters
- GitHub Check: x86_64-apple-darwin / Build depends
- GitHub Check: x86_64-pc-linux-gnu / Build depends
🔇 Additional comments (5)
src/wallet/rpc/encrypt.cpp (2)
83-95: Comment reference clarification LGTMReference updated to “bitcoin#27067” is clear and helpful. No functional changes.
161-171: Comment reference clarification LGTMReference updated to “bitcoin#27067” is clear and helpful. No functional changes.
src/wallet/rpc/wallet.cpp (3)
371-371: Init flag for mnemonic passphrase has-null detection LGTMClear, explicit default.
395-397: Robust handling for mnemonic passphrase containing NULGood defensive check; matches new user guidance.
429-441: Improved error messaging on wallet unlock failures LGTMContextual message for NUL in wallet passphrase is user-friendly and consistent with encrypt RPCs.
src/wallet/rpc/wallet.cpp
Outdated
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
RPC return type changed to string — ensure downstream consumers and docs are updated
Changing RPCResult to STR is a UX improvement, but it’s a breaking return-type change. Please:
- Update user-facing docs/release notes for upgradetohd.
- Verify no internal tooling or external consumers assume a boolean.
To help spot any lingering assumptions about boolean returns, run:
🏁 Script executed:
#!/bin/bash
# Find all usages of upgradetohd across the codebase and show context
rg -n -A2 -B2 '\bupgradetohd\s*\('
# Grep for patterns that might assert boolean truth specifically
rg -n -A2 -B2 'assert.*upgradetohd\(' test/functional | sed 's/^/TEST: /'Length of output: 17217
🏁 Script executed:
#!/bin/bash
rg -n -A20 -B5 "upgradetohd" src/wallet/rpc/wallet.cppLength of output: 8699
🏁 Script executed:
#!/bin/bash
sed -n '165,175p' test/functional/wallet_upgradewallet.pyLength of output: 733
Update upgradetohd RPC tests and documentation for new string return
The upgradetohd RPC now returns a UniValue string (instructions) instead of a boolean. This breaks existing tests and downstream assumptions:
- test/functional/wallet_upgradewallet.py:172
- Change
to something like
assert_equal(wallet.upgradetohd(), True)
result = wallet.upgradetohd() assert isinstance(result, str) and result, f"Unexpected return: {result}"
- Change
- Update user-facing docs and release notes to state that upgradetohd returns an instruction string, not a boolean.
- Grep the repo (and any internal scripts) for
upgradetohd(and replace boolean checks (== True,assert ...) with non-empty string assertions or explicit message checks.
🤖 Prompt for AI Agents
In src/wallet/rpc/wallet.cpp at line 357, the upgradetohd RPC now returns a
string with instructions instead of a boolean. Update the test in
test/functional/wallet_upgradewallet.py at line 172 to check that the result is
a non-empty string rather than asserting it equals True. Also, revise
user-facing documentation and release notes to reflect this change, and search
the codebase for any boolean checks on upgradetohd results to replace them with
checks for a non-empty string or specific instruction messages.
Collaborator
Author
There was a problem hiding this comment.
wow, how does coderabbit knows it before actual CI failure? fixed in force-push
There was a problem hiding this comment.
🧩 Analysis chain
@coderabbit-user Thanks! I checked the force-push:
- Tests: I don’t see any remaining boolean-style assertions on upgradetohd; calls in functional tests no longer assert equality to True.
- Repo scan: No other boolean checks on upgradetohd found outside the RPC definition.
- Docs: I couldn’t find updated docs mentioning the new string return.
Want me to open a follow-up issue to update user-facing docs/release notes for the upgradetohd return-type change? If yes, I’ll file it and link back to this PR and discussion.
🏁 Script executed:
#!/bin/bash
set -e
echo "== Grep functional tests for upgradetohd calls with context =="
rg -n -A2 -B2 $'upgradetohd\\(' test/functional || true
echo
echo "== Look for boolean-style assertions directly on upgradetohd results in tests =="
rg -n -A1 -B2 $'assert.*(==\\s*True|is\\s*True|==\\s*False|is\\s*False).*upgradetohd\\(' test/functional || true
rg -n -A1 -B2 $'assert_equal\\s*\\(.*upgradetohd\\(.*\\),\\s*True\\s*\\)' test/functional || true
echo
echo "== Find all occurrences of upgradetohd outside the C++ RPC definition (to spot scripts/docs) =="
rg -n "upgradetohd" | rg -v "src/wallet/rpc/wallet.cpp" || true
echo
echo "== Search docs for upgradetohd mention (to see if return type is documented) =="
rg -n "upgradetohd" doc docs || true
Length of output: 17815
@coderabbit-user Nice! I rechecked the spots from your grep:
- Tests: no boolean-equality asserts left. wallet_upgradewallet.py Line 172 now asserts the exact string. In wallet_upgradetohd.py there are still truthiness checks (Lines 52, 88, 102, 113, 129, 144, 157, 168, 181, 210) which will continue to pass since the RPC returns a non-empty string; some later cases already assert the exact message (Lines 246–247, 256–257).
- Docs: only historical release-note mentions; no entry yet about the new string return.
Optional follow-ups:
- Tighten a few of those early truthiness asserts in wallet_upgradetohd.py to check the returned message (keeps coverage strong).
- If you’d like, I can open a follow-up issue to update user-facing docs/release notes about upgradetohd returning a string and the null-character guidance. Should I proceed?
🧬 Code Graph Analysis Results
Relevant snippets from src/wallet/rpc/wallet.cpp (lines 1-1097)
The entire file src/wallet/rpc/wallet.cpp is relevant as it contains the implementation of various wallet RPC commands, including:
- Wallet management commands such as
createwallet,loadwallet,unloadwallet,restorewallet,backupwallet. - Wallet encryption commands like
encryptwallet,walletpassphrase,walletpassphrasechange,walletlock. - Address and label management commands like
getnewaddress,getaddressinfo,setlabel,listaddressesbylabel. - Transaction commands such as
sendtoaddress,sendmany,send,listtransactions,gettransaction,abandontransaction. - Coin control commands like
lockunspent,listlockunspent,listunspent. - Wallet upgrade commands like
upgradewallet,upgradetohd,sethdseed. - Import/export commands like
importprivkey,importaddress,importwallet,importmulti,importdescriptors,importelectrumwallet,dumpprivkey,dumpwallet,dumphdinfo. - CoinJoin related commands
setcoinjoinrounds,setcoinjoinamount. - Utility commands like
listwallets,listwalletdir,getwalletinfo,simulaterawtransaction,wipewallettxes,rescanblockchain,abortrescan.
Each RPC command is implemented as a RPCHelpMan object with detailed argument parsing, error handling, locking, wallet context retrieval, and the core logic for the command.
Relevant snippets from src/wallet/rpc/util.h (lines 18-45)
This header declares utility functions used in wallet.cpp for wallet RPC commands:
GetWalletForJSONRPCRequest(const JSONRPCRequest& request): Returns the wallet pointer for the RPC request or throws if no wallet is found.GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& wallet_name): Extracts wallet name from the RPC request URI.EnsureWalletIsUnlocked(const CWallet&): Throws if the wallet is locked.EnsureWalletContext(const CoreContext&): Returns the wallet context or throws if not found.EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create = false): Returns the legacy ScriptPubKey manager or throws if not available.HandleWalletError(const std::shared_ptr<CWallet> wallet, DatabaseStatus& status, bilingual_str& error): Throws JSONRPCError based on wallet loading status.
Relevant snippets from src/wallet/receive.cpp (lines 295-334)
Contains wallet balance and address balance calculation functions:
Balance GetBalance(const CWallet& wallet, int min_depth, bool avoid_reuse, bool fAddLocked): Returns a struct with various balance categories (trusted, untrusted, immature, coinjoin, watchonly, etc.) by iterating over wallet transactions.std::map<CTxDestination, CAmount> GetAddressBalances(const CWallet& wallet): Returns a map of addresses to their balances by iterating over wallet transactions and outputs.
Relevant snippets from src/wallet/rpc/util.cpp (lines 55-151)
GetWalletForJSONRPCRequest(const JSONRPCRequest& request): Returns the wallet pointer for the RPC request or throws if no wallet is loaded or specified.EnsureWalletContext(const CoreContext& context): Returns the wallet context or throws if not found.HandleWalletError(const std::shared_ptr<CWallet> wallet, DatabaseStatus& status, bilingual_str& error): Throws JSONRPCError based on wallet loading status.GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& wallet_name): Extracts wallet name from the RPC request URI.EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create): Returns the legacy ScriptPubKey manager or throws if not available.EnsureWalletIsUnlocked(const CWallet& wallet): Throws if the wallet is locked.ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wallet): Parses the include_watchonly parameter for RPC commands.
Relevant snippets from src/wallet/load.cpp (lines 19-62)
std::vector<fs::path> ListDatabases(const fs::path& wallet_dir): Returns a list of wallet database paths in the wallet directory, used bylistwalletdirRPC.
Relevant snippets from src/wallet/wallet.cpp (lines 163-176, 275-286, 131-155, 217-239, 2159-2204, 288-390, 169-176)
GetWallets(WalletContext& context): Returns the vector of loaded wallets.LoadWallet(WalletContext& context, const std::string& name, ...): Loads a wallet by name, handling concurrency and errors.GetWallet(WalletContext& context, const std::string& name): Returns a loaded wallet by name or nullptr.RemoveWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet, ...): Removes a wallet from the loaded wallets list and updates settings.UnloadWallet(std::shared_ptr<CWallet>&& wallet): Marks a wallet for unloading and waits for release.CWallet::LoadWallet(): Loads wallet data from disk, initializes wallet state, sets CoinJoin salt if needed.CreateWallet(WalletContext& context, const std::string& name, ...): Creates a new wallet with specified options, encrypts if passphrase provided, sets up HD if enabled, adds to context.
Relevant snippets from src/wallet/coinjoin.cpp and src/coinjoin/coinjoin.cpp
- CoinJoin client options and rounds/amount settings are used in RPC commands
setcoinjoinroundsandsetcoinjoinamount.
Relevant snippets from src/core_write.cpp (lines 36-47)
UniValue ValueFromAmount(const CAmount amount): Converts amount in satoshis to a UniValue string with decimal representation, used in RPC results.
Relevant snippets from src/bitcoin-tx.cpp (lines 507-507)
AmountFromValue(const UniValue& value): Parses amount from UniValue parameter, throws on invalid input, used in RPC argument parsing.
Relevant snippets from src/wallet/ismine.h (lines 17-70)
- Defines
ismineflags andisminefilterused to filter wallet-owned outputs in RPC commands.
Relevant snippets from src/wallet/receive.h (lines 13-60)
- Functions to determine ownership of inputs/outputs, get credit/debit amounts, and balance calculations used in wallet RPC commands.
Relevant snippets from src/wallet/scriptpubkeyman.cpp (lines 1122-1128)
LegacyScriptPubKeyMan::HaveKey(const CKeyID &address) const: Checks if the legacy script pubkey manager has a private key for the given address.
Relevant snippets from src/wallet/rpc/signmessage.cpp (lines 14-69)
signmessage(): RPC command to sign a message with a private key of an address, includes wallet unlocking and key retrieval.
Relevant snippets from src/wallet/rpc/backup.cpp (lines 2059-2090)
backupwallet(): RPC command to safely copy the wallet file to a destination path.
Relevant snippets from src/wallet/rpc/transactions.cpp (lines 803-842)
abandontransaction(): RPC command to mark a wallet transaction as abandoned, allowing inputs to be respent.
Relevant snippets from src/wallet/rpc/coins.cpp (lines 161-220)
getbalance(): RPC command to return the total available balance of the wallet, with options for minimum confirmations, watch-only, and avoid reuse.
Relevant snippets from src/wallet/rpc/addresses.cpp (lines 420-561, 563-621)
getaddressinfo(): Returns detailed information about a given address in the wallet.getaddressesbylabel(): Returns list of addresses assigned to a specified label.
Relevant snippets from src/wallet/rpc/spend.cpp (lines 530-641)
fundrawtransaction(): RPC command to add inputs to a raw transaction to meet output value, with fee calculation.
Relevant snippets from src/wallet/rpc/encrypt.cpp (lines 10-272)
encryptwallet(): RPC command to encrypt the wallet with a passphrase.walletpassphrase(): RPC command to unlock the wallet for a specified timeout.walletpassphrasechange(): RPC command to change the wallet passphrase.walletlock(): RPC command to lock the wallet by removing the encryption key from memory.
Relevant snippets from src/wallet/rpc/util.cpp (lines 88-98)
AmountFromValue(const UniValue& value, int decimals): Parses amount from UniValue with decimal precision, throws JSONRPCError on invalid input.
Relevant snippets from src/wallet/rpc/wallet.h (lines 12-14)
- Declaration of
GetWalletRPCCommands()returning the list of wallet RPC commands.
Summary
The file src/wallet/rpc/wallet.cpp implements a comprehensive set of wallet-related RPC commands for Dash Core, including wallet creation, loading, encryption, transaction sending, coin control, import/export, and more. It relies on utility functions from wallet/rpc/util.h and wallet/rpc/util.cpp for wallet context and unlocking, and on wallet core functions for balance and transaction management.
The user comment "wow, how does coderabbit knows it before actual CI failure? fixed in force-push" suggests a fix was made preemptively, likely related to RPC command correctness or error handling. Understanding the RPC command implementations, argument parsing, error handling, and wallet context management in this file and the related utility functions is essential for reviewing or addressing such issues.
✏️ Learnings added
Learnt from: knst
PR: dashpay/dash#6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.285Z
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.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: kwvg
PR: dashpay/dash#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: kwvg
PR: dashpay/dash#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: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: PastaPastaPasta
PR: dashpay/dash#6804
File: src/qt/proposalwizard.cpp:0-0
Timestamp: 2025-08-08T04:30:37.944Z
Learning: In the Dash codebase, direct RPC calling has been removed in recent commits, making suggestions about RPC command validation and error handling for direct RPC calls obsolete. The ProposalWizard and related components no longer use direct RPC calling patterns.
Learnt from: kwvg
PR: dashpay/dash#6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Learnt from: kwvg
PR: dashpay/dash#6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.
Learnt from: UdjinM6
PR: dashpay/dash#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.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
UdjinM6
approved these changes
Aug 9, 2025
Merged
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Follow-up for #6797 and #6792 to update rpc error codes and add user's hint for
upgradetohd.What was done?
See commits.
upgradetohdnow returns a string instead bool (which is always true if no JSONRPCError is returned) which is better user experience.upgradetohdwarns user if there's \0 in user's input of wallet_passphrase or mnemonic_passphrase.How Has This Been Tested?
See updates in tests.
Breaking Changes
upgradetohdreturns string instead bool.Checklist: