perf(crowdnode): persist the fruitless-restore memo per wallet - #975
Merged
Conversation
The in-memory memo that lets restoreState() skip the full post-2022 history scan after a fruitless pass died with the process, so every cold launch of a wallet that never used CrowdNode still paid one ~7s main-thread scan at sync-done. Persist it per wallet in CrowdNodeDefaults (keyed to the row count read before the scans) so a later launch skips the scan when the exact same history was already scanned fruitlessly. - Written only by the account-not-found branch; a nil row count (SDK container not up) writes nothing and preserves a prior launch's memo. - Cleared by resetUserDefaults()/reset() (network change, alien address), resetForWipe()/clearPerWalletKeys (via kPerWalletKeys), and on active- wallet change - the row count is store-global while the scan is wallet-scoped, so a switch always rescans. - A restored seed whose history syncs in later rescans: count changes, memo misses. - Key resolution bypasses resolvedKey's seed-from-legacy step: a bare-key memo can only come from a no-wallet (vacuous) pass and must not be promoted into a wallet's key. Verified on the QA sim with the synthetic 7k mainnet store: launch A scans and persists the memo (7007 == row count), launch B skips the scan entirely, launch C with a stale memo (6900) rescans. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughCrowdNode now persists the fruitless restore transaction count per wallet. Restore skips scans when the stored count matches the current count. Reset and active-wallet changes clear the stored memo. ChangesPer-wallet fruitless restore persistence
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
5 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
CrowdNode.restoreState()runs a full post-2022 history scan when looking for a CrowdNode account. The scan is memoized in-process when it finds nothing (fruitlessRestoreTxCount), but that memo died with the process — so every cold launch of a wallet that never used CrowdNode still paid one ~7s main-thread scan at sync-done (measured ~1.1 ms/row materialization on a 7k-tx store).What was done?
Persisted the memo per wallet in
CrowdNodeDefaults(already scoped by active walletId), so a launch skips the scan when a previous launch already scanned the exact same history fruitlessly:crowdNodeFruitlessRestoreTxCountKey, cached like the otherCrowdNodeDefaultsproperties.restoreState()reads/writes it in place of the old in-memory var (the prefs_-cache plays the old in-memory role).resetUserDefaults()/reset()(network change, alien-address teardown),resetForWipe()andclearPerWalletKeys(forWalletIdHex:)via membership inkPerWalletKeys(full wipe, single-wallet deletion), andhandleActiveWalletChanged()— the row count is store-global while the scan is wallet-scoped, so a memo can't be trusted across a wallet switch.resolvedKey's seed-from-legacy step: this key is post-multi-wallet (no legacy install base), and a bare-key value can only come from a no-active-wallet pass whose wallet-scoped scan was vacuous — seeding it would let a wallet skip a scan it never ran.Orthogonal to the CrowdNode release posture: the state machine stays app/SDK-owned, no DashSync surface is touched, and the change is inert while CrowdNode is hidden.
How Has This Been Tested?
dashpayscheme build (arm64 iOS simulator).checkCrowdNodeStatetrigger, zero "restoring CrowdNode state" / "CrowdNode scan:" log lines — cold-launch scan skipped.Unit-test target is currently broken repo-wide (pre-existing); verification standard per CLAUDE.md is the clean dashpay build + testnet/sim smoke above.
Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit