Remove final DashSync dependency tails - #907
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
romchornyi
left a comment
There was a problem hiding this comment.
There is a blocking risk in the wipe flow: the PIN is removed before wallet deletion completes. If SDK deletion fails, the wallet may remain on the device without its PIN. Additionally, all wallet IDs are deleted through the current network’s manager, which may result in a partial wipe for mainnet/testnet wallets, while the Watch migration causes regular .moved transactions to display a zero amount.
|
@romchornyi Could you please re-review? I pushed 3f046de addressing all three findings:
|
romchornyi
left a comment
There was a problem hiding this comment.
The network-specific deletion and Apple Watch amount fixes look good. One blocking issue remains: app-owned state is still cleared before the wipe succeeds. DWRootModel and DWRecoverModel call restoreToDefaults() immediately after enqueueing the wipe, while DWWillWipeWallet immediately triggers CrowdNode.reset() and clears its preferences. If SDK deletion fails, the wallet and PIN remain, but metadata and preferences have already been lost; these mutations should be moved into the successful wipe commit.
|
@romchornyi Could you please re-review commit cba692a? The remaining wipe blocker is addressed:
|
Summary
Verification
Unit tests were intentionally not added or run for this unlink pass. Runtime smoke, especially upgrade/multi-wallet/wipe and paired Watch compatibility, is still required before release.