dev release with special bindings

[neekolas]

Update Kotlin FFI bindings in xmtpv3.kt for a dev release with special bindings by changing FfiConversationDebugInfo.cursor to List<FfiCursor> and making FfiMessage.sequenceId a required ULong

This dev release updates Kotlin FFI bindings and native libraries. The generated bindings in xmtpv3.kt change FfiConversationDebugInfo.cursor from Long to List<FfiCursor> with corresponding converter updates, add the FfiCursor data class and FfiConverterTypeFfiCursor, and change FfiMessage.sequenceId from optional ULong to required ULong. The native libuniffi_xmtpv3.so binaries are replaced for multiple ABIs, and version metadata is updated.

• Modify generated FFI types and converters in xmtpv3.kt, including FfiConversationDebugInfo.cursor to List<FfiCursor> and FfiMessage.sequenceId to required ULong (xmtpv3.kt)
• Add FfiCursor and FfiConverterTypeFfiCursor for list serialization (xmtpv3.kt)
• Replace libuniffi_xmtpv3.so native libraries for arm64-v8a, armeabi-v7a, x86, and x86_64 (e.g., libuniffi_xmtpv3.so)
• Update version metadata in libxmtp-version.txt

📍Where to Start

Start with the generated FFI type changes in FfiConversationDebugInfo and FfiMessage within xmtpv3.kt, then review the associated converters including FfiConverterTypeFfiCursor and list serialization logic.

---

📊 Macroscope summarized e3f6966. 0 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted

🗂️ Filtered Issues

No issues evaluated.

\

[claude]

Claude finished @neekolas's task —— View job

---

Code Review: Dev Release with Special Bindings

I've completed my review of this FFI binding update. Here are my findings:

---

✅ Overall Assessment

This PR updates auto-generated FFI bindings from the native libxmtp library. The changes are well-structured and the generated code follows proper Kotlin conventions. Since these are generated bindings, most issues would need to be addressed at the FFI generator or native library level.

---

🔍 Key Changes Analyzed

1. FfiMessage.sequenceId - Nullable to Non-Nullable (⚠️ Breaking Change)

Location: library/src/main/java/xmtpv3.kt:11531

Change:

// Before
var `sequenceId`: kotlin.ULong?,

// After  
var `sequenceId`: kotlin.ULong,

Impact:

• ⚠️ BREAKING CHANGE: This is a significant type change that will cause compilation errors if any code was checking for null
• The converter changed from FfiConverterOptionalULong to FfiConverterULong (lines 11517, 11561, 11575)
• Good news: My analysis shows no direct usage of sequenceId in the SDK's wrapper classes (DecodedMessage.kt, Group.kt, Dm.kt)
• Concern: If any app code directly accesses FfiMessage.sequenceId, it will break

Recommendation:

• ✅ Document this breaking change in release notes
• ✅ Consider if a deprecation period was possible (though likely handled at native layer)
• ⚠️ Ensure all messages now have sequence IDs - verify this at the native layer

---

2. FfiConversationDebugInfo.cursor - Type Change

Location: library/src/main/java/xmtpv3.kt:10761

Change:

// Before
var `cursor`: kotlin.Long,

// After
var `cursor`: List<FfiCursor>,

Impact:

• ⚠️ BREAKING CHANGE: Complete type change from scalar to list
• Adds new FfiCursor data class with originatorId and sequenceId (lines 10926-10930)
• Adds proper list converter FfiConverterSequenceTypeFfiCursor (lines 15332-15357)
• Good news: ConversationDebugInfo.kt doesn't expose the cursor field, so external API is not affected
• The cursor field is internal debug information, not part of the public API

Recommendation:

• ✅ This is isolated to debug functionality
• ✅ The wrapper class ConversationDebugInfo doesn't expose this field, limiting the blast radius
• ℹ️ This suggests a protocol-level change to support multiple cursors (likely for distributed/federated scenarios)

---

🔒 Security & Safety Review

Null Safety ✅

• The FfiMessage.sequenceId change from nullable to non-nullable is safer as it eliminates null checks
• However, ensure the native layer always provides a sequence ID (runtime safety concern)
• All converter implementations properly handle serialization/deserialization

Type Safety ✅

• New FfiCursor data class uses appropriate unsigned types (UInt, ULong)
• List converter FfiConverterSequenceTypeFfiCursor properly handles collection serialization
• Buffer operations follow established patterns (read size, then items)

Memory & Performance ✅

• List allocation size calculation is correct: sizeForLength + sizeForItems (line 15344)
• Uses iterator pattern for writing to avoid index-based access overhead (line 15352)
• No obvious memory leaks or buffer overflow vulnerabilities

Cryptographic Operations ✅

• No direct crypto code changes
• Changes are purely data structure updates
• Sequence IDs being required improves message ordering guarantees (important for E2E encryption)

