Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
34 changes: 32 additions & 2 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,25 +175,41 @@ 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.

- name: Upload XCUITest result bundle
if: always()
# `failure()` only — xcresult bundles include the XCUITest activity
# log (which records `typeText` arguments) and may include failure
# screenshots. `importWallet` types the testnet mnemonic into a
# plain TextField, so a successful run's artifact would archive
# the mnemonic in the activity log. Restricting to failures
# narrows the leak surface (you only get the artifact when there's
# something to debug). 7-day retention also reduces exposure.
if: failure()
uses: actions/upload-artifact@v4
with:
name: SwiftExampleAppUITests-xcresult
path: ${{ runner.temp }}/SwiftExampleAppUITests.xcresult
if-no-files-found: ignore
retention-days: 14
retention-days: 7
Comment on lines 198 to +212
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: Failure-only xcresult upload still archives the testnet mnemonic on the most likely failure paths

The if: failure() guard plus 7-day retention narrows but does not eliminate the leak surface. importWallet types the mnemonic into a plain TextField via mnemonicField.typeText(mnemonic) (WalletFlow.swift:285-292), and the XCUITest activity log records typeText arguments. Any failure that occurs after the import step — discovery timeout, balance assertion failure, the navigation-title bug above — produces an xcresult containing the mnemonic verbatim, downloadable by anyone with read access to the workflow run for 7 days. Those post-import failures are exactly the modes most likely to occur in practice. The inline comment correctly flags the tradeoff; consider stronger mitigations as follow-up: restructure the test so the secret is not entered through XCUITest at all (e.g. inject via a launch-environment hook the app reads directly), or post-process the xcresult to strip activity-log entries before upload.

source: ['claude-general', 'codex-general']

🤖 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 `.github/workflows/swift-example-app-ui-smoke.yml`:
- [SUGGESTION] lines 198-212: Failure-only xcresult upload still archives the testnet mnemonic on the most likely failure paths
  The `if: failure()` guard plus 7-day retention narrows but does not eliminate the leak surface. `importWallet` types the mnemonic into a plain `TextField` via `mnemonicField.typeText(mnemonic)` (WalletFlow.swift:285-292), and the XCUITest activity log records `typeText` arguments. Any failure that occurs after the import step — discovery timeout, balance assertion failure, the navigation-title bug above — produces an xcresult containing the mnemonic verbatim, downloadable by anyone with read access to the workflow run for 7 days. Those post-import failures are exactly the modes most likely to occur in practice. The inline comment correctly flags the tradeoff; consider stronger mitigations as follow-up: restructure the test so the secret is not entered through XCUITest at all (e.g. inject via a launch-environment hook the app reads directly), or post-process the xcresult to strip activity-log entries before upload.


- name: Delete disposable simulator
if: always() && steps.simulator.outputs.udid != ''
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))")
}

// 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
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ final class KeyManagerTests: XCTestCase {
])

// Encode to WIF
guard let wif = KeyFormatter.toWIF(originalKey, isTestnet: true) else {
guard let wif = KeyFormatter.toWIF(originalKey, network: .testnet) else {
XCTFail("Failed to encode to WIF")
return
}
Expand Down
Loading
Loading