refactor: swift sdk fixes #2772
Conversation
WalkthroughSwitches Swift packages to Swift 6, replaces many opaque FFI pointers with typed Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as ExampleApp (MainActor)
participant SDK as Swift SDK (@MainActor)
participant FFI as DashSDKFFI
Note right of SDK #faf3d7: local override support
User->>App: Toggle "Use Local DAPI"
App->>SDK: init(network)
SDK->>SDK: read UserDefaults (useLocalhostPlatform)
alt useLocalhostPlatform == true
SDK->>FFI: dash_sdk_create_trusted(config with localhost addrs)
else
SDK->>FFI: dash_sdk_create_trusted(default)
end
SDK->>FFI: dash_sdk_status()
FFI-->>SDK: status JSON
SDK-->>App: deliver status (MainActor)
sequenceDiagram
autonumber
actor User
participant App
participant SPV as SPVClient
participant FFI as dash-spv-ffi
Note right of SPV #f1f7ff: startHeight & peer config
User->>App: Toggle "Use Local Core"
App->>SPV: initialize(startHeight?, restrictToConfiguredPeers)
SPV->>SPV: read corePeerAddresses (UserDefaults)
loop add peers
SPV->>FFI: dash_spv_ffi_config_add_peer(addr)
end
SPV->>FFI: dash_spv_ffi_config_set_restrict_to_configured_peers(flag)
SPV->>FFI: startSync(checkpoint/startHeight)
FFI-->>SPV: progress(current_height, target_height)
SPV->>SPV: compute absoluteCurrent = startFromHeight + current_height
SPV-->>App: SPVSyncProgress(startHeight, absoluteCurrent, target)
App-->>User: update sync UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (14)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Helpers/WIFParser.swift (2)
56-58: Fix Dash WIF version bytes (mainnet should be 0xCC, not 0x80).Using 0x80 will generate Bitcoin WIFs. Dash mainnet WIF version byte is 0xCC; testnet is 0xEF. Update to avoid producing invalid keys for Dash. (docs.dash.org)
Apply this diff:
- // Version byte: 0xef for testnet, 0x80 for mainnet - let versionByte: UInt8 = isTestnet ? 0xef : 0x80 + // Version byte: 0xEF (testnet) or 0xCC (Dash mainnet) + let versionByte: UInt8 = isTestnet ? 0xEF : 0xCC
16-46: Validate version byte and compression flag in parseWIF.Currently any version byte passes, and the compression flag (when present) isn’t checked. Enforce Dash’s 0xCC/0xEF and validate the optional 0x01 flag to reject malformed inputs. (docs.dash.org)
Apply this diff:
// Decode from Base58 guard let decoded = decodeBase58(wif) else { return nil } + // Enforce Dash version bytes (0xCC mainnet, 0xEF testnet) + guard let version = decoded.first, version == 0xCC || version == 0xEF else { + return nil + } + // WIF structure: - // - 1 byte: version (0xCC for testnet, 0xD2 for mainnet) + // - 1 byte: version (0xCC for Dash mainnet, 0xEF for testnet) // - 32 bytes: private key // - (optional) 1 byte: 0x01 for compressed public key // - 4 bytes: checksum let minLength = 1 + 32 + 4 // version + key + checksum let maxLength = minLength + 1 // + compression flag guard decoded.count >= minLength && decoded.count <= maxLength else { return nil } + // If a compression flag is present, it must be 0x01 + if decoded.count == maxLength && decoded[33] != 0x01 { + return nil + } + // Verify checksumpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/KeychainManager.swift (1)
114-152: Bug: listing all keys omits required return flags; result will be empty/errSecParam.SecItemCopyMatching with kSecMatchLimitAll must request attributes (kSecReturnAttributes = true). Add it to get items to delete.
func deleteAllPrivateKeys(for identityId: Data) -> Bool { var query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: serviceName, - kSecMatchLimit as String: kSecMatchLimitAll + kSecMatchLimit as String: kSecMatchLimitAll, + kSecReturnAttributes as String: true ]Optional: log human-readable errors.
- let searchStatus = SecItemCopyMatching(query as CFDictionary, &result) + let searchStatus = SecItemCopyMatching(query as CFDictionary, &result) + if searchStatus != errSecSuccess { + if let msg = SecCopyErrorMessageString(searchStatus, nil) as String? { + print("🔐 KeychainManager: Search failed: \(msg) (\(searchStatus))") + } + }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/SDK/SDKExtensions.swift (1)
8-14: Computed SDK.network returns constant Testnet — risks misuse.This masks the actual network the SDK was initialized with and can cause subtle bugs. Drop this property or back it with real state.
-extension SDK { - var network: SwiftDashSDK.Network { - // In a real implementation, we would track the network during initialization - // For now, return testnet as default - return DashSDKNetwork(rawValue: 1) // Testnet - } -} +// Avoid exposing a misleading constant `network`. Query the source of truth instead.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ContractsView.swift (2)
278-286: Don’t auto-dismiss on failure.The toolbar’s Fetch button dismisses regardless of success; users can’t correct a bad ID. Dismiss only on success.
- Button("Fetch") { - Button("Fetch") { + Button("Fetch") { Task { - await fetchContract() - if !isLoading { - dismiss() - } + await fetchContract() } }
292-317: Wire success path to dismissal and persistence; keep errors onscreen.Move dismissal into the success branch, and (optionally) persist/display the fetched contract.
@MainActor private func fetchContract() async { guard let sdk = appState.sdk else { appState.showError(message: "SDK not initialized") return } do { isLoading = true - if (try await sdk.getDataContract(id: contractIdToFetch)) != nil { - // Convert SDK contract to our model - // For now, we'll show a success message - appState.showError(message: "Contract fetched successfully") + if let _ = try await sdk.getDataContract(id: contractIdToFetch) { + // TODO: convert and append to appState.contracts + // appState.contracts.append(...) + dismiss() } else { appState.showError(message: "Contract not found") } isLoading = false } catch { appState.showError(message: "Failed to fetch contract: \(error.localizedDescription)") isLoading = false } }packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Account.swift (1)
30-63: Double-free risk: reusing a single FFIError across two calls with two defers.Both defers check and free error.message; after the second call, the first defer will free the same pointer again. Use separate error structs (and frees) per call.
- public func derivePrivateKeyWIF(wallet: Wallet, masterPath: String, index: UInt32) throws -> String { - var error = FFIError() + public func derivePrivateKeyWIF(wallet: Wallet, masterPath: String, index: UInt32) throws -> String { + var error1 = FFIError() // Derive master extended private key for this account root let masterPtr = masterPath.withCString { pathCStr in - wallet_derive_extended_private_key(wallet.ffiHandle, wallet.network.ffiValue, pathCStr, &error) + wallet_derive_extended_private_key(wallet.ffiHandle, wallet.network.ffiValue, pathCStr, &error1) } defer { - if error.message != nil { - error_message_free(error.message) + if let msg = error1.message { + error_message_free(msg) } if let m = masterPtr { extended_private_key_free(m) } } guard let master = masterPtr else { - throw KeyWalletError(ffiError: error) + throw KeyWalletError(ffiError: error1) } // Derive child private key as WIF at the given index - let wifPtr = account_derive_private_key_as_wif_at(self.handle, master, index, &error) + var error2 = FFIError() + let wifPtr = account_derive_private_key_as_wif_at(self.handle, master, index, &error2) defer { - if error.message != nil { - error_message_free(error.message) + if let msg = error2.message { + error_message_free(msg) } } guard let ptr = wifPtr else { - throw KeyWalletError(ffiError: error) + throw KeyWalletError(ffiError: error2) } let wif = String(cString: ptr) string_free(ptr) return wif }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/WalletViewModel.swift (1)
161-165: Avoid Double→UInt64 for money; switch to Decimal to prevent rounding bugs.Double can mis-send funds due to FP rounding.
- let amountDuffs = UInt64(amount * 100_000_000) + let amountDuffs: UInt64 = { + var dec = Decimal(amount) * 100_000_000 + var rounded = Decimal() + NSDecimalRound(&rounded, &dec, 0, .plain) + return NSDecimalNumber(decimal: rounded).uint64Value + }()packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Wallet.swift (1)
375-386: Validate the error code before returningaccountCount.The FFI returns a value even on error; mirror your
walletCounthandling and throw on non-zero error.public var accountCount: UInt32 { var error = FFIError() let count = wallet_get_account_count(handle, network.ffiValue, &error) defer { if error.message != nil { error_message_free(error.message) } } - - return count + + if error.code != FFIErrorCode(rawValue: 0) { + // propagate the underlying FFI error + throw KeyWalletError(ffiError: error) + } + return count }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (2)
272-283: Improve error handling for detached sync task.The detached task for sync doesn't preserve the task handle, making it impossible to cancel if needed. Also, updating
WalletService.shareddirectly from a detached context could be problematic if the service instance changes.- Task.detached(priority: .userInitiated) { + syncTask = Task.detached(priority: .userInitiated) { [weak self] in do { try await spvClient.startSync() } catch { await MainActor.run { - WalletService.shared.lastSyncError = error - WalletService.shared.isSyncing = false + self?.lastSyncError = error + self?.isSyncing = false } print("❌ Sync failed: \(error)") } }
465-483: Potential memory leak with timer not being invalidated.The timer created in
beginSPVStatsPollingcaptures references that could prevent proper cleanup. Consider storing and invalidating the timer appropriately.The timer is stored in
spvStatsTimerbut there's no guarantee it's invalidated when the service is deallocated. Add proper cleanup:deinit { // Avoid capturing self across an async boundary; capture the client locally let client = spvClient + spvStatsTimer?.invalidate() + spvStatsTimer = nil Task { @MainActor in client?.stop() } }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TransitionInputView.swift (3)
428-433: Same onChange compatibility concern as above.Use SelectionChangeHandler here as well:
- .onChange(of: value) { _, newValue in - selectedDocumentType = newValue - // Notify parent to update schema - onSpecialAction("documentTypeSelected:\(newValue)") - } + .modifier(SelectionChangeHandler(value: $value) { newValue in + selectedDocumentType = newValue + onSpecialAction("documentTypeSelected:\(newValue)") + })
555-560: onChange compatibility for recipient identity selector.- .onChange(of: value) { _, newValue in - if newValue == "__manual__" { - value = "" - useManualEntry = true - } - } + .modifier(SelectionChangeHandler(value: $value) { newValue in + if newValue == "__manual__" { + value = "" + useManualEntry = true + } + })
339-343: onChange uses iOS 17 signature — add iOS 16 compatibility wrapper.Match the project's existing compatibility pattern so iOS 16 builds continue to compile.
File: packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TransitionInputView.swift (lines 339-343)
- .onChange(of: value) { _, newValue in - selectedContractId = newValue - // Notify parent to update related fields - onSpecialAction("contractSelected:\(newValue)") - } + .modifier(SelectionChangeHandler(value: $value) { newValue in + selectedContractId = newValue + onSpecialAction("contractSelected:\(newValue)") + })Add once in this file:
private struct SelectionChangeHandler<T: Equatable>: ViewModifier { @Binding var value: T let onChange: (T) -> Void func body(content: Content) -> some View { if #available(iOS 17.0, *) { content.onChange(of: value) { _, newValue in onChange(newValue) } } else { content.onChange(of: value) { newValue in onChange(newValue) } } } }
🧹 Nitpick comments (74)
packages/rs-dpp/src/fee/fee_result/mod.rs (1)
151-152: Polish the comment grammar for clarity.Pluralize and tighten phrasing.
- // when we add balance we are sure that all the storage fee and processing fee has - // been paid + // When we add balance, we know all storage and processing fees have been paid.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Models/Network.swift (3)
20-28: Avoid rawValue-based mapping; use explicit enum casesConstructing
DashSDKNetworkvia magic numbers is brittle. Prefer direct cases to prevent breakage if raw values change.Apply:
- var sdkNetwork: SwiftDashSDK.Network { - switch self { - case .mainnet: - return DashSDKNetwork(rawValue: 0) - case .testnet: - return DashSDKNetwork(rawValue: 1) - case .devnet: - return DashSDKNetwork(rawValue: 2) - } - } + var sdkNetwork: SwiftDashSDK.Network { + switch self { + case .mainnet: return .mainnet + case .testnet: return .testnet + case .devnet: return .devnet + } + }If
SwiftDashSDK.Networkdoesn’t expose cases, consider a failable init helper instead to encapsulate rawValue mapping.
31-33: Confirm default network policyDefaulting to
.testnetis fine for dev, but double-check this isn’t shipped in release builds. Consider gating via build config or user setting.
35-45: Mapping toKeyWalletNetworkis clearStraightforward, mirrors cases 1:1. Optionally expose as a computed property (
keyWalletNetwork) for symmetry withsdkNetwork.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokensView.swift (1)
468-474: Clarify UX on preconditions and confine UI mutations to main threadThe combined guard shows “Please select an identity” even when the SDK is nil, and the function mutates UI state. Separate the checks for accurate messaging and mark the function as MainActor-safe.
- private func performTokenAction() async { + @MainActor + private func performTokenAction() async { - guard appState.sdk != nil, - selectedIdentity != nil else { - appState.showError(message: "Please select an identity") - return - } + if appState.sdk == nil { + appState.showError(message: "SDK not initialized. Please configure connection in Options.") + return + } + guard selectedIdentity != nil else { + appState.showError(message: "Please select an identity") + return + }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/KeysListView.swift (5)
5-5: Prefer a semantic, UInt32-backed identifier (avoids lossy Int conversions).Rename the wrapper and store the UInt32 directly for clarity and to prevent potential 32-bit overflow pitfalls.
- struct IdentifiableInt: Identifiable { let id: Int } + struct KeyId: Identifiable { let id: UInt32 }
7-7: Remove redundant = nil on @State optional.Swift optionals default to nil; keep it lean. If you adopt KeyId above, update the type accordingly.
- @State private var showingPrivateKey: IdentifiableInt? = nil + @State private var showingPrivateKey: IdentifiableInt?
25-25: Avoid Int(publicKey.id) conversion; use UInt32-backed wrapper or guard exact conversion.Primary: use the KeyId wrapper proposed above. Alternatively, verify 64-bit-only targets or guard Int(exactly:).
Primary change:
- showingPrivateKey = IdentifiableInt(id: Int(publicKey.id)) + showingPrivateKey = KeyId(id: publicKey.id)Alternative (if keeping Int):
- showingPrivateKey = IdentifiableInt(id: Int(publicKey.id)) + guard let intId = Int(exactly: publicKey.id) else { + assertionFailure("KeyId overflow on this architecture") + return + } + showingPrivateKey = IdentifiableInt(id: intId)
83-83: Drop ‘let _ =’ and gate debug prints.Printing is side-effect only; avoid binding and wrap in DEBUG to keep release logs clean.
- let _ = print("🔑 Sheet presenting for keyId: \(keyId.id)") + #if DEBUG + print("🔑 Sheet presenting for keyId: \(keyId.id)") + #endif
86-86: Eliminate unnecessary cast when using a UInt32-backed KeyId.If you adopt KeyId(id: UInt32), the cast can go away.
- keyId: UInt32(keyId.id), + keyId: keyId.id,packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CreateWalletView.swift (2)
203-211: Optional: align with app guideline to “Use NavigationLink for drill-down”To match the app guideline, consider value-based navigation: push with
NavigationLink(value:)and declarenavigationDestination(for:). This keeps push semantics explicit and avoids boolean state. Example (outside this hunk):// Add once: enum CreateWalletRoute: Hashable { case seedBackup(String) } // In body (inside a NavigationStack scope): .navigationDestination(for: CreateWalletRoute.self) { route in switch route { case .seedBackup(let m): SeedBackupView(mnemonic: m, onConfirm: { createWallet(using: m) }) } } // Where you currently set showBackupScreen = true: path.append(CreateWalletRoute.seedBackup(generatedMnemonic))
207-209: Capture mnemonic to avoid stale state in the confirm closureMinor: capture the current mnemonic so later state changes don’t affect the action.
- onConfirm: { - createWallet(using: generatedMnemonic) - } + onConfirm: { [mnemonic = generatedMnemonic] in + createWallet(using: mnemonic) + }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Helpers/WIFParser.swift (4)
20-23: Correct the WIF structure comment.The version byte description is wrong; Dash uses 0xCC (mainnet) and 0xEF (testnet). Also note the 0x01 compression flag semantics. (docs.dash.org)
Apply this diff:
- // - 1 byte: version (0xCC for testnet, 0xD2 for mainnet) + // - 1 byte: version (0xCC for Dash mainnet, 0xEF for testnet) // - 32 bytes: private key - // - (optional) 1 byte: 0x01 for compressed public key + // - (optional) 1 byte: 0x01 indicating the corresponding public key is compressed // - 4 bytes: checksum
11-13: Fix misleading prefix notes or drop them.The testnet prefix line lists “c” twice and is inaccurate. Either remove the prefix hints or correct them; rely on the version bytes instead.
Apply this diff:
- // - Mainnet: starts with '7' (uncompressed) or 'X' (compressed) - // - Testnet: starts with 'c' (uncompressed) or 'c' (compressed) + // Note: WIF human-readable prefixes vary by network and compression. + // Rely on version bytes (0xCC mainnet, 0xEF testnet) for correctness.
75-108: Prefer SDK/FFI helpers over custom Base58 for reliability and maintenance.Per repo guidelines, the Example App should call Core SDK functions (dash_core_sdk_*) where available. If the Core SDK exposes Base58Check/WIF helpers, replace these bespoke implementations to avoid crypto footguns and reduce maintenance.
Would you like me to wire this to the appropriate dash_core_sdk_* calls (or add thin wrappers) and add round‑trip tests?
Also applies to: 110-159
171-173: Minor: consider moving imports to the top or switching to CryptoKit.Importing at the bottom works but is unconventional. Optionally switch to CryptoKit (
import CryptoKit+Data(SHA256.hash(data: data))) to drop the CommonCrypto dependency on Apple platforms.If CryptoKit isn’t viable across your current targets, keeping CommonCrypto is fine—just confirm Xcode/SPM module availability in CI.
AGENTS.md (1)
46-46: Fix list indentation to satisfy markdownlint (MD005/MD007).Align the MCP bullet with its siblings under “iOS Notes”.
- - iOS Simulator MCP server: see `packages/swift-sdk/IOS_SIMULATOR_MCP.md` for Codex config, tools, and usage. Default output dir set via `IOS_SIMULATOR_MCP_DEFAULT_OUTPUT_DIR`. +- iOS Simulator MCP server: see `packages/swift-sdk/IOS_SIMULATOR_MCP.md` for Codex config, tools, and usage. Default output dir set via `IOS_SIMULATOR_MCP_DEFAULT_OUTPUT_DIR`.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Utils/EnvLoader.swift (1)
4-5: Avoid main-actor isolation for file I/O utility.@mainactor on EnvLoader forces blocking I/O on the main thread. Either remove it or mark specific UI-bound entry points @mainactor.
-@MainActor struct EnvLoader {packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/KeychainManager.swift (1)
5-6: Main-actor isolation for Keychain access is conservative; consider relaxing.Keychain APIs aren’t UI-bound. Keeping this off the main actor avoids accidental blocking.
-@MainActor final class KeychainManager {packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Models/DPP/DPPCoreTypes.swift (1)
202-206: Export Sendable conformance (prefer derived over @unchecked).PlatformValue’s cases are structurally Sendable. Prefer deriving Sendable on the enum and drop the unchecked extension, or at minimum make the conformance public so downstream modules see it.
Option A (preferred): derive Sendable and remove the extension.
-public enum PlatformValue: Codable, Equatable { +public enum PlatformValue: Sendable, Codable, Equatable { @@ -} - -// In Swift 6 strict concurrency, PlatformValue is frequently passed across -// actor boundaries as part of dictionaries. Its cases contain only Sendable -// payloads (Bool, integers, Double, String, Data, arrays/maps of PlatformValue). -// We mark it as @unchecked Sendable to permit usage in cross-actor contexts. - -extension PlatformValue: @unchecked Sendable {} +}Option B (if derivation fails in your toolchain): keep unchecked but export it.
-extension PlatformValue: @unchecked Sendable {} +public extension PlatformValue: @unchecked Sendable {}packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/WalletManager.swift (1)
132-132: Mark HDWallet.createAccount as @discardableResult to avoid underscore assignments.You’re intentionally discarding the return value. Annotate the API to make this pattern idiomatic and silence future warnings at call sites.
Apply at the createAccount declaration (in HDWallet):
- func createAccount(at index: UInt32) -> HDAccount + @discardableResult + func createAccount(at index: UInt32) -> HDAccountpackages/swift-sdk/Sources/SwiftDashSDK/ConcurrencyCompat.swift (1)
7-8: Avoid globally marking OpaquePointer as Sendable.This widens unsoundness across the module. Now that you’re migrating to typed UnsafeMutablePointer<FFI…> handles, place @unchecked Sendable on the specific Swift wrapper types instead (or confine usage to actor-bound APIs).
-extension OpaquePointer: @unchecked Sendable {} +// Avoid global Sendable on raw pointers; prefer wrapper types that encode ownership.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Models/SwiftData/PersistentIdentity.swift (1)
164-221: MainActor on from(_:) is consistent; just ensure heavy crypto isn’t blocking UI.If this path becomes heavy, consider offloading crypto to a background Task and only mutate SwiftData on MainActor.
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/IdentityDetailView.swift (1)
493-493: Missing trailing newlineThe file should end with a newline character for consistency with the rest of the codebase.
-} +} +packages/rs-sdk-ffi/src/spv_bridge.rs (1)
1-1: Consider removing the blanket non_camel_case_types allowanceThe module-level
#![allow(non_camel_case_types)]suppresses warnings for all non-camel-case types. Since this module only contains two simple wrapper functions that don't define any types, this allowance appears unnecessary.-#![allow(non_camel_case_types)]packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedWallet.swift (1)
424-427: Redundant typed pointer conversionThe function returns the handle directly without any transformation. Since
handleis already typed asUnsafeMutablePointer<FFIManagedWalletInfo>, this helper adds no value.Consider removing this helper function and using
handledirectly at call sites, or if the abstraction is valuable for future changes, simplify it:private func getInfoHandle() -> UnsafeMutablePointer<FFIManagedWalletInfo>? { - // The handle is an FFIManagedWalletInfo* (opaque C handle) - return handle + handle }Or remove the function entirely and replace all
getInfoHandle()calls with directhandleusage.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TransitionDetailView.swift (1)
952-954: Simplified nil-check changes validation logicThe change from
guard let signingKey = selectedKeytoguard selectedKey != nilis correct but could be clearer by keeping the original pattern sinceselectedKeyis used later.- guard selectedKey != nil, let keyData = privateKeyData else { + guard let selectedKey = selectedKey, let keyData = privateKeyData else { throw SDKError.invalidParameter("No suitable key with available private key found for signing") }packages/swift-sdk/IOS_SIMULATOR_MCP.md (2)
72-406: Add language identifiers to fenced code blocksMultiple fenced code blocks are missing language identifiers, which impacts syntax highlighting and readability.
Apply language identifiers to the parameter definition blocks. For example:
-``` +```typescript { /** * Udid of target, can also be set with the IDB_UDID env varThis applies to lines 72, 88, 112, 133, 159, 179, 195, 219, 252, 258, 264, 270, 276, 282, 288, 294, 299, and 406.
34-34: Convert bare URLs to proper Markdown linksBare URLs should be converted to proper Markdown links for better formatting.
-Source: https://github.com/joshuayoes/ios-simulator-mcp/blob/main/README.md +Source: [https://github.com/joshuayoes/ios-simulator-mcp/blob/main/README.md](https://github.com/joshuayoes/ios-simulator-mcp/blob/main/README.md)-https://github.com/user-attachments/assets/453ebe7b-cc93-4ac2-b08d-0f8ac8339ad3 +[Demo Video](https://github.com/user-attachments/assets/453ebe7b-cc93-4ac2-b08d-0f8ac8339ad3)Also applies to: 47-47
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (1)
275-283: Consider making peer configuration more flexible.The
readLocalCorePeersmethod defaults to "127.0.0.1" when no configuration is provided, which is reasonable. However, consider allowing multiple default addresses for different networks.Consider supporting network-specific defaults:
-private static func readLocalCorePeers() -> [String] { +private static func readLocalCorePeers(for network: Network? = nil) -> [String] { // If no override is set, default to 127.0.0.1 and let FFI pick port by network let raw = UserDefaults.standard.string(forKey: "corePeerAddresses")?.trimmingCharacters(in: .whitespacesAndNewlines) - let list = (raw?.isEmpty == false ? raw! : "127.0.0.1") + let defaultPeer = network == DashSDKNetwork(rawValue: 0) ? "127.0.0.1:9999" : "127.0.0.1" + let list = (raw?.isEmpty == false ? raw! : defaultPeer) return list .split(separator: ",") .map { $0.trimmingCharacters(in: .whitespaces) } .filter { !$0.isEmpty } }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DiagnosticsView.swift (3)
160-161: Main-actor hop for long-running I/O — confirm SDK calls are non-blocking.runAllQueries runs all queries on the main actor; if any SDK call performs blocking FFI or CPU work, the UI will stutter. Consider keeping only UI mutations on MainActor and running each test in a background Task, hopping back for state updates.
- @MainActor - private func runAllQueries() { + @MainActor + private func runAllQueries() { ... - Task { + Task.detached(priority: .userInitiated) { var testResults: [QueryTestResult] = [] ... - do { - let result = try await query.test() + do { + let result = try await query.test() // if test closures must remain @MainActor, wrap call in MainActor.run let duration = Date().timeIntervalSince(startTime) ... - testResult = QueryTestResult( + let success = QueryTestResult( ... ) - } catch { + } catch { ... } ... - } + } }
173-174: Tighten the test closure isolation if possible.Marking every test as @mainactor serializes them and can throttle throughput. If underlying SDK methods are actor-safe, prefer nonisolated closures and isolate only result handling.
587-592: macOS branch needs AppKit import.NSPasteboard usage in the #else branch requires AppKit to compile for macOS targets.
-#if os(iOS) +#if os(iOS) UIPasteboard.general.string = report #else + import AppKit NSPasteboard.general.clearContents() NSPasteboard.general.setString(report, forType: .string) #endifpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/TransactionService.swift (1)
140-144: Good: non-detached Task with local client avoids non-Sendable capture.Capturing a local SPVClient and using a plain Task is a safe pattern here. Consider surfacing startSync errors to lastError for observability.
Task(priority: .userInitiated) { - try? await client.startSync() + do { try await client.startSync() } + catch { await MainActor.run { self.lastError = error } } }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/UnifiedAppState.swift (2)
75-82: Minor: Redundant MainActor.run inside a @mainactor type.You’re already on MainActor here; direct call is fine.
93-93: Synchronous stop is fine; consider awaiting shutdown when you make it async.When stopSync becomes async or performs I/O, prefer an awaited path to avoid tearing down mid-flight work.
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ContractsView.swift (1)
255-257: Use modern text input APIs.autocapitalization is deprecated. Prefer textInputAutocapitalization(.never).
-TextField("Contract ID", text: $contractIdToFetch) - .textContentType(.none) - .autocapitalization(.none) +TextField("Contract ID", text: $contractIdToFetch) + .textContentType(.none) + .textInputAutocapitalization(.never)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/OptionsView.swift (1)
41-56: Nice: network override toggles; ensure UI is disabled during switch.The isSwitchingNetwork guard is right. Consider disabling these toggles while switching to avoid concurrent re-inits.
Toggle("Use Local DAPI (Platform)", isOn: $appState.useLocalPlatform) + .disabled(isSwitchingNetwork) ... Toggle("Use Local Core (SPV)", isOn: $appState.useLocalCore) + .disabled(isSwitchingNetwork)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DocumentsView.swift (3)
264-269: Only dismiss on success; surface errors to the user.Make createDocument throwing and catch near the button action so the sheet stays open on failure and shows an error. This also aligns with adding real SDK calls later.
@@ - Button("Create") { - Task { - await createDocument() - dismiss() - } - } + Button("Create") { + Task { + do { + try await createDocument() + dismiss() + } catch { + appState.showError(message: error.localizedDescription) + } + } + } @@ - private func createDocument() async { + private func createDocument() async throws {Also applies to: 285-286
286-291: Use Platform SDK functions (dash_sdk_*) instead of stubbing document creation.Per guidelines, call Platform SDK APIs (dash_sdk_*) to build, sign, and submit the document rather than constructing DocumentModel directly. Keep the guard on sdk until replaced.
If you confirm the current API shape, I can wire this to the appropriate dash_sdk_* calls (create/submit) and map SDK errors to UI alerts.
Also applies to: 295-298
307-307: Don’t use an “error” presenter for success UX.Use a dedicated success/toast/banner method (e.g., AppState.showSuccess) for positive feedback; reserve showError for failures.
Proposed addition in AppState (outside this file):
+func showSuccess(message: String) { + // Implement as non-error banner/toast state + errorMessage = message + showError = false + // e.g., set a separate showSuccess flag if available +}Then update here:
- appState.showError(message: "Document created successfully") + appState.showSuccess(message: "Document created successfully")packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/AddressPool.swift (2)
64-69: Pre-size result array for fewer reallocations.Reserve capacity once we know count.
- var addresses: [AddressInfo] = [] + var addresses: [AddressInfo] = [] + addresses.reserveCapacity(count) for i in 0..<count { if let infoPtr = infosPtr[i] { addresses.append(AddressInfo(ffiInfo: infoPtr.pointee)) } }
5-5: Seal the class to prevent unintended subclassing.Marking this wrapper final avoids vtable overhead and subclassing of a raw-pointer owner.
-public class AddressPool { +public final class AddressPool {packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/EdDSAAccount.swift (1)
5-12: Make the wrapper final for safety and perf.These FFI owner classes shouldn’t be subclassed.
-public class EdDSAAccount { +public final class EdDSAAccount {packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/QueryDetailView.swift (3)
462-476: Make DPNS filtering resilient to schema key drift and avoid repeated constants.The upstream payload may expose either "contractId" or "dataContractId". Filter defensively and consider centralizing the DPNS contract ID to avoid duplication across cases.
Apply:
- // Filter results to only show DPNS-related votes - let dpnsVotes = votes.filter { vote in - if let contractId = vote["contractId"] as? String { - return contractId == dpnsContractId - } - return false - } + // Filter results to only show DPNS-related votes (accept both keys) + let dpnsVotes = votes.filter { + ( $0["contractId"] as? String ?? $0["dataContractId"] as? String ) == dpnsContractId + } return dpnsVotesIf desired, add:
private let dpnsContractId = "GWRSAVFMjXx8HpQFaNJMqBV7MBgMK4br5UESsB4S31Ec"and reuse it in all DPNS paths.
490-496: Ordering is hard-coded to ascending; expose or document the choice.You force
orderAscending: true. Either surface this as an input (like other query cases) or add a brief rationale so UI/QA expectations match the API behavior.
500-506: Harden DPNS poll filtering against key name variance.Same concern as votes: accept both "contractId" and "dataContractId".
Apply:
- let dpnsPolls = polls.filter { poll in - if let contractId = poll["contractId"] as? String { - return contractId == dpnsContractId - } - return false - } + let dpnsPolls = polls.filter { + ( $0["contractId"] as? String ?? $0["dataContractId"] as? String ) == dpnsContractId + } return dpnsPollspackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccountCollection.swift (2)
214-216: Confirm memory binding correctness for provider operator keys.
assumingMemoryBound(to: FFIManagedAccount.self)is fine only if the pointer is already bound to that type. Verify the FFI function returns memory with that binding; otherwise prefer returning a typed pointer from the FFI or binding explicitly once at the boundary.
228-230: Same binding concern for provider platform keys.Ensure the raw pointer is correctly bound to
FFIManagedAccountor return a typed pointer from FFI to avoid UB.packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/AccountCollection.swift (2)
25-27: Prefer typed pointers from FFI overassumingMemoryBound.To avoid reliance on binding assumptions, consider having
account_collection_get_provider_operator_keysreturnUnsafeMutablePointer<FFIBLSAccount>?so you can pass it through without rebinding.
36-38: Same suggestion for EdDSA provider platform keys.Return
UnsafeMutablePointer<FFIEdDSAAccount>?from FFI to remove theassumingMemoryBoundhop.packages/swift-sdk/build_ios.sh (1)
30-34: Symlink instead of copying to reduce churn and size.Keeps the Swift package pointing at the built product without duplicating artifacts.
DEST_XCFRAMEWORK_DIR="$SCRIPT_DIR/DashSDKFFI.xcframework" -rm -rf "$DEST_XCFRAMEWORK_DIR" -cp -R "$SRC_XCFRAMEWORK_DIR" "$DEST_XCFRAMEWORK_DIR" +rm -rf "$DEST_XCFRAMEWORK_DIR" +ln -sfn "$SRC_XCFRAMEWORK_DIR" "$DEST_XCFRAMEWORK_DIR"packages/swift-sdk/verify_build.sh (2)
8-9: Call the builder via bash for portability.Invoking the script with bash avoids relying on the executable bit and mismatched shebangs in some environments.
-"$SCRIPT_DIR/build_ios.sh" +bash "$SCRIPT_DIR/build_ios.sh"
14-25: Surface build logs on failure.With
-quiet, failures can be opaque. Capture logs to a file and print its path on failure to ease debugging.- xcodebuild -project "$SCRIPT_DIR/SwiftExampleApp/SwiftExampleApp.xcodeproj" \ + xcodebuild -project "$SCRIPT_DIR/SwiftExampleApp/SwiftExampleApp.xcodeproj" \ -scheme SwiftExampleApp \ -sdk iphonesimulator \ - -destination 'platform=iOS Simulator,name=iPhone 16' \ - -quiet build + -destination 'generic/platform=iOS Simulator' \ + -derivedDataPath "$SCRIPT_DIR/.DerivedData" \ + -quiet build 2>&1 | tee "$SCRIPT_DIR/xcodebuild.log" XC_STATUS=$? set -e if [[ $XC_STATUS -ne 0 ]]; then - echo "❌ Xcode build failed" + echo "❌ Xcode build failed. See $SCRIPT_DIR/xcodebuild.log" exit $XC_STATUS fipackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift (2)
218-219: Avoid magic number for NOT_FOUND.Use a named constant/extension instead of
10to make intent clear and avoid drift if the FFI enum changes.- if error.code == FFIErrorCode(rawValue: 10) { // NOT_FOUND + if error.code == .notFound {Add this (outside the function, in this file):
private extension FFIErrorCode { static let notFound = FFIErrorCode(rawValue: 10) }
257-377: DRY up receive/change address flows.
getReceiveAddressandgetChangeAddressduplicate the same “managedInfo + wallet + address” flow. Consider extracting a private helper that accepts the selector to call.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/IdentitiesView.swift (1)
100-112: Consider extracting balance parsing logicThe balance extraction logic handles both NSNumber and String representations well, but this same pattern appears in
refreshBalance()at lines 256-262. Consider extracting this into a helper function to reduce duplication.Consider creating a helper extension:
extension Dictionary where Key == String, Value == Any { func extractBalance() -> UInt64 { guard let balanceValue = self["balance"] else { return 0 } if let balanceNum = balanceValue as? NSNumber { return balanceNum.uint64Value } else if let balanceString = balanceValue as? String, let balanceUInt = UInt64(balanceString) { return balanceUInt } else { return 0 } } }Then use it in both places:
-if let balanceValue = fetchedIdentity["balance"] { - let newBalanceLocal: UInt64 = { - if let balanceNum = balanceValue as? NSNumber { - return balanceNum.uint64Value - } else if let balanceString = balanceValue as? String, - let balanceUInt = UInt64(balanceString) { - return balanceUInt - } else { - return 0 - } - }() - appState.updateIdentityBalance(id: identityId, newBalance: newBalanceLocal) -} +let newBalance = fetchedIdentity.extractBalance() +appState.updateIdentityBalance(id: identityId, newBalance: newBalance)packages/rs-sdk-ffi/Cargo.toml (2)
20-23: Consider documenting the build requirements for iOS/macOS.The switch to local path dependencies for iOS/macOS targets requires developers to have the
rust-dashcorerepository available locally. This build requirement should be documented in the README or build instructions.Would you like me to help create documentation that explains:
- The required directory structure for building iOS/macOS targets
- Instructions for cloning and positioning the rust-dashcore repository
- Alternative approaches like git submodules or workspace configuration
12-13: Remove empty lines between package metadata and dependencies.These consecutive empty lines are unnecessary and inconsistent with Rust conventions.
- - +packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (1)
79-79: Remove redundant nil initialization.Initializing an optional variable with
nilis redundant in Swift as optionals are automatically initialized tonil.- var startHeight: UInt32? = nil + var startHeight: UInt32?packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/AppState.swift (1)
41-42: TODO: Implement SPV peer reconfiguration.The TODO comment indicates that SPV client peer reconfiguration is not yet implemented when
useLocalCorechanges.Would you like me to help implement the SPV peer reconfiguration logic? This would involve:
- Stopping the current SPV client
- Reinitializing with new peer configuration
- Restarting the sync process
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/SDK/PlatformQueryExtensions.swift (5)
6-12: Excessive @mainactor usage may limit concurrency flexibility.The @mainactor annotation on the extension forces all methods to run on the main thread. While this ensures thread safety, many of these operations (especially network calls through FFI) would benefit from running on background queues. Consider making only UI-affecting methods @mainactor and using more granular concurrency control for others.
9-12: Consider using Swift's built-inUnsafeMutableRawPointerdirectly.The
SendablePtrwrapper is unnecessary sinceUnsafeMutablePointeris alreadySendablewhen the pointee is not accessed across isolation boundaries. You can pass the pointer value directly across async boundaries.Apply this simplification:
- // Helper to pass non-Sendable pointers across @Sendable closures when safe - private final class SendablePtr<T>: @unchecked Sendable { - let ptr: UnsafeMutablePointer<T> - init(_ p: UnsafeMutablePointer<T>) { self.ptr = p } - }
387-388: Repeated pointer conversion pattern could be extracted.The pattern
UnsafePointer(contractHandle.assumingMemoryBound(to: DataContractHandle.self))is repeated multiple times. Consider extracting this into a helper function for cleaner code.Add a helper function at the beginning of the extension:
@inline(__always) private func contractPtr(_ handle: UnsafeMutableRawPointer) -> UnsafePointer<DataContractHandle> { UnsafePointer(handle.assumingMemoryBound(to: DataContractHandle.self)) }Then use it:
-searchParams.data_contract_handle = UnsafePointer(contractHandle.assumingMemoryBound(to: DataContractHandle.self)) +searchParams.data_contract_handle = contractPtr(contractHandle)Also applies to: 407-408, 428-429
1102-1105: Token balance parsing should handle type conversion more safely.The iteration over the JSON dictionary and conversion to
UInt64could be more robust with better error handling.for (tokenId, balance) in json { - if let balanceNum = balance as? NSNumber { - balances[tokenId] = balanceNum.uint64Value + if let balanceNum = balance as? NSNumber { + balances[tokenId] = balanceNum.uint64Value + } else if let balanceStr = balance as? String, let balanceVal = UInt64(balanceStr) { + balances[tokenId] = balanceVal + } else { + print("⚠️ Failed to parse balance for token \(tokenId): \(balance)") } }
1370-1370: Remove trailing semicolon.Swift doesn't require semicolons at the end of statements.
-} +}packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/SDK/StateTransitionExtensions.swift (2)
1278-1280: Remove redundant nil initialization.SwiftLint correctly identifies that initializing optional variables with nil is redundant in Swift.
- var purchasedId: String? = nil - var purchasedOwner: String? = nil - var purchasedRevision: UInt64 = 0 + var purchasedId: String? + var purchasedOwner: String? + var purchasedRevision: UInt64 = 0
2265-2268: Fix indentation inconsistency.There appears to be an indentation issue in the token transfer method.
- ) - } - } - } + ) + } + } + }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DocumentFieldsView.swift (1)
266-271: Replace deprecated autocapitalization API.Use textInputAutocapitalization(.never) instead of .autocapitalization(.none).
- .autocapitalization(.none) + .textInputAutocapitalization(.never) .disableAutocorrection(true)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TransitionInputView.swift (1)
18-33: Prefer predicate helpers / computed flags for token filtering.To align with app guidelines and reduce manual rule checks, use PersistentToken helpers (e.g., mintableTokensPredicate, canManuallyMint) instead of inspecting manualMintingRules directly.
- var mintableTokens: [(token: PersistentToken, contract: PersistentDataContract)] { - var results: [(token: PersistentToken, contract: PersistentDataContract)] = [] - - for contract in dataContracts { - if let tokens = contract.tokens { - for token in tokens { - if token.manualMintingRules != nil { - results.append((token: token, contract: contract)) - } - } - } - } - return results.sorted(by: { $0.token.displayName < $1.token.displayName }) - } + var mintableTokens: [(token: PersistentToken, contract: PersistentDataContract)] { + var results: [(token: PersistentToken, contract: PersistentDataContract)] = [] + for contract in dataContracts { + let tokens = (contract.tokens ?? []).filter { $0.canManuallyMint } // or use mintableTokensPredicate via @Query + results += tokens.map { (token: $0, contract: contract) } + } + return results.sorted { $0.token.displayName < $1.token.displayName } + }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DocumentWithPriceView.swift (2)
156-162: Prefer Task-based debounce to avoid Timer retention quirks and main runloop coupling.- // Set up debounce timer to fetch after user stops typing - debounceTimer = Timer.scheduledTimer(withTimeInterval: 0.5, repeats: false) { _ in - Task { - await fetchDocument() - } - } + // Debounce with cancellable Task (no runloop dependency) + debounceTask?.cancel() + debounceTask = Task { @MainActor in + try? await Task.sleep(nanoseconds: 500_000_000) + await fetchDocument() + }Add state:
@State private var debounceTask: Task<Void, Never>?
203-216: Gate verbose debug prints.Wrap DEBUG logging with #if DEBUG to reduce console noise in release.
- // Debug: Log the entire document to see what fields are available - print("DEBUG: Document fetched successfully") + #if DEBUG + print("DEBUG: Document fetched successfully") print("DEBUG: Document keys: \(document.keys)") for (key, value) in document { print("DEBUG: \(key) = \(value) (type: \(type(of: value)))") // If it's a dictionary, log its contents too if let dict = value as? [String: Any] { print("DEBUG: \(key) contents:") for (subKey, subValue) in dict { print("DEBUG: \(subKey) = \(subValue) (type: \(type(of: subValue)))") } } } + #endifpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DynamicDocumentFormView.swift (2)
213-216: Replace deprecated autocapitalization API.- .autocapitalization(.none) + .textInputAutocapitalization(.never) .disableAutocorrection(true)
372-387: Byte-array representation differs across views; confirm expected payload type.Here you emit [UInt8], while DocumentFieldsView may provide Data for byte arrays. Ensure downstream encoding expects one consistent shape.
I can help normalize both views to a single representation after you confirm what the Platform SDK expects.
| # Treat Swift warnings as errors during xcodebuild | ||
| OTHER_SWIFT_FLAGS: -warnings-as-errors | ||
| SWIFT_TREAT_WARNINGS_AS_ERRORS: YES | ||
| run: | | ||
| xcodebuild \ | ||
| -project SwiftExampleApp/SwiftExampleApp.xcodeproj \ | ||
| -scheme SwiftExampleApp \ | ||
| -sdk iphonesimulator \ | ||
| -destination 'generic/platform=iOS Simulator' \ | ||
| -configuration Debug \ | ||
| ONLY_ACTIVE_ARCH=YES \ | ||
| OTHER_SWIFT_FLAGS="$OTHER_SWIFT_FLAGS" \ | ||
| SWIFT_TREAT_WARNINGS_AS_ERRORS=$SWIFT_TREAT_WARNINGS_AS_ERRORS \ |
There was a problem hiding this comment.
Quote environment variables to prevent word splitting
The OTHER_SWIFT_FLAGS and SWIFT_TREAT_WARNINGS_AS_ERRORS variables should be quoted in the shell command to prevent potential word splitting issues.
xcodebuild \
-project SwiftExampleApp/SwiftExampleApp.xcodeproj \
-scheme SwiftExampleApp \
-sdk iphonesimulator \
-destination 'generic/platform=iOS Simulator' \
-configuration Debug \
ONLY_ACTIVE_ARCH=YES \
- OTHER_SWIFT_FLAGS="$OTHER_SWIFT_FLAGS" \
- SWIFT_TREAT_WARNINGS_AS_ERRORS=$SWIFT_TREAT_WARNINGS_AS_ERRORS \
+ OTHER_SWIFT_FLAGS="${OTHER_SWIFT_FLAGS}" \
+ SWIFT_TREAT_WARNINGS_AS_ERRORS="${SWIFT_TREAT_WARNINGS_AS_ERRORS}" \
build📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Treat Swift warnings as errors during xcodebuild | |
| OTHER_SWIFT_FLAGS: -warnings-as-errors | |
| SWIFT_TREAT_WARNINGS_AS_ERRORS: YES | |
| run: | | |
| xcodebuild \ | |
| -project SwiftExampleApp/SwiftExampleApp.xcodeproj \ | |
| -scheme SwiftExampleApp \ | |
| -sdk iphonesimulator \ | |
| -destination 'generic/platform=iOS Simulator' \ | |
| -configuration Debug \ | |
| ONLY_ACTIVE_ARCH=YES \ | |
| OTHER_SWIFT_FLAGS="$OTHER_SWIFT_FLAGS" \ | |
| SWIFT_TREAT_WARNINGS_AS_ERRORS=$SWIFT_TREAT_WARNINGS_AS_ERRORS \ | |
| # Treat Swift warnings as errors during xcodebuild | |
| OTHER_SWIFT_FLAGS: -warnings-as-errors | |
| SWIFT_TREAT_WARNINGS_AS_ERRORS: YES | |
| run: | | |
| xcodebuild \ | |
| -project SwiftExampleApp/SwiftExampleApp.xcodeproj \ | |
| -scheme SwiftExampleApp \ | |
| -sdk iphonesimulator \ | |
| -destination 'generic/platform=iOS Simulator' \ | |
| -configuration Debug \ | |
| ONLY_ACTIVE_ARCH=YES \ | |
| OTHER_SWIFT_FLAGS="${OTHER_SWIFT_FLAGS}" \ | |
| SWIFT_TREAT_WARNINGS_AS_ERRORS="${SWIFT_TREAT_WARNINGS_AS_ERRORS}" \ | |
| build |
🧰 Tools
🪛 actionlint (1.7.7)
50-50: shellcheck reported issue in this script: SC2086:info:9:34: Double quote to prevent globbing and word splitting
(shellcheck)
🤖 Prompt for AI Agents
.github/workflows/swift-sdk-build.yml around lines 47 to 59: the shell
invocation passes OTHER_SWIFT_FLAGS and SWIFT_TREAT_WARNINGS_AS_ERRORS unquoted
which can cause word splitting; update the xcodebuild command to wrap these
variables in double quotes (e.g. OTHER_SWIFT_FLAGS="$OTHER_SWIFT_FLAGS" and
SWIFT_TREAT_WARNINGS_AS_ERRORS="$SWIFT_TREAT_WARNINGS_AS_ERRORS") so their
values are preserved as single arguments.
| key-wallet = { path = "../../../rust-dashcore/key-wallet" } | ||
| key-wallet-manager = { path = "../../../rust-dashcore/key-wallet-manager", optional = true } | ||
|
|
||
| # Core dependencies | ||
| dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "02d902c9845d5ed9e5cb88fd32a8c254742f20fd" } | ||
| dashcore = { path = "../../../rust-dashcore/dash" } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Path deps point outside packages/; move crates under packages/ per repo layout.*
Keeping all source packages under packages/* simplifies workspaces and tooling.
-key-wallet = { path = "../../../rust-dashcore/key-wallet" }
-key-wallet-manager = { path = "../../../rust-dashcore/key-wallet-manager", optional = true }
-dashcore = { path = "../../../rust-dashcore/dash" }
+key-wallet = { path = "../rust-dashcore/key-wallet" }
+key-wallet-manager = { path = "../rust-dashcore/key-wallet-manager", optional = true }
+dashcore = { path = "../rust-dashcore/dash" }Also add these crates to the workspace members and relocate rust-dashcore/ under packages/ to match guidelines.
🏁 Script executed:
#!/bin/bash
# Verify actual locations and workspace membership
fd -t d -E target -E node_modules -a '^rust-dashcore$' packages
rg -n '^\s*\[workspace\]' -n -C2 Cargo.toml
rg -nP 'members\s*=\s*\[' -n -C2 Cargo.tomlLength of output: 133
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo
echo "=== Directories named 'rust-dashcore' ==="
fd -t d -a rust-dashcore . || true
echo
echo "=== Grep for 'rust-dashcore' in repo ==="
rg -n --hidden --no-ignore-vcs 'rust-dashcore' || true
echo
echo "=== packages/rs-platform-wallet/Cargo.toml (first 200 lines) ==="
if [ -f packages/rs-platform-wallet/Cargo.toml ]; then sed -n '1,200p' packages/rs-platform-wallet/Cargo.toml; else echo "packages/rs-platform-wallet/Cargo.toml not found"; fi
echo
echo "=== Top-level Cargo.toml (first 300 lines) ==="
if [ -f Cargo.toml ]; then sed -n '1,300p' Cargo.toml; else echo "Top-level Cargo.toml not found"; fi
echo
echo "=== All workspace Cargo.toml files containing 'members' ==="
rg -nP '^\s*members\s*=' -S --hidden --no-ignore-vcs || trueLength of output: 8547
Move rust-dashcore into packages/ and update path deps + workspace
Multiple crates reference ../../../rust-dashcore (e.g., packages/rs-platform-wallet/Cargo.toml lines ~14–19); rust-dashcore is not under packages/. Actions required:
- Move rust-dashcore into packages/rust-dashcore.
- Update path dependencies to ../rust-dashcore/ (example below).
- Add the moved rust-dashcore crates to the top-level Cargo.toml members array.
- Update scripts/docs that expect an external rust-dashcore (e.g., scripts/dash_core_version_switcher.py, packages/rs-sdk-ffi/build_ios.sh, packages/swift-sdk/BUILD_GUIDE_FOR_AI.md) and any other occurrences.
-key-wallet = { path = "../../../rust-dashcore/key-wallet" }
-key-wallet-manager = { path = "../../../rust-dashcore/key-wallet-manager", optional = true }
-dashcore = { path = "../../../rust-dashcore/dash" }
+key-wallet = { path = "../rust-dashcore/key-wallet" }
+key-wallet-manager = { path = "../rust-dashcore/key-wallet-manager", optional = true }
+dashcore = { path = "../rust-dashcore/dash" }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| key-wallet = { path = "../../../rust-dashcore/key-wallet" } | |
| key-wallet-manager = { path = "../../../rust-dashcore/key-wallet-manager", optional = true } | |
| # Core dependencies | |
| dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "02d902c9845d5ed9e5cb88fd32a8c254742f20fd" } | |
| dashcore = { path = "../../../rust-dashcore/dash" } | |
| key-wallet = { path = "../rust-dashcore/key-wallet" } | |
| key-wallet-manager = { path = "../rust-dashcore/key-wallet-manager", optional = true } | |
| # Core dependencies | |
| dashcore = { path = "../rust-dashcore/dash" } |
🤖 Prompt for AI Agents
In packages/rs-platform-wallet/Cargo.toml around lines 14 to 19, multiple path
dependencies point to ../../../rust-dashcore which must be relocated under
packages/rust-dashcore; move the entire rust-dashcore folder into
packages/rust-dashcore, update all path entries in this file to use
../rust-dashcore/<crate> (e.g., key-wallet = { path =
"../rust-dashcore/key-wallet" }), then add each moved crate name to the
top-level Cargo.toml members array so the workspace includes them; finally
search and update any scripts/docs that reference the old external rust-dashcore
path (examples: scripts/dash_core_version_switcher.py,
packages/rs-sdk-ffi/build_ios.sh, packages/swift-sdk/BUILD_GUIDE_FOR_AI.md) to
use the new packages/rust-dashcore paths or workspace-aware references.
| // Forward declarations to ensure cross-refs compile regardless of merge order | ||
| typedef struct FFIClientConfig FFIClientConfig; | ||
| // Provide explicit opaque definitions so Swift can import the type names | ||
| typedef struct FFIDashSpvClient { unsigned char _private[0]; } FFIDashSpvClient; | ||
| typedef struct FFIWallet { unsigned char _private[0]; } FFIWallet; | ||
| typedef struct FFIAccount { unsigned char _private[0]; } FFIAccount; | ||
| typedef struct FFIAccountCollection { unsigned char _private[0]; } FFIAccountCollection; | ||
| typedef struct FFIBLSAccount { unsigned char _private[0]; } FFIBLSAccount; | ||
| typedef struct FFIEdDSAAccount { unsigned char _private[0]; } FFIEdDSAAccount; | ||
| typedef struct FFIAddressPool { unsigned char _private[0]; } FFIAddressPool; | ||
| typedef struct FFIManagedAccountCollection { unsigned char _private[0]; } FFIManagedAccountCollection; | ||
| typedef struct FFIWalletManager { unsigned char _private[0]; } FFIWalletManager; | ||
| typedef struct FFIManagedAccount { unsigned char _private[0]; } FFIManagedAccount; | ||
| // Platform SDK opaque handles | ||
| typedef struct SDKHandle { unsigned char _private[0]; } SDKHandle; | ||
| typedef struct DataContractHandle { unsigned char _private[0]; } DataContractHandle; | ||
| typedef struct DocumentHandle { unsigned char _private[0]; } DocumentHandle; | ||
| typedef struct IdentityHandle { unsigned char _private[0]; } IdentityHandle; | ||
| typedef struct IdentityPublicKeyHandle { unsigned char _private[0]; } IdentityPublicKeyHandle; | ||
| typedef struct SignerHandle { unsigned char _private[0]; } SignerHandle; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider the implications of zero-sized array pattern for opaque types.
The zero-sized array pattern unsigned char _private[0] is being used for opaque type definitions. While this works in many C compilers, it's technically undefined behavior in standard C and may cause issues with strict compilers.
Consider using standard forward declarations instead:
-// Forward declarations to ensure cross-refs compile regardless of merge order
-typedef struct FFIClientConfig FFIClientConfig;
-// Provide explicit opaque definitions so Swift can import the type names
-typedef struct FFIDashSpvClient { unsigned char _private[0]; } FFIDashSpvClient;
-typedef struct FFIWallet { unsigned char _private[0]; } FFIWallet;
+// Forward declarations for opaque types
+typedef struct FFIClientConfig FFIClientConfig;
+typedef struct FFIDashSpvClient FFIDashSpvClient;
+typedef struct FFIWallet FFIWallet;Or if you need sized types for some reason, use a non-zero size:
-typedef struct FFIDashSpvClient { unsigned char _private[0]; } FFIDashSpvClient;
+typedef struct FFIDashSpvClient { void *_private; } FFIDashSpvClient;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Forward declarations to ensure cross-refs compile regardless of merge order | |
| typedef struct FFIClientConfig FFIClientConfig; | |
| // Provide explicit opaque definitions so Swift can import the type names | |
| typedef struct FFIDashSpvClient { unsigned char _private[0]; } FFIDashSpvClient; | |
| typedef struct FFIWallet { unsigned char _private[0]; } FFIWallet; | |
| typedef struct FFIAccount { unsigned char _private[0]; } FFIAccount; | |
| typedef struct FFIAccountCollection { unsigned char _private[0]; } FFIAccountCollection; | |
| typedef struct FFIBLSAccount { unsigned char _private[0]; } FFIBLSAccount; | |
| typedef struct FFIEdDSAAccount { unsigned char _private[0]; } FFIEdDSAAccount; | |
| typedef struct FFIAddressPool { unsigned char _private[0]; } FFIAddressPool; | |
| typedef struct FFIManagedAccountCollection { unsigned char _private[0]; } FFIManagedAccountCollection; | |
| typedef struct FFIWalletManager { unsigned char _private[0]; } FFIWalletManager; | |
| typedef struct FFIManagedAccount { unsigned char _private[0]; } FFIManagedAccount; | |
| // Platform SDK opaque handles | |
| typedef struct SDKHandle { unsigned char _private[0]; } SDKHandle; | |
| typedef struct DataContractHandle { unsigned char _private[0]; } DataContractHandle; | |
| typedef struct DocumentHandle { unsigned char _private[0]; } DocumentHandle; | |
| typedef struct IdentityHandle { unsigned char _private[0]; } IdentityHandle; | |
| typedef struct IdentityPublicKeyHandle { unsigned char _private[0]; } IdentityPublicKeyHandle; | |
| typedef struct SignerHandle { unsigned char _private[0]; } SignerHandle; | |
| // Forward declarations for opaque types | |
| typedef struct FFIClientConfig FFIClientConfig; | |
| typedef struct FFIDashSpvClient FFIDashSpvClient; | |
| typedef struct FFIWallet FFIWallet; | |
| typedef struct FFIAccount { unsigned char _private[0]; } FFIAccount; | |
| typedef struct FFIAccountCollection { unsigned char _private[0]; } FFIAccountCollection; | |
| typedef struct FFIBLSAccount { unsigned char _private[0]; } FFIBLSAccount; | |
| typedef struct FFIEdDSAAccount { unsigned char _private[0]; } FFIEdDSAAccount; | |
| typedef struct FFIAddressPool { unsigned char _private[0]; } FFIAddressPool; | |
| typedef struct FFIManagedAccountCollection { unsigned char _private[0]; } FFIManagedAccountCollection; | |
| typedef struct FFIWalletManager { unsigned char _private[0]; } FFIWalletManager; | |
| typedef struct FFIManagedAccount { unsigned char _private[0]; } FFIManagedAccount; | |
| // Platform SDK opaque handles | |
| typedef struct SDKHandle { unsigned char _private[0]; } SDKHandle; | |
| typedef struct DataContractHandle { unsigned char _private[0]; } DataContractHandle; | |
| typedef struct DocumentHandle { unsigned char _private[0]; } DocumentHandle; | |
| typedef struct IdentityHandle { unsigned char _private[0]; } IdentityHandle; | |
| typedef struct IdentityPublicKeyHandle { unsigned char _private[0]; } IdentityPublicKeyHandle; | |
| typedef struct SignerHandle { unsigned char _private[0]; } SignerHandle; |
🤖 Prompt for AI Agents
In packages/rs-sdk-ffi/build_ios.sh around lines 166 to 185, the opaque types
are defined using the zero-sized array pattern (e.g., typedef struct
FFIDashSpvClient { unsigned char _private[0]; } FFIDashSpvClient;) which is
undefined in standard C and can break strict compilers; replace these with
standard forward-declaration opaque types (e.g., typedef struct FFIDashSpvClient
FFIDashSpvClient;) or, if a sized placeholder is required for some toolchain,
use a non-zero-sized private member (e.g., unsigned char _private[1];) for all
listed types (FFIDashSpvClient, FFIWallet, FFIAccount, FFIAccountCollection,
FFIBLSAccount, FFIEdDSAAccount, FFIAddressPool, FFIManagedAccountCollection,
FFIWalletManager, FFIManagedAccount, SDKHandle, DataContractHandle,
DocumentHandle, IdentityHandle, IdentityPublicKeyHandle, SignerHandle) so the
definitions are standards-compliant and safe across compilers.
| // Safety: Forward call to underlying FFI; pointers are opaque | ||
| unsafe { dash_spv_ffi::dash_spv_ffi_config_add_peer(config, addr) } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Safety comment doesn't justify the unsafe block
The safety comment states that pointers are opaque, but doesn't explain why the unsafe operation is sound. The wrapper should document the preconditions required for safe usage (e.g., valid pointers, proper initialization).
- // Safety: Forward call to underlying FFI; pointers are opaque
+ // Safety: This is a thin wrapper over the underlying FFI function.
+ // The caller must ensure:
+ // - `config` points to a valid FFIClientConfig instance
+ // - `addr` points to a valid null-terminated C string
+ // - The lifetime of both pointers extends through this call
unsafe { dash_spv_ffi::dash_spv_ffi_config_add_peer(config, addr) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Safety: Forward call to underlying FFI; pointers are opaque | |
| unsafe { dash_spv_ffi::dash_spv_ffi_config_add_peer(config, addr) } | |
| } | |
| // Safety: This is a thin wrapper over the underlying FFI function. | |
| // The caller must ensure: | |
| // - `config` points to a valid FFIClientConfig instance | |
| // - `addr` points to a valid null-terminated C string | |
| // - The lifetime of both pointers extends through this call | |
| unsafe { dash_spv_ffi::dash_spv_ffi_config_add_peer(config, addr) } | |
| } |
🤖 Prompt for AI Agents
In packages/rs-sdk-ffi/src/spv_bridge.rs around lines 12–14, the current safety
comment ("pointers are opaque") is insufficient; replace it with a concise
safety rationale that lists the required preconditions for the unsafe call
(e.g., `config` and `addr` must be non-null and properly initialized FFI
pointers, valid lifetimes for any referenced data, correct ownership/aliasing
guarantees, and that the FFI library has been initialized and is thread-safe for
this call). Also either add defensive runtime checks (e.g., debug_assert! or
explicit null checks) before the unsafe block or document that callers are
responsible for validating pointers, and mention what invariants are preserved
on success/failure so the unsafe call is clearly justified.
| // Safety: Forward call to underlying FFI; pointers are opaque | ||
| unsafe { dash_spv_ffi::dash_spv_ffi_config_set_restrict_to_configured_peers(config, restrict_peers) } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Safety comment doesn't justify the unsafe block
Similar to the previous function, the safety comment should document the preconditions for safe usage.
- // Safety: Forward call to underlying FFI; pointers are opaque
+ // Safety: This is a thin wrapper over the underlying FFI function.
+ // The caller must ensure:
+ // - `config` points to a valid FFIClientConfig instance
+ // - The lifetime of the config pointer extends through this call
unsafe { dash_spv_ffi::dash_spv_ffi_config_set_restrict_to_configured_peers(config, restrict_peers) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Safety: Forward call to underlying FFI; pointers are opaque | |
| unsafe { dash_spv_ffi::dash_spv_ffi_config_set_restrict_to_configured_peers(config, restrict_peers) } | |
| } | |
| // Safety: This is a thin wrapper over the underlying FFI function. | |
| // The caller must ensure: | |
| // - `config` points to a valid FFIClientConfig instance | |
| // - The lifetime of the config pointer extends through this call | |
| unsafe { dash_spv_ffi::dash_spv_ffi_config_set_restrict_to_configured_peers(config, restrict_peers) } | |
| } |
🤖 Prompt for AI Agents
In packages/rs-sdk-ffi/src/spv_bridge.rs around lines 22 to 24, the current
safety comment on the unsafe FFI call is insufficient; update it to list the
exact preconditions that make calling
dash_spv_ffi::dash_spv_ffi_config_set_restrict_to_configured_peers safe (e.g.,
`config` must be a non-null, properly initialized pointer created by the
corresponding FFI constructor, it must point to a valid heap object for the
lifetime of the call, `restrict_peers` must be a valid boolean value, and there
must be no concurrent mutable aliasing violating Rust aliasing rules), and
mention any responsibility for ownership/cleanup and thread-safety expectations;
then keep the unsafe block but ensure callers meet those preconditions (or add a
null-check/validation in Rust before the unsafe call if appropriate).
| @MainActor | ||
| public func dpnsGetUsername(identityId: String, limit: UInt32?) async throws -> [[String: Any]] { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
DPNS query methods should not require main thread execution.
Multiple DPNS query methods (dpnsGetUsername, dpnsGetNonResolvedContestsForIdentity, dpnsGetCurrentContests, dpnsGetContestedVoteState, dpnsSearch) are marked with @MainActor. These are network operations that don't need main thread execution and could cause UI freezes.
Remove @MainActor from these methods or dispatch FFI calls to background queues to prevent blocking the main thread.
Also applies to: 587-589, 653-654, 686-688, 815-816
🤖 Prompt for AI Agents
In
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/SDK/PlatformQueryExtensions.swift
around lines 555-556 (and similarly at 587-589, 653-654, 686-688, 815-816), the
DPNS query methods are annotated with @MainActor which forces network/FFI calls
onto the main thread; remove the @MainActor annotation from these methods so
they run off the main thread and either (a) perform FFI/network work directly
off the main thread or (b) wrap FFI calls in Task.detached /
DispatchQueue.global to ensure background execution, and if any UI/state updates
remain, explicitly dispatch those back to the main actor (e.g., Task {
@MainActor in ... } or DispatchQueue.main.async).
| func initialize() async { | ||
| do { | ||
| // Initialize Platform SDK | ||
| await MainActor.run { | ||
| platformState.initializeSDK(modelContext: modelContainer.mainContext) | ||
| } | ||
|
|
||
| // Wait for Platform SDK to be ready | ||
| try? await Task.sleep(nanoseconds: 500_000_000) // 0.5 second | ||
| // Initialize Platform SDK | ||
| await MainActor.run { | ||
| platformState.initializeSDK(modelContext: modelContainer.mainContext) | ||
| } | ||
|
|
||
| // Wait for Platform SDK to be ready | ||
| try? await Task.sleep(nanoseconds: 500_000_000) // 0.5 second | ||
|
|
||
| // If SDK reports trusted mode, disable masternode SPV sync | ||
| if let sdk = platformState.sdk { | ||
| do { | ||
| let status: SwiftDashSDK.SDKStatus = try sdk.getStatus() | ||
| let isTrusted = status.mode.lowercased() == "trusted" | ||
| await MainActor.run { self.walletService.setMasternodesEnabled(!isTrusted) } | ||
| } catch { | ||
| // Ignore status errors; keep default (false) until known | ||
| } | ||
| // If SDK reports trusted mode, disable masternode SPV sync | ||
| if let sdk = platformState.sdk { | ||
| do { | ||
| let status: SwiftDashSDK.SDKStatus = try sdk.getStatus() | ||
| let isTrusted = status.mode.lowercased() == "trusted" | ||
| await MainActor.run { self.walletService.setMasternodesEnabled(!isTrusted) } | ||
| } catch { | ||
| // Ignore status errors; keep default (false) until known | ||
| } | ||
|
|
||
| isInitialized = true | ||
| } catch { | ||
| self.error = error | ||
| } | ||
|
|
||
| isInitialized = true | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Initialization sets isInitialized = true even if SDK isn’t ready; avoid fixed sleeps.
- 0.5s sleep is a race; SDK may still be nil.
- isInitialized is set unconditionally, enabling UI while SDK is unavailable.
Gate readiness on platformState.sdk, and poll briefly or refactor initializeSDK to be async and awaitable.
- await MainActor.run {
- platformState.initializeSDK(modelContext: modelContainer.mainContext)
- }
-
- // Wait for Platform SDK to be ready
- try? await Task.sleep(nanoseconds: 500_000_000) // 0.5 second
+ platformState.initializeSDK(modelContext: modelContainer.mainContext)
+ // Wait (bounded) for SDK to appear
+ var attempts = 0
+ while platformState.sdk == nil && attempts < 20 {
+ try? await Task.sleep(nanoseconds: 100_000_000) // 100 ms
+ attempts += 1
+ }
- // If SDK reports trusted mode, disable masternode SPV sync
- if let sdk = platformState.sdk {
+ if let sdk = platformState.sdk {
do {
let status: SwiftDashSDK.SDKStatus = try sdk.getStatus()
let isTrusted = status.mode.lowercased() == "trusted"
- await MainActor.run { self.walletService.setMasternodesEnabled(!isTrusted) }
+ self.walletService.setMasternodesEnabled(!isTrusted)
} catch {
// Ignore status errors; keep default (false) until known
}
}
- isInitialized = true
+ isInitialized = (platformState.sdk != nil)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func initialize() async { | |
| do { | |
| // Initialize Platform SDK | |
| await MainActor.run { | |
| platformState.initializeSDK(modelContext: modelContainer.mainContext) | |
| } | |
| // Wait for Platform SDK to be ready | |
| try? await Task.sleep(nanoseconds: 500_000_000) // 0.5 second | |
| // Initialize Platform SDK | |
| await MainActor.run { | |
| platformState.initializeSDK(modelContext: modelContainer.mainContext) | |
| } | |
| // Wait for Platform SDK to be ready | |
| try? await Task.sleep(nanoseconds: 500_000_000) // 0.5 second | |
| // If SDK reports trusted mode, disable masternode SPV sync | |
| if let sdk = platformState.sdk { | |
| do { | |
| let status: SwiftDashSDK.SDKStatus = try sdk.getStatus() | |
| let isTrusted = status.mode.lowercased() == "trusted" | |
| await MainActor.run { self.walletService.setMasternodesEnabled(!isTrusted) } | |
| } catch { | |
| // Ignore status errors; keep default (false) until known | |
| } | |
| // If SDK reports trusted mode, disable masternode SPV sync | |
| if let sdk = platformState.sdk { | |
| do { | |
| let status: SwiftDashSDK.SDKStatus = try sdk.getStatus() | |
| let isTrusted = status.mode.lowercased() == "trusted" | |
| await MainActor.run { self.walletService.setMasternodesEnabled(!isTrusted) } | |
| } catch { | |
| // Ignore status errors; keep default (false) until known | |
| } | |
| isInitialized = true | |
| } catch { | |
| self.error = error | |
| } | |
| isInitialized = true | |
| } | |
| func initialize() async { | |
| // Initialize Platform SDK | |
| platformState.initializeSDK(modelContext: modelContainer.mainContext) | |
| // Wait (bounded) for SDK to appear | |
| var attempts = 0 | |
| while platformState.sdk == nil && attempts < 20 { | |
| try? await Task.sleep(nanoseconds: 100_000_000) // 100 ms | |
| attempts += 1 | |
| } | |
| if let sdk = platformState.sdk { | |
| do { | |
| let status: SwiftDashSDK.SDKStatus = try sdk.getStatus() | |
| let isTrusted = status.mode.lowercased() == "trusted" | |
| self.walletService.setMasternodesEnabled(!isTrusted) | |
| } catch { | |
| // Ignore status errors; keep default (false) until known | |
| } | |
| } | |
| isInitialized = (platformState.sdk != nil) | |
| } |
🤖 Prompt for AI Agents
In packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/UnifiedAppState.swift
around lines 65 to 86, the initializer sets isInitialized = true after a fixed
0.5s sleep which can race with platformState.sdk becoming available; change
initialize() to wait for SDK readiness instead of an arbitrary sleep: either
make platformState.initializeSDK async and await its completion, or poll
platformState.sdk with a short timeout/retry loop on the MainActor (e.g., check
every 50–100ms up to a max duration) and only proceed once sdk != nil (or
timeout and surface/log an error), then perform the SDK status check and set
isInitialized = true only after sdk readiness is confirmed.
| guard appState.sdk != nil, | ||
| let contract = selectedContract, | ||
| !selectedDocumentType.isEmpty else { | ||
| appState.showError(message: "Please select a contract and document type") | ||
| return | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden create flow: validate ownerId, restore robust loading handling, and avoid success via showError.
- Add a defer to always clear isLoading.
- Validate ownerId hex and length (32 bytes) before constructing the model.
- Keep the UI success path distinct from error reporting (consider adding AppState.showSuccess; see next comment).
@@
- private func createDocument() async {
+ private func createDocument() async {
guard appState.sdk != nil,
let contract = selectedContract,
!selectedDocumentType.isEmpty else {
appState.showError(message: "Please select a contract and document type")
return
}
- isLoading = true
+ isLoading = true
+ defer { isLoading = false }
- // In a real app, we would use the SDK's document creation functionality
+ // TODO: Integrate with Platform SDK (dash_sdk_*) to create & submit the document.
+ // Validate ownerId before proceeding.
+ guard let ownerData = Data(hexString: selectedOwnerId), ownerData.count == 32 else {
+ appState.showError(message: "Invalid owner ID. Expected 32-byte hex.")
+ return
+ }
+
let document = DocumentModel(
id: UUID().uuidString,
contractId: contract.id,
documentType: selectedDocumentType,
- ownerId: Data(hexString: selectedOwnerId) ?? Data(),
+ ownerId: ownerData,
data: documentData,
createdAt: Date(),
updatedAt: Date()
)
appState.documents.append(document)
- appState.showError(message: "Document created successfully")
-
- isLoading = false
+ appState.showError(message: "Document created successfully") // consider showSuccess (see next comment)
}Also applies to: 293-309
| guard appState.sdk != nil else { return } | ||
|
|
||
| // Fetch both regular and contested names in parallel | ||
| async let regularNamesTask = fetchRegularDPNSNames(identity: identity) | ||
| async let contestedNamesTask = fetchContestedDPNSNames(identity: identity) | ||
|
|
||
| let (regular, contested) = await (regularNamesTask, contestedNamesTask) | ||
| // Fetch regular and contested names sequentially to avoid sending non-Sendable results across tasks | ||
| let regular = await fetchRegularDPNSNames(identity: identity) | ||
| let contested = await fetchContestedDPNSNames(identity: identity) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Sequential execution breaks concurrency benefits
The refactored code awaits each fetch operation sequentially, eliminating the performance benefits of concurrent execution that the original async let pattern provided. This could double the loading time when fetching DPNS names.
Consider using a task group to maintain concurrency while avoiding the Sendable issue:
- // Fetch regular and contested names sequentially to avoid sending non-Sendable results across tasks
- let regular = await fetchRegularDPNSNames(identity: identity)
- let contested = await fetchContestedDPNSNames(identity: identity)
+ // Fetch both types concurrently using TaskGroup
+ let results = await withTaskGroup(of: (Int, [String], [String: Any]).self) { group in
+ group.addTask { @MainActor in
+ let result = await self.fetchRegularDPNSNames(identity: identity)
+ return (0, result.0, result.1)
+ }
+ group.addTask { @MainActor in
+ let result = await self.fetchContestedDPNSNames(identity: identity)
+ return (1, result.0, result.1)
+ }
+
+ var regular: ([String], [String: Any]) = ([], [:])
+ var contested: ([String], [String: Any]) = ([], [:])
+
+ for await (type, names, info) in group {
+ if type == 0 {
+ regular = (names, info)
+ } else {
+ contested = (names, info)
+ }
+ }
+
+ return (regular, contested)
+ }
+
+ let regular = results.0
+ let contested = results.1Committable suggestion skipped: line range outside the PR's diff.
|
|
||
| defer { | ||
| dash_sdk_signer_destroy(OpaquePointer(signer)!) | ||
| dash_sdk_signer_destroy(signer.assumingMemoryBound(to: SignerHandle.self)) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Inconsistent typed pointer usage pattern
The code uses signer.assumingMemoryBound(to: SignerHandle.self) in the destructor but OpaquePointer(signer) when passing to SDK functions. This inconsistency could lead to type confusion.
The pattern should be consistent throughout. Either use typed pointers everywhere or opaque pointers everywhere. Let me verify the expected types:
🏁 Script executed:
#!/bin/bash
# Check the expected parameter types for SDK functions that accept signers
rg -A3 -B3 "func.*signer.*OpaquePointer" packages/swift-sdk/Length of output: 3252
🏁 Script executed:
#!/bin/bash
# Check the dash_sdk_signer_destroy function signature
rg -A2 -B2 "dash_sdk_signer_destroy" packages/rs-sdk-ffi/src/signer.rsLength of output: 560
Use the signer pointer helper (signerMut) consistently when calling FFI
dash_sdk_signer_destroy expects a *mut SignerHandle; StateTransitionExtensions.swift already provides signerMut(_: OpaquePointer) -> UnsafeMutablePointer. Change the destructor to use that helper instead of assumingMemoryBound.
File: packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TransitionDetailView.swift (line 617)
Replace:
dash_sdk_signer_destroy(signer.assumingMemoryBound(to: SignerHandle.self))
With:
dash_sdk_signer_destroy(signerMut(signer))
🤖 Prompt for AI Agents
In
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TransitionDetailView.swift
around line 617, the destructor currently uses signer.assumingMemoryBound(to:
SignerHandle.self) but should call the provided signerMut(_: OpaquePointer)
helper which returns an UnsafeMutablePointer<SignerHandle>; replace the
dash_sdk_signer_destroy(signer.assumingMemoryBound(to: SignerHandle.self))
invocation with dash_sdk_signer_destroy(signerMut(signer)) so the FFI
consistently uses the helper.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/rs-platform-wallet/Cargo.toml (1)
14-16: Optional: Centralize rust-dashcore pins to reduce duplication.Consider moving these git+rev specs to workspace-level dependencies or a [patch] override to change the SHA once. This assumes a top-level setup; if adopted, this file can reference workspace entries.
Example change in this file (after defining them in [workspace.dependencies] at the workspace root):
-key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "befd0356bebfcd0d06d1028d8a03bfa4c78bd219" } -key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "befd0356bebfcd0d06d1028d8a03bfa4c78bd219", optional = true } +key-wallet = { workspace = true } +key-wallet-manager = { workspace = true, optional = true } -dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "befd0356bebfcd0d06d1028d8a03bfa4c78bd219" } +dashcore = { workspace = true }Also applies to: 18-18
packages/rs-dpp/Cargo.toml (1)
26-36: Optional: De-duplicate git+rev specs via workspace or [patch].Centralizing the rust-dashcore source pin simplifies future bumps across crates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
packages/rs-dpp/Cargo.toml(1 hunks)packages/rs-platform-wallet/Cargo.toml(1 hunks)packages/rs-sdk-ffi/Cargo.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-sdk-ffi/Cargo.toml
🧰 Additional context used
📓 Path-based instructions (1)
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all source packages under packages/* (JS/TS packages and Rust crates)
Files:
packages/rs-dpp/Cargo.tomlpackages/rs-platform-wallet/Cargo.toml
🧠 Learnings (4)
📓 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-03T16:37:11.605Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2756
File: packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:11-11
Timestamp: 2025-09-03T16:37:11.605Z
Learning: In packages/rs-dpp/Cargo.toml, the abci feature already includes core_rpc_client, and core_rpc_client is defined as ["dep:dashcore-rpc"]. This means rs-drive-abci can access dashcore-rpc types through dpp when using the abci feature.
Applied to files:
packages/rs-dpp/Cargo.tomlpackages/rs-platform-wallet/Cargo.toml
📚 Learning: 2025-03-11T09:39:23.071Z
Learnt from: shumkov
PR: dashpay/platform#2489
File: packages/rs-dpp/Cargo.toml:32-32
Timestamp: 2025-03-11T09:39:23.071Z
Learning: In the Dash Platform project, dependencies are currently managed using Git repository references with tags (repo+tag format in Cargo.toml) rather than published crates, as the team is not currently publishing crates to crates.io.
Applied to files:
packages/rs-dpp/Cargo.tomlpackages/rs-platform-wallet/Cargo.toml
📚 Learning: 2025-01-19T07:36:46.042Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2431
File: packages/rs-drive/Cargo.toml:55-60
Timestamp: 2025-01-19T07:36:46.042Z
Learning: The grovedb dependencies in packages/rs-drive/Cargo.toml and related files are intentionally kept at specific revisions rather than using the latest stable version, with plans to update them at a later time.
Applied to files:
packages/rs-platform-wallet/Cargo.toml
⏰ 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). (18)
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
🔇 Additional comments (4)
packages/rs-platform-wallet/Cargo.toml (2)
14-16: Revs aligned to a single rust-dashcore commit — good.Pinning key-wallet and key-wallet-manager to the same SHA improves reproducibility and cross-crate compatibility.
18-18: dashcore rev bump consistent with wallet deps — good.Keeps the workspace on a single rust-dashcore snapshot.
packages/rs-dpp/Cargo.toml (2)
26-36: Consistent pin across dashcore, key-wallet, manager, spv, and rpc — good.Feature flags for dashcore remain unchanged; optional flags retained.
26-36: Sanity-check dashcore feature names at pinned rev befd0356bebfcd0d06d1028d8a03bfa4c78bd219Unable to retrieve dash/Cargo.toml for that SHA from the verification environment. Confirm that the features referenced in packages/rs-dpp/Cargo.toml (std, secp-recovery, rand, signer, serde — and any others relied on such as bls, eddsa, quorum_validation, message_verification, bincode) are defined in dash/Cargo.toml at that commit to avoid build breaks.
| key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "befd0356bebfcd0d06d1028d8a03bfa4c78bd219" } | ||
| key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "befd0356bebfcd0d06d1028d8a03bfa4c78bd219", optional = true } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the pinned SHA/crates exist.
Quick check to ensure the commit and crate paths exist upstream.
Also applies to: 18-18
🏁 Script executed:
#!/bin/bash
set -euo pipefail
SHA="befd0356bebfcd0d06d1028d8a03bfa4c78bd219"
REPO="https://github.com/dashpay/rust-dashcore"
echo "Checking commit exists:"
git ls-remote "$REPO" "$SHA" | grep -q "$SHA" && echo "OK: $SHA found"
echo "Checking crate Cargo.toml files exist at $SHA:"
base="https://raw.githubusercontent.com/dashpay/rust-dashcore/$SHA"
for path in dash/Cargo.toml key-wallet/Cargo.toml key-wallet-manager/Cargo.toml dashcore-rpc/Cargo.toml dash-spv/Cargo.toml; do
url="$base/$path"
code=$(curl -s -o /dev/null -w "%{http_code}" "$url")
printf "%-40s %s\n" "$path" "$code"
done
# Expect 200 for each listed fileLength of output: 331
Action required — upstream commit befd0356bebfcd0d06d1028d8a03bfa4c78bd219 is missing dashcore-rpc/Cargo.toml
Curl check: dash/, key-wallet/, key-wallet-manager/, dash-spv/ → 200; dashcore-rpc/Cargo.toml → 404. Update the pinned SHA or correct the dependency path in packages/rs-platform-wallet/Cargo.toml (lines 14–16).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
scripts/dash_core_version_switcher.py (2)
242-246: Tighten subprocess usage: avoid blind except, check git availability, add timeoutAddresses Ruff BLE001 and S607; also prevents hangs.
Apply this diff within the current hunk:
- # Prefer git repo root to avoid depending on CWD - try: - repo_root = subprocess.check_output(["git", "rev-parse", "--show-toplevel"], text=True).strip() - except Exception: - repo_root = os.getcwd() + # Prefer git repo root to avoid depending on CWD + git_exe = shutil.which("git") + if not git_exe: + repo_root = os.getcwd() + else: + try: + repo_root = subprocess.check_output( + [git_exe, "rev-parse", "--show-toplevel"], + text=True, + timeout=5, + ).strip() + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError): + repo_root = os.getcwd()Additionally add the missing import near the top:
import shutil
43-47: Windows-safe directory skippingThe current forward-slash checks won’t match on Windows paths. Make the skip test OS-agnostic.
Apply:
- # skip typical build dirs - skip = any(part in dirpath for part in ("/target/", "/.git/", "/node_modules/", "/.build/")) + # skip typical build dirs (OS-agnostic) + skip = any(seg in dirpath.split(os.sep) for seg in ("target", ".git", "node_modules", ".build"))packages/dash-platform-balance-checker/src/main_trusted.rs (2)
49-56: Configure the context provider on the builder to avoid post-build mutationIf SdkBuilder exposes a with_context_provider/with_trusted_context_provider API, prefer injecting it during construction for immutability and to avoid any race where the SDK is used before the provider is set.
Proposed change:
-// Build SDK with mock core (context provider will handle everything) -let sdk = SdkBuilder::new(AddressList::from_iter(addresses)) - .with_core("127.0.0.1", 1, "mock", "mock") // Mock values, won't be used - .build()?; - -// Set our trusted context provider -sdk.set_context_provider(context_provider); +// Build SDK and inject the trusted context provider during construction +let sdk = SdkBuilder::new(AddressList::from_iter(addresses)) + .with_core("127.0.0.1", 1, "mock", "mock") // Mock values, won't be used + .with_context_provider(context_provider) // or .with_trusted_context_provider(...) + .build()?;If the builder lacks this API in the current crate version, keep the existing code.
30-37: Make DAPI addresses configurable (env/CLI) instead of hardcoded IPsHardcoding IPs is brittle. Allow overrides via an env var while keeping current defaults as fallback.
Proposed change:
-// Testnet addresses -let addresses = vec![ - Address::from_str("https://52.13.132.146:1443")?, - Address::from_str("https://52.89.154.48:1443")?, - Address::from_str("https://44.227.137.77:1443")?, - Address::from_str("https://52.40.219.41:1443")?, - Address::from_str("https://54.149.33.167:1443")?, -]; +// Testnet addresses (configurable via DAPI_ADDRESSES="https://host1:1443,https://host2:1443") +let addresses: Vec<Address> = if let Ok(list) = std::env::var("DAPI_ADDRESSES") { + list.split(',') + .map(|s| Address::from_str(s.trim())) + .collect::<Result<_, _>>()? +} else { + vec![ + Address::from_str("https://52.13.132.146:1443")?, + Address::from_str("https://52.89.154.48:1443")?, + Address::from_str("https://44.227.137.77:1443")?, + Address::from_str("https://52.40.219.41:1443")?, + Address::from_str("https://54.149.33.167:1443")?, + ] +};Note: If the Address::from_str error type doesn’t convert to anyhow::Error, wrap with map_err(anyhow::Error::new).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/dash-platform-balance-checker/src/main_trusted.rs(1 hunks)packages/rs-sdk-ffi/Cargo.toml(2 hunks)packages/rs-sdk-ffi/src/spv_bridge.rs(1 hunks)scripts/dash_core_version_switcher.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/rs-sdk-ffi/Cargo.toml
- packages/rs-sdk-ffi/src/spv_bridge.rs
🧰 Additional context used
📓 Path-based instructions (4)
scripts/**
📄 CodeRabbit inference engine (AGENTS.md)
Store repository scripts in scripts/
Files:
scripts/dash_core_version_switcher.py
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
**/*.rs: Use 4-space indentation for Rust files
Format Rust code with rustfmt defaults
Keep Rust code clippy-clean (no warnings)
In Rust, modules use snake_case, types use PascalCase, constants use SCREAMING_SNAKE_CASE
Files:
packages/dash-platform-balance-checker/src/main_trusted.rs
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all source packages under packages/* (JS/TS packages and Rust crates)
Files:
packages/dash-platform-balance-checker/src/main_trusted.rs
**/*.{js,jsx,ts,tsx,rs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx,rs}: Use LF line endings in source files
Ensure a final newline at end of source files
Files:
packages/dash-platform-balance-checker/src/main_trusted.rs
🧠 Learnings (4)
📓 Common learnings
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
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:
packages/dash-platform-balance-checker/src/main_trusted.rs
📚 Learning: 2025-07-23T08:31:42.268Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/wasm-sdk/CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:42.268Z
Learning: Applies to packages/wasm-sdk/src/**/*.rs : Token functions are methods on WasmSdk, not standalone functions; avoid importing them as standalone.
Applied to files:
packages/dash-platform-balance-checker/src/main_trusted.rs
📚 Learning: 2024-10-30T11:19:59.163Z
Learnt from: lklimek
PR: dashpay/platform#2277
File: packages/rs-sdk/tests/fetch/config.rs:233-233
Timestamp: 2024-10-30T11:19:59.163Z
Learning: In the Rust SDK's `rs-sdk/tests` integration tests (e.g., in `packages/rs-sdk/tests/fetch/config.rs`), we cannot create objects during tests because there is no support for object creation in this context. Therefore, hardcoded values for test identities must be used.
Applied to files:
packages/dash-platform-balance-checker/src/main_trusted.rs
🪛 Ruff (0.12.2)
scripts/dash_core_version_switcher.py
244-244: Starting a process with a partial executable path
(S607)
245-245: Do not catch blind exception: Exception
(BLE001)
⏰ 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). (19)
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (rs-sdk-ffi) / Formatting
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (rs-sdk-ffi) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- 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 JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
🔇 Additional comments (2)
scripts/dash_core_version_switcher.py (1)
242-246: Nice: resolve workspace root via Git instead of CWDThis reduces accidental dependence on invocation location (better for CI and subshells).
packages/dash-platform-balance-checker/src/main_trusted.rs (1)
6-6: Import scope tightened to only SdkBuilder — LGTMNo direct use of the Sdk type in this file; trimming the import is correct.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/swift-sdk-build.yml (1)
165-166: Quote SWIFT_TREAT_WARNINGS_AS_ERRORS to avoid word splitting (SC2086).You already quoted OTHER_SWIFT_FLAGS; do the same for SWIFT_TREAT_WARNINGS_AS_ERRORS.
- SWIFT_TREAT_WARNINGS_AS_ERRORS=$SWIFT_TREAT_WARNINGS_AS_ERRORS \ + SWIFT_TREAT_WARNINGS_AS_ERRORS="${SWIFT_TREAT_WARNINGS_AS_ERRORS}" \
🧹 Nitpick comments (2)
.github/workflows/swift-sdk-build.yml (2)
50-53: Avoid masking Homebrew errors when installing cbindgen.Using
|| truehides real failures. Prefer a conditional install.- brew install cbindgen || true + brew list cbindgen >/dev/null 2>&1 || brew install cbindgen
34-49: Optional: Add sccache to speed up Rust builds.This job compiles multiple crates; sccache typically cuts minutes off CI. Remember to quote CC/CXX per prior guidance.
- name: Set up Rust toolchain (stable) uses: dtolnay/rust-toolchain@stable + + - name: Enable sccache for Rust/C/C++ + uses: ./.github/actions/sccache
📜 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 (3)
📓 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:20:55.344Z
Learnt from: CR
PR: dashpay/platform#0
File: AGENTS.md:0-0
Timestamp: 2025-09-07T22:20:55.344Z
Learning: Applies to packages/swift-sdk/** : Swift example app and SDK live in packages/swift-sdk
Applied to files:
.github/workflows/swift-sdk-build.yml
📚 Learning: 2024-11-26T12:46:54.812Z
Learnt from: lklimek
PR: dashpay/platform#2344
File: .github/actions/sccache/action.yaml:0-0
Timestamp: 2024-11-26T12:46:54.812Z
Learning: In the `.github/actions/sccache/action.yaml` file, the environment variables `CC` and `CXX` need quotes around their values (e.g., `CC="sccache cc"`) to be interpreted correctly.
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). (20)
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (rs-sdk-ffi) / Unused dependencies
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Swift SDK and Example build (warnings as errors)
| on: | ||
| pull_request: | ||
| paths: | ||
| - 'packages/swift-sdk/**' | ||
| - '.github/workflows/swift-sdk-build.yml' | ||
| push: | ||
| branches: [ main, master ] | ||
| paths: | ||
| - 'packages/swift-sdk/**' | ||
| - '.github/workflows/swift-sdk-build.yml' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Run this workflow when Rust FFI sources change.
The build_ios.sh compiles crates under packages/rs-*. Currently, PRs that only touch those paths won’t trigger this workflow.
on:
pull_request:
paths:
- - 'packages/swift-sdk/**'
+ - 'packages/swift-sdk/**'
+ - 'packages/rs-*/**'
+ - 'packages/**/Cargo.toml'
+ - 'packages/**/Cargo.lock'
- '.github/workflows/swift-sdk-build.yml'
push:
branches: [ main, master ]
paths:
- - 'packages/swift-sdk/**'
+ - 'packages/swift-sdk/**'
+ - 'packages/rs-*/**'
+ - 'packages/**/Cargo.toml'
+ - 'packages/**/Cargo.lock'
- '.github/workflows/swift-sdk-build.yml'🤖 Prompt for AI Agents
In .github/workflows/swift-sdk-build.yml around lines 3 to 12, the workflow
currently watches only packages/swift-sdk/** and the workflow file itself, but
it should also trigger when Rust FFI crates under packages/rs-* change; update
both the pull_request and push path filters to include the Rust patterns (for
example add packages/rs-*/** or packages/rs-**/**) so changes to those crates
will run the workflow.
| jobs: | ||
| swift-sdk-build: | ||
| name: Swift SDK and Example build (warnings as errors) | ||
| runs-on: macos-15 | ||
| timeout-minutes: 60 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Grant explicit permissions for PR commenting step.
Without write permissions, the github-script step updating PR comments can fail and red-mark the build.
jobs:
swift-sdk-build:
+ permissions:
+ contents: read
+ pull-requests: write
+ issues: write
name: Swift SDK and Example build (warnings as errors)
runs-on: macos-15📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jobs: | |
| swift-sdk-build: | |
| name: Swift SDK and Example build (warnings as errors) | |
| runs-on: macos-15 | |
| timeout-minutes: 60 | |
| jobs: | |
| swift-sdk-build: | |
| permissions: | |
| contents: read | |
| pull-requests: write | |
| issues: write | |
| name: Swift SDK and Example build (warnings as errors) | |
| runs-on: macos-15 | |
| timeout-minutes: 60 |
🤖 Prompt for AI Agents
In .github/workflows/swift-sdk-build.yml around lines 14-19, the job lacks
explicit permissions so the github-script step that updates PR comments can
fail; add a permissions block to the swift-sdk-build job (e.g., permissions:
pull-requests: write) so the github-script has write access to update PR
comments, and ensure the job uses the default GITHUB_TOKEN by not overriding it.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
.github/workflows/swift-sdk-build.yml (3)
5-12: Trigger on Rust FFI changes and Cargo files; also include v2.1-dev pushes.Ensure workflow runs when rs-* crates or Cargo files change, and on pushes to v2.1-dev.
on: pull_request: paths: - - 'packages/swift-sdk/**' - - '.github/workflows/swift-sdk-build.yml' + - 'packages/swift-sdk/**' + - 'packages/rs-*/**' + - 'packages/**/Cargo.toml' + - 'packages/**/Cargo.lock' + - '.github/workflows/swift-sdk-build.yml' push: - branches: [ main, master ] + branches: [ main, master, v2.1-dev ] paths: - - 'packages/swift-sdk/**' - - '.github/workflows/swift-sdk-build.yml' + - 'packages/swift-sdk/**' + - 'packages/rs-*/**' + - 'packages/**/Cargo.toml' + - 'packages/**/Cargo.lock' + - '.github/workflows/swift-sdk-build.yml'
195-207: Quote SWIFT_TREAT_WARNINGS_AS_ERRORS to prevent word splitting.OTHER_SWIFT_FLAGS is quoted; do the same for SWIFT_TREAT_WARNINGS_AS_ERRORS.
- SWIFT_TREAT_WARNINGS_AS_ERRORS=$SWIFT_TREAT_WARNINGS_AS_ERRORS \ + SWIFT_TREAT_WARNINGS_AS_ERRORS="${SWIFT_TREAT_WARNINGS_AS_ERRORS}" \
15-19: Grant explicit permissions for PR commenting, and consider canceling superseded runs.Without write perms, github-script may fail to update PR comments. Optional: add concurrency to save macOS minutes.
jobs: swift-sdk-build: + permissions: + contents: read + pull-requests: write + issues: write name: Swift SDK and Example build (warnings as errors) runs-on: macos-15 timeout-minutes: 60 + concurrency: + group: swift-sdk-build-${{ github.ref }} + cancel-in-progress: true
🧹 Nitpick comments (3)
.github/workflows/swift-sdk-build.yml (3)
47-49: Add Intel simulator target for broader sim compatibility.Some CI/dev boxes still rely on x86_64 iOS Simulator slices.
- rustup target add aarch64-apple-ios aarch64-apple-ios-sim + rustup target add aarch64-apple-ios aarch64-apple-ios-sim x86_64-apple-ios
55-61: Fix cache paths: avoid ${{ env.HOME }} which may not resolve in cache inputs.Use tilde expansion for HOME to make cache reliable.
- path: | - ${{ env.HOME }}/.local/protoc-32.0/bin - ${{ env.HOME }}/.local/protoc-32.0/include + path: | + ~/.local/protoc-32.0/bin + ~/.local/protoc-32.0/include ... - path: | - ${{ env.HOME }}/.local/protoc-32.0/bin - ${{ env.HOME }}/.local/protoc-32.0/include + path: | + ~/.local/protoc-32.0/bin + ~/.local/protoc-32.0/includeIf you prefer env vars, set a job-level
PROTOC_HOMEand reuse it in both cache and run steps.Also applies to: 83-85
63-77: Minor: remove Authorization header for public protoc asset to simplify fork PRs.Download doesn’t require auth; avoids relying on token scopes on forks.
- curl -fsSL -H "Authorization: token ${GITHUB_TOKEN}" \ + curl -fsSL \ -o /tmp/protoc.zip \ "https://github.com/protocolbuffers/protobuf/releases/download/v${VERSION}/protoc-${VERSION}-${OS}.zip"
📜 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 (5)
📓 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:20:55.344Z
Learnt from: CR
PR: dashpay/platform#0
File: AGENTS.md:0-0
Timestamp: 2025-09-07T22:20:55.344Z
Learning: Applies to packages/swift-sdk/** : Swift example app and SDK live in packages/swift-sdk
Applied to files:
.github/workflows/swift-sdk-build.yml
📚 Learning: 2024-11-26T12:46:54.812Z
Learnt from: lklimek
PR: dashpay/platform#2344
File: .github/actions/sccache/action.yaml:0-0
Timestamp: 2024-11-26T12:46:54.812Z
Learning: In the `.github/actions/sccache/action.yaml` file, the environment variables `CC` and `CXX` need quotes around their values (e.g., `CC="sccache cc"`) to be interpreted correctly.
Applied to files:
.github/workflows/swift-sdk-build.yml
📚 Learning: 2025-09-07T22:20:55.344Z
Learnt from: CR
PR: dashpay/platform#0
File: AGENTS.md:0-0
Timestamp: 2025-09-07T22:20:55.344Z
Learning: Applies to packages/rs-sdk-ffi/** : iOS/FFI Rust artifacts live in packages/rs-sdk-ffi
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 Platform SDK functions with the dash_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). (19)
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Swift SDK and Example build (warnings as errors)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/rs-sdk-ffi/build_ios.sh (1)
176-196: Replace zero-sized opaque structs with forward declarations (standards-compliant).Zero-sized arrays are UB in C; forward declarations meet Swift’s needs for pointer-only opaque types and align with our learnings to “use pointers for opaque FFI types.”
Apply:
-// Forward declarations to ensure cross-refs compile regardless of merge order -typedef struct FFIClientConfig FFIClientConfig; -// Provide explicit opaque definitions so Swift can import the type names -typedef struct FFIDashSpvClient { unsigned char _private[0]; } FFIDashSpvClient; -typedef struct FFIWallet { unsigned char _private[0]; } FFIWallet; -typedef struct FFIAccount { unsigned char _private[0]; } FFIAccount; -typedef struct FFIAccountCollection { unsigned char _private[0]; } FFIAccountCollection; -typedef struct FFIBLSAccount { unsigned char _private[0]; } FFIBLSAccount; -typedef struct FFIEdDSAAccount { unsigned char _private[0]; } FFIEdDSAAccount; -typedef struct FFIAddressPool { unsigned char _private[0]; } FFIAddressPool; -typedef struct FFIManagedAccountCollection { unsigned char _private[0]; } FFIManagedAccountCollection; -typedef struct FFIWalletManager { unsigned char _private[0]; } FFIWalletManager; -typedef struct FFIManagedAccount { unsigned char _private[0]; } FFIManagedAccount; -// Platform SDK opaque handles -typedef struct SDKHandle { unsigned char _private[0]; } SDKHandle; -typedef struct DataContractHandle { unsigned char _private[0]; } DataContractHandle; -typedef struct DocumentHandle { unsigned char _private[0]; } DocumentHandle; -typedef struct IdentityHandle { unsigned char _private[0]; } IdentityHandle; -typedef struct IdentityPublicKeyHandle { unsigned char _private[0]; } IdentityPublicKeyHandle; -typedef struct SignerHandle { unsigned char _private[0]; } SignerHandle; +// Forward declarations (opaque; used only via pointers in C/Swift) +typedef struct FFIClientConfig FFIClientConfig; +typedef struct FFIDashSpvClient FFIDashSpvClient; +typedef struct FFIWallet FFIWallet; +typedef struct FFIAccount FFIAccount; +typedef struct FFIAccountCollection FFIAccountCollection; +typedef struct FFIBLSAccount FFIBLSAccount; +typedef struct FFIEdDSAAccount FFIEdDSAAccount; +typedef struct FFIAddressPool FFIAddressPool; +typedef struct FFIManagedAccountCollection FFIManagedAccountCollection; +typedef struct FFIWalletManager FFIWalletManager; +typedef struct FFIManagedAccount FFIManagedAccount; +// Platform SDK opaque handles +typedef struct SDKHandle SDKHandle; +typedef struct DataContractHandle DataContractHandle; +typedef struct DocumentHandle DocumentHandle; +typedef struct IdentityHandle IdentityHandle; +typedef struct IdentityPublicKeyHandle IdentityPublicKeyHandle; +typedef struct SignerHandle SignerHandle;
🧹 Nitpick comments (5)
.github/workflows/swift-sdk-release.yml (4)
60-67: Quote $GITHUB_OUTPUT redirections to satisfy shellcheck and avoid word-splitting.SC2086 warnings; also prefer printf to avoid newline pitfalls.
- name: Zip XCFramework and compute checksum id: zip run: | cd packages/swift-sdk ditto -c -k --sequesterRsrc --keepParent DashSDKFFI.xcframework DashSDKFFI.xcframework.zip swift package compute-checksum DashSDKFFI.xcframework.zip > xc_checksum.txt - echo "checksum=$(cat xc_checksum.txt)" >> $GITHUB_OUTPUT + printf 'checksum=%s\n' "$(<xc_checksum.txt)" >> "$GITHUB_OUTPUT" @@ - name: Determine tag id: tag run: | if [ -n "${{ github.event.inputs.tag }}" ]; then - echo "name=${{ github.event.inputs.tag }}" >> $GITHUB_OUTPUT + printf 'name=%s\n' "${{ github.event.inputs.tag }}" >> "$GITHUB_OUTPUT" else # on tag push, ref_name is the tag - echo "name=${{ github.ref_name }}" >> $GITHUB_OUTPUT + printf 'name=%s\n' "${{ github.ref_name }}" >> "$GITHUB_OUTPUT" fiAlso applies to: 70-76
15-16: Add a concurrency group to avoid racing releases for the same ref.Prevents duplicate uploads if multiple runs trigger close together.
build-and-release: name: Build and release DashSDKFFI runs-on: macos-15 + concurrency: + group: swift-sdk-release-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true
25-26: Pin Xcode version precisely for reproducibility.Using
16.*can drift; consider pinning the exact minor.- xcode-version: '16.*' + xcode-version: '16.0'
60-67: Optional: pass values via env to simplify the script and avoid template nesting.Keeps the JS cleaner and avoids mixing shell and expression logic.
- name: Zip XCFramework and compute checksum id: zip run: | cd packages/swift-sdk ditto -c -k --sequesterRsrc --keepParent DashSDKFFI.xcframework DashSDKFFI.xcframework.zip swift package compute-checksum DashSDKFFI.xcframework.zip > xc_checksum.txt printf 'checksum=%s\n' "$(<xc_checksum.txt)" >> "$GITHUB_OUTPUT" + env: + CHECKSUM_FILE: packages/swift-sdk/xc_checksum.txt @@ - name: Check existing release for changes id: check uses: actions/github-script@v7 - with: - script: | - const tag = core.getInput('name', { required: false }) || process.env.INPUT_NAME || '${{ steps.tag.outputs.name }}'; - const checksumNew = '${{ steps.zip.outputs.checksum }}'.trim(); + env: + TAG_NAME: ${{ steps.tag.outputs.name }} + CHECKSUM_NEW: ${{ steps.zip.outputs.checksum }} + with: + script: | + const tag = process.env.TAG_NAME; + const checksumNew = (process.env.CHECKSUM_NEW || '').trim(); try { const rel = await github.rest.repos.getReleaseByTag({ owner: context.repo.owner, repo: context.repo.repo, tag }); const assets = await github.rest.repos.listReleaseAssets({ owner: context.repo.owner, repo: context.repo.repo, release_id: rel.data.id, per_page: 100 }); const checksumAsset = assets.data.find(a => a.name === 'xc_checksum.txt'); if (!checksumAsset) { core.setOutput('changed', 'true'); return; } const res = await github.request('GET {url}', { url: checksumAsset.url, headers: { Accept: 'application/octet-stream' } }); const checksumOld = (res.data || '').toString().trim(); core.info(`Old checksum: ${checksumOld}, New checksum: ${checksumNew}`); core.setOutput('changed', checksumOld === checksumNew ? 'false' : 'true'); } catch (e) { // No release found -> treat as changed core.info(`No existing release for tag ${tag}. Creating new.`); core.setOutput('changed', 'true'); } result-encoding: stringAlso applies to: 78-105
packages/rs-sdk-ffi/build_ios.sh (1)
320-339: Avoid re-defining RUST_DASHCORE_PATH; reuse earlier var.Minor cleanup; prevents drift if one changes.
Apply:
-# Build dash-spv-ffi from local rust-dashcore for device and simulator -RUST_DASHCORE_PATH="$PROJECT_ROOT/../rust-dashcore" SPV_CRATE_PATH="$RUST_DASHCORE_PATH/dash-spv-ffi"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/swift-sdk-release.yml(1 hunks)packages/rs-sdk-ffi/build_ios.sh(7 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all source packages under packages/* (JS/TS packages and Rust crates)
Files:
packages/rs-sdk-ffi/build_ios.sh
packages/rs-sdk-ffi/**
📄 CodeRabbit inference engine (AGENTS.md)
iOS/FFI Rust artifacts live in packages/rs-sdk-ffi
Files:
packages/rs-sdk-ffi/build_ios.sh
🧠 Learnings (4)
📓 Common learnings
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
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:20:55.344Z
Learnt from: CR
PR: dashpay/platform#0
File: AGENTS.md:0-0
Timestamp: 2025-09-07T22:20:55.344Z
Learning: Applies to packages/rs-sdk-ffi/** : iOS/FFI Rust artifacts live in packages/rs-sdk-ffi
Applied to files:
packages/rs-sdk-ffi/build_ios.sh
📚 Learning: 2025-09-07T22:18:50.883Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/rs-sdk-ffi/**/*.{h,rs} : Follow unified SDK function prefixes: dash_core_sdk_* for Core, dash_sdk_* for Platform, dash_unified_sdk_* for unified APIs
Applied to files:
packages/rs-sdk-ffi/build_ios.sh
📚 Learning: 2025-09-07T22:18:50.883Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/rs-sdk-ffi/**/*.h : In C headers, use pointers for opaque FFI types
Applied to files:
packages/rs-sdk-ffi/build_ios.sh
🪛 actionlint (1.7.7)
.github/workflows/swift-sdk-release.yml
62-62: shellcheck reported issue in this script: SC2086:info:4:43: Double quote to prevent globbing and word splitting
(shellcheck)
70-70: shellcheck reported issue in this script: SC2086:info:2:49: Double quote to prevent globbing and word splitting
(shellcheck)
70-70: shellcheck reported issue in this script: SC2086:info:5:41: Double quote to prevent globbing and word splitting
(shellcheck)
103-103: input "with" is not defined in action "actions/github-script@v7". available inputs are "base-url", "debug", "github-token", "previews", "result-encoding", "retries", "retry-exempt-status-codes", "script", "user-agent"
(action)
104-104: expected scalar node for string value but found mapping node with "!!map" tag
(syntax-check)
⏰ 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). (20)
- GitHub Check: Rust packages (dash-sdk) / Detect immutable structure changes
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- 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: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Swift SDK and Example build (warnings as errors)
- GitHub Check: build-wasm-sdk
🔇 Additional comments (7)
.github/workflows/swift-sdk-release.yml (1)
1-133: Fix validation script flags — re-run linteractionlint rejects '-color=never' and '-shellcheck=enable'; use -no-color and -shellcheck=shellcheck. Run the corrected script below to re-verify.
#!/bin/bash set -euo pipefail # Install actionlint (v1.7.7) ACTIONLINT_VERSION=1.7.7 curl -sSL "https://github.com/rhysd/actionlint/releases/download/v${ACTIONLINT_VERSION}/actionlint_${ACTIONLINT_VERSION}_linux_amd64.tar.gz" -o /tmp/actionlint.tar.gz tar -xzf /tmp/actionlint.tar.gz -C /tmp chmod +x /tmp/actionlint # Lint this workflow (enable shellcheck integration) /tmp/actionlint -no-color -shellcheck=shellcheck .github/workflows/swift-sdk-release.yml || truepackages/rs-sdk-ffi/build_ios.sh (6)
232-233: Header extraction awk clause looks good.
255-258: Good: filtering out C++ namespace wrappers from SPV header.
360-367: Merging device libs is fine.Static combine via libtool for device path looks correct.
418-423: Device library selection logic is sound.Prefers combined lib when present, falls back otherwise.
426-443: packages/rs-sdk-ffi/build_ios.sh (426–443): merge both simulator arches when SPV is present — local verification requiredSandbox couldn't inspect build artifacts (no build/ directory). Run this from the repo root and paste the output:
# Inspect XCFramework .a slices find . -type d -name 'DashSDKFFI.xcframework' -print0 | while IFS= read -r -d '' fw; do echo "Inspecting: $fw" find "$fw" -name '*.a' -print0 | while IFS= read -r -d '' lib; do echo "-> $lib" lipo -info "$lib" || true done done # Inspect thin simulator libs used by the script for p in \ target/aarch64-apple-ios-sim/release/librs_sdk_ffi.a \ target/x86_64-apple-ios/release/librs_sdk_ffi.a \ "$SPV_CRATE_PATH/target/aarch64-apple-ios-sim/release/libdash_spv_ffi.a" \ "$SPV_CRATE_PATH/target/x86_64-apple-ios/release/libdash_spv_ffi.a"; do [ -f "$p" ] && echo "FOUND: $p" && lipo -info "$p" doneIf any simulator .a contains only one slice when SPV libs exist, apply the suggested change in the original comment to merge per-arch simulator libs (arm64 + x86_64).
37-46: Fix simulator min-version flag; use per-target RUSTFLAGS (avoid leaking device flag into sim).packages/rs-sdk-ffi/build_ios.sh (around lines 38–46) currently exports RUSTFLAGS with -C link-arg=-mios-version-min which will leak the device min-version into simulator builds — switch to per-target CARGO_TARGET_*_RUSTFLAGS and allow overriding deployment targets. Apply the diff below and run the post-build LC_BUILD_VERSION check (otool was not available in the verification environment).
-# Set iOS deployment targets to match Xcode 16 SDKs and avoid 10.0 default -export IPHONEOS_DEPLOYMENT_TARGET="18.0" -export IPHONESIMULATOR_DEPLOYMENT_TARGET="18.0" -# Ensure linker uses the same min version when rustc links -export RUSTFLAGS="${RUSTFLAGS:-} -C link-arg=-mios-version-min=${IPHONEOS_DEPLOYMENT_TARGET}" +# Set iOS deployment targets (allow override via env) +export IPHONEOS_DEPLOYMENT_TARGET="${IPHONEOS_DEPLOYMENT_TARGET:-18.0}" +export IPHONESIMULATOR_DEPLOYMENT_TARGET="${IPHONESIMULATOR_DEPLOYMENT_TARGET:-18.0}" +# Ensure rustc uses per-target min versions (device vs simulator) +unset RUSTFLAGS +export CARGO_TARGET_AARCH64_APPLE_IOS_RUSTFLAGS="${CARGO_TARGET_AARCH64_APPLE_IOS_RUSTFLAGS:-} -C link-arg=-mios-version-min=${IPHONEOS_DEPLOYMENT_TARGET}" +export CARGO_TARGET_AARCH64_APPLE_IOS_SIM_RUSTFLAGS="${CARGO_TARGET_AARCH64_APPLE_IOS_SIM_RUSTFLAGS:-} -C link-arg=-mios-simulator-version-min=${IPHONESIMULATOR_DEPLOYMENT_TARGET}" +export CARGO_TARGET_X86_64_APPLE_IOS_RUSTFLAGS="${CARGO_TARGET_X86_64_APPLE_IOS_RUSTFLAGS:-} -C link-arg=-mios-simulator-version-min=${IPHONESIMULATOR_DEPLOYMENT_TARGET}" -echo "Using IPHONEOS_DEPLOYMENT_TARGET=${IPHONEOS_DEPLOYMENT_TARGET}" -echo "Using IPHONESIMULATOR_DEPLOYMENT_TARGET=${IPHONESIMULATOR_DEPLOYMENT_TARGET}" -echo "RUSTFLAGS=${RUSTFLAGS}" +echo "Using IPHONEOS_DEPLOYMENT_TARGET=${IPHONEOS_DEPLOYMENT_TARGET}" +echo "Using IPHONESIMULATOR_DEPLOYMENT_TARGET=${IPHONESIMULATOR_DEPLOYMENT_TARGET}" +echo "CARGO_TARGET_AARCH64_APPLE_IOS_RUSTFLAGS=${CARGO_TARGET_AARCH64_APPLE_IOS_RUSTFLAGS}" +echo "CARGO_TARGET_AARCH64_APPLE_IOS_SIM_RUSTFLAGS=${CARGO_TARGET_AARCH64_APPLE_IOS_SIM_RUSTFLAGS}" +echo "CARGO_TARGET_X86_64_APPLE_IOS_RUSTFLAGS=${CARGO_TARGET_X86_64_APPLE_IOS_RUSTFLAGS}"
| workflow_dispatch: | ||
| inputs: | ||
| tag: | ||
| description: 'Tag name to release (optional; defaults to current ref_name on tag push)' | ||
| required: false | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Require a tag for manual runs or guard against releasing under a branch name.
On workflow_dispatch without input, ${{ github.ref_name }} is a branch ref; you may unintentionally create a release named after a branch.
Option A (simplest) — make the input required:
- required: false
+ required: trueOption B — keep optional, but bail out if empty on manual:
- name: Determine tag
id: tag
run: |
+ set -euo pipefail
if [ -n "${{ github.event.inputs.tag }}" ]; then
printf 'name=%s\n' "${{ github.event.inputs.tag }}" >> "$GITHUB_OUTPUT"
else
- # on tag push, ref_name is the tag
- printf 'name=%s\n' "${{ github.ref_name }}" >> "$GITHUB_OUTPUT"
+ if [ "${{ github.event_name }}" = "push" ]; then
+ # on tag push, ref_name is the tag
+ printf 'name=%s\n' "${{ github.ref_name }}" >> "$GITHUB_OUTPUT"
+ else
+ echo "Tag input is required for workflow_dispatch." >&2
+ exit 1
+ fi
fiAlso applies to: 68-76
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/tests-rs-sdk-ffi-build.yml (2)
95-97: Use simulator-specific min-version flag for simulator triplesFor simulator targets, prefer -mios-simulator-version-min; keep -mios-version-min for device. Also note x86_64-apple-ios is simulator-only, so it should use the simulator flag too (or drop it if you don’t build that target here).
- CARGO_TARGET_AARCH64_APPLE_IOS_RUSTFLAGS: "-C link-arg=-mios-version-min=18.0" - CARGO_TARGET_AARCH64_APPLE_IOS_SIM_RUSTFLAGS: "-C link-arg=-mios-version-min=18.0" - CARGO_TARGET_X86_64_APPLE_IOS_RUSTFLAGS: "-C link-arg=-mios-version-min=18.0" + CARGO_TARGET_AARCH64_APPLE_IOS_RUSTFLAGS: "-C link-arg=-mios-version-min=18.0" + CARGO_TARGET_AARCH64_APPLE_IOS_SIM_RUSTFLAGS: "-C link-arg=-mios-simulator-version-min=18.0" + CARGO_TARGET_X86_64_APPLE_IOS_RUSTFLAGS: "-C link-arg=-mios-simulator-version-min=18.0"Cargo honors per-target flags via CARGO_TARGET__RUSTFLAGS; using the simulator-specific flag helps clang tag objects for the simulator correctly. (doc.rust-lang.org)
100-103: Echo only the active target’s rustflags to reduce noisePrint the flags for the current matrix target instead of all three.
- echo "AARCH64 iOS rustflags: ${CARGO_TARGET_AARCH64_APPLE_IOS_RUSTFLAGS}" - echo "AARCH64 iOS-sim rustflags: ${CARGO_TARGET_AARCH64_APPLE_IOS_SIM_RUSTFLAGS}" - echo "x86_64 iOS rustflags: ${CARGO_TARGET_X86_64_APPLE_IOS_RUSTFLAGS}" + case "${{ matrix.target }}" in + aarch64-apple-ios) + echo "AARCH64 iOS rustflags: ${CARGO_TARGET_AARCH64_APPLE_IOS_RUSTFLAGS}" + ;; + aarch64-apple-ios-sim) + echo "AARCH64 iOS-sim rustflags: ${CARGO_TARGET_AARCH64_APPLE_IOS_SIM_RUSTFLAGS}" + ;; + x86_64-apple-ios) + echo "x86_64 iOS rustflags: ${CARGO_TARGET_X86_64_APPLE_IOS_RUSTFLAGS}" + ;; + esac
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/tests-rs-sdk-ffi-build.yml(1 hunks)packages/rs-sdk-ffi/Cargo.toml(1 hunks)packages/rs-sdk-ffi/build_ios.sh(7 hunks)packages/rs-sdk-ffi/src/lib.rs(1 hunks)packages/rs-sdk-ffi/src/spv_bridge.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/rs-sdk-ffi/src/spv_bridge.rs
- packages/rs-sdk-ffi/Cargo.toml
- packages/rs-sdk-ffi/src/lib.rs
- packages/rs-sdk-ffi/build_ios.sh
🧰 Additional context used
🧠 Learnings (2)
📓 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:20:55.344Z
Learnt from: CR
PR: dashpay/platform#0
File: AGENTS.md:0-0
Timestamp: 2025-09-07T22:20:55.344Z
Learning: Applies to packages/rs-sdk-ffi/** : iOS/FFI Rust artifacts live in packages/rs-sdk-ffi
Applied to files:
.github/workflows/tests-rs-sdk-ffi-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). (180)
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/rs-sdk-ffi/Cargo.toml (2)
20-21: Clarify comment vs. optional dep semanticsThe comment says “always included,” but the dep is optional and enabled via the default feature. Suggest rewording to “included by default via 'dash_spv' feature.”
-# Core SDK integration (always included for unified SDK) +# Core SDK integration (included by default via the 'dash_spv' feature)
12-13: Trim extra blank linesMinor formatting nit to keep the manifest tidy.
- -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/swift-sdk-build.yml(1 hunks).github/workflows/swift-sdk-release.yml(1 hunks)packages/rs-sdk-ffi/Cargo.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/swift-sdk-release.yml
- .github/workflows/swift-sdk-build.yml
🧰 Additional context used
📓 Path-based instructions (2)
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all source packages under packages/* (JS/TS packages and Rust crates)
Files:
packages/rs-sdk-ffi/Cargo.toml
packages/rs-sdk-ffi/**
📄 CodeRabbit inference engine (AGENTS.md)
iOS/FFI Rust artifacts live in packages/rs-sdk-ffi
Files:
packages/rs-sdk-ffi/Cargo.toml
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Always use the swift-rust-ffi-engineer agent for any Swift/Rust FFI integration, Swift wrappers over FFI, Swift/FFI type compatibility debugging, iOS SDK and SwiftExampleApp development, cross-boundary memory management, and refactoring Swift to wrap FFI functions
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:18:50.883Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/rs-sdk-ffi/**/*.{h,rs} : Follow unified SDK function prefixes: dash_core_sdk_* for Core, dash_sdk_* for Platform, dash_unified_sdk_* for unified APIs
Applied to files:
packages/rs-sdk-ffi/Cargo.toml
📚 Learning: 2025-09-03T16:37:11.605Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2756
File: packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:11-11
Timestamp: 2025-09-03T16:37:11.605Z
Learning: In packages/rs-dpp/Cargo.toml, the abci feature already includes core_rpc_client, and core_rpc_client is defined as ["dep:dashcore-rpc"]. This means rs-drive-abci can access dashcore-rpc types through dpp when using the abci feature.
Applied to files:
packages/rs-sdk-ffi/Cargo.toml
📚 Learning: 2025-03-11T09:39:23.071Z
Learnt from: shumkov
PR: dashpay/platform#2489
File: packages/rs-dpp/Cargo.toml:32-32
Timestamp: 2025-03-11T09:39:23.071Z
Learning: In the Dash Platform project, dependencies are currently managed using Git repository references with tags (repo+tag format in Cargo.toml) rather than published crates, as the team is not currently publishing crates to crates.io.
Applied to files:
packages/rs-sdk-ffi/Cargo.toml
📚 Learning: 2025-09-07T22:20:55.344Z
Learnt from: CR
PR: dashpay/platform#0
File: AGENTS.md:0-0
Timestamp: 2025-09-07T22:20:55.344Z
Learning: Applies to packages/rs-sdk-ffi/** : iOS/FFI Rust artifacts live in packages/rs-sdk-ffi
Applied to files:
packages/rs-sdk-ffi/Cargo.toml
📚 Learning: 2025-01-19T07:36:46.042Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2431
File: packages/rs-drive/Cargo.toml:55-60
Timestamp: 2025-01-19T07:36:46.042Z
Learning: The grovedb dependencies in packages/rs-drive/Cargo.toml and related files are intentionally kept at specific revisions rather than using the latest stable version, with plans to update them at a later time.
Applied to files:
packages/rs-sdk-ffi/Cargo.toml
⏰ 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). (177)
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Formatting
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Formatting
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Formatting
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
🔇 Additional comments (2)
packages/rs-sdk-ffi/Cargo.toml (2)
21-21: Rev pin on dash-spv-ffi looks goodPinning to befd0356… aligns with the repo’s git+rev strategy and default feature gating via dep: works as intended.
21-21: Verified — single-source-of-truth rev is consistent across the workspaceAll Cargo.toml references to github.com/dashpay/rust-dashcore use rev befd0356bebfcd0d06d1028d8a03bfa4c78bd219; centralizing this in the workspace is optional.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
packages/rs-sdk-ffi/build_ios.sh (1)
180-200: Zero-sized opaque structs are UB in C; switch to forward declarations.The ZSA pattern can break some compilers and Swift import isn’t required to have sized types. Use plain forward-declared opaque structs.
-// Forward declarations to ensure cross-refs compile regardless of merge order -typedef struct FFIClientConfig FFIClientConfig; -// Provide explicit opaque definitions so Swift can import the type names -typedef struct FFIDashSpvClient { unsigned char _private[0]; } FFIDashSpvClient; -typedef struct FFIWallet { unsigned char _private[0]; } FFIWallet; -typedef struct FFIAccount { unsigned char _private[0]; } FFIAccount; -typedef struct FFIAccountCollection { unsigned char _private[0]; } FFIAccountCollection; -typedef struct FFIBLSAccount { unsigned char _private[0]; } FFIBLSAccount; -typedef struct FFIEdDSAAccount { unsigned char _private[0]; } FFIEdDSAAccount; -typedef struct FFIAddressPool { unsigned char _private[0]; } FFIAddressPool; -typedef struct FFIManagedAccountCollection { unsigned char _private[0]; } FFIManagedAccountCollection; -typedef struct FFIWalletManager { unsigned char _private[0]; } FFIWalletManager; -typedef struct FFIManagedAccount { unsigned char _private[0]; } FFIManagedAccount; -// Platform SDK opaque handles -typedef struct SDKHandle { unsigned char _private[0]; } SDKHandle; -typedef struct DataContractHandle { unsigned char _private[0]; } DataContractHandle; -typedef struct DocumentHandle { unsigned char _private[0]; } DocumentHandle; -typedef struct IdentityHandle { unsigned char _private[0]; } IdentityHandle; -typedef struct IdentityPublicKeyHandle { unsigned char _private[0]; } IdentityPublicKeyHandle; -typedef struct SignerHandle { unsigned char _private[0]; } SignerHandle; +// Forward declarations for opaque types (use pointers in APIs) +typedef struct FFIClientConfig FFIClientConfig; +typedef struct FFIDashSpvClient FFIDashSpvClient; +typedef struct FFIWallet FFIWallet; +typedef struct FFIAccount FFIAccount; +typedef struct FFIAccountCollection FFIAccountCollection; +typedef struct FFIBLSAccount FFIBLSAccount; +typedef struct FFIEdDSAAccount FFIEdDSAAccount; +typedef struct FFIAddressPool FFIAddressPool; +typedef struct FFIManagedAccountCollection FFIManagedAccountCollection; +typedef struct FFIWalletManager FFIWalletManager; +typedef struct FFIManagedAccount FFIManagedAccount; +typedef struct SDKHandle SDKHandle; +typedef struct DataContractHandle DataContractHandle; +typedef struct DocumentHandle DocumentHandle; +typedef struct IdentityHandle IdentityHandle; +typedef struct IdentityPublicKeyHandle IdentityPublicKeyHandle; +typedef struct SignerHandle SignerHandle;.github/workflows/swift-sdk-build.yml (3)
3-14: Trigger on Rust manifest changes too (ensures FFI-only PRs run).Include Cargo.toml/lock to catch version/rev changes without source diffs.
on: pull_request: paths: - 'packages/swift-sdk/**' - 'packages/rs-*/**' + - 'packages/**/Cargo.toml' + - 'packages/**/Cargo.lock' - '.github/workflows/swift-sdk-build.yml' push: branches: [ main, master ] paths: - 'packages/swift-sdk/**' - 'packages/rs-*/**' + - 'packages/**/Cargo.toml' + - 'packages/**/Cargo.lock' - '.github/workflows/swift-sdk-build.yml'
247-263: Quote env expansion for SWIFT_TREAT_WARNINGS_AS_ERRORS.Prevents word splitting; OTHER_SWIFT_FLAGS is already quoted.
ONLY_ACTIVE_ARCH=YES \ OTHER_SWIFT_FLAGS="$OTHER_SWIFT_FLAGS" \ - SWIFT_TREAT_WARNINGS_AS_ERRORS=$SWIFT_TREAT_WARNINGS_AS_ERRORS \ + SWIFT_TREAT_WARNINGS_AS_ERRORS="$SWIFT_TREAT_WARNINGS_AS_ERRORS" \ build
16-21: Add explicit job permissions so PR comment step can write.Without these, actions/github-script may fail to update/create PR comments.
jobs: swift-sdk-build: + permissions: + contents: read + pull-requests: write + issues: write name: Swift SDK and Example build (warnings as errors)
🧹 Nitpick comments (6)
packages/rs-sdk-ffi/build_ios.sh (4)
37-50: Use precise min-version flags and make targets overridable; 18.0 default is risky.Hard-coding iOS 18.0 can drop support for many devices; also use -miphoneos-version-min (device) and -mios-simulator-version-min (sim) for clarity.
-# Set iOS deployment targets to match Xcode 16 SDKs and avoid 10.0 default -export IPHONEOS_DEPLOYMENT_TARGET="18.0" -export IPHONESIMULATOR_DEPLOYMENT_TARGET="18.0" -# Ensure linker uses the same min version when rustc links, but only for iOS targets -export CARGO_TARGET_AARCH64_APPLE_IOS_RUSTFLAGS="${CARGO_TARGET_AARCH64_APPLE_IOS_RUSTFLAGS:-} -C link-arg=-mios-version-min=${IPHONEOS_DEPLOYMENT_TARGET}" -export CARGO_TARGET_AARCH64_APPLE_IOS_SIM_RUSTFLAGS="${CARGO_TARGET_AARCH64_APPLE_IOS_SIM_RUSTFLAGS:-} -C link-arg=-mios-version-min=${IPHONESIMULATOR_DEPLOYMENT_TARGET}" -export CARGO_TARGET_X86_64_APPLE_IOS_RUSTFLAGS="${CARGO_TARGET_X86_64_APPLE_IOS_RUSTFLAGS:-} -C link-arg=-mios-version-min=${IPHONESIMULATOR_DEPLOYMENT_TARGET}" +# Allow overrides; default to a broader baseline (adjust if policy differs) +export IPHONEOS_DEPLOYMENT_TARGET="${IPHONEOS_DEPLOYMENT_TARGET:-13.0}" +export IPHONESIMULATOR_DEPLOYMENT_TARGET="${IPHONESIMULATOR_DEPLOYMENT_TARGET:-13.0}" +# Ensure rustc link flags match Xcode semantics +export CARGO_TARGET_AARCH64_APPLE_IOS_RUSTFLAGS="${CARGO_TARGET_AARCH64_APPLE_IOS_RUSTFLAGS:-} -C link-arg=-miphoneos-version-min=${IPHONEOS_DEPLOYMENT_TARGET}" +export CARGO_TARGET_AARCH64_APPLE_IOS_SIM_RUSTFLAGS="${CARGO_TARGET_AARCH64_APPLE_IOS_SIM_RUSTFLAGS:-} -C link-arg=-mios-simulator-version-min=${IPHONESIMULATOR_DEPLOYMENT_TARGET}" +export CARGO_TARGET_X86_64_APPLE_IOS_RUSTFLAGS="${CARGO_TARGET_X86_64_APPLE_IOS_RUSTFLAGS:-} -C link-arg=-mios-simulator-version-min=${IPHONESIMULATOR_DEPLOYMENT_TARGET}"
211-237: Comment says we “Fix ManagedWalletInfo → FFIManagedWalletInfo”, but no transform is applied.Either implement the rename in this AWK block or update the comment to avoid confusion.
Example (add a sed after AWK if needed):
sed -i '' 's/\bManagedWalletInfo\b/FFIManagedWalletInfo/g' "$MERGED_HEADER"
391-406: Generate module.modulemap conditionally.Referencing SPV/KeyWallet modules when headers are absent will break consumers. Create entries only when the header exists.
-cat > "$OUTPUT_DIR/module.modulemap" << EOF -module DashSDKFFI { - header "dash_sdk_ffi.h" - export * -} - -module DashSPVFFI { - header "dash_spv_ffi.h" - export * -} - -module KeyWalletFFI { - header "key_wallet_ffi.h" - export * -} -EOF +: > "$OUTPUT_DIR/module.modulemap" +{ + echo 'module DashSDKFFI {' + echo ' header "dash_sdk_ffi.h"' + echo ' export *' + echo '}' + [ -f "$HEADERS_DIR/dash_spv_ffi.h" ] && { + echo 'module DashSPVFFI {' + echo ' header "dash_spv_ffi.h"' + echo ' export *' + echo '}' + } + [ -f "$HEADERS_DIR/key_wallet_ffi.h" ] && { + echo 'module KeyWalletFFI {' + echo ' header "key_wallet_ffi.h"' + echo ' export *' + echo '}' + } +} >> "$OUTPUT_DIR/module.modulemap"Note: Move module.modulemap creation after the header copy (Lines 409–429).
446-463: Universal sim: prefer universal SPV lib before combining.When BUILD_ARCH=universal and both SPV sim archives exist, lipo them first to avoid arch gaps.
- SIM_SPV_LIB="" + SIM_SPV_LIB="" + if [ "$BUILD_ARCH" = "universal" ] && \ + [ -f "$SPV_CRATE_PATH/target/aarch64-apple-ios-sim/release/libdash_spv_ffi.a" ] && \ + [ -f "$SPV_CRATE_PATH/target/x86_64-apple-ios/release/libdash_spv_ffi.a" ]; then + echo -e "${GREEN}Creating universal SPV simulator library...${NC}" + lipo -create \ + "$SPV_CRATE_PATH/target/aarch64-apple-ios-sim/release/libdash_spv_ffi.a" \ + "$SPV_CRATE_PATH/target/x86_64-apple-ios/release/libdash_spv_ffi.a" \ + -output "$OUTPUT_DIR/simulator/libdash_spv_ffi_universal.a" + SIM_SPV_LIB="$OUTPUT_DIR/simulator/libdash_spv_ffi_universal.a" + fi.github/workflows/swift-sdk-build.yml (2)
170-236: Skip PR comment step for forks to avoid failures.Fork PR tokens can’t write comments; guard the step to keep builds green.
- - 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 uses: actions/github-script@v7
52-55: Make cbindgen install deterministic and fail-fast.Avoid masking install failures and pin via cargo for reproducibility.
- - name: Install cbindgen (for header generation) - run: | - brew install cbindgen || true + - name: Install cbindgen (for header generation) + run: | + set -euxo pipefail + if ! command -v cbindgen >/dev/null 2>&1; then + cargo install --locked cbindgen@0.26.0 + fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/swift-sdk-build.yml(1 hunks)packages/rs-sdk-ffi/build_ios.sh(7 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all source packages under packages/* (JS/TS packages and Rust crates)
Files:
packages/rs-sdk-ffi/build_ios.sh
packages/rs-sdk-ffi/**
📄 CodeRabbit inference engine (AGENTS.md)
iOS/FFI Rust artifacts live in packages/rs-sdk-ffi
Files:
packages/rs-sdk-ffi/build_ios.sh
🧠 Learnings (8)
📓 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:20:55.344Z
Learnt from: CR
PR: dashpay/platform#0
File: AGENTS.md:0-0
Timestamp: 2025-09-07T22:20:55.344Z
Learning: Applies to packages/rs-sdk-ffi/** : iOS/FFI Rust artifacts live in packages/rs-sdk-ffi
Applied to files:
packages/rs-sdk-ffi/build_ios.sh.github/workflows/swift-sdk-build.yml
📚 Learning: 2025-09-07T22:18:50.883Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/rs-sdk-ffi/**/*.{h,rs} : Follow unified SDK function prefixes: dash_core_sdk_* for Core, dash_sdk_* for Platform, dash_unified_sdk_* for unified APIs
Applied to files:
packages/rs-sdk-ffi/build_ios.sh
📚 Learning: 2025-09-07T22:18:50.883Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/rs-sdk-ffi/**/*.h : In C headers, use pointers for opaque FFI types
Applied to files:
packages/rs-sdk-ffi/build_ios.sh
📚 Learning: 2025-09-07T22:18:50.883Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/swift-sdk/Package.swift : If the xcframework is missing, create a symlink or update Package.swift accordingly
Applied to files:
packages/rs-sdk-ffi/build_ios.sh
📚 Learning: 2025-09-07T22:20:55.344Z
Learnt from: CR
PR: dashpay/platform#0
File: AGENTS.md:0-0
Timestamp: 2025-09-07T22:20:55.344Z
Learning: Applies to packages/swift-sdk/** : Swift example app and SDK live in packages/swift-sdk
Applied to files:
.github/workflows/swift-sdk-build.yml
📚 Learning: 2024-11-26T12:46:54.812Z
Learnt from: lklimek
PR: dashpay/platform#2344
File: .github/actions/sccache/action.yaml:0-0
Timestamp: 2024-11-26T12:46:54.812Z
Learning: In the `.github/actions/sccache/action.yaml` file, the environment variables `CC` and `CXX` need quotes around their values (e.g., `CC="sccache cc"`) to be interpreted correctly.
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 Platform SDK functions with the dash_sdk_* prefix
Applied to files:
.github/workflows/swift-sdk-build.yml
🪛 actionlint (1.7.7)
.github/workflows/swift-sdk-build.yml
90-90: shellcheck reported issue in this script: SC2086:info:2:40: Double quote to prevent globbing and word splitting
(shellcheck)
253-253: shellcheck reported issue in this script: SC2086:info:9:34: Double quote to prevent globbing and word splitting
(shellcheck)
⏰ 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). (180)
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
🔇 Additional comments (1)
.github/workflows/swift-sdk-build.yml (1)
48-51: Confirm whether x86_64-apple-ios is required for the XCFrameworkNo matches for "x86_64-apple-ios" were found in packages/swift-sdk/build_ios.sh — verify whether build_ios.sh or any iOS build step produces an x86_64 simulator slice. If it does, add x86_64-apple-ios to the rustup target list in .github/workflows/swift-sdk-build.yml (the workflow currently only adds aarch64 targets). Run locally: rg -nC2 -F 'x86_64-apple-ios' packages/swift-sdk || true
There was a problem hiding this comment.
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/SPV/SPVClient.swift (1)
330-342: Clear callbacks before destroying native client.
Ensure native won’t call back after destruction.Apply this diff:
private func destroyClient() { if let client = client { + // Clear callbacks defensively before destroy + var empty = FFIEventCallbacks() + empty.user_data = nil + dash_spv_ffi_client_set_event_callbacks(client, empty) dash_spv_ffi_client_destroy(client) self.client = nil } if let config = config { dash_spv_ffi_config_destroy(config) self.config = nil } - callbackContext = nil + // Release context after callbacks are cleared and client/config are gone + callbackContext = nil }
♻️ Duplicate comments (1)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (1)
103-105: Typed FFI handles rely on unsafe C zero-sized wrapper structs — switch to opaque structs or RawPointer.
Reiterating earlier note: if FFIDashSpvClient/FFIClientConfig are declared in C as zero-sized wrappers (unsigned char _private[0]), some toolchains mis-handle size/alignment, and Swift typed pointers can be UB. Prefer forward-declared opaque structs in headers or fall back to UnsafeMutableRawPointer in Swift until headers are fixed.Example C header change (preferred):
typedef struct FFIDashSpvClient FFIDashSpvClient; typedef struct FFIClientConfig FFIClientConfig;Swift stays with UnsafeMutablePointer safely after regenerating bindings. If headers can’t change now, consider:
- Keep Swift fields as UnsafeMutableRawPointer? and cast only at call sites.
🧹 Nitpick comments (7)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (7)
112-114: Expose a runtime setter for restrictToConfiguredPeers.
Right now the flag is applied only during initialize. Consider a setter mirroring setMasternodeSyncEnabled that updates config and calls dash_spv_ffi_client_update_config, so apps can toggle it without re-init.
183-203: Local-core peers: verify error-memory ownership and gate logs; also handle add_peer rc consistently.
- If dash_spv_ffi_get_last_error returns owned memory, free it (or switch to an API that fills a buffer/FFIError). Please confirm ownership rules.
- Consider gating the failure prints behind swiftLoggingEnabled (consistent with other logs).
Potential tightening in-place:
- addr.withCString { cstr in - let rc = dash_spv_ffi_config_add_peer(configPtr, cstr) - if rc != 0, let err = dash_spv_ffi_get_last_error() { - let msg = String(cString: err) - print("[SPV][Config] add_peer failed for \(addr): \(msg)") - } - } + addr.withCString { cstr in + let rc = dash_spv_ffi_config_add_peer(configPtr, cstr) + if rc != 0, let err = dash_spv_ffi_get_last_error() { + let msg = String(cString: err) + if swiftLoggingEnabled { + print("[SPV][Config] add_peer failed for \(addr): \(msg)") + } + // If error strings must be freed, call the appropriate free here. + } + }
204-209: Check rc when enabling restrict-to-configured-peers.
Avoid silently ignoring failures.- _ = dash_spv_ffi_config_set_restrict_to_configured_peers(configPtr, true) + let rc = dash_spv_ffi_config_set_restrict_to_configured_peers(configPtr, true) + if rc != 0, let err = dash_spv_ffi_get_last_error(), swiftLoggingEnabled { + print("[SPV][Config] restrict_to_configured_peers failed: \(String(cString: err))") + }
278-286: Peer-address parsing: consider basic validation and IPv6 support.
Comma-splitting is fine, but you may want to trim duplicates and accept bracketed IPv6 literals like [::1]:port.
489-491: Absolute heights: ensure targetHeight uses the same coordinate system.
You add startFromHeight to currentHeight but keep targetHeight as returned by FFI. Confirm whether total_height is absolute; if it’s relative to the checkpoint, add startFromHeight to it for consistency.
523-528: Wallet manager pointer cast: prefer typed return over assumingMemoryBound.
There’s a mismatch with the PR summary claiming a typed return. If the FFI now returns UnsafeMutablePointer?, return it directly; otherwise the binding is fine but brittle.If FFI is typed, simplify:
- let managerPtr = dash_spv_ffi_client_get_wallet_manager(client) - return managerPtr?.assumingMemoryBound(to: FFIWalletManager.self) + let managerPtr: UnsafeMutablePointer<FFIWalletManager>? = dash_spv_ffi_client_get_wallet_manager(client) + return managerPtr
654-666: Progress math: overflow-safe addition and coordinate consistency.
- &+ avoids traps but can wrap silently. Consider addingReportingOverflow to log/floor on overflow (practically unlikely, but safer).
- Also confirm targetHeight vs currentHeight units (absolute vs relative), as noted above.
Example:
let base = client?.startFromHeight ?? 0 let (sum, overflow) = base.addingReportingOverflow(ffiProgress.current_height) let absoluteCurrent = overflow ? UInt32.max : sum
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift(10 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.swift
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all source packages under packages/* (JS/TS packages and Rust crates)
Files:
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift
packages/swift-sdk/**
📄 CodeRabbit inference engine (AGENTS.md)
Swift example app and SDK live in packages/swift-sdk
Files:
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift
🧠 Learnings (2)
📓 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:18:50.883Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/rs-sdk-ffi/**/*.h : In C headers, use pointers for opaque FFI types
Applied to files:
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift
🧬 Code graph analysis (1)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (1)
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift (1)
currentHeight(476-493)
⏰ 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). (15)
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (rs-sdk-ffi) / Unused dependencies
- GitHub Check: Rust packages (rs-sdk-ffi) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: build-wasm-sdk
- GitHub Check: Swift SDK and Example build (warnings as errors)
🔇 Additional comments (7)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (7)
38-40: Adding startHeight to SPVSyncProgress looks good.
Exposes checkpoint context for UIs without breaking existing fields.
116-118: startFromHeight tracking — LGTM.
Keeps absolute-height context for UI.
150-162: One-time SPV logging init — LGTM.
Defensive default and scoped init guard are good.
165-177: Confirm FFINetwork mapping for devnet/custom.
You map DashSDKNetwork(rawValue: 2) to FFINetwork(rawValue: 3). Please verify that 3 is the correct devnet enum on the FFI side across all updated crates.
179-182: Nil config guard — LGTM.
Early-fails with a specific error.
261-263: Persisting checkpoint to startFromHeight — LGTM.
Keeps UI normalization consistent with configured checkpoint.
706-707: Completion propagates startHeight — LGTM.
Keeps UI post-sync consistent.
| // Perform synchronous teardown to avoid capturing self in an escaping Task | ||
| if let c = client { | ||
| dash_spv_ffi_client_stop(c) | ||
| } | ||
| destroyClient() | ||
| } |
There was a problem hiding this comment.
Teardown race: callbacks can fire after destroy; cancel sync and clear callbacks before stop/destroy.
CallbackContext is passed with passUnretained and is released here via callbackContext = nil in destroyClient. If native code invokes a callback after this point, userData may dangle. Ensure you (1) cancel sync, (2) clear event callbacks (user_data = NULL, fn ptrs = NULL), then (3) stop and destroy.
Apply this diff to harden deinit:
deinit {
- // Perform synchronous teardown to avoid capturing self in an escaping Task
- if let c = client {
- dash_spv_ffi_client_stop(c)
- }
- destroyClient()
+ // Synchronous teardown: cancel sync, clear callbacks, then stop and destroy
+ if let c = client {
+ // 1) Cancel any in-flight sync to stop progress/completion callbacks
+ dash_spv_ffi_client_cancel_sync(c)
+ // 2) Clear event callbacks to prevent late calls into freed context
+ var empty = FFIEventCallbacks()
+ empty.user_data = nil
+ dash_spv_ffi_client_set_event_callbacks(c, empty)
+ // 3) Stop the client
+ dash_spv_ffi_client_stop(c)
+ }
+ destroyClient()
}And ensure destroyClient also clears callbacks just before destroy (see next comment).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Perform synchronous teardown to avoid capturing self in an escaping Task | |
| if let c = client { | |
| dash_spv_ffi_client_stop(c) | |
| } | |
| destroyClient() | |
| } | |
| deinit { | |
| // Synchronous teardown: cancel sync, clear callbacks, then stop and destroy | |
| if let c = client { | |
| // 1) Cancel any in-flight sync to stop progress/completion callbacks | |
| dash_spv_ffi_client_cancel_sync(c) | |
| // 2) Clear event callbacks to prevent late calls into freed context | |
| var empty = FFIEventCallbacks() | |
| empty.user_data = nil | |
| dash_spv_ffi_client_set_event_callbacks(c, empty) | |
| // 3) Stop the client | |
| dash_spv_ffi_client_stop(c) | |
| } | |
| destroyClient() | |
| } |
🤖 Prompt for AI Agents
In packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift around lines
135-140, the deinit teardown currently stops and destroys the native client
after releasing the callback context, which can cause dangling userData if
native callbacks fire; modify deinit to first cancel any synchronous operations,
then clear all event callbacks by setting native user_data to NULL and all
callback function pointers to NULL, and only after callbacks are cleared call
dash_spv_ffi_client_stop(c) and destroyClient(); also update destroyClient to
explicitly clear the native callbacks (user_data = NULL and fn pointers = NULL)
immediately before invoking the native destroy so that no callbacks can observe
a released callbackContext.
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)
392-399: Main-actor updates required in cancelSync().You’re mutating @published properties off the main actor. Wrap in MainActor to avoid concurrency violations and UI glitches.
public func cancelSync() { guard let client = client, isSyncing else { return } syncCancelled = true dash_spv_ffi_client_cancel_sync(client) - isSyncing = false - syncProgress = nil + Task { @MainActor in + self.isSyncing = false + self.syncProgress = nil + } }
♻️ Duplicate comments (2)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (2)
103-105: Typed FFI pointers: ensure C types are truly opaque structs (avoid zero-sized members).This mirrors a prior note: if FFIDashSpvClient/FFIClientConfig/FFIWalletManager are declared with zero-sized arrays, Swift typed pointers are risky. Prefer forward-declared opaque structs in C.
- Verify the headers’ typedefs:
#!/bin/bash rg -n -C2 $'typedef struct\\s+FFIDashSpvClient\\b|typedef struct\\s+FFIClientConfig\\b|typedef struct\\s+FFIWalletManager\\b' packages
- If you see patterns like
unsigned char _private[0];, switch to opaque structs:// In the C/FFI headers -typedef struct FFIDashSpvClient { unsigned char _private[0]; } FFIDashSpvClient; +typedef struct FFIDashSpvClient FFIDashSpvClient; -typedef struct FFIClientConfig { unsigned char _private[0]; } FFIClientConfig; +typedef struct FFIClientConfig FFIClientConfig; -typedef struct FFIWalletManager { unsigned char _private[0]; } FFIWalletManager; +typedef struct FFIWalletManager FFIWalletManager;
- Re: getWalletManager: if the header already returns
UnsafeMutablePointer<FFIWalletManager>?, dropassumingMemoryBound:- let managerPtr = dash_spv_ffi_client_get_wallet_manager(client) - return managerPtr?.assumingMemoryBound(to: FFIWalletManager.self) + return dash_spv_ffi_client_get_wallet_manager(client)If the header returns
OpaquePointer?orUnsafeMutableRawPointer?, cast explicitly rather than usingassumingMemoryBoundwithout proof of provenance:let raw = dash_spv_ffi_client_get_wallet_manager(client) return raw.flatMap { UnsafeMutablePointer<FFIWalletManager>($0) }Also applies to: 523-528
135-140: Teardown ordering: cancel sync and clear native callbacks before stop/destroy to prevent UAF.Callbacks still reference callbackContext via user_data. Clear them at the native layer before destroying the client. Also cancel an in-flight sync first.
Apply:
deinit { - // Perform synchronous teardown to avoid capturing self in an escaping Task - if let c = client { - dash_spv_ffi_client_stop(c) - } - destroyClient() + // Synchronous teardown: cancel sync, clear callbacks, then stop and destroy + if let c = client { + // 1) Cancel any in-flight sync to stop progress/completion callbacks + dash_spv_ffi_client_cancel_sync(c) + // 2) Clear event callbacks to prevent late calls into freed context + var empty = FFIEventCallbacks() + empty.user_data = nil + dash_spv_ffi_client_set_event_callbacks(c, empty) + // 3) Stop the client + dash_spv_ffi_client_stop(c) + } + destroyClient() } private func destroyClient() { - if let client = client { - dash_spv_ffi_client_destroy(client) - self.client = nil - } + if let client = client { + // Clear callbacks again defensively just before destroy + var empty = FFIEventCallbacks() + empty.user_data = nil + dash_spv_ffi_client_set_event_callbacks(client, empty) + dash_spv_ffi_client_destroy(client) + self.client = nil + } if let config = config { dash_spv_ffi_config_destroy(config) self.config = nil } - callbackContext = nil + callbackContext = nil }Also applies to: 330-342
🧹 Nitpick comments (4)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (4)
112-114: Expose runtime toggle for restrict-to-configured-peers.The public flag isn’t applied after initialize. Add a didSet to propagate changes and update the running client.
- public var restrictToConfiguredPeers: Bool = false + public var restrictToConfiguredPeers: Bool = false { + didSet { + guard oldValue != restrictToConfiguredPeers, let cfg = self.config else { return } + _ = dash_spv_ffi_config_set_restrict_to_configured_peers(cfg, restrictToConfiguredPeers) + if let c = self.client { + _ = dash_spv_ffi_client_update_config(c, cfg) + } + if swiftLoggingEnabled { print("[SPV][Config] restrict-to-configured-peers=\(restrictToConfiguredPeers)") } + } + }Also applies to: 204-209
159-162: Gate verbose prints behind swiftLoggingEnabled.User-Agent is always printed; make it consistent with the logging flag.
- // 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)") }Also applies to: 230-241
476-483: Rate calculation should use elapsed time between block events.Currently divides blocksDiff by total time since sync start, producing a decaying rate. Track a last timestamp.
- if lastBlockHeight > 0 { - let blocksDiff = height - lastBlockHeight - let timeDiff = Date().timeIntervalSince(syncStartTime ?? Date()) - let rate = timeDiff > 0 ? Double(blocksDiff) / timeDiff : 0 + if lastBlockHeight > 0 { + let blocksDiff = height - lastBlockHeight + let now = Date().timeIntervalSince1970 + let lastTs = self.lastBlockTimestamp ?? (self.syncStartTime?.timeIntervalSince1970 ?? now) + let timeDiff = now - lastTs + let rate = timeDiff > 0 ? Double(blocksDiff) / timeDiff : 0 … } - lastBlockHeight = height + lastBlockHeight = height + lastBlockTimestamp = Date().timeIntervalSince1970Add a field near other sync tracking vars:
- private var lastBlockHeight: UInt32 = 0 + private var lastBlockHeight: UInt32 = 0 + private var lastBlockTimestamp: TimeInterval?Also applies to: 498-499
165-177: Deduplicate FFINetwork mapping.Mapping is repeated in three places. Centralize to a helper to prevent drift.
@inline(__always) private func mapToFFINetwork(_ net: Network) -> FFINetwork { switch net { case DashSDKNetwork(rawValue: 0): return FFINetwork(rawValue: 0) case DashSDKNetwork(rawValue: 1): return FFINetwork(rawValue: 1) case DashSDKNetwork(rawValue: 2): return FFINetwork(rawValue: 3) default: return FFINetwork(rawValue: 1) } }Then use
mapToFFINetwork(network)in initialize/checkpoint helpers.Also applies to: 575-585, 598-604
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift(11 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.swift
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all source packages under packages/* (JS/TS packages and Rust crates)
Files:
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift
packages/swift-sdk/**
📄 CodeRabbit inference engine (AGENTS.md)
Swift example app and SDK live in packages/swift-sdk
Files:
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift
🧠 Learnings (2)
📓 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:18:50.883Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/rs-sdk-ffi/**/*.h : In C headers, use pointers for opaque FFI types
Applied to files:
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift
🧬 Code graph analysis (1)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (2)
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift (1)
currentHeight(476-493)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (5)
spvClient(490-554)spvClient(556-560)spvClient(562-572)spvClient(574-589)spvClient(591-595)
⏰ 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). (16)
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (rs-sdk-ffi) / Formatting
- GitHub Check: Rust packages (rs-sdk-ffi) / Unused dependencies
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Swift SDK and Example build (warnings as errors)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
🔇 Additional comments (4)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (4)
38-39: Absolute-height progress wiring looks good.Adding startHeight and normalizing currentHeight to absolute space is consistent with SwiftExampleApp’s consumption and avoids UI jumps when starting from a checkpoint. No further action needed here.
Also applies to: 261-263, 654-656, 661-665, 705-709
671-676: UI coalescing via timestamp snapshotting is clean.Capturing a value-type snapshot before hopping to MainActor avoids races; coalescing interval is reasonable.
656-663: Confirm targetHeight semantics (absolute vs relative)currentHeight is converted to absolute ((client?.startFromHeight ?? 0) &+ ffiProgress.current_height) but targetHeight is forwarded as ffiProgress.total_height — if total_height is relative-to-checkpoint the UI denominator will be incorrect.
Verify FFIDetailedSyncProgress/FFISyncProgress.total_height in the generated FFI header (e.g. packages/rs-sdk-ffi/include/dash_spv_ffi.h or the rust FFI crate/cbindgen output). If total_height is relative, change to:
targetHeight: (client?.startFromHeight ?? 0) &+ ffiProgress.total_height and update any progress/percentage calculations and logs.
Location: packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (handleProgressUpdate, ~lines 655–663).
183-203: Free the C string returned by dash_spv_ffi_get_last_errordash_spv_ffi_get_last_error() is called but the returned C string is never freed — this will leak if the FFI allocates it. Use the FFI string-free API (e.g. dash_sdk_string_free or the SPV-specific free function if one exists) after converting to a Swift String. Apply to the call sites in packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (the local-core peers loop and the client start/error handling; also check the nearby 231–241 range).
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (2)
108-109: Fix type safety issue with typed FFI pointers.The client and config pointers are now strongly typed, but the type definitions show zero-sized arrays which is unusual for opaque types in C/FFI boundaries.
139-141: Implement proper cleanup in deinit to prevent resource leaks.The minimal teardown approach defers cleanup responsibility to callers, but this can lead to resource leaks if callers forget to call
stop(). The SPV client holds FFI resources that must be properly released.Apply this diff to ensure resources are always cleaned up:
deinit { - // Minimal teardown; prefer explicit stop() by callers. + // Ensure resources are cleaned up even if stop() wasn't called + if let c = client { + dash_spv_ffi_client_stop(c) + } + destroyClient() }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (1)
8-9: Consider the implications of @unchecked Sendable.The
SendableBoxwrapper uses@unchecked Sendableto bypass Swift's concurrency checking. While this allows moving non-Sendable types across actor boundaries, it places the burden of thread-safety on the developer. Ensure that the wrapped SPVClient is only accessed in a thread-safe manner.
🧹 Nitpick comments (4)
packages/swift-sdk/Sources/SwiftDashSDK/ConcurrencyCompat.swift (2)
7-7: Consider a typed wrapper over OpaquePointer instead of retroactive Sendable.Safer alternative: define a small wrapper (e.g., FFIHandle) and make only that @unchecked Sendable with documented invariants (lifetime, synchronization). This avoids global conformance and reduces UB risk if invariants drift.
I can sketch a minimal wrapper and a migration grep to update call sites if you want.
1-1: Remove unused import.Foundation isn’t referenced here.
Apply:
-import Foundationpackages/swift-sdk/Package.swift (1)
29-30: Package-level Swift 6 mode is correct; consider also enforcing per-target for Xcode quirks.
swiftLanguageModes: [.v6]is valid for tools-version 6 manifests. Some Xcode 16 builds still require per-target language mode to actually enable Swift 6 diagnostics; consider addingswiftSettings: [.swiftLanguageMode(.v6)]toSwiftDashSDKfor consistency. (github.com)Add inside the
SwiftDashSDKtarget:swiftSettings: [ .swiftLanguageMode(.v6) ],packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (1)
79-79: Remove redundant nil initialization.Initializing an optional variable with nil is redundant in Swift as optionals are nil by default.
-var startHeight: UInt32? = nil +var startHeight: UInt32?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/swift-sdk/Package.swift(2 hunks)packages/swift-sdk/Sources/SwiftDashSDK/ConcurrencyCompat.swift(1 hunks)packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift(18 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift(11 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
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/ConcurrencyCompat.swiftpackages/swift-sdk/Package.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swiftpackages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all source packages under packages/* (JS/TS packages and Rust crates)
Files:
packages/swift-sdk/Sources/SwiftDashSDK/ConcurrencyCompat.swiftpackages/swift-sdk/Package.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swiftpackages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift
packages/swift-sdk/**
📄 CodeRabbit inference engine (AGENTS.md)
Swift example app and SDK live in packages/swift-sdk
Files:
packages/swift-sdk/Sources/SwiftDashSDK/ConcurrencyCompat.swiftpackages/swift-sdk/Package.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swiftpackages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift
packages/swift-sdk/Package.swift
📄 CodeRabbit inference engine (CLAUDE.md)
If the xcframework is missing, create a symlink or update Package.swift accordingly
Files:
packages/swift-sdk/Package.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 (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:20:55.344Z
Learnt from: CR
PR: dashpay/platform#0
File: AGENTS.md:0-0
Timestamp: 2025-09-07T22:20:55.344Z
Learning: Applies to packages/swift-sdk/** : Swift example app and SDK live in packages/swift-sdk
Applied to files:
packages/swift-sdk/Package.swift
📚 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:
packages/swift-sdk/Package.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
📚 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:
packages/swift-sdk/Package.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
📚 Learning: 2025-09-07T22:18:50.883Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/swift-sdk/Package.swift : If the xcframework is missing, create a symlink or update Package.swift accordingly
Applied to files:
packages/swift-sdk/Package.swift
📚 Learning: 2025-09-07T22:18:50.883Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/rs-sdk-ffi/**/*.h : In C headers, use pointers for opaque FFI types
Applied to files:
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift
🧬 Code graph analysis (2)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (1)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (8)
stop(320-327)getCheckpointHeight(596-611)initialize(145-277)start(302-318)getLatestCheckpointHeight(572-593)startSync(345-387)getStats(531-548)getSyncSnapshot(551-567)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (5)
spvClient(490-554)spvClient(556-560)spvClient(562-572)spvClient(574-589)spvClient(591-595)
🪛 SwiftLint (0.57.0)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
[Warning] 79-79: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
⏰ 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). (14)
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
- GitHub Check: build-wasm-sdk
- GitHub Check: Swift SDK and Example build (warnings as errors)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
🔇 Additional comments (7)
packages/swift-sdk/Sources/SwiftDashSDK/ConcurrencyCompat.swift (2)
9-11: FFIDetailedSyncProgress sendability looks fine—confirm it’s a POD with no interior pointers.If it contains raw pointers/handles or requires lifetime management, prefer a wrapper instead of unchecked Sendable. If it’s part of the SDK’s public API across actors, you may need to make the conformance public rather than package/internal.
3-11: Gate Sendable conformances to Swift 6 and narrow them to package scope; confirm public-API visibility.Apply:
-// Swift 6 sendability adjustments for FFI pointers and wrappers. -// These are safe under our usage patterns where FFI pointers are thread-confined -// or explicitly synchronized at the Rust boundary. - -extension OpaquePointer: @retroactive @unchecked Sendable {} - -// FFI value types from DashSDKFFI headers used across actor boundaries -// These are plain C structs and treated as inert data blobs. -extension FFIDetailedSyncProgress: @retroactive @unchecked Sendable {} +// Swift 6 sendability adjustments for FFI pointers and wrappers. +// These are safe under our usage patterns where FFI pointers are thread-confined +// or explicitly synchronized at the Rust boundary. +#if swift(>=6.0) +package extension OpaquePointer: @retroactive @unchecked Sendable {} + +// FFI value types from DashSDKFFI headers used across actor boundaries. +// These are plain C structs and treated as inert data blobs. +package extension FFIDetailedSyncProgress: @retroactive @unchecked Sendable {} +#endifVerify: packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/SDK/StateTransitionExtensions.swift:100 exposes a public func returning OpaquePointer — confirm the example app target is part of the same Swift package; if it is a separate exported module, either make the conformance public or move/export it from the module that needs it.
packages/swift-sdk/Package.swift (1)
1-1: Ensure CI mac builders use Xcode 16+ (Swift 6)Manifest bumped to swift-tools-version: 6.0 (packages/swift-sdk/Package.swift). Verification run returned no workflow matches or local xcode/swift — confirm .github/workflows reference Xcode 16.x or that CI installs an explicit Swift 6 toolchain (search for setup-xcode, xcode-version, DEVELOPER_DIR, XCODE_VERSION).
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (3)
185-203: LGTM! Local core peer configuration.The implementation properly reads local core peers from UserDefaults, adds them via FFI, and automatically enables restrict-to-configured-peers mode when using local core. Error handling for peer addition failures is appropriate with logging for debugging.
419-422: Good: Using weak references in event callbacks prevents retain cycles.The callbacks now properly capture weak references to avoid retain cycles when events are dispatched to the main thread.
262-263: startFromHeight usage verified — no changes requiredSet at init when a checkpoint is chosen and consistently passed as startHeight on all SPVSyncProgress objects; FFI progress adds startFromHeight to ffiProgress.current_height to produce absoluteCurrent, and block-based updates use absolute block heights directly.
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (1)
499-514: Good: Proper isolation of non-Sendable progress data.The implementation correctly copies all needed values from the non-Sendable
progressparameter to local primitives before using them in the async context, avoiding potential data races.
| path: "Sources/SwiftDashSDK", | ||
| exclude: ["KeyWallet/README.md"] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Target path/exclude OK; verify the excluded file and the XCFramework path exist.
Exclude is fine. Please also confirm the FFI XCFramework is present at the declared path; if it’s generated elsewhere, add a repo-local symlink or adjust path: per the guidelines.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) README exclude exists?
if [ -f "packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/README.md" ]; then
echo "OK: README exists and will be excluded."
else
echo "Note: README not found at Sources/SwiftDashSDK/KeyWallet/README.md (exclude is harmless if file is absent)."
fi
# 2) XCFramework presence and basic sanity
FOUND=$(fd -t d '^DashSDKFFI\.xcframework$' packages | head -n1 || true)
if [ -z "${FOUND:-}" ]; then
echo "ERROR: DashSDKFFI.xcframework not found under packages/. If it lives elsewhere, create a symlink at packages/swift-sdk/DashSDKFFI.xcframework."
exit 1
fi
echo "Found XCFramework at: $FOUND"
# 3) List supported platforms/architectures from Info.plist
python - << 'PY'
import plistlib, sys, pathlib, json
p = pathlib.Path("packages/swift-sdk/DashSDKFFI.xcframework/Info.plist")
try:
with p.open('rb') as f:
info = plistlib.load(f)
libs = info.get("AvailableLibraries") or info.get("AvailableLibraries", [])
out = [{"platform": l.get("SupportedPlatform"), "archs": l.get("SupportedArchitectures")} for l in libs]
print(json.dumps(out, indent=2))
except FileNotFoundError:
print("Note: Info.plist not found at expected location; printing tree for debugging.")
import os
for root, dirs, files in os.walk("packages/swift-sdk/DashSDKFFI.xcframework"):
for name in files:
if name.endswith(".swiftmodule") or name.endswith(".h") or name.endswith(".modulemap") or name == "Info.plist":
print(os.path.join(root, name))
PYLength of output: 182
DashSDKFFI.xcframework missing — add repo-local XCFramework or symlink
- Exclude OK: packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/README.md exists.
- Verification: script could not find packages/swift-sdk/DashSDKFFI.xcframework. Add the XCFramework at packages/swift-sdk/DashSDKFFI.xcframework, create a repo-local symlink there to the actual artifact, or update packages/swift-sdk/Package.swift (lines 26–27) to the correct path.
🤖 Prompt for AI Agents
In packages/swift-sdk/Package.swift around lines 26–27, the package expects a
local DashSDKFFI.xcframework at packages/swift-sdk/DashSDKFFI.xcframework but
the file is missing; either (1) add the XCFramework to
packages/swift-sdk/DashSDKFFI.xcframework in the repo, (2) create a repo-local
symlink at that path that points to the actual XCFramework artifact, or (3)
update the Package.swift paths to point to the actual location of
DashSDKFFI.xcframework; implement one of these fixes and ensure the manifest
references the corrected path so the package resolution succeeds.
| func handleProgressUpdate(_ ffiProgress: FFIDetailedSyncProgress) { | ||
|
|
||
| // Calculate estimated time remaining | ||
| var estimatedTime: TimeInterval? = nil | ||
| if ffiProgress.estimated_seconds_remaining > 0 { | ||
| estimatedTime = Double(ffiProgress.estimated_seconds_remaining) | ||
| } | ||
| // Compute stage and ETA outside the actor | ||
| let stage: SPVSyncStage = { | ||
| if ffiProgress.percentage < 0.3 { return .headers } | ||
| if ffiProgress.percentage < 0.7 { return .masternodes } | ||
| if ffiProgress.percentage < 1.0 { return .transactions } | ||
| return .complete | ||
| }() | ||
| let estimatedTime: TimeInterval? = (ffiProgress.estimated_seconds_remaining > 0) | ||
| ? TimeInterval(ffiProgress.estimated_seconds_remaining) | ||
| : nil | ||
|
|
||
| if client?.swiftLoggingEnabled == true { | ||
| let pct = max(0.0, min(ffiProgress.percentage, 1.0)) * 100.0 | ||
| let cur = ffiProgress.current_height | ||
| let tot = ffiProgress.total_height | ||
| let rate = ffiProgress.headers_per_second | ||
| let eta = ffiProgress.estimated_seconds_remaining | ||
| print("[SPV][Progress] stage=\(stage.rawValue) pct=\(String(format: "%.2f", pct))% height=\(cur)/\(tot) rate=\(String(format: "%.2f", rate)) hdr/s eta=\(eta)s") | ||
| } | ||
|
|
||
| let progress = SPVSyncProgress( | ||
| stage: stage, | ||
| headerProgress: min(ffiProgress.percentage / 0.3, 1.0), | ||
| masternodeProgress: min(max((ffiProgress.percentage - 0.3) / 0.4, 0), 1.0), | ||
| transactionProgress: min(max((ffiProgress.percentage - 0.7) / 0.3, 0), 1.0), | ||
| currentHeight: ffiProgress.current_height, | ||
| targetHeight: ffiProgress.total_height, | ||
| rate: ffiProgress.headers_per_second, | ||
| estimatedTimeRemaining: estimatedTime | ||
| ) | ||
|
|
||
| let now = Date().timeIntervalSince1970 | ||
| if let client = self.client, now - client.lastProgressUIUpdate >= client.progressUICoalesceInterval { | ||
| client.lastProgressUIUpdate = now | ||
| Task { @MainActor in | ||
| guard let clientStrong = self.client else { return } | ||
| clientStrong.syncProgress = progress | ||
| clientStrong.delegate?.spvClient(clientStrong, didUpdateSyncProgress: progress) | ||
| // Update UI/state on main actor | ||
| guard let client = self.client else { return } | ||
|
|
||
| if client.swiftLoggingEnabled { | ||
| let pct = max(0.0, min(ffiProgress.percentage, 1.0)) * 100.0 | ||
| let cur = ffiProgress.current_height | ||
| let tot = ffiProgress.total_height | ||
| let rate = ffiProgress.headers_per_second | ||
| let eta = ffiProgress.estimated_seconds_remaining | ||
| print("[SPV][Progress] stage=\(stage.rawValue) pct=\(String(format: "%.2f", pct))% height=\(cur)/\(tot) rate=\(String(format: "%.2f", rate)) hdr/s eta=\(eta)s") | ||
| } | ||
|
|
||
| let absoluteCurrent: UInt32 = client.startFromHeight &+ ffiProgress.current_height | ||
| let progress = SPVSyncProgress( | ||
| stage: stage, | ||
| headerProgress: min(ffiProgress.percentage / 0.3, 1.0), | ||
| masternodeProgress: min(max((ffiProgress.percentage - 0.3) / 0.4, 0), 1.0), | ||
| transactionProgress: min(max((ffiProgress.percentage - 0.7) / 0.3, 0), 1.0), | ||
| currentHeight: absoluteCurrent, | ||
| targetHeight: ffiProgress.total_height, | ||
| startHeight: client.startFromHeight, | ||
| rate: ffiProgress.headers_per_second, | ||
| estimatedTimeRemaining: estimatedTime | ||
| ) | ||
|
|
||
| let now = Date().timeIntervalSince1970 | ||
| if now - client.lastProgressUIUpdate >= client.progressUICoalesceInterval { | ||
| client.lastProgressUIUpdate = now | ||
| client.syncProgress = progress | ||
| client.delegate?.spvClient(client, didUpdateSyncProgress: progress) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Progress calculations need validation for overflow and edge cases.
The progress update handler performs arithmetic on heights but doesn't validate against overflow or handle edge cases where targetHeight might be less than startFromHeight.
Apply this diff to add proper validation:
func handleProgressUpdate(_ ffiProgress: FFIDetailedSyncProgress) {
// Compute stage and ETA outside the actor
let stage: SPVSyncStage = {
if ffiProgress.percentage < 0.3 { return .headers }
if ffiProgress.percentage < 0.7 { return .masternodes }
if ffiProgress.percentage < 1.0 { return .transactions }
return .complete
}()
let estimatedTime: TimeInterval? = (ffiProgress.estimated_seconds_remaining > 0)
? TimeInterval(ffiProgress.estimated_seconds_remaining)
: nil
// Update UI/state on main actor
guard let client = self.client else { return }
if client.swiftLoggingEnabled {
let pct = max(0.0, min(ffiProgress.percentage, 1.0)) * 100.0
let cur = ffiProgress.current_height
let tot = ffiProgress.total_height
let rate = ffiProgress.headers_per_second
let eta = ffiProgress.estimated_seconds_remaining
print("[SPV][Progress] stage=\(stage.rawValue) pct=\(String(format: "%.2f", pct))% height=\(cur)/\(tot) rate=\(String(format: "%.2f", rate)) hdr/s eta=\(eta)s")
}
- let absoluteCurrent: UInt32 = client.startFromHeight &+ ffiProgress.current_height
+ // Validate heights to prevent overflow and ensure target >= start
+ let safeStart = client.startFromHeight
+ let safeCurrent = min(UInt32.max - safeStart, ffiProgress.current_height)
+ let absoluteCurrent: UInt32 = safeStart &+ safeCurrent
+ let safeTarget = max(safeStart, ffiProgress.total_height)
+
let progress = SPVSyncProgress(
stage: stage,
headerProgress: min(ffiProgress.percentage / 0.3, 1.0),
masternodeProgress: min(max((ffiProgress.percentage - 0.3) / 0.4, 0), 1.0),
transactionProgress: min(max((ffiProgress.percentage - 0.7) / 0.3, 0), 1.0),
currentHeight: absoluteCurrent,
- targetHeight: ffiProgress.total_height,
+ targetHeight: safeTarget,
startHeight: client.startFromHeight,
rate: ffiProgress.headers_per_second,
estimatedTimeRemaining: estimatedTime
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func handleProgressUpdate(_ ffiProgress: FFIDetailedSyncProgress) { | |
| // Calculate estimated time remaining | |
| var estimatedTime: TimeInterval? = nil | |
| if ffiProgress.estimated_seconds_remaining > 0 { | |
| estimatedTime = Double(ffiProgress.estimated_seconds_remaining) | |
| } | |
| // Compute stage and ETA outside the actor | |
| let stage: SPVSyncStage = { | |
| if ffiProgress.percentage < 0.3 { return .headers } | |
| if ffiProgress.percentage < 0.7 { return .masternodes } | |
| if ffiProgress.percentage < 1.0 { return .transactions } | |
| return .complete | |
| }() | |
| let estimatedTime: TimeInterval? = (ffiProgress.estimated_seconds_remaining > 0) | |
| ? TimeInterval(ffiProgress.estimated_seconds_remaining) | |
| : nil | |
| if client?.swiftLoggingEnabled == true { | |
| let pct = max(0.0, min(ffiProgress.percentage, 1.0)) * 100.0 | |
| let cur = ffiProgress.current_height | |
| let tot = ffiProgress.total_height | |
| let rate = ffiProgress.headers_per_second | |
| let eta = ffiProgress.estimated_seconds_remaining | |
| print("[SPV][Progress] stage=\(stage.rawValue) pct=\(String(format: "%.2f", pct))% height=\(cur)/\(tot) rate=\(String(format: "%.2f", rate)) hdr/s eta=\(eta)s") | |
| } | |
| let progress = SPVSyncProgress( | |
| stage: stage, | |
| headerProgress: min(ffiProgress.percentage / 0.3, 1.0), | |
| masternodeProgress: min(max((ffiProgress.percentage - 0.3) / 0.4, 0), 1.0), | |
| transactionProgress: min(max((ffiProgress.percentage - 0.7) / 0.3, 0), 1.0), | |
| currentHeight: ffiProgress.current_height, | |
| targetHeight: ffiProgress.total_height, | |
| rate: ffiProgress.headers_per_second, | |
| estimatedTimeRemaining: estimatedTime | |
| ) | |
| let now = Date().timeIntervalSince1970 | |
| if let client = self.client, now - client.lastProgressUIUpdate >= client.progressUICoalesceInterval { | |
| client.lastProgressUIUpdate = now | |
| Task { @MainActor in | |
| guard let clientStrong = self.client else { return } | |
| clientStrong.syncProgress = progress | |
| clientStrong.delegate?.spvClient(clientStrong, didUpdateSyncProgress: progress) | |
| // Update UI/state on main actor | |
| guard let client = self.client else { return } | |
| if client.swiftLoggingEnabled { | |
| let pct = max(0.0, min(ffiProgress.percentage, 1.0)) * 100.0 | |
| let cur = ffiProgress.current_height | |
| let tot = ffiProgress.total_height | |
| let rate = ffiProgress.headers_per_second | |
| let eta = ffiProgress.estimated_seconds_remaining | |
| print("[SPV][Progress] stage=\(stage.rawValue) pct=\(String(format: "%.2f", pct))% height=\(cur)/\(tot) rate=\(String(format: "%.2f", rate)) hdr/s eta=\(eta)s") | |
| } | |
| let absoluteCurrent: UInt32 = client.startFromHeight &+ ffiProgress.current_height | |
| let progress = SPVSyncProgress( | |
| stage: stage, | |
| headerProgress: min(ffiProgress.percentage / 0.3, 1.0), | |
| masternodeProgress: min(max((ffiProgress.percentage - 0.3) / 0.4, 0), 1.0), | |
| transactionProgress: min(max((ffiProgress.percentage - 0.7) / 0.3, 0), 1.0), | |
| currentHeight: absoluteCurrent, | |
| targetHeight: ffiProgress.total_height, | |
| startHeight: client.startFromHeight, | |
| rate: ffiProgress.headers_per_second, | |
| estimatedTimeRemaining: estimatedTime | |
| ) | |
| let now = Date().timeIntervalSince1970 | |
| if now - client.lastProgressUIUpdate >= client.progressUICoalesceInterval { | |
| client.lastProgressUIUpdate = now | |
| client.syncProgress = progress | |
| client.delegate?.spvClient(client, didUpdateSyncProgress: progress) | |
| } | |
| } | |
| } | |
| func handleProgressUpdate(_ ffiProgress: FFIDetailedSyncProgress) { | |
| // Compute stage and ETA outside the actor | |
| let stage: SPVSyncStage = { | |
| if ffiProgress.percentage < 0.3 { return .headers } | |
| if ffiProgress.percentage < 0.7 { return .masternodes } | |
| if ffiProgress.percentage < 1.0 { return .transactions } | |
| return .complete | |
| }() | |
| let estimatedTime: TimeInterval? = (ffiProgress.estimated_seconds_remaining > 0) | |
| ? TimeInterval(ffiProgress.estimated_seconds_remaining) | |
| : nil | |
| // Update UI/state on main actor | |
| guard let client = self.client else { return } | |
| if client.swiftLoggingEnabled { | |
| let pct = max(0.0, min(ffiProgress.percentage, 1.0)) * 100.0 | |
| let cur = ffiProgress.current_height | |
| let tot = ffiProgress.total_height | |
| let rate = ffiProgress.headers_per_second | |
| let eta = ffiProgress.estimated_seconds_remaining | |
| print("[SPV][Progress] stage=\(stage.rawValue) pct=\(String(format: "%.2f", pct))% height=\(cur)/\(tot) rate=\(String(format: "%.2f", rate)) hdr/s eta=\(eta)s") | |
| } | |
| // Validate heights to prevent overflow and ensure target >= start | |
| let safeStart = client.startFromHeight | |
| let safeCurrent = min(UInt32.max - safeStart, ffiProgress.current_height) | |
| let absoluteCurrent: UInt32 = safeStart &+ safeCurrent | |
| let safeTarget = max(safeStart, ffiProgress.total_height) | |
| let progress = SPVSyncProgress( | |
| stage: stage, | |
| headerProgress: min(ffiProgress.percentage / 0.3, 1.0), | |
| masternodeProgress: min(max((ffiProgress.percentage - 0.3) / 0.4, 0), 1.0), | |
| transactionProgress: min(max((ffiProgress.percentage - 0.7) / 0.3, 0), 1.0), | |
| currentHeight: absoluteCurrent, | |
| targetHeight: safeTarget, | |
| startHeight: client.startFromHeight, | |
| rate: ffiProgress.headers_per_second, | |
| estimatedTimeRemaining: estimatedTime | |
| ) | |
| let now = Date().timeIntervalSince1970 | |
| if now - client.lastProgressUIUpdate >= client.progressUICoalesceInterval { | |
| client.lastProgressUIUpdate = now | |
| client.syncProgress = progress | |
| client.delegate?.spvClient(client, didUpdateSyncProgress: progress) | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift around lines 624
to 668, the progress handler performs unchecked arithmetic and percentage math
that can overflow or produce invalid progress when targetHeight <
startFromHeight or when FFI values are out of expected ranges; update it to
validate and clamp inputs before use: ensure ffiProgress.percentage is clamped
0.0...1.0, treat negative or zero headers_per_second/estimated_seconds_remaining
as nil/zero appropriately, compute absoluteCurrent using a safe checked addition
(or guard against overflow by capping to UInt32.max) and then clamp
currentHeight between startFromHeight and targetHeight, guard/normalize
targetHeight >= startFromHeight (if not, set targetHeight = startFromHeight),
and compute header/masternode/transaction progress with safe denominators and
clamped results (no division by zero, values constrained 0.0...1.0) so
downstream UI and delegate always receive consistent, non-overflowing progress
values.
|
|
||
| // Stop any ongoing sync | ||
| await stopSync() | ||
| stopSync() |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify that stopSync() properly cancels all async operations.
The method calls stopSync() which cancels sync and stops the stats timer, but doesn't await any cleanup. Ensure that all async operations are properly cancelled before proceeding with network switch.
🏁 Script executed:
#!/bin/bash
# Check stopSync implementation
echo "Checking stopSync implementation..."
rg -n "func stopSync\(\)" packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift -A 10
# Check if there are any dangling async operations
echo -e "\nChecking for async tasks that might not be cancelled..."
rg -n "syncTask|balanceUpdateTask" packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swiftLength of output: 504
Cancel and await syncTask & balanceUpdateTask in stopSync
stopSync() (WalletService.swift:285–292) cancels spvClient and the stats timer but does not cancel or await syncTask and balanceUpdateTask (declared at WalletService.swift:27–28). Cancel both tasks and ensure they complete before switching networks—either make stopSync() async and await syncTask?.value / await balanceUpdateTask?.value, or call syncTask?.cancel() / balanceUpdateTask?.cancel() and spawn a Task to await their completion; ensure the tasks observe Task.isCancelled.
|
✅ 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: "79bbbcf0bcd5d05c75ed58bac44e6124379d591e55f7fcd05ffc70663e432c74"
)Xcode manual integration:
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (2)
329-341: Clear event callbacks before destroy.Avoids dangling userData if native side posts after destroy.
private func destroyClient() { if let client = client { + var empty = FFIEventCallbacks() + empty.user_data = nil + dash_spv_ffi_client_set_event_callbacks(client, empty) dash_spv_ffi_client_destroy(client) self.client = nil } @@ callbackContext = nil }
366-377: Use-after-free risk: passRetained context and release in completion.Using passUnretained + takeUnretainedValue assumes Swift lifetime outlives native callbacks. Switch to passRetained and balance with takeRetainedValue in spvCompletionCallback.
- let contextPtr = Unmanaged.passUnretained(context).toOpaque() + let contextPtr = Unmanaged.passRetained(context).toOpaque()And update spvCompletionCallback:
- let context = Unmanaged<CallbackContext>.fromOpaque(userData).takeUnretainedValue() + let context = Unmanaged<CallbackContext>.fromOpaque(userData).takeRetainedValue()packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (6)
16-18: TypedetailedSyncProgressprecisely (avoidAny?).Use a concrete type to improve safety and readability.
Apply:
- @Published public var detailedSyncProgress: Any? // Use SPVClient.SyncProgress + @Published public var detailedSyncProgress: SyncProgress?
74-102: Concurrency bug: mixed capture (clientvsclientLocal) and unnecessaryawaiton sync API.Inside Task.detached you correctly use
clientLocal, but Line 99 callsclient.getCheckpointHeight(...)(notclientLocal) and addsawaitto a synchronous method. This crosses actor boundaries with a non-Sendable reference and won’t compile.Apply:
- var startHeight: UInt32? = nil + var startHeight: UInt32? ... - if let h = await client.getCheckpointHeight(beforeTimestamp: ts) { + if let h = clientLocal.getCheckpointHeight(beforeTimestamp: ts) { startHeight = h }
271-283: Avoid Task.detached for capturing non-SendableSPVClient; store the task handle.Use Task (inherits MainActor isolation here) and keep a reference to cancel/await later.
Apply:
- // Kick off sync without blocking the main thread - Task.detached(priority: .userInitiated) { + // Kick off sync without blocking the main thread (keeps MainActor isolation) + self.syncTask = Task(priority: .userInitiated) { do { try await spvClient.startSync() } catch { await MainActor.run { WalletService.shared.lastSyncError = error WalletService.shared.isSyncing = false } print("❌ Sync failed: \(error)") } }Follow-up: In stopSync(), cancel this task and (ideally) await it to finish.
465-483: RemoveawaitfromgetStats()and keep the actor hop minimal.
getStats()is synchronous; awaiting it is invalid.Apply:
- Task.detached(priority: .utility) { + Task.detached(priority: .utility) { let clientBox = await MainActor.run { WalletService.shared[keyPath: \WalletService.spvClient].map(SendableBox.init) } guard let client = clientBox?.value else { return } - guard let stats = await client.getStats() else { return } + guard let stats = client.getStats() else { return } await MainActor.run { if stats.headerHeight > 0 { WalletService.shared.latestHeaderHeight = max(WalletService.shared.latestHeaderHeight, stats.headerHeight) } if stats.filterHeight > 0 { WalletService.shared.latestFilterHeight = max(WalletService.shared.latestFilterHeight, stats.filterHeight) } } }
598-611: Stage mapping bug:.masternodesmapped to.filters.Map it to a dedicated stage or to
.downloading. Best: add.masternodestoSyncStage.Apply:
switch stage { case .idle: return .idle case .headers: return .headers case .masternodes: - return .filters + return .masternodes case .transactions: return .downloading case .complete: return .complete }And add
case masternodesinSyncStagebelow.
624-631: Expose amasternodesstage for UI parity with SPV.Keeps UI semantics clear.
Apply:
public enum SyncStage { case idle case connecting case headers case filters + case masternodes case downloading case complete }
♻️ Duplicate comments (6)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (4)
108-110: Revisit typed FFI pointers; fix opaque C type declarations.The Swift typed pointers are fine, but if the C headers still use zero-sized arrays for opaque structs, switch to forward-declared opaque structs to avoid UB and importer quirks. This is a repeat of a prior note.
-typedef struct FFIDashSpvClient { unsigned char _private[0]; } FFIDashSpvClient; +typedef struct FFIDashSpvClient FFIDashSpvClient;
139-141: Teardown race: ensure callbacks are cleared and sync is canceled before client/config destroy.Even with “prefer explicit stop()”, deinit should be defensive: late FFI callbacks could dereference a released context. Clear callbacks and cancel sync prior to stop/destroy (also update destroyClient).
deinit { - // Minimal teardown; prefer explicit stop() by callers. + // Defensive teardown in case callers forgot to stop: + if let c = client { + // 1) Cancel sync to stop progress/completion callbacks + dash_spv_ffi_client_cancel_sync(c) + // 2) Clear callbacks (user_data = NULL and function ptrs = NULL) + var empty = FFIEventCallbacks() + empty.user_data = nil + dash_spv_ffi_client_set_event_callbacks(c, empty) + // 3) Stop client + dash_spv_ffi_client_stop(c) + } + destroyClient() }
624-668: Clamp percentages and guard against UInt32 overflow when adding heights.Unchecked
startFromHeight &+ current_heightcan overflow; also clamp percentages to 0…1 and ensure target >= start.- let absoluteCurrent: UInt32 = client.startFromHeight &+ ffiProgress.current_height + let safeStart = client.startFromHeight + let addend = min(ffiProgress.current_height, UInt32.max &- safeStart) + let absoluteCurrent: UInt32 = safeStart &+ addend + let clampedPct = min(max(ffiProgress.percentage, 0.0), 1.0) + let safeTarget = max(safeStart, ffiProgress.total_height) let progress = SPVSyncProgress( stage: stage, - headerProgress: min(ffiProgress.percentage / 0.3, 1.0), - masternodeProgress: min(max((ffiProgress.percentage - 0.3) / 0.4, 0), 1.0), - transactionProgress: min(max((ffiProgress.percentage - 0.7) / 0.3, 0), 1.0), + headerProgress: min(clampedPct / 0.3, 1.0), + masternodeProgress: min(max((clampedPct - 0.3) / 0.4, 0), 1.0), + transactionProgress: min(max((clampedPct - 0.7) / 0.3, 0), 1.0), currentHeight: absoluteCurrent, - targetHeight: ffiProgress.total_height, + targetHeight: safeTarget, startHeight: client.startFromHeight, rate: ffiProgress.headers_per_second, estimatedTimeRemaining: estimatedTime )
320-327: Stop should first cancel sync and clear callbacks to avoid late events.Call cancel_sync, clear event callbacks, then stop; also reset delegate-facing flags after stop.
public func stop() { guard let client = client else { return } - - dash_spv_ffi_client_stop(client) + // 1) Cancel any in-flight sync work + dash_spv_ffi_client_cancel_sync(client) + // 2) Clear event callbacks so late native calls are no-ops + var empty = FFIEventCallbacks() + empty.user_data = nil + dash_spv_ffi_client_set_event_callbacks(client, empty) + // 3) Stop the client + dash_spv_ffi_client_stop(client) self.isConnected = false self.isSyncing = false self.syncProgress = nil }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (2)
8-9: Unchecked Sendable wrapper demands strict discipline.Using @unchecked Sendable for SPVClient crossings can mask data races unless the underlying client is proven thread-safe. Prefer confining SPVClient to an actor or pass immutable snapshots across actors.
Would you like a minimal actor wrapper around SPVClient to remove @unchecked? I can draft it.
303-307: Ensure sync task cleanup when switching networks.
stopSync()doesn’t cancel/awaitsyncTask; race possible withspvClient?.stop()and deallocation.Recommend: make stopSync() async, call
syncTask?.cancel()and thenawait syncTask?.valueafter signallingspvClient.cancelSync().
🧹 Nitpick comments (10)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (8)
13-17: Prefer Task @mainactor over DispatchQueue for actor-hopping.Replace the main-queue hop with structured concurrency to respect @mainactor and avoid callback reentrancy ordering issues.
- DispatchQueue.main.async { - context.handleProgressUpdate(snapshot) - } + Task { @MainActor in + context.handleProgressUpdate(snapshot) + }
26-30: Use Task @mainactor here as well (and coordinate with retained userData if adopted).Consistent actor hop improves clarity. If you switch to passRetained on context (see startSync comment), balance it here with takeRetainedValue().
- let errorString: String? = errorMsg.map { String(cString: $0) } + let errorString: String? = errorMsg.map { String(cString: $0) } @@ - DispatchQueue.main.async { - context.handleSyncCompletion(success: success, error: errorString) - } + Task { @MainActor in + context.handleSyncCompletion(success: success, error: errorString) + }
121-128: Document invariants and thread-affinity for sync-tracking fields.These are main-actor only; add a brief comment to prevent accidental cross-actor access. Also see overflow-safe math in progress handler below.
184-204: Local-core peer injection: add basic address validation and IPv6 support.Before adding, validate entries (“host[:port]”), support bracketed IPv6, and log rejects.
- for addr in peers { - addr.withCString { cstr in + for addr in peers { + // Basic validation; allow IPv4, bracketed IPv6, or hostnames with optional :port + guard addr.contains(".") || addr.contains(":") else { + if swiftLoggingEnabled { print("[SPV][Config] Skipping invalid peer: \(addr)") } + continue + } + addr.withCString { cstr in let rc = dash_spv_ffi_config_add_peer(configPtr, cstr) if rc != 0, let err = dash_spv_ffi_get_last_error() { let msg = String(cString: err) print("[SPV][Config] add_peer failed for \(addr): \(msg)") } } }
223-243: Gate the “Setting user agent …” print behind swiftLoggingEnabled.Avoid noisy stdout in production.
- // Always print what we're about to set for easier debugging - print("Setting user agent to \(ua)") + if swiftLoggingEnabled { print("Setting user agent to \(ua)") }
279-287: Minor: accept comma or semicolon separators and trim robustly.Improves DX for environment/user defaults.
- return list - .split(separator: ",") + return list + .split(whereSeparator: { $0 == "," || $0 == ";" || $0.isNewline }) .map { $0.trimmingCharacters(in: .whitespaces) } .filter { !$0.isEmpty }
481-491: Fix rate calculation to use delta time, not time since start.Current logic divides the block delta by total elapsed time, underreporting speed. Track last timestamp to compute per-interval rate, or compute overall average consistently.
- let timeDiff = Date().timeIntervalSince(syncStartTime ?? Date()) - let rate = timeDiff > 0 ? Double(blocksDiff) / timeDiff : 0 + let now = Date() + let timeDiff = now.timeIntervalSince(syncStartTime ?? now) + let sinceLast = (now.timeIntervalSince1970 - (lastBlockTimestamp ?? now.timeIntervalSince1970)) + let rate = (sinceLast > 0) ? Double(blocksDiff) / sinceLast : ((timeDiff > 0) ? Double(height - startFromHeight) / timeDiff : 0) @@ syncProgress = updatedProgress delegate?.spvClient(self, didUpdateSyncProgress: updatedProgress) } @@ - lastBlockHeight = height + lastBlockHeight = height + lastBlockTimestamp = Date().timeIntervalSince1970Add property:
@Published public var lastError: String? + // Last block event timestamp (seconds) + private var lastBlockTimestamp: TimeInterval?
680-689: Remove duplicate completion logs.You log both before and inside the @mainactor Task; keep the latter only.
- if client?.swiftLoggingEnabled == true { - if success { - print("[SPV][Complete] Sync finished successfully") - } else { - print("[SPV][Complete] Sync failed: \(error ?? "unknown error")") - } - } + // Logging handled inside @MainActor task belowpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (2)
88-96: Optional: avoid magic bits for network filtering.Introduce named flags or a helper on
Network(e.g.,matches(walletNetworksMask:)) to replace1/2/8.
366-383: Nit: simplify optional height unwrap.Minor readability tweak.
Apply:
- blockHeight: hdTx.blockHeight != nil ? Int64(hdTx.blockHeight!) : nil, + blockHeight: hdTx.blockHeight.map(Int64.init),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift(19 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift(12 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
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/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all source packages under packages/* (JS/TS packages and Rust crates)
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)
Swift example app and SDK live 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 (4)
📓 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:18:50.883Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/rs-sdk-ffi/**/*.h : In C headers, use pointers for opaque FFI types
Applied to files:
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift
📚 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:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
📚 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:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
🧬 Code graph analysis (2)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (2)
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift (1)
currentHeight(476-493)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (5)
spvClient(490-555)spvClient(557-561)spvClient(563-573)spvClient(575-590)spvClient(592-596)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (1)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (8)
stop(320-327)getCheckpointHeight(596-611)initialize(145-277)start(302-318)getLatestCheckpointHeight(572-593)startSync(345-387)getStats(531-548)getSyncSnapshot(551-567)
🪛 SwiftLint (0.57.0)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
[Warning] 79-79: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
⏰ 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). (15)
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (rs-sdk-ffi) / Unused dependencies
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (rs-sdk-ffi) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-wasm-sdk
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Swift SDK and Example build (warnings as errors)
🔇 Additional comments (15)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (11)
42-44: Good: expose startHeight for absolute progress.This unblocks consumers (see WalletService) to compute normalized progress reliably.
95-95: Main-actor isolation for SPVClient looks right.Matches UI usage and callback delivery design.
117-119: Expose restrictToConfiguredPeers is helpful.Defaulting to true when using localhost peers is sane.
151-159: One-time logging init looks good.Scope-limited and environment-driven.
205-210: LGTM: restrict-to-configured-peers flag is applied to the config.Matches the public property.
251-264: Start checkpoint logic looks sound.Stores UI base height; see progress overflow guard below.
419-423: LGTM: event delivery hops to @mainactor with weak client.Prevents retain cycles and keeps UI-thread invariants.
Also applies to: 441-450
716-721: Sendable conformance for SPVStats is appropriate.Safe to pass across concurrency domains.
724-734: Sendable snapshot is good.Matches async consumers.
166-178: Verify FFINetwork mapping for devnet (rawValue: 3).Sandbox searches failed (ripgrep produced no results / nl missing). Confirm that the FFI's FFINetwork enum assigns devnet = 3 and that dash_spv_ffi_config_new(FFINetwork(rawValue: 3)) returns a devnet config; if not, use the correct FFINetwork value or add an explicit devnet constructor/wrapper.
Location: packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift lines 166–178.
Verify locally: rg -n --hidden -S -e 'enum FFINetwork' -e 'FFINetwork' -e 'dash_spv_ffi_config_' (run from repo root) or inspect the FFI C headers/bindings.
522-527: Avoid assumingMemoryBound — return a typed FFI pointer or bindMemory explicitlyConfirm whether dash_spv_ffi_client_get_wallet_manager is declared to return FFIWalletManager* in the FFI C header; if the FFI can expose a typed FFIWalletManager* then Swift will import UnsafeMutablePointer? and you can return that directly. If not, replace assumingMemoryBound with an explicit bindMemory and update other call sites (KeyWallet/WalletManager.swift) that use assumingMemoryBound.
- let managerPtr = dash_spv_ffi_client_get_wallet_manager(client) - return managerPtr?.assumingMemoryBound(to: FFIWalletManager.self) + #if canImport(DashSDKFFI) // when FFI returns typed pointer + return dash_spv_ffi_client_get_wallet_manager(client) + #else + let raw = dash_spv_ffi_client_get_wallet_manager(client) + return raw?.bindMemory(to: FFIWalletManager.self, capacity: 1) + #endifLocation: packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (≈lines 522–527); callers: packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift.
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (4)
4-4: Good: preconcurrency import is appropriate.Helps avoid stricter sendability checks when importing legacy APIs.
49-53: Good: safe deinit cleanup.Capturing the client then calling stop on MainActor avoids capturing self and UI-thread violations.
501-515: LGTM: progress normalization is correct.Clamps [0,1], protects against degenerate denominators, and updates UI on MainActor.
74-127: Do not removeawait— SPVClient is @mainactor, so its methods are actor‑isolated.SPVClient.swift declares
@MainActor public class SPVClient, so instance-method calls from non‑main contexts must useawait; keep theawaitusage in WalletService (packages/swift-sdk/SwiftExampleApp/.../WalletService.swift lines 74–127, 465–483, 522–547). If you need synchronous access, mark specific cheap gettersnonisolatedor batch hops withMainActor.run. (github.com)Likely an incorrect or invalid review comment.
| try await clientLocal.initialize(dataDir: dataDir, masternodesEnabled: mnEnabled, startHeight: startHeight) | ||
|
|
||
| // Start the SPV client | ||
| try client.start() | ||
| try await clientLocal.start() | ||
| print("✅ SPV Client initialized and started successfully for \(net.rawValue)") | ||
|
|
||
| // Seed UI with latest checkpoint height if we don't have a header yet | ||
| let seedHeight = client.getLatestCheckpointHeight() | ||
| let seedHeight = await clientLocal.getLatestCheckpointHeight() | ||
| await MainActor.run { |
There was a problem hiding this comment.
Remove await from synchronous SPVClient calls.
initialize, start, and getLatestCheckpointHeight are synchronous per SPVClient.swift.
Apply:
- try await clientLocal.initialize(dataDir: dataDir, masternodesEnabled: mnEnabled, startHeight: startHeight)
+ try clientLocal.initialize(dataDir: dataDir, masternodesEnabled: mnEnabled, startHeight: startHeight)
- try await clientLocal.start()
+ try clientLocal.start()
- let seedHeight = await clientLocal.getLatestCheckpointHeight()
+ let seedHeight = clientLocal.getLatestCheckpointHeight()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try await clientLocal.initialize(dataDir: dataDir, masternodesEnabled: mnEnabled, startHeight: startHeight) | |
| // Start the SPV client | |
| try client.start() | |
| try await clientLocal.start() | |
| print("✅ SPV Client initialized and started successfully for \(net.rawValue)") | |
| // Seed UI with latest checkpoint height if we don't have a header yet | |
| let seedHeight = client.getLatestCheckpointHeight() | |
| let seedHeight = await clientLocal.getLatestCheckpointHeight() | |
| await MainActor.run { | |
| try clientLocal.initialize(dataDir: dataDir, masternodesEnabled: mnEnabled, startHeight: startHeight) | |
| // Start the SPV client | |
| try clientLocal.start() | |
| print("✅ SPV Client initialized and started successfully for \(net.rawValue)") | |
| // Seed UI with latest checkpoint height if we don't have a header yet | |
| let seedHeight = clientLocal.getLatestCheckpointHeight() | |
| await MainActor.run { |
🤖 Prompt for AI Agents
In
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
around lines 119 to 127, the review notes that initialize, start, and
getLatestCheckpointHeight are synchronous SPVClient methods but were called with
await; remove the unnecessary await keywords and use plain try for initialize
and start (or just call them without await) and call getLatestCheckpointHeight
without await, preserving the MainActor.run block that follows.
| Task.detached(priority: .utility) { | ||
| let (clientBox, prevTx, prevMn): (SendableBox<SPVClient>?, Double, Double) = await MainActor.run { | ||
| (WalletService.shared[keyPath: \WalletService.spvClient].map(SendableBox.init), WalletService.shared.transactionProgress, WalletService.shared.masternodeProgress) | ||
| } | ||
| let client = clientBox?.value | ||
|
|
||
| // 1) Headers: use detailed current/total from progress callback | ||
| let headerPct = min(1.0, max(0.0, Double(progress.currentHeight) / Double(max(1, progress.targetHeight)))) | ||
| 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)) | ||
|
|
||
| // 2) Filters: prefer snapshot lastSyncedFilterHeight / headerHeight; fallback to stats ratio | ||
| var txPct = prevTx | ||
| if let snap = client?.getSyncSnapshot(), snap.headerHeight > 0 { | ||
| txPct = min(1.0, max(0.0, Double(snap.lastSyncedFilterHeight) / Double(snap.headerHeight))) | ||
| } else if let stats = client?.getStats(), stats.headerHeight > 0 { | ||
| txPct = min(1.0, max(0.0, Double(stats.filterHeight) / Double(stats.headerHeight))) | ||
| 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 | ||
| } | ||
|
|
||
| // 3) Masternodes: show only synced/unsynced (no misleading ratio) | ||
| var mnPct = prevMn | ||
| if let snap = client?.getSyncSnapshot() { | ||
| mnPct = snap.masternodesSynced ? 1.0 : 0.0 | ||
| let mnPctFinal: Double | ||
| if let snap = await client?.getSyncSnapshot() { | ||
| mnPctFinal = snap.masternodesSynced ? 1.0 : 0.0 | ||
| } else { | ||
| mnPctFinal = prevMn | ||
| } | ||
|
|
||
| await MainActor.run { | ||
| self.headerProgress = headerPct | ||
| self.transactionProgress = txPct | ||
| self.masternodeProgress = mnPct | ||
| WalletService.shared.headerProgress = headerPct | ||
| WalletService.shared.transactionProgress = txPctFinal | ||
| WalletService.shared.masternodeProgress = mnPctFinal | ||
| } |
There was a problem hiding this comment.
Remove await from synchronous snapshot/stats calls; keep header/tx/mn progress computation.
These SPVClient APIs are synchronous; awaiting them is invalid.
Apply:
- if let snap = await client?.getSyncSnapshot(), snap.headerHeight > 0 {
+ if let snap = 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 {
+ } else if let stats = client?.getStats(), stats.headerHeight > 0 {
txPctFinal = min(1.0, max(0.0, Double(stats.filterHeight) / Double(stats.headerHeight)))
} else {
txPctFinal = prevTx
}
- if let snap = await client?.getSyncSnapshot() {
+ if let snap = client?.getSyncSnapshot() {
mnPctFinal = snap.masternodesSynced ? 1.0 : 0.0
} else {
mnPctFinal = prevMn
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Task.detached(priority: .utility) { | |
| let (clientBox, prevTx, prevMn): (SendableBox<SPVClient>?, Double, Double) = await MainActor.run { | |
| (WalletService.shared[keyPath: \WalletService.spvClient].map(SendableBox.init), WalletService.shared.transactionProgress, WalletService.shared.masternodeProgress) | |
| } | |
| let client = clientBox?.value | |
| // 1) Headers: use detailed current/total from progress callback | |
| let headerPct = min(1.0, max(0.0, Double(progress.currentHeight) / Double(max(1, progress.targetHeight)))) | |
| 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)) | |
| // 2) Filters: prefer snapshot lastSyncedFilterHeight / headerHeight; fallback to stats ratio | |
| var txPct = prevTx | |
| if let snap = client?.getSyncSnapshot(), snap.headerHeight > 0 { | |
| txPct = min(1.0, max(0.0, Double(snap.lastSyncedFilterHeight) / Double(snap.headerHeight))) | |
| } else if let stats = client?.getStats(), stats.headerHeight > 0 { | |
| txPct = min(1.0, max(0.0, Double(stats.filterHeight) / Double(stats.headerHeight))) | |
| 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 | |
| } | |
| // 3) Masternodes: show only synced/unsynced (no misleading ratio) | |
| var mnPct = prevMn | |
| if let snap = client?.getSyncSnapshot() { | |
| mnPct = snap.masternodesSynced ? 1.0 : 0.0 | |
| let mnPctFinal: Double | |
| if let snap = await client?.getSyncSnapshot() { | |
| mnPctFinal = snap.masternodesSynced ? 1.0 : 0.0 | |
| } else { | |
| mnPctFinal = prevMn | |
| } | |
| await MainActor.run { | |
| self.headerProgress = headerPct | |
| self.transactionProgress = txPct | |
| self.masternodeProgress = mnPct | |
| WalletService.shared.headerProgress = headerPct | |
| WalletService.shared.transactionProgress = txPctFinal | |
| WalletService.shared.masternodeProgress = mnPctFinal | |
| } | |
| Task.detached(priority: .utility) { | |
| let (clientBox, prevTx, prevMn): (SendableBox<SPVClient>?, Double, Double) = await MainActor.run { | |
| (WalletService.shared[keyPath: \WalletService.spvClient].map(SendableBox.init), WalletService.shared.transactionProgress, WalletService.shared.masternodeProgress) | |
| } | |
| let client = clientBox?.value | |
| 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 txPctFinal: Double | |
| if let snap = client?.getSyncSnapshot(), snap.headerHeight > 0 { | |
| txPctFinal = min(1.0, max(0.0, Double(snap.lastSyncedFilterHeight) / Double(snap.headerHeight))) | |
| } else if let stats = 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 = client?.getSyncSnapshot() { | |
| mnPctFinal = snap.masternodesSynced ? 1.0 : 0.0 | |
| } else { | |
| mnPctFinal = prevMn | |
| } | |
| await MainActor.run { | |
| WalletService.shared.headerProgress = headerPct | |
| WalletService.shared.transactionProgress = txPctFinal | |
| WalletService.shared.masternodeProgress = mnPctFinal | |
| } |
🤖 Prompt for AI Agents
In
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
around lines 522 to 553, the review notes that SPVClient.getSyncSnapshot() and
SPVClient.getStats() are synchronous APIs but are being called with await;
remove the await keywords from those calls (both occurrences where
getSyncSnapshot() is used and the one getStats() call) so the snapshot/stats
access is synchronous, keep the surrounding nil checks, header/tx/mn progress
computation and the MainActor runs unchanged, and ensure optional chaining on
client remains intact.
The point of this PR is to get swift sdk cleaned up from multiple warnings (over 500). We also went to swift 6. We also tidied up our build scripts to remove hacks. CI is now in place that verifies that swift works as well.
Summary by CodeRabbit
New Features
Improvements
Chores
Bug Fixes
Documentation