Add ability to send directly to other imported wallets#697
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request adds wallet-selection functionality to the send flow on both Android and iOS platforms. Users can now choose another wallet as the send destination, with its first address automatically fetched and displayed. The feature includes an optional toggle to show the raw address for verification purposes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EnterAddressView as EnterAddressView (UI)
participant Database
participant WalletManager
User->>EnterAddressView: Tap wallet picker button
activate EnterAddressView
EnterAddressView->>Database: Query allSortedActive() wallets<br/>(excluding currentWalletId)
Database-->>EnterAddressView: Return wallet list
EnterAddressView->>User: Display wallet picker modal
deactivate EnterAddressView
User->>EnterAddressView: Select wallet from list
activate EnterAddressView
EnterAddressView->>WalletManager: Load firstAddress() for wallet
activate WalletManager
WalletManager-->>EnterAddressView: Return address
deactivate WalletManager
EnterAddressView->>EnterAddressView: Update address state<br/>Set selectedWallet metadata<br/>Dismiss picker
EnterAddressView->>User: Display selected wallet header<br/>with address and toggle
deactivate EnterAddressView
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR adds a wallet-picker button to the send flow's address entry UI, allowing users to self-transfer to any other imported wallet, and adds a "Show address" toggle to reveal the resolved raw address. The feature is implemented consistently across iOS and Android.
Confidence Score: 4/5Safe to merge after fixing the iOS WalletManager resource leak; remaining findings are non-blocking. One P1 finding (iOS native resource leak on every wallet selection) should be resolved before shipping. All other comments are P2 style/quality suggestions that do not block correctness. ios/Cove/Flows/SendFlow/SetAmountScreen/EnterAddressView.swift — WalletManager must be closed after use. Important Files Changed
Sequence DiagramsequenceDiagram
actor User
participant EnterAddressView
participant ModalBottomSheet/Sheet
participant Database
participant WalletManager (dest)
User->>EnterAddressView: Tap wallet icon
EnterAddressView->>ModalBottomSheet/Sheet: showingWalletPicker = true
ModalBottomSheet/Sheet->>Database: allSortedActive() filtered by currentWalletId
Database-->>ModalBottomSheet/Sheet: WalletMetadata list
User->>ModalBottomSheet/Sheet: Select wallet
ModalBottomSheet/Sheet->>WalletManager (dest): WalletManager(id)
WalletManager (dest)->>WalletManager (dest): firstAddress()
WalletManager (dest)-->>ModalBottomSheet/Sheet: addressInfo
ModalBottomSheet/Sheet->>EnterAddressView: address = unformatted address
note over EnterAddressView: selectedWallet set, showingWalletPicker = false
EnterAddressView-->>User: Shows wallet card + Show address toggle
User->>EnterAddressView: Tap Show address
EnterAddressView-->>User: Reveals raw Bitcoin address
Reviews (1): Last reviewed commit: "Add ability to send directly to other im..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Task { | ||
| if let wm = try? WalletManager(id: wallet.id), | ||
| let addrInfo = try? await wm.firstAddress() | ||
| { | ||
| address = addrInfo.addressUnformatted() | ||
| selectedWallet = wallet | ||
| showRawAddress = false | ||
| showingWalletPicker = false | ||
| } |
There was a problem hiding this comment.
WalletManager is constructed and used to fetch the address, but there is no close() call (unlike the Android counterpart at EnterAddressView.kt:303 which calls wm.close()). Since WalletManager wraps a native Rust object via UniFFI, skipping cleanup leaks the underlying file handles / DB connections each time the user selects a wallet.
Task {
guard let wm = try? WalletManager(id: wallet.id) else { return }
defer { wm.close() } // add this
guard let addrInfo = try? await wm.firstAddress() else { return }
address = addrInfo.addressUnformatted()
selectedWallet = wallet
showRawAddress = false
showingWalletPicker = false
}| .sheet(isPresented: $showingWalletPicker) { | ||
| NavigationView { | ||
| List { | ||
| let wallets = (try? Database().wallets().allSortedActive()) ?? [] |
There was a problem hiding this comment.
Database().wallets().allSortedActive() is placed directly inside the List body with no memoization. In SwiftUI, the body property is called on every state change (e.g., the showingWalletPicker sheet being animated open/closed), so this synchronous database call fires on the main thread repeatedly. Consider loading the wallet list once into a @State variable via .task on the sheet.
| coroutineScope.launch { | ||
| try { | ||
| val wm = WalletManager(wallet.id) | ||
| val addressInfo = wm.firstAddress() | ||
| address = addressInfo.address().unformatted() | ||
| onAddressChanged(address) | ||
| selectedWallet = wallet | ||
| showRawAddress = false | ||
| showingWalletPicker = false | ||
| wm.close() | ||
| } catch (e: Exception) { | ||
| Log.w(tag, "Error wallet items: ${e.message}") | ||
| } |
There was a problem hiding this comment.
When WalletManager(wallet.id) or wm.firstAddress() throws, the error is logged but the bottom sheet remains open with no indication to the user that the selection failed. The same gap exists in the iOS code (try? swallows errors silently). Consider showing a toast or Snackbar so the user knows they need to retry.
| val addressInfo = wm.firstAddress() | ||
| address = addressInfo.address().unformatted() |
There was a problem hiding this comment.
firstAddress() reuses the same address on repeated sends
Both platforms call firstAddress() / wm.firstAddress() to populate the destination. HD wallets derive a fresh address per receipt for on-chain privacy. If the user sends to the same wallet multiple times, they always use the same address, linking transactions. Consider using nextUnusedAddress() (or equivalent) if the API provides it.
There was a problem hiding this comment.
@oyetanishq i think we should be using nextAddress not first address
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/EnterAddressView.kt (1)
146-167: Avoid!!on avar— capture to a localvalfor a clean smart-cast.
selectedWalletis avar, so the compiler can't smart-cast after the!= nullcheck, forcingselectedWallet!!.name. Idiomatic Kotlin is to capture to a localvaland branch on that.- if (selectedWallet != null) { + val dest = selectedWallet + if (dest != null) { Column(modifier = Modifier.fillMaxWidth()) { ... Text( - selectedWallet!!.name, + dest.name,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/EnterAddressView.kt` around lines 146 - 167, The code uses selectedWallet!! inside the UI block because selectedWallet is a var and not smart-cast; fix this by immediately capturing it into a local val (e.g., val wallet = selectedWallet) after the null-check and use wallet.name (and any other wallet fields) throughout the Column/Row/Text block (replace selectedWallet!! occurrences). Update any references inside EnterAddressView's UI block to use the local val to avoid the !! operator.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/EnterAddressView.kt`:
- Around line 185-198: The "Show address" control currently only sets
showRawAddress = true and never collapses; update the clickable handler to
toggle showRawAddress instead of only setting true, and update the UI
text/content to reflect both states (e.g., show "Hide address" or the raw
address when showRawAddress is true, and "Show address" when false). Locate the
branch around showRawAddress in EnterAddressView (the Text composable with
modifier = Modifier.clickable { ... }) and change the onClick to toggle the
Boolean and adjust the visible text/content so the control mirrors a reveal/hide
toggle.
- Around line 280-286: The current catch in the remember block that reads
Database().wallets().allSortedActive() swallows exceptions and returns an empty
list; change the catch to log the throwable (not just .message) using the app's
logging utility (e.g., Timber.e or Log.e) including a clear context message like
"Failed to load wallets" and the caught exception, then return emptyList() as
fallback; update the catch around Database().wallets().allSortedActive() in
EnterAddressView's wallets remember block to include that logging of the
throwable.
- Around line 293-308: The WalletManager instance (WalletManager(wallet.id)) may
leak on exceptions because wm.close() is only called on the success path; wrap
its lifetime so it is always closed (either use Kotlin's use { } on
WalletManager if it implements Closeable/AutoCloseable, or create a try { ... }
finally { wm.close() } around the calls to firstAddress() and downstream logic
in the clickable coroutineScope block) and move the state updates (address,
onAddressChanged, selectedWallet, showRawAddress, showingWalletPicker) inside
that protected scope; also change the catch logging to pass the exception object
as the third argument to Log.w (e.g., Log.w(tag, "Error fetching first address
for ${wallet.id}", e)).
In `@ios/Cove/Flows/SendFlow/SetAmountScreen/EnterAddressView.swift`:
- Around line 72-82: The current UI in EnterAddressView uses a one-way reveal
controlled by the showRawAddress Bool so once shown it cannot be hidden; change
the Button logic to toggle showRawAddress instead of only setting it true and
update the button label to show "Show address" when showRawAddress is false and
"Hide address" when true (the Text(address) branch stays the same), ensuring the
Button that currently reads "Show address" toggles the state; locate
showRawAddress and the Button in EnterAddressView.swift to implement this simple
toggle behavior.
- Around line 95-122: Replace the deprecated NavigationView with NavigationStack
for the wallet picker sheet; hoist the database query out of the List into an
`@State` var (e.g. wallets: [Wallet] = []) and populate it in a .task when
showingWalletPicker (call Database().wallets().allSortedActive() once) instead
of calling it inside the List body; remove the try? that swallows errors around
WalletManager(id:) and await wm.firstAddress() and surface failures via the
app's alertState (or at least log them) so that failures update alertState and
present to the user instead of silently failing; keep the existing assignments
to address, selectedWallet, showRawAddress and showingWalletPicker on success.
---
Nitpick comments:
In
`@android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/EnterAddressView.kt`:
- Around line 146-167: The code uses selectedWallet!! inside the UI block
because selectedWallet is a var and not smart-cast; fix this by immediately
capturing it into a local val (e.g., val wallet = selectedWallet) after the
null-check and use wallet.name (and any other wallet fields) throughout the
Column/Row/Text block (replace selectedWallet!! occurrences). Update any
references inside EnterAddressView's UI block to use the local val to avoid the
!! operator.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e07c400e-1c67-40db-83fb-455279fdeac7
📒 Files selected for processing (3)
android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/EnterAddressView.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowSetAmountScreen.ktios/Cove/Flows/SendFlow/SetAmountScreen/EnterAddressView.swift
| if (showRawAddress) { | ||
| Text( | ||
| text = address, | ||
| color = MaterialTheme.colorScheme.onSurfaceVariant, | ||
| fontSize = 12.sp, | ||
| ) | ||
| } else { | ||
| Text( | ||
| text = "Show address", | ||
| color = CoveColor.LinkBlue, | ||
| fontSize = 12.sp, | ||
| modifier = Modifier.clickable { showRawAddress = true } | ||
| ) | ||
| } |
There was a problem hiding this comment.
"Show address" is one-way — mirror a hide/show toggle.
Once tapped, showRawAddress stays true with no way back to the collapsed state. A simple toggle is more user-friendly and matches the reveal/hide pattern used for sensitive data elsewhere.
- if (showRawAddress) {
- Text(
- text = address,
- color = MaterialTheme.colorScheme.onSurfaceVariant,
- fontSize = 12.sp,
- )
- } else {
- Text(
- text = "Show address",
- color = CoveColor.LinkBlue,
- fontSize = 12.sp,
- modifier = Modifier.clickable { showRawAddress = true }
- )
- }
+ Text(
+ text = if (showRawAddress) "Hide address" else "Show address",
+ color = CoveColor.LinkBlue,
+ fontSize = 12.sp,
+ modifier = Modifier.clickable { showRawAddress = !showRawAddress }
+ )
+ if (showRawAddress) {
+ Spacer(Modifier.height(4.dp))
+ Text(
+ text = address,
+ color = MaterialTheme.colorScheme.onSurfaceVariant,
+ fontSize = 12.sp,
+ )
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (showRawAddress) { | |
| Text( | |
| text = address, | |
| color = MaterialTheme.colorScheme.onSurfaceVariant, | |
| fontSize = 12.sp, | |
| ) | |
| } else { | |
| Text( | |
| text = "Show address", | |
| color = CoveColor.LinkBlue, | |
| fontSize = 12.sp, | |
| modifier = Modifier.clickable { showRawAddress = true } | |
| ) | |
| } | |
| Text( | |
| text = if (showRawAddress) "Hide address" else "Show address", | |
| color = CoveColor.LinkBlue, | |
| fontSize = 12.sp, | |
| modifier = Modifier.clickable { showRawAddress = !showRawAddress } | |
| ) | |
| if (showRawAddress) { | |
| Spacer(Modifier.height(4.dp)) | |
| Text( | |
| text = address, | |
| color = MaterialTheme.colorScheme.onSurfaceVariant, | |
| fontSize = 12.sp, | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/EnterAddressView.kt`
around lines 185 - 198, The "Show address" control currently only sets
showRawAddress = true and never collapses; update the clickable handler to
toggle showRawAddress instead of only setting true, and update the UI
text/content to reflect both states (e.g., show "Hide address" or the raw
address when showRawAddress is true, and "Show address" when false). Locate the
branch around showRawAddress in EnterAddressView (the Text composable with
modifier = Modifier.clickable { ... }) and change the onClick to toggle the
Boolean and adjust the visible text/content so the control mirrors a reveal/hide
toggle.
| val wallets = remember { | ||
| try { | ||
| Database().wallets().allSortedActive().filter { it.id != currentWalletId } | ||
| } catch (e: Exception) { | ||
| emptyList() | ||
| } | ||
| } |
There was a problem hiding this comment.
Swallowed exception when loading wallets.
detekt flagged this correctly: on a DB read failure the user sees an empty list with no diagnostic. Log the exception (with the throwable, not just .message) so failures are diagnosable in the field.
val wallets = remember {
try {
Database().wallets().allSortedActive().filter { it.id != currentWalletId }
} catch (e: Exception) {
+ Log.w(tag, "Failed to load wallets for picker", e)
emptyList()
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val wallets = remember { | |
| try { | |
| Database().wallets().allSortedActive().filter { it.id != currentWalletId } | |
| } catch (e: Exception) { | |
| emptyList() | |
| } | |
| } | |
| val wallets = remember { | |
| try { | |
| Database().wallets().allSortedActive().filter { it.id != currentWalletId } | |
| } catch (e: Exception) { | |
| Log.w(tag, "Failed to load wallets for picker", e) | |
| emptyList() | |
| } | |
| } |
🧰 Tools
🪛 detekt (1.23.8)
[warning] 283-283: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/EnterAddressView.kt`
around lines 280 - 286, The current catch in the remember block that reads
Database().wallets().allSortedActive() swallows exceptions and returns an empty
list; change the catch to log the throwable (not just .message) using the app's
logging utility (e.g., Timber.e or Log.e) including a clear context message like
"Failed to load wallets" and the caught exception, then return emptyList() as
fallback; update the catch around Database().wallets().allSortedActive() in
EnterAddressView's wallets remember block to include that logging of the
throwable.
| .clickable { | ||
| coroutineScope.launch { | ||
| try { | ||
| val wm = WalletManager(wallet.id) | ||
| val addressInfo = wm.firstAddress() | ||
| address = addressInfo.address().unformatted() | ||
| onAddressChanged(address) | ||
| selectedWallet = wallet | ||
| showRawAddress = false | ||
| showingWalletPicker = false | ||
| wm.close() | ||
| } catch (e: Exception) { | ||
| Log.w(tag, "Error wallet items: ${e.message}") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm WalletManager implements AutoCloseable (so .use{} or .close() is required/available)
rg -nP --type=kotlin -C2 '\bclass\s+WalletManager\b'
rg -nP -C3 'WalletManager' -g '*.kt' | rg -i 'AutoCloseable|Closeable|close\(\)' | head -40Repository: bitcoinppl/cove
Length of output: 2886
WalletManager resource leak on the failure path — ensure .close() is always called.
wm.close() only executes after firstAddress() succeeds. If firstAddress() or any intermediate call throws, control jumps to the catch block and the WalletManager UniFFI handle is never released. Per the FFI Resource Management guideline, Closeable UniFFI objects must always be closed. Additionally, Log.w(tag, "Error wallet items: ${e.message}") should pass the exception object as the second argument: Log.w(tag, "Error wallet items: ${e.message}", e).
🛡️ Proposed fix using try/finally
.clickable {
coroutineScope.launch {
- try {
- val wm = WalletManager(wallet.id)
- val addressInfo = wm.firstAddress()
- address = addressInfo.address().unformatted()
- onAddressChanged(address)
- selectedWallet = wallet
- showRawAddress = false
- showingWalletPicker = false
- wm.close()
- } catch (e: Exception) {
- Log.w(tag, "Error wallet items: ${e.message}")
- }
+ val wm = try {
+ WalletManager(wallet.id)
+ } catch (e: Exception) {
+ Log.w(tag, "Failed to open wallet ${wallet.id}", e)
+ return@launch
+ }
+ try {
+ val addressInfo = wm.firstAddress()
+ address = addressInfo.address().unformatted()
+ onAddressChanged(address)
+ selectedWallet = wallet
+ showRawAddress = false
+ showingWalletPicker = false
+ } catch (e: Exception) {
+ Log.w(tag, "Error fetching first address for ${wallet.id}", e)
+ } finally {
+ wm.close()
+ }
+ }
}Alternatively, use .use {} if WalletManager implements AutoCloseable (it does via Closeable):
WalletManager(wallet.id).use { wm ->
try {
val addressInfo = wm.firstAddress()
address = addressInfo.address().unformatted()
onAddressChanged(address)
selectedWallet = wallet
showRawAddress = false
showingWalletPicker = false
} catch (e: Exception) {
Log.w(tag, "Error fetching first address for ${wallet.id}", e)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .clickable { | |
| coroutineScope.launch { | |
| try { | |
| val wm = WalletManager(wallet.id) | |
| val addressInfo = wm.firstAddress() | |
| address = addressInfo.address().unformatted() | |
| onAddressChanged(address) | |
| selectedWallet = wallet | |
| showRawAddress = false | |
| showingWalletPicker = false | |
| wm.close() | |
| } catch (e: Exception) { | |
| Log.w(tag, "Error wallet items: ${e.message}") | |
| } | |
| } | |
| } | |
| .clickable { | |
| coroutineScope.launch { | |
| val wm = try { | |
| WalletManager(wallet.id) | |
| } catch (e: Exception) { | |
| Log.w(tag, "Failed to open wallet ${wallet.id}", e) | |
| return@launch | |
| } | |
| try { | |
| val addressInfo = wm.firstAddress() | |
| address = addressInfo.address().unformatted() | |
| onAddressChanged(address) | |
| selectedWallet = wallet | |
| showRawAddress = false | |
| showingWalletPicker = false | |
| } catch (e: Exception) { | |
| Log.w(tag, "Error fetching first address for ${wallet.id}", e) | |
| } finally { | |
| wm.close() | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/EnterAddressView.kt`
around lines 293 - 308, The WalletManager instance (WalletManager(wallet.id))
may leak on exceptions because wm.close() is only called on the success path;
wrap its lifetime so it is always closed (either use Kotlin's use { } on
WalletManager if it implements Closeable/AutoCloseable, or create a try { ... }
finally { wm.close() } around the calls to firstAddress() and downstream logic
in the clickable coroutineScope block) and move the state updates (address,
onAddressChanged, selectedWallet, showRawAddress, showingWalletPicker) inside
that protected scope; also change the catch logging to pass the exception object
as the third argument to Log.w (e.g., Log.w(tag, "Error fetching first address
for ${wallet.id}", e)).
| if showRawAddress { | ||
| Text(address) | ||
| .font(.caption) | ||
| .foregroundColor(.secondary) | ||
| } else { | ||
| Button("Show address") { | ||
| showRawAddress = true | ||
| } | ||
| .font(.caption) | ||
| .foregroundColor(.blue) | ||
| } |
There was a problem hiding this comment.
"Show address" is one-way — no way to hide it again.
Once the user taps "Show address", showRawAddress stays true for the rest of the flow (only reset on wallet selection). Consider making this a toggle (Show address ↔ Hide address) so users can collapse the raw string again after verifying, matching typical reveal/hide patterns for sensitive fields.
🔁 Proposed toggle
- if showRawAddress {
- Text(address)
- .font(.caption)
- .foregroundColor(.secondary)
- } else {
- Button("Show address") {
- showRawAddress = true
- }
- .font(.caption)
- .foregroundColor(.blue)
- }
+ Button(showRawAddress ? "Hide address" : "Show address") {
+ showRawAddress.toggle()
+ }
+ .font(.caption)
+ .foregroundColor(.blue)
+ if showRawAddress {
+ Text(address)
+ .font(.caption)
+ .foregroundColor(.secondary)
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ios/Cove/Flows/SendFlow/SetAmountScreen/EnterAddressView.swift` around lines
72 - 82, The current UI in EnterAddressView uses a one-way reveal controlled by
the showRawAddress Bool so once shown it cannot be hidden; change the Button
logic to toggle showRawAddress instead of only setting it true and update the
button label to show "Show address" when showRawAddress is false and "Hide
address" when true (the Text(address) branch stays the same), ensuring the
Button that currently reads "Show address" toggles the state; locate
showRawAddress and the Button in EnterAddressView.swift to implement this simple
toggle behavior.
| .sheet(isPresented: $showingWalletPicker) { | ||
| NavigationView { | ||
| List { | ||
| let wallets = (try? Database().wallets().allSortedActive()) ?? [] | ||
| ForEach(wallets.filter { $0.id != presenter.manager.id }, id: \.id) { wallet in | ||
| Button(action: { | ||
| Task { | ||
| if let wm = try? WalletManager(id: wallet.id), | ||
| let addrInfo = try? await wm.firstAddress() | ||
| { | ||
| address = addrInfo.addressUnformatted() | ||
| selectedWallet = wallet | ||
| showRawAddress = false | ||
| showingWalletPicker = false | ||
| } | ||
| } | ||
| }) { | ||
| HStack { | ||
| Text(wallet.name) | ||
| Spacer() | ||
| } | ||
| } | ||
| } | ||
| } | ||
| .navigationTitle("Select Wallet") | ||
| .navigationBarItems(trailing: Button("Cancel") { showingWalletPicker = false }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is NavigationView deprecated in iOS 16+ and should new SwiftUI code use NavigationStack?
💡 Result:
Yes, NavigationView is deprecated in iOS 16+ (and equivalent versions on other platforms). Apple recommends that new SwiftUI code targeting iOS 16+ use NavigationStack for stack-based navigation (replacing NavigationView with .stack style) and NavigationSplitView for multi-column navigation.
Citations:
- 1: https://developer.apple.com/documentation/swiftui/migrating-to-new-navigation-types
- 2: https://developer.apple.com/documentation/swiftui/navigationstack
- 3: https://developer.apple.com/documentation/swiftui/navigation
🏁 Script executed:
# Find iOS deployment target
find . -name "*.xcodeproj" -o -name "project.pbxproj" -o -name "*.plist" | head -20Repository: bitcoinppl/cove
Length of output: 390
🏁 Script executed:
# Check for iOS minimum version in common config files
rg "IPHONEOS_DEPLOYMENT_TARGET|platform.*ios|mobileOS" --type swift --type yaml --type json ios/ | head -20Repository: bitcoinppl/cove
Length of output: 41
🏁 Script executed:
# Search for NavigationStack usage in the codebase
rg "NavigationStack" ios/ --type swift | head -20Repository: bitcoinppl/cove
Length of output: 1660
🏁 Script executed:
# Search for NavigationView usage patterns
rg "NavigationView" ios/ --type swift | head -20Repository: bitcoinppl/cove
Length of output: 150
🏁 Script executed:
# Look for similar database query patterns
rg "Database()\." ios/ --type swift -A 2 | head -30Repository: bitcoinppl/cove
Length of output: 475
🏁 Script executed:
# Extract iOS deployment target from project.pbxproj
grep -i "IPHONEOS_DEPLOYMENT_TARGET" ios/Cove.xcodeproj/project.pbxproj | head -5Repository: bitcoinppl/cove
Length of output: 214
🏁 Script executed:
# Read the actual EnterAddressView.swift file
wc -l ios/Cove/Flows/SendFlow/SetAmountScreen/EnterAddressView.swiftRepository: bitcoinppl/cove
Length of output: 125
🏁 Script executed:
# Search for `@State` patterns with task/onAppear for lazy loading
rg "@State.*\n.*task\(|@State.*\n.*onAppear" ios/Cove --type swift -A 3 | head -30Repository: bitcoinppl/cove
Length of output: 250
🏁 Script executed:
# Look for error handling patterns in sheets
rg "\.sheet.*\{|\.alert.*error" ios/Cove --type swift -B 2 -A 5 | head -40Repository: bitcoinppl/cove
Length of output: 3069
🏁 Script executed:
# Read the full EnterAddressView.swift file
cat -n ios/Cove/Flows/SendFlow/SetAmountScreen/EnterAddressView.swiftRepository: bitcoinppl/cove
Length of output: 6523
🏁 Script executed:
# Search for `@State` patterns that load data
rg "@State.*var.*=" ios/Cove --type swift -A 2 | grep -A 2 "task\|onAppear" | head -30Repository: bitcoinppl/cove
Length of output: 620
🏁 Script executed:
# Check how errors are typically surfaced in this app (alertState pattern)
rg "app.alertState.*=" ios/Cove --type swift | head -15Repository: bitcoinppl/cove
Length of output: 2026
Replace deprecated NavigationView with NavigationStack and surface errors in wallet picker sheet.
The wallet picker sheet has three issues:
NavigationViewis deprecated as of iOS 16 and this project targets iOS 18.4; useNavigationStackinstead.- The database query on line 98 (
Database().wallets().allSortedActive()) executes inside theListbody and re-runs on every sheet body re-evaluation. Hoist it to an@Stateproperty and load it in.taskto execute once when the sheet appears. - Lines 102–103 silently swallow errors from
WalletManagerinitialization and address derivation withtry?. If either fails, the user sees no feedback. Surface errors using the app'salertStatepattern or at minimum log them.
♻️ Suggested refactor
- `@State` private var showingWalletPicker: Bool = false
+ `@State` private var showingWalletPicker: Bool = false
+ `@State` private var pickerWallets: [WalletMetadata] = []
+ `@State` private var pickerError: String?
...
- .sheet(isPresented: $showingWalletPicker) {
- NavigationView {
- List {
- let wallets = (try? Database().wallets().allSortedActive()) ?? []
- ForEach(wallets.filter { $0.id != presenter.manager.id }, id: \.id) { wallet in
+ .sheet(isPresented: $showingWalletPicker) {
+ NavigationStack {
+ List {
+ ForEach(pickerWallets.filter { $0.id != presenter.manager.id }, id: \.id) { wallet in
Button(action: {
Task {
- if let wm = try? WalletManager(id: wallet.id),
- let addrInfo = try? await wm.firstAddress()
- {
+ do {
+ let wm = try WalletManager(id: wallet.id)
+ let addrInfo = try await wm.firstAddress()
address = addrInfo.addressUnformatted()
selectedWallet = wallet
showRawAddress = false
showingWalletPicker = false
+ } catch {
+ pickerError = error.localizedDescription
}
}
}) { ... }
}
}
+ .task {
+ pickerWallets = (try? Database().wallets().allSortedActive()) ?? []
+ }
.navigationTitle("Select Wallet")
.navigationBarItems(trailing: Button("Cancel") { showingWalletPicker = false })
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ios/Cove/Flows/SendFlow/SetAmountScreen/EnterAddressView.swift` around lines
95 - 122, Replace the deprecated NavigationView with NavigationStack for the
wallet picker sheet; hoist the database query out of the List into an `@State` var
(e.g. wallets: [Wallet] = []) and populate it in a .task when
showingWalletPicker (call Database().wallets().allSortedActive() once) instead
of calling it inside the List body; remove the try? that swallows errors around
WalletManager(id:) and await wm.firstAddress() and surface failures via the
app's alertState (or at least log them) so that failures update alertState and
present to the user instead of silently failing; keep the existing assignments
to address, selectedWallet, showRawAddress and showingWalletPicker on success.
|
Thanks @oyetanishq this looks good, please address some of the issues, but I will work with the designer to improve the design before merging, so I will turn this into a draft PR for now. |
praveenperera
force-pushed
the
self-transfer
branch
from
1352321 to
a74c85a
Compare
praveenperera
added
needs design
2 participants
Summary
self-transferof funds between different imported wallet.Show addresstoggle, allowing users to manually verify the underlying raw address.Images
Testing
Platform Coverage
Checklist
Summary by CodeRabbit