Skip to content

fix(swift-sdk): fixed ios app transaction display#3081

Merged
QuantumExplorer merged 1 commit intov3.1-devfrom
fix/ios-app-transaction-display
Mar 17, 2026
Merged

fix(swift-sdk): fixed ios app transaction display#3081
QuantumExplorer merged 1 commit intov3.1-devfrom
fix/ios-app-transaction-display

Conversation

@ZocoLini
Copy link
Copy Markdown
Collaborator

@ZocoLini ZocoLini commented Feb 12, 2026

The transactions list view was messy and trying to display information without following the FFI specification:

  • Made the UI more compact and elegant imo
  • if a transaction is confirmed is decided following the FFI documentation, removing in the process the half confirmed state, that's an information not provided by the FFI

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Summary by CodeRabbit

  • Refactor

    • Confirmation shown as "Pending" or "Confirmed" instead of numeric counts
    • Transaction model simplified (fewer metadata fields) and retrieval no longer throws
  • Wallet

    • Wallet creation and persistence flow improved; seeds are generated and stored with stronger persistence and error handling
  • UI

    • Transaction list streamlined to a tappable list, removing separate loading/error states
    • Transaction type, icon, and color now derive from amount sign
    • Fees and block height render only when applicable

@ZocoLini ZocoLini marked this pull request as draft February 12, 2026 19:25
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 12, 2026

Warning

Rate limit exceeded

@ZocoLini has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 21 minutes and 32 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f8e9acbd-532f-4271-b879-598c04500415

📥 Commits

Reviewing files that changed from the base of the PR and between abcb665 and b673473.

📒 Files selected for processing (4)
  • packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionDetailView.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift
📝 Walkthrough

Walkthrough

Wallet transaction model and APIs were simplified (removed confirmations and type, made height non-optional, and getTransactions() non-throwing). Core wallet creation flow updated to compute and persist birthHeight and to generate/store seed during creation. UI adjusted to derive direction from netAmount, use isConfirmed, and remove loading/error UI around transaction lists.

Changes

Cohort / File(s) Summary
Core wallet creation & persistence
packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift
Wallet creation now computes birthHeight (based on isImport and network), uses addWalletAndSerialize with birthHeight, generates and stores seed with explicit error handling, and inserts/persists the wallet into SwiftData context. Minor layout/flow adjustments around related helpers.
Managed account & transaction model
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swift
getTransactions signature changed to non-throwing, parameterless getTransactions() -> [WalletTransaction]. WalletTransaction simplified: height is UInt32 (non-optional); removed confirmations and type; isConfirmed now checks blockHash != nil && height > 0. Error paths use preconditionFailure on FFI failure.
Transaction detail UI
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionDetailView.swift
Direction (label, icon, color) now derived from netAmount (positive = received, negative = sent, else self-transfer). Status uses isConfirmed. Block height rendering uses height != 0. Fee shown when netAmount < 0. Updated initializer usage to match simplified WalletTransaction.
Transaction list UI
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift
Removed loading/error state properties and related UI; loadTransactions() is now synchronous and uses walletManager.getTransactions(for:). TransactionRowView updated to use netAmount for icon/color and isConfirmed for status badge. Layout and fee display adjusted; selection opens TransactionDetailView.

Sequence Diagram(s)

sequenceDiagram
    participant UI as Client/UI
    participant Manager as CoreWalletManager
    participant Engine as WalletEngine/FFI
    participant Store as SwiftData/Storage

    UI->>Manager: createWallet(params, isImport, network)
    Manager->>Engine: compute/addWalletAndSerialize(..., birthHeight)
    Engine-->>Manager: walletHandle
    Manager->>Engine: generateSeedIfNeeded(walletHandle)
    Engine-->>Manager: seedData
    Manager->>Store: insert Wallet entity (walletHandle, birthHeight, seedData)
    Store-->>Manager: persist ack
    Manager-->>UI: created wallet metadata
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I hop through code with whiskers bright,
Heights now certain, types take flight,
Net amounts guide each arrow's turn,
Seeds tucked safe where snapshots burn,
A little rabbit cheers the night. 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(swift-sdk): fixed ios app transaction display' accurately describes the main change—fixing iOS app transaction display to align with FFI specifications and improve UI clarity.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ios-app-transaction-display
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 12, 2026

