refactor: consolodate Network structs and enum variants into one#3567
Merged
QuantumExplorer merged 1 commit intov3.1-devfrom May 1, 2026
Merged
refactor: consolodate Network structs and enum variants into one#3567QuantumExplorer merged 1 commit intov3.1-devfrom
QuantumExplorer merged 1 commit intov3.1-devfrom
Conversation
Contributor
📝 WalkthroughWalkthroughReplaces multiple local FFI network enums and raw numeric network parameters with a single workspace-provided Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
ZocoLini
force-pushed
the
refactor/network-ffi
branch
2 times, most recently
from
2f13c84 to
7dc2eaf
Compare
Collaborator
|
🕓 Ready for review (commit fa100d8) |
![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: 7
🤖 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-ffi/src/identity_keys_from_mnemonic.rs`:
- Around line 98-105: The conversion from FFINetwork to Network must explicitly
normalize local/regtest to testnet to preserve previous WIF bytes: change the
kw_network assignment where you do let kw_network: Network = network.into()
(used by ExtendedPrivKey::new_master() and to_wif()) to map Network::Regtest =>
Network::Testnet (e.g., let kw_network = match network.into() { Network::Regtest
=> Network::Testnet, n => n }), so the DERIVATION path and WIF version selection
remain compatible with existing local/dev fixtures; add a brief unit/regression
test asserting the WIF output for a regtest FFINetwork equals the testnet WIF to
lock this behavior.
In `@packages/rs-platform-wallet-ffi/src/platform_wallet_info.rs`:
- Around line 12-13: Several extern "C" functions (e.g., the FFINetwork param in
platform_wallet_info.rs and other files) take FFINetwork by value which is
unsafe if callers pass invalid discriminants; change the signature to accept a
primitive (u8) for the network parameter, validate it explicitly (map or
try_from the u8 into FFINetwork), and return the existing ErrorInvalidNetwork
when conversion fails instead of relying on implicit conversion. Update all
affected functions and callers (e.g., where FFINetwork was used in manager.rs,
identity_keys_from_mnemonic.rs, derive_identity_key_at_slot.rs, derivation.rs,
spv.rs, persistence.rs) to accept u8, perform the validation at the FFI
boundary, and only use the safe FFINetwork enum internally after successful
validation. Ensure tests and any helper constructors/FFI wrappers are updated to
construct/validate the enum from u8 to avoid UB.
In `@packages/swift-sdk/Sources/SwiftDashSDK/DashNetwork.swift`:
- Around line 19-26: The initializer public init(ffiNetwork: FFINetwork)
currently maps unknown FFINetwork values to .mainnet; instead make the failure
explicit by turning it into a failable initializer (change to public
init?(ffiNetwork: FFINetwork)) and replace the default case with return nil, so
unknown numeric values do not silently coerce to .mainnet; update callers to
handle the optional result. Target the initializer named init(ffiNetwork:) in
DashNetwork (DashNetwork.swift) when making this change.
In
`@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- Line 1421: The code currently defaults a missing persistentWallet.network to
.testnet when deriving keys; instead, fail fast: in the method on
PlatformWalletPersistenceHandler where you compute "let network: Network =
persistentWallet.network?.network ?? .testnet", replace the silent fallback with
explicit handling (e.g., guard/if let) so that if persistentWallet.network or
its .network is nil you return/throw a descriptive error (or call the method's
failure callback) rather than continuing with .testnet; update callers or error
types accordingly to propagate this new error state.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/AddIdentityKeyView.swift`:
- Line 305: The code currently falls back to a hardcoded .testnet when
appState.sdk is nil (let network = appState.sdk?.network ?? .testnet); change
the fallback to use the appState's selected network instead so
derivation/validation always uses the UI's current network (e.g., let network =
appState.sdk?.network ?? appState.network or appState.selectedNetwork depending
on the property name). Apply the same change to the other occurrences referenced
(the blocks around lines 333-337 and 349-354) so all places use appState's
network as the fallback rather than .testnet; update any variable names
(network, appState, sdk) accordingly.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/AddressQueriesView.swift`:
- Around line 218-221: The code in AddressQueriesView currently infers Network
from the text in viewModel.addressInput using a hasPrefix heuristic which can
mis-classify addresses; replace that logic so the UI uses the SDK's active
network (the app's network state) rather than parsing the address string.
Concretely, in the places where you set let network: Network = ... (the lines
using viewModel.addressInput.lowercased().hasPrefix("tdashevo")), remove the
string-prefix check and read the active network from the SDK/app state (for
example a viewModel.network or the SDK’s Network.current/active accessor) so
bech32m rendering always matches the SDK’s selected network.
In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/KeysListView.swift`:
- Around line 459-463: The code falls back to .testnet when appState.sdk is nil,
causing incorrect WIF prefixes; update the fallback to use the app's selected
network instead. In KeysListView.swift change the network selection so it reads
the SDK network or the app's selected network (e.g., let network =
appState.sdk?.network ?? appState.network) before calling
WIFParser.encodeToWIF(privateKeyData, network: network) so exported WIF uses the
active app network when the SDK is unavailable.
🪄 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: 7b4d289c-bef1-4abe-b89b-568c30648f42
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (73)
Cargo.tomlpackages/rs-platform-wallet-ffi/Cargo.tomlpackages/rs-platform-wallet-ffi/src/core_wallet/wallet.rspackages/rs-platform-wallet-ffi/src/derivation.rspackages/rs-platform-wallet-ffi/src/derive_identity_key_at_slot.rspackages/rs-platform-wallet-ffi/src/identity_derive_and_persist.rspackages/rs-platform-wallet-ffi/src/identity_keys_from_mnemonic.rspackages/rs-platform-wallet-ffi/src/manager.rspackages/rs-platform-wallet-ffi/src/persistence.rspackages/rs-platform-wallet-ffi/src/platform_addresses/fund_from_asset_lock.rspackages/rs-platform-wallet-ffi/src/platform_wallet_info.rspackages/rs-platform-wallet-ffi/src/sign_with_mnemonic_resolver.rspackages/rs-platform-wallet-ffi/src/spv.rspackages/rs-platform-wallet-ffi/src/types.rspackages/rs-platform-wallet-ffi/src/wallet.rspackages/rs-platform-wallet-ffi/src/wallet_restore_types.rspackages/rs-platform-wallet-ffi/tests/integration_tests.rspackages/rs-sdk-ffi/Cargo.tomlpackages/rs-sdk-ffi/src/address/mod.rspackages/rs-sdk-ffi/src/crypto/mod.rspackages/rs-sdk-ffi/src/identity/top_up_from_addresses.rspackages/rs-sdk-ffi/src/sdk.rspackages/rs-sdk-ffi/src/signer_simple.rspackages/rs-sdk-ffi/src/system/queries/platform_status.rspackages/rs-sdk-ffi/src/system/status.rspackages/rs-sdk-ffi/src/token/claim.rspackages/rs-sdk-ffi/src/token/emergency_action.rspackages/rs-sdk-ffi/src/token/freeze.rspackages/rs-sdk-ffi/src/types.rspackages/rs-sdk-ffi/test_header.hpackages/rs-sdk-ffi/tests/context_provider_test.rspackages/rs-unified-sdk-ffi/Cargo.tomlpackages/rs-unified-sdk-ffi/src/lib.rspackages/swift-sdk/Sources/SwiftDashSDK/Address/Addresses.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Address/PlatformAddressInfo.swiftpackages/swift-sdk/Sources/SwiftDashSDK/DashNetwork.swiftpackages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Helpers/WIFParser.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Address.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyDerivation.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyManager.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyWalletTypes.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedPlatformAccount.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Wallet.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Models/Network.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/ManagedCoreWallet.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/DashPayService.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWallet.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerSPV.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletTypes.swiftpackages/swift-sdk/Sources/SwiftDashSDK/SDK.swiftpackages/swift-sdk/Sources/SwiftDashSDK/SwiftDashSDK.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Utils/KeyValidation.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Utils/PrivateKeyUtils.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/AppState.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/ContentView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Models/DashAddress.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CreateWalletView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/SwiftExampleAppApp.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/AddIdentityKeyView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/AddressQueriesView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/KeysListView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/LoadIdentityView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/SDKMethodTests.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/SimpleTransitionTests.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/StateTransitionTests.swift
💤 Files with no reviewable changes (3)
- packages/rs-sdk-ffi/test_header.h
- packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyWalletTypes.swift
- packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletTypes.swift
ZocoLini
force-pushed
the
refactor/network-ffi
branch
from
7dc2eaf to
a38f114
Compare
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: "04921aebea18f186f3fa906d984957c544d6e29d85ac39c7a04053d49e44cd3a"
)Xcode manual integration:
|
ZocoLini
force-pushed
the
refactor/network-ffi
branch
from
a38f114 to
f518fb6
Compare
ZocoLini
force-pushed
the
refactor/network-ffi
branch
from
f518fb6 to
fa100d8
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift (1)
1400-1506:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMove identity-key derivation back behind Rust FFI boundary.
This method still performs mnemonic → seed → path-building → private-key derivation in Swift, which violates the SDK boundary rules and keeps sensitive key-derivation logic on the Swift side. Swift should only persist Rust-emitted
(path_string, 32_private_key_bytes)to Keychain.As per coding guidelines: “Do not orchestrate multi-step derivation pipelines in Swift …”, “Do not build derivation paths in Swift”, and “Accept (path_string, 32_private_key_bytes) from Rust FFI calls and write directly to Keychain as the only allowed Swift-side persistence step after Rust key derivation.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift` around lines 1400 - 1506, The deriveAndStoreIdentityKey function is doing mnemonic→seed→path→private-key derivation in Swift (calls to WalletStorage.retrieveMnemonicUTF8Bytes, Mnemonic.toSeed, KeyDerivation.getIdentityAuthenticationPath and key_wallet_derive_private_key_from_seed); move this logic behind the Rust FFI by calling a single FFI entry (export a Rust function that takes walletId/indices/network and returns (path_string, 32_byte_private_key) or an error) and then change deriveAndStoreIdentityKey to only call that FFI, validate the returned (path, key) tuple, and pass the key and returned path directly into KeychainManager.shared.storeIdentityPrivateKey with the existing metadata; remove Swift-side path building, seed handling, and FFI derive loops so Swift only persists Rust-emitted path+private key.packages/rs-platform-wallet-ffi/src/spv.rs (2)
208-209:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate SPV start docs to match typed
FFINetworkparameter.The comment still describes raw numeric codes even though the function now accepts
FFINetwork.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-platform-wallet-ffi/src/spv.rs` around lines 208 - 209, Update the doc comment for the SPV start function to reference the typed FFINetwork enum instead of raw numeric codes: state that the parameter is an FFINetwork (variants: Mainnet, Testnet, Devnet, Regtest) and remove the "0/1/2/3" numeric mapping; keep the note about user_agent being optional (null uses default). Locate the comment above the SPV start function that takes an FFINetwork and replace the numeric description with the enum-variant description.
223-252:⚠️ Potential issue | 🟠 MajorAdd validation or error handling for the network parameter at the FFI boundary.
The conversion at line 251 (
let net: crate::types::Network = network.into();) is unconditional and has no error handling. Although anErrorInvalidNetworkvariant exists, theout_errorparameter is explicitly ignored. If the externaldash_networkcrate'sFrom<FFINetwork>implementation cannot validate invalid discriminants passed from C, this bypasses safety guarantees. Consider either:
- Validating the raw network discriminant before conversion
- Using
TryFromand propagating errors viaout_error- Documenting that invalid networks will panic
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-platform-wallet-ffi/src/spv.rs` around lines 223 - 252, The FFI function platform_wallet_manager_spv_start currently performs an unconditional conversion let net: crate::types::Network = network.into() which can accept invalid discriminants; update this to validate the incoming FFINetwork before converting and return a proper FFI error via out_error instead of allowing a silent invalid conversion or panic. Specifically, replace the direct into() with a guarded conversion (e.g., check the raw discriminant or use a TryFrom/TryInto for FFINetwork -> Network), and on failure set *out_error = PlatformWalletFFIError::ErrorInvalidNetwork (or the appropriate variant) and return PlatformWalletFFIResult::Error; ensure you reference platform_wallet_manager_spv_start, FFINetwork, out_error and PlatformWalletFFIError in the change so invalid network values are rejected at the boundary.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/ContentView.swift (1)
264-269:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHardcoded recovery network can recreate wallets on the wrong chain
Line 264 hardcodes
.testnet, so recovery can re-create a wallet under the wrong network when the app is on mainnet/devnet/regtest. Use the active app network (or persisted wallet network metadata, if available).Suggested fix
- let platformNetwork: Network = .testnet + let platformNetwork: Network = platformState.currentNetwork🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/ContentView.swift` around lines 264 - 269, The code hardcodes platformNetwork = .testnet before calling walletManager.createWallet(mnemonic: mnemonic, network: platformNetwork, name: label), which can recreate wallets on the wrong chain; instead read the app's active network (or the persisted wallet network metadata if available) and pass that into walletManager.createWallet. Replace the hardcoded platformNetwork assignment with a call to the app/network provider (e.g., currentNetwork, AppConfig.network, or loadSavedWalletNetwork(for: mnemonic)) and use that value for the network parameter so recovered wallets are created on the correct chain.
♻️ Duplicate comments (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/AddIdentityKeyView.swift (1)
305-305:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse current app network instead of hardcoded
.testnetfallbackLine 305 can run derivation + validation on the wrong network when
appState.sdkis nil. Fall back to the selected app network to keep key material/network context consistent.Suggested fix
- let network = appState.sdk?.network ?? .testnet + let network = appState.sdk?.network ?? appState.currentNetworkAlso applies to: 333-337, 349-354
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/AddIdentityKeyView.swift` at line 305, The code in AddIdentityKeyView uses "let network = appState.sdk?.network ?? .testnet" which falls back to a hardcoded .testnet when appState.sdk is nil; change the fallback to the app's selected network (e.g., use "?? appState.network" or the appropriate appState.selectedNetwork property) so derivation/validation uses the current app network; replace the same pattern at the other occurrences (around the blocks at the ranges noted: 333–337 and 349–354) so all uses in AddIdentityKeyView consistently fall back to the app's network instead of .testnet.
🧹 Nitpick comments (1)
packages/rs-sdk-ffi/src/system/queries/platform_status.rs (1)
63-63: Consider explicit network mapping for defensive API stability.The current code uses
sdk.network.to_string()wheresdk.networkisdashcore::Network. While theDisplayimplementation is stable and produces canonical names ("bitcoin", "testnet3", "regtest", "signet"), explicit mapping would decouple this JSON field from upstream implementation details. This is a defensive pattern for public API contracts but not critical given the current stability of the underlying enum.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk-ffi/src/system/queries/platform_status.rs` at line 63, Replace the direct to_string() usage for sdk.network with an explicit match that maps dashcore::Network variants to the canonical API strings (e.g., Network::Bitcoin => "bitcoin", Testnet => "testnet3", Regtest => "regtest", Signet => "signet"), assign the result to network_str, and include a fallback arm (e.g., unreachable!() or a default) if new variants are added to force compile-time awareness; locate the usage of sdk.network and the variable network_str in platform_status.rs and implement the match there to decouple the JSON field from dashcore::Network's Display implementation.
🤖 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-sdk-ffi/src/address/mod.rs`:
- Around line 30-34: Change the extern "C" signature of
dash_sdk_encode_platform_address to accept a plain u32 for the network (e.g.,
network: u32) instead of FFINetwork, then validate/convert that u32 to
FFINetwork inside the function using a safe match or TryFrom (e.g., 0 =>
FFINetwork::Mainnet, etc.) and return an Err DashSDKResult for any invalid
discriminant; update the subsequent conversion site where .into() was used to
use the validated enum value. Apply the same pattern to all other FFI entry
points that currently take FFINetwork directly so the C boundary defensively
checks the u32 and rejects invalid codes rather than invoking undefined
behavior.
In
`@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- Line 2061: In PlatformWalletPersistenceHandler (during wallet restore/export),
stop coercing missing w.network to .testnet; instead check if w.network is nil
and either skip that wallet or surface an error rather than assigning a
default—i.e., only set entry.network = w.network.ffiValue when w.network != nil,
and handle the nil case by returning/failing fast or continuing (skip) so you
don't silently rehydrate onto the wrong network.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CreateWalletView.swift`:
- Around line 283-290: selectedNetworks is omitting createForRegtest, causing
regtest selection to never be resolved; update the array in the CreateWalletView
where selectedNetworks is built to include createForRegtest (respecting
shouldShowDevnet logic if applicable) so regtest appears in the compactMap, and
also update the hasNetworkSelected computed property to include createForRegtest
(e.g., createForMainnet || createForTestnet || createForRegtest ||
createForDevnet) so both selection resolution and validation align with
setupInitialNetworkSelection and the regtest state.
---
Outside diff comments:
In `@packages/rs-platform-wallet-ffi/src/spv.rs`:
- Around line 208-209: Update the doc comment for the SPV start function to
reference the typed FFINetwork enum instead of raw numeric codes: state that the
parameter is an FFINetwork (variants: Mainnet, Testnet, Devnet, Regtest) and
remove the "0/1/2/3" numeric mapping; keep the note about user_agent being
optional (null uses default). Locate the comment above the SPV start function
that takes an FFINetwork and replace the numeric description with the
enum-variant description.
- Around line 223-252: The FFI function platform_wallet_manager_spv_start
currently performs an unconditional conversion let net: crate::types::Network =
network.into() which can accept invalid discriminants; update this to validate
the incoming FFINetwork before converting and return a proper FFI error via
out_error instead of allowing a silent invalid conversion or panic.
Specifically, replace the direct into() with a guarded conversion (e.g., check
the raw discriminant or use a TryFrom/TryInto for FFINetwork -> Network), and on
failure set *out_error = PlatformWalletFFIError::ErrorInvalidNetwork (or the
appropriate variant) and return PlatformWalletFFIResult::Error; ensure you
reference platform_wallet_manager_spv_start, FFINetwork, out_error and
PlatformWalletFFIError in the change so invalid network values are rejected at
the boundary.
In
`@packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift`:
- Around line 1400-1506: The deriveAndStoreIdentityKey function is doing
mnemonic→seed→path→private-key derivation in Swift (calls to
WalletStorage.retrieveMnemonicUTF8Bytes, Mnemonic.toSeed,
KeyDerivation.getIdentityAuthenticationPath and
key_wallet_derive_private_key_from_seed); move this logic behind the Rust FFI by
calling a single FFI entry (export a Rust function that takes
walletId/indices/network and returns (path_string, 32_byte_private_key) or an
error) and then change deriveAndStoreIdentityKey to only call that FFI, validate
the returned (path, key) tuple, and pass the key and returned path directly into
KeychainManager.shared.storeIdentityPrivateKey with the existing metadata;
remove Swift-side path building, seed handling, and FFI derive loops so Swift
only persists Rust-emitted path+private key.
In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/ContentView.swift`:
- Around line 264-269: The code hardcodes platformNetwork = .testnet before
calling walletManager.createWallet(mnemonic: mnemonic, network: platformNetwork,
name: label), which can recreate wallets on the wrong chain; instead read the
app's active network (or the persisted wallet network metadata if available) and
pass that into walletManager.createWallet. Replace the hardcoded platformNetwork
assignment with a call to the app/network provider (e.g., currentNetwork,
AppConfig.network, or loadSavedWalletNetwork(for: mnemonic)) and use that value
for the network parameter so recovered wallets are created on the correct chain.
---
Duplicate comments:
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/AddIdentityKeyView.swift`:
- Line 305: The code in AddIdentityKeyView uses "let network =
appState.sdk?.network ?? .testnet" which falls back to a hardcoded .testnet when
appState.sdk is nil; change the fallback to the app's selected network (e.g.,
use "?? appState.network" or the appropriate appState.selectedNetwork property)
so derivation/validation uses the current app network; replace the same pattern
at the other occurrences (around the blocks at the ranges noted: 333–337 and
349–354) so all uses in AddIdentityKeyView consistently fall back to the app's
network instead of .testnet.
---
Nitpick comments:
In `@packages/rs-sdk-ffi/src/system/queries/platform_status.rs`:
- Line 63: Replace the direct to_string() usage for sdk.network with an explicit
match that maps dashcore::Network variants to the canonical API strings (e.g.,
Network::Bitcoin => "bitcoin", Testnet => "testnet3", Regtest => "regtest",
Signet => "signet"), assign the result to network_str, and include a fallback
arm (e.g., unreachable!() or a default) if new variants are added to force
compile-time awareness; locate the usage of sdk.network and the variable
network_str in platform_status.rs and implement the match there to decouple the
JSON field from dashcore::Network's Display implementation.
🪄 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: db72fea7-2f69-4564-b558-fda4a9f8cd2e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (94)
Cargo.tomlpackages/rs-platform-wallet-ffi/Cargo.tomlpackages/rs-platform-wallet-ffi/src/core_wallet/wallet.rspackages/rs-platform-wallet-ffi/src/derivation.rspackages/rs-platform-wallet-ffi/src/derive_identity_key_at_slot.rspackages/rs-platform-wallet-ffi/src/identity_derive_and_persist.rspackages/rs-platform-wallet-ffi/src/identity_keys_from_mnemonic.rspackages/rs-platform-wallet-ffi/src/manager.rspackages/rs-platform-wallet-ffi/src/persistence.rspackages/rs-platform-wallet-ffi/src/platform_addresses/fund_from_asset_lock.rspackages/rs-platform-wallet-ffi/src/platform_wallet_info.rspackages/rs-platform-wallet-ffi/src/sign_with_mnemonic_resolver.rspackages/rs-platform-wallet-ffi/src/spv.rspackages/rs-platform-wallet-ffi/src/types.rspackages/rs-platform-wallet-ffi/src/wallet.rspackages/rs-platform-wallet-ffi/src/wallet_restore_types.rspackages/rs-platform-wallet-ffi/tests/integration_tests.rspackages/rs-platform-wallet/src/wallet/apply.rspackages/rs-sdk-ffi/Cargo.tomlpackages/rs-sdk-ffi/src/address/mod.rspackages/rs-sdk-ffi/src/crypto/mod.rspackages/rs-sdk-ffi/src/identity/top_up_from_addresses.rspackages/rs-sdk-ffi/src/sdk.rspackages/rs-sdk-ffi/src/signer_simple.rspackages/rs-sdk-ffi/src/system/queries/platform_status.rspackages/rs-sdk-ffi/src/system/status.rspackages/rs-sdk-ffi/src/token/claim.rspackages/rs-sdk-ffi/src/token/emergency_action.rspackages/rs-sdk-ffi/src/token/freeze.rspackages/rs-sdk-ffi/src/types.rspackages/rs-sdk-ffi/test_header.hpackages/rs-sdk-ffi/tests/context_provider_test.rspackages/rs-unified-sdk-ffi/Cargo.tomlpackages/rs-unified-sdk-ffi/src/lib.rspackages/swift-sdk/Sources/SwiftDashSDK/Address/Addresses.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Address/PlatformAddressInfo.swiftpackages/swift-sdk/Sources/SwiftDashSDK/DashNetwork.swiftpackages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Helpers/WIFParser.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Address.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyDerivation.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyManager.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyWalletTypes.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedAccount.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedPlatformAccount.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Wallet.swiftpackages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Models/Network.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDPNSName.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDashpayContactRequest.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDashpayProfile.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDataContract.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDocument.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentIdentity.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentPlatformAddressesSyncState.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTokenBalance.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentWallet.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentWalletManagerMetadata.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/ManagedCoreWallet.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/DashPayService.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWallet.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerSPV.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletTypes.swiftpackages/swift-sdk/Sources/SwiftDashSDK/SDK.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Services/DataManager.swiftpackages/swift-sdk/Sources/SwiftDashSDK/SwiftDashSDK.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Utils/KeyValidation.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Utils/PrivateKeyUtils.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/AppState.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/ContentView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Models/DashAddress.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/ShieldedService.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/ZKSyncService.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/ViewModels/SendViewModel.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CreateWalletView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/SwiftExampleAppApp.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Utils/ContractDownloader.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/AddIdentityKeyView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/AddressQueriesView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/Components/RecipientPickerView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/ContractsTabView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/KeysListView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/LoadIdentityView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/OptionsView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TokenActionPermissionsView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/SDKMethodTests.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/SimpleTransitionTests.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/StateTransitionTests.swift
💤 Files with no reviewable changes (6)
- packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyWalletTypes.swift
- packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/SwiftExampleAppApp.swift
- packages/swift-sdk/Sources/SwiftDashSDK/SwiftDashSDK.swift
- packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletTypes.swift
- packages/swift-sdk/Sources/SwiftDashSDK/Models/Network.swift
- packages/rs-sdk-ffi/test_header.h
✅ Files skipped from review due to trivial changes (13)
- packages/rs-unified-sdk-ffi/src/lib.rs
- packages/rs-platform-wallet-ffi/Cargo.toml
- packages/rs-sdk-ffi/src/system/status.rs
- packages/rs-platform-wallet/src/wallet/apply.rs
- Cargo.toml
- packages/rs-platform-wallet-ffi/src/types.rs
- packages/swift-sdk/Sources/SwiftDashSDK/Address/Addresses.swift
- packages/rs-sdk-ffi/src/token/freeze.rs
- packages/swift-sdk/Sources/SwiftDashSDK/Utils/KeyValidation.swift
- packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWallet.swift
- packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/StateTransitionTests.swift
- packages/rs-sdk-ffi/src/signer_simple.rs
- packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyDerivation.swift
🚧 Files skipped from review as they are similar to previous changes (24)
- packages/rs-platform-wallet-ffi/src/platform_addresses/fund_from_asset_lock.rs
- packages/rs-sdk-ffi/Cargo.toml
- packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/SimpleTransitionTests.swift
- packages/rs-platform-wallet-ffi/src/wallet.rs
- packages/rs-unified-sdk-ffi/Cargo.toml
- packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/ManagedPlatformAccount.swift
- packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/SDKMethodTests.swift
- packages/rs-sdk-ffi/src/token/emergency_action.rs
- packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/Address.swift
- packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs
- packages/swift-sdk/Sources/SwiftDashSDK/Helpers/WIFParser.swift
- packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/KeysListView.swift
- packages/swift-sdk/Sources/SwiftDashSDK/SDK.swift
- packages/rs-sdk-ffi/src/sdk.rs
- packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Models/DashAddress.swift
- packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift
- packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift
- packages/rs-platform-wallet-ffi/src/identity_keys_from_mnemonic.rs
- packages/rs-platform-wallet-ffi/src/platform_wallet_info.rs
- packages/rs-platform-wallet-ffi/src/identity_derive_and_persist.rs
- packages/rs-platform-wallet-ffi/src/manager.rs
- packages/rs-platform-wallet-ffi/src/derivation.rs
- packages/rs-sdk-ffi/src/crypto/mod.rs
- packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/AddressQueriesView.swift
QuantumExplorer
approved these changes
May 1, 2026
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
3 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.
In our codebase, for some reason, we used integer to represent networks, at the same time we had DashSDKNetwork and PlatformNetwork variants, all of them wrapping dash_network::Network and das_network::FFINetwork, with this PR I replace every network variant with dash_network::Network and das_network::FFINetwork, simplifying our codebase
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Refactor
Tests