feat: platform wallet #2855
feat: platform wallet #2855
Conversation
WalkthroughAdds two new Rust crates (platform-encryption and platform-wallet-ffi), extensive FFI surfaces (C and Swift) for platform wallet identities/contacts, new wallet/core modules and refactors in rs-platform-wallet, DashPay contact request creation/sending in rs-sdk, reworks dependencies to local paths, introduces broad Swift SDK models/services/persistence, removes legacy rs-sdk-ffi core/unified layers, and updates examples/UI. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor SwiftApp
participant SwiftSDK as Swift (PlatformWallet/IdentityManager/ManagedIdentity)
participant FFI as rs-sdk-ffi (re-exports)
participant PWalletFFI as rs-platform-wallet-ffi
participant PWallet as rs-platform-wallet
SwiftApp->>SwiftSDK: ManagedIdentity.sendContactRequest(...)
SwiftSDK->>FFI: managed_identity_send_contact_request(...)
FFI->>PWalletFFI: managed_identity_send_contact_request(...)
PWalletFFI->>PWallet: ManagedIdentity.add_sent_contact_request(...)
Note over PWallet: If reciprocal incoming exists → auto-establish contact
PWallet-->>PWalletFFI: Result / error
PWalletFFI-->>FFI: PlatformWalletFFIResult
FFI-->>SwiftSDK: Status
SwiftSDK-->>SwiftApp: Throw on error or return success
sequenceDiagram
autonumber
actor Client
participant SDK as rs-sdk (Sdk)
participant Enc as platform-encryption
participant Platform as Platform Node
Client->>SDK: send_contact_request(input, signer, ecdh provider)
SDK->>SDK: Resolve recipient identity / keys
alt ECDH (Client-side)
SDK->>Enc: derive_shared_key_ecdh(priv, pub)
SDK->>Enc: encrypt_extended_public_key(key, iv, xpub)
SDK->>Enc: encrypt_account_label(key, iv, label?)
else ECDH (SDK-side)
SDK->>Enc: derive/encrypt via provided provider
end
SDK->>SDK: Build DashPay contactRequest document
SDK->>Platform: Submit state transition
Platform-->>SDK: Ack with document id
SDK-->>Client: SendContactRequestResult
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (13)
packages/rs-platform-wallet-ffi/README.md (4)
49-98: Add a contact request workflow example to demonstrate the advertised contact system.The example demonstrates wallet and identity manager setup but omits the contact system (sent/incoming/established contacts), which is highlighted as a major feature on line 13. Adding a concrete workflow—e.g., sending a contact request, querying incoming requests, establishing a contact—would significantly improve practical utility for developers integrating the contact system.
Extend the example to include contact request operations:
// Set identity manager for testnet result = platform_wallet_info_set_identity_manager( wallet_handle, NETWORK_TYPE_TESTNET, manager_handle, &error ); + // Create and add a managed identity + Handle identity_handle = NULL_HANDLE; + result = managed_identity_create_from_identity_bytes( + identity_bytes, + &identity_handle, + &error + ); + + if (result != PLATFORM_WALLET_FFI_SUCCESS) { + printf("Error: %s\n", error.message); + platform_wallet_ffi_error_free(error); + identity_manager_destroy(manager_handle); + platform_wallet_info_destroy(wallet_handle); + return 1; + } + + result = identity_manager_add_identity(manager_handle, identity_handle, &error); + + // Send a contact request + result = managed_identity_send_contact_request(identity_handle, recipient_id, &error); + + // Query sent contact requests + Handle request_ids_handle = NULL_HANDLE; + result = managed_identity_get_sent_contact_request_ids(identity_handle, &request_ids_handle, &error); // Cleanup + managed_identity_destroy(identity_handle); identity_manager_destroy(manager_handle); platform_wallet_info_destroy(wallet_handle);
101-154: Add parameter descriptions and signatures to the API overview.The API overview lists function names but omits parameter types, return semantics, and descriptions (e.g., what parameters does
platform_wallet_info_create_from_mnemonic()take? What does it return besides the status code?). Developers currently must inspect the header file to understand the interface, which reduces usability.Enhance the API overview by including parameter and return information. For example:
### Wallet Management - `platform_wallet_info_create_from_seed()` - Create wallet from seed bytes - `platform_wallet_info_create_from_mnemonic()` - Create wallet from BIP39 mnemonic + `platform_wallet_info_create_from_seed(seed_bytes, seed_len, handle, error)` - Create wallet from seed bytes. Returns Handle via out-parameter; error details on failure. + `platform_wallet_info_create_from_mnemonic(mnemonic, passphrase, handle, error)` - Create wallet from BIP39 mnemonic (24 or 12 words). Optional passphrase for security.
18-37: Document header file location and cbindgen build process.The README explains how to build the library but does not mention how to obtain or generate the
platform_wallet_ffi.hheader file referenced on line 44. Given that this is an FFI crate using cbindgen (as noted in the AI summary), clarify whether the header is pre-committed or generated, and provide instructions.Add a subsection after the "Building" section:
The dynamic library will be available at: - Linux: `target/release/libplatform_wallet_ffi.so` - macOS: `target/release/libplatform_wallet_ffi.dylib` - Windows: `target/release/platform_wallet_ffi.dll` +### Generating C/C++ Header + +The C header file is generated using cbindgen during the build process: + +```bash +cbindgen --config cbindgen.toml -o platform_wallet_ffi.h +``` + +Or include it directly from the generated artifact: + +``` +target/release/platform_wallet_ffi.h +```
192-194: Clarify thread safety constraints and consequences.Line 194 advises "you should not use the same handle from multiple threads simultaneously" but does not explain what happens if violated (e.g., data race, panic, undefined behavior). This incomplete guidance may leave developers uncertain about severity and mitigation.
Expand the thread safety note with specifics:
## Thread Safety The library uses thread-safe storage for handles, making it safe to use from multiple threads. However, you should not use the same handle from multiple threads simultaneously. + +**Important**: Concurrent access to the same handle without synchronization will result in undefined behavior and potential data corruption. If you need shared access, use external synchronization (e.g., mutexes) or clone/copy the underlying data before passing to different threads.packages/rs-platform-wallet/src/platform_wallet_info/wallet_transaction_checker.rs (1)
8-8: Minor documentation inconsistency.The comment references "ManagedWalletInfo" but the actual field being delegated to is
wallet_info.Apply this diff to fix the comment:
-/// Implement WalletTransactionChecker by delegating to ManagedWalletInfo +/// Implement WalletTransactionChecker by delegating to wallet_infopackages/rs-platform-wallet/src/block_time.rs (1)
35-38: Consider clarifying age threshold semantics.The
is_older_thanmethod uses strict inequality (>) rather than>=, meaning a block exactly at the threshold age is considered "not older than". This is internally consistent with the tests, but the method name might suggest inclusive behavior to some users.Consider enhancing the documentation to clarify this behavior:
- /// Check if this block time is older than a given age in milliseconds + /// Check if this block time is older than a given age in milliseconds + /// Returns true only if the age strictly exceeds max_age_millis pub fn is_older_than(&self, current_timestamp: TimestampMillis, max_age_millis: u64) -> bool { (current_timestamp - self.timestamp) > max_age_millis }packages/rs-platform-wallet/src/contact_request.rs (1)
43-68: LGTM with optional refactor suggestion.The ContactRequest constructor correctly initializes all fields. The
#[allow(clippy::too_many_arguments)]is justified for this data-transfer struct.For future consideration, a builder pattern could improve ergonomics:
impl ContactRequest { pub fn builder(sender_id: Identifier, recipient_id: Identifier) -> ContactRequestBuilder { ContactRequestBuilder::new(sender_id, recipient_id) } }However, this is purely optional and the current implementation is perfectly acceptable for FFI-facing data structures.
packages/swift-sdk/SwiftTests/Tests/SwiftDashSDKTests/ContactRequestTests.swift (1)
196-209: Assert deterministic behaviour for empty encrypted keys.Line 196 currently lets the test pass whether
ContactRequest.createsucceeds or throws, so regressions around empty-key validation will go unnoticed. Please make the expectation explicit—e.g., assert that the call throwsPlatformWalletErrorif empty keys must be rejected (or assert success if that’s the contract). Tightening the assertion keeps the test meaningful.- do { - let contactRequest = try ContactRequest.create( - senderId: senderId, - recipientId: recipientId, - senderKeyIndex: 0, - recipientKeyIndex: 0, - accountReference: 0, - encryptedPublicKey: emptyKey, - createdAt: UInt64(Date().timeIntervalSince1970 * 1000) - ) - XCTAssertNotNil(contactRequest) - } catch { - // Expected to fail - empty key should not be allowed - XCTAssertTrue(error is PlatformWalletError) - } + XCTAssertThrowsError( + try ContactRequest.create( + senderId: senderId, + recipientId: recipientId, + senderKeyIndex: 0, + recipientKeyIndex: 0, + accountReference: 0, + encryptedPublicKey: emptyKey, + createdAt: UInt64(Date().timeIntervalSince1970 * 1000) + ) + ) { error in + XCTAssertTrue(error is PlatformWalletError) + }packages/rs-platform-wallet-ffi/tests/integration_tests.rs (1)
80-113: Destroy managed identity handles during cleanup.Line 80 and Line 286 insert identities into
MANAGED_IDENTITY_STORAGE, but the corresponding handles never get released. The entries stick around for the lifetime of the test process, so repeated runs (especially with--ignored) can accumulate leaked handles and hide lifecycle bugs. Please callmanaged_identity_destroy(identity_handle)once the manager has cloned the identity.platform_wallet_identifier_array_free(array); // Cleanup + managed_identity_destroy(identity_handle); identity_manager_destroy(manager_handle); } @@ // Cleanup + managed_identity_destroy(identity_handle); identity_manager_destroy(retrieved_manager_handle); identity_manager_destroy(manager_handle); platform_wallet_info_destroy(wallet_handle);Also applies to: 286-317
packages/rs-platform-wallet-ffi/IMPLEMENTATION_SUMMARY.md (2)
119-129: Add language identifier to code block for better rendering.The dependency chain diagram would benefit from a language identifier (e.g.,
textormermaid) for proper syntax highlighting and rendering.Apply this diff:
-``` +```text rs-sdk (library) ↑ platform-wallet (optional dependency on rs-sdk)
406-414: Consider using proper heading syntax.The "Total:" line uses emphasis (
**Total:**) instead of a heading, which may affect document structure and navigation.Consider changing to a proper heading if it represents a section:
-**Total: ~3,620 lines of production-quality code** +### Total + +~3,620 lines of production-quality codepackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/DashPayService.swift (1)
277-290: Use a stable identifier for SwiftUI diffingBecause
idis a freshUUID()each time we build aDashPayContactRequest, SwiftUI treats every refresh as brand-new data and will blow away row state/animations. Please deriveidfrom request data (e.g., sender/recipient IDs plus timestamp) so it stays stable across reloads:-struct DashPayContactRequest: Identifiable { - let id: UUID = UUID() +struct DashPayContactRequest: Identifiable { + let id: String let senderId: Identifier let recipientId: Identifier let createdAt: Date let encryptedPublicKey: Data init(from contactRequest: ContactRequest) throws { self.senderId = try contactRequest.getSenderId() self.recipientId = try contactRequest.getRecipientId() let timestamp = try contactRequest.getCreatedAt() self.createdAt = Date(timeIntervalSince1970: Double(timestamp) / 1000.0) self.encryptedPublicKey = try contactRequest.getEncryptedPublicKey() + self.id = [ + senderId.hexString, + recipientId.hexString, + String(timestamp) + ].joined(separator: ":") } }packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletFFI.swift (1)
8-403: Consider adding doc comments for maintainability.While FFI declarations are internal implementation details, brief doc comments explaining parameters and behavior would help future maintainers, especially for complex functions like
managed_identity_send_contact_requestthat take many parameters.Example for one function:
/// Creates a Platform Wallet from a BIP39 seed. /// - Parameters: /// - seed: 64-byte seed derived from mnemonic /// - seed_len: Length of seed (must be 64) /// - out_handle: Output handle to created wallet /// - out_error: Output error if creation fails /// - Returns: Success or error result code @_silgen_name("platform_wallet_info_create_from_seed") func platform_wallet_info_create_from_seed( _ seed: UnsafePointer<UInt8>?, _ seed_len: Int, _ out_handle: UnsafeMutablePointer<Handle>, _ out_error: UnsafeMutablePointer<PlatformWalletFFIError> ) -> PlatformWalletFFIResultThis is entirely optional since the public API surface should be documented instead.
| /** | ||
| * Add a sent contact request | ||
| * @param identity_handle Identity handle | ||
| * @param contact_id Contact identity ID | ||
| * @param label Optional label (can be NULL) | ||
| * @param timestamp Request timestamp | ||
| * @param out_error Optional error output | ||
| * @return Result code | ||
| */ | ||
| PlatformWalletFFIResult managed_identity_add_sent_contact_request( | ||
| Handle identity_handle, | ||
| IdentifierBytes contact_id, | ||
| const char* label, | ||
| uint64_t timestamp, | ||
| PlatformWalletFFIError* out_error | ||
| ); | ||
|
|
||
| /** | ||
| * Add an incoming contact request | ||
| * @param identity_handle Identity handle | ||
| * @param contact_id Contact identity ID | ||
| * @param label Optional label (can be NULL) | ||
| * @param timestamp Request timestamp | ||
| * @param out_error Optional error output | ||
| * @return Result code | ||
| */ | ||
| PlatformWalletFFIResult managed_identity_add_incoming_contact_request( | ||
| Handle identity_handle, | ||
| IdentifierBytes contact_id, | ||
| const char* label, | ||
| uint64_t timestamp, | ||
| PlatformWalletFFIError* out_error | ||
| ); | ||
|
|
||
| /** | ||
| * Remove a sent contact request | ||
| * @param identity_handle Identity handle | ||
| * @param contact_id Contact identity ID | ||
| * @param out_error Optional error output | ||
| * @return Result code | ||
| */ | ||
| PlatformWalletFFIResult managed_identity_remove_sent_contact_request( | ||
| Handle identity_handle, | ||
| IdentifierBytes contact_id, | ||
| PlatformWalletFFIError* out_error | ||
| ); | ||
|
|
||
| /** | ||
| * Remove an incoming contact request | ||
| * @param identity_handle Identity handle | ||
| * @param contact_id Contact identity ID | ||
| * @param out_error Optional error output | ||
| * @return Result code | ||
| */ | ||
| PlatformWalletFFIResult managed_identity_remove_incoming_contact_request( | ||
| Handle identity_handle, | ||
| IdentifierBytes contact_id, | ||
| PlatformWalletFFIError* out_error | ||
| ); | ||
|
|
||
| /** | ||
| * Get all sent contact request IDs | ||
| * @param identity_handle Identity handle | ||
| * @param out_array Output array (caller must free with platform_wallet_identifier_array_free) | ||
| * @param out_error Optional error output | ||
| * @return Result code | ||
| */ | ||
| PlatformWalletFFIResult managed_identity_get_sent_contact_request_ids( | ||
| Handle identity_handle, | ||
| IdentifierArray* out_array, | ||
| PlatformWalletFFIError* out_error | ||
| ); | ||
|
|
||
| /** | ||
| * Get all incoming contact request IDs | ||
| * @param identity_handle Identity handle | ||
| * @param out_array Output array (caller must free with platform_wallet_identifier_array_free) | ||
| * @param out_error Optional error output | ||
| * @return Result code | ||
| */ | ||
| PlatformWalletFFIResult managed_identity_get_incoming_contact_request_ids( | ||
| Handle identity_handle, | ||
| IdentifierArray* out_array, | ||
| PlatformWalletFFIError* out_error | ||
| ); | ||
|
|
||
| /** | ||
| * Get all established contact IDs | ||
| * @param identity_handle Identity handle | ||
| * @param out_array Output array (caller must free with platform_wallet_identifier_array_free) | ||
| * @param out_error Optional error output | ||
| * @return Result code | ||
| */ | ||
| PlatformWalletFFIResult managed_identity_get_established_contact_ids( | ||
| Handle identity_handle, | ||
| IdentifierArray* out_array, | ||
| PlatformWalletFFIError* out_error | ||
| ); | ||
|
|
||
| /** | ||
| * Check if a contact is established | ||
| * @param identity_handle Identity handle | ||
| * @param contact_id Contact identity ID | ||
| * @param out_is_established Output boolean | ||
| * @param out_error Optional error output | ||
| * @return Result code | ||
| */ | ||
| PlatformWalletFFIResult managed_identity_is_contact_established( | ||
| Handle identity_handle, | ||
| IdentifierBytes contact_id, | ||
| bool* out_is_established, | ||
| PlatformWalletFFIError* out_error | ||
| ); | ||
|
|
||
| /** | ||
| * Remove an established contact | ||
| * @param identity_handle Identity handle | ||
| * @param contact_id Contact identity ID | ||
| * @param out_error Optional error output | ||
| * @return Result code | ||
| */ | ||
| PlatformWalletFFIResult managed_identity_remove_established_contact( | ||
| Handle identity_handle, | ||
| IdentifierBytes contact_id, | ||
| PlatformWalletFFIError* out_error | ||
| ); | ||
|
|
There was a problem hiding this comment.
Header/implementation mismatch for contact APIs.
The C header still declares the old managed_identity_add/remove_* functions and omits the new managed_identity_send_contact_request, managed_identity_accept_contact_request, managed_identity_reject_contact_request, managed_identity_get_sent_contact_request, managed_identity_get_incoming_contact_request, managed_identity_get_established_contact, plus every contact_request_* and established_contact_* entry point we now export from Rust. A C client compiling against this header will either fail to link (symbols not found) or have no way to call the new APIs. Please update the header so its prototypes exactly mirror the extern "C" surface we re-export in Rust.
🤖 Prompt for AI Agents
packages/rs-platform-wallet-ffi/platform_wallet_ffi.h lines 449-575: the header
currently declares old managed_identity_add/remove_* contact functions and is
missing the new exported Rust APIs, causing a header/implementation mismatch;
update this section so the C prototypes exactly match the Rust extern "C"
exports — remove or replace the old managed_identity_add/remove_* declarations
with the new managed_identity_send_contact_request,
managed_identity_accept_contact_request,
managed_identity_reject_contact_request,
managed_identity_get_sent_contact_request,
managed_identity_get_incoming_contact_request,
managed_identity_get_established_contact declarations, and add all
contact_request_* and established_contact_* entrypoint prototypes exported from
Rust, ensuring parameter types, names, return type (PlatformWalletFFIResult) and
error pointer (PlatformWalletFFIError*) signatures match the Rust definitions
exactly and include the same comments and caller-owned/free semantics for any
returned arrays.
| ## Error Handling | ||
|
|
||
| All functions return a `PlatformWalletFFIResult` status code. Check for `PLATFORM_WALLET_FFI_SUCCESS` and handle errors appropriately. | ||
|
|
||
| Error codes: | ||
| - `PLATFORM_WALLET_FFI_SUCCESS` - Operation succeeded | ||
| - `PLATFORM_WALLET_FFI_ERROR_INVALID_HANDLE` - Invalid handle provided | ||
| - `PLATFORM_WALLET_FFI_ERROR_NULL_POINTER` - Null pointer provided | ||
| - `PLATFORM_WALLET_FFI_ERROR_SERIALIZATION` - Serialization failed | ||
| - `PLATFORM_WALLET_FFI_ERROR_DESERIALIZATION` - Deserialization failed | ||
| - `PLATFORM_WALLET_FFI_ERROR_IDENTITY_NOT_FOUND` - Identity not found | ||
| - `PLATFORM_WALLET_FFI_ERROR_CONTACT_NOT_FOUND` - Contact not found | ||
| - And more... |
There was a problem hiding this comment.
Incomplete error codes documentation — finish the error enumeration.
Line 176 uses "And more..." which leaves developers guessing about other error codes they should handle. Since error handling is critical for FFI robustness, provide an exhaustive list or explicitly reference where the complete enum is defined (e.g., "See platform_wallet_ffi.h for the full PlatformWalletFFIError enum").
Consider replacing the vague ellipsis:
- And more...
+ See `platform_wallet_ffi.h` for the complete error code enumeration.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Error Handling | |
| All functions return a `PlatformWalletFFIResult` status code. Check for `PLATFORM_WALLET_FFI_SUCCESS` and handle errors appropriately. | |
| Error codes: | |
| - `PLATFORM_WALLET_FFI_SUCCESS` - Operation succeeded | |
| - `PLATFORM_WALLET_FFI_ERROR_INVALID_HANDLE` - Invalid handle provided | |
| - `PLATFORM_WALLET_FFI_ERROR_NULL_POINTER` - Null pointer provided | |
| - `PLATFORM_WALLET_FFI_ERROR_SERIALIZATION` - Serialization failed | |
| - `PLATFORM_WALLET_FFI_ERROR_DESERIALIZATION` - Deserialization failed | |
| - `PLATFORM_WALLET_FFI_ERROR_IDENTITY_NOT_FOUND` - Identity not found | |
| - `PLATFORM_WALLET_FFI_ERROR_CONTACT_NOT_FOUND` - Contact not found | |
| - And more... | |
| ## Error Handling | |
| All functions return a `PlatformWalletFFIResult` status code. Check for `PLATFORM_WALLET_FFI_SUCCESS` and handle errors appropriately. | |
| Error codes: | |
| - `PLATFORM_WALLET_FFI_SUCCESS` - Operation succeeded | |
| - `PLATFORM_WALLET_FFI_ERROR_INVALID_HANDLE` - Invalid handle provided | |
| - `PLATFORM_WALLET_FFI_ERROR_NULL_POINTER` - Null pointer provided | |
| - `PLATFORM_WALLET_FFI_ERROR_SERIALIZATION` - Serialization failed | |
| - `PLATFORM_WALLET_FFI_ERROR_DESERIALIZATION` - Deserialization failed | |
| - `PLATFORM_WALLET_FFI_ERROR_IDENTITY_NOT_FOUND` - Identity not found | |
| - `PLATFORM_WALLET_FFI_ERROR_CONTACT_NOT_FOUND` - Contact not found | |
| - See `platform_wallet_ffi.h` for the complete error code enumeration. |
🤖 Prompt for AI Agents
In packages/rs-platform-wallet-ffi/README.md around lines 164 to 176, the error
codes section ends with "And more..." which is incomplete; update the
documentation to either enumerate all PlatformWalletFFI error codes or point
readers to the definitive source. Fix by replacing the ellipsis with either a
full list of the remaining error constants from the FFI (copy the names and
short descriptions from the codebase) or add an explicit reference such as "See
platform_wallet_ffi.h for the full PlatformWalletFFIError enum" with a relative
path link; ensure the wording makes it clear where to find the canonical enum
and remove the vague phrasing.
| let mut seed_array = [0u8; 64]; | ||
| seed_array.copy_from_slice(seed_slice); | ||
|
|
||
| // Create wallet from seed - use empty network list, accounts can be added later | ||
| let wallet = match key_wallet::Wallet::from_seed_bytes( | ||
| seed_array, | ||
| &[], // No networks initially | ||
| WalletAccountCreationOptions::None, // No accounts initially | ||
| ) { | ||
| Ok(w) => w, | ||
| Err(e) => { | ||
| if !out_error.is_null() { | ||
| unsafe { | ||
| *out_error = PlatformWalletFFIError::new( | ||
| PlatformWalletFFIResult::ErrorWalletOperation, | ||
| format!("Failed to create wallet from seed: {}", e), | ||
| ); | ||
| } | ||
| } | ||
| return PlatformWalletFFIResult::ErrorWalletOperation; | ||
| } | ||
| }; | ||
|
|
||
| // Create ManagedWalletInfo from the wallet | ||
| let wallet_info = key_wallet::wallet::ManagedWalletInfo::from_wallet(&wallet); | ||
|
|
||
| // Create PlatformWalletInfo wrapping the ManagedWalletInfo | ||
| let platform_wallet = PlatformWalletInfo { | ||
| wallet_info, | ||
| identity_managers: std::collections::BTreeMap::new(), | ||
| }; | ||
|
|
||
| // Store in handle storage |
There was a problem hiding this comment.
Zeroize the seed buffer before returning
We copy the wallet seed into seed_array on Line 44 but never wipe it. That leaves 64 bytes of highly sensitive material on the stack after this FFI returns, which is a security posture gap for wallet code. Please clear the local copy regardless of success or failure before exiting the function.
Apply this diff to zeroize the local seed:
- let wallet = match key_wallet::Wallet::from_seed_bytes(
- seed_array,
- &[], // No networks initially
- WalletAccountCreationOptions::None, // No accounts initially
- ) {
+ let wallet_result = key_wallet::Wallet::from_seed_bytes(
+ seed_array,
+ &[], // No networks initially
+ WalletAccountCreationOptions::None, // No accounts initially
+ );
+ seed_array.fill(0);
+ let wallet = match wallet_result {
Ok(w) => w,
Err(e) => {
if !out_error.is_null() {
unsafe {
*out_error = PlatformWalletFFIError::new(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mut seed_array = [0u8; 64]; | |
| seed_array.copy_from_slice(seed_slice); | |
| // Create wallet from seed - use empty network list, accounts can be added later | |
| let wallet = match key_wallet::Wallet::from_seed_bytes( | |
| seed_array, | |
| &[], // No networks initially | |
| WalletAccountCreationOptions::None, // No accounts initially | |
| ) { | |
| Ok(w) => w, | |
| Err(e) => { | |
| if !out_error.is_null() { | |
| unsafe { | |
| *out_error = PlatformWalletFFIError::new( | |
| PlatformWalletFFIResult::ErrorWalletOperation, | |
| format!("Failed to create wallet from seed: {}", e), | |
| ); | |
| } | |
| } | |
| return PlatformWalletFFIResult::ErrorWalletOperation; | |
| } | |
| }; | |
| // Create ManagedWalletInfo from the wallet | |
| let wallet_info = key_wallet::wallet::ManagedWalletInfo::from_wallet(&wallet); | |
| // Create PlatformWalletInfo wrapping the ManagedWalletInfo | |
| let platform_wallet = PlatformWalletInfo { | |
| wallet_info, | |
| identity_managers: std::collections::BTreeMap::new(), | |
| }; | |
| // Store in handle storage | |
| let mut seed_array = [0u8; 64]; | |
| seed_array.copy_from_slice(seed_slice); | |
| // Create wallet from seed - use empty network list, accounts can be added later | |
| let wallet_result = key_wallet::Wallet::from_seed_bytes( | |
| seed_array, | |
| &[], // No networks initially | |
| WalletAccountCreationOptions::None, // No accounts initially | |
| ); | |
| seed_array.fill(0); | |
| let wallet = match wallet_result { | |
| Ok(w) => w, | |
| Err(e) => { | |
| if !out_error.is_null() { | |
| unsafe { | |
| *out_error = PlatformWalletFFIError::new( | |
| PlatformWalletFFIResult::ErrorWalletOperation, | |
| format!("Failed to create wallet from seed: {}", e), | |
| ); | |
| } | |
| } | |
| return PlatformWalletFFIResult::ErrorWalletOperation; | |
| } | |
| }; | |
| // Create ManagedWalletInfo from the wallet | |
| let wallet_info = key_wallet::wallet::ManagedWalletInfo::from_wallet(&wallet); | |
| // Create PlatformWalletInfo wrapping the ManagedWalletInfo | |
| let platform_wallet = PlatformWalletInfo { | |
| wallet_info, | |
| identity_managers: std::collections::BTreeMap::new(), | |
| }; | |
| // Store in handle storage |
🤖 Prompt for AI Agents
In packages/rs-platform-wallet-ffi/src/platform_wallet_info.rs around lines 44
to 76, the local seed_array (64 bytes) is left in memory after returning; ensure
the seed buffer is zeroized on all exit paths. After creating the wallet (or
immediately before every return on error), overwrite or zeroize seed_array
(e.g., call the Zeroize trait or fill the array with zeros) so the sensitive
bytes are cleared; place the zeroization both in the error branch before
returning and after successful use (before continuing/returning) to guarantee no
residual secret remains on the stack.
| // Derive the first authentication key (identity_index 0, key_index 0) | ||
| let identity_index = 0u32; | ||
| let key_index = 0u32; | ||
|
|
||
| // Build identity authentication derivation path | ||
| // Path format: m/9'/COIN_TYPE'/5'/0'/identity_index'/key_index' | ||
| use key_wallet::bip32::{ChildNumber, DerivationPath}; | ||
| use key_wallet::dip9::{ | ||
| IDENTITY_AUTHENTICATION_PATH_MAINNET, IDENTITY_AUTHENTICATION_PATH_TESTNET, | ||
| }; | ||
|
|
||
| let base_path = match network { | ||
| Network::Dash => IDENTITY_AUTHENTICATION_PATH_MAINNET, | ||
| Network::Testnet => IDENTITY_AUTHENTICATION_PATH_TESTNET, | ||
| _ => { | ||
| return Err(PlatformWalletError::InvalidIdentityData( | ||
| "Unsupported network for identity derivation".to_string(), | ||
| )); | ||
| } | ||
| }; | ||
|
|
||
| // Create full derivation path: base path + identity_index' + key_index' | ||
| let mut full_path = DerivationPath::from(base_path); | ||
| full_path = full_path.extend([ | ||
| ChildNumber::from_hardened_idx(identity_index).map_err(|e| { | ||
| PlatformWalletError::InvalidIdentityData(format!("Invalid identity index: {}", e)) | ||
| })?, | ||
| ChildNumber::from_hardened_idx(key_index).map_err(|e| { | ||
| PlatformWalletError::InvalidIdentityData(format!("Invalid key index: {}", e)) | ||
| })?, | ||
| ]); | ||
|
|
||
| // Derive the extended private key at this path | ||
| let auth_key = wallet | ||
| .derive_extended_private_key(network, &full_path) | ||
| .map_err(|e| { | ||
| PlatformWalletError::InvalidIdentityData(format!( | ||
| "Failed to derive authentication key: {}", | ||
| e | ||
| )) | ||
| })?; | ||
|
|
||
| // Get public key bytes and hash them | ||
| use dashcore::secp256k1::Secp256k1; | ||
| use key_wallet::bip32::ExtendedPubKey; | ||
| let secp = Secp256k1::new(); | ||
| let public_key = ExtendedPubKey::from_priv(&secp, &auth_key); | ||
| let public_key_bytes = public_key.public_key.serialize(); | ||
| let key_hash = ripemd160_sha256(&public_key_bytes); | ||
|
|
||
| // Create a fixed-size array from the hash | ||
| let mut key_hash_array = [0u8; 20]; | ||
| key_hash_array.copy_from_slice(&key_hash); | ||
|
|
||
| // Query Platform for identity by public key hash | ||
| match dpp::identity::Identity::fetch(&sdk, PublicKeyHash(key_hash_array)).await { | ||
| Ok(Some(identity)) => { | ||
| let identity_id = identity.id(); | ||
|
|
||
| // Add identity to manager if not already present | ||
| if !self | ||
| .identity_manager(network) | ||
| .map(|mgr| mgr.identities().contains_key(&identity_id)) | ||
| .unwrap_or(false) | ||
| { | ||
| self.identity_manager_mut(network) | ||
| .add_identity(identity.clone())?; | ||
| } | ||
|
|
||
| // Fetch DashPay contact requests for this identity | ||
| match sdk | ||
| .fetch_all_contact_requests_for_identity(&identity, Some(100)) | ||
| .await | ||
| { | ||
| Ok((sent_docs, received_docs)) => { | ||
| // Process sent contact requests | ||
| for (_doc_id, maybe_doc) in sent_docs { | ||
| if let Some(doc) = maybe_doc { | ||
| if let Ok(contact_request) = parse_contact_request_document(&doc) { | ||
| // Add to managed identity | ||
| if let Some(managed_identity) = self | ||
| .identity_manager_mut(network) | ||
| .managed_identity_mut(&identity_id) | ||
| { | ||
| managed_identity.add_sent_contact_request(contact_request); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Process received contact requests | ||
| for (_doc_id, maybe_doc) in received_docs { | ||
| if let Some(doc) = maybe_doc { | ||
| if let Ok(contact_request) = parse_contact_request_document(&doc) { | ||
| // Add to managed identity | ||
| if let Some(managed_identity) = self | ||
| .identity_manager_mut(network) | ||
| .managed_identity_mut(&identity_id) | ||
| { | ||
| managed_identity | ||
| .add_incoming_contact_request(contact_request); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| identities_processed.push(identity_id); | ||
| } |
There was a problem hiding this comment.
Process each matured identity instead of hardcoding index 0
Right now we only check that some matured asset lock exists and then derive identity m/…/0'/0'. Any matured transaction that registered an identity on a different derivation index will never be discovered, so their DashPay contact requests remain unfetched. ImmatureTransaction already carries the derivation metadata we recorded when tracking these asset locks—please iterate the incoming matured_transactions, pull the identity/key indices from each asset lock, and derive the corresponding key hash per item instead of assuming 0/0. Otherwise the wallet breaks as soon as a second identity matures.
| if recipient_key.purpose() != Purpose::DECRYPTION { | ||
| return Err(Error::Generic(format!( | ||
| "Recipient key at index {} is not a decryption key", | ||
| input.recipient_key_index | ||
| ))); |
There was a problem hiding this comment.
Recipient key purpose check rejects valid identities
DashPay identities expose their ECDH keys with Purpose::ENCRYPTION (see existing wallet code in platform_wallet_info/contact_requests.rs). Requiring Purpose::DECRYPTION makes every legitimate recipient fail this guard, so create_contact_request will always error out. Please align the check with the encryption purpose.
- if recipient_key.purpose() != Purpose::DECRYPTION {
+ if recipient_key.purpose() != Purpose::ENCRYPTION {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if recipient_key.purpose() != Purpose::DECRYPTION { | |
| return Err(Error::Generic(format!( | |
| "Recipient key at index {} is not a decryption key", | |
| input.recipient_key_index | |
| ))); | |
| if recipient_key.purpose() != Purpose::ENCRYPTION { | |
| return Err(Error::Generic(format!( | |
| "Recipient key at index {} is not a decryption key", | |
| input.recipient_key_index | |
| ))); |
🤖 Prompt for AI Agents
In packages/rs-sdk/src/platform/dashpay/contact_request.rs around lines 229-233,
the recipient key purpose check currently requires Purpose::DECRYPTION which
rejects valid DashPay identities that expose ECDH keys as Purpose::ENCRYPTION;
change the guard to check for Purpose::ENCRYPTION (or accept both ENCRYPTION and
DECRYPTION if broader compatibility is desired) and update the returned error
message to reflect the expected purpose so legitimate recipients no longer fail
this check.
| public static func create( | ||
| senderId: Identifier, | ||
| recipientId: Identifier, | ||
| senderKeyIndex: UInt32, | ||
| recipientKeyIndex: UInt32, | ||
| accountReference: UInt32, | ||
| encryptedPublicKey: Data, | ||
| createdAt: UInt64 | ||
| ) throws -> ContactRequest { | ||
| var handle: Handle = NULL_HANDLE | ||
| var error = PlatformWalletFFIError() | ||
| var ffiSenderId = identifierToFFI(senderId) | ||
| var ffiRecipientId = identifierToFFI(recipientId) | ||
|
|
||
| let result = encryptedPublicKey.withUnsafeBytes { keyPtr in | ||
| contact_request_create( | ||
| ffiSenderId, | ||
| ffiRecipientId, | ||
| senderKeyIndex, | ||
| recipientKeyIndex, | ||
| accountReference, | ||
| keyPtr.baseAddress?.assumingMemoryBound(to: UInt8.self), | ||
| encryptedPublicKey.count, | ||
| createdAt, | ||
| &handle, | ||
| &error | ||
| ) | ||
| } | ||
|
|
||
| guard result == Success else { | ||
| throw PlatformWalletError(result: result, error: error) | ||
| } | ||
|
|
||
| return ContactRequest(handle: handle) | ||
| } |
There was a problem hiding this comment.
Pass coreHeightCreatedAt into the FFI create call
contact_request_create expects both the Core height and the creation timestamp. We currently skip the height, so the timestamp argument shifts into the height slot and the handle pointer shifts into the timestamp slot, corrupting the call. Please thread coreHeightCreatedAt through the API and pass it along to the FFI layer.
- public static func create(
+ public static func create(
senderId: Identifier,
recipientId: Identifier,
senderKeyIndex: UInt32,
recipientKeyIndex: UInt32,
accountReference: UInt32,
encryptedPublicKey: Data,
+ coreHeightCreatedAt: UInt32,
createdAt: UInt64
) throws -> ContactRequest {
var handle: Handle = NULL_HANDLE
var error = PlatformWalletFFIError()
var ffiSenderId = identifierToFFI(senderId)
var ffiRecipientId = identifierToFFI(recipientId)
let result = encryptedPublicKey.withUnsafeBytes { keyPtr in
contact_request_create(
ffiSenderId,
ffiRecipientId,
senderKeyIndex,
recipientKeyIndex,
accountReference,
keyPtr.baseAddress?.assumingMemoryBound(to: UInt8.self),
encryptedPublicKey.count,
+ coreHeightCreatedAt,
createdAt,
&handle,
&error
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static func create( | |
| senderId: Identifier, | |
| recipientId: Identifier, | |
| senderKeyIndex: UInt32, | |
| recipientKeyIndex: UInt32, | |
| accountReference: UInt32, | |
| encryptedPublicKey: Data, | |
| createdAt: UInt64 | |
| ) throws -> ContactRequest { | |
| var handle: Handle = NULL_HANDLE | |
| var error = PlatformWalletFFIError() | |
| var ffiSenderId = identifierToFFI(senderId) | |
| var ffiRecipientId = identifierToFFI(recipientId) | |
| let result = encryptedPublicKey.withUnsafeBytes { keyPtr in | |
| contact_request_create( | |
| ffiSenderId, | |
| ffiRecipientId, | |
| senderKeyIndex, | |
| recipientKeyIndex, | |
| accountReference, | |
| keyPtr.baseAddress?.assumingMemoryBound(to: UInt8.self), | |
| encryptedPublicKey.count, | |
| createdAt, | |
| &handle, | |
| &error | |
| ) | |
| } | |
| guard result == Success else { | |
| throw PlatformWalletError(result: result, error: error) | |
| } | |
| return ContactRequest(handle: handle) | |
| } | |
| public static func create( | |
| senderId: Identifier, | |
| recipientId: Identifier, | |
| senderKeyIndex: UInt32, | |
| recipientKeyIndex: UInt32, | |
| accountReference: UInt32, | |
| encryptedPublicKey: Data, | |
| coreHeightCreatedAt: UInt32, | |
| createdAt: UInt64 | |
| ) throws -> ContactRequest { | |
| var handle: Handle = NULL_HANDLE | |
| var error = PlatformWalletFFIError() | |
| var ffiSenderId = identifierToFFI(senderId) | |
| var ffiRecipientId = identifierToFFI(recipientId) | |
| let result = encryptedPublicKey.withUnsafeBytes { keyPtr in | |
| contact_request_create( | |
| ffiSenderId, | |
| ffiRecipientId, | |
| senderKeyIndex, | |
| recipientKeyIndex, | |
| accountReference, | |
| keyPtr.baseAddress?.assumingMemoryBound(to: UInt8.self), | |
| encryptedPublicKey.count, | |
| coreHeightCreatedAt, | |
| createdAt, | |
| &handle, | |
| &error | |
| ) | |
| } | |
| guard result == Success else { | |
| throw PlatformWalletError(result: result, error: error) | |
| } | |
| return ContactRequest(handle: handle) | |
| } |
| var bytesPtr: UnsafeMutablePointer<UInt8>? = nil | ||
| var length: Int = 0 | ||
| var error = PlatformWalletFFIError() | ||
|
|
||
| let result = contact_request_get_encrypted_public_key(handle, &bytesPtr, &length, &error) | ||
| guard result == Success else { | ||
| throw PlatformWalletError(result: result, error: error) | ||
| } | ||
|
|
||
| defer { | ||
| if let ptr = bytesPtr { | ||
| platform_wallet_bytes_free(ptr) | ||
| } | ||
| } |
There was a problem hiding this comment.
Free encrypted key buffer with the correct length
platform_wallet_bytes_free reconstructs a Vec using both the pointer and its length. Passing only the pointer leaves the length parameter uninitialised, causing undefined behaviour (or a compile error, depending on the header). Please provide the length we just received:
defer {
if let ptr = bytesPtr {
- platform_wallet_bytes_free(ptr)
+ platform_wallet_bytes_free(ptr, length)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var bytesPtr: UnsafeMutablePointer<UInt8>? = nil | |
| var length: Int = 0 | |
| var error = PlatformWalletFFIError() | |
| let result = contact_request_get_encrypted_public_key(handle, &bytesPtr, &length, &error) | |
| guard result == Success else { | |
| throw PlatformWalletError(result: result, error: error) | |
| } | |
| defer { | |
| if let ptr = bytesPtr { | |
| platform_wallet_bytes_free(ptr) | |
| } | |
| } | |
| var bytesPtr: UnsafeMutablePointer<UInt8>? = nil | |
| var length: Int = 0 | |
| var error = PlatformWalletFFIError() | |
| let result = contact_request_get_encrypted_public_key(handle, &bytesPtr, &length, &error) | |
| guard result == Success else { | |
| throw PlatformWalletError(result: result, error: error) | |
| } | |
| defer { | |
| if let ptr = bytesPtr { | |
| platform_wallet_bytes_free(ptr, length) | |
| } | |
| } |
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 120-120: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
🤖 Prompt for AI Agents
In packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ContactRequest.swift
around lines 120 to 133, the defer block calls platform_wallet_bytes_free(ptr)
with only the pointer while the FFI expects both pointer and length; update the
defer to call platform_wallet_bytes_free(ptr, length) (passing the length
variable received from contact_request_get_encrypted_public_key) so the Vec is
reconstructed with the correct size before freeing.
| let result = identity_manager_get_primary_identity_id(handle, &ffiId, &error) | ||
|
|
||
| if result == ErrorIdentityNotFound { | ||
| return nil | ||
| } | ||
|
|
||
| guard result == Success else { | ||
| throw PlatformWalletError(result: result, error: error) | ||
| } |
There was a problem hiding this comment.
Free the not-found error to avoid leaking
When identity_manager_get_primary_identity_id returns ErrorIdentityNotFound, the FFI populates error with an allocated message. Because we early-return nil instead of throwing, that buffer is never released and leaks every time this path is hit. Please free the error before returning. For example:
let result = identity_manager_get_primary_identity_id(handle, &ffiId, &error)
if result == ErrorIdentityNotFound {
+ platform_wallet_error_free(error)
return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let result = identity_manager_get_primary_identity_id(handle, &ffiId, &error) | |
| if result == ErrorIdentityNotFound { | |
| return nil | |
| } | |
| guard result == Success else { | |
| throw PlatformWalletError(result: result, error: error) | |
| } | |
| let result = identity_manager_get_primary_identity_id(handle, &ffiId, &error) | |
| if result == ErrorIdentityNotFound { | |
| platform_wallet_error_free(error) | |
| return nil | |
| } | |
| guard result == Success else { | |
| throw PlatformWalletError(result: result, error: error) | |
| } |
🤖 Prompt for AI Agents
In packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/IdentityManager.swift
around lines 96-104, when identity_manager_get_primary_identity_id returns
ErrorIdentityNotFound the FFI-populated error must be freed before returning
nil; call the appropriate FFI error-free function (for example the generated
free function for the error type, e.g. platform_wallet_error_free(error) or
similar) to release the allocated message buffer, then return nil (and nul the
pointer if required by the free function).
| /// Accept a contact request from another identity | ||
| public func acceptContactRequest(senderId: Identifier) throws { | ||
| var error = PlatformWalletFFIError() | ||
| var ffiSenderId = identifierToFFI(senderId) | ||
|
|
||
| let result = managed_identity_accept_contact_request(handle, ffiSenderId, &error) | ||
| guard result == Success else { | ||
| throw PlatformWalletError(result: result, error: error) | ||
| } | ||
| } |
There was a problem hiding this comment.
Pass the expected request handle into managed_identity_accept_contact_request.
The FFI function accepts (Handle identity_handle, Handle request_handle, PlatformWalletFFIError* out_error), but this code supplies an IdentifierBytes. As with sendContactRequest, the ABI mismatch breaks the call entirely. You need to hand the FFI a valid contact-request handle (e.g. obtained from managed_identity_get_incoming_contact_request) or update both sides of the interface together. citeturn0files1
| /// Get the identity manager for a specific network | ||
| public func getIdentityManager(for network: PlatformNetwork) throws -> IdentityManager { | ||
| // Check if we already have it cached | ||
| if let manager = identityManagers[network] { | ||
| return manager | ||
| } | ||
|
|
||
| var managerHandle: Handle = NULL_HANDLE | ||
| var error = PlatformWalletFFIError() | ||
|
|
||
| let result = platform_wallet_info_get_identity_manager( | ||
| handle, | ||
| network.ffiValue, | ||
| &managerHandle, | ||
| &error | ||
| ) | ||
|
|
||
| guard result == Success else { | ||
| throw PlatformWalletError(result: result, error: error) | ||
| } | ||
|
|
||
| let manager = IdentityManager(handle: managerHandle) | ||
| identityManagers[network] = manager | ||
| return manager | ||
| } | ||
|
|
||
| /// Set the identity manager for a specific network | ||
| public func setIdentityManager(_ manager: IdentityManager, for network: PlatformNetwork) throws { | ||
| var error = PlatformWalletFFIError() | ||
|
|
||
| let result = platform_wallet_info_set_identity_manager( | ||
| handle, | ||
| network.ffiValue, | ||
| manager.handle, | ||
| &error | ||
| ) | ||
|
|
||
| guard result == Success else { | ||
| throw PlatformWalletError(result: result, error: error) | ||
| } | ||
|
|
||
| identityManagers[network] = manager | ||
| } | ||
| } |
There was a problem hiding this comment.
Create missing identity managers on demand
platform_wallet_info_get_identity_manager returns ErrorInvalidNetwork for a fresh wallet because the underlying identity_managers map is empty until something registers a manager via platform_wallet_info_set_identity_manager. With the current guard this method throws for every new wallet, so the integration suite (and real clients) can never obtain a manager. Please detect the invalid-network case, create a new IdentityManager, register it with setIdentityManager, and return it instead of bubbling up the error.
Apply this diff:
- guard result == Success else {
- throw PlatformWalletError(result: result, error: error)
- }
-
- let manager = IdentityManager(handle: managerHandle)
- identityManagers[network] = manager
- return manager
+ if result == Success {
+ let manager = IdentityManager(handle: managerHandle)
+ identityManagers[network] = manager
+ return manager
+ }
+
+ if result == ErrorInvalidNetwork {
+ let manager = try IdentityManager.create()
+ try setIdentityManager(manager, for: network)
+ return manager
+ }
+
+ throw PlatformWalletError(result: result, error: error)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Get the identity manager for a specific network | |
| public func getIdentityManager(for network: PlatformNetwork) throws -> IdentityManager { | |
| // Check if we already have it cached | |
| if let manager = identityManagers[network] { | |
| return manager | |
| } | |
| var managerHandle: Handle = NULL_HANDLE | |
| var error = PlatformWalletFFIError() | |
| let result = platform_wallet_info_get_identity_manager( | |
| handle, | |
| network.ffiValue, | |
| &managerHandle, | |
| &error | |
| ) | |
| guard result == Success else { | |
| throw PlatformWalletError(result: result, error: error) | |
| } | |
| let manager = IdentityManager(handle: managerHandle) | |
| identityManagers[network] = manager | |
| return manager | |
| } | |
| /// Set the identity manager for a specific network | |
| public func setIdentityManager(_ manager: IdentityManager, for network: PlatformNetwork) throws { | |
| var error = PlatformWalletFFIError() | |
| let result = platform_wallet_info_set_identity_manager( | |
| handle, | |
| network.ffiValue, | |
| manager.handle, | |
| &error | |
| ) | |
| guard result == Success else { | |
| throw PlatformWalletError(result: result, error: error) | |
| } | |
| identityManagers[network] = manager | |
| } | |
| } | |
| /// Get the identity manager for a specific network | |
| public func getIdentityManager(for network: PlatformNetwork) throws -> IdentityManager { | |
| // Check if we already have it cached | |
| if let manager = identityManagers[network] { | |
| return manager | |
| } | |
| var managerHandle: Handle = NULL_HANDLE | |
| var error = PlatformWalletFFIError() | |
| let result = platform_wallet_info_get_identity_manager( | |
| handle, | |
| network.ffiValue, | |
| &managerHandle, | |
| &error | |
| ) | |
| if result == Success { | |
| let manager = IdentityManager(handle: managerHandle) | |
| identityManagers[network] = manager | |
| return manager | |
| } | |
| if result == ErrorInvalidNetwork { | |
| let manager = try IdentityManager.create() | |
| try setIdentityManager(manager, for: network) | |
| return manager | |
| } | |
| throw PlatformWalletError(result: result, error: error) | |
| } | |
| /// Set the identity manager for a specific network | |
| public func setIdentityManager(_ manager: IdentityManager, for network: PlatformNetwork) throws { | |
| var error = PlatformWalletFFIError() | |
| let result = platform_wallet_info_set_identity_manager( | |
| handle, | |
| network.ffiValue, | |
| manager.handle, | |
| &error | |
| ) | |
| guard result == Success else { | |
| throw PlatformWalletError(result: result, error: error) | |
| } | |
| identityManagers[network] = manager | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
packages/rs-platform-wallet/src/platform_wallet_info/matured_transactions.rs (1)
86-239:asset_lock_transactionsslice is effectively ignored; only identity0/0is ever discoveredThis method currently:
- Uses
asset_lock_transactionsonly to checkis_empty().- Always derives a single authentication key at
identity_index = 0,key_index = 0.- Calls
Identity::fetchexactly once and can push at most oneidentity_idintoidentities_processed.Any matured asset lock that registered an identity on a different derivation index will never be discovered, so additional identities’ DashPay contact requests will not be fetched.
ImmatureTransactionalready carries the derivation metadata for each asset lock; you should iterate overasset_lock_transactions, pull the identity/key indices per item, derive the key hash for each, and query/add/fetch contacts per identity instead of assuming0/0globally.This is the same functional gap that was raised in a previous review on this file and remains unresolved.
🧹 Nitpick comments (13)
packages/rs-platform-wallet/src/platform_wallet_info/wallet_info_interface.rs (1)
80-86: Clarify the misleading comment.The comment says "Use the default trait implementation which filters utxos" but the code provides a custom filtering implementation. The filtering logic is correct, but the comment should accurately describe what the code does.
Apply this diff to clarify:
- fn get_spendable_utxos(&self) -> BTreeSet<&Utxo> { - // Use the default trait implementation which filters utxos - self.utxos() + fn get_spendable_utxos(&self) -> BTreeSet<&Utxo> { + // Filter to spendable UTXOs: not locked and either confirmed or instantlocked + self.utxos() .into_iter() .filter(|utxo| !utxo.is_locked && (utxo.is_confirmed || utxo.is_instantlocked)) .collect() }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift (1)
325-325: Optional: Remove redundant nil initialization.In Swift, optional variables are implicitly initialized to
nil, so the explicit= nilis redundant.Apply this diff:
- var navigationDestination: AnyView? = nil + var navigationDestination: AnyView?packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (1)
230-233: Consider adding a warning about handle usage in documentation.Exposing the raw FFI handle is appropriate for filter match queries, but callers could misuse it by making arbitrary FFI calls that might violate memory safety or invalidate internal state.
Consider enhancing the documentation:
- // Public accessor for client handle (needed for filter match queries) + /// Public accessor for the underlying FFI client handle. + /// + /// - Warning: This handle is provided for advanced use cases such as filter match queries. + /// Do not use this handle to make FFI calls that modify client state, as this may + /// invalidate the Swift wrapper's internal state or cause memory safety issues. + /// + /// - Returns: The underlying FFI client handle, or nil if not initialized public var clientHandle: UnsafeMutablePointer<FFIDashSpvClient>? { return client }packages/rs-platform-wallet/src/established_contact.rs (1)
85-95: LGTM!The account management methods correctly handle adding and removing accounts with proper duplicate prevention.
Optional: Consider using
HashSetforaccepted_accountsif scalability matters.The current
Vec<u32>implementation withcontains()has O(n) lookup. If the list of accepted accounts could grow large, consider using aHashSet<u32>for O(1) operations:pub accepted_accounts: std::collections::HashSet<u32>,However, for typical contact scenarios with a small number of accounts, the current implementation is perfectly adequate.
packages/rs-platform-wallet/src/platform_wallet_info/wallet_transaction_checker.rs (1)
27-47: Consider flattening nested conditionals for better readability.The nested if statements could be simplified using early continues or by combining conditions to reduce nesting depth.
Here's a more readable alternative:
// If the transaction is relevant, and it's an asset lock, automatically fetch identities - if result.is_relevant { - use dashcore::transaction::special_transaction::TransactionPayload; - - if matches!( - &tx.special_transaction_payload, - Some(TransactionPayload::AssetLockPayloadType(_)) - ) { - // Check if we have an SDK configured for this network - if let Some(identity_manager) = self.identity_managers.get(&network) { - if identity_manager.sdk.is_some() { - // Call the identity fetching logic - if let Err(e) = self - .fetch_identity_and_contacts_for_asset_lock(wallet, network, tx) - .await - { - eprintln!("Failed to fetch identity for asset lock: {}", e); - } - } - } - } - } + if result.is_relevant { + use dashcore::transaction::special_transaction::TransactionPayload; + + if let Some(TransactionPayload::AssetLockPayloadType(_)) = &tx.special_transaction_payload { + if let Some(identity_manager) = self.identity_managers.get(&network) { + if identity_manager.sdk.is_some() { + if let Err(e) = self + .fetch_identity_and_contacts_for_asset_lock(wallet, network, tx) + .await + { + tracing::error!("Failed to fetch identity for asset lock: {}", e); + } + } + } + } + }This reduces one level of nesting by using
if letpattern matching instead ofmatches!followed by anotherif.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (2)
10-28: Consider guard statement instead of force unwrap.Line 24 uses force unwrap (
group.next()!) which is safe in the current implementation since two tasks are always added to the group. However, using a guard statement would make this safer against future refactoring mistakes and more explicit about the invariant.Apply this diff:
- // Return first result (either completion or timeout) - let result = try await group.next()! - group.cancelAll() - return result + // Return first result (either completion or timeout) + guard let result = try await group.next() else { + throw TimeoutError() // Should never happen since we added two tasks + } + group.cancelAll() + return result
581-602: Consider making timeout configurable.The 30-second timeout for storage clear operations provides good protection against indefinite hangs. However, the optimal timeout may vary based on storage size and device performance.
Consider making the timeout configurable:
public func clearSpvStorage(fullReset: Bool = true, timeoutSeconds: TimeInterval = 30) { // ... try await withTimeout(seconds: timeoutSeconds) { // clear operations } }Alternatively, use different timeouts for full reset vs. sync state clear:
let timeout = fullReset ? 30.0 : 10.0 try await withTimeout(seconds: timeout) { // clear operations }packages/rs-platform-wallet/src/identity_manager/mod.rs (2)
18-27: Consider hidingIdentityManagerfields behind accessorsYou already have dedicated accessors (
identity,identities,managed_identity,sdk, etc.), butidentities,primary_identity_id, andsdkare allpub. That allows external callers to mutate internal state directly and bypass invariants (e.g., primary identity handling) and leads to direct field access likemanager.sdkinstead of the accessor. Consider making these fields private and using the accessor methods throughout (including inmatured_transactions.rs) to keep the API surface tighter.
39-125: IdentityManager tests exercise core lifecycle wellThe test module covers adding/removing identities, primary identity switching, and basic
ManagedIdentitymetadata, which is a good baseline and matches intended behavior. If needed later, you could add a duplicate‑identity test, but nothing is blocking here.packages/rs-platform-wallet/src/platform_wallet_info/matured_transactions.rs (1)
242-328: ContactRequest document parsing is correct but drops optional fieldsThe parser defensively extracts
toUserId,senderKeyIndex,recipientKeyIndex,accountReference,encryptedPublicKey, and timestamps with clear error messages, which aligns with the DashPay contact request schema andContactRequest::new. However, any optional fields such as an encrypted account label or auto‑accept proof present on the document are currently discarded. If you plan to support those features in the wallet UI, consider extendingContactRequest(or adding a separate constructor) so this helper can preserve them instead of always defaulting toNone.packages/rs-platform-wallet/src/platform_wallet_info/contact_requests.rs (3)
29-147: add_sent_contact_request wiring and invariants look solidThis helper sensibly validates that the identity exists, enforces uniqueness of the DashPay receiving account at both wallet and managed‑wallet layers, derives and registers the account when missing, updates
last_used, and then delegates toManagedIdentity::add_sent_contact_request. The flow is coherent and keeps both representations in sync.One small nit:
send_contact_requestalso derives the same account path/xpub before calling this helper, so you’re doing the derivation work twice in that path. If you care about avoiding that duplication, consider plumbing the already‑derived xpub into this function.
323-471: Clarify behavior on local failure after a successful SDK send, and consider improving stored metadataThe wrapper around
sdk.send_contact_requestis nicely structured, but there’s a subtle behavior issue:
- If the SDK call succeeds (the contact request is on Platform) and
add_sent_contact_requestthen fails (e.g., wallet or managed-account write issues), the method returns an error even though the remote state was updated. Callers see it as a full failure, while the network state actually contains the request.If this is acceptable, it may be worth documenting it as a “remote success, local persistence error”; otherwise, consider making the local step more robust or providing a way for callers to reconcile.
Separately, the locally stored
ContactRequestuses a dummy 96‑byte zero vector forencrypted_public_keyand a hard-codedcore_height_created_atof100000. Since the real encrypted xpub and core height are available from the SDK/document, you may want to persist those to keep the local representation closer to what’s on Platform.
492-828: Good error-path coverage in tests; add at least one happy-path scenarioThe test module does a nice job covering:
- Identity-not-found errors for sent/incoming requests.
- Sender/recipient mismatches and missing/wrong-purpose keys.
- Accepting requests when identity or contact doesn’t exist.
What’s missing is any positive-path coverage for:
add_sent_contact_requestsuccessfully creating accounts and updating metadata.add_incoming_contact_requestsuccessfully wiring up an external account and adding the request.- The
send_contact_requestasync wrapper (even with a mocked SDK).Adding at least one success-case test per path would better guard the account-key wiring and would help catch issues like the encryptedPublicKey/xpub mismatch earlier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
packages/rs-dapi/Cargo.toml(1 hunks)packages/rs-platform-wallet/Cargo.toml(1 hunks)packages/rs-platform-wallet/src/established_contact.rs(1 hunks)packages/rs-platform-wallet/src/identity_manager/initializers.rs(1 hunks)packages/rs-platform-wallet/src/identity_manager/mod.rs(1 hunks)packages/rs-platform-wallet/src/platform_wallet_info/contact_requests.rs(1 hunks)packages/rs-platform-wallet/src/platform_wallet_info/matured_transactions.rs(1 hunks)packages/rs-platform-wallet/src/platform_wallet_info/wallet_info_interface.rs(1 hunks)packages/rs-platform-wallet/src/platform_wallet_info/wallet_transaction_checker.rs(1 hunks)packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift(2 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift(8 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-platform-wallet/src/identity_manager/initializers.rs
🧰 Additional context used
📓 Path-based instructions (4)
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/SPV/SPVClient.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-platform-wallet/src/platform_wallet_info/wallet_transaction_checker.rspackages/rs-platform-wallet/src/established_contact.rspackages/rs-platform-wallet/src/platform_wallet_info/matured_transactions.rspackages/rs-platform-wallet/src/platform_wallet_info/wallet_info_interface.rspackages/rs-platform-wallet/src/platform_wallet_info/contact_requests.rspackages/rs-platform-wallet/src/identity_manager/mod.rs
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/Services/WalletService.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.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/CoreContentView.swift
🧠 Learnings (23)
📓 Common learnings
Learnt from: shumkov
Repo: dashpay/platform PR: 2489
File: packages/rs-dpp/Cargo.toml:32-32
Timestamp: 2025-03-11T09:39:23.071Z
Learning: In the Dash Platform project, dependencies are currently managed using Git repository references with tags (repo+tag format in Cargo.toml) rather than published crates, as the team is not currently publishing crates to crates.io.
Learnt from: lklimek
Repo: dashpay/platform PR: 2254
File: packages/rs-sdk/src/sdk.rs:585-585
Timestamp: 2024-10-18T15:39:51.172Z
Learning: The 'platform' project uses Rust version 1.80, so code in 'packages/rs-sdk' can use features available in Rust 1.80, such as the `abs_diff()` method.
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
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/mimic/test_quorum.rs:159-164
Timestamp: 2024-11-20T16:16:01.830Z
Learning: QuantumExplorer prefers not to receive auto-generated messages asking to post on social media.
📚 Learning: 2024-10-29T14:40:54.727Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/src/core/transaction.rs:0-0
Timestamp: 2024-10-29T14:40:54.727Z
Learning: In `packages/rs-sdk/src/platform/document_query.rs` and `packages/rs-sdk/src/core/transaction.rs`, certain places don't implement `IntoInner`, so direct error mappings cannot be simplified using `IntoInner`. A TODO comment has been added to address this in a future PR.
Applied to files:
packages/rs-platform-wallet/src/platform_wallet_info/wallet_transaction_checker.rspackages/rs-platform-wallet/src/platform_wallet_info/matured_transactions.rs
📚 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/rs-platform-wallet/src/platform_wallet_info/wallet_transaction_checker.rs
📚 Learning: 2024-10-03T11:51:06.980Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2201
File: packages/rs-platform-version/src/version/v2.rs:1186-1188
Timestamp: 2024-10-03T11:51:06.980Z
Learning: In the `IdentityTransitionVersions` structure within `packages/rs-platform-version/src/version/v2.rs`, the field `credit_withdrawal` does not need the `identity_` prefix since it is already encompassed within identity state transitions.
Applied to files:
packages/rs-platform-wallet/src/platform_wallet_info/wallet_transaction_checker.rspackages/rs-platform-wallet/src/platform_wallet_info/matured_transactions.rspackages/rs-platform-wallet/src/identity_manager/mod.rs
📚 Learning: 2025-03-11T09:39:23.071Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2489
File: packages/rs-dpp/Cargo.toml:32-32
Timestamp: 2025-03-11T09:39:23.071Z
Learning: In the Dash Platform project, dependencies are currently managed using Git repository references with tags (repo+tag format in Cargo.toml) rather than published crates, as the team is not currently publishing crates to crates.io.
Applied to files:
packages/rs-dapi/Cargo.tomlpackages/rs-platform-wallet/Cargo.toml
📚 Learning: 2025-09-03T16:37:11.605Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2756
File: packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:11-11
Timestamp: 2025-09-03T16:37:11.605Z
Learning: In packages/rs-dpp/Cargo.toml, the abci feature already includes core_rpc_client, and core_rpc_client is defined as ["dep:dashcore-rpc"]. This means rs-drive-abci can access dashcore-rpc types through dpp when using the abci feature.
Applied to files:
packages/rs-dapi/Cargo.tomlpackages/rs-platform-wallet/Cargo.toml
📚 Learning: 2025-10-15T14:45:30.856Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2716
File: packages/dashmate/src/test/constants/services.js:4-4
Timestamp: 2025-10-15T14:45:30.856Z
Learning: In the dashmate codebase (packages/dashmate), during the DAPI Rust migration (rs-dapi), the old service keys `dapi_api` and `dapi_core_streams` are intentionally kept in `generateEnvsFactory.js` for backward compatibility even though the test constants in `src/test/constants/services.js` have been updated to use `rs_dapi`. These deprecated keys will be removed in a future PR after the transition is complete.
Applied to files:
packages/rs-dapi/Cargo.tomlpackages/rs-platform-wallet/Cargo.toml
📚 Learning: 2025-01-19T07:36:46.042Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2431
File: packages/rs-drive/Cargo.toml:55-60
Timestamp: 2025-01-19T07:36:46.042Z
Learning: The grovedb dependencies in packages/rs-drive/Cargo.toml and related files are intentionally kept at specific revisions rather than using the latest stable version, with plans to update them at a later time.
Applied to files:
packages/rs-dapi/Cargo.tomlpackages/rs-platform-wallet/Cargo.toml
📚 Learning: 2024-10-29T10:42:00.521Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-dapi-client/Cargo.toml:22-22
Timestamp: 2024-10-29T10:42:00.521Z
Learning: In `packages/rs-dapi-client/Cargo.toml`, `backon` will not work without the `tokio-sleep` feature in our setup, so it's unnecessary to declare `tokio-sleep` as a separate feature in the `[features]` section.
Applied to files:
packages/rs-dapi/Cargo.tomlpackages/rs-platform-wallet/Cargo.toml
📚 Learning: 2024-11-28T13:49:17.301Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2317
File: packages/rs-dapi-client/src/address_list.rs:175-180
Timestamp: 2024-11-28T13:49:17.301Z
Learning: In Rust code in `packages/rs-dapi-client/src/address_list.rs`, do not change the interface of deprecated methods like `add_uri`, even to fix potential panics.
Applied to files:
packages/rs-dapi/Cargo.toml
📚 Learning: 2024-12-05T09:29:38.918Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2375
File: packages/rs-drive-abci/Cargo.toml:61-63
Timestamp: 2024-12-05T09:29:38.918Z
Learning: In the `drive-abci` package, avoid adding unused dependencies like `hashbrown` to `Cargo.toml`. The team relies on CI to detect dependency version issues.
Applied to files:
packages/rs-dapi/Cargo.toml
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_owner_identity/v1/mod.rs:19-30
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In the Rust file `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_owner_identity/v1/mod.rs`, within the `create_owner_identity_v1` function, the `add_public_keys` method of the `Identity` struct cannot fail and does not require explicit error handling.
Applied to files:
packages/rs-platform-wallet/src/platform_wallet_info/matured_transactions.rspackages/rs-platform-wallet/src/identity_manager/mod.rs
📚 Learning: 2024-10-10T10:30:19.883Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2232
File: packages/rs-sdk/src/mock/sdk.rs:90-95
Timestamp: 2024-10-10T10:30:19.883Z
Learning: In `packages/rs-sdk/src/mock/sdk.rs`, the `load_expectations` method in `MockDashPlatformSdk` remains asynchronous (`async`) for backward compatibility, even though it now delegates to the synchronous `load_expectations_sync` method.
Applied to files:
packages/rs-platform-wallet/src/platform_wallet_info/matured_transactions.rspackages/rs-platform-wallet/Cargo.toml
📚 Learning: 2024-10-30T11:19:59.163Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/tests/fetch/config.rs:233-233
Timestamp: 2024-10-30T11:19:59.163Z
Learning: In the Rust SDK's `rs-sdk/tests` integration tests (e.g., in `packages/rs-sdk/tests/fetch/config.rs`), we cannot create objects during tests because there is no support for object creation in this context. Therefore, hardcoded values for test identities must be used.
Applied to files:
packages/rs-platform-wallet/src/platform_wallet_info/contact_requests.rspackages/rs-platform-wallet/src/identity_manager/mod.rs
📚 Learning: 2024-10-18T15:39:51.172Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2254
File: packages/rs-sdk/src/sdk.rs:585-585
Timestamp: 2024-10-18T15:39:51.172Z
Learning: The 'platform' project uses Rust version 1.80, so code in 'packages/rs-sdk' can use features available in Rust 1.80, such as the `abs_diff()` method.
Applied to files:
packages/rs-platform-wallet/Cargo.toml
📚 Learning: 2025-09-07T22:18:50.883Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/rs-sdk-ffi/**/*.{h,rs} : Follow unified SDK function prefixes: dash_core_sdk_* for Core, dash_sdk_* for Platform, dash_unified_sdk_* for unified APIs
Applied to files:
packages/rs-platform-wallet/Cargo.toml
📚 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/Services/WalletService.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift
📚 Learning: 2025-10-09T15:59:18.338Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2716
File: packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsReader.js:325-334
Timestamp: 2025-10-09T15:59:18.338Z
Learning: In `packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsReader.js`, the continuous sync restart intentionally uses `Math.max(1, lastSyncedBlockHeight)` rather than `lastSyncedBlockHeight + 1` because the last block's processing status is uncertain at restart time. This conservative approach ensures no blocks are missed by reprocessing the last synced block, accepting potential duplicate processing for data completeness.
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.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 : Use NavigationLink for drill-down navigation in the UI
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.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 : Implement proper loading and error states in UI screens
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.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 : Use SwiftUI with Query for reactive data in views
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.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/CoreContentView.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/CoreContentView.swift
🧬 Code graph analysis (7)
packages/rs-platform-wallet/src/platform_wallet_info/wallet_transaction_checker.rs (1)
packages/rs-platform-wallet/src/platform_wallet_info/mod.rs (1)
identity_manager(41-43)
packages/rs-platform-wallet/src/established_contact.rs (2)
packages/rs-platform-wallet/src/contact_request.rs (1)
new(46-68)packages/rs-platform-wallet/src/managed_identity/identity_ops.rs (1)
new(10-20)
packages/rs-platform-wallet/src/platform_wallet_info/matured_transactions.rs (4)
packages/rs-platform-wallet/src/established_contact.rs (1)
new(39-53)packages/rs-platform-wallet/src/identity_manager/initializers.rs (2)
new(12-14)sdk(31-33)packages/rs-platform-wallet/src/platform_wallet_info/mod.rs (1)
new(26-31)packages/rs-platform-wallet/src/identity_manager/accessors.rs (1)
managed_identity(62-64)
packages/rs-platform-wallet/src/platform_wallet_info/wallet_info_interface.rs (1)
packages/rs-platform-wallet/src/platform_wallet_info/mod.rs (1)
new(26-31)
packages/rs-platform-wallet/src/platform_wallet_info/contact_requests.rs (6)
packages/rs-platform-wallet/src/identity_manager/accessors.rs (2)
identity(13-15)managed_identity(62-64)packages/rs-platform-wallet/src/platform_wallet_info/accessors.rs (1)
identity(36-39)packages/rs-platform-wallet/src/managed_identity/contact_requests.rs (4)
add_sent_contact_request(15-27)add_incoming_contact_request(39-51)accept_incoming_request(63-79)create_contact_request(98-113)packages/rs-sdk/src/platform/dashpay/contact_request.rs (5)
from(76-78)from(82-84)send_contact_request(378-456)id(67-72)create_contact_request(164-354)packages/rs-platform-wallet/src/identity_manager/initializers.rs (2)
sdk(31-33)new(12-14)packages/rs-platform-wallet/src/contact_request.rs (1)
new(46-68)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (1)
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (3)
getBlocksWithTransactionsCount(528-531)clearStorage(475-492)clearSyncState(495-510)
packages/rs-platform-wallet/src/identity_manager/mod.rs (3)
packages/rs-platform-wallet/src/identity_manager/accessors.rs (3)
managed_identity(62-64)identity(13-15)identities(25-30)packages/rs-platform-wallet/src/identity_manager/initializers.rs (1)
new(12-14)packages/rs-platform-wallet/src/managed_identity/identity_ops.rs (2)
new(10-20)id(23-25)
🪛 SwiftLint (0.57.0)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift
[Warning] 325-325: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
🔇 Additional comments (23)
packages/rs-platform-wallet/src/platform_wallet_info/wallet_info_interface.rs (3)
18-30: LGTM! Constructors correctly initialize identity_managers.The factory methods properly delegate to
ManagedWalletInfoconstructors and consistently initializeidentity_managersas an emptyBTreeMap, matching the pattern used in thenew()method.
32-78: LGTM! Clean delegation pattern.All property accessors and
utxos()correctly delegate to the underlyingwallet_info, following the facade pattern appropriately.
88-155: LGTM! Remaining delegations are correct.All balance, transaction, account, and payment methods properly delegate to the underlying
wallet_infowith correct parameter passing and error handling.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift (4)
125-131: LGTM! Clear button properly disabled during sync/initialization.The disabled state is correctly implemented with appropriate visual feedback (gray background, reduced opacity) and matches the guard in
clearSyncData.
174-180: LGTM! Navigation properly implemented.The navigation destination for filter matches follows SwiftUI best practices, with environment objects correctly propagated to
FilterMatchesView.As per coding guidelines
305-313: LGTM! Defensive guard prevents unsafe operations.The guard correctly prevents clearing sync data during sync or initialization, with a diagnostic print for debugging. This aligns with the UI disabled state and prevents potential data corruption.
336-352: LGTM! Conditional navigation elegantly implemented.The conditional rendering based on
navigationDestinationis clean and follows SwiftUI patterns. When a destination is provided, only the label becomes tappable, which provides clear visual feedback for interactive elements.As per coding guidelines
packages/swift-sdk/Sources/SwiftDashSDK/SPV/SPVClient.swift (2)
512-531: LGTM! Clean transaction query API.The methods are well-documented, handle the uninitialized case appropriately, and provide a clean Swift API over the FFI boundary. The documentation clearly explains the persistence behavior and the meaning of a 0 return value.
230-233: Inconsistency with AI-generated summary.The AI summary states: "These accessors/methods appear in two locations in the file, effectively duplicating the public API surface." However, I only observe a single occurrence of the
clientHandleproperty and the transaction query methods in the provided code. The implementation does not appear to be duplicated.Also applies to: 512-531
packages/rs-platform-wallet/Cargo.toml (2)
12-12: Verify that dash-sdk feature selection is complete.The
dash-sdkdependency explicitly disables default features and only enables["dashpay-contract", "dpns-contract"]. Confirm that these two features are sufficient and that no other critical features (e.g., signing, persistence, or protocol features) are needed for platform wallet functionality.
12-13: Fix broken path dependencies that reference non-existentrust-dashcoredirectory.Verification confirms that dependencies on lines 14, 15, and 20 reference paths that do not exist:
key-wallet = { path = "../../../rust-dashcore/key-wallet" }key-wallet-manager = { path = "../../../rust-dashcore/key-wallet-manager", optional = true }dashcore = { path = "../../../rust-dashcore/dash" }The
../../../rust-dashcore/directory is not present in the repository, and these crates do not exist as workspace members. This will cause immediate build failures.These dependencies must either:
- Be sourced from crates.io with explicit versions, or
- Use Git repository references with tags (per established project pattern), or
- Be imported from a monorepo workspace structure that includes rust-dashcore as a member
Additionally, this pattern contradicts the established dependency management approach using Git refs with tags rather than local paths.
⛔ Skipped due to learnings
Learnt from: shumkov Repo: dashpay/platform PR: 2489 File: packages/rs-dpp/Cargo.toml:32-32 Timestamp: 2025-03-11T09:39:23.071Z Learning: In the Dash Platform project, dependencies are currently managed using Git repository references with tags (repo+tag format in Cargo.toml) rather than published crates, as the team is not currently publishing crates to crates.io.Learnt from: QuantumExplorer Repo: dashpay/platform PR: 2431 File: packages/rs-drive/Cargo.toml:55-60 Timestamp: 2025-01-19T07:36:46.042Z Learning: The grovedb dependencies in packages/rs-drive/Cargo.toml and related files are intentionally kept at specific revisions rather than using the latest stable version, with plans to update them at a later time.Learnt from: shumkov Repo: dashpay/platform PR: 2375 File: packages/rs-drive-abci/Cargo.toml:61-63 Timestamp: 2024-12-05T09:29:38.918Z Learning: In the `drive-abci` package, avoid adding unused dependencies like `hashbrown` to `Cargo.toml`. The team relies on CI to detect dependency version issues.Learnt from: lklimek Repo: dashpay/platform PR: 1924 File: packages/rs-sdk/src/sdk.rs:855-870 Timestamp: 2024-12-03T15:00:50.385Z Learning: In `packages/rs-sdk/src/sdk.rs`, remember that `rustls_pemfile::certs` returns an iterator over `Result`, not a `Result` directly. Handle it accordingly in future code reviews.Learnt from: lklimek Repo: dashpay/platform PR: 2277 File: packages/rs-dapi-client/Cargo.toml:22-22 Timestamp: 2024-10-29T10:42:00.521Z Learning: In `packages/rs-dapi-client/Cargo.toml`, `backon` will not work without the `tokio-sleep` feature in our setup, so it's unnecessary to declare `tokio-sleep` as a separate feature in the `[features]` section.Learnt from: CR Repo: dashpay/platform PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-09-07T22:18:50.883Z Learning: Applies to packages/rs-sdk-ffi/**/*.{h,rs} : Follow unified SDK function prefixes: dash_core_sdk_* for Core, dash_sdk_* for Platform, dash_unified_sdk_* for unified APIsLearnt from: lklimek Repo: dashpay/platform PR: 2254 File: packages/rs-sdk/src/sdk.rs:585-585 Timestamp: 2024-10-18T15:39:51.172Z Learning: The 'platform' project uses Rust version 1.80, so code in 'packages/rs-sdk' can use features available in Rust 1.80, such as the `abs_diff()` method.Learnt from: lklimek Repo: dashpay/platform PR: 2277 File: packages/rs-sdk/src/core/transaction.rs:0-0 Timestamp: 2024-10-29T14:40:54.727Z Learning: In `packages/rs-sdk/src/platform/document_query.rs` and `packages/rs-sdk/src/core/transaction.rs`, certain places don't implement `IntoInner`, so direct error mappings cannot be simplified using `IntoInner`. A TODO comment has been added to address this in a future PR.Learnt from: lklimek Repo: dashpay/platform PR: 2716 File: packages/dashmate/src/test/constants/services.js:4-4 Timestamp: 2025-10-15T14:45:30.856Z Learning: In the dashmate codebase (packages/dashmate), during the DAPI Rust migration (rs-dapi), the old service keys `dapi_api` and `dapi_core_streams` are intentionally kept in `generateEnvsFactory.js` for backward compatibility even though the test constants in `src/test/constants/services.js` have been updated to use `rs_dapi`. These deprecated keys will be removed in a future PR after the transition is complete.Learnt from: QuantumExplorer Repo: dashpay/platform PR: 2257 File: packages/rs-drive-abci/tests/strategy_tests/masternodes.rs:212-220 Timestamp: 2024-11-20T20:43:41.185Z Learning: In `packages/rs-drive-abci/tests/strategy_tests/masternodes.rs`, the pattern of generating a `PrivateKey`, converting it to bytes, and reconstructing a `BlsPrivateKey` from those bytes is intentional. Avoid suggesting to simplify this code in future reviews.Learnt from: QuantumExplorer Repo: dashpay/platform PR: 2756 File: packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:11-11 Timestamp: 2025-09-03T16:37:11.605Z Learning: In packages/rs-dpp/Cargo.toml, the abci feature already includes core_rpc_client, and core_rpc_client is defined as ["dep:dashcore-rpc"]. This means rs-drive-abci can access dashcore-rpc types through dpp when using the abci feature.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_* prefixLearnt from: lklimek Repo: dashpay/platform PR: 2232 File: packages/rs-sdk/src/mock/sdk.rs:90-95 Timestamp: 2024-10-10T10:30:19.883Z Learning: In `packages/rs-sdk/src/mock/sdk.rs`, the `load_expectations` method in `MockDashPlatformSdk` remains asynchronous (`async`) for backward compatibility, even though it now delegates to the synchronous `load_expectations_sync` method.packages/rs-platform-wallet/src/established_contact.rs (5)
10-35: LGTM!The struct definition is well-documented and appropriately models a bidirectional contact relationship. Public fields are suitable for the FFI context mentioned in the PR objectives.
37-53: LGTM!The constructor correctly initializes all fields with sensible defaults. The implementation is clean and straightforward.
55-73: LGTM!The alias and note management methods follow a consistent and idiomatic pattern for managing optional fields.
75-83: LGTM!The visibility toggle methods are straightforward and correctly implemented.
98-215: LGTM!The test module provides comprehensive coverage of all public API methods with clear, well-structured tests. The inclusion of edge case testing (e.g., duplicate account handling) demonstrates thorough validation.
packages/rs-platform-wallet/src/platform_wallet_info/wallet_transaction_checker.rs (3)
1-11: LGTM: Clean trait implementation setup.The imports are well-organized and the async_trait implementation is appropriate for async trait methods.
12-24: LGTM: Correct delegation pattern.The method correctly delegates to the underlying wallet_info while preserving all parameters and the async signature.
49-49: Good design: Original result preserved regardless of fetch outcome.Returning the original
resultensures that identity/contact fetching failures don't break the transaction check flow, which is the correct behavior for this post-processing hook.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift (2)
246-318: LGTM! Proper initialization lifecycle management.The initialization flow properly tracks state with the
isInitializingflag and handles both success and error cases correctly. The restoration of persistent blocks hit count with overflow protection is well-implemented, and error handling ensures the flag is always reset even on failure.
469-474: LGTM! Proper state restoration on startup.The persistent blocks hit count restoration is well-implemented with appropriate overflow protection and logging. This ensures UI counters reflect the actual wallet state from persistent storage rather than starting from zero.
packages/rs-platform-wallet/src/platform_wallet_info/matured_transactions.rs (1)
32-69: Single‑transaction wrapper is reasonable but depends on metadata never being neededThe helper builds an
ImmatureTransactionwith synthetic block metadata and delegates to the main routine. Given the comment that only the transaction body matters for identity discovery, this is fine for now, but if later logic starts usingheight/block_hash/maturity_confirmations, this wrapper will need to be revisited to pass through real values where available.packages/rs-platform-wallet/src/platform_wallet_info/contact_requests.rs (1)
475-489: accept_incoming_request correctly delegates and normalizes errorsThis method cleanly looks up the managed identity, delegates to
ManagedIdentity::accept_incoming_request, and converts theOptioninto either the established contact or aContactRequestNotFounderror. The behavior is straightforward and matches the managed-identity API.
| dashcore-rpc = { path = "../../../rust-dashcore/rpc-client" } | ||
| dash-spv = { path = "../../../rust-dashcore/dash-spv" } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify local path dependencies exist and are properly referenced.
The change from remote git references to local path dependencies (../../../rust-dashcore/rpc-client and ../../../rust-dashcore/dash-spv) represents a significant shift in dependency management. While this aligns with the PR's stated objective to prefer local path dependencies, cross-workspace references can be fragile.
Please verify:
- Both paths exist relative to the repository root and are correctly resolved
- The local
rust-dashcoreworkspace is properly initialized and discoverable by Cargo - No circular dependency chains are introduced
- Feature flags and crate visibility are preserved when using local paths vs. remote git references
Run the following script to confirm the paths are accessible:
🏁 Script executed:
#!/bin/bash
# Description: Verify local path dependencies exist and can be built
# Check if path dependencies exist
echo "=== Checking path existence ==="
test -d "rust-dashcore/rpc-client" && echo "✓ rpc-client path exists" || echo "✗ rpc-client path NOT found"
test -d "rust-dashcore/dash-spv" && echo "✓ dash-spv path exists" || echo "✗ dash-spv path NOT found"
# Verify Cargo can resolve the dependencies without errors
echo -e "\n=== Verifying Cargo can resolve dependencies ==="
cd packages/rs-dapi
cargo check --lib 2>&1 | head -20Length of output: 245
</shell_calls>
🏁 Script executed:
#!/bin/bash
# Discover the actual repository structure and locate the missing directories
echo "=== Repository root structure ==="
ls -la | head -20
echo -e "\n=== Looking for 'rpc-client' directory anywhere in repo ==="
find . -maxdepth 3 -type d -name "rpc-client" 2>/dev/null | head -10
echo -e "\n=== Looking for 'dash-spv' directory anywhere in repo ==="
find . -maxdepth 3 -type d -name "dash-spv" 2>/dev/null | head -10
echo -e "\n=== Looking for 'rust-dashcore' directory ==="
find . -maxdepth 2 -type d -name "rust-dashcore" 2>/dev/null
echo -e "\n=== Content of packages/rs-dapi/Cargo.toml (lines 80-100) ==="
sed -n '80,100p' packages/rs-dapi/Cargo.tomlLength of output: 2049
🏁 Script executed:
#!/bin/bash
# Check repository root structure and workspace configuration
echo "=== Repository root listing ==="
ls -la
echo -e "\n=== Top-level Cargo.toml (if exists) ==="
if [ -f "Cargo.toml" ]; then
cat Cargo.toml | head -30
else
echo "No Cargo.toml at repository root"
fi
echo -e "\n=== Looking for all directories named 'rust-dashcore' ==="
find . -maxdepth 2 -type d -name "*dashcore*" 2>/dev/null
echo -e "\n=== Checking packages directory structure ==="
ls -la packages/ 2>/dev/null | head -20
echo -e "\n=== Looking for Cargo.toml files at top level and subdirs ==="
find . -maxdepth 2 -name "Cargo.toml" -type f 2>/dev/null | head -20Length of output: 4539
Incorrect path references will cause build failures.
The local path dependencies in lines 88-89 reference non-existent directories:
-
dashcore-rpc(line 88): Path../../../rust-dashcore/rpc-clientdoes not exist. Therpc-clientdirectory is not present anywhere in the repository. -
dash-spv(line 89): Whiledash-spvexists in the repository atpackages/dash-spv, the path is incorrectly specified as../../../rust-dashcore/dash-spv. The correct relative path frompackages/rs-dapi/is../dash-spv(matching the pattern used for other workspace dependencies likedppanddapi-grpc).
Both dependencies will fail to resolve during build. Correct the paths to either:
- Point to the actual locations within the workspace (e.g.,
../dash-spvfor dash-spv) - Or remove
dashcore-rpcentirely if it cannot be sourced from the repository
🤖 Prompt for AI Agents
In packages/rs-dapi/Cargo.toml around lines 88 to 89, the two local path
dependencies point to non-existent locations; update dash-spv to the correct
workspace relative path and remove or fix dashcore-rpc: change the dash-spv path
from "../../../rust-dashcore/dash-spv" to "../dash-spv" to match the workspace
layout, and for dashcore-rpc either remove that dependency if the rpc-client
crate is not present in the repository or replace its path with the actual
relative path to the crate if it exists elsewhere in the repo; ensure Cargo.toml
paths are valid and run cargo metadata/build to verify resolution.
| #[allow(unused_imports)] | ||
| use crate::ContactRequest; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove the incorrect #[allow(unused_imports)] attribute.
The ContactRequest type is actively used in the struct fields (lines 19, 22) and throughout the test module. This attribute suppresses a warning that wouldn't occur.
Apply this diff to remove the unnecessary attribute:
-#[allow(unused_imports)]
use crate::ContactRequest;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[allow(unused_imports)] | |
| use crate::ContactRequest; | |
| use crate::ContactRequest; |
🤖 Prompt for AI Agents
In packages/rs-platform-wallet/src/established_contact.rs around lines 6 to 7,
remove the incorrect #[allow(unused_imports)] attribute applied to the
ContactRequest import; ContactRequest is used in struct fields and tests, so
simply delete the #[allow(unused_imports)] line so the import is a normal use
statement (use crate::ContactRequest;) and leave the rest of the file unchanged.
| pub(crate) fn add_incoming_contact_request( | ||
| &mut self, | ||
| wallet: &mut Wallet, | ||
| network: Network, | ||
| identity_id: &Identifier, | ||
| friend_identity: &Identity, | ||
| request: ContactRequest, | ||
| ) -> Result<(), PlatformWalletError> { | ||
| if self | ||
| .identity_manager(network) | ||
| .and_then(|manager| manager.managed_identity(identity_id)) | ||
| .is_none() | ||
| { | ||
| return Err(PlatformWalletError::IdentityNotFound(*identity_id)); | ||
| } | ||
|
|
||
| if friend_identity.id() != request.sender_id { | ||
| return Err(PlatformWalletError::InvalidIdentityData( | ||
| "Incoming contact request sender does not match provided identity".to_string(), | ||
| )); | ||
| } | ||
|
|
||
| let sender_key = friend_identity | ||
| .public_keys() | ||
| .get(&request.sender_key_index) | ||
| .ok_or_else(|| { | ||
| PlatformWalletError::InvalidIdentityData( | ||
| "Sender identity is missing the declared encryption key".to_string(), | ||
| ) | ||
| })?; | ||
|
|
||
| if sender_key.purpose() != Purpose::ENCRYPTION { | ||
| return Err(PlatformWalletError::InvalidIdentityData( | ||
| "Sender key purpose must be ENCRYPTION".to_string(), | ||
| )); | ||
| } | ||
|
|
||
| if self | ||
| .identity_manager(network) | ||
| .and_then(|manager| manager.managed_identity(identity_id)) | ||
| .and_then(|managed| { | ||
| managed | ||
| .identity | ||
| .public_keys() | ||
| .get(&request.recipient_key_index) | ||
| }) | ||
| .is_none() | ||
| { | ||
| return Err(PlatformWalletError::InvalidIdentityData( | ||
| "Recipient identity is missing the declared encryption key".to_string(), | ||
| )); | ||
| } | ||
|
|
||
| let request_created_at = request.created_at; | ||
| let friend_identity_id = request.sender_id.to_buffer(); | ||
| let friend_identity_identifier = Identifier::from(friend_identity_id); | ||
| let user_identity_id = identity_id.to_buffer(); | ||
| let account_index = request.account_reference; | ||
| let encrypted_public_key = request.encrypted_public_key.clone(); | ||
|
|
||
| let account_key = DashpayAccountKey { | ||
| index: account_index, | ||
| user_identity_id, | ||
| friend_identity_id, | ||
| }; | ||
|
|
||
| let account_type = AccountType::DashpayExternalAccount { | ||
| index: account_index, | ||
| user_identity_id, | ||
| friend_identity_id, | ||
| }; | ||
|
|
||
| let wallet_has_account = wallet | ||
| .accounts | ||
| .get(&network) | ||
| .and_then(|collection| collection.account_of_type(account_type)) | ||
| .is_some(); | ||
|
|
||
| if wallet_has_account { | ||
| return Err(PlatformWalletError::DashpayExternalAccountAlreadyExists { | ||
| identity: *identity_id, | ||
| contact: friend_identity_identifier, | ||
| network, | ||
| account_index, | ||
| }); | ||
| } | ||
|
|
||
| let account_xpub = ExtendedPubKey::decode(&encrypted_public_key).map_err(|err| { | ||
| PlatformWalletError::InvalidIdentityData(format!( | ||
| "Failed to decode DashPay contact account xpub: {err}" | ||
| )) | ||
| })?; | ||
|
|
||
| wallet | ||
| .add_account(account_type, network, Some(account_xpub)) | ||
| .map_err(|err| { | ||
| PlatformWalletError::InvalidIdentityData(format!( | ||
| "Failed to add DashPay external account to wallet: {err}" | ||
| )) | ||
| })?; | ||
|
|
||
| let managed_has_account = self | ||
| .wallet_info | ||
| .accounts(network) | ||
| .and_then(|collection| collection.dashpay_external_accounts.get(&account_key)) | ||
| .is_some(); | ||
|
|
||
| if managed_has_account { | ||
| return Err(PlatformWalletError::DashpayExternalAccountAlreadyExists { | ||
| identity: *identity_id, | ||
| contact: friend_identity_identifier, | ||
| network, | ||
| account_index, | ||
| }); | ||
| } | ||
|
|
||
| self.wallet_info | ||
| .add_managed_account(wallet, account_type, network) | ||
| .map_err(|err| { | ||
| PlatformWalletError::InvalidIdentityData(format!( | ||
| "Failed to add managed DashPay external account: {err}" | ||
| )) | ||
| })?; | ||
|
|
||
| let managed_account_collection = self | ||
| .wallet_info | ||
| .accounts_mut(network) | ||
| .ok_or(PlatformWalletError::NoAccountsForNetwork(network))?; | ||
|
|
||
| let managed_account = managed_account_collection | ||
| .dashpay_external_accounts | ||
| .get_mut(&account_key) | ||
| .ok_or_else(|| { | ||
| PlatformWalletError::InvalidIdentityData( | ||
| "Managed DashPay external account is missing".to_string(), | ||
| ) | ||
| })?; | ||
|
|
||
| managed_account.metadata.last_used = Some(request_created_at); | ||
|
|
||
| self.identity_manager_mut(network) | ||
| .managed_identity_mut(identity_id) | ||
| .ok_or(PlatformWalletError::IdentityNotFound(*identity_id))? | ||
| .add_incoming_contact_request(request); | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Encrypted contact account key is treated as a raw xpub – incoming requests will fail to attach accounts
This function does good validation on identities and key purposes, but the account-creation step appears inconsistent with how contact requests are produced:
parse_contact_request_document(inmatured_transactions.rs) builds aContactRequestby copying the document’sencryptedPublicKeybytes.- In the SDK (
rs-sdk/src/platform/dashpay/contact_request.rs), that field is generated as an AES-encrypted extended public key (a fixed 96‑byte blob), not a plain BIP32 xpub. - Here,
add_incoming_contact_requestpassesrequest.encrypted_public_keydirectly intoExtendedPubKey::decode, which expects a raw xpub encoding.
Unless you decrypt encrypted_public_key somewhere before calling this method, ExtendedPubKey::decode will consistently fail for real contact requests, and accepting an incoming request will never create the corresponding DashPay external account. You likely need a decryption step (via the SDK or platform-encryption) that turns the encrypted blob into the raw xpub bytes, or a different field in ContactRequest that holds the decrypted xpub for local use.
Also, tests currently only cover early‑exit error paths, so this mismatch wouldn’t be caught by them.
I'll verify the review comment by examining the relevant code sections and their interactions.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find parse_contact_request_document and check how it handles encryptedPublicKey
rg -n "parse_contact_request_document" --type rs -A 30
Length of output: 26
🏁 Script executed:
#!/bin/bash
# Find the ContactRequest struct definition
rg -n "struct ContactRequest" --type rs -A 10Length of output: 26
🏁 Script executed:
#!/bin/bash
# Find ExtendedPubKey::decode usage
rg -n "ExtendedPubKey::decode" --type rs -B 3 -A 3Length of output: 26
🏁 Script executed:
#!/bin/bash
# Search for encryption/decryption related to contact requests
rg -n "encrypted_public_key" --type rs -B 2 -A 2Length of output: 26
🏁 Script executed:
#!/bin/bash
# Find tests for add_incoming_contact_request
rg -n "add_incoming_contact_request" --type rs -B 2 -A 15Length of output: 26
🏁 Script executed:
#!/bin/bash
# Search for platform-encryption usage with contact requests
rg -n "decrypt|Decrypt" --type rs | grep -i contactLength of output: 26
Let me fix the ripgrep file type and search more broadly:
---
🏁 Script executed:
```shell
#!/bin/bash
# Search using correct rust type and without type restriction
rg -n "parse_contact_request_document" -A 30
Length of output: 9401
🏁 Script executed:
#!/bin/bash
# Find the ContactRequest struct definition
rg -n "struct ContactRequest" -A 10Length of output: 5151
🏁 Script executed:
#!/bin/bash
# Find ExtendedPubKey::decode usage in contact context
rg -n "ExtendedPubKey::decode" -B 3 -A 3Length of output: 802
🏁 Script executed:
#!/bin/bash
# Search for encrypted_public_key field references
rg -n "encrypted_public_key" -B 2 -A 2Length of output: 16291
🏁 Script executed:
#!/bin/bash
# Find test files related to contact_requests
fd ".*test.*contact" --type fLength of output: 0
🏁 Script executed:
#!/bin/bash
# Find matured_transactions.rs file
fd "matured_transactions" --type fLength of output: 76
Let me search for decryption logic and the context available in add_incoming_contact_request:Now let me search more specifically for the decrypt function in the contact request codebase:
---
🏁 Script executed:
```shell
#!/bin/bash
# Look for decrypt functions in contact request related code
rg -n "decrypt" packages/rs-sdk/src/platform/dashpay/contact_request.rs -B 3 -A 3
Length of output: 273
🏁 Script executed:
#!/bin/bash
# Look for ECDH key derivation or shared key generation in the wallet code
rg -n "shared_key\|derive.*key\|ecdh" packages/rs-platform-wallet/src/platform_wallet_info/contact_requests.rs -B 2 -A 2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there's a decrypt function being called anywhere in contact_requests.rs
rg -n "decrypt\|Decrypt" packages/rs-platform-wallet/src/platform_wallet_info/contact_requests.rs -B 3 -A 3Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for the full parse_contact_request_document implementation to see what it does
rg -n "fn parse_contact_request_document" packages/rs-platform-wallet/src/platform_wallet_info/matured_transactions.rs -A 100 | head -120Length of output: 2917
Encrypted contact account key is treated as a raw xpub – incoming requests will fail to attach accounts
This function does good validation on identities and key purposes, but the account-creation step appears inconsistent with how contact requests are produced:
parse_contact_request_document(inmatured_transactions.rs) builds aContactRequestby copying the document'sencryptedPublicKeybytes.- In the SDK (
rs-sdk/src/platform/dashpay/contact_request.rs), that field is generated as an AES-encrypted extended public key (a fixed 96‑byte blob), not a plain BIP32 xpub. - Here,
add_incoming_contact_requestpassesrequest.encrypted_public_keydirectly intoExtendedPubKey::decode, which expects a raw xpub encoding.
Unless you decrypt encrypted_public_key somewhere before calling this method, ExtendedPubKey::decode will consistently fail for real contact requests, and accepting an incoming request will never create the corresponding DashPay external account. You likely need a decryption step (via the SDK or platform-encryption) that turns the encrypted blob into the raw xpub bytes, or a different field in ContactRequest that holds the decrypted xpub for local use.
Also, tests currently only cover early‑exit error paths, so this mismatch wouldn't be caught by them.
| .fetch_identity_and_contacts_for_asset_lock(wallet, network, tx) | ||
| .await | ||
| { | ||
| eprintln!("Failed to fetch identity for asset lock: {}", e); |
There was a problem hiding this comment.
Replace eprintln! with proper logging framework.
Using eprintln! for error logging is not suitable for production code. The Dash Platform codebase should use a proper logging framework like tracing for structured logging with appropriate log levels.
Apply this diff to use proper logging:
- {
- eprintln!("Failed to fetch identity for asset lock: {}", e);
- }
+ {
+ tracing::error!("Failed to fetch identity for asset lock: {}", e);
+ }Make sure tracing is added as a dependency if not already present.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
packages/rs-platform-wallet/src/platform_wallet_info/wallet_transaction_checker.rs
around line 42, replace the eprintln! call with the tracing logging macro:
import tracing (add use tracing::error; at top) and change eprintln!("Failed to
fetch identity for asset lock: {}", e); to error!("Failed to fetch identity for
asset lock: {}", e); and ensure tracing is added to Cargo.toml (e.g., tracing =
"0.1") and enabled in the crate so structured, leveled logging is used instead
of eprintln!.
| public var spvClientHandle: UnsafeMutablePointer<FFIDashSpvClient>? { | ||
| spvClient?.clientHandle | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify public exposure of raw FFI pointer.
Exposing a raw UnsafeMutablePointer<FFIDashSpvClient>? as a public API could be a safety concern. Misuse of this pointer by external consumers could lead to memory corruption or crashes.
Consider whether:
- External consumers truly need direct access to the FFI handle
- A safer abstraction (e.g., specific query methods) would be more appropriate
- At minimum, this should be documented with clear usage guidelines and safety warnings
Run the following script to check how this property is used across the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Find all usages of spvClientHandle to verify access patterns
# Search for direct property access
rg -nP --type=swift -C3 '\bspvClientHandle\b'
# Search for FFIDashSpvClient usage patterns
rg -nP --type=swift -C3 '\bFFIDashSpvClient\b'Length of output: 4669
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for all references to spvClientHandle including call sites
rg -nP --type=swift 'spvClientHandle' --no-heading
# Also check if it's referenced in comments or documentation
fd -e md -e txt | xargs grep -l 'spvClientHandle' 2>/dev/null || true
# Check the git history to understand intent
git log --oneline -S 'spvClientHandle' -- '*.swift' | head -20 || true
Length of output: 182
🏁 Script executed:
#!/bin/bash
# Check for any test files that might use spvClientHandle
fd -e swift 'test|spec|Test|Spec' | xargs grep -l 'spvClientHandle' 2>/dev/null || echo "No test files using spvClientHandle"Length of output: 35
Remove unused spvClientHandle property or provide usage pattern.
spvClientHandle is defined but never used anywhere in the codebase, including tests. Since SPVClient.clientHandle already exposes the same raw FFI pointer publicly, this appears to be an unnecessary wrapper.
If this property is intended for external consumers, include a concrete usage example or documentation. If not actively needed, remove it to keep the public API surface clean.
🤖 Prompt for AI Agents
In
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/WalletService.swift
around lines 173-175, the public property spvClientHandle is an unused wrapper
around spvClient.clientHandle; remove the spvClientHandle property declaration
to shrink the public API surface, update any references if they exist (there
should be none), run the project build and tests to confirm no breakage, and
alternatively if this was intended for external consumers reintroduce it instead
with a documented usage example in the API comments and a unit test
demonstrating the intended access pattern.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swift (2)
38-53: Consider adding a timeout or cancellation check to the polling loop.The
while walletService.isSyncingloop could run indefinitely if sync never completes. Additionally, if the user navigates away, the Task continues running.Consider checking for Task cancellation:
while walletService.isSyncing { + if Task.isCancelled { return } try? await Task.sleep(nanoseconds: 500_000_000) // 0.5 seconds }Alternatively, add a maximum wait time to prevent indefinite polling.
186-221: Unusedproxyparameter inScrollViewReader.The
proxyparameter is declared but never used. If programmatic scrolling is not needed, remove theScrollViewReaderwrapper. Otherwise, implement the scrolling functionality.- private var filtersList: some View { - ScrollViewReader { proxy in - List { + private var filtersList: some View { + List { ForEach(Array(filteredFilters.enumerated()), id: \.element.id) { index, filter in // ... } // ... } .listStyle(.plain) - } }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Models/FilterMatch.swift (2)
11-41: LGTM! Clean and safe FFI data model.The
CompactFilterstruct correctly handles FFI data conversion with proper nil-checking and memory copying. TheData(bytes:count:)initializer ensures bytes are copied, making the Swift object safe even if the FFI layer frees the underlying memory.Optional enhancement: Consider adding
Equatableconformance to facilitate testing and comparisons:struct CompactFilter: Identifiable, Equatable { // ... existing code }
76-108: LGTM! Correct FFI tuple conversion.The tuple-to-Data conversion (lines 99-103) using
withUnsafeBytesis the appropriate technique for converting fixed-size FFI tuples to SwiftData. The closure captures and copies the bytes, ensuring memory safety.Optional enhancement: Consider adding
Equatableconformance for easier testing:struct FilterMatchEntry: Identifiable, Equatable { // ... existing code }packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/FilterMatchService.swift (1)
262-277: Consider simplifying duplicate removal logic.The current approach works correctly but could be streamlined using Set operations for better readability.
- // Merge with existing filters and sort - var allFilters = filters + compactFilters.filters - allFilters.sort { $0.height > $1.height } // Descending order - - // Remove duplicates by height - var seenHeights = Set<UInt32>() - allFilters = allFilters.filter { filter in - if seenHeights.contains(filter.height) { - return false - } else { - seenHeights.insert(filter.height) - return true - } - } - - filters = allFilters + // Merge with existing filters, remove duplicates, and sort + let allFiltersDict = Dictionary( + (filters + compactFilters.filters).map { ($0.height, $0) }, + uniquingKeysWith: { first, _ in first } + ) + filters = allFiltersDict.values.sorted { $0.height > $1.height }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Models/FilterMatch.swift(1 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/FilterMatchService.swift(1 hunks)packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swift(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/swift-sdk/**/*.{swift,h,m,c}
📄 CodeRabbit inference engine (CLAUDE.md)
iOS SDK must combine both Core (SPV wallet) and Platform (identity/documents) functionality in the unified SDK with proper function naming:
dash_core_sdk_*prefix for Core functions,dash_sdk_*prefix for Platform functions, anddash_unified_sdk_*prefix for unified functions
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Models/FilterMatch.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/FilterMatchService.swift
**/swift-sdk/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/swift-sdk/**/*.swift: Swift code should properly wrap FFI functions and maintain type safety across the Swift/Rust boundary
Make DPP types public in Swift to ensure proper visibility across module boundaries
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Models/FilterMatch.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/FilterMatchService.swift
**/swift-sdk/**/*.{h,swift}
📄 CodeRabbit inference engine (CLAUDE.md)
iOS framework must handle C header issues by using pointers for opaque FFI types
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Models/FilterMatch.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/FilterMatchService.swift
**/SwiftExampleApp/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/SwiftExampleApp/**/*.swift: Use SwiftUI for UI development in SwiftExampleApp
Use SwiftData for data persistence in SwiftExampleApp
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Models/FilterMatch.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/FilterMatchService.swift
packages/swift-sdk/SwiftExampleApp/**/*.swift
📄 CodeRabbit inference engine (packages/swift-sdk/SwiftExampleApp/CLAUDE.md)
packages/swift-sdk/SwiftExampleApp/**/*.swift: Usedash_core_sdk_*prefix for Core SDK functions,dash_sdk_*prefix for Platform SDK functions, anddash_unified_sdk_*prefix for Unified SDK functions
Use SwiftData for local persistence with key models: PersistentIdentity, PersistentDocument, PersistentContract, PersistentToken, PersistentTokenBalance, and PersistentPublicKey
Always validate private keys match their public keys using KeyValidation.validatePrivateKeyForPublicKey before cryptographic operations
Use UnifiedAppState to coordinate Core and Platform features, WalletService for SPV wallet operations, PlatformService for identity and document operations, DataManager for SwiftData persistence, and KeychainManager for secure key storage
All Codable types must be Equatable for SwiftData predicates to function correctly
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Models/FilterMatch.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/FilterMatchService.swift
packages/swift-sdk/SwiftExampleApp/**/*View.swift
📄 CodeRabbit inference engine (packages/swift-sdk/SwiftExampleApp/CLAUDE.md)
packages/swift-sdk/SwiftExampleApp/**/*View.swift: Use @query decorator with SwiftData for reactive data binding in SwiftUI views
Break complex SwiftUI views into smaller components to avoid compiler timeouts
Use NavigationLink for drill-down navigation in SwiftUI
Implement proper loading and error states in SwiftUI views
Use English plural forms for token display names
Files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swift
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Applies to **/swift-sdk/**/*.{swift,h,m,c} : iOS SDK must combine both Core (SPV wallet) and Platform (identity/documents) functionality in the unified SDK with proper function naming: `dash_core_sdk_*` prefix for Core functions, `dash_sdk_*` prefix for Platform functions, and `dash_unified_sdk_*` prefix for unified functions
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Use Rust for core platform components (Drive, DAPI server, DPP implementation)
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:38.019Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*.swift : Use UnifiedAppState to coordinate Core and Platform features, WalletService for SPV wallet operations, PlatformService for identity and document operations, DataManager for SwiftData persistence, and KeychainManager for secure key storage
Learnt from: shumkov
Repo: dashpay/platform PR: 2489
File: packages/rs-dpp/Cargo.toml:32-32
Timestamp: 2025-03-11T09:39:23.071Z
Learning: In the Dash Platform project, dependencies are currently managed using Git repository references with tags (repo+tag format in Cargo.toml) rather than published crates, as the team is not currently publishing crates to crates.io.
Learnt from: lklimek
Repo: dashpay/platform PR: 2254
File: packages/rs-sdk/src/sdk.rs:585-585
Timestamp: 2024-10-18T15:39:51.172Z
Learning: The 'platform' project uses Rust version 1.80, so code in 'packages/rs-sdk' can use features available in Rust 1.80, such as the `abs_diff()` method.
Learnt from: lklimek
Repo: dashpay/platform PR: 2318
File: .github/workflows/tests-build-image.yml:45-45
Timestamp: 2024-11-13T10:31:30.891Z
Learning: In the dashpay/platform repository, changes to `.github/workflows/tests-build-image.yml` that switch the Docker image platform from `linux/arm64` to `linux/amd64` for testing purposes are acceptable when required to run on GitHub-hosted runners. ARM64 testing is covered on the testnet.
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/mimic/test_quorum.rs:159-164
Timestamp: 2024-11-20T16:16:01.830Z
Learning: QuantumExplorer prefers not to receive auto-generated messages asking to post on social media.
📚 Learning: 2025-11-25T13:10:38.019Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:38.019Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*View.swift : Implement proper loading and error states in SwiftUI views
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swift
📚 Learning: 2025-11-25T13:10:38.019Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:38.019Z
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/FilterMatchesView.swift
📚 Learning: 2025-11-25T13:10:38.019Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:38.019Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*View.swift : Use Query decorator with SwiftData for reactive data binding in SwiftUI views
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swift
📚 Learning: 2025-11-25T13:10:23.481Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Applies to **/SwiftExampleApp/**/*.swift : Use SwiftUI for UI development in SwiftExampleApp
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swift
📚 Learning: 2025-11-25T13:10:38.019Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:38.019Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*View.swift : Use NavigationLink for drill-down navigation in SwiftUI
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swift
📚 Learning: 2025-11-25T13:10:38.019Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:38.019Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*.swift : Use UnifiedAppState to coordinate Core and Platform features, WalletService for SPV wallet operations, PlatformService for identity and document operations, DataManager for SwiftData persistence, and KeychainManager for secure key storage
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/FilterMatchService.swift
📚 Learning: 2025-11-25T13:10:38.019Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:38.019Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/PersistentToken.swift : In PersistentToken model, use indexed boolean properties (canManuallyMint, canManuallyBurn, canFreeze, hasDistribution, isPaused) for easy filtering
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Models/FilterMatch.swift
📚 Learning: 2025-11-25T13:10:38.019Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:38.019Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/*View.swift : Use English plural forms for token display names
Applied to files:
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swift
🧬 Code graph analysis (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swift (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/FilterMatchService.swift (6)
initialize(66-71)jumpTo(79-98)reload(134-138)isFilterMatched(59-61)checkPrefetch(101-131)hexEncodedString(302-304)
🪛 SwiftLint (0.57.0)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swift
[Warning] 186-186: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
🔇 Additional comments (17)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/FilterMatchesView.swift (4)
15-25: DualwalletServiceusage pattern is functional but could be cleaner.The view uses both
@EnvironmentObject var walletService(line 16) and aninit(walletService:)parameter (line 23). This works because@StateObjectmust be initialized ininit, but it creates a subtle coupling where the caller must ensure both sources refer to the same instance.This pattern is acceptable for now, as the preview and usage sites typically provide the same instance to both.
226-306: Well-implemented loading, error, and empty states.The view properly implements all three state views with appropriate messaging and styling. The error view includes retry functionality that respects sync status, and the empty view adapts its message based on the current display mode. As per coding guidelines, this follows the requirement to implement proper loading and error states in SwiftUI views.
311-398: Clean component extraction following coding guidelines.The
FilterRowis properly extracted as a separate component, following the guideline to break complex SwiftUI views into smaller components. The expandable detail section with hex preview and copy functionality is well-implemented.
402-410: LGTM!The preview correctly provides
WalletService.sharedto both the initializer and as an environment object, matching the view's requirements.packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Models/FilterMatch.swift (3)
43-74: LGTM! Well-structured collection wrapper.The
CompactFilterscollection safely converts FFI data with appropriate nil-checking. The helper properties (isEmpty,count) provide a clean Swift API over the FFI types.
110-141: LGTM! Consistent collection pattern.The
FilterMatchesstruct follows the same safe FFI conversion pattern asCompactFilters, maintaining consistency across the codebase.
143-169: Good error handling with clear user guidance.The localized error descriptions are user-friendly and actionable. However, the error message remapping at lines 159-161 warrants verification.
Verify the "Client not initialized" → "busy syncing" mapping:
The code interprets an FFI error message "Client not initialized" as meaning the client is busy syncing (lines 159-161). This mapping seems unusual—typically "not initialized" and "busy" are distinct states. Please confirm that:
- The FFI layer actually returns "Client not initialized" when the client is busy syncing
- This mapping aligns with the actual client lifecycle states
If the FFI error messages are misleading, consider whether the FFI layer should return more accurate error codes/messages instead of relying on string parsing in Swift.
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/FilterMatchService.swift (10)
1-13: LGTM: Clean service structure with proper concurrency annotations.The
@MainActorannotation ensures thread-safe access to published properties from the UI layer, and theObservableObjectconformance integrates well with SwiftUI.
14-49: LGTM: Well-structured properties with appropriate access control.The weak reference to
WalletServiceprevents retain cycles, and the batch size of 1,000 is safely within the documented FFI constraint of 10,000.
51-61: LGTM: Efficient computed properties leveraging Set membership.The O(1) set lookups are optimal for checking matched heights.
65-71: LGTM: Clear initialization flow.Proper sequencing of matched heights loading followed by initial batch loading.
73-76: LGTM: Simple and appropriate for sync progress updates.
78-98: LGTM: Proper underflow protection and range validation.The underflow checks when calculating
startHeight(lines 92-96) correctly handle edge cases near the beginning of the range.
100-131: LGTM: Bidirectional prefetching with proper underflow guards.The prefetch logic correctly handles scrolling in both directions and includes appropriate underflow protection (lines 114-118).
133-138: LGTM: Straightforward reload implementation.
185-196: LGTM: Proper underflow protection when calculating initial batch range.The logic correctly handles cases where the height range is smaller than the batch size.
286-305: LGTM: Appropriate helper extensions.The
Hashableconformance correctly uses height as the unique identifier, and the hex encoding helper follows standard Swift conventions.
| /// Load matched filter heights from FFI | ||
| private func loadMatchedHeights() async { | ||
| guard let walletService = walletService, | ||
| let client = walletService.spvClientHandle, | ||
| let range = heightRange else { | ||
| print("❌ FilterMatchService: Cannot load matched heights - client not available") | ||
| return | ||
| } | ||
|
|
||
| print("🔍 FilterMatchService: Loading matched filter heights...") | ||
|
|
||
| // Query FFI for filter matches | ||
| let matchesPtr = dash_spv_ffi_client_get_filter_matched_heights( | ||
| client, | ||
| range.lowerBound, | ||
| range.upperBound + 1 | ||
| ) | ||
|
|
||
| defer { | ||
| if let ptr = matchesPtr { | ||
| dash_spv_ffi_filter_matches_destroy(ptr) | ||
| } | ||
| } | ||
|
|
||
| guard let ptr = matchesPtr else { | ||
| print("❌ FilterMatchService: Failed to load matched heights") | ||
| return | ||
| } | ||
|
|
||
| // Convert to Swift models | ||
| let ffiMatches = ptr.pointee | ||
| let filterMatches = FilterMatches(from: ffiMatches) | ||
|
|
||
| // Extract heights | ||
| var heights = Set<UInt32>() | ||
| for entry in filterMatches.entries { | ||
| heights.insert(entry.height) | ||
| } | ||
|
|
||
| matchedHeights = heights | ||
| print("🔍 FilterMatchService: Found \(matchedHeights.count) matched filter heights") | ||
| } |
There was a problem hiding this comment.
Potential overflow when calculating exclusive upper bound.
Line 157 computes range.upperBound + 1, which will overflow if upperBound is UInt32.max. While blockchain heights are unlikely to reach 4 billion in the near term, consider using overflow-safe arithmetic or explicit overflow handling to prevent runtime errors.
Apply this diff to add overflow protection:
- let matchesPtr = dash_spv_ffi_client_get_filter_matched_heights(
- client,
- range.lowerBound,
- range.upperBound + 1
- )
+ // Use explicit overflow handling to prevent crash at UInt32.max
+ let exclusiveEnd = range.upperBound == UInt32.max
+ ? UInt32.max
+ : range.upperBound + 1
+
+ let matchesPtr = dash_spv_ffi_client_get_filter_matched_heights(
+ client,
+ range.lowerBound,
+ exclusiveEnd
+ )🤖 Prompt for AI Agents
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/FilterMatchService.swift
lines 142-183: the call uses range.upperBound + 1 which will overflow when
upperBound == UInt32.max; compute an overflow-safe exclusive upper bound first
(e.g. if range.upperBound == UInt32.max then use UInt32.max as the exclusive
bound, otherwise use range.upperBound + 1) and pass that value to
dash_spv_ffi_client_get_filter_matched_heights; ensure the computed value is a
UInt32 and keep the existing defer/cleanup and conversion logic unchanged.
| let endHeight = min(range.upperBound + 1, startHeight + batchSize) | ||
|
|
||
| print("🔍 FilterMatchService: Loading filters from \(startHeight) to \(endHeight) (client available)") | ||
|
|
||
| // Check if this range is already loaded | ||
| let requestedRange = startHeight...endHeight | ||
| if loadedRanges.contains(where: { $0.overlaps(requestedRange) }) { | ||
| return | ||
| } |
There was a problem hiding this comment.
Fix range handling inconsistency and overflow risk.
Two issues with range calculations:
-
Overflow risk (line 218):
range.upperBound + 1will overflow ifupperBoundisUInt32.max. -
Range inconsistency (lines 223 vs 278):
- Line 223:
requestedRange = startHeight...endHeighttreatsendHeightas inclusive - Line 278: Stores
startHeight...(endHeight - 1), treatingendHeightas exclusive - This mismatch breaks the overlap detection on line 224, allowing duplicate range loads.
- Line 223:
Apply this diff to fix both issues:
- // Calculate end height (exclusive, max batchSize)
- let endHeight = min(range.upperBound + 1, startHeight + batchSize)
+ // Calculate end height (exclusive, max batchSize) with overflow protection
+ let maxEndHeight = range.upperBound == UInt32.max
+ ? UInt32.max
+ : range.upperBound + 1
+ let endHeight = min(maxEndHeight, startHeight + batchSize)
print("🔍 FilterMatchService: Loading filters from \(startHeight) to \(endHeight) (client available)")
// Check if this range is already loaded
- let requestedRange = startHeight...endHeight
+ let requestedRange = startHeight...(endHeight - 1) // Convert exclusive to inclusive for consistency
if loadedRanges.contains(where: { $0.overlaps(requestedRange) }) {
return
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 10
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/Helpers/WIFParser.swift (1)
9-46: <function_calls>
"Dash" cryptocurrency "version byte" WIF import format specification
Looking at the verification results, I can now provide the final rewritten review comment:
Fix critical WIF documentation errors that contradict the implementation
The
parseWIFcomments contain multiple errors that conflict with the actual Bitcoin/Dash WIF specification and theencodeToWIFimplementation:
Version bytes are wrong: Line 20 claims
0xCC(testnet) and0xD2(mainnet), but the correct Bitcoin/Dash WIF standard uses0xef(testnet) and0x80(mainnet)—whichencodeToWIFcorrectly implements. TheparseWIFfunction doesn't validate version bytes, so it would silently accept keys with wrong versions.Base58 prefixes are wrong: Lines 11–12 claim mainnet starts with
'7'(uncompressed) or'X'(compressed), and testnet with'c'or'c'. The correct prefixes are:
- Mainnet:
'5'(uncompressed) or'K'/'L'(compressed)- Testnet:
'9'(uncompressed) or'c'(compressed)Update the comments to match the actual Bitcoin/Dash WIF specification and the correct implementation in
encodeToWIF.packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift (2)
608-635: Bug: Staleaddressesreference after generation.After calling
generateAddresses, the code searches the originaladdressesarray captured at the start of the method. Newly generated addresses won't be found becauseaddressesis a snapshot, not a reference to the live collection.Apply this diff to re-fetch addresses after generation:
// Generate new addresses if all are used try await generateAddresses(for: account, count: 10, type: type) - // Return the first newly generated address - guard let newAddress = addresses.first(where: { !$0.isUsed }) else { + // Re-fetch addresses after generation and return the first unused + let updatedAddresses: [HDAddress] + switch type { + case .external: + updatedAddresses = account.externalAddresses + case .internal: + updatedAddresses = account.internalAddresses + case .coinJoin: + updatedAddresses = account.coinJoinAddresses + case .identity: + updatedAddresses = account.identityFundingAddresses + } + + guard let newAddress = updatedAddresses.first(where: { !$0.isUsed }) else { throw WalletError.addressGenerationFailed }
787-797: Silent failure on Keychain write.
SecItemAddresult is discarded. If the Keychain write fails (e.g., disk full, access denied), the caller won't know the seed wasn't stored. For seed data, this could cause data loss.Apply this diff to propagate Keychain errors:
private class KeychainWrapper { - func set(_ data: Data, forKey key: String) { + func set(_ data: Data, forKey key: String) throws { let query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrAccount as String: key, kSecValueData as String: data ] SecItemDelete(query as CFDictionary) - SecItemAdd(query as CFDictionary, nil) + let status = SecItemAdd(query as CFDictionary, nil) + guard status == errSecSuccess else { + throw WalletError.databaseError("Keychain write failed: \(status)") + } }Note: This requires updating callers to handle the thrown error.
packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/WalletService.swift (1)
334-407: Do not log plaintext PIN increateWallet
createWalletlogs the actual PIN:print("PIN: \(pin)")Even though your custom
Drop this line or, if you really need it for debugging, log only the length (e.g.
PIN length) — the UICreateWalletViewalready logs the length, so you can likely remove this entirely.
♻️ Duplicate comments (3)
packages/swift-sdk/Sources/SwiftDashSDK/FFI/Signer.swift (1)
58-63: Duplicate concern: canSign also ignores privateKeys.This method always returns
true, ignoring theprivateKeysdictionary. This is inconsistent with the key management infrastructure and compounds the confusion raised in the previous comment about unused keys.Addressed by the previous review comment suggesting either using
privateKeysfor realistic behavior or removing the unused infrastructure.packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/FilterMatchService.swift (1)
154-158: Overflow risk onrange.upperBound + 1and range semantics mismatch.These issues were previously flagged:
- Line 157 and 218:
range.upperBound + 1will overflow ifupperBoundisUInt32.max- Line 223 uses
startHeight...endHeight(treatingendHeightas inclusive) but line 278 storesstartHeight...(endHeight - 1), causing overlap detection to failPlease address the fixes suggested in the previous review.
Also applies to: 217-226
packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/WalletService.swift (1)
171-174: Consider narrowing public exposure of rawFFIDashSpvClientpointer
public var spvClientHandle: UnsafeMutablePointer<FFIDashSpvClient>?exposes a raw FFI handle to all SDK consumers. That’s useful for advanced integrations (like platform‑wallet FFI), but also easy to misuse and can bypass the safety invariants enforced bySPVClient.If only internal PlatformWallet wrappers need this, consider making it
internaland routing external use through typed methods (e.g. explicit “filter match”/identity operations). If it must stay public, add clear API docs about lifetime and threading expectations so consumers don’t dereference it afterWalletService/SPVClientteardown.
🧹 Nitpick comments (45)
packages/swift-sdk/Package.swift (1)
8-9: Raising minimum iOS/macOS versions is a breaking change—confirm and document support policyChanging the package platforms to
.iOS(.v17)and.macOS(.v14)means apps targeting earlier OS versions can no longer consumeSwiftDashSDK. Please confirm this matches the intended support matrix (including theDashSDKFFI.xcframeworkbuild settings and CI/Xcode toolchain), and ensure release notes / README / migration docs clearly call out this minimum OS bump for SDK consumers.packages/swift-sdk/Sources/SwiftDashSDK/Models/TokenModel.swift (1)
28-54: Consider DRY’ing formatting and constrainingdecimalsFunctionally this block is fine, but a couple of small improvements are possible:
- The formatting logic (divisor,
tokenAmount,String(format:)) is duplicated across all four formatted properties; extracting a small private helper (e.g.formatAmount(_:)) would reduce repetition.- Since
decimalsflows into bothpow(10, Double(decimals))and the dynamic"%.\(decimals)f"format, you may want to ensure it’s clamped to a sensible non‑negative range at construction time (or via a helper) to avoid surprising behavior if upstream data is ever malformed.Both are non-blocking, but would slightly harden and simplify this model.
packages/swift-sdk/Sources/SwiftDashSDK/Helpers/TestKeyGenerator.swift (2)
8-19: Consider fixing UInt32 endianness in the seed for cross‑platform determinism
withUnsafeBytes(of: keyIndex) { Data($0) }relies on the platform’s native endianness. That’s fine for iOS‑only tests, but if these test keys ever need to match values generated on other platforms (Rust, JS, etc.), you probably want to make the encoding explicit (e.g.keyIndex.bigEndian) so the SHA256 input is unambiguous and stable across architectures.Example:
var seedData = Data() seedData.append(identityId) var beIndex = keyIndex.bigEndian withUnsafeBytes(of: &beIndex) { seedData.append(contentsOf: $0) } seedData.append(purpose)
22-45: Use numeric key IDs (e.g.UInt32) instead ofStringfor the keys mapThe API currently builds a
[String: Data]and then convertskeyIdtoStringto look it up. That works, but it’s a bit awkward and less type‑safe than using a numeric key type directly (e.g.[UInt32: Data]).You could simplify and avoid string conversions:
static func generateTestPrivateKeys(identityId: Data) -> [UInt32: Data] { var keys: [UInt32: Data] = [:] keys[0] = generateTestPrivateKey(identityId: identityId, keyIndex: 0, purpose: 0) keys[1] = generateTestPrivateKey(identityId: identityId, keyIndex: 1, purpose: 0) keys[2] = generateTestPrivateKey(identityId: identityId, keyIndex: 2, purpose: 3) keys[3] = generateTestPrivateKey(identityId: identityId, keyIndex: 3, purpose: 3) return keys } static func getPrivateKey(identityId: Data, keyId: UInt32) -> Data? { let keys = generateTestPrivateKeys(identityId: identityId) return keys[keyId] }Given this is test/demo‑only, this is purely a cleanliness/readability improvement.
packages/swift-sdk/Sources/SwiftDashSDK/Models/TransitionTypes.swift (1)
5-67: Models look solid; consider addingCodablefor JSON-based configsThe three structs are clean, immutable, and
Sendable-safe; there are no correctness or safety issues here.Given that transition definitions are loaded from JSON in the wasm SDK (
api-definitions.json), it’s likely these models will eventually be serialized/deserialized from a similar config on iOS. In that case, conforming them toCodablenow will simplify mapping, reduce boilerplate, and keep the Swift SDK aligned with the existing JSON-driven flow. You can still introduceCodingKeyslater if any field names differ between Swift and JSON (e.g.,defaultValuevs. adefaultkey). Based on learnings, this keeps the Swift models in sync with how transitions are defined elsewhere.A minimal change would look like:
-public struct TransitionDefinition: Sendable { +public struct TransitionDefinition: Sendable, Codable { @@ -public struct TransitionInput: Sendable { +public struct TransitionInput: Sendable, Codable { @@ -public struct SelectOption: Sendable { +public struct SelectOption: Sendable, Codable {packages/swift-sdk/Sources/SwiftDashSDK/DPP/DPPIdentity.swift (2)
6-34: DPPIdentity model looks solid; consider centralizing credits-per-DASH constantStruct shape and visibility look good and match DPP identity semantics; making this type
publicandSendableis aligned with the Swift DPP visibility guidelines. The only thing I’d tweak isformattedBalance: the100_000_000_000credits-per-DASH ratio is hard‑coded here. If this ratio is defined anywhere else (in Rust or Swift), it’d be safer to expose it as a shared constant (e.g.,CreditsPerDash) and reuse it here to avoid divergence if economics change in one place but not the other. As per coding guidelines and learnings, keeping these DPP-facing types public is correct.
62-91: Clarify duplicate-key expectations in factory helpers (Dictionary(uniqueKeysWithValues:))Both
createhelpers buildkeysDictviaDictionary(uniqueKeysWithValues:). That will trap at runtime ifpublicKeysever contains duplicateidvalues. If all callsites are guaranteed to pass uniqueIdentityPublicKey.ids this is fine, but it’s worth making that precondition explicit in docs, or adding a small defensive step (e.g., deduplicating by last/first occurrence or asserting in debug only). Also, thekeysDictconversion is duplicated between the two helpers; you could share a tiny private helper or have theidData:version call through to theid:version to keep behavior in one place.packages/swift-sdk/Sources/SwiftDashSDK/Models/TokenAction.swift (1)
27-30: Consider removing or documenting the intent ofisEnabled.Currently,
isEnabledalways returnstruefor all cases. If action-specific enablement logic is planned for the future, consider adding a// TODO:comment. Otherwise, callers could simply assume all actions are enabled without needing this property.packages/swift-sdk/Sources/SwiftDashSDK/Config/TestnetNodes.swift (3)
41-60: Align HPMasternode hash naming with MasternodeInfo (or document why it differs)
HPMasternodeInfousesprotxTxHashwith CodingKey"protx-tx-hash", whileMasternodeInfousesproTxHash→"pro-tx-hash". This looks like an unintentional naming divergence and may be confusing or cause decoding mismatches if the YAML/schema reuses the same field name for both node types.If the underlying config uses the same field name as regular masternodes, consider aligning like this:
-public struct HPMasternodeInfo: Codable, Sendable { - public let protxTxHash: String +public struct HPMasternodeInfo: Codable, Sendable { + public let proTxHash: String @@ - public init(protxTxHash: String, owner: KeyInfo, voter: KeyInfo, payout: KeyInfo) { - self.protxTxHash = protxTxHash + public init(proTxHash: String, owner: KeyInfo, voter: KeyInfo, payout: KeyInfo) { + self.proTxHash = proTxHash @@ - enum CodingKeys: String, CodingKey { - case protxTxHash = "protx-tx-hash" + enum CodingKeys: String, CodingKey { + case proTxHash = "pro-tx-hash"If the different key is intentional (e.g. matches an existing
.testnet_nodes.ymlschema), a short comment explaining the discrepancy would help future readers.
80-87: Clarify thatloadFromYAMLis currently a stub returning sample dataThe docstring and signature suggest real YAML loading, but the implementation always returns
createSampleTestnetNodes()and ignores the file system. That’s fine for an initial scaffold, but the current contract is a bit misleading.Consider either:
- Marking this clearly as a TODO stub, or
- Renaming the API to reflect fallback behavior (e.g.
loadFromYAMLOrSample), then tightening the contract once real loading is implemented.For example:
- /// Load testnet nodes from a YAML file + /// Load testnet nodes from a YAML file. + /// TODO: Implement actual YAML loading from bundle/documents; currently returns sample data.
91-103: Explicitly mark embedded private keys as test‑only dataThe sample masternode / HPMN entries embed WIF‑looking private keys directly in source. That’s often acceptable for deterministic test fixtures, but it’s worth making the intent explicit to avoid accidental reuse in any non‑test context.
Suggestion:
- Add a comment making clear these are testnet/dev‑only keys and must never be used on mainnet or for real funds.
- Optionally, move them into a dedicated test/fixture file if they’re not needed in the production SDK target.
For example:
- let sampleMasternode = MasternodeInfo( + // NOTE: Testnet/dev-only keys; DO NOT USE on mainnet or with real funds. + let sampleMasternode = MasternodeInfo( @@ - let sampleHPMasternode = HPMasternodeInfo( + // NOTE: Testnet/dev-only keys; DO NOT USE on mainnet or with real funds. + let sampleHPMasternode = HPMasternodeInfo(packages/swift-sdk/Sources/SwiftDashSDK/Core/Utils/DataContractParser.swift (1)
4-7: LGTM! Public visibility aligns with DPP type exposure guidelines.The visibility changes enable external modules to parse data contracts, which is consistent with the PR's goal of expanding DPP data-contract support across the codebase.
Optional: Consider API design improvements for future iterations.
While the current implementation is functional, consider these refinements for a more robust public API:
- Type safety: The
[String: Any]parameter is error-prone. Consider a typedContractDatastruct or at least document expected keys.- Dependency coupling: Requiring
ModelContextties callers to SwiftData. A protocol-based approach could offer more flexibility.- Side effects: The method mutates
modelContextextensively but this isn't obvious from the signature. Consider documenting or returning inserted entities.As per coding guidelines, making DPP types public ensures proper visibility across module boundaries.
packages/swift-sdk/Sources/SwiftDashSDK/Core/Models/HDWalletModels.swift (2)
12-24: AccountCategory enum looks consistent with CoreWalletManager; consider raw values for stabilityThe set of cases here matches what
CoreWalletManager.derivationPath(for:category:index:network:)switches over, which is good for keeping derivation logic and modeling in sync. You might consider givingAccountCategoryaStringraw value and usingcategory.rawValuefor IDs/serialization instead of relying on"\(category)", to make the textual representation explicitly stable across refactors.
86-108: FFIAccountType alongside AccountInfo.category – consider clarifying or centralizing the mapping
AccountDetailInfoexposes bothaccount.category(Swift-sideAccountCategory) and anFFIAccountTypefrom the Rust side. If those two concepts are meant to be equivalent or tightly coupled, it might be worth documenting that invariant here or centralizing the mapping in one place to avoid accidental mismatches later. If they intentionally differ (e.g., FFI has a coarser or richer taxonomy), a brief doc comment onaccountTypeexplaining how it relates tocategorywould help future readers.packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/HDWallet.swift (2)
45-70: Consider making the initializer public for cross-module usage.The
initlacks an explicit access modifier and defaults tointernal. If external modules need to createHDWalletinstances, consider addingpublic:- init(label: String, network: AppNetwork, isWatchOnly: Bool = false, isImported: Bool = false) { + public init(label: String, network: AppNetwork, isWatchOnly: Bool = false, isImported: Bool = false) {The networks bitfield values (1, 2, 8) correctly use powers of 2 for bit operations, distinct from
DashSDKNetworkraw values—this is the expected pattern for FFINetworks.
72-74: Silent fallback to.testnetcould mask data issues.The fallback is safe, but if
networkcontains an unexpected value (e.g., from a migration or data corruption), this silently recovers without any indication. Consider logging when the fallback is triggered to aid debugging:public var dashNetwork: AppNetwork { - return AppNetwork(rawValue: network) ?? .testnet + if let appNetwork = AppNetwork(rawValue: network) { + return appNetwork + } + // TODO: Consider logging this fallback for debugging + return .testnet }packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyWalletTypes.swift (2)
301-367: Consider addingCodableconformance for persistence consistency.The
Balancestruct hasCodableconformance (line 191), butUTXOdoes not. IfUTXOinstances need to be persisted (e.g., for SwiftData or serialization), addingCodablewould ensure consistency with other value types in this module. Based on learnings, SwiftData models requireCodableconformance.Apply this diff:
-public struct UTXO: Identifiable, Equatable, Sendable { +public struct UTXO: Identifiable, Equatable, Codable, Sendable {Note: Verify that all properties are
Codable-compatible.Data,String,UInt32, andUInt64are allCodable, so this should work without additional implementation.
237-240: Optional: Consider making the formatting helper reusable.The
formatDash(_:)helper is currently private toBalance, but formatting DASH amounts from duffs is likely needed elsewhere in the codebase. Consider making it a public static method or aUInt64extension for broader reusability.Example refactor:
extension UInt64 { /// Format an amount in duffs as DASH string /// - Returns: Formatted string like "1.23456789 DASH" public func formatAsDash() -> String { let dash = Double(self) / 100_000_000.0 return String(format: "%.8f DASH", dash) } }Then update the Balance properties:
public var formattedConfirmed: String { confirmed.formatAsDash() }packages/swift-sdk/Sources/SwiftDashSDK/Helpers/WIFParser.swift (1)
53-73: Consider whetherisTestnetdefault should remaintruefor a public APINow that
encodeToWIFis public, defaultingisTestnettotruemay be surprising, as most callers would typically expect mainnet behavior by default.If most production usage is mainnet, consider flipping the default to
false(or requiring the argument explicitly) before this API is widely consumed, to avoid subtle mis-encodings. If testnet-as-default is intentional, a brief doc comment noting this rationale would help.packages/swift-sdk/Sources/SwiftDashSDK/DPP/DPPStateTransition.swift (3)
14-53: StateTransitionType coverage and naming look good; consider clarifying raw vs display namesThe enum cleanly covers identity, data contract, document batch, and token transitions.
rawValue(case name) vsname(user‑facing label) is a good split; if this is intended to map to a canonical DPP/FFI representation, a brief doc comment noting that distinction would reduce ambiguity for future contributors.
180-204: DocumentsBatchTransition and DocumentTransition enum structure make sense; watch Codable shape vs wire formatBatching per‑document transitions under
DocumentsBatchTransitionand modeling individual actions via theDocumentTransitionenum is a clear design. SinceDocumentTransitionrelies on synthesizedCodablefor an enum with associated values, the encoded JSON shape will follow Swift’s default tagging scheme; if you later need this to match the exact DPP wire format, you may want a customencode/decodeto avoid surprises.
443-467: Broadcast/wait request types look appropriate; consider documenting timeout units
BroadcastStateTransitionRequestusingany StateTransitionplusskipValidation/dryRunflags is a good abstraction, andWaitForStateTransitionResultRequestmirrors the DAPI semantics. Sincetimeoutis aTimeIntervalwith a default30, adding a short doc comment specifying the unit (seconds vs milliseconds) will help avoid confusion when wiring this up to the underlying DAPI/FFI calls.packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/CoreWalletManager.swift (1)
303-307: HardcodedcurrentHeight: 0may affect confirmation counts.The TODO notes the intent to get height from SPV client. With
currentHeight = 0, transaction confirmation counts may be inaccurate until this is wired up. Consider adding a follow-up task to address this.Would you like me to open an issue to track integrating the SPV client's current height here?
packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/TransactionService.swift (1)
42-42: Redundant module prefix.Since this file is within the
SwiftDashSDKmodule, theSwiftDashSDK.prefix onSDKTransactionBuilderis unnecessary. Consider removing for consistency with the updatedSPVClienttype on line 16.- let builder = SwiftDashSDK.SDKTransactionBuilder(network: wallet.dashNetwork.sdkNetwork, feePerKB: feePerKB) + let builder = SDKTransactionBuilder(network: wallet.dashNetwork.sdkNetwork, feePerKB: feePerKB)packages/swift-sdk/Sources/SwiftDashSDK/Models/IdentityModel.swift (1)
26-26:[String: Any]prevents futureCodable/Sendableconformance.The
contestedDpnsInfoproperty uses[String: Any], which is notCodableorSendable. WhileIdentityModelcurrently doesn't require these conformances, this will block adding them in the future. Per the learnings, allCodabletypes must beEquatablefor SwiftData predicates to function correctly.Consider using
[String: PlatformValue]instead, which is already defined inDPPTypes.swiftand conforms to bothCodableandSendable.- public var contestedDpnsInfo: [String: Any] = [:] + public var contestedDpnsInfo: [String: PlatformValue] = [:]packages/swift-sdk/Sources/SwiftDashSDK/DPP/DPPTypes.swift (1)
141-155: Silently dropping unconvertible values may hide data loss.The
compactMapfor arrays and the conditional insertion for dictionaries silently discard values that cannot be converted toPlatformValue. This could mask data integrity issues.Consider logging dropped values or returning
nilfor the entire container if any element fails conversion, depending on the desired semantics.packages/swift-sdk/Sources/SwiftDashSDK/DPP/DPPDataContract.swift (1)
470-472:JsonSchemaPropertyValueindirect enum appears incomplete.This enum only has a single
.propertycase. If it's intended as a placeholder for future expansion (e.g., to support$ref, array items,oneOf/anyOf), consider adding a TODO comment. Otherwise, the wrapper may be unnecessary and theitemsproperty could directly useJsonSchemaProperty?.-public indirect enum JsonSchemaPropertyValue: Codable, Equatable, Sendable { - case property(JsonSchemaProperty) -} +// TODO: Expand to support $ref, oneOf, anyOf if needed +public indirect enum JsonSchemaPropertyValue: Codable, Equatable, Sendable { + case property(JsonSchemaProperty) +}packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift (4)
21-55: String-encoded authorization fields: consider a lightweight typed wrapper
ChangeControlRules.authorizedToMakeChange/adminActionTakersand theAuthorizedActionTakers.identity(_:)/group(_:)helpers rely on string conventions ("NoOne","ContractOwner","Identity:<base58>","Group:<n>"). That’s flexible but a bit “stringly typed” and easy to mistype.You might consider a small value type (e.g.
struct ActionTakerRef { let raw: String }or an enum-with-associated-values) or at least centralizing the string prefixes ("Identity:","Group:") asstatic letconstants to avoid duplication and typos. Existing API surface can remain string-based so this would be an internal refactor only.Also applies to: 144-156
59-72: Avoid magic default fordistributionType
TokenPerpetualDistribution.distributionTypeis initialized to"{}"and cannot be customized via the initializer. If"{}"has a specific semantic (e.g. “no config” or default JSON), it would be clearer to either:
- Expose it as an initializer parameter with a default, or
- At least encapsulate it in a named constant.
For example:
-public struct TokenPerpetualDistribution: Codable, Equatable, Sendable { - public var distributionType: String +public struct TokenPerpetualDistribution: Codable, Equatable, Sendable { + public static let defaultDistributionType = "{}" + public var distributionType: String @@ - public init(distributionRecipient: String = "AllEqualShare", enabled: Bool = true) { - self.distributionType = "{}" + public init( + distributionType: String = TokenPerpetualDistribution.defaultDistributionType, + distributionRecipient: String = "AllEqualShare", + enabled: Bool = true + ) { + self.distributionType = distributionType self.distributionRecipient = distributionRecipient self.enabled = enabled }This keeps behavior the same while making the default explicit and overridable.
99-118: DistributionEvent: consider future-proofing trigger/recipient encodingsRight now
triggerTypeandrecipientare free-formStrings, but the initializer always sets"Time"and"AllHolders". To reduce the chance of typos as more trigger/recipient variants are added, consider:
- Introducing small enums or namespaced string constants for known
triggerType/recipientvalues, and- Using those in the initializer and any parsing/serialization paths.
This keeps the external representation as strings while giving call sites something more self-documenting to work with.
175-255: Align enum conformances and keep control-rule wiring in sync with app modelsThese enums form the core “vocabulary” for token configuration and history:
ControlRuleTypeChangeControlRuleTypeTokenEventTypeIdentityTypeA couple of suggestions:
- Consider Codable/Equatable/CaseIterable where these are persisted or serialized
If these types are ever stored via SwiftData, sent over FFI, or encoded/decoded, adding consistent conformances will reduce friction:
-public enum ControlRuleType: Sendable { +public enum ControlRuleType: Sendable, CaseIterable, Codable, Equatable { -public enum ChangeControlRuleType: Sendable { +public enum ChangeControlRuleType: Sendable, CaseIterable, Codable, Equatable { -public enum TokenEventType: String, CaseIterable, Sendable { +public enum TokenEventType: String, CaseIterable, Codable, Equatable, Sendable { -public enum IdentityType: String, CaseIterable, Sendable { +public enum IdentityType: String, CaseIterable, Codable, Equatable, Sendable {This also aligns with the project’s pattern that persistence-oriented types are Codable+Equatable. Based on learnings, this helps SwiftData predicates and cross-module visibility.
- Avoid divergence between
ControlRuleTypeandChangeControlRuleTypeThe two enums currently share most cases, with
ChangeControlRuleTypeaddingtradeMode. If they are meant to stay in lockstep conceptually, consider documenting that or deriving one from the other (or using a single enum plus a marker) so new rule kinds don’t get added to one and forgotten in the other.
- Double-check
TokenEventType.requiresHistorysemantics
requiresHistoryreturnstrueonly for config-like / admin events. If that’s intentional (vs. also requiring history entries for “normal” user-facing events likemint/transfer), this is fine; otherwise, it’s worth re-confirming the desired behavior before wiring into any history filters.
- Ensure app-side wiring for new control rule types
Per the SwiftExampleApp guidelines, when new control rules (e.g.,
tradeMode) are introduced, the correspondingPersistentTokenmodel, computed properties, predicate helpers, andDataContractParsershould be updated to understand them. Please verify those pieces are in sync with these enums.packages/swift-sdk/Sources/SwiftDashSDK/Core/Models/FilterMatch.swift (2)
52-63: Consider pre-reserving array capacity for performance.When iterating over FFI collections, pre-allocating array capacity avoids repeated reallocations.
/// Initialize from FFI struct public init(from ffiFilters: FFICompactFilters) { var filters: [CompactFilter] = [] + filters.reserveCapacity(Int(ffiFilters.count)) if let filtersPtr = ffiFilters.filters { for i in 0..<ffiFilters.count { let ffiFilter = filtersPtr.advanced(by: Int(i)).pointee filters.append(CompactFilter(from: ffiFilter)) } } self.filters = filters }
119-130: Consider pre-reserving array capacity here as well.Same optimization opportunity as
CompactFilters./// Initialize from FFI struct public init(from ffiMatches: FFIFilterMatches) { var entries: [FilterMatchEntry] = [] + entries.reserveCapacity(Int(ffiMatches.count)) if let entriesPtr = ffiMatches.entries { for i in 0..<ffiMatches.count { let ffiEntry = entriesPtr.advanced(by: Int(i)).pointee entries.append(FilterMatchEntry(from: ffiEntry)) } } self.entries = entries }packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/FilterMatchService.swift (1)
286-296: Hashable conformance ignoresdatafield — verify this is intentional.The
Hashableimplementation only considersheight, meaning two filters with the same height but different data would be considered equal. This appears intentional for deduplication by height (line 267-275), but document this behavior to prevent future confusion.// MARK: - CompactFilter Hashable Conformance +/// Note: Equality and hashing are based solely on `height` to support +/// deduplication by block height. Filters at the same height are considered +/// equivalent regardless of their data content. extension CompactFilter: Hashable { public static func == (lhs: CompactFilter, rhs: CompactFilter) -> Bool { lhs.height == rhs.height } public func hash(into hasher: inout Hasher) { hasher.combine(height) } }packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentProperty.swift (1)
36-51: Consider adding a separator to the composite ID to prevent potential collisions.The current ID construction by directly appending data could theoretically produce collisions for different input combinations. For example,
contractId + "ab".data + "c".dataproduces the same bytes ascontractId + "a".data + "bc".data.Using a delimiter (like a null byte or length prefix) would guarantee uniqueness:
public init(contractId: Data, documentTypeName: String, name: String, type: String) { // Create unique ID by combining contract ID, document type name, and property name var idData = contractId + idData.append(0) // Separator idData.append(documentTypeName.data(using: .utf8) ?? Data()) + idData.append(0) // Separator idData.append(name.data(using: .utf8) ?? Data()) self.id = idDatapackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentIndex.swift (1)
29-51: Same composite ID concern asPersistentProperty; also note silent JSON encoding failure.The ID construction has the same potential collision issue mentioned for
PersistentProperty. Additionally, if JSON encoding fails (lines 44-48), the fallback to emptyData()silently discards the properties array without logging or error handling.Consider logging the failure or using a consistent separator pattern:
// Store properties as JSON array if let jsonData = try? JSONSerialization.data(withJSONObject: properties, options: []) { self.propertiesJSON = jsonData } else { + // Log warning for debugging + print("Warning: Failed to encode index properties for \(name)") self.propertiesJSON = Data() }packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDocumentType.swift (1)
54-58: Consider handling name encoding failure in ID generation.If
name.data(using: .utf8)returnsnil(edge case with invalid strings), the ID will only containcontractId, potentially causing collisions between document types with the same contract but different names that fail to encode.public init(contractId: Data, name: String, schemaJSON: Data, propertiesJSON: Data) { // Create unique ID by combining contract ID and name var idData = contractId - idData.append(name.data(using: .utf8) ?? Data()) + guard let nameData = name.data(using: .utf8) else { + fatalError("Document type name must be valid UTF-8: \(name)") + } + idData.append(nameData) self.id = idDatapackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTokenBalance.swift (1)
55-63: Potential precision loss with Double arithmetic for large balances.Using
Doublefor division can cause precision loss whenbalanceis large (tokens with many decimals or high supply). For financial calculations, consider usingDecimalinstead.public var formattedBalance: String { guard let decimals = tokenDecimals else { return "\(balance)" } - let divisor = pow(10.0, Double(decimals)) - let amount = Double(balance) / divisor - return String(format: "%.\(decimals)f", amount) + let divisor = Decimal(sign: .plus, exponent: Int(decimals), significand: 1) + let amount = Decimal(balance) / divisor + return (amount as NSDecimalNumber).stringValue }packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDocument.swift (1)
176-178: Consider using structured logging instead ofprint().For an SDK library,
print()statements pollute client app console output. Consider usingos_logor a logging framework that can be configured or disabled.packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentDataContract.swift (4)
85-97: Consider consistent empty-data handling on serialization failure.The setters silently fall back to
Data()if JSON serialization fails (lines 94, 108). While this prevents crashes, it could mask serialization issues with complex objects. For debugging purposes, consider logging serialization failures.Also applies to: 99-111
224-229:lastUpdatedmodified even when no document is removed.
removeDocumentunconditionally updateslastUpdatedat line 228 regardless of whether a document was actually removed. This could cause unnecessary sync cycles.public func removeDocument(withId documentId: String) { + let countBefore = documents.count if let docIdData = Data.identifier(fromBase58: documentId) { documents.removeAll { $0.id == docIdData } } - lastUpdated = Date() + if documents.count != countBefore { + lastUpdated = Date() + } }
267-271: Force unwrap in predicate may be fragile.Line 269 uses
contract.lastSyncedAt!after a nil check. While Swift's short-circuit evaluation makes this safe at runtime, the pattern is error-prone for future modifications. Consider using optional chaining or a different approach.public static func needsSyncPredicate(olderThan date: Date) -> Predicate<PersistentDataContract> { #Predicate<PersistentDataContract> { contract in - contract.lastSyncedAt == nil || contract.lastSyncedAt! < date + contract.lastSyncedAt.map { $0 < date } ?? true } }
310-324: Incomplete token conversion intoContractModel().The method returns an empty
tokensarray with a TODO-style comment. This causes data loss when round-tripping: tokens present in the originalContractModelwill not be preserved after persistence and retrieval.Would you like me to help implement the token decoding from
tokensData? Alternatively, consider opening an issue to track this.packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/WalletService.swift (2)
226-324: Checkconfigure’s async init ordering vs wallet loading
configure(modelContainer:network:)now:
- Starts SPV initialization and
CoreWalletManagerconstruction in a detached task.- Sets
walletManagerinside aMainActor.runblock within that task.- Calls
loadCurrentWallet()immediately at the end ofconfigure, before the async init completes.Because
loadCurrentWallet()isprivateand only called here, on first launch with existing wallets it will likely run whilewalletManageris stillnil, socurrentWalletnever gets set unless something else triggers loading later. You may want to:
- Invoke
loadCurrentWallet()from the block that assignswalletManager(once it’s non‑nil), or- Add a
didSetonwalletManagerthat callsloadCurrentWallet()when initialized.The new
isInitializingflag and initial sync snapshot application otherwise look consistent with the async init flow.Also applies to: 282-287, 295-301, 307-311, 315-317
637-663:switchNetworkis async but currently synchronous
public func switchNetwork(to network: AppNetwork) asyncdoesn’tawaitanything — it performs only synchronous work and then callsconfigure, which itself schedules async work internally. That’s fine functionally, but:
- Call sites that
awaitit don’t actually gain a sequencing guarantee with respect to SPV/wallet initialization.- If you don’t plan to add
awaited work here soon, you could dropasyncto make the contract clearer; otherwise it’s reasonable to keep for future‑proofing but be aware it is effectively sync today.
| // Check if we're near the beginning and need newer data | ||
| if displayedIndex < prefetchThreshold { | ||
| if let newestLoaded = filters.first?.height, | ||
| newestLoaded < range.upperBound { | ||
| let startHeight = min(range.upperBound - batchSize + 1, newestLoaded + 1) | ||
| await loadBatch(startHeight: startHeight) | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential underflow when range.upperBound < batchSize - 1.
Line 127 computes range.upperBound - batchSize + 1, which will underflow if upperBound < batchSize - 1. This could happen when the blockchain height is less than the batch size.
// Check if we're near the beginning and need newer data
if displayedIndex < prefetchThreshold {
if let newestLoaded = filters.first?.height,
newestLoaded < range.upperBound {
- let startHeight = min(range.upperBound - batchSize + 1, newestLoaded + 1)
+ // Avoid underflow when upperBound < batchSize
+ let startHeight: UInt32
+ if range.upperBound >= batchSize - 1 {
+ startHeight = min(range.upperBound - batchSize + 1, newestLoaded + 1)
+ } else {
+ startHeight = newestLoaded + 1
+ }
await loadBatch(startHeight: startHeight)
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/FilterMatchService.swift
around lines 123 to 130, the computation `range.upperBound - batchSize + 1` can
underflow when `range.upperBound < batchSize - 1`; change the startHeight
calculation to perform safe/clamped subtraction (compute the candidate as the
clamped lower bound: e.g. if range.upperBound < batchSize - 1 use 0 or 1 as the
candidate) and then take the max between that candidate and `newestLoaded + 1`;
ensure you use integer-safe operations (or use Swift’s max/guard checks) so no
unsigned underflow can occur before calling `await loadBatch(startHeight:)`.
| public final class TestSigner: Signer, @unchecked Sendable { | ||
| private var privateKeys: [String: Data] = [:] | ||
| init() { | ||
|
|
||
| public init() { | ||
| // Initialize with some test private keys for demo purposes | ||
| // In a real app, these would be securely stored and retrieved | ||
| privateKeys["11111111111111111111111111111111"] = Data(repeating: 0x01, count: 32) | ||
| privateKeys["22222222222222222222222222222222"] = Data(repeating: 0x02, count: 32) | ||
| privateKeys["33333333333333333333333333333333"] = Data(repeating: 0x03, count: 32) | ||
| } |
There was a problem hiding this comment.
Critical: Data race on mutable state with @unchecked Sendable.
The class is marked @unchecked Sendable but contains unsynchronized mutable state (privateKeys dictionary). This bypasses Swift's concurrency safety checks and can lead to data races when the signer is accessed from multiple threads, especially since addPrivateKey and removePrivateKey mutate the dictionary without protection.
Apply this diff to add thread-safe access:
public final class TestSigner: Signer, @unchecked Sendable {
- private var privateKeys: [String: Data] = [:]
+ private let queue = DispatchQueue(label: "com.dashpay.testsigner.privatekeys")
+ private var _privateKeys: [String: Data] = [:]
+
+ private var privateKeys: [String: Data] {
+ get { queue.sync { _privateKeys } }
+ set { queue.sync { _privateKeys = newValue } }
+ }
public init() {
- // Initialize with some test private keys for demo purposes
- // In a real app, these would be securely stored and retrieved
- privateKeys["11111111111111111111111111111111"] = Data(repeating: 0x01, count: 32)
- privateKeys["22222222222222222222222222222222"] = Data(repeating: 0x02, count: 32)
- privateKeys["33333333333333333333333333333333"] = Data(repeating: 0x03, count: 32)
+ queue.sync {
+ // Initialize with some test private keys for demo purposes
+ // In a real app, these would be securely stored and retrieved
+ _privateKeys["11111111111111111111111111111111"] = Data(repeating: 0x01, count: 32)
+ _privateKeys["22222222222222222222222222222222"] = Data(repeating: 0x02, count: 32)
+ _privateKeys["33333333333333333333333333333333"] = Data(repeating: 0x03, count: 32)
+ }
}Then update the helper methods:
public func addPrivateKey(_ key: Data, forIdentity identityId: String) {
- privateKeys[identityId] = key
+ queue.sync { _privateKeys[identityId] = key }
}
public func removePrivateKey(forIdentity identityId: String) {
- privateKeys.removeValue(forKey: identityId)
+ queue.sync { _privateKeys.removeValue(forKey: identityId) }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/swift-sdk/Sources/SwiftDashSDK/FFI/Signer.swift around lines 25-34,
the TestSigner is marked @unchecked Sendable but stores mutable privateKeys
without synchronization, causing potential data races; fix by adding a private
serial (or concurrent with barrier) DispatchQueue to guard access, update all
reads to use queue.sync and all mutations (addPrivateKey, removePrivateKey, any
initial setup) to use queue.async or queue.sync with barrier for writes, and
keep the class final (or remove @unchecked Sendable if you prefer making an
actor) so concurrent access is serialized and the dictionary is safely
accessed/modified.
| public func sign(identityPublicKey: Data, data: Data) -> Data? { | ||
| // In a real implementation, this would: | ||
| // 1. Find the identity by its public key | ||
| // 2. Retrieve the corresponding private key from secure storage | ||
| // 3. Sign the data using the private key | ||
| // 4. Return the signature | ||
|
|
||
| // For demo purposes, we'll create a mock signature | ||
| // based on the public key and data | ||
| var signature = Data() | ||
| signature.append(contentsOf: "SIGNATURE:".utf8) | ||
| signature.append(identityPublicKey.prefix(32)) | ||
| signature.append(data.prefix(32)) | ||
|
|
||
| // Ensure signature is at least 64 bytes (typical for ECDSA) | ||
| while signature.count < 64 { | ||
| signature.append(0) | ||
| } | ||
|
|
||
| return signature | ||
| } |
There was a problem hiding this comment.
Major: privateKeys dictionary is never used in signing.
The sign method creates a mock signature without consulting the privateKeys dictionary, making the addPrivateKey/removePrivateKey methods misleading. Either use the stored keys to determine signing capability, or remove the unused key management infrastructure.
Consider one of these approaches:
Option 1: Use privateKeys to enable realistic testing
public func sign(identityPublicKey: Data, data: Data) -> Data? {
- // In a real implementation, this would:
- // 1. Find the identity by its public key
- // 2. Retrieve the corresponding private key from secure storage
- // 3. Sign the data using the private key
- // 4. Return the signature
-
- // For demo purposes, we'll create a mock signature
- // based on the public key and data
+ // Check if we have a key for this identity
+ let keyId = identityPublicKey.base58EncodedString // or another mapping
+ guard privateKeys[keyId] != nil else {
+ return nil // Cannot sign without private key
+ }
+
+ // Create a mock signature based on the public key and data
var signature = Data()Option 2: Remove unused key management
public final class TestSigner: Signer, @unchecked Sendable {
- private var privateKeys: [String: Data] = [:]
-
public init() {
- // Initialize with some test private keys for demo purposes
- // In a real app, these would be securely stored and retrieved
- privateKeys["11111111111111111111111111111111"] = Data(repeating: 0x01, count: 32)
- privateKeys["22222222222222222222222222222222"] = Data(repeating: 0x02, count: 32)
- privateKeys["33333333333333333333333333333333"] = Data(repeating: 0x03, count: 32)
}
- public func addPrivateKey(_ key: Data, forIdentity identityId: String) {
- privateKeys[identityId] = key
- }
-
- public func removePrivateKey(forIdentity identityId: String) {
- privateKeys.removeValue(forKey: identityId)
- }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/swift-sdk/Sources/SwiftDashSDK/FFI/Signer.swift around lines 36 to
56 the sign(identityPublicKey:data:) method ignores the privateKeys dictionary
making addPrivateKey/removePrivateKey pointless; update sign to consult
privateKeys: look up the private key by the provided identityPublicKey and if
not found return nil (or throw) to reflect signing capability, and if found use
that key material when constructing the mock signature (e.g., include a
hash/bytes derived from the stored private key instead of or in addition to the
public key), preserving the existing minimum-length padding behavior;
alternatively, if you prefer Option 2, remove the privateKeys storage and its
add/remove methods and update callers/tests accordingly so there is no unused
key management API.
| init(ffiBalance: FFIBalance) { | ||
| self.confirmed = ffiBalance.confirmed | ||
| self.unconfirmed = ffiBalance.unconfirmed | ||
| self.immature = ffiBalance.immature | ||
| self.total = ffiBalance.total | ||
| } | ||
|
|
||
| /// Public initializer for Balance | ||
| public init(confirmed: UInt64 = 0, unconfirmed: UInt64 = 0, immature: UInt64 = 0) { | ||
| self.confirmed = confirmed | ||
| self.unconfirmed = unconfirmed | ||
| self.immature = immature | ||
| self.total = confirmed + unconfirmed + immature | ||
| } |
There was a problem hiding this comment.
Inconsistent total computation between initializers.
The FFI initializer (lines 197-202) assigns total directly from ffiBalance.total, while the public initializer (lines 205-210) computes it as confirmed + unconfirmed + immature. This creates two different computation paths for the same property, potentially violating struct invariants if the FFI provides a total that doesn't match the sum of its components.
Consider either:
- Option 1 (preferred): Make
totala computed property instead of stored:
- public let total: UInt64
+ public var total: UInt64 {
+ confirmed + unconfirmed + immature
+ }And remove the total assignment from both initializers.
- Option 2: Have the FFI initializer also compute
total:
init(ffiBalance: FFIBalance) {
self.confirmed = ffiBalance.confirmed
self.unconfirmed = ffiBalance.unconfirmed
self.immature = ffiBalance.immature
- self.total = ffiBalance.total
+ self.total = ffiBalance.confirmed + ffiBalance.unconfirmed + ffiBalance.immature
}Option 1 is cleaner and eliminates the possibility of inconsistent state.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| init(ffiBalance: FFIBalance) { | |
| self.confirmed = ffiBalance.confirmed | |
| self.unconfirmed = ffiBalance.unconfirmed | |
| self.immature = ffiBalance.immature | |
| self.total = ffiBalance.total | |
| } | |
| /// Public initializer for Balance | |
| public init(confirmed: UInt64 = 0, unconfirmed: UInt64 = 0, immature: UInt64 = 0) { | |
| self.confirmed = confirmed | |
| self.unconfirmed = unconfirmed | |
| self.immature = immature | |
| self.total = confirmed + unconfirmed + immature | |
| } | |
| init(ffiBalance: FFIBalance) { | |
| self.confirmed = ffiBalance.confirmed | |
| self.unconfirmed = ffiBalance.unconfirmed | |
| self.immature = ffiBalance.immature | |
| self.total = ffiBalance.confirmed + ffiBalance.unconfirmed + ffiBalance.immature | |
| } | |
| /// Public initializer for Balance | |
| public init(confirmed: UInt64 = 0, unconfirmed: UInt64 = 0, immature: UInt64 = 0) { | |
| self.confirmed = confirmed | |
| self.unconfirmed = unconfirmed | |
| self.immature = immature | |
| self.total = confirmed + unconfirmed + immature | |
| } |
🤖 Prompt for AI Agents
In packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyWalletTypes.swift around
lines 197-210 there is an inconsistency: the FFI initializer assigns total from
ffiBalance.total while the public initializer computes total as confirmed +
unconfirmed + immature, which can yield inconsistent state; fix it by making
total a computed property (preferred): change the total stored property to a
computed property that returns confirmed + unconfirmed + immature and remove any
assignments to total from both initializers; alternatively, if you must keep
total stored, update the FFI initializer to compute total = ffiBalance.confirmed
+ ffiBalance.unconfirmed + ffiBalance.immature so both initializers compute
total the same way.
| public static func from(_ contract: ContractModel) -> PersistentDataContract { | ||
| let idData = Data.identifier(fromBase58: contract.id) ?? Data() | ||
| let serializedContract = (try? JSONSerialization.data(withJSONObject: contract.schema)) ?? Data() | ||
|
|
||
| let persistent = PersistentDataContract( | ||
| id: idData, | ||
| name: contract.name, | ||
| serializedContract: serializedContract, | ||
| version: contract.version, | ||
| ownerId: contract.ownerId, | ||
| schema: contract.schema, | ||
| documentTypesList: contract.documentTypes, | ||
| keywords: contract.keywords, | ||
| description: contract.description, | ||
| hasTokens: !contract.tokens.isEmpty | ||
| ) | ||
|
|
||
| return persistent | ||
| } |
There was a problem hiding this comment.
Missing network parameter and potential uniqueness constraint issue.
-
Unlike the SwiftExampleApp version (see
from(_ model: ContractModel, network: String)), this method doesn't accept anetworkparameter and defaults to "testnet" via the initializer. -
If
Data.identifier(fromBase58: contract.id)returnsnil, an emptyData()is used forid. Multiple contracts with invalid IDs would violate the@Attribute(.unique)constraint.
- public static func from(_ contract: ContractModel) -> PersistentDataContract {
- let idData = Data.identifier(fromBase58: contract.id) ?? Data()
+ public static func from(_ contract: ContractModel, network: String = "testnet") -> PersistentDataContract? {
+ guard let idData = Data.identifier(fromBase58: contract.id), !idData.isEmpty else {
+ return nil
+ }
let serializedContract = (try? JSONSerialization.data(withJSONObject: contract.schema)) ?? Data()
let persistent = PersistentDataContract(
id: idData,
name: contract.name,
serializedContract: serializedContract,
version: contract.version,
ownerId: contract.ownerId,
schema: contract.schema,
documentTypesList: contract.documentTypes,
keywords: contract.keywords,
description: contract.description,
- hasTokens: !contract.tokens.isEmpty
+ hasTokens: !contract.tokens.isEmpty,
+ network: network
)
return persistent
}| public func updateRevision(_ newRevision: Int64) { | ||
| self.revision = Int32(newRevision) | ||
| self.updatedAt = Date() | ||
| } |
There was a problem hiding this comment.
Potential data loss from Int64 to Int32 truncation.
updateRevision(_ newRevision: Int64) truncates to Int32, which will silently overflow for values > 2,147,483,647. Either use consistent types or add bounds checking.
public func updateRevision(_ newRevision: Int64) {
- self.revision = Int32(newRevision)
+ self.revision = Int32(clamping: newRevision)
self.updatedAt = Date()
}| public var formattedBalance: String { | ||
| let dashAmount = Double(balance) / 100_000_000_000 | ||
| return String(format: "%.8f DASH", dashAmount) | ||
| } |
There was a problem hiding this comment.
Incorrect divisor for DASH balance formatting.
DASH uses 8 decimal places (1 DASH = 100,000,000 duffs), but the divisor here is 100_000_000_000 (11 zeros). This will show balances ~1000x smaller than they should be.
public var formattedBalance: String {
- let dashAmount = Double(balance) / 100_000_000_000
+ let dashAmount = Double(balance) / 100_000_000
return String(format: "%.8f DASH", dashAmount)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public var formattedBalance: String { | |
| let dashAmount = Double(balance) / 100_000_000_000 | |
| return String(format: "%.8f DASH", dashAmount) | |
| } | |
| public var formattedBalance: String { | |
| let dashAmount = Double(balance) / 100_000_000 | |
| return String(format: "%.8f DASH", dashAmount) | |
| } |
🤖 Prompt for AI Agents
In
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentIdentity.swift
around lines 86–89, the DASH balance divisor is incorrect (100_000_000_000);
change it to 100_000_000 so 1 DASH = 100,000,000 duffs (e.g., compute dashAmount
= Double(balance) / 100_000_000) and keep the existing String(format:) to render
eight decimal places; optionally use Decimal for higher precision if needed.
| let persistent = PersistentIdentity( | ||
| identityId: identity.id, | ||
| balance: Int64(identity.balance), | ||
| revision: 0, | ||
| isLocal: identity.isLocal, | ||
| alias: identity.alias, | ||
| dpnsName: identity.dpnsName, | ||
| mainDpnsName: identity.mainDpnsName, | ||
| identityType: identity.type, | ||
| network: network.rawValue, | ||
| walletId: identity.walletId | ||
| ) |
There was a problem hiding this comment.
Unsafe UInt64 ↔ Int64 conversions for balance.
At line 175, Int64(identity.balance) can crash if balance > Int64.max. At line 204, UInt64(balance) will produce incorrect values if balance is negative. Consider using safe conversions.
- balance: Int64(identity.balance),
+ balance: Int64(clamping: identity.balance),And for the reverse:
- balance: UInt64(balance),
+ balance: balance >= 0 ? UInt64(balance) : 0,Also applies to: 202-217
🤖 Prompt for AI Agents
In
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentIdentity.swift
around lines 173-184 and 202-217, the current direct casts
Int64(identity.balance) and UInt64(balance) are unsafe and can crash or produce
incorrect values when balance is out of range; replace these with safe
conversions: when mapping from UInt64 to Int64 use Int64(exactly:
identity.balance) and handle the nil (either by returning an error, clamping to
Int64.max, or using a fallback value) and when mapping from Int64 to UInt64 use
UInt64(exactly: balance) after validating balance >= 0 (or use clamping/explicit
failure handling) so you never force-unwrap or silently produce wrong values;
ensure you propagate or log errors/handle the fallback consistently where the
conversion occurs.
| return IdentityPublicKey( | ||
| id: KeyID(keyId), | ||
| purpose: purpose, | ||
| securityLevel: securityLevel, | ||
| contractBounds: contractBounds?.first.map { .singleContract(id: $0) }, | ||
| keyType: keyType, | ||
| readOnly: readOnly, | ||
| data: publicKeyData, | ||
| disabledAt: disabledAt.map { TimestampMillis($0) } | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Script executed:
# First, find the KeyContractBounds enum definition
ast-grep --pattern 'enum KeyContractBounds {
$$$
}'Repository: dashpay/platform
<shell_result>
Repository: dashpay/platform
Exit code: 0
</shell_result>
Script executed:
# Also search for KeyContractBounds with rg in case ast-grep misses it
rg -n 'KeyContractBounds' --type swift -C 3Repository: dashpay/platform
<shell_result>
Repository: dashpay/platform
Exit code: 1
</shell_result>
Script executed:
# Check IdentityPublicKey definition to understand its KeyContractBounds parameter
rg -n 'struct IdentityPublicKey|class IdentityPublicKey' --type swift -A 20Repository: dashpay/platform
<shell_result>
Repository: dashpay/platform
Exit code: 0
stdout:
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift:107:public struct IdentityPublicKey: Codable, Equatable, Sendable {
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-108- public let id: KeyID
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-109- public let purpose: KeyPurpose
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-110- public let securityLevel: SecurityLevel
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-111- public let contractBounds: ContractBounds?
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-112- public let keyType: KeyType
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-113- public let readOnly: Bool
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-114- public let data: BinaryData
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-115- public let disabledAt: TimestampMillis?
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-116-
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-117- /// Check if the key is currently disabled
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-118- public var isDisabled: Bool {
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-119- guard let disabledAt = disabledAt else { return false }
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-120- let currentTime = TimestampMillis(Date().timeIntervalSince1970 * 1000)
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-121- return disabledAt <= currentTime
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-122- }
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-123-
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-124- public init(
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-125- id: KeyID,
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-126- purpose: KeyPurpose,
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-127- securityLevel: SecurityLevel,
</shell_result>
Script executed:
# Search for ContractBounds definition
rg -n 'enum ContractBounds|struct ContractBounds' --type swift -C 5Repository: dashpay/platform
<shell_result>
Repository: dashpay/platform
Exit code: 0
stdout:
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-142- }
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-143-}
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-144-
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-145-// MARK: - Contract Bounds
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-146-
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift:147:public enum ContractBounds: Codable, Equatable, Sendable {
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-148- case singleContract(id: Identifier)
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-149- case singleContractDocumentType(id: Identifier, documentTypeName: String)
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-150-
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-151- public var description: String {
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-152- switch self {
</shell_result>
Script executed:
# Check the full PersistentPublicKey.swift file to understand the implementation
wc -l packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentPublicKey.swiftRepository: dashpay/platform
<shell_result>
Repository: dashpay/platform
Exit code: 0
stdout:
142 packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentPublicKey.swift
</shell_result>
Script executed:
# Get the full PersistentPublicKey.swift to see the complete context
cat -n packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentPublicKey.swiftRepository: dashpay/platform
<shell_result>
Repository: dashpay/platform
Exit code: 0
stdout:
1 import Foundation
2 import SwiftData
3
4 /// SwiftData model for persisting public key data
5 @Model
6 public final class PersistentPublicKey {
7 // MARK: - Core Properties
8 public var keyId: Int32
9 public var purpose: String
10 public var securityLevel: String
11 public var keyType: String
12 public var readOnly: Bool
13 public var disabledAt: Int64?
14
15 // MARK: - Key Data
16 public var publicKeyData: Data
17
18 // MARK: - Contract Bounds
19 public var contractBoundsData: Data?
20
21 // MARK: - Private Key Reference (optional)
22 public var privateKeyKeychainIdentifier: String?
23
24 // MARK: - Metadata
25 public var identityId: String
26 public var createdAt: Date
27 public var lastAccessed: Date?
28
29 // MARK: - Relationships
30 @Relationship(inverse: \PersistentIdentity.publicKeys)
31 public var identity: PersistentIdentity?
32
33 // MARK: - Initialization
34 public init(
35 keyId: Int32,
36 purpose: KeyPurpose,
37 securityLevel: SecurityLevel,
38 keyType: KeyType,
39 publicKeyData: Data,
40 readOnly: Bool = false,
41 disabledAt: Int64? = nil,
42 contractBounds: [Data]? = nil,
43 identityId: String
44 ) {
45 self.keyId = keyId
46 self.purpose = String(purpose.rawValue)
47 self.securityLevel = String(securityLevel.rawValue)
48 self.keyType = String(keyType.rawValue)
49 self.publicKeyData = publicKeyData
50 self.readOnly = readOnly
51 self.disabledAt = disabledAt
52 if let contractBounds = contractBounds {
53 self.contractBoundsData = try? JSONSerialization.data(withJSONObject: contractBounds.map { $0.base64EncodedString() })
54 } else {
55 self.contractBoundsData = nil
56 }
57 self.identityId = identityId
58 self.createdAt = Date()
59 }
60
61 // MARK: - Computed Properties
62 public var contractBounds: [Data]? {
63 get {
64 guard let data = contractBoundsData,
65 let json = try? JSONSerialization.jsonObject(with: data),
66 let strings = json as? [String] else {
67 return nil
68 }
69 return strings.compactMap { Data(base64Encoded: $0) }
70 }
71 set {
72 if let newValue = newValue {
73 contractBoundsData = try? JSONSerialization.data(withJSONObject: newValue.map { $0.base64EncodedString() })
74 } else {
75 contractBoundsData = nil
76 }
77 }
78 }
79
80 public var purposeEnum: KeyPurpose? {
81 guard let purposeInt = UInt8(purpose) else { return nil }
82 return KeyPurpose(rawValue: purposeInt)
83 }
84
85 public var securityLevelEnum: SecurityLevel? {
86 guard let levelInt = UInt8(securityLevel) else { return nil }
87 return SecurityLevel(rawValue: levelInt)
88 }
89
90 public var keyTypeEnum: KeyType? {
91 guard let typeInt = UInt8(keyType) else { return nil }
92 return KeyType(rawValue: typeInt)
93 }
94
95 public var isDisabled: Bool {
96 disabledAt != nil
97 }
98
99 /// Check if this public key has an associated private key identifier
100 public var hasPrivateKeyIdentifier: Bool {
101 privateKeyKeychainIdentifier != nil
102 }
103 }
104
105 // MARK: - Conversion Extensions
106
107 extension PersistentPublicKey {
108 /// Convert to IdentityPublicKey
109 public func toIdentityPublicKey() -> IdentityPublicKey? {
110 guard let purpose = purposeEnum,
111 let securityLevel = securityLevelEnum,
112 let keyType = keyTypeEnum else {
113 return nil
114 }
115
116 return IdentityPublicKey(
117 id: KeyID(keyId),
118 purpose: purpose,
119 securityLevel: securityLevel,
120 contractBounds: contractBounds?.first.map { .singleContract(id: $0) },
121 keyType: keyType,
122 readOnly: readOnly,
123 data: publicKeyData,
124 disabledAt: disabledAt.map { TimestampMillis($0) }
125 )
126 }
127
128 /// Create from IdentityPublicKey
129 public static func from(_ publicKey: IdentityPublicKey, identityId: String) -> PersistentPublicKey? {
130 return PersistentPublicKey(
131 keyId: Int32(publicKey.id),
132 purpose: publicKey.purpose,
133 securityLevel: publicKey.securityLevel,
134 keyType: publicKey.keyType,
135 publicKeyData: publicKey.data,
136 readOnly: publicKey.readOnly,
137 disabledAt: publicKey.disabledAt.map { Int64($0) },
138 contractBounds: publicKey.contractBounds != nil ? [publicKey.contractBounds!.contractId] : nil,
139 identityId: identityId
140 )
141 }
142 }
</shell_result>
Script executed:
# Get full ContractBounds definition including any computed properties
rg -n 'public enum ContractBounds' --type swift -A 30Repository: dashpay/platform
<shell_result>
Repository: dashpay/platform
Exit code: 0
stdout:
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift:147:public enum ContractBounds: Codable, Equatable, Sendable {
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-148- case singleContract(id: Identifier)
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-149- case singleContractDocumentType(id: Identifier, documentTypeName: String)
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-150-
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-151- public var description: String {
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-152- switch self {
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-153- case .singleContract(let id):
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-154- return "Limited to contract: \(id.toBase58())"
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-155- case .singleContractDocumentType(let id, let docType):
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-156- return "Limited to \(docType) in contract: \(id.toBase58())"
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-157- }
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-158- }
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-159-
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-160- public var contractId: Identifier {
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-161- switch self {
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-162- case .singleContract(let id):
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-163- return id
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-164- case .singleContractDocumentType(let id, _):
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-165- return id
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-166- }
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-167- }
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-168-}
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-169-
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-170-// MARK: - Type Aliases
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-171-// These are used for compatibility with the FFI layer
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-172-public typealias KeyID = UInt32
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-173-public typealias BinaryData = Data
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-174-public typealias TimestampMillis = UInt64
packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift-175-public typealias Identifier = Data
</shell_result>
Script executed:
# Check if ContractBounds has a contractId computed property
rg -n 'contractId' packages/swift-sdk/Sources/SwiftDashSDK/IdentityTypes.swift -C 2Repository: dashpay/platform
<shell_result>
Repository: dashpay/platform
Exit code: 0
stdout:
158- }
159-
160: public var contractId: Identifier {
161- switch self {
162- case .singleContract(let id):
</shell_result>
Correct enum name in verification request—it's ContractBounds, not KeyContractBounds.
The concern about only using the first contract bound is valid. toIdentityPublicKey() stores an array of Data in the model but only converts the first element to IdentityPublicKey, which accepts a single ContractBounds. While this appears intentional (the domain model only supports single-contract variants), the silent discarding of additional contract bounds could be made explicit or documented. Consider whether multi-contract storage is necessary or if the model should be constrained to single bounds.
| // MARK: - Query Helpers | ||
| extension PersistentToken { | ||
| public static func mintableTokensPredicate() -> Predicate<PersistentToken> { | ||
| #Predicate<PersistentToken> { token in | ||
| token.manualMintingRules != nil | ||
| } | ||
| } | ||
|
|
||
| public static func burnableTokensPredicate() -> Predicate<PersistentToken> { | ||
| #Predicate<PersistentToken> { token in | ||
| token.manualBurningRules != nil | ||
| } | ||
| } | ||
|
|
||
| public static func freezableTokensPredicate() -> Predicate<PersistentToken> { | ||
| #Predicate<PersistentToken> { token in | ||
| token.freezeRules != nil | ||
| } | ||
| } | ||
|
|
||
| public static func distributionTokensPredicate() -> Predicate<PersistentToken> { | ||
| #Predicate<PersistentToken> { token in | ||
| token.perpetualDistribution != nil || token.preProgrammedDistribution != nil | ||
| } | ||
| } | ||
|
|
||
| public static func pausedTokensPredicate() -> Predicate<PersistentToken> { | ||
| #Predicate<PersistentToken> { token in | ||
| token.isPaused == true | ||
| } | ||
| } | ||
|
|
||
| public static func tokensByContractPredicate(contractId: Data) -> Predicate<PersistentToken> { | ||
| #Predicate<PersistentToken> { token in | ||
| token.contractId == contractId | ||
| } | ||
| } | ||
|
|
||
| public static func tokensWithControlRulePredicate(rule: ControlRuleType) -> Predicate<PersistentToken> { | ||
| switch rule { | ||
| case .manualMinting: | ||
| return #Predicate<PersistentToken> { token in | ||
| token.manualMintingRules != nil | ||
| } | ||
| case .manualBurning: | ||
| return #Predicate<PersistentToken> { token in | ||
| token.manualBurningRules != nil | ||
| } | ||
| case .freeze: | ||
| return #Predicate<PersistentToken> { token in | ||
| token.freezeRules != nil | ||
| } | ||
| case .unfreeze: | ||
| return #Predicate<PersistentToken> { token in | ||
| token.unfreezeRules != nil | ||
| } | ||
| case .destroyFrozenFunds: | ||
| return #Predicate<PersistentToken> { token in | ||
| token.destroyFrozenFundsRules != nil | ||
| } | ||
| case .emergencyAction: | ||
| return #Predicate<PersistentToken> { token in | ||
| token.emergencyActionRules != nil | ||
| } | ||
| case .conventions: | ||
| return #Predicate<PersistentToken> { token in | ||
| token.conventionsChangeRules != nil | ||
| } | ||
| case .maxSupply: | ||
| return #Predicate<PersistentToken> { token in | ||
| token.maxSupplyChangeRules != nil | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Script executed:
#!/bin/bash
# Search for ControlRuleType enum definition
echo "=== ControlRuleType enum definition ==="
rg -n "enum ControlRuleType" --type swift -A 15
echo ""
echo "=== ChangeControlRuleType enum definition ==="
rg -n "enum ChangeControlRuleType" --type swift -A 15
echo ""
echo "=== Search for ControlRuleType usages ==="
rg -n "ControlRuleType" --type swift -B 2 -A 2Repository: dashpay/platform
<shell_result>
Repository: dashpay/platform
Exit code: 0
stdout:
=== ControlRuleType enum definition ===
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift:176:public enum ControlRuleType: Sendable {
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-177- case conventions
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-178- case maxSupply
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-179- case manualMinting
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-180- case manualBurning
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-181- case freeze
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-182- case unfreeze
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-183- case destroyFrozenFunds
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-184- case emergencyAction
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-185-}
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-186-
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-187-/// Types of change control rules for token configuration
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-188-public enum ChangeControlRuleType: Sendable {
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-189- case conventions
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-190- case maxSupply
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-191- case manualMinting
=== ChangeControlRuleType enum definition ===
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift:188:public enum ChangeControlRuleType: Sendable {
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-189- case conventions
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-190- case maxSupply
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-191- case manualMinting
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-192- case manualBurning
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-193- case freeze
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-194- case unfreeze
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-195- case destroyFrozenFunds
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-196- case emergencyAction
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-197- case tradeMode
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-198-}
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-199-
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-200-// MARK: - Token Event Types
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-201-
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-202-/// Types of token history events
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-203-public enum TokenEventType: String, CaseIterable, Sendable {
=== Search for ControlRuleType usages ===
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Models/SwiftData/PersistentToken.swift-13-public typealias AuthorizedActionTakers = SwiftDashSDK.AuthorizedActionTakers
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Models/SwiftData/PersistentToken.swift-14-public typealias TokenTradeMode = SwiftDashSDK.TokenTradeMode
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Models/SwiftData/PersistentToken.swift:15:public typealias ControlRuleType = SwiftDashSDK.ControlRuleType
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Models/SwiftData/PersistentToken.swift:16:public typealias ChangeControlRuleType = SwiftDashSDK.ChangeControlRuleType
--
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift-239-// MARK: - Control Rules Methods
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift-240-extension PersistentToken {
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift:241: public func getChangeControlRules(for type: ChangeControlRuleType) -> ChangeControlRules? {
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift-242- switch type {
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift-243- case .conventions: return conventionsChangeRules
--
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift-253- }
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift-254-
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift:255: public func setChangeControlRules(_ rules: ChangeControlRules, for type: ChangeControlRuleType) {
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift-256- switch type {
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift-257- case .conventions: conventionsChangeRules = rules
--
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift-308- }
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift-309-
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift:310: public static func tokensWithControlRulePredicate(rule: ControlRuleType) -> Predicate<PersistentToken> {
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift-311- switch rule {
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift-312- case .manualMinting:
--
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-174-
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-175-/// Types of control rules that can be configured on tokens
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift:176:public enum ControlRuleType: Sendable {
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-177- case conventions
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-178- case maxSupply
--
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-186-
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-187-/// Types of change control rules for token configuration
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift:188:public enum ChangeControlRuleType: Sendable {
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-189- case conventions
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift-190- case maxSupply
</shell_result>
Script executed:
#!/bin/bash
# Search for tradeMode or tradeModeRules in PersistentToken model
echo "=== Search for tradeMode in PersistentToken model ==="
rg -n "tradeMode" --type swift packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift -B 2 -A 2
echo ""
echo "=== Check PersistentToken model structure around control rules ==="
rg -n "burnableTokensPredicate\|tradeModeRules\|tradeMode" --type swift packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift -B 1 -A 1Repository: dashpay/platform
<shell_result>
Repository: dashpay/platform
Exit code: 1
stdout:
=== Search for tradeMode in PersistentToken model ===
48-
49- // Marketplace rules
50: public var tradeMode: TokenTradeMode
51: public var tradeModeChangeRules: ChangeControlRules?
52-
53- // Main control group
--
95- self.keepsDirectPurchaseHistory = true
96- self.mintingAllowChoosingDestination = true
97: self.tradeMode = TokenTradeMode.notTradeable
98-
99- self.createdAt = Date()
--
173-
174- public var canChangeTradeMode: Bool {
175: tradeModeChangeRules != nil
176- }
177-
--
206-
207- public var isTradeable: Bool {
208: tradeMode != .notTradeable
209- }
210-
--
249- case .destroyFrozenFunds: return destroyFrozenFundsRules
250- case .emergencyAction: return emergencyActionRules
251: case .tradeMode: return tradeModeChangeRules
252- }
253- }
--
263- case .destroyFrozenFunds: destroyFrozenFundsRules = rules
264- case .emergencyAction: emergencyActionRules = rules
265: case .tradeMode: tradeModeChangeRules = rules
266- }
267-
=== Check PersistentToken model structure around control rules ===
</shell_result>
Add missing .tradeMode case to tokensWithControlRulePredicate for consistency.
The asymmetry between ControlRuleType (8 cases) and ChangeControlRuleType (9 cases) is confirmed. ChangeControlRuleType includes .tradeMode which has a corresponding tradeModeChangeRules property in PersistentToken and is handled in both getChangeControlRules() and setChangeControlRules() methods.
However, tokensWithControlRulePredicate(rule:) lacks a case for .tradeMode, creating an inconsistency: users can set/get trade mode control rules but cannot query tokens by trade mode control rules. Add the missing case:
case .tradeMode:
return #Predicate<PersistentToken> { token in
token.tradeModeChangeRules != nil
}Also update ControlRuleType enum to include .tradeMode to match ChangeControlRuleType.
🤖 Prompt for AI Agents
In
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentToken.swift
around lines 272–345, tokensWithControlRulePredicate is missing the .tradeMode
branch; add a case for .tradeMode that returns a Predicate checking
token.tradeModeChangeRules != nil, and also update the ControlRuleType enum
(where defined) to include .tradeMode so the enum and predicate cases are
consistent with ChangeControlRuleType and the existing tradeModeChangeRules
property.
Issue being fixed or feature implemented
What was done?
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Improvements
Dependencies
✏️ Tip: You can customize this high-level summary in your review settings.