chore(swift-sdk): remove dash-spv-ffi crate usage, spv is wrapped by platform-wallet#3644
chore(swift-sdk): remove dash-spv-ffi crate usage, spv is wrapped by platform-wallet#3644ZocoLini wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe PR removes the ChangesReplace dash-spv-ffi with dash-network
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
✅ Review complete (commit 25c0e3d) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3644 +/- ##
============================================
- Coverage 88.25% 88.24% -0.01%
============================================
Files 2494 2506 +12
Lines 304580 307938 +3358
============================================
+ Hits 268812 271745 +2933
- Misses 35768 36193 +425
🚀 New features to boost your workflow:
|
|
✅ 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: "b58a3d0ea1a24481a957edca4a5b2048a3d0fae8d96948a230758b3cfe759dd8"
)Xcode manual integration:
|
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
This PR mostly does the intended surface cleanup: it removes direct dash-spv-ffi exposure and routes Swift through platform-wallet instead. The one confirmed regression is logging: the new SDK.enableLogging() path no longer enables the platform_wallet and dash_spv tracing targets, so SPV/platform-wallet diagnostics disappear for Swift callers. The other proposed FFI compatibility findings do not hold up as review comments on this SHA: one is an intentional breaking-surface removal, and the other is not introduced by this PR at all.
Reviewed commit: 25c0e3d
🟡 1 suggestion(s)
1 additional finding
🟡 suggestion: `dash_sdk_enable_logging` drops platform-wallet and SPV tracing targets
packages/rs-sdk-ffi/src/lib.rs (lines 103-106)
LoggingPreferences.configure() now calls SDK.enableLogging(level: .info) after initializeSPVLogging() was removed. In this implementation, the fallback EnvFilter only enables dash_sdk, rs_sdk, rs_sdk_ffi, dapi_grpc, h2, tower, hyper, and tonic. It does not enable platform_wallet or dash_spv, even though SPV is now wrapped by platform-wallet and those crates still emit tracing output. Because EnvFilter defaults unspecified targets to off, Swift clients that follow the new API lose the platform-wallet/SPV diagnostics they previously got from the old SPV logging initializer.
💡 Suggested change
let filter_string = format!(
"dash_sdk={log_level},rs_sdk={log_level},rs_sdk_ffi={log_level},\
platform_wallet={log_level},platform_wallet_ffi={log_level},dash_spv={log_level},\
dapi_grpc={log_level},h2={log_level},tower={log_level},\
hyper={log_level},tonic={log_level}"
);
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-sdk-ffi/src/lib.rs`:
- [SUGGESTION] lines 103-106: `dash_sdk_enable_logging` drops platform-wallet and SPV tracing targets
`LoggingPreferences.configure()` now calls `SDK.enableLogging(level: .info)` after `initializeSPVLogging()` was removed. In this implementation, the fallback `EnvFilter` only enables `dash_sdk`, `rs_sdk`, `rs_sdk_ffi`, `dapi_grpc`, `h2`, `tower`, `hyper`, and `tonic`. It does not enable `platform_wallet` or `dash_spv`, even though SPV is now wrapped by `platform-wallet` and those crates still emit tracing output. Because `EnvFilter` defaults unspecified targets to `off`, Swift clients that follow the new API lose the platform-wallet/SPV diagnostics they previously got from the old SPV logging initializer.
If we are planning to wrap all dash-spv logic in platform-wallet and platform-wallet-ffi, it makes more sense to remove this symbols from the final binary so nobody can use them
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Breaking Changes
initializeSPVLogging()public method from Swift SDK. Migrate code to useenableLogging(level:)for logging initialization instead.Refactor