-
Notifications
You must be signed in to change notification settings - Fork 22
fix: prevent wallet deletion on screenshot during backup reminder #745
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When users took a screenshot during the backup reminder flow (scenario 2), the app incorrectly deleted all wallets and created a new passphrase. This change ensures screenshots only show a warning without any destructive wallet operations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughA computed property in BackupInfoViewController that determines screenshot behavior was modified. The Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
DashWallet/Sources/UI/Setup/SecureWallet/BackupInfo/BackupInfoViewController.swift (1)
248-250: Critical security fix correctly implemented.The change from
type == .reminderto alwaysfalsecorrectly prevents the destructive wallet deletion bug during backup reminder screenshots. This is the right fix - screenshots should never trigger wallet deletion in any scenario.Optional: Consider adding documentation for future maintainability
Since this property now always returns
false(and should remain that way for security), consider adding a comment explaining the reasoning:extension BackupInfoViewController { + /// Always returns false to prevent destructive wallet operations on screenshot. + /// Screenshots should only show a warning dialog, never delete/recreate wallets. var shouldCreateNewWalletOnScreenshot: Bool { false } }This helps future maintainers understand why the property exists and why it should remain
false.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
DashWallet/Sources/UI/Setup/SecureWallet/BackupInfo/BackupInfoViewController.swift
🧰 Additional context used
📓 Path-based instructions (2)
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Use SwiftUI for all new UI components. Do NOT create new UIKit ViewControllers, XIB files, or Storyboards. Use thin UIViewController wrappers only when integrating with existing UIKit navigation.
Always use guard statements instead of force unwrapping for optional coordinates (latitude, longitude). Never force unwraplatitude!orlongitude!- use guard with error handling.
For generic type inference in Swift closures, add explicit return type annotations (e.g.,{ merchant -> MerchantAnnotation? in }) when compilation fails with generic parameter errors.
In SwiftUI conditional compilation, use computed properties instead of inline#ifstatements to avoid buildExpression errors. Move complex conditions into dedicated computed properties.
For dictionary initialization with conditional compilation, use closure-based initialization instead of inline conditionals to avoid syntax errors.
When removing debug print statements, ensure switch cases remain valid (addbreakif case becomes empty). Never leave empty switch cases.
Conditional compilation in Swift: use#if DASHPAYfor DashPay features and#if PIGGYCARDS_ENABLEDfor PiggyCards features. Wrap all provider-specific code in appropriate flags.
Always use debug flags like#if DEBUGto wrap debug print statements. Remove debug logging before release commits. Search forprint("statements before any release.
Use emoji prefixes in debug logs for easy filtering: 🔍 for search, 📍 for location, 🗺️ for map, 🎯 for targeted debugging. This improves ability to filter console output.
Always validate CLLocationCoordinate2D.isValid before using coordinates. Implement fallback chains for CTX API data. Test coordinate edge cases thoroughly in unit tests.
Use os_log framework for production-safe logging instead of print statements. os_log messages are only logged in debug builds and stripped in release builds.
Files:
DashWallet/Sources/UI/Setup/SecureWallet/BackupInfo/BackupInfoViewController.swift
**/*.{swift,m,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 4-space indentation and 180-character line limit (100 recommended). Follow NYTimes Objective-C Style Guide for ObjC code. Use SwiftFormat/SwiftLint for Swift code formatting.
Files:
DashWallet/Sources/UI/Setup/SecureWallet/BackupInfo/BackupInfoViewController.swift
🧠 Learnings (2)
📚 Learning: 2025-12-17T21:42:15.776Z
Learnt from: CR
Repo: dashpay/dashwallet-ios PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T21:42:15.776Z
Learning: Applies to DashWallet/Sources/Models/Uphold/DWUpholdMainnetConstants.m : The DWUpholdMainnetConstants.m file gets automatically formatted by the clang-format build phase script, adding unwanted blank lines. Restore this file before committing if it only has whitespace changes.
Applied to files:
DashWallet/Sources/UI/Setup/SecureWallet/BackupInfo/BackupInfoViewController.swift
📚 Learning: 2025-09-05T04:46:12.717Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 728
File: DashWallet.xcodeproj/project.pbxproj:1595-1599
Timestamp: 2025-09-05T04:46:12.717Z
Learning: In iOS projects, the `DashWallet.xcodeproj/project.pbxproj` file is automatically generated and managed by Xcode. Manual changes to this file should not be made, and the changes shown in diffs are typically the result of Xcode updating project configuration, dependencies, or build settings.
Applied to files:
DashWallet/Sources/UI/Setup/SecureWallet/BackupInfo/BackupInfoViewController.swift
HashEngineering
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very simple solution.
Summary
Background
There are 4 scenarios when users can view their recovery phrase:
Scenario 2 had
shouldCreateNewWalletOnScreenshotreturningtrue, which triggered wallet deletion on screenshot. This is now fixed to returnfalse.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.