✅ DashSDKFFI.xcframework built for this PR.

SwiftPM (host the zip at a stable URL, then use):

.binaryTarget(
  name: "DashSDKFFI",
  url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
  checksum: "4197a89b31fc701b8ff26cd306f22f33875c25cd4df5219cb5bc440ffcc9caf6"
)

Xcode manual integration:

  • Download 'DashSDKFFI.xcframework' artifact from the run link above.
  • Drag it into your app target (Frameworks, Libraries & Embedded Content) and set Embed & Sign.
  • If using the Swift wrapper package, point its binaryTarget to the xcframework location or add the package and place the xcframework at the expected path.

Base automatically changed from feat/iOSSupport to v3.1-dev March 4, 2026 02:23
@github-actions github-actions Bot added this to the v3.1.0 milestone Mar 4, 2026
@ZocoLini ZocoLini force-pushed the fix/ios-app-transaction-display branch 3 times, most recently from 17ecb5b to e3889aa Compare March 16, 2026 19:01
@ZocoLini ZocoLini marked this pull request as ready for review March 16, 2026 22:43
@ZocoLini ZocoLini requested a review from shumkov as a code owner March 16, 2026 22:43
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swift (1)

62-69: ⚠️ Potential issue | 🟠 Major

assert is stripped in release builds - FFI failure would be silent.

assert(success, ...) only triggers in debug builds. In release builds with optimization (-O), if managed_core_account_get_transactions returns false, the code continues silently with potentially invalid state. Consider using precondition (which is kept in release builds) or returning an empty array with a logged warning.

🔒 Recommended fix using precondition
         let success = managed_core_account_get_transactions(handle, &transactionsPtr, &count)

-        assert(success, "This can only fail if any pointer is nil")
+        guard success else {
+            // FFI contract: this can only fail if any pointer is nil, which shouldn't happen
+            // Return empty rather than crash, but log for debugging
+            assertionFailure("managed_core_account_get_transactions failed unexpectedly")
+            return []
+        }

Or if the failure truly indicates a programming error that should never occur:

-        assert(success, "This can only fail if any pointer is nil")
+        precondition(success, "managed_core_account_get_transactions failed - invalid handle state")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swift`
around lines 62 - 69, Replace the debug-only assert on the FFI call so failures
aren’t ignored in release: in ManagedAccount.swift where you call
managed_core_account_get_transactions (and currently do assert(success, ...)),
either use precondition(success, ...) to ensure the program traps in release
builds or handle the false case by logging a warning and returning an empty
array; update the code around
managed_core_account_get_transactions/transactionsPtr/count to explicitly check
success and act accordingly instead of relying on assert.
🧹 Nitpick comments (3)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift (2)

54-74: Consider extracting shared type logic to WalletTransaction.

The typeIcon and typeColor computed properties duplicate logic from TransactionDetailView. Moving these to a WalletTransaction extension would centralize the logic.

♻️ Optional refactor: Add to WalletTransaction
// In WalletTransaction extension or struct
public var typeIcon: String {
    switch netAmount {
    case let amount where amount > 0: return "arrow.down.circle.fill"
    case let amount where amount < 0: return "arrow.up.circle.fill"
    default: return "arrow.triangle.2.circlepath"
    }
}

public var typeColor: Color {
    switch netAmount {
    case let amount where amount > 0: return .green
    case let amount where amount < 0: return .red
    default: return .blue
    }
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift`
around lines 54 - 74, Move the duplicated computed-property logic for icons and
colors out of TransactionListView and TransactionDetailView into
WalletTransaction so it's centralized: create public computed properties (e.g.,
typeIcon and typeColor) on WalletTransaction that use transaction.netAmount to
decide the icon and Color, then replace the local computed properties in
TransactionListView and any similar logic in TransactionDetailView to use the
new WalletTransaction.typeIcon and WalletTransaction.typeColor. Ensure you keep
the existing symbol names (typeIcon, typeColor) so callers need only switch to
accessing them on the WalletTransaction instance.

