feat: platform wallet #2855
feat: platform wallet #2855
Conversation
WalkthroughAdds a new platform-encryption crate, a large rs-platform-wallet-ffi FFI layer, Platform Wallet Rust modules (identity management, contacts, DIP‑15 flows), SDK DashPay contact request features, Swift bindings and app UI updates, and updates workspace manifests to prefer local path dependencies over remote git sources. Changes
Sequence Diagram(s)sequenceDiagram
participant Swift as Swift App
participant FFI as Platform Wallet FFI
participant Rust as Platform Wallet (Rust)
participant Crypto as Platform Encryption
Swift->>FFI: platform_wallet_info_create_from_mnemonic(mnemonic, passphrase)
FFI->>Rust: create PlatformWalletInfo from mnemonic
Rust-->>FFI: return wallet handle
FFI-->>Swift: wallet handle
Swift->>FFI: identity_manager_create()
FFI->>Rust: create IdentityManager
Rust-->>FFI: manager handle
FFI-->>Swift: manager handle
Swift->>FFI: managed_identity_send_contact_request(identityHandle, requestHandle)
FFI->>Rust: forward send request
Rust->>Crypto: derive_shared_key_ecdh(priv, pub)
Crypto-->>Rust: 32-byte shared key
Rust->>Crypto: encrypt_extended_public_key(shared_key, iv, xpub)
Crypto-->>Rust: iv + ciphertext
Rust->>Rust: store sent contact request
Rust-->>FFI: success
rect `#E8F4FF`
Note right of Rust: On incoming matching sent request -> auto-establish
Swift->>FFI: managed_identity_add_incoming_contact_request(...)
FFI->>Rust: add incoming request
alt match found
Rust->>Rust: create EstablishedContact and update storages
else
Rust->>Rust: store incoming request
end
Rust-->>FFI: status
FFI-->>Swift: stored/established
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
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.
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
Chores