Conversation
🦋 Changeset detectedLatest commit: 8002443 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughCredential salt is added to the shared credential contract, database schema, address derivation, server responses, KYC and activity flows, client account initialization, tests, cache invalidation, and package Changesets. ChangesCredential salt flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthAPI
participant CredentialsDB
participant deriveAddress
Client->>AuthAPI: Authenticate credential
AuthAPI->>CredentialsDB: Read credential and salt
AuthAPI->>deriveAddress: Derive account with factory, x, y, and salt
deriveAddress-->>AuthAPI: Return derived account address
AuthAPI-->>Client: Authentication response including salt
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cruzdanilo
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a credential salt for address derivation and account initialization across the common, server, and mobile packages. It adds a salt column with a hex check constraint to the database schema, updates API endpoints (authentication, registration, and passkey) to retrieve and return the salt, and integrates it into the account client creation process. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7163f50a-fc04-4626-aa68-304a73dc98f3
📒 Files selected for processing (14)
.changeset/fuzzy-cows-breathe.md.changeset/neat-eggs-turn.md.changeset/quick-koalas-thread.mdcommon/accountInit.tscommon/deriveAddress.tscommon/validation.tsserver/api/auth/authentication.tsserver/api/auth/registration.tsserver/api/passkey.tsserver/database/schema.tsserver/test/api/auth.test.tsserver/utils/createCredential.tssrc/utils/accountClient.tssrc/utils/server.ts
|
close #1143 |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0a3fdbee-d558-4037-889e-3f4d6bcfe5b1
📒 Files selected for processing (14)
.changeset/fuzzy-cows-breathe.md.changeset/neat-eggs-turn.md.changeset/quick-koalas-thread.mdcommon/accountInit.tscommon/deriveAddress.tscommon/validation.tsserver/api/auth/authentication.tsserver/api/auth/registration.tsserver/api/passkey.tsserver/database/schema.tsserver/test/api/auth.test.tsserver/utils/createCredential.tssrc/utils/accountClient.tssrc/utils/server.ts
There was a problem hiding this comment.
♻️ Duplicate comments (3)
server/database/schema.ts (1)
46-46: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdjust the regex to allow 32-byte salts.
The
credentials_salt_hex_checkconstraint restricts thesaltto exactly 40 hex characters (20 bytes), which matches an address size. However, standard CREATE2 salts are encoded asuint256and are typically 32 bytes (up to 64 hex characters). If the intent is to support generic salts, the regex should allow up to 64 characters to prevent rejecting valid 32-byte salts.💡 Proposed fix
- check("credentials_salt_hex_check", sql`${salt} ~ '^0x[0-9a-fA-F]{40}$'`), + check("credentials_salt_hex_check", sql`${salt} ~ '^0x[0-9a-fA-F]{1,64}$'`),server/api/passkey.ts (1)
41-41: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winParse the salt to ensure proper validation, checksumming, and type safety.
Since
factoryis explicitly parsed to satisfy theAddressbrand and apply checksumming, the rawsaltstrings should be parsed similarly. Passing raw strings risks TypeScript errors (due to a missingAddressbrand vsstring) and bypasses type validation when satisfying schema outputs.
server/api/passkey.ts#L41-L41: parse the rawcredential.saltfrom the database (e.g.,credential.salt ? parse(Address, credential.salt) : undefined).server/api/auth/authentication.ts#L438-L438: parse the rawcredential.salthere as well.server/utils/createCredential.ts#L66-L66: parsezeroAddressto satisfy theAddressbrand requirements in the returned object.server/test/api/auth.test.ts (1)
87-87: 📐 Maintainability & Code Quality | 🟡 Minor | 💤 Low valueAdd an end-to-end non-default salt test.
All four assertions validate only
zeroAddress, so a regression that discards a stored custom salt and returns the default would pass. Add one persisted non-zero salt fixture and assert the exact value through authentication and registration.
server/test/api/auth.test.ts#L87-L87: add assertion for a non-default salt fixture here.server/test/api/auth.test.ts#L469-L470: add assertion for a non-default salt fixture here.server/test/api/auth.test.ts#L566-L568: add assertion for a non-default salt fixture here.server/test/api/auth.test.ts#L714-L715: add assertion for a non-default salt fixture here.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b2259f4d-ff24-44e7-a42d-9c8418a611cc
📒 Files selected for processing (10)
.changeset/fuzzy-cows-breathe.md.changeset/neat-eggs-turn.mdserver/api/auth/authentication.tsserver/api/auth/registration.tsserver/api/passkey.tsserver/database/schema.tsserver/test/api/auth.test.tsserver/utils/createCredential.tssrc/utils/accountClient.tssrc/utils/server.ts
de531ae to
5a2f223
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/utils/createCredential.ts (1)
39-48: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winExplicitly persist the
saltvalue in the database.The
saltvalue is computed and used for address derivation, but it is omitted from the databaseinsertstatement. While the database currently defaults tozeroAddress(which happens to match the hardcodedsaltvalue here), omitting it is implicit and fragile. If thesaltgeneration logic ever changes to use a random or custom value, the database will silently store the wrong salt, breaking future authentication.As per coding guidelines, "prefer explicit over implicit."
💡 Proposed fix
await database.insert(credentials).values([ { account, id: credentialId, publicKey, factory, + salt, transports: options?.webauthn?.transports, source: options?.source, }, ]);Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9df70ae5-2802-40f9-9059-de07cd2d8b2f
📒 Files selected for processing (14)
.changeset/fuzzy-cows-breathe.md.changeset/neat-eggs-turn.md.changeset/quick-koalas-thread.mdcommon/accountInit.tscommon/deriveAddress.tscommon/validation.tsserver/api/auth/authentication.tsserver/api/auth/registration.tsserver/api/passkey.tsserver/database/schema.tsserver/test/api/auth.test.tsserver/utils/createCredential.tssrc/utils/accountClient.tssrc/utils/server.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a2f223636
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67c38eccda
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1143 +/- ##
==========================================
+ Coverage 70.74% 70.92% +0.18%
==========================================
Files 262 262
Lines 11276 11432 +156
Branches 3785 3839 +54
==========================================
+ Hits 7977 8108 +131
- Misses 3013 3031 +18
- Partials 286 293 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e33f11998a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8f248439-0c85-46b1-a349-f420e1b4b3bc
📒 Files selected for processing (15)
.changeset/fuzzy-cows-breathe.md.changeset/neat-eggs-turn.mdserver/api/auth/authentication.tsserver/api/auth/registration.tsserver/api/kyc.tsserver/api/passkey.tsserver/database/schema.tsserver/hooks/activity.tsserver/test/api/auth.test.tsserver/test/api/kyc.test.tsserver/test/hooks/activity.test.tsserver/utils/createCredential.tssrc/utils/accountClient.tssrc/utils/queryClient.tssrc/utils/server.ts
08b9e1f to
079492f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 079492f5db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf35abe92b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
a3663b1 to
7059eec
Compare
1 participant
Summary
Testing
Summary by CodeRabbit