44-46: Synchronous FFI call on main thread.

loadTransactions() performs a synchronous FFI call directly on the main thread. For an example app with quick operations this is acceptable, but for production usage consider wrapping in a Task { @mainactor in } with the actual work dispatched to a background thread to prevent UI blocking during larger transaction sets.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift`
around lines 44 - 46, loadTransactions currently calls
walletService.walletManager.getTransactions(for: wallet) synchronously on the
main thread; change it to perform the FFI call off the main thread and only
update self.transactions on the main actor — for example, wrap the operation in
Task { } or Task.detached to call getTransactions on a background thread, await
the result, then assign to self.transactions inside Task { `@MainActor` in ... }
(refer to loadTransactions, walletService, and walletManager.getTransactions) so
the heavy FFI work does not block the UI.
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionDetailView.swift (1)

64-67: Consider simplifying the conditional for readability.

The double negation !transaction.isConfirmed ? "Pending" : "Confirmed" is correct but slightly harder to parse. Flipping the order improves clarity.

♻️ Suggested improvement
                         TransactionDetailRow(
                             label: "Status",
-                            value: !transaction.isConfirmed ? "Pending" : "Confirmed"
+                            value: transaction.isConfirmed ? "Confirmed" : "Pending"
                         )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionDetailView.swift`
around lines 64 - 67, The ternary in TransactionDetailRow currently uses a
double negation (!transaction.isConfirmed ? "Pending" : "Confirmed"); replace it
with a direct check on transaction.isConfirmed (transaction.isConfirmed ?
"Confirmed" : "Pending") in the TransactionDetailView so the intent is clearer
and easier to read.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swift`:
- Around line 55-57: The docstring for the transactions getter in
ManagedAccount.swift mentions a removed parameter `currentHeight`; update the
comment for the method (the "Get all transactions for this account" docblock) to
remove or replace the `- Parameter currentHeight:` line so it matches the
current function signature (e.g., the method named getAllTransactions /
transactions retrieval function in ManagedAccount). Ensure the Returns
description remains accurate and no references to `currentHeight` remain in the
docblock.

---

Outside diff comments:
In `@packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swift`:
- Around line 62-69: Replace the debug-only assert on the FFI call so failures
aren’t ignored in release: in ManagedAccount.swift where you call
managed_core_account_get_transactions (and currently do assert(success, ...)),
either use precondition(success, ...) to ensure the program traps in release
builds or handle the false case by logging a warning and returning an empty
array; update the code around
managed_core_account_get_transactions/transactionsPtr/count to explicitly check
success and act accordingly instead of relying on assert.

---

Nitpick comments:
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionDetailView.swift`:
- Around line 64-67: The ternary in TransactionDetailRow currently uses a double
negation (!transaction.isConfirmed ? "Pending" : "Confirmed"); replace it with a
direct check on transaction.isConfirmed (transaction.isConfirmed ? "Confirmed" :
"Pending") in the TransactionDetailView so the intent is clearer and easier to
read.

