chore: bump dash-spv-ffi to v0.41-dev branch #2852
Conversation
WalkthroughUpdated Rust FFI Cargo dependency to a moving branch; multiple Swift SDK and example-app updates including new WalletSyncScheduler actor, batched SPV progress dispatch, managed-wallet visibility tweak, FFI transaction build-and-sign and getTxid helpers, and example app wiring for signing and broadcasting. Changes
Sequence Diagram(s)sequenceDiagram
participant SPV as SPV C callback thread
participant Dispatcher as SPVProgressDispatcher (actor)
participant Main as MainActor / UI
participant Callback as SPVClient callback
SPV->>Dispatcher: enqueue(progress update)
Note right of Dispatcher `#E6F7FF`: coalesces latest update\nand schedules dispatch after interval
Dispatcher-->>Main: dispatch(coalesced progress)
Main->>Callback: handleProgressUpdate(progress)
Callback->>Main: update UI (throttled)
sequenceDiagram
participant WalletService
participant Scheduler as WalletSyncScheduler (actor)
participant WalletManager
participant FFI as SwiftDashSDK (Wallet / ManagedWallet)
participant Signing as Transaction.buildAndSign
WalletService->>Scheduler: enqueue(walletId)
Scheduler-->>WalletService: flush(ids) after debounce
WalletService->>WalletManager: getManagedWallet(for: id)
WalletManager->>FFI: wallet/managed info retrieval
WalletService->>Signing: buildAndSign(managedWallet, outputs...)
Signing-->>WalletService: signedTx (raw bytes)
WalletService->>WalletService: record pending HDTransaction
WalletService->>SPV: attempt broadcast (may rollback on failure)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
✅ 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: "fb985038ba2e844997086084eb2bab0bf0928c575773b7a8fc0fe4e8aae2af89"
)Xcode manual integration:
|
- Add walletId to SPVTransactionEvent; plumb through wallet-specific FFI callback - Introduce WalletSyncScheduler actor to debounce and coalesce per-wallet syncs - Remove per-block full sync; target wallet sync on tx events and periodic blocks - Reduce MainActor work by batching UI updates after sync - Fix actor isolation in scheduler Task (no optional self property access)
- Change visibility from private to internal to allow internal helpers to access the opaque FFI handle. - No functional behavior change.
- Use stable C strings for output addresses and free them to avoid dangling pointers - Switch to typed withUnsafeBytes pointers for clarity and safety - Add buildAndSign(managedWallet:wallet:...) convenience API - Add getTxid(from:) helper to extract transaction ID
- Add getFFIWallet(for:) to fetch SwiftDashSDK.Wallet - Add getManagedWallet(for:) to create a ManagedWallet for UTXO operations
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/WalletManager.swift (1)
788-801: Consider caching ManagedWallet instances for production.The comment on line 796-797 correctly notes that creating a new
ManagedWalletinstance on each call may not be optimal for production. SinceManagedWalletwraps UTXO state, repeatedly creating new instances could impact performance and require careful synchronization with UTXO updates.Consider implementing a cached instance tied to wallet lifecycle, invalidated when UTXOs change (e.g., after transactions or sync completion).
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (2)
784-784: Replace unused closure parameters with underscores.The unused parameters
_accountIndexand_isOursin the closure should be replaced with_to follow Swift conventions and eliminate the static analysis warnings.Apply this diff:
- callbacks.on_wallet_transaction = { walletIdPtr, _accountIndex, txidPtr, confirmed, amount, addressesPtr, blockHeight, _isOurs, userData in + callbacks.on_wallet_transaction = { walletIdPtr, _, txidPtr, confirmed, amount, addressesPtr, blockHeight, _, userData inBased on learnings
799-802: Remove redundant nil initialization.Line 799 initializes
walletIdtonil, which is redundant for optional types in Swift. The variable can be declared without explicit initialization.Apply this diff:
- var walletId: String? = nil + var walletId: String? if let walletIdPtr = walletIdPtr { walletId = String(cString: walletIdPtr) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedWallet.swift(1 hunks)packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Transaction.swift(8 hunks)packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift(12 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift(5 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletSyncScheduler.swift(1 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/WalletManager.swift(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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/KeyWallet/ManagedWallet.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/WalletManager.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swiftpackages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Transaction.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletSyncScheduler.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/Wallet/WalletManager.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletSyncScheduler.swift
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/Tests/**/*.swift : Use TestSigner for transaction signing in tests
Learnt from: CR
Repo: dashpay/platform PR: 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
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
Repo: dashpay/platform PR: 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/Wallet/WalletManager.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Transaction.swift
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
Repo: dashpay/platform PR: 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/Wallet/WalletManager.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Transaction.swift
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/Tests/**/*.swift : Use TestSigner for transaction signing in tests
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Transaction.swift
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/{PersistentPublicKey,PersistentIdentity}*.swift : Link private keys to PersistentPublicKey, not to PersistentIdentity
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
🧬 Code graph analysis (5)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/WalletManager.swift (2)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Models/Network.swift (1)
toKeyWalletNetwork(36-45)packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift (1)
getWallet(200-223)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (6)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/WalletManager.swift (4)
syncWalletStateFromRust(705-763)updateBalance(682-701)getFFIWallet(774-786)getManagedWallet(791-801)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionListView.swift (1)
loadTransactions(81-104)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/TransactionService.swift (1)
loadTransactions(71-83)packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Transaction.swift (2)
buildAndSign(144-216)getTxid(221-245)packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (1)
enqueue(40-48)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletSyncScheduler.swift (2)
enqueue(18-21)enqueueMany(23-26)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (6)
spvClient(813-921)spvClient(923-926)spvClient(928-943)spvClient(945-1004)spvClient(1006-1025)spvClient(1027-1029)
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Transaction.swift (2)
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift (1)
currentHeight(473-490)packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedWallet.swift (1)
getInfoHandle(424-427)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletSyncScheduler.swift (1)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (1)
enqueue(40-48)
🪛 SwiftLint (0.57.0)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
[Warning] 678-678: TODOs should be resolved (Make this configurable or use ...)
(todo)
[Warning] 685-685: TODOs should be resolved (Broadcast transaction via SPV ...)
(todo)
[Warning] 694-694: TODOs should be resolved (Extract actual fee from transa...)
(todo)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift
[Warning] 784-784: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
[Warning] 784-784: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
[Warning] 799-799: 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). (8)
- 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 Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
- GitHub Check: Determine changed packages
- GitHub Check: Swift SDK and Example build (warnings as errors)
🔇 Additional comments (14)
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedWallet.swift (1)
422-427: LGTM: Access level widened for cross-module FFI integration.The change from
privatetointernalvisibility forgetInfoHandle()is intentional and necessary to support the newbuildAndSignflow in Transaction.swift (line 163), which requires access to the managed wallet handle for FFI calls.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletSyncScheduler.swift (1)
1-51: LGTM: Clean debounced batching actor.The
WalletSyncSchedulerimplementation correctly:
- Uses an actor for thread-safe state management
- Coalesces wallet IDs into a Set to avoid duplicates
- Implements debouncing via
Task.sleepwith a configurable interval- Uses weak self to prevent retain cycles in the scheduled task
- Provides
flushNow()for deterministic testingNote: The current design does not reset the debounce timer on subsequent enqueues during the debounce window. This is acceptable and keeps the implementation simple, but be aware that rapid enqueues will still flush after the first debounce interval expires.
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Transaction.swift (5)
2-2: LGTM: Darwin import for C memory management.The
import Darwinstatement is necessary to support the new manual C string allocation pattern usingstrdupandfreefor FFI interop.
43-82: LGTM: Memory-safe C string management for FFI.The refactored output handling correctly:
- Uses
strdupto create stable C strings that outlive thewithCStringclosure- Preallocates arrays to avoid reallocation
- Guards against allocation failure and frees partial allocations on error (lines 51-54)
- Frees all allocated C strings in the defer block (lines 74-75)
This pattern ensures memory safety across FFI boundaries where Swift temporary strings cannot be used.
135-216: LGTM: Well-structured build-and-sign implementation.The new
buildAndSignmethod correctly:
- Validates inputs (non-empty outputs, non-watch-only wallet)
- Obtains the managed wallet handle for UTXO access
- Uses the same memory-safe C string allocation pattern as
build()- Calls
wallet_build_and_sign_transactionFFI with proper parameters- Frees all allocated resources in the defer block
- Copies transaction data before freeing the FFI buffer
This one-step build-and-sign flow is more efficient than separate build + sign calls.
218-245: LGTM: Clean TXID extraction from raw bytes.The new
getTxidmethod correctly:
- Binds raw bytes to
UInt8for the FFI call- Calls
transaction_get_txid_from_byteswith proper parameters- Frees both the error message and the returned string in the defer block
- Copies the TXID string before freeing the FFI buffer
This enables extracting transaction IDs without requiring separate storage or parsing logic.
108-108: LGTM: Explicit pointer types for clarity.The addition of explicit type annotations to
withUnsafeBytesclosures improves type safety and code clarity when working with FFI boundaries. This is consistent with modern Swift FFI patterns.Also applies to: 267-267, 270-270, 309-309
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (7)
31-86: LGTM! Well-designed progress throttling mechanism.The SPVProgressDispatcher actor effectively batches and throttles progress notifications to reduce main thread load. The 1.0-second interval and the grace period logic (lines 70-79) for catching late-arriving updates are reasonable design choices for this use case.
88-97: LGTM! Correctly integrated with the new throttling dispatcher.The callback now properly enqueues progress updates through the SPVProgressDispatcher instead of directly dispatching to the main thread, which aligns with the performance optimization goals.
245-252: LGTM! Wallet ID support properly integrated.The addition of optional
walletIdtoSPVTransactionEventand its propagation throughhandleTransactionEventis consistent throughout the codebase. This aligns with the wallet-aware transaction handling shown in the WalletService snippets, where transactions can be targeted to specific wallets.Also applies to: 907-918
305-305: LGTM! Reasonable UI throttling adjustment.Increasing the UI coalesce interval to 0.5 seconds aligns with the performance optimization goals and is still responsive enough for user experience.
524-543: LGTM! Proper state cleanup.The addition of
blocksHit = 0at line 542 correctly resets this counter when clearing storage, maintaining consistency with other state resets in the method.
1087-1260: LGTM! Significant performance optimization with proper actor isolation.This refactor successfully moves heavy progress calculations off the MainActor while maintaining thread safety:
- Lines 1090-1102: Extracts Sendable primitives from MainActor-isolated properties upfront
- Lines 1104-1236: Performs all heavy computation in a detached task with appropriate priority
- Lines 1238-1259: Updates MainActor properties and throttles expensive delegate calls
The stage determination logic (lines 1156-1166) and progress constraints (lines 1168-1222) are complex but appear sound. The use of overflow-safe arithmetic (e.g.,
&-at line 1147) and weak captures (line 1239) demonstrates attention to safety.
31-34: Visibility appropriately scoped.The
SPVProgressDispatcheractor is correctly scoped as internal since it's an implementation detail of the throttling mechanism. Public APIs likeSPVTransactionEventandSPVSyncProgressremain properly public per coding guidelines.As per coding guidelines
| guard let walletManager = self.walletManager else { | ||
| throw WalletError.notImplemented("WalletManager not available") | ||
| } | ||
|
|
||
| // Get the FFI wallet and managed wallet from WalletManager | ||
| guard let ffiWallet = try? await walletManager.getFFIWallet(for: wallet), | ||
| let managedWallet = try? await walletManager.getManagedWallet(for: wallet) else { | ||
| throw WalletError.notImplemented("Unable to access wallet for transaction signing") | ||
| } | ||
|
|
||
| // Get current blockchain height (default to 0 if not available) | ||
| let currentHeight = UInt32(wallet.lastSyncedHeight) | ||
|
|
||
| // Build transaction outputs | ||
| let outputs = [SwiftDashSDK.Transaction.Output(address: address, amount: amount)] | ||
|
|
||
| // Build and sign transaction using the FFI | ||
| let signedTxData = try SwiftDashSDK.Transaction.buildAndSign( | ||
| managedWallet: managedWallet, | ||
| wallet: ffiWallet, | ||
| accountIndex: 0, | ||
| outputs: outputs, | ||
| feePerKB: 1000, // TODO: Make this configurable or use fee estimation | ||
| currentHeight: currentHeight | ||
| ) | ||
|
|
||
| // Extract TXID from the signed transaction | ||
| let txid = try SwiftDashSDK.Transaction.getTxid(from: signedTxData) | ||
|
|
There was a problem hiding this comment.
Ensure ManagedWallet and Wallet share the same FFI handle
WalletManager.getManagedWallet(for:) (see WalletManager.swift Lines 790-800) internally calls getFFIWallet again, so the guard here ends up returning two different SwiftDashSDK.Wallet wrappers—one for ffiWallet, another hidden inside the new ManagedWallet. Transaction.buildAndSign expects both arguments to reference the same underlying wallet handle; otherwise the C layer reports KeyWalletError.invalidState("Failed to get managed wallet handle"), and signing aborts. The try? also erases the original error, making the failure opaque.
Please reuse a single fetched wallet when creating the managed wallet and propagate the real error:
- guard let ffiWallet = try? await walletManager.getFFIWallet(for: wallet),
- let managedWallet = try? await walletManager.getManagedWallet(for: wallet) else {
- throw WalletError.notImplemented("Unable to access wallet for transaction signing")
- }
+ let ffiWallet: SwiftDashSDK.Wallet
+ let managedWallet: SwiftDashSDK.ManagedWallet
+ do {
+ ffiWallet = try await walletManager.getFFIWallet(for: wallet)
+ managedWallet = try SwiftDashSDK.ManagedWallet(wallet: ffiWallet)
+ } catch {
+ throw WalletError.walletError("Unable to access wallet for transaction signing: \(error)")
+ }Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 678-678: TODOs should be resolved (Make this configurable or use ...)
(todo)
…management Add full transaction broadcasting support to Swift SDK: - Implement synchronous and async broadcast methods in SPVClient - Fix WalletManager handle ownership (SPV client owns the handle) - Add getManagedWalletWithCurrentState() to access SPV-managed UTXOs - Update transaction building to use current UTXO state from SPV - Add transaction rollback on broadcast failure - Remove debounced sync scheduler in favor of direct syncing - Update UI to show broadcast status and TXID 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (1)
615-685: Improve error propagation and fee handling in sendTransactionThe new
sendTransactionflow correctly:
- Ensures there's an active wallet and sufficient balance.
- Builds outputs, calls
Transaction.buildAndSign, extracts txid, and persists a pendingHDTransaction.- Broadcasts via
spvClient.broadcastTransactionAsyncand rolls back the pending record on broadcast failure.Two refinements would make this more robust and debuggable:
- Don't swallow SDK errors when fetching wallets (lines 628–629)
Using
try?in the guard hides the underlying reason:guard let ffiWallet = try? await walletManager.getFFIWallet(for: wallet), let managedWallet = try? await walletManager.getManagedWallet(for: wallet) else { throw WalletError.notImplemented("Unable to access wallet for transaction signing") }Consider propagating the real error instead:
let ffiWallet: SwiftDashSDK.Wallet let managedWallet: SwiftDashSDK.ManagedWallet do { ffiWallet = try await walletManager.getFFIWallet(for: wallet) managedWallet = try await walletManager.getManagedWallet(for: wallet) } catch { throw WalletError.walletError("Unable to access wallet for transaction signing: \(error.localizedDescription)") }
- Address the hard‑coded fee TODO (lines 645 and 655)
Both
feePerKB: 1000inbuildAndSignandtransaction.fee = 1000are fixed values. Wire these through a central fee policy so UI and on‑chain fees remain consistent and configurable.
🧹 Nitpick comments (5)
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedWallet.swift (1)
27-36: Clarify managedWalletInfo ownership and free function alignmentThe new
internal init(managedWalletInfo:network:)takes overmanagedWalletInfoanddeinitalways callsffi_managed_wallet_free(handle), but the comment still says “caller is responsible for ensuring the pointer remains valid”. That reads like a borrowed pointer, while the implementation is owning.Given other call sites of
wallet_manager_get_managed_wallet_infousemanaged_wallet_info_free, please double‑check that:
managedWalletInfois indeed expected to be freed viaffi_managed_wallet_free, and- callers never also call
managed_wallet_info_freeon the same pointer.If
ManagedWalletis intended to own the pointer, consider updating the doc to explicitly say that callers must not freemanagedWalletInfoafter wrapping it.Also applies to: 433-438
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/TransactionService.swift (1)
50-67: Prefer async SPV broadcast and reuse hex helper
broadcastTransaction(_:)currently:
- Converts bytes to hex manually.
- Calls the synchronous
SPVClient.broadcastTransaction(_:)from a@MainActorasync context.To avoid blocking the main thread and reduce duplication, consider:
- // Convert transaction bytes to hex string - let txHex = transaction.rawTransaction.map { String(format: "%02x", $0) }.joined() - - // Broadcast through SPV client - try await spvClient.broadcastTransaction(txHex) + // Convert transaction bytes to hex string + let txHex = transaction.rawTransaction.hexString + + // Broadcast through SPV client on a background task + try await spvClient.broadcastTransactionAsync(txHex)This uses the new async broadcasting API and the shared
Data.hexStringhelper for consistency with other call sites.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/WalletManager.swift (1)
771-787: Avoid swallowing SDK errors when fetching the FFI wallet
getFFIWallet(for:)wrapssdkWalletManager.getWalletintry?, so any underlyingKeyWalletErroris discarded and the caller only sees a genericWalletError.walletError("Unable to retrieve FFI wallet from SDK").To make failures diagnosable (especially around ID/network mismatches), consider:
- guard let ffiWallet = try? sdkWalletManager.getWallet(id: walletId, network: network) else { - throw WalletError.walletError("Unable to retrieve FFI wallet from SDK") - } - - return ffiWallet + do { + if let ffiWallet = try sdkWalletManager.getWallet(id: walletId, network: network) { + return ffiWallet + } + throw WalletError.walletError("Wallet not found in SDK wallet manager") + } catch { + throw WalletError.walletError("Unable to retrieve FFI wallet from SDK: \(error.localizedDescription)") + }Same pattern can be applied if you later want richer errors from
getManagedWallet(for:).packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift (1)
590-621: Align pointer binding with other call sites in getManagedWalletWithCurrentStateIn
getManagedWalletWithCurrentState, you still usebindMemory(to: UInt8.self).baseAddresswhereas other withUnsafeBytes sites have been normalized toidBytes.baseAddress:guard let managedInfo = walletId.withUnsafeBytes({ idBytes in let idPtr = idBytes.bindMemory(to: UInt8.self).baseAddress return wallet_manager_get_managed_wallet_info(handle, idPtr, &error) }) else { ... }For consistency and to avoid unnecessary binding, you can match the other helpers:
- guard let managedInfo = walletId.withUnsafeBytes({ idBytes in - let idPtr = idBytes.bindMemory(to: UInt8.self).baseAddress - return wallet_manager_get_managed_wallet_info(handle, idPtr, &error) - }) else { + guard let managedInfo = walletId.withUnsafeBytes({ idBytes in + let idPtr = idBytes.baseAddress + return wallet_manager_get_managed_wallet_info(handle, idPtr, &error) + }) else {That also makes it clearer that we’re just passing the raw 32‑byte buffer through, like other FFI calls. Combined with
ManagedWallet’s new initializer, this keeps the FFI access pattern uniform.packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (1)
954-1004: Concurrency pattern in broadcastTransactionAsync can mirror startSync for pointer safetyThe new broadcasting APIs are a good addition, but
broadcastTransactionAsynccurrently captures theUnsafeMutablePointer<FFIDashSpvClient>directly into aTask.detachedclosure:public func broadcastTransactionAsync(_ transactionHex: String) async throws { guard let client = client else { throw SPVError.notInitialized } let txHexCopy = transactionHex try await Task.detached { var txHexCString = txHexCopy let result = txHexCString.withCString { cstr in dash_spv_ffi_client_broadcast_transaction(client, cstr) } ... }.value }To better respect Swift’s concurrency rules around non‑Sendable pointers and match your
startSyncpattern, you can capture a numeric address and reconstruct the pointer in the detached task:- guard let client = client else { + guard let client = client else { throw SPVError.notInitialized } - let txHexCopy = transactionHex - try await Task.detached { + let clientAddr = UInt(bitPattern: client) + let txHexCopy = transactionHex + try await Task.detached { + guard let clientPtr = UnsafeMutablePointer<FFIDashSpvClient>(bitPattern: clientAddr) else { + throw SPVError.notInitialized + } var txHexCString = txHexCopy let result = txHexCString.withCString { cstr in - dash_spv_ffi_client_broadcast_transaction(client, cstr) + dash_spv_ffi_client_broadcast_transaction(clientPtr, cstr) } ... }.valueFunctionally identical, but it avoids capturing a main‑actor–isolated, non‑Sendable pointer directly across actor boundaries and keeps this code consistent with the rest of the file’s FFI patterns.
Also applies to: 1382-1413
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedWallet.swift(3 hunks)packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift(10 hunks)packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift(15 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift(2 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/SendTransactionView.swift(4 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/HDWallet.swift(1 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/TransactionService.swift(1 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/WalletManager.swift(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/HDWallet.swift
🧰 Additional context used
📓 Path-based instructions (3)
packages/swift-sdk/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Make DPP types public in Swift where needed for visibility
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/SendTransactionView.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedWallet.swiftpackages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/WalletManager.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/TransactionService.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/Views/SendTransactionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/WalletManager.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/TransactionService.swift
packages/swift-sdk/SwiftExampleApp/**/*View.swift
📄 CodeRabbit inference engine (packages/swift-sdk/SwiftExampleApp/CLAUDE.md)
packages/swift-sdk/SwiftExampleApp/**/*View.swift: Use SwiftUI with @query for reactive data in views
Break complex SwiftUI views into smaller components to avoid compiler timeouts
Use NavigationLink for drill-down navigation in the UI
Implement proper loading and error states in UI screens
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/SendTransactionView.swift
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: dashpay/platform PR: 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: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/Tests/**/*.swift : Use TestSigner for transaction signing in tests
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*View.swift : Implement proper loading and error states in UI screens
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/SendTransactionView.swift
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/Tests/**/*.swift : Use TestSigner for transaction signing in tests
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/SendTransactionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/TransactionService.swift
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*View.swift : Break complex SwiftUI views into smaller components to avoid compiler timeouts
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/SendTransactionView.swift
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/LocalDataContractsView*.swift : Use LocalDataContractsView to load data contracts from the network
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/SendTransactionView.swift
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
Repo: dashpay/platform PR: 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/Views/SendTransactionView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/WalletManager.swift
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
Repo: dashpay/platform PR: 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/Views/SendTransactionView.swift
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*.swift : Use Unified SDK functions with the dash_unified_sdk_* prefix
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/SendTransactionView.swift
📚 Learning: 2025-09-07T22:19:59.217Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-09-07T22:19:59.217Z
Learning: Store private keys in the iOS Keychain using KeychainManager
Applied to files:
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift
📚 Learning: 2024-11-25T07:48:09.831Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2345
File: packages/wallet-utils-contract/src/lib.rs:14-17
Timestamp: 2024-11-25T07:48:09.831Z
Learning: In the `wallet-utils-contract` (file `packages/wallet-utils-contract/src/lib.rs`), the `OWNER_ID_BYTES` constant is intentionally initialized to all zeros.
Applied to files:
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift
📚 Learning: 2025-10-09T15:59:28.329Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2716
File: packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs:181-187
Timestamp: 2025-10-09T15:59:28.329Z
Learning: In `packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs`, the maintainer requires logging full state transition bytes (`tx_bytes = hex::encode(st_bytes)`) at debug level when a state transition passes CheckTx but is removed from the block by the proposer, to facilitate debugging of this rare edge case.
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/TransactionService.swift
🧬 Code graph analysis (3)
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (7)
spvClient(802-910)spvClient(912-924)spvClient(926-942)spvClient(944-1007)spvClient(1009-1028)spvClient(1030-1032)error(78-80)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/TransactionService.swift (1)
broadcastTransaction(50-68)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Wallet/WalletManager.swift (3)
packages/js-evo-sdk/src/sdk.ts (1)
wallet(177-177)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Models/Network.swift (1)
toKeyWalletNetwork(36-45)packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift (2)
getWallet(201-224)getManagedWalletWithCurrentState(597-621)
🪛 SwiftLint (0.57.0)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift
[Warning] 784-784: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
[Warning] 784-784: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
[Warning] 799-799: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
[Warning] 645-645: TODOs should be resolved (Make this configurable or use ...)
(todo)
⏰ 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). (4)
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
- GitHub Check: Swift SDK and Example build (warnings as errors)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
🔇 Additional comments (5)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/SendTransactionView.swift (1)
96-101: UI transaction status and TXID display look solidThe updated progress message (“Building and broadcasting transaction…”) and success alert showing a truncated TXID improve clarity, and
isSendingis correctly reset on both success and failure paths. No issues from a correctness or UX standpoint.Also applies to: 117-119, 137-140
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (1)
695-705: Transaction history + SPV progress handling changes look correct
loadTransactions()now recomputes confirmations fromheaderCurrentHeightand updateshdTx.confirmations/isPendingaccordingly when both block height and tip height are known; this aligns UI state with current header sync without over‑eagerly modifying mempool entries.- The updated
spvClient(_:didUpdateSyncProgress:)moves heavy normalization and percentage math into a detached task while keeping only the final UI updates on theMainActor, which should significantly reduce main‑thread load and jitter.No functional issues spotted here; the structure matches the new SPV progress model in
SPVClient.Also applies to: 801-907
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (3)
783-815: Wallet‑scoped transaction events and walletId propagation look correctThe extended callbacks now:
- Distinguish between generic mempool events (
on_mempool_transaction_*withwalletId: nil) and wallet‑specific events viaon_wallet_transaction.- Parse
walletIdPtrinto a SwiftStringbefore hopping to theMainActor, then pass it through tohandleTransactionEvent(txid:confirmed:amount:addresses:blockHeight:walletId:), which in turn wraps it intoSPVTransactionEvent.The nil/non‑nil separation is clear and preserves the old behavior for callers that don’t care about wallet scoping, while enabling richer routing in the app. No issues here.
Also applies to: 907-918
525-543: Resetting blocksHit in clearStorage is a sensible state cleanupAfter
dash_spv_ffi_client_clear_storage, you now resetblocksHitto 0 along withisConnected,isSyncing,syncProgress, andlastError. That keeps the in‑memory metrics consistent with the wiped on‑disk state and avoids stale “blocks hit” counts after a full reset. Looks good.
31-37: FFIDetailedSyncProgress lifetime assumptions already documented and verifiedThe review's concern about FFIDetailedSyncProgress lifetime is already addressed in the codebase. ConcurrencyCompat.swift explicitly documents FFI value types as "plain C structs and treated as inert data blobs", and FFIDetailedSyncProgress is marked @unchecked Sendable to confirm this property. The implementation is safe: the callback immediately dereferences the pointer with
.pointee(line 93), creating a value copy that is then safely stored and dispatched up to 1 second later. Field access inhandleProgressUpdate(lines 1139–1180) confirms only scalar types are accessed (stage, heights, percentages, durations), with no pointer dereferences or heap data. No further verification needed.
Issue being fixed or feature implemented
bump dash-spv-ffi to use v0.41-dev
What was done?
How Has This Been Tested?
Built
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.