ci: Use self hosted mac runner #2776
Conversation
WalkthroughReplaced the Swift SDK CI job runner with a self-hosted macOS ARM64 runner and adjusted WalletService sync-progress handling to capture Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant SPV as SPVClientDelegate
participant WS as WalletService
participant UI as Main/UI Task
SPV->>WS: didUpdateSyncProgress(stage, current, target)
note right of WS #f0f5ff: capture stageRawValue = stage.rawValue
WS->>WS: mappedStage = WalletService.mapSyncStage(stage)
WS->>UI: update UI with computed progress (uses mappedStage)
UI-->>WS: UI updated
note left of SPV #fff7e6: SPV_SWIFT_LOG uses stageRawValue for logging
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ 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❌ Failed checks (1 warning)
✅ Passed checks (2 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 (3)
.github/workflows/swift-sdk-build.yml (3)
3-15: Do not run untrusted fork PRs on self-hosted runners.Running
pull_requestjobs on self-hosted runners is a high-risk supply-chain hole. Gate the job for forks (or require a maintainer label) and set minimal permissions.Apply at the job level:
jobs: swift-sdk-build: name: Swift SDK and Example build (warnings as errors) + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false - runs-on: [self-hosted, macOS, ARM64] + runs-on: [self-hosted, macOS, ARM64] + permissions: + contents: read + actions: read + issues: write + pull-requests: write + concurrency: + group: swift-sdk-build-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: trueAlternative (maintainer-gated): replace the
if:with:if: github.event_name != 'pull_request' || contains(join(fromJSON(toJSON(github.event.pull_request.labels)).*.name, ','), 'safe-to-run')Also applies to: 16-22
52-55: Make cbindgen installation deterministic on self-hosted.Relying on Homebrew may fail if brew isn’t installed or PATH differs; fall back to cargo install when missing.
- - name: Install cbindgen (for header generation) - run: | - brew install cbindgen || true + - name: Ensure cbindgen (for header generation) + run: | + set -euo pipefail + if ! command -v cbindgen >/dev/null 2>&1; then + if command -v brew >/dev/null 2>&1; then + brew install cbindgen + else + cargo install cbindgen --locked + fi + fi
170-236: Guard the PR comment step and avoid failures on forks.On forks,
GITHUB_TOKENis read-only; creating/updating comments will fail. Gate this step to same-repo PRs (if you don’t gate the whole job).- - name: Comment/update PR with artifact link and usage guide (skip if unchanged) - if: github.event_name == 'pull_request' + - name: Comment/update PR with artifact link and usage guide (skip if unchanged) + if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false
🧹 Nitpick comments (3)
.github/workflows/swift-sdk-build.yml (3)
26-31: Pin Xcode to an exact minor to reduce churn.Avoid surprise toolchain shifts; pin to a known-good version (update as needed).
- xcode-version: '16.*' + xcode-version: '16.2' # or your validated minor
237-264: Optional: cache Swift build outputs to speed CI.Caching
.buildcan materially reduce Swift build times on repeated runs.+ - name: Cache SwiftPM build artifacts + uses: actions/cache@v4 + with: + path: | + packages/swift-sdk/.build + key: swiftpm-${{ runner.os }}-${{ hashFiles('packages/swift-sdk/Package.resolved') }} + restore-keys: | + swiftpm-${{ runner.os }}-
36-39: Minor: verify rustup/toolchain availability on the runner.Self-hosted boxes drift; add a quick
rustc --version/cargo --versionecho after installs to fail fast if PATH is off.Also applies to: 140-149
📜 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 (1)
📓 Common learnings
Learnt from: lklimek
PR: dashpay/platform#2318
File: .github/workflows/tests-build-image.yml:45-45
Timestamp: 2024-11-13T10:31:30.891Z
Learning: In the dashpay/platform repository, changes to `.github/workflows/tests-build-image.yml` that switch the Docker image platform from `linux/arm64` to `linux/amd64` for testing purposes are acceptable when required to run on GitHub-hosted runners. ARM64 testing is covered on the testnet.
⏰ 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). (1)
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (1)
.github/workflows/swift-sdk-build.yml (1)
19-19: Switch to self-hosted macOS ARM64 runner looks good.Matches the PR goal and should unblock Swift SDK builds. Please verify the self-hosted runner actually has labels exactly: self-hosted, macOS, ARM64, and Xcode 16.x preinstalled for setup-xcode to select.
|
✅ 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: "f29f55265bad260331f8b6dcd8c755993a5de147987f805e470dc22f311ad318"
)Xcode manual integration:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (3)
500-517: De-duplicate headerPct computation and avoid double-writes to headerProgress.
headerPctis computed in both tasks, andheaderProgressis assigned in both places. Compute once and reuse; updateheaderProgressonly on the MainActor task.Apply:
@@ - 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)) + 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)) + Task { @MainActor in @@ - Task.detached(priority: .utility) { + Task.detached(priority: .utility) { @@ - 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)) + // headerPct computed once above; reuse it here @@ - await MainActor.run { - WalletService.shared.headerProgress = headerPct + await MainActor.run { WalletService.shared.transactionProgress = txPctFinal WalletService.shared.masternodeProgress = mnPctFinal }Also applies to: 530-534, 551-555
519-521: Use os.Logger for structured, filterable logs instead of print.Keeps logs consistent and cheap when disabled. Optional, but recommended for prod builds.
511-517: Make detailedSyncProgress strongly typed.The property is
Any?, yet you always assignSyncProgress. Tighten the type to improve safety and call-site ergonomics.- @Published public var detailedSyncProgress: Any? // Use SPVClient.SyncProgress + @Published public var detailedSyncProgress: SyncProgress?
📜 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(2 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: lklimek
PR: dashpay/platform#2318
File: .github/workflows/tests-build-image.yml:45-45
Timestamp: 2024-11-13T10:31:30.891Z
Learning: In the dashpay/platform repository, changes to `.github/workflows/tests-build-image.yml` that switch the Docker image platform from `linux/arm64` to `linux/amd64` for testing purposes are acceptable when required to run on GitHub-hosted runners. ARM64 testing is covered on the testnet.
⏰ 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)
498-499: Good capture to stabilize logging across async boundaries.Taking
stage.rawValueinto a local before launching async work prevents mismatches in printed stage. LGTM.
600-613: Confirm stage mapping: .masternodes → .filters.Mapping
.masternodesto.filtersmay be intentional for legacy UI semantics. Please confirm this is desired; otherwise it can confuse progress display.
… the MainActor block
QuantumExplorer
changed the title
Issue being fixed or feature implemented
Swift SDK build workflow fails on GitHub-hosted runners
What was done?
Breaking Changes
None
Checklist:
Summary by CodeRabbit