feat(swift-sdk): use SPV-synced quorums for Platform proof verification#3417
Open
QuantumExplorer wants to merge 7 commits intov3.1-devfrom
Open
feat(swift-sdk): use SPV-synced quorums for Platform proof verification#3417QuantumExplorer wants to merge 7 commits intov3.1-devfrom
QuantumExplorer wants to merge 7 commits intov3.1-devfrom
Conversation
Bridge the SPV client's locally synced masternode list data to the Platform SDK's context provider, replacing the dependency on a trusted HTTP quorum endpoint. When an SPV client handle is available, the SDK is created with callback-based quorum lookups that call through to ffi_dash_spv_get_quorum_public_key and ffi_dash_spv_get_platform_activation_height. A "Fallback to Trusted Quorums" toggle (default: ON) in Settings lets users fall back to the HTTP provider when SPV data is not yet synced. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 42 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds SPV-backed quorum support across Rust FFI, platform wallet, Swift SDK, and example app: new SPV context providers and FFI entrypoint, Swift exposures of raw SPV client handles, an SDK initializer accepting that handle, UI toggle and app-state fallback logic, and Cargo feature/dependency updates. Changes
Sequence DiagramssequenceDiagram
participant UI as App (Swift UI)
participant AppState as AppState
participant Wallet as WalletService
participant SDK as Swift SDK
participant FFI as Rust FFI
participant Spv as SPV Provider
UI->>AppState: initializeSDK(spvClientHandle?)
AppState->>Wallet: request spvClientHandle
Wallet-->>AppState: spvClientHandle
alt spvClientHandle present
AppState->>SDK: init(network, spvClientHandle)
SDK->>FFI: dash_sdk_create_with_spv_context(config, handle)
FFI->>Spv: create SpvContextProvider(handle)
Spv-->>FFI: provider constructed
FFI->>FFI: dash_sdk_create_extended(config with provider)
FFI-->>SDK: result (handle or error)
SDK-->>AppState: initialized / throws
else fallback allowed
AppState->>SDK: init(network) (trusted)
SDK->>FFI: dash_sdk_create_trusted(config)
FFI-->>SDK: result
SDK-->>AppState: initialized
else no fallback
AppState-->>UI: error / throw SDKError.invalidState
end
AppState-->>UI: SDK ready or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Collaborator
Review GateCommit:
|
Contributor
|
✅ 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: "93f085d7f665b623b4cc170feb6c8fc04da6702b601f887c52f8e5181e471994"
)Xcode manual integration:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/AppState.swift`:
- Around line 45-49: The published property useTrustedQuorumFallback currently
only persists the flag; update its didSet to immediately reapply the SDK
proof-verification mode so the running app reflects the change. After
UserDefaults.standard.set(...), invoke the function that configures the SDK
trust mode (for example the AppState method that initializes or configures the
SDK client—if you have a method like applySDKMode(), reconfigureClient(), or
setTrustedQuorumFallback(on:)), passing the new useTrustedQuorumFallback value;
if no such function exists, call the code path that reconnects or reinitializes
the SDK client so proof verification is updated immediately. Ensure you
reference AppState and useTrustedQuorumFallback when wiring this change.
In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/UnifiedAppState.swift`:
- Around line 126-133: The current network-switch flow lets
walletService.switchNetwork(to:) destroy the SPV client while platformState.sdk
still holds callbacks to the old core handle; update
UnifiedAppState.handleNetworkSwitch to explicitly detach and destroy the old SDK
before tearing down the SPV client: capture the existing platformState.sdk (or
call a teardown method like
platformState.sdk?.destroy()/platformState.clearSDK()), set platformState.sdk =
nil (or call the SDK's close/unbind) on the MainActor to remove callbacks, then
call walletService.switchNetwork(to:) and only after obtaining
walletService.spvClientHandle recreate and assign the new SDK via
platformState.updateSPVClientHandle/new SDK creation — ensuring the old SDK is
fully unbound before SPV teardown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed48fb63-d5a9-41ff-b965-9df30eae9aab
📒 Files selected for processing (7)
packages/swift-sdk/Sources/SwiftDashSDK/Core/SPV/SPVClient.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Core/SPV/SPVContextProvider.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Core/Services/WalletService.swiftpackages/swift-sdk/Sources/SwiftDashSDK/SDK.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/AppState.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/UnifiedAppState.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/OptionsView.swift
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/UnifiedAppState.swift
Outdated
Show resolved
Hide resolved
The quorum/activation-height bridging no longer round-trips through Swift callbacks. Instead, rs-sdk-ffi now depends on dash-spv-ffi directly and implements SpvContextProvider in Rust, calling ffi_dash_spv_get_quorum_public_key and ffi_dash_spv_get_platform_activation_height without leaving Rust. A new FFI function dash_sdk_create_with_spv_context(config, spv_client) replaces the Swift-side ContextProviderCallbacks setup. Swift just passes the raw FFIDashSpvClient pointer. - Add dash-spv-ffi dependency to rs-sdk-ffi - Add spv_context_provider.rs implementing ContextProvider trait - Add dash_sdk_create_with_spv_context FFI function - Remove SPVContextProvider.swift (no longer needed) - Update SDK.swift to call dash_sdk_create_with_spv_context Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3417 +/- ##
============================================
- Coverage 79.93% 79.90% -0.03%
============================================
Files 2861 2862 +1
Lines 280236 280312 +76
============================================
- Hits 223997 223993 -4
- Misses 56239 56319 +80
🚀 New features to boost your workflow:
|
Add SpvContextProvider to platform-wallet behind the `spv-context` feature flag. It holds Arc<RwLock<MasternodeListEngine>> + Network and implements the ContextProvider trait by reading quorum data directly from the in-memory masternode list — zero FFI involvement. The rs-sdk-ffi bridge (which calls dash-spv-ffi functions as Rust calls in the same binary) remains as the pragmatic adapter until dash-spv-ffi exposes a public accessor for FFIDashSpvClient's MasternodeListEngine (currently pub(crate)). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rs-platform-wallet/Cargo.toml`:
- Line 25: The tokio dependency must enable its sync feature for the spv-context
feature and the feature list must use the modern dep: syntax; update Cargo.toml
so the tokio dependency becomes optional with features = ["sync"] (e.g. tokio =
{ version = "1.41", optional = true, features = ["sync"] }) and update the
spv-context feature entry to include "dep:tokio/sync" (instead of the old name)
so spv_context_provider.rs can safely use tokio::sync::RwLock.
In `@packages/rs-platform-wallet/src/spv_context_provider.rs`:
- Around line 115-120: The match that sets `height` currently falls through to
`_ => 0`, silently mapping unsupported `self.network` to 0; replace that
fallback with an explicit error return (e.g., return Err(...) or propagate a
suitable error) so callers are notified of an unsupported Network variant.
Modify the block that computes `height` (the match on `self.network` referencing
`Network::Mainnet`, `Network::Testnet`, `Network::Devnet`) to return a
Result<u64, E> (or use the crate's existing error type) and produce a clear
error for the unsupported branch instead of using 0. Ensure callers of the
enclosing function are updated to handle the Result if necessary.
- Around line 79-80: The call to self.masternode_engine.blocking_read() inside
get_quorum_public_key (which is invoked from async
parse_proof_with_metadata_and_proof) can panic; replace blocking_read() with
try_read() on the Tokio RwLock (self.masternode_engine.try_read()) and handle
the None case gracefully by returning an appropriate error or early failure from
get_quorum_public_key (or falling back to a safe default) instead of unwrapping.
Ensure you reference and use the masternode_lists_around_height result from the
acquired guard when present, and propagate a clear error back to the async proof
verification path so it doesn’t panic.
- Around line 76-77: Validate and reject out-of-range quorum_type before
converting to LLMQType: replace the lossy cast "(quorum_type as u8).into()" with
a fallible conversion using TryInto (e.g., let llmq_type: LLMQType =
(quorum_type.try_into()?) or map the TryInto error to a meaningful error) so
values >255 are handled instead of wrapping; keep
QuorumHash::from_byte_array(quorum_hash) as-is and ensure any error returned
propagates to the caller before using ml.quorums.get(&llmq_type).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0b462740-73cb-43f3-9a1b-7653a611fe5e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
packages/rs-platform-wallet/Cargo.tomlpackages/rs-platform-wallet/src/lib.rspackages/rs-platform-wallet/src/spv_context_provider.rspackages/rs-sdk-ffi/src/spv_context_provider.rs
✅ Files skipped from review due to trivial changes (1)
- packages/rs-platform-wallet/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-sdk-ffi/src/spv_context_provider.rs
- AppState: trigger SDK reconnect when useTrustedQuorumFallback changes so the running app reflects the new setting immediately - UnifiedAppState: nil out old SDK before destroying SPV client during network switch to prevent use-after-free on the context provider's core_handle pointer - Cargo.toml: explicitly enable tokio "sync" feature for blocking_read() support - spv_context_provider.rs: replace lossy `as u8` cast with try_from + error, add comment explaining why blocking_read is safe, return error for unsupported networks instead of silent 0 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bump rust-dashcore to 3f65002 which includes the FFIDashSpvClient::masternode_list_engine() accessor (PR #608). dash_sdk_create_with_spv_context now: 1. Casts the void* to FFIDashSpvClient 2. Extracts Arc<RwLock<MasternodeListEngine>> directly 3. Creates platform_wallet::SpvContextProvider (pure Rust) The FFI bridge module (rs-sdk-ffi/src/spv_context_provider.rs) is deleted — quorum lookups go directly from the Platform SDK through the pure-Rust provider to the in-memory masternode list, with zero FFI involvement in the data path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Member
Author
|
All review feedback addressed. Additionally, bumped rust-dashcore to |
Contributor
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/SwiftExampleApp/SwiftExampleApp/AppState.swift (1)
215-267:⚠️ Potential issue | 🟠 MajorMake the network switch commit atomically.
The new SPV-first failure paths can throw after the collections are cleared and
dataManager.currentNetworkis advanced, but beforesdkis replaced. On that path the UI says “new network” while platform calls still go through the old SDK instance. IfcurrentNetworkshould also be authoritative, it needs the same success-only commit behavior.♻️ Possible fix
func switchNetwork(to network: AppNetwork) async { guard let modelContext = modelContext else { return } - - // Clear current data - identities.removeAll() - contracts.removeAll() - documents.removeAll() - tokens.removeAll() - - // Update DataManager's current network - dataManager?.currentNetwork = network // Re-initialize SDK with new network do { isLoading = true @@ - sdk = newSDK + identities.removeAll() + contracts.removeAll() + documents.removeAll() + tokens.removeAll() + dataManager?.currentNetwork = network + sdk = newSDK🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/AppState.swift` around lines 215 - 267, The switchNetwork method mutates state (clearing identities/contracts/documents/tokens and advancing dataManager?.currentNetwork) before potentially failing during SDK creation and load; make the switch atomic by first creating and initializing the new SDK and loading contracts/data (use local temporaries like let newSDK and temp collections), and only after loadKnownContractsIntoSDK and loadPersistedData succeed assign sdk = newSDK, set dataManager?.currentNetwork = network, and replace identities/contracts/documents/tokens with the newly loaded data; keep all error-throwing work (SDK(network:), loadKnownContractsIntoSDK, loadPersistedData) before mutating shared state so failures leave the existing sdk and currentNetwork unchanged.
🧹 Nitpick comments (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/AppState.swift (1)
104-124: Extract the SPV/trusted SDK creation path into one helper.The same fallback tree now lives in both
initializeSDKandswitchNetwork. The next change to error handling or provider selection is easy to land in one path and miss in the other.Also applies to: 234-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/AppState.swift` around lines 104 - 124, Extract the SPV/trusted SDK creation logic used in initializeSDK and switchNetwork into a single helper function (e.g., makeSDKForNetwork(network:sdkNetwork, spvHandle:spvClientHandle?, useFallback:useTrustedQuorumFallback) -> SDK) that encapsulates the fallback tree: try constructing SDK(network: , spvClientHandle:) if spvHandle != nil, on failure fall back to SDK(network:) only if useFallback is true, otherwise rethrow; if spvHandle is nil create SDK(network:) when useFallback is true or throw SDKError.invalidState when false. Replace the duplicated blocks in initializeSDK and switchNetwork to call this new helper and preserve the same NSLog messages and error propagation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/AppState.swift`:
- Around line 143-146: The race occurs because updateSPVClientHandle(_:) runs in
handleNetworkSwitch() while switchNetwork(...) can be started asynchronously
from the currentNetwork didSet in OptionsView; modify handleNetworkSwitch() so
that after calling updateSPVClientHandle(_:) it immediately triggers/awaits the
SDK rebuild (call switchNetwork(...) directly or await the spawned Task) to
ensure the SDK is rebuilt with the new spvClientHandle before returning;
reference updateSPVClientHandle(_:), handleNetworkSwitch(), switchNetwork(...),
currentNetwork didSet/OptionsView to locate and change the code paths so the
handle update is always followed synchronously by the SDK rebuild.
---
Outside diff comments:
In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/AppState.swift`:
- Around line 215-267: The switchNetwork method mutates state (clearing
identities/contracts/documents/tokens and advancing dataManager?.currentNetwork)
before potentially failing during SDK creation and load; make the switch atomic
by first creating and initializing the new SDK and loading contracts/data (use
local temporaries like let newSDK and temp collections), and only after
loadKnownContractsIntoSDK and loadPersistedData succeed assign sdk = newSDK, set
dataManager?.currentNetwork = network, and replace
identities/contracts/documents/tokens with the newly loaded data; keep all
error-throwing work (SDK(network:), loadKnownContractsIntoSDK,
loadPersistedData) before mutating shared state so failures leave the existing
sdk and currentNetwork unchanged.
---
Nitpick comments:
In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/AppState.swift`:
- Around line 104-124: Extract the SPV/trusted SDK creation logic used in
initializeSDK and switchNetwork into a single helper function (e.g.,
makeSDKForNetwork(network:sdkNetwork, spvHandle:spvClientHandle?,
useFallback:useTrustedQuorumFallback) -> SDK) that encapsulates the fallback
tree: try constructing SDK(network: , spvClientHandle:) if spvHandle != nil, on
failure fall back to SDK(network:) only if useFallback is true, otherwise
rethrow; if spvHandle is nil create SDK(network:) when useFallback is true or
throw SDKError.invalidState when false. Replace the duplicated blocks in
initializeSDK and switchNetwork to call this new helper and preserve the same
NSLog messages and error propagation behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1d056420-6f71-49b1-ad49-3434618ac7b3
📒 Files selected for processing (4)
packages/rs-platform-wallet/Cargo.tomlpackages/rs-platform-wallet/src/spv_context_provider.rspackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/AppState.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/UnifiedAppState.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/rs-platform-wallet/Cargo.toml
- packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/UnifiedAppState.swift
- packages/rs-platform-wallet/src/spv_context_provider.rs
Remove the switchNetwork() call from currentNetwork.didSet to prevent it racing with UnifiedAppState.handleNetworkSwitch(). The didSet now only persists the preference. handleNetworkSwitch() is the single coordinator: it nils the old SDK, switches the wallet, updates the SPV handle, then calls switchNetwork() to rebuild the SDK. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace blocking_read() with try_read() in SpvContextProvider to avoid panicking when called from within a Tokio async context (proof verification runs inside async tasks) - Use dep: prefix syntax in spv-context feature to suppress implicit feature creation for optional dependencies (Cargo 2021 edition) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
The iOS app currently relies on a
TrustedHttpContextProviderthat fetches quorum public keys from a centralized HTTP endpoint (quorums.testnet.networks.dash.org). This means the app trusts a centralized server, doesn't work offline, and can't work for regtest/local environments where no endpoint exists.This PR bridges the SPV client's locally synced masternode list data to the Platform SDK's context provider, enabling trustless proof verification using quorum data already synced by the Core SPV client.
What was done?
Pure Rust:
platform-wallet+rs-sdk-ffiplatform-wallet/spv_context_provider.rs(new) —SpvContextProviderholdingArc<RwLock<MasternodeListEngine>>+Network, implementingContextProviderby reading quorum data directly from the in-memory masternode list engine. Zero FFI. Gated behindspv-contextfeature flag.rs-sdk-ffi/sdk.rs—dash_sdk_create_with_spv_context(config, spv_client)casts theFFIDashSpvClient*, extracts theMasternodeListEnginevia the new public accessor (rust-dashcore PR fix(dashmate): invalid testnet TenderDash genesis #608), and creates the pure-Rust provider. No FFI bridge module needed.rust-dashcoreto3f65002which includesFFIDashSpvClient::masternode_list_engine().Swift SDK
SPVClient.swift— AddedunsafeFFIClientPointerto expose theFFIDashSpvClient*.SDK.swift— Addedinit(network:spvClientHandle:)callingdash_sdk_create_with_spv_context.WalletService.swift— AddedspvClientHandlecomputed property.SwiftExampleApp
AppState.swift— AddeduseTrustedQuorumFallbacksetting (default ON), rewroteinitializeSDK/switchNetworkto try SPV quorums first with trusted fallback. Toggle triggers SDK reconnect immediately.UnifiedAppState.swift— PasseswalletService.spvClientHandleto SDK init; nils out old SDK before SPV client teardown on network switch to prevent use-after-free.OptionsView.swift— Added "Fallback to Trusted Quorums" toggle.Architecture
Swift only passes the raw
FFIDashSpvClient*pointer once at SDK creation. All quorum lookups stay entirely in Rust, reading directly from the same in-memory masternode list engine that the SPV client populates during sync.How Has This Been Tested?
cargo check -p rs-sdk-ffipassescargo check -p platform-wallet --features spv-contextpassesBreaking Changes
None. The existing
SDK.init(network:)trusted initializer remains unchanged. The new SPV path is additive and the trusted fallback defaults to ON.Checklist: