Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .github/workflows/swift-example-app-ui-smoke.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,20 @@ name: Swift Example App UI Smoke

'on':
workflow_dispatch:
schedule:
# 23:00 UTC daily. Aligns with the main `Tests` workflow's nightly
# cron (also 23:00 UTC) but runs on a different runner pool
# (self-hosted macOS), so they don't compete for compute. The Swift
# discovery test hits testnet DAPI; if that becomes a contention
# signal, shift to 02:00 UTC.
- cron: "0 23 * * *"

concurrency:
# Prevents an in-flight manual dispatch from being clobbered by the
# cron firing (or vice versa) on the single self-hosted Mac. Cancels
# the older run in favor of the newer one.
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

permissions:
contents: read
Expand Down Expand Up @@ -161,13 +175,22 @@ jobs:
env:
SIM_UDID: ${{ steps.simulator.outputs.udid }}
RESULT_BUNDLE_PATH: ${{ runner.temp }}/SwiftExampleAppUITests.xcresult
# Forwarded into the XCUITest runner as `UI_TEST_MNEMONIC` —
# `xcodebuild test` strips the `TEST_RUNNER_` prefix before
# passing env vars through to the test process. Empty on PRs
# from forks (GitHub withholds secrets there), and the test
# self-skips when the value is empty.
TEST_RUNNER_UI_TEST_MNEMONIC: ${{ secrets.UI_TEST_MNEMONIC }}
run: |
set -euo pipefail
xcodebuild test \
-project SwiftExampleApp/SwiftExampleApp.xcodeproj \
-scheme SwiftExampleApp \
-destination "platform=iOS Simulator,id=$SIM_UDID" \
-only-testing:SwiftExampleAppUITests/SwiftExampleAppUITests/testCreateGeneratedWalletFlow \
-only-testing:SwiftExampleAppUITests/WalletPersistenceTests/testWalletPersistsAcrossRelaunch \
-only-testing:SwiftExampleAppUITests/WalletPersistenceTests/testWalletDeletionCleanupSurvivesRelaunch \
-only-testing:SwiftExampleAppUITests/CreditTransferTest/testImportWalletAndDiscoverIdentity \
-parallel-testing-enabled NO \
-maximum-concurrent-test-simulator-destinations 1 \
-resultBundlePath "$RESULT_BUNDLE_PATH"
Comment thread
llbartekll marked this conversation as resolved.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,27 @@ class AppState: ObservableObject {
@Published var showError = false
@Published var errorMessage = ""

/// `true` from the moment a network change is requested until the
/// new SDK is bound. Spans the full async cycle (didSet → Task →
/// `switchNetwork` → `sdk = newSDK`), so consumers can wait on it
/// as a real readiness signal. UI bindings should treat
/// `appState.sdk != nil && !isSwitchingNetwork` as "connected on
/// the current network" — `appState.sdk != nil` alone is true even
/// while `switchNetwork` is still tearing down the previous SDK.
@Published var isSwitchingNetwork: Bool = false

/// Monotonic request id for in-flight switches. If two switches
/// overlap (user taps mainnet → testnet before the first lands), the
/// earlier task's completion would otherwise clear `isSwitchingNetwork`
/// while the later switch is still running. Each new request bumps
/// this counter and the spawned task only clears the flag when its
/// captured id still matches.
private var networkSwitchRequestID: UInt64 = 0

@Published var currentNetwork: Network {
didSet {
UserDefaults.standard.set(Int(currentNetwork.rawValue), forKey: "currentNetwork")
Task {
await switchNetwork(to: currentNetwork)
}
UserDefaults.standard.set(currentNetwork.rawValue, forKey: "currentNetwork")
beginNetworkSwitch()
}
}

Expand All @@ -27,7 +42,23 @@ class AppState: ObservableObject {
UserDefaults.standard.set(useDockerSetup, forKey: "useLocalhostPlatform")
UserDefaults.standard.set(useDockerSetup, forKey: "useLocalhostCore")
UserDefaults.standard.set(useDockerSetup, forKey: "useLocalhost")
Task { await switchNetwork(to: currentNetwork) }
beginNetworkSwitch()
}
}