---

📱 Mobile Performance Considerations

Positive:

• ✅ List converter uses Kotlin's List() constructor for efficient allocation
• ✅ No unnecessary boxing/unboxing
• ✅ Proper use of unsigned types (UInt, ULong) for FFI boundaries

Potential Concerns:

• ⚠️ cursor changing to List<FfiCursor> could increase memory usage if lists are large
• ℹ️ Consider the typical size of cursor lists - if it's unbounded, there could be memory implications
• ✅ Since this is debug info, impact should be minimal in production

---

🧪 Test Coverage

Current Status:

• ❌ No direct test files found for FfiMessage or FfiConversationDebugInfo
• ℹ️ These are generated bindings, so unit tests at this level may not be expected
• ✅ The wrapper classes (DecodedMessage, ConversationDebugInfo) likely have tests

Recommendations:

1. Integration Tests: Verify that messages are properly deserialized with the new required sequenceId
2. Edge Cases: Test cursor list with 0, 1, and multiple elements
3. Migration Tests: If upgrading from a version with nullable sequenceId, test message compatibility
4. Debug Info Tests: Ensure ConversationDebugInfo works with the new cursor structure

Suggested Test Cases:

// Test sequenceId is always present
@Test
fun `FfiMessage sequenceId is required and deserialized correctly`()

// Test cursor list deserialization
@Test  
fun `FfiConversationDebugInfo cursor list handles empty, single, and multiple cursors`()

// Test backward compatibility if needed
@Test
fun `Messages from older versions are migrated correctly`()

---

🛠️ Code Quality & Best Practices

Kotlin Conventions ✅

• Proper use of data classes
• Backtick identifiers for FFI compatibility (e.g., `cursor`)
• Consistent naming conventions
• Proper companion object declarations

Generated Code Quality ✅

• Consistent structure across all converters
• Proper use of @suppress annotations
• Buffer operations follow safe patterns
• No lint violations apparent

---

⚠️ Potential Issues & Risks

1. Breaking Change Communication

   • Priority: HIGH
   • The sequenceId change from nullable to required is a breaking change
   • Impact: Any code directly accessing FfiMessage.sequenceId will break
   • Mitigation: Clear release notes and migration guide

2. Runtime Null Safety

   • Priority: MEDIUM
   • Ensure native library always provides sequence ID
   • If native returns null, this will crash at FFI boundary
   • Mitigation: Verify native implementation guarantees non-null

3. Cursor List Size

   • Priority: LOW
   • Unknown if cursor lists are bounded
   • Could impact memory on resource-constrained devices
   • Mitigation: Profile memory usage with realistic data

4. Missing Public API Updates

   • Priority: LOW
   • ConversationDebugInfo doesn't expose the new cursor field
   • Consider if debug APIs should expose this information
   • Mitigation: Add accessor if needed for debugging

---

📋 Recommendations Summary

Must Do:

• Document breaking changes in release notes
• Add integration tests for required sequenceId
• Verify native layer always provides non-null sequence IDs

Should Do:

• Add tests for cursor list serialization
• Consider exposing cursor information in ConversationDebugInfo wrapper
• Profile memory usage with realistic cursor list sizes

Nice to Have:

• Add KDoc comments explaining cursor list semantics
• Consider adding validation for cursor list invariants
• Document the migration path for apps using sequenceId

---

📄 Files Reviewed

• library/src/main/java/xmtpv3.kt:10758-10803 - FfiConversationDebugInfo changes
• library/src/main/java/xmtpv3.kt:10926-10955 - New FfiCursor data class
• library/src/main/java/xmtpv3.kt:11499-11576 - FfiMessage sequenceId changes
• library/src/main/java/xmtpv3.kt:15332-15357 - FfiCursor list converter
• library/src/main/java/libxmtp-version.txt - Version metadata
• Native libraries (.so files) - Binary updates verified

---

✅ Conclusion

This is a well-formed FFI binding update with proper converter implementations. The main concerns are:

1. The breaking change to sequenceId requiring clear communication
2. Ensuring runtime safety for the non-nullable sequenceId
3. Adding appropriate test coverage

The code quality is excellent for generated code, and the changes follow Kotlin best practices. The wrapper classes provide good insulation from the FFI layer, limiting the breaking change impact.

Approval Recommendation: ✅ Approve with recommendations - Address breaking change documentation and add integration tests.

---

[neekolas]

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

• dev release with special bindings #491 👈 (View in Graphite)
• Pass through shouldPush flag #490
• Add deleteMessageLocally #476
• Switch to spotless for formatting and clean up lint config #484
• Upgrade to latest Android Gradle Plugin #483
• Add countMessages #482
• Android Testing Overhaul #477
• Fix android CI #475
• main

---

How to use the Graphite Merge Queue

Add the label mergequeue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

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