chore: bump dash-spv-ffi to v0.41-dev branch #2852
Conversation
WalkthroughMultiple Swift SDK and example-app changes: new wallet sync scheduler and batched SPV progress handling, added build-and-sign and getTxid FFI helpers, a ManagedWallet visibility tweak, and a Rust FFI Cargo dependency moved from a fixed tag to a development branch. Changes
Sequence Diagram(s)sequenceDiagram
participant SPV as SPV C callback thread
participant Dispatcher as SPVProgressDispatcher (actor)
participant CallbackCtx as CallbackContext
participant Main as MainActor / UI
SPV->>Dispatcher: enqueue(progress)
Note right of Dispatcher `#E6F7FF`: coalesces latest progress\nand schedules dispatch
Dispatcher-->>Main: dispatch(coalescedProgress)
Main->>CallbackCtx: handleProgressUpdate(progress)
CallbackCtx->>Main: update UI (throttled)
sequenceDiagram
participant WalletService
participant Scheduler as WalletSyncScheduler (actor)
participant WalletManager
participant FFI as SwiftDashSDK (ManagedWallet/Wallet)
participant Signing as Transaction.buildAndSign
WalletService->>Scheduler: enqueue(walletId)
Scheduler-->>WalletService: after debounce -> flush(ids)
WalletService->>WalletManager: getManagedWallet(for: id)
WalletManager->>FFI: getFFIWallet(...) / construct ManagedWallet
WalletService->>Signing: buildAndSign(managedWallet, outputs...)
Signing-->>WalletService: signedTx (raw bytes)
WalletService->>WalletService: record pending HDTransaction (not broadcast)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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)
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