/// Bumps `networkSwitchRequestID`, raises `isSwitchingNetwork`, and
/// spawns the SDK-rebuild task. Only the task that owns the latest
/// request id may lower `isSwitchingNetwork` again — overlapping
/// switches' earlier tasks no-op on completion.
private func beginNetworkSwitch() {
networkSwitchRequestID &+= 1
let requestID = networkSwitchRequestID
isSwitchingNetwork = true
Task {
await switchNetwork(to: currentNetwork)
if requestID == networkSwitchRequestID {
isSwitchingNetwork = false
}
}
}
Comment on lines +53 to 63
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Guard the SDK rebuild itself, not just isSwitchingNetwork.

The request ID only prevents an old task from clearing the spinner early. A stale Task can still finish later and overwrite sdk, dataManager.currentNetwork, and isLoading after a newer switch has already started, which can leave the app bound to the wrong network while the UI looks “connected”. Thread the token into switchNetwork and bail before each shared-state write / after each await.

🔧 Suggested shape of the fix
         Task {
-            await switchNetwork(to: currentNetwork)
+            await switchNetwork(to: currentNetwork, requestID: requestID)
             if requestID == networkSwitchRequestID {
                 isSwitchingNetwork = false
             }
         }

-    func switchNetwork(to network: Network) async {
+    func switchNetwork(to network: Network, requestID: UInt64) async {
         guard let modelContext = modelContext else { return }
+        guard requestID == networkSwitchRequestID else { return }

         dataManager?.currentNetwork = network
         do {
             isLoading = true
             let newSDK = try SDK(network: network)
+            guard requestID == networkSwitchRequestID else { return }
             sdk = newSDK
             await loadKnownContractsIntoSDK(sdk: newSDK, modelContext: modelContext)
+            guard requestID == networkSwitchRequestID else { return }
             isLoading = false
         } catch {
+            guard requestID == networkSwitchRequestID else { return }
             sdk = nil
             ...
         }
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/AppState.swift` around
lines 53 - 63, The current requestID only prevents clearing the spinner early
but doesn’t stop a stale Task from updating shared state; modify
beginNetworkSwitch to pass the current networkSwitchRequestID (token) into
switchNetwork (e.g., call switchNetwork(to:currentNetwork, token: requestID))
and then update switchNetwork to early-return whenever the passed token no
longer equals the latest networkSwitchRequestID before writing any shared
properties (sdk, dataManager.currentNetwork, isLoading, isSwitchingNetwork) and
after each await point; ensure every mutation of shared state in switchNetwork
is guarded by comparing the token to networkSwitchRequestID so stale tasks
cannot overwrite newer state.


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ struct ContentView: View {

// Tab 3: Identities
IdentitiesTabView()
.accessibilityIdentifier("rootTab.identities")
.tabItem {
Label("Identities", systemImage: "person.crop.circle")
}
Expand All @@ -107,6 +108,7 @@ struct ContentView: View {

// Tab 5: Settings (includes Platform section)
SettingsView()
.accessibilityIdentifier("rootTab.settings")
.tabItem {
Label("Settings", systemImage: "gearshape")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ struct CreateWalletView: View {

Section {
Toggle("Import Existing Wallet", isOn: $showImportOption)
.accessibilityIdentifier("createWallet.importToggle")
} header: {
Text("Options")
}
Expand Down Expand Up @@ -164,6 +165,7 @@ struct CreateWalletView: View {
.autocorrectionDisabled()
.lineLimit(3...6)
.focused($focusedField, equals: .mnemonic)
.accessibilityIdentifier("createWallet.mnemonicField")
} header: {
Text("Recovery Phrase")
} footer: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,11 @@ struct IdentitiesContentView: View {
} label: {
Label("Re-scan for Identities", systemImage: "magnifyingglass")
}
.accessibilityIdentifier("identities.searchWalletsMenuItem")
} label: {
Image(systemName: "plus")
}
.accessibilityIdentifier("identities.addMenu")
}
}
.sheet(isPresented: $showingLoadIdentity) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ struct IdentityRow: View {
}
.padding(.vertical, 4)
}
.accessibilityIdentifier("identities.row.\(identity.identityIdBase58)")
}

private func refreshBalance() async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,12 @@ struct IdentityDetailView: View {
Text(identity.formattedBalance)
.foregroundColor(.blue)
.fontWeight(.medium)
.accessibilityIdentifier("identityDetail.balanceLabel")
// Display string is "%.8f DASH" — rounding hides
// sub-1000-credit deltas. Expose the raw credit
// count via accessibilityValue for tests that
// need exact numbers.
.accessibilityValue("\(UInt64(bitPattern: identity.balance))")
Comment on lines 139 to +147
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 Suggestion: UInt64(bitPattern: identity.balance) can mask negative-balance regressions through the new >0 assertion

PersistentIdentity.balance is Int64 while the SDK's credit balance is u64. accessibilityValue("\(UInt64(bitPattern: identity.balance))") reinterprets the bit pattern, so a negative Int64 (which itself would indicate an upstream FFI/persistence bug) silently surfaces as a huge positive UInt64. readIdentityBalanceCredits in WalletFlow.swift:700 then strips non-digits and parses, and CreditTransferTest.swift:138-142 only asserts XCTAssertGreaterThan(credits, 0, …). A negative-Int64 regression would (a) reinterpret to ~Int64.max * 2, (b) survive the digit filter, and (c) pass the floor — defeating the new regression-detection guard described in the in-file comment. Options: emit String(identity.balance) and reject a leading - in the parser; add an upper-bound sanity check (legitimate testnet balances are nowhere near Int64.max); or assert non-negativity at the FFI boundary so views never see a negative Int64.

source: ['claude', 'codex']

🤖 Fix this 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/SwiftExampleApp/SwiftExampleApp/Views/IdentityDetailView.swift`:
- [SUGGESTION] lines 139-147: `UInt64(bitPattern: identity.balance)` can mask negative-balance regressions through the new `>0` assertion
  `PersistentIdentity.balance` is `Int64` while the SDK's credit balance is `u64`. `accessibilityValue("\(UInt64(bitPattern: identity.balance))")` reinterprets the bit pattern, so a negative `Int64` (which itself would indicate an upstream FFI/persistence bug) silently surfaces as a huge positive `UInt64`. `readIdentityBalanceCredits` in `WalletFlow.swift:700` then strips non-digits and parses, and `CreditTransferTest.swift:138-142` only asserts `XCTAssertGreaterThan(credits, 0, …)`. A negative-`Int64` regression would (a) reinterpret to ~`Int64.max * 2`, (b) survive the digit filter, and (c) pass the floor — defeating the new regression-detection guard described in the in-file comment. Options: emit `String(identity.balance)` and reject a leading `-` in the parser; add an upper-bound sanity check (legitimate testnet balances are nowhere near `Int64.max`); or assert non-negativity at the FFI boundary so views never see a negative `Int64`.

Comment on lines +142 to +147
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 Suggestion: UInt64(bitPattern: identity.balance) reinterprets negative Int64 and bypasses the new >0 floor

PersistentIdentity.balance is stored as Int64 while the SDK's underlying credit balance is u64. Exposing it via .accessibilityValue("\(UInt64(bitPattern: identity.balance))") reinterprets the bit pattern, so a negative Int64 (which itself would already indicate an upstream FFI/persistence bug) silently surfaces as a huge positive UInt64. readIdentityBalanceCredits (WalletFlow.swift:696-708) then strips non-digit characters and parses, and CreditTransferTest.swift:137-142 only asserts XCTAssertGreaterThan(credits, 0, …). A regression that produces a negative Int64 would (a) reinterpret to ~UInt64.max, (b) survive the digit filter, and (c) pass the >0 floor — defeating the regression-detection guard the in-file comment promises. Options: emit String(identity.balance) and reject a leading - in the parser; add an upper-bound sanity check (legitimate testnet balances are nowhere near Int64.max); or assert non-negativity at the FFI boundary so views never see a negative Int64.

source: ['claude', 'codex']

🤖 Fix this 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/SwiftExampleApp/SwiftExampleApp/Views/IdentityDetailView.swift`:
- [SUGGESTION] lines 142-147: `UInt64(bitPattern: identity.balance)` reinterprets negative Int64 and bypasses the new `>0` floor
  `PersistentIdentity.balance` is stored as `Int64` while the SDK's underlying credit balance is `u64`. Exposing it via `.accessibilityValue("\(UInt64(bitPattern: identity.balance))")` reinterprets the bit pattern, so a negative `Int64` (which itself would already indicate an upstream FFI/persistence bug) silently surfaces as a huge positive `UInt64`. `readIdentityBalanceCredits` (`WalletFlow.swift:696-708`) then strips non-digit characters and parses, and `CreditTransferTest.swift:137-142` only asserts `XCTAssertGreaterThan(credits, 0, …)`. A regression that produces a negative `Int64` would (a) reinterpret to ~`UInt64.max`, (b) survive the digit filter, and (c) pass the `>0` floor — defeating the regression-detection guard the in-file comment promises. Options: emit `String(identity.balance)` and reject a leading `-` in the parser; add an upper-bound sanity check (legitimate testnet balances are nowhere near `Int64.max`); or assert non-negativity at the FFI boundary so views never see a negative `Int64`.

}

// Top-up entry point. Hidden for purely-local rows
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ struct OptionsView: View {
@State private var showingDataManagement = false
@State private var showingAbout = false
@State private var showingContracts = false
@State private var isSwitchingNetwork = false
@State private var sdkStatus: SDKStatus?
@State private var isLoadingStatus = false

Expand Down Expand Up @@ -50,27 +49,22 @@ struct OptionsView: View {
get: { appState.currentNetwork },
set: { newNetwork in
if newNetwork != appState.currentNetwork {
isSwitchingNetwork = true
Task {
// Auto-disable Docker when leaving Local
if newNetwork != .regtest && appState.useDockerSetup {
appState.useDockerSetup = false
}

// Update platform state (which will trigger SDK switch)
appState.currentNetwork = newNetwork

// Reset per-network services. TODO(platform-wallet):
// Once PlatformWalletManager supports network
// switching cleanly, call into it here.
try? walletManager.stopSpv()
platformBalanceSyncService.reset()
shieldedService.reset()

await MainActor.run {
isSwitchingNetwork = false
}
// Auto-disable Docker when leaving Local
if newNetwork != .regtest && appState.useDockerSetup {
appState.useDockerSetup = false
}

// `currentNetwork.didSet` (in AppState) flips
// `isSwitchingNetwork` for us and awaits the
// SDK rebind, so the status label below stays
// in the switching state across the entire
// async cycle. Reset per-network services
// alongside the switch — these don't gate
// readiness, they just clean up stale UI.
appState.currentNetwork = newNetwork
try? walletManager.stopSpv()
platformBalanceSyncService.reset()
shieldedService.reset()
}
}
)) {
Expand All @@ -79,17 +73,14 @@ struct OptionsView: View {
}
}
.pickerStyle(SegmentedPickerStyle())
.disabled(isSwitchingNetwork)
.disabled(appState.isSwitchingNetwork)
.accessibilityIdentifier("options.networkPicker")

if appState.currentNetwork == .regtest {
// `useDockerSetup.didSet` (in AppState) drives the
// SDK rebuild and `isSwitchingNetwork`; no view-side
// onChange is needed.
Toggle("Use Docker Setup", isOn: $appState.useDockerSetup)
.onChange(of: appState.useDockerSetup) { _, _ in
isSwitchingNetwork = true
Task {
await appState.switchNetwork(to: appState.currentNetwork)
await MainActor.run { isSwitchingNetwork = false }
}
}
.help("Connect to local dashmate Docker network.")

if appState.useDockerSetup {
Expand Down Expand Up @@ -129,23 +120,29 @@ struct OptionsView: View {
HStack {
Text("Network Status")
Spacer()
if isSwitchingNetwork {
HStack(spacing: 4) {
ProgressView()
.scaleEffect(0.8)
Text("Switching...")
Group {
if appState.isSwitchingNetwork {
HStack(spacing: 4) {
ProgressView()
.scaleEffect(0.8)
Text("Switching...")
.font(.caption)
.foregroundColor(.secondary)
}
} else if appState.sdk != nil {
Label("Connected", systemImage: "checkmark.circle.fill")
.font(.caption)
.foregroundColor(.green)
} else {
Label("Disconnected", systemImage: "xmark.circle.fill")
.font(.caption)
.foregroundColor(.secondary)
.foregroundColor(.red)
}
} else if appState.sdk != nil {
Label("Connected", systemImage: "checkmark.circle.fill")
.font(.caption)
.foregroundColor(.green)
} else {
Label("Disconnected", systemImage: "xmark.circle.fill")
.font(.caption)
.foregroundColor(.red)
}
// Tests wait on this label transitioning to "Connected"
// after a network switch (signal-based, not sleep-based).
.accessibilityElement(children: .combine)
.accessibilityIdentifier("options.networkStatusLabel")
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ struct SearchWalletsForIdentitiesView: View {
}
.pickerStyle(.menu)
.disabled(isSearching || hdWallets.count < 1)
.accessibilityIdentifier("searchWallets.walletPicker")
}
}
}
Expand All @@ -169,6 +170,7 @@ struct SearchWalletsForIdentitiesView: View {
Text("+\(finding.foundCount)")
.fontWeight(.semibold)
.foregroundColor(finding.foundCount > 0 ? .green : .secondary)
.accessibilityIdentifier("searchWallets.foundCountLabel")
}
if let err = finding.error {
// No `.lineLimit` — identity-derivation errors can
Expand Down Expand Up @@ -298,6 +300,7 @@ struct SearchWalletsForIdentitiesView: View {
|| selectedWalletId == nil
|| selectedManagedWallet == nil
)
.accessibilityIdentifier("searchWallets.searchButton")
if selectedWalletId != nil && selectedManagedWallet == nil {
Text("This wallet isn't loaded in the wallet manager yet. "
+ "Restore it from the Wallets tab and try again.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,15 @@ struct TransitionDetailView: View {
ForEach(identities, id: \.identityIdBase58) { identity in
Text(identity.displayName)
.tag(identity.identityIdBase58)
.accessibilityIdentifier("transition.senderIdentityOption.\(identity.identityIdBase58)")
}
}
.pickerStyle(MenuPickerStyle())
.padding()
.frame(maxWidth: .infinity, alignment: .leading)
.background(Color.gray.opacity(0.1))
.cornerRadius(8)
.accessibilityIdentifier("transition.senderIdentityPicker")
}
}
}
Expand All @@ -188,6 +190,7 @@ struct TransitionDetailView: View {
.foregroundColor(.white)
.cornerRadius(10)
.disabled(!enabled)
.accessibilityIdentifier("transition.executeButton")
}

private var resultView: some View {
Expand All @@ -197,6 +200,7 @@ struct TransitionDetailView: View {
.foregroundColor(isError ? .red : .green)
Text(isError ? "Error" : "Success")
.font(.headline)
.accessibilityIdentifier("transition.resultStatusLabel")
Spacer()
Button("Copy") {
UIPasteboard.general.string = resultText
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ struct TransitionInputView: View {
}
}
.padding(.vertical, 4)
.accessibilityIdentifier("transition.input.\(input.name)")
}

@ViewBuilder
Expand Down Expand Up @@ -539,6 +540,7 @@ struct TransitionInputView: View {
.foregroundColor(.white)
.cornerRadius(8)
}
.accessibilityIdentifier("transition.input.\(input.name).manualEntryButton")
}
} else {
Picker("Select Identity", selection: $value) {
Expand All @@ -560,11 +562,13 @@ struct TransitionInputView: View {
useManualEntry = true
}
}
.accessibilityIdentifier("transition.input.\(input.name).recipientPicker")
}
} else {
VStack(alignment: .leading, spacing: 8) {
TextField("Enter recipient identity ID", text: $value)
.textFieldStyle(RoundedBorderTextFieldStyle())
.accessibilityIdentifier("transition.input.\(input.name).manualEntryField")

if !identities.isEmpty {
Button(action: {
Expand Down
Loading
Loading