In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift`:
- Around line 54-74: Move the duplicated computed-property logic for icons and
colors out of TransactionListView and TransactionDetailView into
WalletTransaction so it's centralized: create public computed properties (e.g.,
typeIcon and typeColor) on WalletTransaction that use transaction.netAmount to
decide the icon and Color, then replace the local computed properties in
TransactionListView and any similar logic in TransactionDetailView to use the
new WalletTransaction.typeIcon and WalletTransaction.typeColor. Ensure you keep
the existing symbol names (typeIcon, typeColor) so callers need only switch to
accessing them on the WalletTransaction instance.
- Around line 44-46: loadTransactions currently calls
walletService.walletManager.getTransactions(for: wallet) synchronously on the
main thread; change it to perform the FFI call off the main thread and only
update self.transactions on the main actor — for example, wrap the operation in
Task { } or Task.detached to call getTransactions on a background thread, await
the result, then assign to self.transactions inside Task { `@MainActor` in ... }
(refer to loadTransactions, walletService, and walletManager.getTransactions) so
the heavy FFI work does not block the UI.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0aa01f8b-3e09-44de-aee1-5fcf7fe132db

📥 Commits

Reviewing files that changed from the base of the PR and between 9ebc7e4 and e3889aa.

📒 Files selected for processing (4)
  • packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionDetailView.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift

Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This PR correctly simplifies the transaction model to align with the FFI spec, removing client-side confirmation counting and derived type fields. The only real concern is replacing a guard/throw with an assert that is stripped in release builds, silently converting FFI failures into empty transaction lists. The codex-general agent hallucinated three blocking findings by flagging pre-existing code not changed in this PR.

Reviewed commit: e3889aa

🟡 2 suggestion(s) | 💬 2 nitpick(s)

🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swift`:
- [SUGGESTION] lines 58-64: assert replaces guard/throw — FFI failures silently return empty array in release builds
  The old code used `guard success else { throw KeyWalletError.invalidState(...) }` which propagated FFI failures to callers. The new code uses `assert(success, ...)` which is stripped in optimized (release) builds, and the function signature changed from `throws -> [WalletTransaction]` to `-> [WalletTransaction]`, so callers can no longer handle the error at all.

In practice, because `transactionsPtr` is initialized to `nil` and `count` to `0`, the subsequent `guard count > 0, let ptr = transactionsPtr` will catch the failure and return `[]`. So there's no crash risk — but FFI failures are silently swallowed as empty results rather than surfaced as errors.

The developer comment suggests this path is believed to be impossible, but defensive coding would still handle it explicitly.

