Repository navigation
Conversation
-newwallet=1, set by the Qt "Create new wallet" path, stayed set for
the whole session. Every later ScanForWalletTransactions returned early
("Created new wallet, no need to scan"), so the rescans of
importaddress, importpubkey, importmulti and the BIP47 rescan did
nothing. importmulti then dereferenced the null scannedRange and
crashed (SIGSEGV on regtest).
-rescan=1, soft-set on every mnemonic first run and also passed by
users, stayed set too. ScanForWalletTransactions reads it, so every
later rescan ignored its own start block and began at nMnemonicBlock
(genesis for non-mnemonic wallets). Mnemonic wallets missed imported
history before that block, and forward rescans re-read the chain from
there while holding cs_main and cs_wallet. -rescan is documented as a
startup option, and upstream only reads it for the startup scan.
Clear both in CreateWalletFromFile right after the startup scan. They
go together: with only -newwallet cleared, the BIP47 rescan in a Create
session would scan from nMnemonicBlock. Also make importmulti check
keys against the tip when the scan returns null (shutdown, unreadable
last blocks) instead of crashing.
Tests: add wallet_startup_scan_args.py. It fails on the old code
(importmulti crash) and without the -rescan clear (scan from block 0).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
Summary by CodeRabbit
WalkthroughThe change clears ChangesWallet startup scan handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the primary change: clearing -newwallet and -rescan after the startup scan. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The pull request description includes both required sections. It clearly states the intent, affected behaviors, code changes, regression test, and testing results. It is complete and aligned with the … Full details: Description check
-
Explanation
The pull request description includes both required sections. It clearly states the intent, affected behaviors, code changes, regression test, and testing results. It is complete and aligned with the pull request objectives.
-
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
codeant-ai
Bot
added
the
size:M
CodeAnt-AI DescriptionKeep startup scan settings from disrupting later wallet imports What Changed
Impact
💡 Usage GuideChecking Your Pull RequestEvery time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later. Talking to CodeAnt AIGot a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask: This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code. ExamplePreserve Org Learnings with CodeAntYou can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input: This helps CodeAnt AI learn and adapt to your team's coding style and standards. ExampleRetrigger reviewAsk CodeAnt AI to review the PR again, by typing: Check Your Repository HealthTo analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health. |
CodeAnt PR Risk: Medium Risk
Assessed commit: |
This branch has not been deployed
1 participant
PR intention
-newwalletand-rescanare startup options, but they stayed set for the whole process, andCWallet::ScanForWalletTransactionsreads them on every call. Two bugs follow:-newwallet=1): every later rescan returns early, soimportaddress,importpubkey,importmultiand the BIP47 rescan import nothing, andimportmultidereferences the null scan result. Repro:firod -regtest -newwallet=1,generate 10,importmulti '[{"scriptPubKey":"76a914000000000000000000000000000000000000000088ac","timestamp":0,"watchonly":true,"internal":true}]'→ SIGSEGV.-rescan=1(soft-set on every mnemonic first run, or passed by the user): every later rescan ignores its own start block and starts atnMnemonicBlock(222400 on mainnet; genesis for non-mnemonic wallets). Imported history before that block is missed, and each import or BIP47 rescan re-reads the chain from there while holdingcs_mainandcs_wallet. Upstream only reads-rescanfor the startup scan.Code changes brief
CreateWalletFromFile: clear both args right after the startup scan, which is itself unchanged.ForceSetArg(…, "0"), because an empty value reads as true. The two clears belong together: with only-newwalletcleared, the BIP47 rescan after "Create new wallet" would start atnMnemonicBlock.importmulti: a null scan result (shutdown during the scan, unreadable last block) now checks every key against the tip and reports "Failed to rescan before time …" instead of crashing.qa/rpc-tests/wallet_startup_scan_args.py: a node started with-newwallet=1 -rescanmust find another node's payment throughimportmulti, and that scan must not start at block 0. It fails on master (crash) and with the-rescanclear removed (scan from block 0).-wcdateleaks the same way; Wallet: Stop restore args leaking past their use #2015 clears it at the same spot, and the two branches merge cleanly.Testing
Linux Release build with GUI and tests. ctest passes except
WalletUiTests::themeTintColors, which fails on master too.Regtest, old vs. new
firod(100 blocks dated 2020, then 10 dated 2021;importmultiwith a 2021 timestamp, so it should start at block 101):importaddress,importpubkeyimportmultiimportmulti-rescan: startup scan /importmultiqa tests passing: the new one plus
notifications,transactions_verification_after_restart,zapwallettxes,importprunedfunds,wallet,wallet-hd,wallet-dump,bip47-walletrestore,consolidation,listtransactions. Not exercised: the Qt BIP47 rescan itself; it is the sameScanForWalletTransactionscall.🤖 Generated with Claude Code