xmtp-ios icon indicating copy to clipboard operation
xmtp-ios copied to clipboard

Update to libxmtp 4.6.0-dev.e8b24fd

Open xmtptools opened this issue 1 month ago β€’ 2 comments

This PR updates the iOS bindings to libxmtp version 4.6.0-dev.e8b24fd.

Changes:

  • Updated XMTP.podspec version to 4.6.0-dev.e8b24fd
  • 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

xmtptools avatar Nov 06 '25 01:11 xmtptools

Claude finished @xmtptools's task β€”β€” View job


Code Review Complete

Tasks

  • [x] Read and analyze Package.swift changes
  • [x] Review XMTP.podspec version update
  • [x] Analyze xmtpv3.swift binding changes
  • [x] Check for potential bugs or issues
  • [x] Evaluate security concerns
  • [x] Review test coverage implications
  • [x] Provide comprehensive feedback

Review Summary

This PR updates libxmtp from version 1.6.1-rc2.5597847 to 4.6.0-dev.e8b24fd. The changes consist primarily of auto-generated Swift FFI bindings with one new API feature.

πŸ”΄ Critical Issues

1. Version Regression Concern (Package.swift:28, XMTP.podspec:3)

  • Previous version: 4.6.1-rc3 (release candidate 3)
  • New version: 4.6.0-dev.e8b24fd (dev build)
  • This appears to be a downgrade from RC3 to a dev build, which is unusual and potentially problematic
  • ⚠️ Action Required: Confirm this is intentional. Typically you'd move from RC β†’ release or RC β†’ newer RC, not RC β†’ dev build
  • If intentional, this should be clearly documented in the PR description

⚠️ Moderate Issues

2. Missing Public API Exposure (Sources/XMTPiOS/Libxmtp/xmtpv3.swift:2226-2228)

  • New FFI method streamMessageDeletions added to receive notifications when disappearing messages are deleted
  • The new callback interface FfiMessageDeletionCallback is only available at the FFI layer
  • Missing: No corresponding Swift API in Conversations.swift to expose this functionality to SDK users
  • Recommendation: Add a public-facing method like:
    public func streamMessageDeletions(
        onClose: (() -> Void)? = nil
    ) -> AsyncThrowingStream<Data, Error>
    
    This would match the pattern used by other streaming methods (see Conversations.swift:650-706)

3. No Test Coverage for New Feature

  • The new streamMessageDeletions functionality has no corresponding tests
  • Existing tests in DmTests.swift, GroupTests.swift, and GroupPermissionsTests.swift contain disappearing message tests but don't cover the deletion callback
  • Recommendation: Add tests for:
    • Streaming message deletions in groups
    • Streaming message deletions in DMs
    • Callback behavior when messages are deleted by the disappearing messages worker

4. Large Auto-Generated File Addition (Sources/XMTPiOSDynamic/Libxmtp/xmtpv3.swift)

  • Added a new 14,875-line file that appears to be a duplicate of the static bindings
  • File header indicates it's auto-generated: // This file was autogenerated by some hot garbage in the uniffi crate.
  • Question: Is this intentional? The file path suggests this is for dynamic linking, but:
    • No corresponding update to Package.swift to reference a dynamic binary target
    • The existing Package.swift only references LibXMTPSwiftFFI (static)
    • This could lead to confusion about which bindings to use

βœ… Positive Observations

5. Dependency Management

  • Binary checksum properly updated (Package.swift:29)
  • Download URL correctly updated to match new version
  • CocoaPods spec version aligned with Package.swift

6. Code Quality of Generated Bindings

  • Auto-generated code follows consistent patterns
  • Proper error handling with rustCall wrappers
  • Memory management appears sound (proper deinit, uniffiClonePointer)
  • Thread safety considered with @unchecked Sendable annotations

7. API Design of New Feature

  • New callback interface follows existing patterns (FfiMessageCallback, FfiConversationCallback)
  • Proper cleanup with FfiStreamCloser return type
  • Async/await integration is correct

πŸ”’ Security Considerations

8. No Security Concerns Identified

  • Checksum validation ensures binary integrity
  • No exposure of sensitive data in new APIs
  • Message deletion callback only receives message IDs (Data), not message contents
  • Auto-generated bindings don't introduce new attack surfaces

πŸ“‹ Recommendations

  1. URGENT: Clarify version number regression from RC3 β†’ dev build
  2. Add Swift-level API to expose streamMessageDeletions functionality
  3. Add comprehensive test coverage for message deletion streaming
  4. Clarify the purpose of Sources/XMTPiOSDynamic/Libxmtp/xmtpv3.swift file
  5. Consider documenting the new disappearing messages deletion callback in release notes
  6. Run existing test suite to ensure no regressions: ./script/run_tests.sh

Performance Considerations

  • Auto-generated bindings typically have minimal overhead
  • New streaming callback should be lightweight (only receives message IDs)
  • No performance concerns identified in the changes

Overall Assessment: The binding updates appear mechanically sound, but there are concerns about the version regression and incomplete API exposure. The PR would benefit from clarification on the versioning and completion of the public API layer for the new deletion streaming feature.


claude[bot] avatar Nov 06 '25 01:11 claude[bot]

Add xmtpv3.FfiConversationsProtocol.streamMessageDeletions and update bindings to libxmtp 4.6.0-dev.e8b24fd in xmtpv3.swift with checksum checks 61355 and 4903

Introduce a new async deletion stream callback and FFI glue in xmtpv3.swift, add the dynamic target bindings, and point build metadata to the 4.6.0-dev.e8b24fd artifacts.

πŸ“Where to Start

Start with xmtpv3.FfiConversationsProtocol.streamMessageDeletions in xmtpv3.swift.


πŸ“Š Macroscope summarized 1ae674d. 1 file reviewed, 38 issues evaluated, 35 issues filtered, 2 comments posted. View details

macroscopeapp[bot] avatar Nov 06 '25 01:11 macroscopeapp[bot]