fix: follow-up #27068 - apply changes for mnemonic too #6792
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
WalkthroughThe changes update the handling of mnemonic and mnemonic passphrase strings in wallet-related code. The release notes clarify that both wallet and mnemonic passphrases may contain null characters and update the referenced issue numbers. In wallet RPC commands and wallet implementation code, explicit Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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: 0
♻️ Duplicate comments (1)
src/wallet/rpc/backup.cpp (1)
2024-2029: Duplicate of the previous comment – same SecureString-to-UniValue copy occurs here.
🧹 Nitpick comments (1)
doc/release-notes-27068.md (1)
5-7: Clarify that both wallet and mnemonic passphrases allow null charactersOnly mnemonic passphrases are mentioned now, yet the codebase (and PR description) indicate the same treatment for wallet passphrases. Consider re-phrasing the bullet to cover both cases so users don’t assume the wallet passphrase support was dropped.
Also, you may tighten “Prior to this change” → “Before this change” if you want to avoid the wordiness flagged by tooling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
doc/release-notes-27068.md(1 hunks)src/wallet/rpc/backup.cpp(2 hunks)src/wallet/rpc/wallet.cpp(2 hunks)src/wallet/wallet.cpp(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
doc/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the doc directory (documentation)
Files:
doc/release-notes-27068.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/wallet/rpc/backup.cppsrc/wallet/rpc/wallet.cppsrc/wallet/wallet.cpp
🧠 Learnings (5)
📓 Common learnings
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#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: 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/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: 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.
doc/release-notes-27068.md (1)
Learnt from: kwvg
PR: #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.
src/wallet/rpc/backup.cpp (3)
Learnt from: kwvg
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: kwvg
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: kwvg
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.
src/wallet/rpc/wallet.cpp (3)
Learnt from: kwvg
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: kwvg
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: kwvg
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.
src/wallet/wallet.cpp (3)
Learnt from: kwvg
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: kwvg
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: kwvg
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.
🧬 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 (4)
request(31-31)JSONRPCError(56-62)JSONRPCError(56-56)error(58-58)
🪛 LanguageTool
doc/release-notes-27068.md
[style] ~6-~6: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ...ases may now contain null characters. Prior to this change, only characters up to the ...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
🔇 Additional comments (10)
src/wallet/rpc/backup.cpp (2)
905-913: PassingSecureStringdirectly fixes the null-character truncation – looks goodReplacing the earlier
.c_str()conversion ensures the entire passphrase (including embedded NULs) is retained. ✅
911-912: Sensitive-data copy intoUniValue– double-check exposure is acceptable
pushKV()stores theSecureStringin a regularstd::stringinside theUniValue, so the plaintext mnemonic & passphrase will sit in heap memory until the RPC result is destroyed (and that memory will not be scrub-zeroed). This behaviour already existed for wallet passphrases, but the wider data now included may warrant revisiting:• Confirm that exposing full mnemonics/passphrases in RPC output under
-rpcallow...policies is intentional.
• If not, consider redacting or gating behind an explicit debug flag.No change required if this is indeed the desired UX, but worth a second look.
src/wallet/wallet.cpp (2)
863-872: LGTM! Proper SecureString initialization for null character support.The capacity reservation pattern correctly implements the changes from bitcoin#27068, ensuring that mnemonic and mnemonic passphrase strings can contain null characters. The 256-byte capacity is appropriate for mnemonic phrases.
883-891: LGTM! Consistent SecureString initialization across wallet types.This segment applies the same null character support pattern to the descriptor wallet path, maintaining consistency with the HD wallet creation path. The implementation correctly follows the bitcoin#27068 changes.
src/wallet/rpc/wallet.cpp (6)
374-376: LGTM: Variable renamed and capacity reserved appropriately.The renaming from
secureWalletPassphrasetowallet_passphraseimproves consistency with naming conventions, and the explicitreserve(100)call ensures adequate capacity for the SecureString.
382-382: LGTM: Direct assignment using std::string_view supports null characters.The change from using
.c_str()to direct assignment viastd::string_viewproperly supports null characters in wallet passphrases, which aligns with the PR objectives to extend bitcoin#27068 changes to Dash-specific code.
385-389: LGTM: Mnemonic handling updated consistently.The variable renaming from
secureMnemonictomnemonicand the use ofstd::string_viewfor direct assignment (line 388) maintains consistency with the wallet passphrase changes and properly supports null characters in mnemonics.
391-395: LGTM: Mnemonic passphrase handling updated consistently.The variable renaming from
secureMnemonicPassphrasetomnemonic_passphraseand the use ofstd::string_viewfor direct assignment (line 394) completes the consistent refactoring pattern and properly supports null characters in mnemonic passphrases.
406-406: LGTM: Function call updated to use new variable names.The
UpgradeToHDfunction call correctly uses the renamed variables (mnemonic,mnemonic_passphrase,wallet_passphrase), maintaining the same functionality while supporting the new null character capability.
408-409: LGTM: Conditional logic updated consistently.Both the condition check (line 408) and the
EncryptWalletcall (line 409) correctly use the renamedwallet_passphrasevariable, maintaining the existing logic flow.
Ohhh, I found a bug!!! PR is updated. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
doc/release-notes-27068.md (1)
7-7: Address the previous review feedback.The past review comment suggests changing "null character were" to "null characters were" for proper grammar.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
doc/release-notes-27068.md(1 hunks)src/wallet/bip39.cpp(1 hunks)test/functional/test_framework/util.py(1 hunks)test/functional/wallet_mnemonicbits.py(2 hunks)test/functional/wallet_upgradetohd.py(8 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/wallet/bip39.cpp
- test/functional/wallet_mnemonicbits.py
🧰 Additional context used
📓 Path-based instructions (2)
test/functional/**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
Functional tests should be written in Python and placed in test/functional/
Files:
test/functional/test_framework/util.pytest/functional/wallet_upgradetohd.py
doc/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the doc directory (documentation)
Files:
doc/release-notes-27068.md
🧠 Learnings (4)
📓 Common learnings
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#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: 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/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.
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.
📚 Learning: in the test framework consolidation pr (#6718), user kwvg prefers to limit functional changes to tho...
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:
test/functional/test_framework/util.pytest/functional/wallet_upgradetohd.py
📚 Learning: in pr #6761, kwvg acknowledged a null pointer check issue in chainlocksigner::cleanup() method but d...
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.
Applied to files:
doc/release-notes-27068.md
📚 Learning: pull request #6543 is focused on move-only changes and refactoring, specifically backporting from bi...
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:
doc/release-notes-27068.md
🧬 Code Graph Analysis (1)
test/functional/wallet_upgradetohd.py (2)
test/functional/test_framework/util.py (3)
assert_equal(69-74)get_mnemonic(540-565)assert_raises_rpc_error(132-148)test/functional/test_framework/test_node.py (1)
get_wallet_rpc(360-366)
🪛 LanguageTool
doc/release-notes-27068.md
[style] ~6-~6: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ...ases may now contain null characters. Prior to this change, only characters up to the ...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
⏰ 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). (8)
- GitHub Check: linux64_sqlite-test / Test source
- GitHub Check: linux64_nowallet-test / Test source
- GitHub Check: linux64-test / Test source
- GitHub Check: linux64_multiprocess-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: win64-build / Build source
🔇 Additional comments (11)
doc/release-notes-27068.md (1)
4-7: Documentation update looks good.The extension to include mnemonic passphrases alongside wallet passphrases is appropriate and consistent with the PR objectives. The issue reference updates (#6780 #6792) correctly reflect the Dash-specific implementation.
test/functional/test_framework/util.py (1)
540-547: LGTM! Function signature change is appropriate.The modification to return a tuple
(mnemonic, mnemonic_passphrase)instead of just the mnemonic string aligns well with the PR objectives to support mnemonic passphrases alongside mnemonics.test/functional/wallet_upgradetohd.py (9)
113-115: LGTM! Proper handling of mnemonic tuple.The changes correctly unpack the mnemonic tuple returned by
get_mnemonic()and pass the components as separate arguments toupgradetohd().
129-131: LGTM! Consistent mnemonic tuple handling.The test correctly verifies that the mnemonic remains the same while the mnemonic passphrase differs, demonstrating proper support for separate mnemonic passphrase parameters.
144-144: LGTM! Proper argument passing.The test correctly passes both mnemonic components to
upgradetohd()for wallet recovery.
157-157: LGTM! Consistent with updated function signature.The test properly handles the mnemonic tuple for the large keypool scenario.
168-168: LGTM! Correct parameter handling.The test appropriately passes mnemonic components when skipping initial rescan.
181-181: LGTM! Proper encryption test setup.The test correctly handles mnemonic parameters during wallet encryption upgrade.
207-212: LGTM! Comprehensive error handling verification.The tests properly verify error scenarios for encrypted wallets using the updated function signature.
249-250: LGTM! Proper verification of custom mnemonic and passphrase.The test correctly verifies that both the custom mnemonic and custom passphrase are properly stored and retrieved.
254-260: Excellent test case for null character handling.This new test case directly addresses the PR objectives by verifying that mnemonic passphrases can contain null characters. The test correctly demonstrates that:
- A trailing null character in the passphrase results in different wallet behavior (balance = 0 vs 12)
- The null character is preserved in the stored passphrase
This is a valuable addition that validates the core functionality described in the release notes.
|
This pull request has conflicts, please rebase. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
doc/release-notes-27068.md (2)
4-5: Align bullet formatting for consistencyThe second item is indented but lacks the leading dash used for the first item, so the Markdown list will render unevenly. Prefix line 5 with the same “- ” to keep the list uniform.
6-7: Prefer plainer wording“Prior to this change” is slightly verbose; “Before this change” (or simply “Previously”) is clearer and consistent with the terse style elsewhere in the release notes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
doc/release-notes-27068.md(1 hunks)src/wallet/bip39.cpp(1 hunks)src/wallet/hdchain.cpp(1 hunks)src/wallet/rpc/backup.cpp(2 hunks)src/wallet/rpc/wallet.cpp(4 hunks)src/wallet/wallet.cpp(3 hunks)test/functional/test_framework/util.py(1 hunks)test/functional/wallet_mnemonicbits.py(2 hunks)test/functional/wallet_upgradetohd.py(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/wallet/rpc/backup.cpp
🚧 Files skipped from review as they are similar to previous changes (7)
- src/wallet/bip39.cpp
- test/functional/wallet_mnemonicbits.py
- src/wallet/rpc/wallet.cpp
- test/functional/test_framework/util.py
- src/wallet/hdchain.cpp
- test/functional/wallet_upgradetohd.py
- src/wallet/wallet.cpp
🧰 Additional context used
📓 Path-based instructions (1)
doc/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the doc directory (documentation)
Files:
doc/release-notes-27068.md
🧠 Learnings (3)
📓 Common learnings
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#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: 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/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.
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.
📚 Learning: in pr #6761, kwvg acknowledged a null pointer check issue in chainlocksigner::cleanup() method but d...
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.
Applied to files:
doc/release-notes-27068.md
📚 Learning: pull request #6543 is focused on move-only changes and refactoring, specifically backporting from bi...
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:
doc/release-notes-27068.md
🪛 LanguageTool
doc/release-notes-27068.md
[style] ~6-~6: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ...ases may now contain null characters. Prior to this change, only characters up to the ...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
⏰ 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: x86_64-apple-darwin / Build depends
- GitHub Check: Lint / Run linters
- GitHub Check: x86_64-pc-linux-gnu_multiprocess / Build depends
- GitHub Check: x86_64-w64-mingw32 / Build depends
- GitHub Check: arm-linux-gnueabihf / Build depends
- GitHub Check: x86_64-pc-linux-gnu / Build depends
- GitHub Check: x86_64-pc-linux-gnu_nowallet / Build depends
…ng null character 960afcf feat: upgradetohd return string with futher instructions instead bool (Konstantin Akimov) 4f42cfa doc: add extra help for case if entered wallet passphrae is incorrect (Konstantin Akimov) Pull request description: ## 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. `upgradetohd` now returns a string instead bool (which is always true if no JSONRPCError is returned) which is better user experience. `upgradetohd` warns 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 `upgradetohd` returns string instead bool. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 960afcf Tree-SHA512: 42351fbe84b824b04120a19814f4a24829bd2c8efab3079ff94d06af4a486d4c13f99d62ebe1e940f483e4151ddf54b8c47ffd31762c49bd0b6735524da9f6e8
3 participants
Issue being fixed or feature implemented
bitcoin#27068 introduced support of null character in SecureString initialization, but this feature has not been supported for mnemonic passphrases.
What was done?
This PR apply changed introduced in bitcoin#27078 for dash specific code.
How Has This Been Tested?
Run unit & functional tests.
Breaking Changes
RPC
upgradetohdsupports null characters in mnemonic passphrase and in wallet's passphrase.Checklist: