V3 HMAC Support #49
V3 HMAC Support #49
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the WalkthroughThis pull request encompasses modifications across multiple files in a project, focusing on dependency updates, test case revisions, and subtle configuration changes. The changes primarily involve updating the XMTP Node SDK, adjusting Docker composition commands, modifying integration tests, and refining API server logging. The modifications suggest an evolution of the project's communication and integration infrastructure, with a particular emphasis on group messaging and SDK compatibility. Changes
Sequence DiagramsequenceDiagram
participant Client
participant XMTPNodeSDK
participant APIServer
Client->>XMTPNodeSDK: Create Client
XMTPNodeSDK-->>Client: Client Instance
Client->>APIServer: Subscribe with Metadata
APIServer->>XMTPNodeSDK: Process Subscriptions
XMTPNodeSDK-->>APIServer: Subscription Confirmation
APIServer-->>Client: Subscription Response
Poem
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 (
|
rygine
force-pushed
the
coda/test-hmac
branch
3 times, most recently
from
26e3fd5 to
5fdc3bd
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
integration/src/index.ts (2)
63-63: Use a constant or enum forconsentStatecomparisonComparing
consentStatedirectly to the numeric value1can reduce code readability. Consider defining a constant or enum to represent the consent state.Apply this diff to improve readability:
+const CONSENT_STATE_ALLOWED = 1; ... const consentedConversations = (await xmtpClient.conversations.list()).filter( - (c) => c.consentState === 1 + (c) => c.consentState === CONSENT_STATE_ALLOWED );
92-92: Remove unused variableinviteAndIntroSubscriptionsThe
inviteAndIntroSubscriptionsarray is initialized but not used. Consider removing it to clean up the code.Apply this diff to remove the unused variable:
- const inviteAndIntroSubscriptions: Subscription[] = [];integration/src/index.test.ts (2)
116-117: Clarify message delivery expectations in the testThe test sends messages with comments about delivery expectations. Ensure that the messages correspond with the intended delivery logic, and adjust the comments or logic if needed.
2-2: Remove unnecessary import ofexpectif globally availableIf
expectis globally available in your testing environment, consider removing the explicit import to clean up the code.Apply this diff to remove the redundant import:
-import { expect, test, afterAll, describe } from "bun:test"; +import { test, afterAll, describe } from "bun:test";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (45)
integration/bun.lockbis excluded by!**/bun.lockbintegration/gen/notifications/v1/service_pb.d.tsis excluded by!**/gen/**integration/gen/notifications/v1/service_pb.jsis excluded by!**/gen/**pkg/proto/identity/api/v1/identity.pb.gois excluded by!**/*.pb.gopkg/proto/identity/api/v1/identity_grpc.pb.gois excluded by!**/*.pb.gopkg/proto/identity/associations/association.pb.gois excluded by!**/*.pb.gopkg/proto/identity/associations/signature.pb.gois excluded by!**/*.pb.gopkg/proto/identity/credential.pb.gois excluded by!**/*.pb.gopkg/proto/keystore_api/v1/keystore.pb.gois excluded by!**/*.pb.gopkg/proto/message_api/v1/authn.pb.gois excluded by!**/*.pb.gopkg/proto/message_api/v1/message_api.pb.gois excluded by!**/*.pb.gopkg/proto/message_contents/ciphertext.pb.gois excluded by!**/*.pb.gopkg/proto/message_contents/composite.pb.gois excluded by!**/*.pb.gopkg/proto/message_contents/contact.pb.gois excluded by!**/*.pb.gopkg/proto/message_contents/content.pb.gois excluded by!**/*.pb.gopkg/proto/message_contents/conversation_reference.pb.gois excluded by!**/*.pb.gopkg/proto/message_contents/ecies.pb.gois excluded by!**/*.pb.gopkg/proto/message_contents/frames.pb.gois excluded by!**/*.pb.gopkg/proto/message_contents/invitation.pb.gois excluded by!**/*.pb.gopkg/proto/message_contents/message.pb.gois excluded by!**/*.pb.gopkg/proto/message_contents/private_key.pb.gois excluded by!**/*.pb.gopkg/proto/message_contents/private_preferences.pb.gois excluded by!**/*.pb.gopkg/proto/message_contents/public_key.pb.gois excluded by!**/*.pb.gopkg/proto/message_contents/signature.pb.gois excluded by!**/*.pb.gopkg/proto/message_contents/signed_payload.pb.gois excluded by!**/*.pb.gopkg/proto/mls/api/v1/mls.pb.gois excluded by!**/*.pb.gopkg/proto/mls/database/intents.pb.gois excluded by!**/*.pb.gopkg/proto/mls/message_contents/association.pb.gois excluded by!**/*.pb.gopkg/proto/mls/message_contents/content.pb.gois excluded by!**/*.pb.gopkg/proto/mls/message_contents/content_types/reaction.pb.gois excluded by!**/*.pb.gopkg/proto/mls/message_contents/credential.pb.gois excluded by!**/*.pb.gopkg/proto/mls/message_contents/group_membership.pb.gois excluded by!**/*.pb.gopkg/proto/mls/message_contents/group_metadata.pb.gois excluded by!**/*.pb.gopkg/proto/mls/message_contents/group_mutable_metadata.pb.gois excluded by!**/*.pb.gopkg/proto/mls/message_contents/group_permissions.pb.gois excluded by!**/*.pb.gopkg/proto/mls/message_contents/transcript_messages.pb.gois excluded by!**/*.pb.gopkg/proto/mls_validation/v1/service.pb.gois excluded by!**/*.pb.gopkg/proto/mls_validation/v1/service_grpc.pb.gois excluded by!**/*.pb.gopkg/proto/xmtpv4/envelopes/envelopes.pb.gois excluded by!**/*.pb.gopkg/proto/xmtpv4/message_api/message_api.pb.gois excluded by!**/*.pb.gopkg/proto/xmtpv4/message_api/message_api_grpc.pb.gois excluded by!**/*.pb.gopkg/proto/xmtpv4/message_api/misbehavior_api.pb.gois excluded by!**/*.pb.gopkg/proto/xmtpv4/message_api/misbehavior_api_grpc.pb.gois excluded by!**/*.pb.gopkg/proto/xmtpv4/payer_api/payer_api.pb.gois excluded by!**/*.pb.gopkg/proto/xmtpv4/payer_api/payer_api_grpc.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (6)
.gitignore(2 hunks)dev/integration(1 hunks)integration/package.json(1 hunks)integration/src/index.test.ts(4 hunks)integration/src/index.ts(4 hunks)pkg/api/api.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- dev/integration
🔇 Additional comments (13)
integration/src/index.ts (4)
24-24: Ensure all callers handle the new asynchronousrandomClientfunctionThe
randomClientfunction has been changed toasync. Please verify that all places whererandomClient()is called are updated to handle the returned Promise appropriately.
67-67: Verify the updated HMAC key retrieval methodThe method for retrieving HMAC keys has changed to
xmtpClient.conversations.hmacKeys(). Ensure this method returns the expected data structure compatible with the rest of the code.
35-39: Confirm parameters forClient.createThe parameters passed to
Client.createhave been updated. Verify that thesigner,encKey, and configuration options align with the new SDK requirements.
13-13: Check compatibility ofgetRandomValuesin your environmentUsing
getRandomValuesfrom"node:crypto"requires Node.js 19 or newer. Ensure that your deployment environment supports this or consider a polyfill for earlier versions.integration/src/index.test.ts (4)
39-39: VerifyinstallationIdusage withalix.accountAddressIn the
registerInstallationcall,installationIdis set toalix.accountAddress. Confirm thataccountAddressis the intended value forinstallationIdin this context.
86-86: Handle the result ofawait bo.conversations.newGroupEnsure that
boGroupreceives the correct conversation object and that any asynchronous operations are properly handled.
95-97: Validate the expected number of HMAC keysYou expect
conversationHmacKeys.lengthto be3. Verify that this aligns with the actual HMAC key generation logic, and adjust the test expectation if necessary.
88-91:⚠️ Potential issueAwait synchronization before listing conversations
There might be a timing issue when syncing conversations. Make sure to await
alix.conversations.syncAll()before listing conversations to ensure synchronization is complete.Apply this diff to correctly await synchronization:
expect((await alix.conversations.list()).length).toEqual(0); - await alix.conversations.syncAll(); + await alix.conversations.syncAll(); const alixGroups = await alix.conversations.list();Likely invalid or redundant comment.
pkg/api/api.go (1)
145-146: Improved logging forSubscribeWithMetadataThe updated logging message
"Subscribing"provides clearer insight into the operation being performed, enhancing debug ability.integration/package.json (2)
14-14: Verify stability of @xmtp/[email protected]The version number (0.0.x) suggests this is a pre-release or beta version. Please ensure:
- This version is stable enough for production use
- There are no critical bugs or breaking changes
15-15: Verify compatibility with viem@^2.22.2The significant minor version jump (2.7 → 2.22) might introduce new features or deprecations.
✅ Verification successful
Version upgrade to viem@^2.22.2 appears safe
The codebase uses only stable, core wallet functionality from viem (wallet creation, signing, and chain operations). No breaking changes were found between v2.7 and v2.22.2 that would affect these basic operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check changelog for breaking changes curl -s https://raw.githubusercontent.com/wagmi-dev/viem/main/CHANGELOG.md | grep -A 10 "2.22.2" # Check for compatibility issues gh api graphql -f query=' { repository(owner: "wagmi-dev", name: "viem") { issues(first: 5, states: OPEN, labels: ["breaking-change"], orderBy: {field: CREATED_AT, direction: DESC}) { nodes { title url } } } }'Length of output: 397
Script:
#!/bin/bash # Try fetching release notes for relevant versions gh api repos/wagmi-dev/viem/releases --paginate | jq -r '.[] | select(.tag_name | startswith("v2.")) | select(.tag_name >= "v2.7.0" and .tag_name <= "v2.22.2") | "Version: \(.tag_name)\n\(.body)\n---"' # Search for viem usage patterns rg -l "from ['\"](viem|@viem)" --type ts --type js # Look for specific viem imports ast-grep --pattern 'import { $_ } from "viem"'Length of output: 332
Script:
#!/bin/bash # Check specific viem usage in the file cat integration/src/index.ts # Look for specific viem function calls ast-grep --pattern 'import { $WHAT } from "viem"' integration/src/index.tsLength of output: 3384
.gitignore (2)
7-7: Verify if mise.toml should be version controlledDevelopment environment configuration files are typically version controlled to ensure consistent development environments across the team.
Please confirm if ignoring
mise.tomlaligns with the team's development environment management strategy.
25-25: LGTM: Consistent with other Swift ignoresThe addition of
swift/mls_validationfollows the established pattern of ignoring generated Swift directories.
Summary by CodeRabbit
Release Notes
Dependency Updates
viemlibrary version@xmtp/xmtp-jswith@xmtp/node-sdkInfrastructure Changes
.gitignoreto excludemise.tomlandswift/mls_validationdocker-composetodocker composeTesting Improvements
Backend Enhancements