chore: Bump Xmtp #1323
chore: Bump Xmtp #1323
Conversation
WalkthroughThe pull request introduces significant type declaration modifications across various components within the chat application. Specifically, the type of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (14)
ios/Converse.xcodeproj/project.pbxproj (1)
Line range hint
453-476: Security review recommended for crypto dependencies.The project now includes several cryptography-related dependencies. While the configuration is correct, it's important to ensure these dependencies are properly vetted.
Consider:
- Documenting the security implications of each crypto dependency
- Setting up a process for regular security updates of these dependencies
- Implementing runtime integrity checks for the crypto modules
Also applies to: 498-502
ios/ConverseNotificationExtension/Profile.swift (2)
Line range hint
19-24: Improve error handling in profile refresh flowThe current implementation silently ignores errors from
refreshProfileFromBackend. Consider adding proper error handling to track profile refresh failures.- try? await refreshProfileFromBackend(account: account, address: formattedAddress) + do { + try await refreshProfileFromBackend(account: account, address: formattedAddress) + } catch { + print("Failed to refresh profile for \(formattedAddress): \(error)") + // Consider implementing retry logic or error reporting + }
Line range hint
28-50: Enhance API interaction robustnessThe current implementation has several areas for improvement:
- URL construction could fail silently
- Error types are not specific
- Response handling could be more structured
Consider these improvements:
+enum ProfileError: Error { + case invalidURL + case networkError(Error) + case decodingError(Error) +} func refreshProfileFromBackend(account: String, address: String) async throws { let apiURI = getApiURI() - if (apiURI != nil && !apiURI!.isEmpty) { + guard let apiURI = apiURI, !apiURI.isEmpty, + let url = URL(string: "\(apiURI)/api/profile") else { + throw ProfileError.invalidURL + } - let profileURI = "\(apiURI ?? "")/api/profile" - let response = try await withUnsafeThrowingContinuation { continuation in - AF.request(profileURI, method: .get, parameters: ["address": address]).validate().responseData { response in + AF.request(url, method: .get, parameters: ["address": address]) + .validate() + .responseDecodable(of: ProfileSocials.self) { response in if let data = response.data { continuation.resume(returning: data) return } if let err = response.error { - continuation.resume(throwing: err) + continuation.resume(throwing: ProfileError.networkError(err)) return } } } - - // Create an instance of JSONDecoder - let decoder = JSONDecoder() - - if let socials = try? decoder.decode(ProfileSocials.self, from: response) { - saveProfileSocials(account: account, address: address, socials: socials) - } + saveProfileSocials(account: account, address: address, socials: response)utils/xmtpRN/sync.ts (2)
Line range hint
120-133: Consider extracting retry configuration and enhancing type safety.The error handling and retry mechanism are well implemented. However, consider these improvements:
- Extract retry configuration to constants/config for easier maintenance
- Add type safety to the error handling
+const RETRY_CONFIG = { + retries: 5, + delay: 1000, + factor: 2, + maxDelay: 30000, +} as const; await retryWithBackoff({ fn: () => streamConversations(account), - retries: 5, - delay: 1000, - factor: 2, - maxDelay: 30000, + ...RETRY_CONFIG, context: `streaming conversations for ${account}`, -}).catch((e) => { +}).catch((e: Error) => { logger.error(e, { context: `Failed to stream conversations for ${account} no longer retrying`, }); });
Line range hint
134-146: Consolidate duplicate retry and error handling patterns.The message streaming code duplicates the retry configuration and error handling pattern from the conversation streaming code. Consider extracting this common pattern into a reusable function.
+const streamWithRetry = async ( + streamFn: () => Promise<void>, + account: string, + operationType: string +) => { + return retryWithBackoff({ + fn: () => streamFn(), + ...RETRY_CONFIG, + context: `streaming ${operationType} for ${account}`, + }).catch((e: Error) => { + logger.error(e, { + context: `Failed to stream ${operationType} for ${account} no longer retrying`, + }); + }); +}; -await retryWithBackoff({ - fn: () => streamAllMessages(account), - retries: 5, - delay: 1000, - factor: 2, - maxDelay: 30000, - context: `streaming all messages for ${account}`, -}).catch((e) => { - logger.error(e, { - context: `Failed to stream all messages for ${account} no longer retrying`, - }); -}); +await streamWithRetry( + () => streamAllMessages(account), + account, + 'all messages' +);This refactoring would also apply to the conversation streaming code above.
components/Chat/Message/message-content-types/message-remote-attachment.tsx (1)
Line range hint
15-17: Address TODO comment or document the rationale.There's an unhandled string content case that returns null. This should either be implemented or documented why it's intentionally skipped.
Would you like me to help implement the string content handling or create an issue to track this?
features/conversation-list/hooks/useMessageText.ts (2)
Line range hint
18-42: Consider refactoring message type handling.The current implementation has multiple type checks and content handling paths. Consider refactoring to use a more maintainable pattern, such as a message type registry or strategy pattern.
Example approach:
type MessageHandler = (message: DecodedMessage<any>) => string; const messageHandlers: Record<string, MessageHandler> = { reply: (message: DecodedMessage<ReplyCodec>) => { const content = message.content(); return typeof content === "string" ? content : content.content.text; }, staticAttachment: () => "Attachment", groupUpdated: () => "conversation updated", default: (message) => { const content = message.content(); return typeof content === "string" ? content : message.fallback; } }; // In the hook: const handler = messageHandlers[getMessageContentType(message.contentTypeId)] || messageHandlers.default; return handler(message);
Line range hint
41-46: Consider adding error type information to logger.The error logging could be enhanced with type information to better track type-related issues during the XMTP library update.
- logger.error("Error getting message text", { - error: e, - contentTypeId: message.contentTypeId, - }); + logger.error("Error getting message text", { + error: e, + errorType: e instanceof Error ? e.constructor.name : typeof e, + contentTypeId: message.contentTypeId, + messageType: getMessageContentType(message.contentTypeId), + });features/conversation/composer/composer.tsx (1)
379-380: Consider using type predicate for better type safetyWhile the type assertion is correct, consider using a type predicate to improve type safety:
-const messageTyped = - replyMessage as DecodedMessage<RemoteAttachmentCodec>; +const messageTyped = replyMessage as DecodedMessage<RemoteAttachmentCodec>; +if (!messageTyped.content()) return null;ios/ConverseNotificationExtension/Spam.swift (3)
Line range hint
15-19: Avoid usingfatalErrorto prevent application crashesUsing
fatalError("should not get here")will crash the application if neither data nor error is received from the network request. It's recommended to handle this case gracefully to prevent unexpected crashes in production.Apply this diff to handle the unexpected case without crashing:
if let data = response.data { continuation.resume(returning: data) return } if let err = response.error { continuation.resume(throwing: err) return } - fatalError("should not get here") + let unknownError = NSError( + domain: "com.yourdomain.app", + code: -1, + userInfo: [NSLocalizedDescriptionKey: "Unknown error occurred"] + ) + continuation.resume(throwing: unknownError)
Line range hint
153-199:senderSpamScoreis not computed and remains zeroIn
computeSpamScoreV3Message, the variablesenderSpamScoreis initialized but never modified within the function, so it always remains zero. This means the sender's spam score isn't contributing to the total spam score as intended. Consider fetching the sender's spam score to include it in the calculation.Apply this diff to fetch and include the sender's spam score:
var senderSpamScore: Double = 0 do { try await client.preferences.syncConsent() let groupDenied = try await client.preferences.conversationState(conversationId: conversation.id) == .denied if groupDenied { // Network consent will override other checks return 1 } let senderInboxId = decodedMessage.senderAddress let senderDenied = try await client.preferences.inboxIdState(inboxId: senderInboxId) == .denied if senderDenied { // Network consent will override other checks return 1 } let senderAllowed = try await client.preferences.inboxIdState(inboxId: senderInboxId) == .allowed if senderAllowed { // Network consent will override other checks return -1 } let convoAllowed = try await client.preferences.conversationState(conversationId: conversation.id) == .allowed if convoAllowed { // Network consent will override other checks return -1 } if case .group(let group) = conversation { if let senderAddresses = try await group.members.first(where: {$0.inboxId == senderInboxId})?.addresses { for address in senderAddresses { if try await client.preferences.addressState(address:address) == .denied { return 1 } } for address in senderAddresses { if try await client.preferences.addressState(address: address) == .allowed { return -1 } } } } else if case .dm(let dm) = conversation { let peer = try dm.peerInboxId if try await client.preferences.inboxIdState(inboxId: peer) == .allowed { return -1 } if try await client.preferences.inboxIdState(inboxId: peer) == .denied { return 1 } } } catch { // } + let senderAddress = decodedMessage.senderAddress + senderSpamScore = await getSenderSpamScore(address: senderAddress, apiURI: apiURI) let contentType = getContentTypeString(type: decodedMessage.encodedContent.type) let messageContent = String(data: decodedMessage.encodedContent.content, encoding: .utf8) let messageSpamScore = getMessageSpamScore(message: messageContent, contentType: contentType) return senderSpamScore + messageSpamScore
Line range hint
22-199: Refactor repeated consent state checks into helper functionsThere are multiple instances where consent states are checked using
client.preferences. This repetition can be reduced by refactoring the consent state checks into reusable helper functions, improving code maintainability and readability.Example refactoring:
// Define helper functions func isConversationAllowed(client: XMTP.Client, conversationId: String) async throws -> Bool { return try await client.preferences.conversationState(conversationId: conversationId) == .allowed } func isConversationDenied(client: XMTP.Client, conversationId: String) async throws -> Bool { return try await client.preferences.conversationState(conversationId: conversationId) == .denied } // Apply in your code let convoAllowed = try await isConversationAllowed(client: client, conversationId: conversation.id) let convoDenied = try await isConversationDenied(client: client, conversationId: conversation.id)ios/ConverseNotificationExtension/Xmtp/Messages.swift (2)
22-23: Optimize by reducing duplicate calls toconversationStateThe code calls
xmtpClient.preferences.conversationState(conversationId:)twice. To improve efficiency, consider calling it once and storing the result.Apply this diff:
// Consent list is loaded in computeSpamScoreGroupWelcome - let convoAllowed = try await xmtpClient.preferences.conversationState(conversationId: conversation.id) == .allowed - let convoDenied = try await xmtpClient.preferences.conversationState(conversationId: conversation.id) == .denied + let convoState = try await xmtpClient.preferences.conversationState(conversationId: conversation.id) + let convoAllowed = convoState == .allowed + let convoDenied = convoState == .denied
Line range hint
49-58: Ensure proper synchronization before message handlingWhen handling decoded messages, ensure that the conversation is properly synchronized before processing to avoid missing any messages.
Consider adding a synchronization step if not already present.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (20)
components/Chat/ChatGroupUpdatedMessage.tsx(1 hunks)components/Chat/Message/message-content-types/message-remote-attachment.tsx(1 hunks)components/Chat/Message/message-content-types/message-reply.tsx(5 hunks)components/Chat/Message/message-content-types/message-simple-text.tsx(1 hunks)components/Chat/Message/message-content-types/message-static-attachment.tsx(1 hunks)components/Chat/Message/message-utils.tsx(2 hunks)features/GroupInvites/joinGroup/JoinGroup.client.ts(0 hunks)features/conversation-list/hooks/useMessageText.ts(1 hunks)features/conversation/composer/composer.tsx(4 hunks)ios/Converse.xcodeproj/project.pbxproj(4 hunks)ios/ConverseNotificationExtension/Profile.swift(1 hunks)ios/ConverseNotificationExtension/Spam.swift(5 hunks)ios/ConverseNotificationExtension/Xmtp/Messages.swift(1 hunks)ios/Podfile(2 hunks)package.json(1 hunks)queries/useV3ConversationListQuery.ts(1 hunks)utils/logout/index.tsx(2 hunks)utils/xmtpRN/client.ts(4 hunks)utils/xmtpRN/contacts.ts(3 hunks)utils/xmtpRN/sync.ts(2 hunks)
💤 Files with no reviewable changes (1)
- features/GroupInvites/joinGroup/JoinGroup.client.ts
🔇 Additional comments (21)
ios/Converse.xcodeproj/project.pbxproj (1)
Line range hint 426-449: New resource bundles added for enhanced functionality.
The addition of crypto and security-related resource bundles (CryptoSwift, SQLCipher, SwiftProtobuf) aligns with the XMTP library update.
Let's verify the XMTP version and its dependencies:
Also applies to: 490-494
✅ Verification successful
Resource bundles are correctly configured for XMTP integration
The addition of crypto-related resource bundles (CryptoSwift, SQLCipher, SwiftProtobuf) in the project configuration is consistent with the XMTP SDK requirements, as evidenced by the extensive XMTP usage throughout the codebase, particularly in the notification extension service. The bundles are properly configured to support:
- XMTP message handling and encryption
- Secure storage with SQLCipher
- Protocol buffer serialization with SwiftProtobuf
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check XMTP version and related dependencies in Podfile.lock
# Search for XMTP and related crypto dependencies versions
rg -A 3 "XMTP|CryptoSwift|SQLCipher|SwiftProtobuf" Podfile.lock
Length of output: 118
Script:
#!/bin/bash
# Let's try to find the Podfile and any XMTP related configurations
fd -t f "Podfile|podfile"
# Also search for XMTP related imports or configurations in source files
rg -i "xmtp|cryptoswift|sqlcipher|swiftprotobuf" --type swift --type objc
Length of output: 10965
utils/xmtpRN/sync.ts (1)
Line range hint 120-146: Verify compatibility with updated XMTP library version.
Since this PR includes an XMTP library update, please verify that:
- The streaming behavior remains consistent with the new version
- Error types and handling align with any changes in the library
- The retry mechanism is still appropriate for any new failure modes
✅ Verification successful
Based on the search results, I can now generate the final response:
The streaming implementation is compatible with XMTP v11.5.0
The code correctly uses the XMTP client's streaming APIs:
streamAllMessagesandstreammethods are properly implemented with error handling and retries- The streaming behavior aligns with the XMTP v11.5.0 client interface
- Error handling through
retryWithBackoffis appropriate for network-related failures
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for XMTP version changes and related streaming API usage
rg -A 3 "@xmtp/xmtp-js" package.json
rg -A 5 "streamConversations|streamAllMessages" --type typescript
Length of output: 266
Script:
#!/bin/bash
# Let's try searching without file type restriction and check implementation details
rg -A 5 'streamConversations|streamAllMessages'
# Also check for any XMTP client initialization and stream usage patterns
rg -A 5 'new Client|\.stream'
# Look for any XMTP-related type definitions or interfaces
ast-grep --pattern 'interface $_ {
$$$
stream$_($$$) {
$$$
}
$$$
}'
Length of output: 4417
Script:
#!/bin/bash
# Let's check the XMTP client type and its usage
rg -A 5 "ConverseXmtpClientType"
# Check for any XMTP-related imports and their versions
rg "@xmtp/.*" --type typescript
# Look for any changes in error handling patterns
ast-grep --pattern 'catch($_) {
$$$
}'
Length of output: 32413
components/Chat/Message/message-content-types/message-remote-attachment.tsx (1)
7-7: LGTM! Type simplification looks good.
The change from tuple type to direct type reference aligns with the broader type improvements across the codebase.
components/Chat/Message/message-content-types/message-simple-text.tsx (1)
13-13: LGTM! Type simplification is consistent.
The type change from DecodedMessage<[TextCodec]> to DecodedMessage<TextCodec> aligns with the broader type improvements and maintains consistency across components.
features/conversation-list/hooks/useMessageText.ts (1)
19-19: LGTM! Type assertion simplified correctly.
The type assertion change from DecodedMessage<[ReplyCodec]> to DecodedMessage<ReplyCodec> aligns with the broader type improvements.
components/Chat/Message/message-content-types/message-static-attachment.tsx (1)
21-21: Type signature update looks good!
The change from DecodedMessage<[StaticAttachmentCodec]> to DecodedMessage<StaticAttachmentCodec> aligns with the broader type refinement effort and correctly represents that this message type uses a single codec rather than a tuple of codecs.
Let's verify consistent type usage across the codebase:
✅ Verification successful
Type signature update is consistent across the codebase
The verification confirms that there are no remaining instances of tuple-style [StaticAttachmentCodec] in the codebase. All usages of StaticAttachmentCodec follow the correct non-tuple type pattern, including in related files like the composer, client, and message utilities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining tuple-style codec types that might need updating
rg -l '\[StaticAttachmentCodec\]' --type typescript
Length of output: 88
Script:
#!/bin/bash
# Search for any remaining tuple-style codec types that might need updating
# Using ripgrep without file type filter and checking for StaticAttachmentCodec usage
rg '\[StaticAttachmentCodec\]'
# Also search for general StaticAttachmentCodec usage for context
rg 'StaticAttachmentCodec'
Length of output: 873
components/Chat/Message/message-utils.tsx (2)
8-11: Import changes enhance type safety
Good addition of specific type imports that support the broader type refinement effort.
32-76: Type predicate functions properly typed
The update to use DecodedMessageWithCodecsType instead of any improves type safety across all message type checks. The special handling of ReplyCodec and TransactionReferenceCodec with SupportedCodecsType correctly accounts for nested message content.
Let's verify that all message type checks are consistently updated:
✅ Verification successful
Type predicate functions consistently use DecodedMessageWithCodecsType
The verification confirms that:
- No instances of
message: anyremain in the codebase DecodedMessageWithCodecsTypeis consistently used across all message-related functions- The type is properly used in message utilities, hooks, and data handling components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining 'any' types in message type checks
rg "message:\s*any" --type typescript
# Verify consistent usage of DecodedMessageWithCodecsType
rg "message:\s*DecodedMessageWithCodecsType" --type typescript
Length of output: 174
Script:
#!/bin/bash
# Check for any remaining 'any' types in message type checks
rg "message:\s*any"
# Verify consistent usage of DecodedMessageWithCodecsType
rg "message:\s*DecodedMessageWithCodecsType"
# Look for other message type predicates that might need updating
ast-grep --pattern 'function is$_Message(message: $_) {
$$$
}'
Length of output: 1628
utils/xmtpRN/client.ts (3)
44-54: Type definition improvement: More precise codec type definition
The explicit tuple type definition for SupportedCodecsType provides better type safety and clearer documentation of supported codecs compared to the previous inferred type.
76-78: Type enhancement: More accurate async type definition
The new type definition using Awaited<ReturnType> better represents the asynchronous nature of the messages method and ensures type safety when working with decoded messages.
166-167: Verify the impact of parameter type change
The change from inboxId to installationId suggests a significant change in how clients are identified and managed.
✅ Verification successful
Parameter type change from inboxId to installationId is a safe refactor
The verification shows that the change is consistent with the codebase's usage:
- The only caller of
dropXmtpClientinutils/logout/index.tsxalready usesgetInboxId()and casts it toInstallationId - The codebase shows a clear separation between
inboxIdusage (for user identification in conversations/contacts) and installation management - No breaking changes are introduced as the type conversion is handled at the call site
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining uses of inboxId with dropXmtpClient
rg "dropXmtpClient.*inboxId" -A 2
# Check for any documentation or migration guides
fd -t f "MIGRATION|UPGRADE|README" -x cat {} \; | rg -i "xmtp.*installation"
Length of output: 117
Script:
#!/bin/bash
# Let's check for all usages of dropXmtpClient to understand the change impact
rg "dropXmtpClient" -A 2
# Check the Client class implementation to understand the parameter type history
ast-grep --pattern 'class Client {
$$$
dropClient($_) {
$$$
}
$$$
}'
# Look for any references to inboxId in the same directory
rg "inboxId" utils/xmtpRN/
Length of output: 2207
components/Chat/ChatGroupUpdatedMessage.tsx (1)
23-23: Type simplification: Improved type clarity
The change from DecodedMessage<[GroupUpdatedCodec]> to DecodedMessage<GroupUpdatedCodec> removes unnecessary tuple wrapping, making the type definition more straightforward while maintaining type safety.
utils/xmtpRN/contacts.ts (2)
128-128: Consistent consent state updates across functions
The same property and value changes have been consistently applied across all consent-related functions (inboxIds and groups), maintaining uniformity in the API.
Also applies to: 136-136, 188-188, 196-196
66-66: API alignment: Updated consent state property names
The change from permissionType to state with values allowed/denied aligns with standard permission terminology and appears to be part of the XMTP library update.
Also applies to: 74-74
✅ Verification successful
Property rename from permissionType to state is consistently applied
The verification confirms that:
- No remaining instances of the old
permissionTypeproperty exist in the codebase - The new
stateproperty with valuesallowed/deniedis consistently used across both XMTP contacts and group invite features - The change appears to be fully implemented without any mixed usage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining uses of old permission types
rg "permissionType.*:.*['\"](?:allow|deny)['\"]"
# Check for any documentation about the consent state changes
fd -t f "CHANGELOG|README|UPGRADE" -x cat {} \; | rg -i "consent.*state"
Length of output: 132
Script:
#!/bin/bash
# Search for any remaining instances of permissionType in the codebase
rg "permissionType"
# Search for state with allowed/denied values
rg "state['\"]?\s*:\s*['\"](?:allowed|denied)['\"]"
# Look for any XMTP-related configuration or type definitions
fd -t f "xmtp|XMTP" -x cat {} \; | rg -i "state|permission"
Length of output: 977
queries/useV3ConversationListQuery.ts (1)
17-21: 🛠️ Refactor suggestion
Verify the impact of optional account parameter
The use of optional chaining (?.) on the account parameter could lead to undefined keys in the query cache. This might cause issues with cache management and query invalidation.
Consider adding a guard clause:
export const conversationListKey = (account: string) => [
QueryKeys.V3_CONVERSATION_LIST,
- account?.toLowerCase(),
+ account ? account.toLowerCase() : undefined,
];utils/logout/index.tsx (2)
24-24: LGTM: Type import aligns with XMTP SDK update
The import of InstallationId from the XMTP SDK's build path is correct and matches the library's type definitions.
162-162: Verify type assertion safety
The type assertion to InstallationId is necessary for the XMTP SDK update, but we should ensure that getInboxId always returns a valid InstallationId.
✅ Verification successful
Type assertion is safe and well-implemented
The type assertion to InstallationId is safe because:
getInboxIdis a wrapper aroundClient.getOrCreateInboxIdwhich is part of the XMTP SDK- The environment is properly typed and validated as
"dev" | "production" | "local" - The function is consistently used across the codebase for XMTP client operations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check implementation of getInboxId
ast-grep --pattern 'function getInboxId($_) {
$$$
}'
Length of output: 56
Script:
#!/bin/bash
# Let's try searching for getInboxId with ripgrep to find its implementation
rg "getInboxId" -A 5
Length of output: 3572
Script:
#!/bin/bash
# Let's check the Client.getOrCreateInboxId implementation and env usage
rg "Client\.getOrCreateInboxId" -A 3
rg "env =" -A 3
Length of output: 5106
components/Chat/Message/message-content-types/message-reply.tsx (1)
27-27: LGTM: Type definitions properly aligned with XMTP SDK
The changes to use direct codec types instead of array types (DecodedMessage<ReplyCodec> instead of DecodedMessage<[ReplyCodec]>) align with the XMTP SDK update and improve type safety.
Also applies to: 166-166, 216-216, 232-237, 250-252
✅ Verification successful
Based on the search results, I can now generate the final response as we have sufficient evidence about the type usage across the codebase.
Type changes are correctly aligned with XMTP SDK
The changes from array codec types to direct codec types (e.g., DecodedMessage<ReplyCodec> instead of DecodedMessage<[ReplyCodec]>) are consistent with the XMTP SDK's type system. The search results confirm that:
- All other message type usages in the codebase follow the same pattern of using direct codec types
- No instances of array codec types (
DecodedMessage<[...Codec]>) were found - The type is used consistently across message handling components and utilities
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining array codec types that need updating
rg -A 2 "DecodedMessage<\[.*Codec\]>"
# Check for consistent usage of the new type
ast-grep --pattern 'DecodedMessage<$_Codec>'
Length of output: 86
Script:
#!/bin/bash
# Search for DecodedMessage type usage in the codebase
rg -A 2 "DecodedMessage"
# Search for specific codec type usage
rg -A 2 "ReplyCodec"
# Find all TypeScript files that might contain message type definitions
fd -e ts -e tsx --exec grep -l "Codec"
Length of output: 30003
package.json (1)
86-86: Verify compatibility with other XMTP dependencies
The update to @xmtp/react-native-sdk@^3.1.1 looks good, but let's verify compatibility with other XMTP-related dependencies.
✅ Verification successful
Let me verify the compatibility between these XMTP packages by checking their peer dependencies and version requirements.
XMTP dependencies are compatible
The verification shows that all XMTP dependencies are compatible:
@xmtp/[email protected]only requires peer dependencies for React ecosystem@xmtp/frames-clientrequires@xmtp/xmtp-js >9.3.1, which is satisfied by the installed version 11.5.0- Other XMTP packages don't have any peer dependency requirements
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check compatibility between XMTP dependencies
# Expected: No conflicting peer dependencies
# Get package details from npm
echo "Checking @xmtp/[email protected] peer dependencies..."
curl -s "https://registry.npmjs.org/@xmtp/react-native-sdk/3.1.1" | jq -r '.peerDependencies'
# List all installed XMTP packages and their versions
echo -e "\nInstalled XMTP packages:"
cat package.json | jq -r '.dependencies | with_entries(select(.key | startswith("@xmtp")))'
Length of output: 805
Script:
#!/bin/bash
# Check peer dependencies of other XMTP packages
echo "Checking @xmtp/[email protected] peer dependencies..."
curl -s "https://registry.npmjs.org/@xmtp/xmtp-js/11.5.0" | jq -r '.peerDependencies'
echo -e "\nChecking @xmtp/[email protected] peer dependencies..."
curl -s "https://registry.npmjs.org/@xmtp/frames-client/0.5.4" | jq -r '.peerDependencies'
echo -e "\nChecking content type packages peer dependencies..."
for pkg in [email protected] [email protected] [email protected] [email protected] [email protected]; do
echo -e "\n@xmtp/$pkg:"
curl -s "https://registry.npmjs.org/@xmtp/$pkg" | jq -r '.peerDependencies'
done
Length of output: 2450
features/conversation/composer/composer.tsx (2)
353-353: Type assertion simplified correctly
The type assertion has been correctly updated from array type to direct type, aligning with the new XMTP SDK type system.
419-420: Type assertions in ReplyPreviewMessageContent updated correctly
The type assertions for both StaticAttachmentCodec and ReplyCodec have been correctly updated to use direct types instead of array types, consistent with the new SDK's type system.
Also applies to: 457-457
| let formattedAddress = address.lowercased() | ||
| if let profile = profileState?.profiles?[address] ?? profileState?.profiles?[formattedAddress] { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add address format validation
While removing the web3 dependency simplifies the code, we should still validate the Ethereum address format to prevent invalid addresses from being processed.
Consider adding a simple regex validation:
+func isValidEthereumAddress(_ address: String) -> Bool {
+ let pattern = "^0x[0-9a-fA-F]{40}$"
+ return address.range(of: pattern, options: .regularExpression) != nil
+}
func getProfile(account: String, address: String) async -> Profile? {
var profileState = getProfilesStore(account: account)?.state
+ guard isValidEthereumAddress(address) else { return nil }
let formattedAddress = address.lowercased()Committable suggestion skipped: line range outside the PR's diff.
| @@ -18,7 +18,7 @@ install! 'cocoapods', | |||
|
|
|||
| # Version must match version from XMTP Podspec (matching @xmtp/react-native-sdk from package.json) | |||
| # https://github.com/xmtp/xmtp-react-native/blob/v2.6.2/ios/XMTPReactNative.podspec#L29 | |||
| $xmtpVersion = '3.0.6' | |||
| $xmtpVersion = '3.0.14' | |||
There was a problem hiding this comment.
💡 Codebase verification
Version mismatch detected between package.json and Podfile
The XMTP versions are not aligned:
- package.json: @xmtp/react-native-sdk: ^3.1.1
- Podfile: XMTP pod version: 3.0.14
This version mismatch could lead to compatibility issues. The Podfile version should be updated to match the SDK version being used in package.json.
🔗 Analysis chain
Verify XMTP version compatibility
The XMTP pod version has been updated to '3.0.14'. Please ensure:
- This version is compatible with the @xmtp/react-native-sdk version in package.json
- Review the release notes for any breaking changes or security fixes
Let's verify the version compatibility:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check package.json for @xmtp/react-native-sdk version
jq -r '.dependencies["@xmtp/react-native-sdk"]' package.json
# Check GitHub releases for breaking changes
gh api repos/xmtp/xmtp-react-native/releases | jq -r '.[].body' | grep -A 5 "3.0.14"
Length of output: 154
Script:
#!/bin/bash
# Let's check the actual content of package.json to understand the dependency version better
cat package.json | jq '.dependencies'
# Let's also check if there are any direct references to XMTP versions in the codebase
rg "xmtp" -i --type=json --type=podfile -A 2 -B 2
Length of output: 6723
Script:
#!/bin/bash
# Let's check the Podfile content directly
cat ios/Podfile
# Let's also try to find any XMTP version references in the repository
rg "xmtp.*version" -i
Length of output: 3989
| let senderSpamScore = await getSenderSpamScore(address: firstAddress, apiURI: apiURI) | ||
| return senderSpamScore | ||
| } |
There was a problem hiding this comment.
Handle potential nil value in firstAddress
When accessing inviterAddresses.first, there is a possibility that it could be nil. Ensure to handle this case to prevent unexpected crashes.
Apply this diff to safely unwrap firstAddress:
if let firstAddress = inviterAddresses.first {
let senderSpamScore = await getSenderSpamScore(address: firstAddress, apiURI: apiURI)
return senderSpamScore
+ } else {
+ // Handle the case where inviterAddresses is empty
+ return 0
}Committable suggestion skipped: line range outside the PR's diff.
Performance Comparison ReportSignificant Changes To DurationThere are no entries Meaningless Changes To DurationShow entries
Show details
Render Count ChangesThere are no entries Render IssuesThere are no entries Added ScenariosThere are no entries Removed ScenariosThere are no entries |
Bumped XMTP Fixed types Removed web3 usage in Swift
alexrisch
force-pushed
the
ar/bump-xmtp
branch
from
2bfd56b to
176d90b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
ios/Converse.xcodeproj/project.pbxproj (1)
490-493: Consider security implications of SQLCipher bundle.The addition of SQLCipher bundle suggests encrypted database usage. This is a good security practice, especially for a messaging app.
components/PinnedConversations/PinnedV3DMConversation.tsx (2)
138-140: Consider extracting shared message preview logicThe message preview logic is duplicated between PinnedV3GroupConversation and PinnedV3DMConversation. Additionally, it has the same type-safety concerns regarding null assertions.
Consider creating a shared hook:
// hooks/useMessagePreview.ts export function useMessagePreview(message: unknown, isUnread: boolean) { return useMemo(() => ({ shouldDisplay: message != null && isTextMessage(message) && isUnread, message: message != null && isTextMessage(message) ? message : null, }), [message, isUnread]); }Then use it in both components:
- const displayMessagePreview = - conversation.lastMessage && - isTextMessage(conversation.lastMessage) && - isUnread; + const { shouldDisplay, message: previewMessage } = useMessagePreview( + conversation.lastMessage, + isUnread + ); // Later in JSX - {displayMessagePreview && ( - <PinnedMessagePreview message={conversation.lastMessage!} /> - )} + {shouldDisplay && previewMessage && ( + <PinnedMessagePreview message={previewMessage} /> + )}This refactor would:
- Eliminate code duplication
- Improve type safety by removing non-null assertions
- Make the preview logic more maintainable and testable
Line range hint
1-168: Consider implementing component composition for pinned conversationsBoth PinnedV3GroupConversation and PinnedV3DMConversation share significant logic and structure. This duplication increases maintenance overhead and the risk of inconsistencies.
Consider implementing a composition-based approach:
- Create a base PinnedConversationContainer component that handles common logic (context menu, navigation, preview rendering)
- Create separate GroupConversationContent and DMConversationContent components for specific rendering logic
- Use composition to combine them
Would you like me to provide a detailed implementation example?
components/Chat/Message/message-utils.tsx (1)
52-55: Consider simplifying the isReplyMessage implementationThe intermediate variable assignment is unnecessary since the function only returns the result of the comparison.
export function isReplyMessage( message: DecodedMessageWithCodecsType ): message is DecodedMessage<ReplyCodec, SupportedCodecsType> { - const isReply = getMessageContentType(message.contentTypeId) === "reply"; - return isReply; + return getMessageContentType(message.contentTypeId) === "reply"; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (22)
components/Chat/ChatGroupUpdatedMessage.tsx(1 hunks)components/Chat/Message/message-content-types/message-remote-attachment.tsx(1 hunks)components/Chat/Message/message-content-types/message-reply.tsx(5 hunks)components/Chat/Message/message-content-types/message-simple-text.tsx(1 hunks)components/Chat/Message/message-content-types/message-static-attachment.tsx(1 hunks)components/Chat/Message/message-utils.tsx(2 hunks)components/PinnedConversations/PinnedV3DMConversation.tsx(1 hunks)components/PinnedConversations/PinnedV3GroupConversation.tsx(1 hunks)features/GroupInvites/joinGroup/JoinGroup.client.ts(0 hunks)features/conversation-list/hooks/useMessageText.ts(1 hunks)features/conversation/composer/composer.tsx(4 hunks)ios/Converse.xcodeproj/project.pbxproj(4 hunks)ios/ConverseNotificationExtension/Profile.swift(1 hunks)ios/ConverseNotificationExtension/Spam.swift(5 hunks)ios/ConverseNotificationExtension/Xmtp/Messages.swift(1 hunks)ios/Podfile(2 hunks)package.json(1 hunks)queries/useV3ConversationListQuery.ts(1 hunks)utils/logout/index.tsx(2 hunks)utils/xmtpRN/client.ts(4 hunks)utils/xmtpRN/contacts.ts(3 hunks)utils/xmtpRN/sync.ts(2 hunks)
💤 Files with no reviewable changes (1)
- features/GroupInvites/joinGroup/JoinGroup.client.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- ios/ConverseNotificationExtension/Xmtp/Messages.swift
- utils/xmtpRN/contacts.ts
- features/conversation-list/hooks/useMessageText.ts
- components/Chat/Message/message-content-types/message-simple-text.tsx
- ios/ConverseNotificationExtension/Profile.swift
- components/Chat/ChatGroupUpdatedMessage.tsx
- components/Chat/Message/message-content-types/message-remote-attachment.tsx
- queries/useV3ConversationListQuery.ts
- package.json
- components/Chat/Message/message-content-types/message-static-attachment.tsx
- utils/logout/index.tsx
- utils/xmtpRN/sync.ts
- ios/Podfile
- features/conversation/composer/composer.tsx
- components/Chat/Message/message-content-types/message-reply.tsx
- ios/ConverseNotificationExtension/Spam.swift
🔇 Additional comments (8)
ios/Converse.xcodeproj/project.pbxproj (2)
Line range hint 426-449: LGTM: Required resource bundles are properly configured.
The addition of essential bundles like CryptoSwift, SQLCipher, and SwiftProtobuf aligns with the XMTP library requirements and security best practices.
Also applies to: 490-493
Line range hint 426-437: Verify bundle versions compatibility.
The addition of new resource bundles should be checked for version compatibility with the updated XMTP library.
Also applies to: 490-493
✅ Verification successful
Let me try to verify this concern using a different approach by checking the Podfile and related iOS configuration files.
Bundle versions are properly configured and compatible
Based on the Podfile analysis, the XMTP version is explicitly pinned to '3.0.14' and all the related dependencies (CryptoSwift, SQLCipher, SwiftProtobuf) are properly configured in both the main target and notification extension target. The resource bundles are correctly set up in the project.pbxproj with appropriate build configurations and code signing settings. The post-install hooks ensure proper version compatibility and build settings across all pods.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check Podfile.lock for bundle versions
# This helps ensure all added bundles are compatible with the new XMTP version
if [ -f "Podfile.lock" ]; then
echo "Checking bundle versions in Podfile.lock..."
grep -A 5 "SPEC CHECKSUMS:" Podfile.lock | grep -E "CryptoSwift|SQLCipher|SwiftProtobuf|XMTP"
else
echo "Podfile.lock not found"
fi
Length of output: 80
Script:
#!/bin/bash
# Check Podfile and related configuration files
echo "=== Checking Podfile ==="
if [ -f "ios/Podfile" ]; then
cat ios/Podfile
fi
echo -e "\n=== Checking project.pbxproj for XMTP related configurations ==="
if [ -f "ios/Converse.xcodeproj/project.pbxproj" ]; then
grep -A 5 "XMTP" ios/Converse.xcodeproj/project.pbxproj
fi
echo -e "\n=== Listing all .bundle references in project.pbxproj ==="
if [ -f "ios/Converse.xcodeproj/project.pbxproj" ]; then
grep -B 2 -A 2 "\.bundle" ios/Converse.xcodeproj/project.pbxproj
fi
Length of output: 9547
components/Chat/Message/message-utils.tsx (2)
8-11: LGTM: Import changes align with type safety improvements
The addition of DecodedMessageWithCodecsType and SupportedCodecsType imports enhances type safety by replacing any previous usage of any types.
32-33: LGTM: Function signatures consistently use typed parameters
The update of all message type-checking functions to use DecodedMessageWithCodecsType instead of any improves type safety and maintains consistency across the codebase.
Also applies to: 37-38, 42-43, 47-48, 52-53, 58-59, 63-64, 68-69, 75-76
utils/xmtpRN/client.ts (4)
20-20: LGTM: Updated imports align with XMTP SDK changes
The addition of DecodedMessageUnion and InstallationId types from the SDK improves type safety and reflects the library's evolution.
Also applies to: 28-28
44-54: LGTM: Enhanced type safety with explicit codec tuple
The change from a generic type to an explicit tuple type for SupportedCodecsType provides better type checking and autocompletion support.
76-78: LGTM: Improved type definition using Awaited
Using Awaited with the return type of messages() method ensures proper typing for asynchronous operations.
166-167: Verify the impact of InstallationId parameter change
The parameter type change from InboxId to InstallationId in dropXmtpClient might affect existing code that calls this function.
| const displayMessagePreview = | ||
| group.lastMessage && isTextMessage(group.lastMessage) && isUnread; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider handling undefined messages more gracefully
While the null check for group.lastMessage improves safety, there's still a non-null assertion (!) being used when passing the message to PinnedMessagePreview. This could be problematic if the message becomes null between the check and render.
Consider this safer approach:
- const displayMessagePreview =
- group.lastMessage && isTextMessage(group.lastMessage) && isUnread;
+ const lastMessage = group.lastMessage;
+ const displayMessagePreview =
+ lastMessage != null && isTextMessage(lastMessage) && isUnread;
// Later in JSX
- {displayMessagePreview && <PinnedMessagePreview message={group.lastMessage!} />}
+ {displayMessagePreview && lastMessage && (
+ <PinnedMessagePreview message={lastMessage} />
+ )}Committable suggestion skipped: line range outside the PR's diff.
* chore: Bump Xmtp Bumped XMTP Fixed types Removed web3 usage in Swift * rebase and fixed types
Bumped XMTP
Fixed types
Removed web3 usage in Swift
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
package.jsonandPodfilefor improved performance and stability.