In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift`:
- [SUGGESTION] lines 17-30: Empty-state view was removed — new wallets see a blank list
  The previous implementation had an `emptyStateView` with an icon ('doc.text.magnifyingglass'), heading ('No Transactions Yet'), and helper text. This was removed entirely; a new wallet with no transactions now shows an empty List with no user guidance. The PR description says the UI was made 'more compact and elegant', so this may be intentional — but the empty state provides important UX for first-time users.

Comment thread packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swift Outdated
@ZocoLini ZocoLini force-pushed the fix/ios-app-transaction-display branch from e3889aa to 9b05176 Compare March 17, 2026 02:54
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift (1)

36-41: Consider async wrapper for FFI-backed transaction loading.

loadTransactions() synchronously calls walletManager.getTransactions(for:) which performs FFI calls (see CoreWalletManager.swift:152-158). When invoked from .task or .refreshable, this runs on the main thread and may cause brief UI jank if the wallet has many transactions.

For the example app this is likely acceptable, but for production use you may want to wrap the call in a Task.detached or move it off the main thread.

♻️ Optional async wrapper
-    private func loadTransactions() {
-      self.transactions = walletService.walletManager.getTransactions(for: wallet)
+    private func loadTransactions() async {
+        let txs = await Task.detached { [walletService, wallet] in
+            walletService.walletManager.getTransactions(for: wallet)
+        }.value
+        self.transactions = txs
     }

Note: This requires marking the call sites as await loadTransactions().

Also applies to: 44-46

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift`
around lines 36 - 41, loadTransactions() currently calls
walletManager.getTransactions(for:) synchronously (FFI-backed) from UI entry
points (.task and .refreshable), which can block the main thread; change the
call sites to invoke loadTransactions from a background task (e.g. Task.detached
{ await loadTransactions() } or Task { await loadTransactions() } depending on
desired priority) and make loadTransactions async so it awaits
walletManager.getTransactions(for:) off the main thread. Update the two call
sites shown (the .task and .refreshable blocks) to use await and ensure
loadTransactions(), and any callers, are marked async to propagate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift`:
- Around line 36-41: loadTransactions() currently calls
walletManager.getTransactions(for:) synchronously (FFI-backed) from UI entry
points (.task and .refreshable), which can block the main thread; change the
call sites to invoke loadTransactions from a background task (e.g. Task.detached
{ await loadTransactions() } or Task { await loadTransactions() } depending on
desired priority) and make loadTransactions async so it awaits
walletManager.getTransactions(for:) off the main thread. Update the two call
sites shown (the .task and .refreshable blocks) to use await and ensure
loadTransactions(), and any callers, are marked async to propagate the change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d6762219-371f-4e41-be84-c85bbf29441d

📥 Commits

Reviewing files that changed from the base of the PR and between e3889aa and 9b05176.

📒 Files selected for processing (4)
  • packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionDetailView.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift

Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Incremental push adds only .codecov.yml coverage exclusion patterns for generated gRPC bindings, data contract crates, version tables, and simple-signer. No application logic, FFI, or Swift changes. Nothing to flag.

Reviewed commit: 9b05176

@ZocoLini ZocoLini force-pushed the fix/ios-app-transaction-display branch 3 times, most recently from eebbebf to 6c96a03 Compare March 17, 2026 14:59
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift (1)

109-114: ⚠️ Potential issue | 🔴 Critical

Bug: importWallet doesn't pass isImport: true to createWallet, causing incorrect birthHeight.

The importWallet method calls createWallet without passing isImport: true, so birthHeight defaults to 0 instead of 730_000 for mainnet imports. This means imported wallets will sync from an incorrect starting point and may miss historical transactions or experience unnecessary sync delays.

Per the relevant context snippet (Context snippet 1), birthHeight is passed directly to wallet_manager_add_wallet_from_mnemonic_return_serialized_bytes() and sets the wallet's sync starting point.

🐛 Proposed fix
 func importWallet(label: String, network: AppNetwork, mnemonic: String, pin: String) async throws -> HDWallet {
-    let wallet = try await createWallet(label: label, mnemonic: mnemonic, pin: pin)
-    wallet.isImported = true
-    try modelContainer.mainContext.save()
+    let wallet = try await createWallet(label: label, mnemonic: mnemonic, pin: pin, isImport: true)
     return wallet
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift`
around lines 109 - 114, The importWallet function currently calls createWallet
without signaling an import, causing birthHeight to remain default; update
importWallet to call createWallet(label:network:mnemonic:pin:isImport:) with
isImport: true so the created HDWallet is given the correct birthHeight (e.g.,
730_000 for mainnet) used when calling
wallet_manager_add_wallet_from_mnemonic_return_serialized_bytes, then persist
and return the wallet as before.
🧹 Nitpick comments (1)
packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift (1)

150-159: Force-unwrap on FFI call may crash if wallet/account is not found.

try! will crash the app if getManagedAccount fails (e.g., invalid wallet ID, account not found). Consider using try? with a fallback or propagating the error.

