ci: swift CI fixes #2775
ci: swift CI fixes #2775
Conversation
WalkthroughSPV delegate protocol annotated with @mainactor; SPV sync stages gain Sendable conformance and new cases. WalletService SPV delegate methods changed visibility/isolation and map SPV stages to a Sendable UI-facing SyncStage; didReceiveTransaction now reloads transactions and refreshes balance. CI build step switched to xcodebuild with warnings-as-errors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor SPV as SPVClient
participant Delegate as SPVClientDelegate (@MainActor)
participant WS as WalletService
participant UI as UI State
rect rgba(200,230,255,0.18)
note right of Delegate: Delegate callbacks on MainActor
SPV->>Delegate: didUpdateSyncProgress(progress)
Delegate->>WS: spvClient(_:didUpdateSyncProgress:)
WS-->>UI: update syncProgress / headerProgress / detailedSyncProgress
end
rect rgba(220,255,220,0.16)
SPV->>Delegate: didReceiveTransaction(event)
Delegate->>WS: spvClient(_:didReceiveTransaction:)
WS->>WS: process transaction
WS-->>UI: reload transactions
WS-->>UI: refresh balance
end
SPV->>Delegate: didReceiveBlock(block)
Delegate->>WS: spvClient(_:didReceiveBlock:)
WS-->>UI: update block/state
alt sync finished
SPV->>Delegate: didCompleteSync(success,error)
Delegate->>WS: spvClient(_:didCompleteSync:)
WS-->>UI: set isSyncing / lastSyncError
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Pre-merge checks✅ Passed checks (3 passed)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
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/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (1)
599-599: Inconsistent isolation modifier on helper methodThe
mapSyncStagehelper method still hasnonisolatedmodifier (line 599), but it's being called from within the@MainActor-isolated extension. This inconsistency should be fixed.Apply this diff to fix the inconsistency:
- nonisolated private static func mapSyncStage(_ stage: SPVSyncStage) -> SyncStage { + private static func mapSyncStage(_ stage: SPVSyncStage) -> SyncStage {
🧹 Nitpick comments (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (1)
523-555: Complex async task in delegate callback could delay responseThe
didUpdateSyncProgressmethod spawns a detached task that performs multiple async operations including fetching sync snapshots and stats. This could potentially delay the delegate callback response if the SPVClient expects quick returns.Consider moving the heavy computation to a separate method that can be debounced or throttled:
+ private var syncProgressUpdateTask: Task<Void, Never>? + public func spvClient(_ client: SPVClient, didUpdateSyncProgress progress: SPVSyncProgress) { // ... existing immediate updates ... - Task.detached(priority: .utility) { + // Cancel previous task if still running + syncProgressUpdateTask?.cancel() + syncProgressUpdateTask = Task.detached(priority: .utility) { // ... existing async work ... } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
packages/swift-sdk/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Make DPP types public in Swift where needed for visibility
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
packages/swift-sdk/SwiftExampleApp/**/*.swift
📄 CodeRabbit inference engine (packages/swift-sdk/SwiftExampleApp/CLAUDE.md)
packages/swift-sdk/SwiftExampleApp/**/*.swift: Use Core SDK functions with the dash_core_sdk_* prefix
Use Platform SDK functions with the dash_sdk_* prefix
Use Unified SDK functions with the dash_unified_sdk_* prefix
Prefer using PersistentToken predicate helpers (e.g., mintableTokensPredicate, tokensWithControlRulePredicate) instead of manual filtering
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
packages/swift-sdk/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep the Swift app and SDK code in packages/swift-sdk
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
🧠 Learnings (1)
📓 Common learnings
Learnt from: QuantumExplorer
PR: dashpay/platform#2257
File: packages/rs-drive-abci/src/mimic/test_quorum.rs:159-164
Timestamp: 2024-11-20T16:16:01.830Z
Learning: QuantumExplorer prefers not to receive auto-generated messages asking to post on social media.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Swift SDK and Example build (warnings as errors)
🔇 Additional comments (4)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (4)
489-490: Good actor isolation for UI safetyThe addition of
@MainActorto the SPVClientDelegate extension ensures all delegate callbacks execute on the main actor, which is appropriate since these methods update@Publishedproperties that drive UI updates. This prevents potential race conditions and ensures thread-safe UI updates.
564-574: Good addition of UI updates on transaction receiptThe
didReceiveTransactionmethod now properly refreshes the UI by loading transactions and updating the balance. The use ofTask { @MainActor in ... }ensures these UI updates happen on the main actor.
617-632: Public types properly exposed for SDK visibilityThe addition of public
SyncProgressstruct andSyncStageenum follows the coding guideline to "Make DPP types public in Swift where needed for visibility". These types are appropriately marked aspublicto support the SPV sync progress representation.
491-556: Verify SPVClient delegate isolation after removingnonisolatedConfirm SPVClient either calls its delegate on a background thread and expects nonisolated, or explicitly hops to MainActor before invoking — removal of
nonisolatedmust not change runtime threading expectations.Run to locate the client and call sites and inspect invocation threading:
- rg -n --hidden -S '\b(class|struct|protocol)\s+SPVClient\b|SPVClient.swift'
- rg -n --hidden -S 'didUpdateSyncProgress|spvClient('
- rg -n --hidden -S 'DispatchQueue.main.async|MainActor.run|@mainactor|Task\s*('
If SPVClient invokes delegates from background threads, either restore
nonisolatedon delegate methods or ensure callers hop to the MainActor before calling.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (2)
600-614: Fix wrong stage mapping; align isolation with the extensionmasternodes is mapped to filters; that’s misleading in the UI. Also, drop nonisolated for consistency with the @mainactor extension.
- nonisolated private static func mapSyncStage(_ stage: SPVSyncStage) -> SyncStage { + private static func mapSyncStage(_ stage: SPVSyncStage) -> SyncStage { switch stage { case .idle: return .idle case .headers: return .headers case .masternodes: - return .filters + return .masternodes case .transactions: return .downloading case .complete: return .complete } }
171-176: Do not log secrets (PIN) to consoleLogging the PIN is a security risk and may leak sensitive data via logs.
- print("PIN: \(pin)") + // Do not log secrets like PINs
🧹 Nitpick comments (6)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (6)
491-523: Remove redundant Task { @mainactor } wrapperThis method is already @mainactor; the extra Task adds scheduling overhead with no benefit.
- Task { @MainActor in - let base = Double(startHeight) - let numer = max(0.0, Double(currentHeight) - base) - let denom = max(1.0, Double(targetHeight) - base) - let headerPct = min(1.0, max(0.0, numer / denom)) - - WalletService.shared.syncProgress = headerPct - WalletService.shared.headerProgress = headerPct - - WalletService.shared.detailedSyncProgress = SyncProgress( - current: UInt64(currentHeight), - total: UInt64(targetHeight), - rate: rate, - progress: headerPct, - stage: mappedStage - ) - - if ProcessInfo.processInfo.environment["SPV_SWIFT_LOG"] == "1" { - print("📊 Sync progress: \(stageRawValue) - \(Int(overall * 100))%") - } - } + let base = Double(startHeight) + let numer = max(0.0, Double(currentHeight) - base) + let denom = max(1.0, Double(targetHeight) - base) + let headerPct = min(1.0, max(0.0, numer / denom)) + + WalletService.shared.syncProgress = headerPct + WalletService.shared.headerProgress = headerPct + + WalletService.shared.detailedSyncProgress = SyncProgress( + current: UInt64(currentHeight), + total: UInt64(targetHeight), + rate: rate, + progress: headerPct, + stage: mappedStage + ) + + if ProcessInfo.processInfo.environment["SPV_SWIFT_LOG"] == "1" { + print("📊 Sync progress: \(stageRawValue) - \(Int(overall * 100))%") + }Side note: consider giving detailedSyncProgress a concrete type instead of Any? to avoid type erasure in the UI.
524-556: Avoid duplicate FFI calls; reuse snapshotgetSyncSnapshot() is called twice. Query once and reuse; fall back to getStats() only if needed.
- let txPctFinal: Double - if let snap = await client?.getSyncSnapshot(), snap.headerHeight > 0 { - txPctFinal = min(1.0, max(0.0, Double(snap.lastSyncedFilterHeight) / Double(snap.headerHeight))) - } else if let stats = await client?.getStats(), stats.headerHeight > 0 { - txPctFinal = min(1.0, max(0.0, Double(stats.filterHeight) / Double(stats.headerHeight))) - } else { - txPctFinal = prevTx - } - - let mnPctFinal: Double - if let snap = await client?.getSyncSnapshot() { - mnPctFinal = snap.masternodesSynced ? 1.0 : 0.0 - } else { - mnPctFinal = prevMn - } + // Query snapshot once to avoid redundant FFI calls + let snapshot = await client?.getSyncSnapshot() + let txPctFinal: Double + if let snap = snapshot, snap.headerHeight > 0 { + txPctFinal = min(1.0, max(0.0, Double(snap.lastSyncedFilterHeight) / Double(snap.headerHeight))) + } else if let stats = await client?.getStats(), stats.headerHeight > 0 { + txPctFinal = min(1.0, max(0.0, Double(stats.filterHeight) / Double(stats.headerHeight))) + } else { + txPctFinal = prevTx + } + + let mnPctFinal: Double = (snapshot?.masternodesSynced == true) ? 1.0 : prevMn
565-575: Inline the main-actor work; consider coalescing UI updatesNo need for a Task on the main actor here; also consider debouncing if bursts of tx events are expected.
- // Update transactions and balance - Task { @MainActor in - await loadTransactions() - updateBalance() - } + // Update transactions and balance + await loadTransactions() + updateBalance()
577-593: Finalize progress on success; clear on failureReflect completion in progress fields; clear on failure to avoid stale UI.
if success { if ProcessInfo.processInfo.environment["SPV_SWIFT_LOG"] == "1" { print("✅ Sync completed successfully") } + syncProgress = 1.0 + headerProgress = 1.0 } else { if ProcessInfo.processInfo.environment["SPV_SWIFT_LOG"] == "1" { print("❌ Sync failed: \(error ?? "Unknown error")") } lastSyncError = SPVError.syncFailed(error ?? "Unknown error") + syncProgress = nil + detailedSyncProgress = nil }
594-599: Expose connection state to UI (optional)Consider @published isConnected and peerCount to reflect connectivity in views.
616-634: Avoid public duplicate types; add missing masternodes stageThese types shadow SDK concepts and are public. Unless you intend to expose them, prefer internal and include .masternodes.
-public struct SyncProgress { +struct SyncProgress { @@ -} - -public enum SyncStage { +} + +enum SyncStage { case idle case connecting case headers case filters + case masternodes case downloading case complete }If the SDK already exposes a SyncProgress, consider using SwiftDashSDK’s type directly (or a typealias) and remove these.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
packages/swift-sdk/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Make DPP types public in Swift where needed for visibility
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
packages/swift-sdk/SwiftExampleApp/**/*.swift
📄 CodeRabbit inference engine (packages/swift-sdk/SwiftExampleApp/CLAUDE.md)
packages/swift-sdk/SwiftExampleApp/**/*.swift: Use Core SDK functions with the dash_core_sdk_* prefix
Use Platform SDK functions with the dash_sdk_* prefix
Use Unified SDK functions with the dash_unified_sdk_* prefix
Prefer using PersistentToken predicate helpers (e.g., mintableTokensPredicate, tokensWithControlRulePredicate) instead of manual filtering
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
packages/swift-sdk/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep the Swift app and SDK code in packages/swift-sdk
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
🧠 Learnings (1)
📓 Common learnings
Learnt from: QuantumExplorer
PR: dashpay/platform#2257
File: packages/rs-drive-abci/src/mimic/test_quorum.rs:159-164
Timestamp: 2024-11-20T16:16:01.830Z
Learning: QuantumExplorer prefers not to receive auto-generated messages asking to post on social media.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Swift SDK and Example build (warnings as errors)
🔇 Additional comments (2)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (2)
489-490: Good: Delegate isolation on the main actorMarking the SPVClientDelegate extension as @mainactor is the right call for UI-bound updates.
559-564: LGTM: Lightweight block event loggingNon-blocking and guarded by env flag.
|
✅ 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: "416298989ae550be543f3153f62cf8f98ef97977a845640ddf9bea0239d0b499"
)Xcode manual integration:
|
There was a problem hiding this comment.
Actionable comments posted: 0
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/SPV/SPVClient.swift (1)
472-499: Block rate calc can be wildly wrong on reorgs and grows stale over time
- Underflow risk:
height - lastBlockHeight(UInt32) can wrap on reorg/out-of-order blocks.- Staleness:
timeDiffusessyncStartTime, so the denominator only increases; rate trends to 0.Compute rate using time since last block event and clamp diff to non-negative.
Apply this diff in handleBlockEvent:
- if isSyncing, let progress = syncProgress { - // Update height tracking for rate calculation - if lastBlockHeight > 0 { - let blocksDiff = height - lastBlockHeight - let timeDiff = Date().timeIntervalSince(syncStartTime ?? Date()) - let rate = timeDiff > 0 ? Double(blocksDiff) / timeDiff : 0 + if isSyncing, let progress = syncProgress { + // Update height tracking for rate calculation with reorg-safe logic + let now = Date() + if lastBlockHeight > 0 { + let blocksDiff: UInt32 = (height >= lastBlockHeight) ? (height - lastBlockHeight) : 0 + let elapsed = now.timeIntervalSince(lastBlockEventTime ?? syncStartTime ?? now) + let rate = elapsed > 0 ? Double(blocksDiff) / elapsed : 0 let updatedProgress = SPVSyncProgress( stage: progress.stage, headerProgress: progress.headerProgress, masternodeProgress: progress.masternodeProgress, transactionProgress: progress.transactionProgress, currentHeight: height, targetHeight: progress.targetHeight, startHeight: self.startFromHeight, rate: rate, estimatedTimeRemaining: progress.estimatedTimeRemaining ) syncProgress = updatedProgress delegate?.spvClient(self, didUpdateSyncProgress: updatedProgress) } - lastBlockHeight = height + lastBlockHeight = height + lastBlockEventTime = now }Add the backing field near the other sync-tracking vars:
// Add near Line 121 sync-tracking properties private var lastBlockEventTime: Date?
🧹 Nitpick comments (4)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (1)
231-233: Unconditional console log for User-AgentAlways printing the UA can be noisy. Gate behind
swiftLoggingEnabled.- // Always print what we're about to set for easier debugging - print("Setting user agent to \(ua)") + if swiftLoggingEnabled { + print("[SPV][Config] Setting User-Agent to \(ua)") + }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (3)
490-556: Drop redundant Task hops; already on MainActorThis delegate runs on the main actor. The inner
Task { @MainActor in … }adds overhead without benefit. Assign directly.- Task { @MainActor in - let base = Double(startHeight) - let numer = max(0.0, Double(currentHeight) - base) - let denom = max(1.0, Double(targetHeight) - base) - let headerPct = min(1.0, max(0.0, numer / denom)) - - WalletService.shared.syncProgress = headerPct - WalletService.shared.headerProgress = headerPct - - WalletService.shared.detailedSyncProgress = SyncProgress( - current: UInt64(currentHeight), - total: UInt64(targetHeight), - rate: rate, - progress: headerPct, - stage: mappedStage - ) - - if ProcessInfo.processInfo.environment["SPV_SWIFT_LOG"] == "1" { - print("📊 Sync progress: \(stageRawValue) - \(Int(overall * 100))%") - } - } + let base = Double(startHeight) + let numer = max(0.0, Double(currentHeight) - base) + let denom = max(1.0, Double(targetHeight) - base) + let headerPct = min(1.0, max(0.0, numer / denom)) + + self.syncProgress = headerPct + self.headerProgress = headerPct + + self.detailedSyncProgress = SyncProgress( + current: UInt64(currentHeight), + total: UInt64(targetHeight), + rate: rate, + progress: headerPct, + stage: mappedStage + ) + + if ProcessInfo.processInfo.environment["SPV_SWIFT_LOG"] == "1" { + print("📊 Sync progress: \(stageRawValue) - \(Int(overall * 100))%") + }Additionally, prefer a strong type for
detailedSyncProgressto avoidAny?. If acceptable, change its declaration to:@Published public var detailedSyncProgress: SyncProgress?
564-574: Reloading entire tx list on every tx may be heavyConsider appending/incremental update or throttling with a short debounce if bursts are possible.
- Task { @MainActor in - await loadTransactions() - updateBalance() - } + Task { @MainActor in + // TODO: consider incremental append or coalesced reload + await loadTransactions() + updateBalance() + }
593-598: Connection status: consider surfacing peer countOptional: expose a published
peerCountor UI indicator here to reflect connectivity.public func spvClient(_ client: SPVClient, didChangeConnectionStatus connected: Bool, peers: Int) { if ProcessInfo.processInfo.environment["SPV_SWIFT_LOG"] == "1" { print("🌐 Connection status: \(connected ? "Connected" : "Disconnected") - \(peers) peers") } + // Example: + // self.isConnected = connected + // self.latestPeerCount = peers }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift(2 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
packages/swift-sdk/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Make DPP types public in Swift where needed for visibility
Files:
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
packages/swift-sdk/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep the Swift app and SDK code in packages/swift-sdk
Files:
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
packages/swift-sdk/SwiftExampleApp/**/*.swift
📄 CodeRabbit inference engine (packages/swift-sdk/SwiftExampleApp/CLAUDE.md)
packages/swift-sdk/SwiftExampleApp/**/*.swift: Use Core SDK functions with the dash_core_sdk_* prefix
Use Platform SDK functions with the dash_sdk_* prefix
Use Unified SDK functions with the dash_unified_sdk_* prefix
Prefer using PersistentToken predicate helpers (e.g., mintableTokensPredicate, tokensWithControlRulePredicate) instead of manual filtering
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
🧠 Learnings (1)
📓 Common learnings
Learnt from: QuantumExplorer
PR: dashpay/platform#2257
File: packages/rs-drive-abci/src/mimic/test_quorum.rs:159-164
Timestamp: 2024-11-20T16:16:01.830Z
Learning: QuantumExplorer prefers not to receive auto-generated messages asking to post on social media.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Swift SDK and Example build (warnings as errors)
🔇 Additional comments (5)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (2)
59-65: Sendable SPVSyncStage + new cases: LGTMConforms to Sendable and adds clear user-facing states. Ensure any switch statements over SPVSyncStage are exhaustive across the SDK/app (WalletService.mapSyncStage already covers them).
85-92: @mainactor on SPVClientDelegate: LGTMGood move to guarantee UI-safe delivery of callbacks. This matches how callbacks are marshalled to the main actor.
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (3)
558-563: Block callback: LGTMPublic exposure aligns with delegate’s main-actor isolation and current logging.
576-591: Sync completion handling: LGTMMain-actor updates and error propagation look correct.
625-632: Sendable SyncStage: LGTMMatches cross-actor usage. Mapping from SPVSyncStage covers all cases.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/swift-sdk-build.yml (3)
239-251: Add DerivedData isolation and disable code signing in CI.Prevents cross-run cache contamination and avoids signing hiccups for simulator builds.
Apply this diff:
- - name: Build SwiftDashSDK package (warnings as errors) + - name: Build SwiftDashSDK package (warnings as errors) working-directory: packages/swift-sdk env: OTHER_SWIFT_FLAGS: -warnings-as-errors SWIFT_TREAT_WARNINGS_AS_ERRORS: YES run: | - xcodebuild \ + set -euxo pipefail + DERIVED_DATA="$RUNNER_TEMP/DerivedData/SwiftDashSDK" + xcodebuild \ -package-path . \ -scheme SwiftDashSDK \ -destination 'generic/platform=iOS Simulator' \ -configuration Debug \ ONLY_ACTIVE_ARCH=YES \ + CODE_SIGNING_ALLOWED=NO \ + -derivedDataPath "$DERIVED_DATA" \ OTHER_SWIFT_FLAGS="$OTHER_SWIFT_FLAGS" \ SWIFT_TREAT_WARNINGS_AS_ERRORS=$SWIFT_TREAT_WARNINGS_AS_ERRORS \ build
239-251: Enable strict concurrency checking to catch actor/sendable issues early.Given recent @MainActor/sendable changes, treat concurrency diagnostics as hard failures in CI.
Apply this diff:
env: OTHER_SWIFT_FLAGS: -warnings-as-errors SWIFT_TREAT_WARNINGS_AS_ERRORS: YES + SWIFT_STRICT_CONCURRENCY: complete run: | set -euxo pipefail DERIVED_DATA="$RUNNER_TEMP/DerivedData/SwiftDashSDK" xcodebuild \ -package-path . \ -scheme SwiftDashSDK \ -destination 'generic/platform=iOS Simulator' \ -configuration Debug \ ONLY_ACTIVE_ARCH=YES \ CODE_SIGNING_ALLOWED=NO \ -derivedDataPath "$DERIVED_DATA" \ OTHER_SWIFT_FLAGS="$OTHER_SWIFT_FLAGS" \ SWIFT_TREAT_WARNINGS_AS_ERRORS=$SWIFT_TREAT_WARNINGS_AS_ERRORS \ + SWIFT_STRICT_CONCURRENCY=$SWIFT_STRICT_CONCURRENCY \ build
247-249: Consider building both sim arches when validating library code.ONLY_ACTIVE_ARCH=YES builds a single arch (arm64 on this runner). If you want to catch x86_64-sim issues preemptively, run a second build with arch override or remove ONLY_ACTIVE_ARCH for this step.
Example follow-up step (optional):
+ - name: (Optional) Validate x86_64 simulator build + if: always() + working-directory: packages/swift-sdk + run: | + xcodebuild \ + -package-path . \ + -scheme SwiftDashSDK \ + -destination 'generic/platform=iOS Simulator' \ + -configuration Debug \ + ONLY_ACTIVE_ARCH=NO ARCHS="x86_64" EXCLUDED_ARCHS="" \ + CODE_SIGNING_ALLOWED=NO \ + build
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/swift-sdk-build.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: QuantumExplorer
PR: dashpay/platform#2257
File: packages/rs-drive-abci/src/mimic/test_quorum.rs:159-164
Timestamp: 2024-11-20T16:16:01.830Z
Learning: QuantumExplorer prefers not to receive auto-generated messages asking to post on social media.
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*.swift : Use Platform SDK functions with the dash_sdk_* prefix
Applied to files:
.github/workflows/swift-sdk-build.yml
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*.swift : Use Core SDK functions with the dash_core_sdk_* prefix
Applied to files:
.github/workflows/swift-sdk-build.yml
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
PR: dashpay/platform#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Applies to packages/swift-sdk/** : Keep the Swift app and SDK code in packages/swift-sdk
Applied to files:
.github/workflows/swift-sdk-build.yml
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
PR: dashpay/platform#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Applies to packages/rs-sdk-ffi/build_ios.sh : Build the iOS framework via packages/rs-sdk-ffi/build_ios.sh
Applied to files:
.github/workflows/swift-sdk-build.yml
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*.swift : Use Unified SDK functions with the dash_unified_sdk_* prefix
Applied to files:
.github/workflows/swift-sdk-build.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Swift SDK and Example build (warnings as errors)
🔇 Additional comments (2)
.github/workflows/swift-sdk-build.yml (2)
239-251: Good switch to xcodebuild with warnings-as-errors.This enforces stricter CI on the Swift package and aligns with the rest of the workflow.
243-246: Verify scheme/product exists and matches "SwiftDashSDK"No verification output was produced; ensure packages/swift-sdk/Package.swift declares a product named "SwiftDashSDK" and that
xcodebuild -list -package-path packages/swift-sdkshows a scheme with the exact same name — update .github/workflows/swift-sdk-build.yml (lines 243–246) if it differs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/swift-sdk-build.yml (4)
239-254: Drop no-op SWIFT_SUPPRESS_WARNINGS; also treat C/ObjC warnings as errorsSWIFT_SUPPRESS_WARNINGS isn’t a recognized build setting and has no effect. Add GCC_TREAT_WARNINGS_AS_ERRORS to fail on non-Swift warnings too.
Apply:
- env: - OTHER_SWIFT_FLAGS: -warnings-as-errors - SWIFT_TREAT_WARNINGS_AS_ERRORS: YES - SWIFT_SUPPRESS_WARNINGS: NO + env: + OTHER_SWIFT_FLAGS: -warnings-as-errors + SWIFT_TREAT_WARNINGS_AS_ERRORS: YES + GCC_TREAT_WARNINGS_AS_ERRORS: YES @@ - OTHER_SWIFT_FLAGS="$OTHER_SWIFT_FLAGS" \ - SWIFT_TREAT_WARNINGS_AS_ERRORS=$SWIFT_TREAT_WARNINGS_AS_ERRORS \ - SWIFT_SUPPRESS_WARNINGS=$SWIFT_SUPPRESS_WARNINGS \ + OTHER_SWIFT_FLAGS="$OTHER_SWIFT_FLAGS" \ + SWIFT_TREAT_WARNINGS_AS_ERRORS=$SWIFT_TREAT_WARNINGS_AS_ERRORS \ + GCC_TREAT_WARNINGS_AS_ERRORS=$GCC_TREAT_WARNINGS_AS_ERRORS \ build
244-254: Isolate DerivedData to avoid stale cache issues on self-hosted runnersUse a per-run DerivedData path for reproducible builds.
Apply:
xcodebuild \ -project SwiftExampleApp/SwiftExampleApp.xcodeproj \ -scheme SwiftDashSDK \ -destination 'generic/platform=iOS Simulator' \ -configuration Debug \ + -derivedDataPath "$RUNNER_TEMP/DerivedData-SwiftDashSDK" \ ARCHS=arm64 \ ONLY_ACTIVE_ARCH=YES \
267-280: Mirror warning-settings fix for ExampleApp buildRemove SWIFT_SUPPRESS_WARNINGS; add GCC_TREAT_WARNINGS_AS_ERRORS; also isolate DerivedData.
Apply:
env: # Treat Swift warnings as errors during xcodebuild OTHER_SWIFT_FLAGS: -warnings-as-errors SWIFT_TREAT_WARNINGS_AS_ERRORS: YES - SWIFT_SUPPRESS_WARNINGS: NO + GCC_TREAT_WARNINGS_AS_ERRORS: YES @@ xcodebuild \ -project SwiftExampleApp/SwiftExampleApp.xcodeproj \ -scheme SwiftExampleApp \ -sdk iphonesimulator \ -destination 'generic/platform=iOS Simulator' \ -configuration Debug \ + -derivedDataPath "$RUNNER_TEMP/DerivedData-SwiftExampleApp" \ ARCHS=arm64 \ ONLY_ACTIVE_ARCH=YES \ OTHER_SWIFT_FLAGS="$OTHER_SWIFT_FLAGS" \ SWIFT_TREAT_WARNINGS_AS_ERRORS=$SWIFT_TREAT_WARNINGS_AS_ERRORS \ - SWIFT_SUPPRESS_WARNINGS=$SWIFT_SUPPRESS_WARNINGS \ + GCC_TREAT_WARNINGS_AS_ERRORS=$GCC_TREAT_WARNINGS_AS_ERRORS \ build
249-251: Avoid forcing ARCHS unless strictly requiredLet the destination select the correct sim arch. For portability (e.g., Intel builders or future changes), drop explicit ARCHS.
Apply:
- ARCHS=arm64 \ ONLY_ACTIVE_ARCH=YES \Also applies to: 275-276
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/swift-sdk-build.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: QuantumExplorer
PR: dashpay/platform#2257
File: packages/rs-drive-abci/src/mimic/test_quorum.rs:159-164
Timestamp: 2024-11-20T16:16:01.830Z
Learning: QuantumExplorer prefers not to receive auto-generated messages asking to post on social media.
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*.swift : Use Platform SDK functions with the dash_sdk_* prefix
Applied to files:
.github/workflows/swift-sdk-build.yml
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*.swift : Use Core SDK functions with the dash_core_sdk_* prefix
Applied to files:
.github/workflows/swift-sdk-build.yml
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*.swift : Use Unified SDK functions with the dash_unified_sdk_* prefix
Applied to files:
.github/workflows/swift-sdk-build.yml
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
PR: dashpay/platform#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Applies to packages/swift-sdk/** : Keep the Swift app and SDK code in packages/swift-sdk
Applied to files:
.github/workflows/swift-sdk-build.yml
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
PR: dashpay/platform#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Applies to packages/rs-sdk-ffi/build_ios.sh : Build the iOS framework via packages/rs-sdk-ffi/build_ios.sh
Applied to files:
.github/workflows/swift-sdk-build.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Swift SDK and Example build (warnings as errors)
🔇 Additional comments (2)
.github/workflows/swift-sdk-build.yml (2)
150-153: FFI build entrypoint verified — no change requiredpackages/swift-sdk/build_ios.sh cd's into packages/rs-sdk-ffi, runs ./build_ios.sh (fails if missing), checks for DashUnifiedSDK.xcframework or DashSDKFFI.xcframework, and copies the result into the Swift package — the workflow call is correct.
26-30: Confirm Xcode 16 availability on self-hosted macOS runnerVerification failed in this environment (not macOS): "/usr/bin/xcodebuild: No such file or directory", "ls: cannot access '/Applications': No such file or directory", "xcode-select: command not found". Run these on the actual self-hosted macOS runner and confirm Xcode 16.x is installed and selectable:
/usr/bin/xcodebuild -version ls -1 /Applications | rg -n '^Xcode_?16' xcode-select -pIf Xcode 16.x is not installed/selectable, preinstall it on the runner or use a macOS-hosted runner with Xcode 16.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores