Test dynamic library build
This PR updates the iOS bindings to libxmtp version 4.6.0-dev.c4b44a9.
Changes:
- Updated XMTP.podspec version to 4.6.0-dev.c4b44a9
- 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: main
Claude finished @xmtptools's task —— View job
Code Review: libxmtp 4.6.0-dev.c4b44a9 Update
I've reviewed the PR and here's my feedback:
✅ Code Quality & Best Practices
Auto-generated Code: The vast majority of changes (14k+ lines) are auto-generated Swift bindings from the Rust uniffi system. This is standard practice and the code follows the expected patterns for uniffi-generated bindings.
Package Updates: The Package.swift and XMTP.podspec updates look correct:
- Binary URL references updated to
swift-bindings-1.6.1-rc3.c4b44a9 - Checksums updated for the new binaries
- Version bumped to
4.6.1-devin podspec
New Feature: The streamMessageDeletions implementation follows the same pattern as other streaming callbacks (streamMessages, streamConsent, etc.), which is good for consistency.
⚠️ Potential Bugs & Issues
1. Integer Overflow Risk (CONFIRMED)
Macroscope correctly identified a potential issue in Sources/XMTPiOSDynamic/Libxmtp/xmtpv3.swift:367:
currentHandle += 1 // Line 367
This can overflow at UInt64.max (18,446,744,073,709,551,615) and cause a runtime crash. While reaching this limit is extremely unlikely in practice (would require billions of billions of callback registrations), it's still a potential crash vector.
Impact: Low probability but HIGH severity if it occurs (app crash).
Recommended fix:
currentHandle &+= 1 // Use wrapping increment
if currentHandle == 0 { currentHandle = 1 } // Skip 0 to avoid null pointer confusion
Note: The same issue exists in the static version at Sources/XMTPiOS/Libxmtp/xmtpv3.swift:364, but since that's an older file not modified in this PR, it's a pre-existing issue.
🔒 Security Concerns
No major security issues identified. The changes are primarily:
- Binary checksum updates (good security practice)
- Auto-generated FFI bindings (inherits security from Rust libxmtp)
- Callback handling that follows existing patterns
⚡ Performance Considerations
Auto-generated Code: The uniffi-generated code uses standard patterns for FFI (Foreign Function Interface) with Rust. No performance red flags.
Handle Map: The UniffiHandleMap<T> uses NSLock for thread safety, which is appropriate for this use case. The lock scope is minimal (just the critical section), which is good.
Callback Pattern: The new FfiMessageDeletionCallback uses the same vtable pattern as other callbacks, so performance should be consistent with existing streaming functionality.
🧪 Test Coverage
Missing Tests: There are no tests for the new streamMessageDeletions functionality. While this is an auto-generated binding to libxmtp (which presumably has its own tests), it would be beneficial to have integration tests in the iOS SDK.
Suggested test coverage:
- Test that
streamMessageDeletionscallback receives message IDs when messages are deleted - Test that the stream closer properly terminates the stream
- Test error handling if the callback throws
Files to check for test patterns: Tests/XMTPTests/GroupTests.swift, Tests/XMTPTests/DmTests.swift (which already test disappearing messages)
📋 Summary
Approve with recommendations:
- HIGH PRIORITY: Fix the integer overflow issue in
UniffiHandleMap.insert()in the dynamic module (line 367) - MEDIUM PRIORITY: Consider fixing the same issue in the static module (line 364) as a separate cleanup
- LOW PRIORITY: Add integration tests for
streamMessageDeletionsto verify the callback works as expected
The binary updates and version bumps look correct. The new feature implementation follows existing patterns. The main concern is the overflow risk which, while unlikely, should be addressed.
Upgrade XMTP iOS bindings to libxmtp 4.6.0-dev.c4b44a9 and add xmtpv3.FfiConversationsProtocol.streamMessageDeletions with dynamic LibXMTPSwiftFFIDynamic packaging
Switch the Swift package to a dynamic binary target and add a new xmtpv3.FfiConversationsProtocol.streamMessageDeletions(callback: FfiMessageDeletionCallback) -> FfiStreamCloser, including callback bridging, vtable registration, and checksum validation. Update podspec version to 4.6.1-dev.
📍Where to Start
Start with the new streaming API in xmtpv3.streamMessageDeletions, then review the callback implementation and vtable registration in the same file.
Macroscope summarized 5a157f5.