Fix Bitcoin send screen freezing by preventing division by zero#115
Conversation
- Create organized Documentation/ folder structure - Move all scattered MD files to appropriate categories: - Architecture: Design guidelines - Deployment: Fastlane deployment instructions - Setup: Configuration and setup guides - Implementation: Feature documentation (Striga, Cards, etc) - Guides: Testing and development guides - Update README with links to new documentation structure - Clean up root directory from documentation clutter
- Move test files to Tests/TestFiles/ - Move Bitcoin icon to Resources/Icons/ - Move Lightning validator to Tools/Scripts/ - Move Fastfile_testers to fastlane/ - Move certificates to Certificates/ (gitignored) - Move logs to Logs/ (gitignored) - Update .gitignore to exclude Certificates/ and Logs/ - Root directory now contains only essential files
- Create .local/ folder structure for local development files - Move certificates and logs to .local/ (gitignored) - Keep copies of critical Fastlane files in both locations for safety - Add comprehensive gitignore rules for local development files - Document .local folder structure with README - Verified deployment still works (Build 12 confirmed on TestFlight)
- Add guards to prevent division by zero when exchangeRate is 0 - Protect against NaN/Infinity values in amountInSats calculations - Fix isInsufficientFunds computed property to handle invalid exchange rates - Prevent app crashes and UI freezing on Bitcoin send screen 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThis pull request removes numerous Striga- and card-related documentation files, adds a new Documentation README and a LICENSE, updates the main README and .gitignore, and adds defensive guards in AmountEntryScreen.swift to validate exchange-rate-dependent calculations. Changes
Sequence Diagram(s)(omitted — changes are documentation deletions and a small local calculation guard; no new control flow to depict) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ 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). (1)
✨ 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Claude finished @eminogrande's task —— View job Pull Request ReviewReviewing the Bitcoin send screen division by zero fix and documentation reorganization. Todo List:
Code Review Feedback✅ Strengths
|
![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
🧹 Nitpick comments (7)
.gitignore (4)
130-131: Anchor Certificates directory to repo root (prevent overmatching)."Certificates/" without a leading slash ignores any directory named Certificates at any depth. If your intent is to ignore the root-level Certificates folder only, anchor it.
Apply:
-Certificates/ +/Certificates/
135-138: *Duplicate ignore pattern for .log; consolidate to a single rule."*.log" is already ignored on Line 103. Keeping duplicates adds noise and can cause confusion when maintaining the file.
Apply:
-# Logs -Logs/ -*.log +# Logs +Logs/
133-134: Duplicate ignore for provisioning profiles; remove redundancy."*.mobileprovision" is already included under the Provisioning Profiles section (Line 79). The second rule is redundant.
Apply:
-# Additional provisioning profiles in root (backup rule) -*.mobileprovision +# (removed duplicate) *.mobileprovision already ignored above
139-141: Consider anchoring local dev directories to root.If these are meant to be top-level only, add a leading slash to prevent overmatching nested folders named local/. Keep as-is if you intend to ignore at any depth.
Possible update:
-.local/ -local/ +/.local/ +/local/Documentation/README.md (1)
19-24: Avoid referencing removed Striga/Card docs or add links to new equivalents.This section lists Striga integration, card specs, and WebView setup, but those docs were reportedly removed in this PR. Either:
- Link to their new consolidated locations, or
- Generalize these bullets to avoid implying the presence of specific (now-missing) docs.
Example generalization:
- Striga integration documentation - Card functionality specifications - WebView setup and configuration + Payment integration overview and patterns + Card-related feature specifications (if applicable) + WebView usage, configuration, and security best practicesREADME.md (1)
14-23: Cross-check PR objectives vs. code changes (missing functional changes).The PR description mentions fixes to the Bitcoin send screen (division-by-zero guards, NaN/Infinity handling). This commit only modifies docs and .gitignore. Please confirm that the functional changes to the send screen are included in this PR or will arrive in a separate PR.
I can help scan the codebase for the affected computed properties (e.g., amountInSats, isInsufficientFunds) and verify guards are in place—let me know if you want a follow-up script.
Nuri/Nuri/Sources/View Components/AmountEntryScreen.swift (1)
175-180: Consider extracting EUR-to-sats conversion logic for better maintainability.The same conversion logic and NaN/Infinity checks appear in multiple places (
secondaryDisplayText,amountInSats,toggleCurrency, etc.). Consider extracting this into a helper method to reduce duplication and ensure consistency.private func convertEurToSats(_ eurAmount: Double) -> Double { guard exchangeRate > 0 else { return 0 } let sats = (eurAmount / exchangeRate) * 100_000_000 if sats.isNaN || sats.isInfinite { return 0 } return sats }Then use it consistently across the codebase:
-let sats = (amountValue / exchangeRate) * 100_000_000 -// Protect against NaN/Infinity before converting to Int -if sats.isNaN || sats.isInfinite { - return "₿ --" -} -return "₿ " + String(Int(round(sats))) +let sats = convertEurToSats(amountValue) +if sats == 0 { + return "₿ --" +} +return "₿ " + String(Int(round(sats)))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Resources/Icons/Bitcoin-Logo-SVG-Icon.svgis excluded by!**/*.svg
📒 Files selected for processing (15)
.gitignore(1 hunks)CARD_DETAILS_IMPLEMENTATION.md(0 hunks)CARD_IMPLEMENTATION_CONFLICTS_SUMMARY.md(0 hunks)CARD_SANDBOX_FIX_SUMMARY.md(0 hunks)CARD_VERIFICATION_IMPLEMENTATION_SUMMARY.md(0 hunks)Documentation/README.md(1 hunks)FINAL_CARD_VERIFICATION_SUMMARY.md(0 hunks)Nuri/Nuri/Sources/View Components/AmountEntryScreen.swift(3 hunks)PROXY_IMPLEMENTATION_VERIFICATION.md(0 hunks)README.md(2 hunks)STRIGA_CARD_FLOW_DOCUMENTATION.md(0 hunks)STRIGA_IMPLEMENTATION.md(0 hunks)STRIGA_REVIEW_SUMMARY.md(0 hunks)STRIGA_WEBVIEW_SETUP.md(0 hunks)TESTING_GUIDE.md(0 hunks)
💤 Files with no reviewable changes (11)
- STRIGA_WEBVIEW_SETUP.md
- PROXY_IMPLEMENTATION_VERIFICATION.md
- STRIGA_IMPLEMENTATION.md
- CARD_DETAILS_IMPLEMENTATION.md
- TESTING_GUIDE.md
- STRIGA_CARD_FLOW_DOCUMENTATION.md
- STRIGA_REVIEW_SUMMARY.md
- CARD_VERIFICATION_IMPLEMENTATION_SUMMARY.md
- CARD_SANDBOX_FIX_SUMMARY.md
- CARD_IMPLEMENTATION_CONFLICTS_SUMMARY.md
- FINAL_CARD_VERIFICATION_SUMMARY.md
⏰ 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). (1)
- GitHub Check: claude-review
🔇 Additional comments (6)
Documentation/README.md (1)
37-41: Quick Links Verified – No Changes NeededThe
TESTING_GUIDE.mdandDESIGN_GUIDELINES.mdfiles both exist underDocumentation/Guides/andDocumentation/Architecture/respectively, so the existing Quick Links are correct. No updates to the README are required.README.md (1)
37-53: Fastlane lanes and docs are in sync
- fastlane/Fastfile defines both
lane :deploy(line 5) andlane :archive(line 116)- The
:archivelane writes to./builds/Nuri.xcarchiveas documented- Documentation/Deployment/DEPLOYMENT_INSTRUCTIONS.md is present
No further changes required.
Nuri/Nuri/Sources/View Components/AmountEntryScreen.swift (4)
240-241: LGTM! Excellent defensive programming against division by zero.The guard clause properly prevents division by zero errors that would cause the app to freeze. Returning 0 for invalid exchange rates is a sensible fallback.
254-259: LGTM! Comprehensive protection against invalid floating point values.The implementation correctly handles both the division by zero prevention and NaN/Infinity checks. The early return of 0 prevents downstream crashes and UI freezing.
276-277: LGTM! Consistent guard pattern for exchange rate validation.The guard clause maintains consistency with other methods and prevents calculation errors in the insufficient funds logic when the exchange rate is invalid.
195-199: Verify user experience when exchange rate is invalid.While the guard prevents crashes, silently ignoring toggle actions when
exchangeRate <= 0might confuse users. Consider providing visual feedback or temporarily disabling the toggle button.Should the toggle button be disabled or show some indication when the exchange rate is invalid? This would improve user experience by clearly communicating why the action isn't working.
This commit adds an MIT License to make the project fully open source, allowing anyone to use, modify, and distribute the software freely.
|
Claude finished @eminogrande's task —— View job Reviewing Bitcoin send screen division by zero fix and documentation reorganization Todo List:
Pull Request ReviewReviewing the Bitcoin send screen division by zero fix and documentation reorganization. ✅ Strengths
|
1 participant
Summary
• Fixed Bitcoin send screen freezing issue caused by division by zero errors
• Added comprehensive guards to prevent app crashes when exchangeRate is invalid
• Improved error handling for NaN/Infinity values in amount calculations
Changes Made
• Added guards in
amountInSatscomputed property to prevent division by zero whenexchangeRateis 0• Protected against NaN/Infinity values in EUR to sats conversion calculations
• Fixed
isInsufficientFundscomputed property to handle invalid exchange rates safely• Prevents UI freezing and app crashes on the Bitcoin send screen
Test Plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores