Update to libxmtp 4.6.0-dev.9df3055
This PR updates the iOS bindings to libxmtp version 4.6.0-dev.9df3055.
Changes:
- Updated XMTP.podspec version to 4.6.0-dev.9df3055
- Updated static binary (LibXMTPSwiftFFI) URL and checksum in Package.swift
- Updated dynamic binary (LibXMTPSwiftFFIDynamic) URL and checksum in Package.swift
- Updated Swift source files (xmtpv3.swift) for both static and dynamic bindings
Base branch: insipx/dynamic-swift
Claude finished @xmtptools's task ββ View job
Code Review Complete β
I've reviewed this PR which updates iOS bindings to libxmtp 4.6.0-dev.9df3055. Here's my detailed feedback:
π΄ Critical Issues
1. Thread-Safety Race Condition in Dynamic Bindings (Sources/XMTPiOSDynamic/Libxmtp/xmtpv3.swift:391)
Severity: HIGH - Already flagged by Macroscope
The ConcurrentHandleMap.count property reads map.count without acquiring the lock, while all mutations are protected by locks. This creates a data race.
Location: Sources/XMTPiOSDynamic/Libxmtp/xmtpv3.swift:391
var count: Int {
get {
map.count // β Unprotected read while mutations use lock
}
}
Fix needed:
var count: Int {
get {
lock.withLock { map.count } // β
Protected read
}
}
Impact: This can cause crashes or undefined behavior when the map is read while being modified concurrently. Since this is generated code, the fix may need to be applied upstream in the UniFFI generator.
β οΈ Medium Priority Issues
2. Missing Test Coverage for New FFI Methods
Severity: MEDIUM
The following new FFI methods have no test coverage:
FfiConversation.leaveGroup()- No tests found in the test suiteFfiGroupSyncSummaryreturn type - Not validated in testsFfiForkRecoveryOptsparameter - No tests for fork recovery functionalityFfiCursortype - New type with no usage tests
Recommendation: Add tests for:
// Test leaveGroup functionality
func testLeaveGroup() async throws {
// Create group, member leaves, verify state
}
// Test sync summary return values
func testSyncAllConversationsReturnsCorrectSummary() async throws {
// Verify numEligible and numSynced are populated correctly
}
// Test fork recovery options
func testClientCreationWithForkRecoveryOpts() async throws {
// Test different FfiForkRecoveryPolicy values
}
3. Unsafe Sendable Conformance - @unchecked Sendable
Severity: MEDIUM
Found 18 instances of @unchecked Sendable in the FFI bindings. While this is standard for UniFFI-generated code, it bypasses Swift 6's concurrency safety checks.
Concern: The classes marked with @unchecked Sendable (like FfiConversation, FfiConversations, etc.) need careful manual verification that they're actually thread-safe, especially given the race condition found in ConcurrentHandleMap.
Recommendation: Document why each FFI object is safe to send across actor boundaries, or consider using proper actor isolation in the high-level Swift wrapper code.
4. Breaking API Change - Sync Methods Now Return Data
Severity: MEDIUM
The signature of sync methods has changed:
syncAllConversations(consentStates:)now returnsFfiGroupSyncSummary(wasvoid)syncPreferences()now returnsFfiGroupSyncSummary(wasvoid)
Impact: Any existing code that calls these methods needs to handle the return value. The high-level Swift API wrappers may need updates.
Recommendation: Check if Sources/XMTPiOS/Conversations.swift or similar files use these methods and update them to utilize the sync summary data.
βΉοΈ Low Priority / Informational
5. Large Generated File Added (14,666 lines)
The new dynamic bindings file (Sources/XMTPiOSDynamic/Libxmtp/xmtpv3.swift) is massive. This is expected for generated code, but consider:
- Ensure it's properly marked as generated in
.gitattributesto exclude from diffs - Consider splitting the module if build times become an issue
- Verify the dynamic vs static bindings have identical APIs (beyond the thread-safety bug)
6. Package.swift and Podspec Updates Look Correct
β
Binary checksums updated properly
β
URLs point to correct release artifacts
β
Version strings are consistent across files (4.6.0-dev.9df3055)
7. New Types Are Well-Structured
The new FFI types follow good Swift conventions:
FfiCursor: Simple value type with properEquatableandHashableconformanceFfiGroupSyncSummary: Clear naming, appropriate types (UInt64)FfiForkRecoveryOpts: Good use of optionals for optional parametersFfiForkRecoveryPolicy: Clean enum with clear cases
π Security Considerations
8. Fork Recovery Feature Needs Documentation
The new FfiForkRecoveryOpts parameter in createClient() introduces fork recovery functionality. This is a security-sensitive feature that could affect group state consistency.
Questions to address:
- What are the security implications of different
FfiForkRecoveryPolicyvalues? - Should
disableRecoveryResponsesdefault totruein production? - What happens if
workerIntervalNsis set too low?
Recommendation: Add documentation or assertions to guide developers on safe usage.
9. Data Exposure in Error Handling
The generic error handling in this FFI layer is appropriate, but verify that error messages don't leak sensitive data (encryption keys, private keys, etc.) through the FFI boundary.
π Performance Considerations
10. Sync Summary Could Reduce Redundant Calls
Positive: The new FfiGroupSyncSummary return type is a good improvement. It allows callers to see numEligible vs numSynced without making additional queries.
11. Async/Await Properly Used
β
All new FFI methods that should be async are correctly marked and use uniffiRustCallAsync
β Summary
Merge Recommendation: β οΈ MERGE WITH CAUTION
This PR should be mergeable after addressing:
- MUST FIX: Thread-safety issue in
ConcurrentHandleMap.count(Sources/XMTPiOSDynamic/Libxmtp/xmtpv3.swift:391) - SHOULD ADD: Test coverage for
leaveGroup(),FfiGroupSyncSummary, and fork recovery - SHOULD VERIFY: High-level Swift code handles new return values from sync methods
Strengths:
- Clean generated code structure
- Good type safety with new structs and enums
- Proper async/await usage
- Version updates are consistent
Generated Code Note: Since this is largely generated from UniFFI, some issues (like the thread-safety bug) may need to be fixed in the upstream generator rather than manually in this PR.
Update XMTP iOS bindings to libxmtp 4.6.0-dev.9df3055 and add FfiConversation.leaveGroup() plus structured sync results in Sources/XMTPiOS/Libxmtp/xmtpv3.swift
Update the Swift bindings to the specified libxmtp build and adjust FFI APIs and models to current Rust interfaces.
-
Add
FfiConversation.leaveGroup()implemented viauniffi_xmtpv3_fn_method_fficonversation_leave_groupin xmtpv3.swift -
Change
FfiConversationsProtocol.syncAllConversations(consentStates:)andFfiXmtpClientProtocol.syncPreferences()to returnFfiGroupSyncSummarywith updated rust buffer converters in xmtpv3.swift -
Modify
connectToBackend(v3Host:gatewayHost:isSecure:appVersion:)andcreateClient(..., forkRecoveryOpts:)signatures and FFI calls in xmtpv3.swift -
Introduce new types and converters:
FfiCursor,FfiForkRecoveryOpts,FfiGroupSyncSummary,FfiForkRecoveryPolicy, and extendFfiMessage,FfiGroupUpdated, andGenericErrorin xmtpv3.swift -
Add generated dynamic bindings file xmtpv3.swift
-
Update binary target URLs/checksums in Package.swift and bump podspec version in XMTP.podspec
πWhere to Start
Start with the updated FFI API surface and method implementations in Sources/XMTPiOS/Libxmtp/xmtpv3.swift, focusing on FfiConversation.leaveGroup(), FfiConversations.syncAllConversations(...), and FfiXmtpClient.syncPreferences() in xmtpv3.swift.
π Macroscope summarized bc1d6fd. 3 files reviewed, 12 issues evaluated, 10 issues filtered, 0 comments posted. View details
[!WARNING] This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite. Learn more
- #606
π (View in Graphite) - #603

main
This stack of pull requests is managed by Graphite. Learn more about stacking.