♻️ Suggested refactor
-    public func getTransactions(for wallet: HDWallet, accountIndex: UInt32 = 0) -> [WalletTransaction] {
-        // Get managed account
-        let managedAccount = try! sdkWalletManager.getManagedAccount(
+    public func getTransactions(for wallet: HDWallet, accountIndex: UInt32 = 0) -> [WalletTransaction] {
+        guard let managedAccount = try? sdkWalletManager.getManagedAccount(
             walletId: wallet.walletId,
             accountIndex: accountIndex,
             accountType: .standardBIP44
-        )
-
+        ) else {
+            return []
+        }
         return managedAccount.getTransactions()
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift`
around lines 150 - 159, The getTransactions(for:accountIndex:) function uses a
force-try on the FFI call sdkWalletManager.getManagedAccount which can crash if
the wallet/account is missing; change this to safely handle errors by using try?
(or rethrow the error) and provide a fallback or propagate the error: replace
the force-try in getTransactions(for:accountIndex:) when calling
sdkWalletManager.getManagedAccount (and subsequently calling
managedAccount.getTransactions()) with a safe optional binding or throw from
getTransactions so callers can handle the failure, ensuring you reference
getTransactions(for:accountIndex:) and
getManagedAccount(walletId:accountIndex:accountType:) in the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift`:
- Around line 92-94: Fix the comment typo in CoreWalletManager by changing "ans"
to "and" in the inline comment that precedes the calls
modelContainer.mainContext.insert(wallet) and try
modelContainer.mainContext.save(); update the comment to read "// Insert wallet
into context and save it".

In `@packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swift`:
- Around line 63-66: The call site in ManagedAccount.getTransactions uses
preconditionFailure(success, ...) which always crashes; replace this misuse by
checking success properly: either use precondition(success, "Invalid state:
managed_core_account_get_transactions can only fail if any pointer is nil") so
it only traps when condition is false, or (preferred) use a guard around the FFI
call (managed_core_account_get_transactions) to handle a false success value
gracefully (returning an empty result or throwing) and log or surface the
pointer-nil failure instead of unconditionally crashing; update the
managed_core_account_get_transactions handling in getTransactions to use the
chosen defensive pattern.

In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift`:
- Line 52: Update the user-facing string in TransactionListView by changing the
Text view's literal from "Transaction will appear here once you send or receive
Dash" to use the plural "Transactions" so it reads "Transactions will appear
here once you send or receive Dash"; locate the Text(...) in
TransactionListView.swift (within the TransactionListView) and replace the
single-word noun to correct the grammar.
- Around line 41-73: The computed view properties emptyStateView and
transactionList are declared outside the TransactionListView struct; move both
properties inside the TransactionListView body so they become members of the
struct, ensuring they can access instance state like selectedTransaction and
sortedTransactions and use TransactionRowView; verify the closing brace for
TransactionListView is placed after these properties so the file compiles.

---

Outside diff comments:
In `@packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift`:
- Around line 109-114: The importWallet function currently calls createWallet
without signaling an import, causing birthHeight to remain default; update
importWallet to call createWallet(label:network:mnemonic:pin:isImport:) with
isImport: true so the created HDWallet is given the correct birthHeight (e.g.,
730_000 for mainnet) used when calling
wallet_manager_add_wallet_from_mnemonic_return_serialized_bytes, then persist
and return the wallet as before.

---

Nitpick comments:
In `@packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift`:
- Around line 150-159: The getTransactions(for:accountIndex:) function uses a
force-try on the FFI call sdkWalletManager.getManagedAccount which can crash if
the wallet/account is missing; change this to safely handle errors by using try?
(or rethrow the error) and provide a fallback or propagate the error: replace
the force-try in getTransactions(for:accountIndex:) when calling
sdkWalletManager.getManagedAccount (and subsequently calling
managedAccount.getTransactions()) with a safe optional binding or throw from
getTransactions so callers can handle the failure, ensuring you reference
getTransactions(for:accountIndex:) and
getManagedAccount(walletId:accountIndex:accountType:) in the fix.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3e5c5330-fecb-452e-85ac-12455b93a47d

📥 Commits

Reviewing files that changed from the base of the PR and between 9b05176 and abcb665.

📒 Files selected for processing (4)
  • packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift
  • packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionDetailView.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionDetailView.swift

Comment thread packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swift Outdated
@ZocoLini ZocoLini force-pushed the fix/ios-app-transaction-display branch from 6c96a03 to 83b88ac Compare March 17, 2026 15:03
@ZocoLini ZocoLini force-pushed the fix/ios-app-transaction-display branch from 83b88ac to b673473 Compare March 17, 2026 15:05
@QuantumExplorer QuantumExplorer merged commit d3791b2 into v3.1-dev Mar 17, 2026
34 checks passed
@QuantumExplorer QuantumExplorer deleted the fix/ios-app-transaction-display branch March 17, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants