Release to Staging v2.9.16 - 2026-02-12#1741
Conversation
* chore: bump mobile app version to 2.9.15 Update build numbers and deployment timestamps after successful deployment. * bump android version code --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Justin Hernandez <justin.hernandez@self.xyz>
* fix: update celo-sepolia in upgrade script to correct chain ID * feat: add celo-sepolia deployed contracts to registry.json * fix: upgrade script now reads version from contract directly * fix: add library linking for kyc related contracts in upgrade script * feat: IdentityVerificationHub v2.13.0 deployed on Celo Sepolia - Implementation: 0x244c93516Abd58E1952452d3D8C4Ce7D454776B8 - Code-only upgrade (no new initializer), adds KYC support - Tx: 0xf24d1c2cd4fd0055237f406a9850ce6e24f538ed09e57ff09755ed142fcc903c * chore: yarn prettier * Feat/new gcp verifier (#1719) * feat: new gcp jwt verifier * lint: contracts --------- Co-authored-by: Nesopie <87437291+Nesopie@users.noreply.github.com>
* feat: celo mainnet deployments * fix: upgrade script now reads version from contract directly * fix: add library linking for kyc related contracts in upgrade script * feat: IdentityVerificationHub v2.13.0 deployed on Celo - Implementation: 0x0D911083b2F2236D79EF20bb58AAf6009a1220B5 - Changelog: Upgrade * feat: update with new gcpJwtVerifier contract * chore: yarn prettier * Feat/new gcp verifier (#1719) * feat: new gcp jwt verifier * lint: contracts --------- Co-authored-by: Nesopie <87437291+Nesopie@users.noreply.github.com>
* Sumsub: Update keychain and types * sumsub: ProvingMachine changes - WIP * fix: remove duplicate identifier * update proving machine * Refactor && Continue onchain registration if user left the app * fix register flow * Add hooks to KycSuccessScreen * Integrate KycVerifiedScreen (#1686) * Integrate KycVerifiedScreen & Fix race conditions * yarn lint * lint * lint * add mock kyc * fix disclose flow * yarn lint * Feat/add kyc home screen card design (#1708) * feat: add new designs to the kycIdCard * refactor: Update KycIdCard design to match IdCard styling * feat: update document cards + dev document * feat: update empty id card for new design * feat: update pending document card design * feat: update expired doc + unregistered doc cards from new design * fix: unregisted id card button links to continue registration screen * fix: logo design on document cards * feat: add 6 different backgrounds for ids deterministically shows 1 of 6 backgrounds for each document | fix: fixed document designs not displaying correctly. * chore: trigger CI rebuild * feat: Integrate PendingIdCard to Homescreen * fix KycIdCard.tsx --------- Co-authored-by: seshanthS <seshanth@protonmail.com> * lint * fix tests * fix: cleanup only on unmount * coderabbit comments * fix: cleanup unused code * fix: edge case for German Passports with D<< nationality code * fix tests * review comments * review comments * lint * Hide duplicated cards in Homescreen * remove console.log * fix patch * remove unused vars * agent updates * agent feedback * abstract colors and formatting * agent feedback * Regenerate Sumsub patch-package patch * fix: handle malformed kyc payload in card background selector * re-add for clean up --------- Co-authored-by: Evi Nova <66773372+Tranquil-Flow@users.noreply.github.com> Co-authored-by: Evi Nova <tranquil_flow@protonmail.com> Co-authored-by: Justin Hernandez <justin.hernandez@self.xyz>
* agent points feedback * agent feeddback
* refactor(app): share homescreen card dimensions * formatting * fix logic
* Show badge for inactive documents * fix * refactor to use the new flag * add inactive check to ProveScreen * lint * fix for proving button not working * use new qrHashlogic * increase bundle size threshold to 46MB * remove commented out line * add kyc related changes --------- Co-authored-by: seshanthS <seshanth@protonmail.com>
* agent feedback * fix settings screen color
* attempt to make test less flaky * fix flaky test again * lint
* enable sumsub * bugbot feedback
transphorm
had a problem deploying
to
development
GitHub Actions
Failure
transphorm
temporarily deployed
to
development
GitHub Actions
Inactive
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces comprehensive KYC (Know Your Customer) verification support with new document types, WebSocket-based verification tracking, card component variants for different document states, and smart contract deployments. The KYC feature flag is removed, making this functionality always available. Version bumps: app 2.9.15→2.9.16, Android 140→142, iOS 212→213. Changes
Sequence DiagramssequenceDiagram
participant App as Mobile App
participant Store as Pending KYC Store
participant WebSocket as Sumsub WebSocket
participant Navigation as Navigation
participant Success as KycSuccessScreen
App->>Store: 1. usePendingKycRecovery on startup
App->>Store: 2. Fetch & clean expired verifications
Store-->>App: Return non-expired pending list
App->>WebSocket: 3. Subscribe to userId for each pending
Note over WebSocket: Maintains active socket connections
WebSocket->>WebSocket: 4. Monitor verification events
alt Verification Completes
WebSocket->>Store: 5. updateVerificationStatus(userId, 'processing')
WebSocket->>Store: 6. Deserialize applicant info → create KYC data
WebSocket->>Store: 7. Store KYC document with documentId
WebSocket->>Navigation: 8. Navigate to KYCVerified(documentId)
Navigation->>Success: 9. Route to KycSuccessScreen
Success->>WebSocket: 10. Subscribe again for status updates
else Verification Fails
WebSocket->>Store: 5. updateVerificationStatus(userId, 'failed', error)
WebSocket->>Navigation: 6. Navigate to KycFailure
else Timeout (48h)
App->>Store: 5. removeExpiredVerifications()
Store->>Navigation: 6. Clear from pending list
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
packages/mobile-sdk-alpha/tests/proving/provingMachine.documentProcessor.test.ts (1)
152-158:⚠️ Potential issue | 🟠 MajorUpdate
buildProtocolStatetype to includeREGISTER_KYCand add it to all test fixtures.The production
DeployedCircuitstype incommon/src/utils/types.tsincludesREGISTER_KYC: string[], but the test helper's type definition at lines 152–158 omits it. This creates a type mismatch: only one fixture (line 654–661) includesREGISTER_KYC: [], while all others skip it. SinceprovingMachine.tsreturnsREGISTER_KYCas a valid circuit name for KYC documents, all fixtures should include this field to match the actual production shape.Update the type definition and add
REGISTER_KYC: []to everydeployedCircuitsfixture in this file:Type definition fix
deployedCircuits: { REGISTER: string[]; REGISTER_ID: string[]; REGISTER_AADHAAR: string[]; + REGISTER_KYC: string[]; DSC: string[]; DSC_ID: string[]; };app/src/proving/validateDocument.ts (1)
197-213:⚠️ Potential issue | 🟠 MajorAdd explicit guard to skip KYC documents or throw error in
getAlternativeCSCA.KYC documents can reach
getAlternativeCSCAand will silently return an empty map (sincekyc.public_keysis alwaysnull), causingisUserRegisteredWithAlternativeCSCAto perpetually returnisRegistered: false. This masks incomplete KYC support—other code paths likeRecoverWithPhraseScreenexplicitly throw "KYC is not supported yet," butvalidateDocumentsilently degrades. Either:
- Add an early skip/guard in
checkAndUpdateRegistrationStatesto exclude kyc documents- Throw an error in
getAlternativeCSCAwhen kyc is passed (consistent with recovery screens)- Implement
kyc.fetch_public_keysand populate public keys properlyAlso update the JSDoc on lines 198–200 to mention KYC alongside Aadhaar, or remove it if KYC is not actually supported here.
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tsx (1)
126-128:⚠️ Potential issue | 🟡 MinorUser-facing description doesn't mention KYC documents.
Now that KYC documents can reach this confirmation screen, the copy still only references "passport, biometric ID or Aadhaar card." Users verifying via KYC will see text that doesn't describe their document type, which is confusing and could undermine trust in the flow.
Consider updating the description to be document-category-aware (e.g., accept the category as a parameter and adjust the wording), or at minimum add "KYC identity document" to the list.
contracts/tasks/upgrade/upgrade.ts (1)
106-115:⚠️ Potential issue | 🟠 MajorPotential chainId/txServiceUrl mismatch for
celo-sepolia.
celo-sepoliaon Line 109 shares the celo mainnet Safe Transaction Service URL, but itschainId(Line 68) is11142220— different from celo mainnet's42220. WhenSafeApiKitis initialized (Line 118) withchainId: 11142220nandtxServiceUrlpointing to the celo mainnet service, the service may reject requests or return mismatched Safe data. Ensure the Safe Transaction Service at this URL actually supports chainId11142220.app/src/screens/documents/selection/LogoConfirmationScreen.tsx (1)
62-103:⚠️ Potential issue | 🟡 MinorConsider guarding against a missing
accessToken.userIdon the success path.If
fetchAccessToken()returns a token butuserIdis undefined/null (e.g., backend issue), line 94 would navigate toKycSuccesswith an undefineduserId, potentially causing downstream issues on that screen.🛡️ Suggested defensive check
// Verification succeeded - navigate to KycSuccessScreen + if (!accessToken.userId) { + console.error('Sumsub verification succeeded but userId is missing'); + navigation.navigate('KycFailure', { + countryCode, + canRetry: true, + }); + return; + } navigation.navigate('KycSuccess', { userId: accessToken.userId });packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
76-78:⚠️ Potential issue | 🟡 Minor
getMappingKeysilently returns'DSC_ID'for KYC documents in the DSC path.If
circuitType === 'dsc'anddocumentCategory === 'kyc', this falls through to'DSC_ID'. While_generateCircuitInputscorrectly throws for this case (lines 124–126),resolveWebSocketUrlis called before that ininitTeeConnection, which would resolve a bogus WebSocket URL. Consider throwing here explicitly, consistent with the guard in_generateCircuitInputs.Proposed fix
// circuitType === 'dsc' + if (documentCategory === 'aadhaar' || documentCategory === 'kyc') { + throw new Error(`DSC circuit type is not supported for ${documentCategory} documents`); + } return documentCategory === 'passport' ? 'DSC' : 'DSC_ID';app/src/screens/kyc/KYCVerifiedScreen.tsx (1)
100-103:⚠️ Potential issue | 🟡 MinorTypo: "proof" should be "prove".
Line 102: "you can use to proof your identity" → "you can use to prove your identity".
✏️ Fix
<Description style={styles.description}> - Next Self will generate a zk proof specifically for this device that - you can use to proof your identity. + Next Self will generate a zk proof specifically for this device that + you can use to prove your identity. </Description>packages/mobile-sdk-alpha/src/components/buttons/HeldPrimaryButtonProveScreen.tsx (1)
17-25:⚠️ Potential issue | 🟠 MajorBreaking change to public API:
HeldPrimaryButtonProveScreenPropsrequires new prop.The new required prop
hasCheckedForInactiveDocument: booleanis a breaking change sinceHeldPrimaryButtonProveScreenis exported in the mobile-sdk-alpha public API. The only identified consumer (BottomVerifyBar) has been updated, but external consumers of the package will need to update their code. Document this breaking change in the release notes or CHANGELOG if not already done.common/src/utils/passports/passport.ts (1)
47-72:⚠️ Potential issue | 🟠 MajorNormalize
fullNamefor OFAC matching per compliance rules.Line 61 concatenates
parsedApplicantInfo.fullNamedirectly without applying required normalization. Per coding guidelines for this file path, names must be normalized using case-folding, Unicode NFKC normalization, and diacritics removal for OFAC matching. Additionally, the learnings indicate OFAC hash commitments should use the Poseidon hash function with 'ofac-v1' domain separator and per-issuer salt rather than SHA256.The type widening to
IDDocumentis backward-compatible and poses no issues at call sites.common/src/utils/circuits/registerInputs.ts (1)
105-112:⚠️ Potential issue | 🔴 Critical🚨 Secret and PII logged to console in
generateTEEInputsAadhaarRegister.This
console.logdumps the rawsecret(a cryptographic secret/private key) and the fullaadhaarDataobject (which contains PII — name, DOB, Aadhaar number, etc.) to the console. While this appears to be pre-existing code not introduced in this PR, it's a critical security violation that should be addressed.As per coding guidelines: "NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers."
Proposed fix
- console.log( - 'publicKeys-aadhaar', - publicKeys, - 'secret-aadhaar', - secret, - 'aadhaarData-aadhaar', - aadhaarData - ); + console.log('generateTEEInputsAadhaarRegister: publicKeys count:', publicKeys.length);app/src/providers/passportDataProvider.tsx (1)
696-697:⚠️ Potential issue | 🟠 MajorType mismatch:
IPassportContext.setDataacceptsPassportDatabutstorePassportDataimplementation acceptsIDDocument.The interface declares
setData: (data: PassportData) => Promise<void>(line 691), while the assigned implementationstorePassportDataaccepts the broaderIDDocumenttype (which isAadhaarData | KycData | PassportData). This breaks the type contract and prevents callers from passingKycDataorAadhaarDatathrough the context, even though the runtime function handles them.Update the interface to match the implementation:
Proposed fix
- setData: (data: PassportData) => Promise<void>; + setData: (data: IDDocument) => Promise<void>;
🤖 Fix all issues with AI agents
In `@app/src/components/homescreen/IdCard.tsx`:
- Around line 274-283: The getCountryDemonym function is incorrectly treating
any input containing '<' as German; remove the special-case check that inspects
the original code (code.includes('<')) and its D<< fallback. Instead use the
already-sanitized upperCode (from uppercasing, stripping '<', and trimming) to
look up COUNTRY_DEMONYMS and return that value or upperCode as the fallback;
keep the early guards (if !code / if !upperCode) intact and reference
getCountryDemonym and COUNTRY_DEMONYMS when making the change.
- Around line 307-317: The switch in navigateToDocumentOnboarding (inside
IdCard.tsx) omits the 'kyc' case so tapping the inactive-warning "Continue"
silently no-ops for KYC documents; add a case 'kyc' that routes to the same
onboarding flow as passport/id_card (e.g.,
navigation.navigate('DocumentOnboarding')), mirroring the fix applied in
ProveScreen, so KYC documents correctly navigate to the onboarding screen.
In `@app/src/components/homescreen/KycIdCard.tsx`:
- Around line 148-153: The getTruncatedId function is prepending "0x" to KYC
document IDs which is misleading for non-hex IDs; update getTruncatedId to stop
adding the "0x" prefix and return either a truncated plain ID (e.g.,
`${idNumber.slice(0,4)}..${idNumber.slice(-3)}`) when length > 10 or the raw
idNumber when shorter, ensuring both branches use the same no-prefix format and
preserving the existing idNumber/null checks.
In `@app/src/components/homescreen/UnregisteredIdCard.tsx`:
- Around line 131-150: The YStack used as the interactive "Complete
Registration" control in UnregisteredIdCard.tsx is missing an accessibility role
so screen readers won't announce it as a button; update the YStack that has
onPress={onRegisterPress} to include the proper accessibility prop (e.g.,
accessibilityRole="button" or role="button") and any relevant
accessibilityLabel/accessible props as needed so VoiceOver/TalkBack treat it as
a button; ensure you apply this change specifically to the YStack containing the
"Complete Registration" Text.
In `@app/src/hooks/usePendingKycRecovery.ts`:
- Around line 57-106: The effect can stall when processingWithDocument exists
but navigationRef.isReady() is false because nothing triggers the effect to
re-run; update usePendingKycRecovery so that when processingWithDocument is
found and navigationRef.isReady() is false you register a one-time listener or
short-lived polling tied to navigation readiness (or navigation container ref's
ready callback) that re-attempts the navigation logic for that specific userId,
and only mark the userId in hasAttemptedRecoveryRef after successful navigation;
specifically modify the useEffect handling of processingWithDocument to
subscribe to navigation readiness (or setInterval with clear) and on readiness
call navigationRef.navigate('KYCVerified', { documentId:
processingWithDocument.documentId }) then add processingWithDocument.userId to
hasAttemptedRecoveryRef and clean up the listener/poll to avoid memory
leaks—keep existing behavior for pending items using subscribe and ensure
cleanup in the effect return.
In `@app/src/screens/dev/DevSettingsScreen.tsx`:
- Around line 57-82: The handler handleRemoveExpirationDateFlagPress should wrap
the async work in a try/catch and provide user feedback: call
loadDocumentCatalogDirectlyFromKeychain() and find the selectedDocument inside a
try block, if no selectedDocument show an Alert explaining no document is
selected, otherwise delete selectedDocument.hasExpirationDate, await
saveDocumentCatalogDirectlyToKeychain(catalog), then show a success Alert; in
the catch show an Alert with the error message and log the error so
saveDocumentCatalogDirectlyToKeychain/loadDocumentCatalogDirectlyFromKeychain
failures aren’t silently swallowed.
In `@app/src/screens/home/HomeScreen.tsx`:
- Around line 314-320: Guard the expiration check so we only run it for
documents that can expire: read the document metadata
(metadata.hasExpirationDate) and only compute attributes.expiryDateSlice and
call checkDocumentExpiration when that flag is truthy; then return
<ExpiredIdCard key={metadata.id} /> only if metadata.hasExpirationDate is true
and checkDocumentExpiration(...) returns true. This mirrors the mobile SDK
pattern in isDocumentValidForProving() and avoids unnecessary processing for
KYC/non-expiring docs.
In `@app/src/screens/home/PointsInfoScreen.tsx`:
- Around line 99-123: The cleanup effect is re-running because `callbacks` may
be a new object each render; fix by stabilizing that reference: memoize the
lookup with useMemo(() => getModalCallbacks(callbackId), [callbackId]) and use
that memoized `callbacks` in `handleNextPress` and the useEffect so the effect
only cleans up on real `callbackId` changes/unmount, or alternatively keep
`callbackId` in the effect deps and store the latest callbacks in a ref (e.g.,
`callbacksRef.current = getModalCallbacks(callbackId)`) then read from
`callbacksRef` inside `handleNextPress` and the cleanup while removing
`callbacks` from the dependency array; update references to `callbacks` in
`handleNextPress`, the cleanup, and the unregister call
(`unregisterModalCallbacks`) accordingly.
In `@app/src/screens/kyc/KYCVerifiedScreen.tsx`:
- Around line 84-86: The catch block in KYCVerifiedScreen currently logs the raw
error object in the catch of the registration trigger; replace that direct
logging with the sanitized message from the existing sanitizeErrorMessage
utility (the same approach used in useSumsubLauncher.ts) so sensitive
SDK/document data isn't emitted. Locate the catch in the KYCVerifiedScreen
component (the block that currently does console.error('[KYCVerifiedScreen]
Failed to trigger registration:', err)) and call sanitizeErrorMessage(err) and
log its result (including the same context prefix) instead of logging err
directly.
- Around line 39-87: handleGenerateProof can be invoked multiple times because
there's no loading guard; add a local state flag (e.g., isLoading) and wire it
to the Generate Proof button's disabled/loading prop, early-return if isLoading
at the start of handleGenerateProof, set isLoading = true right after
buttonTap() and clear it in a finally block so setSelectedDocument,
loadSelectedDocument, removePendingVerification and
selfClient.emit(SdkEvents.DOCUMENT_OWNERSHIP_CONFIRMED, ...) run only once per
tap; ensure any error path also clears isLoading.
In `@app/src/screens/verification/ProveScreen.tsx`:
- Around line 123-136: The switch in navigateToDocumentOnboarding ignores
documentCategory === 'kyc', causing no navigation; add a 'kyc' case to the
switch that calls navigate(...) to the correct onboarding screen (e.g.,
navigate('DocumentOnboarding') or navigate('KycOnboarding') depending on your
routing), so documentMetadata.documentCategory === 'kyc' will route the user
when the modal "Continue" triggers navigateToDocumentOnboarding; update the
switch inside navigateToDocumentOnboarding accordingly and keep the dependency
array [navigate] unchanged.
In `@common/src/utils/circuits/registerInputs.ts`:
- Around line 274-284: In mapDisclosuresToKycFields, the
['issuing_state','ADDRESS'] pair is wrong: update the mapping array (the mapping
variable inside mapDisclosuresToKycFields) so that 'issuing_state' is not mapped
to 'ADDRESS' — either remove the ['issuing_state', ...] entry entirely or map it
to an appropriate field such as 'COUNTRY' (or add a new 'ISSUING_COUNTRY' KYC
field and its length in KYC_FIELD_LENGTHS and map to that) and ensure you adjust
KYC_FIELD_LENGTHS if you introduce 'ISSUING_COUNTRY'; do not leave issuing_state
mapped to ADDRESS.
In `@common/src/utils/passports/validate.ts`:
- Around line 290-291: The function generateKycCommitment currently lacks an
explicit return type and only returns a string inside the
isKycDocument(passportData) branch, causing an inferred string | undefined;
update generateKycCommitment to have an explicit return type of string and
handle the non-KYC case explicitly (e.g., throw a clear error or return a
defined default) so it never returns undefined, ensuring assignment to
commitment: string is safe; additionally remove the unused import
generateKycNullifier from the top of the file.
In `@contracts/deployments/registry.json`:
- Around line 155-182: The JSON contains duplicate "2.13.0" keys causing the
celo-sepolia deployment to be lost; locate the "2.13.0" entries in the registry
JSON and merge their "deployments" objects so a single "2.13.0" entry contains
both "celo-sepolia" and "celo" under the same "deployments" map (preserve each
deployment's impl, deployedAt, deployedBy, gitCommit, initializerVersion,
initializerFunction, changelog and gitTag fields).
In `@contracts/tasks/upgrade/upgrade.ts`:
- Around line 709-725: The upgrade currently skips parameterized initializers
(the block using proxyContract.interface.getFunction(initializerName) only
encodes when initFragment.inputs.length === 0), leaving initData as "0x" and
never calling required initializers; fix by either (A) supporting parameterized
initializers: read initializer arguments from the target version metadata (e.g.,
enhance getVersionInfo(contractId, newVersion) to return a field like
initializerArgs or initializerParams and then call
iface.encodeFunctionData(initializerName, initializerArgs) when
initFragment.inputs.length > 0), or (B) fail loudly: if
initFragment.inputs.length > 0 and no initializer args are present, throw or log
an error and abort the upgrade (instead of silently setting initData), making
use of symbols initializerName, initFragment, proxyContract.interface,
getVersionInfo, noNewInitializer, and initData to locate and implement the
change.
🟡 Minor comments (14)
packages/mobile-sdk-alpha/src/constants/colors.ts-9-9 (1)
9-9:⚠️ Potential issue | 🟡 MinorBreaking change:
amber500value updated from#F2E3C8to#F59E0B.This is a publicly exported constant from the SDK. Any consumer relying on the previous
amber500value will see a visual change (cream → actual amber). The old value now lives inwarmCream(line 69), which is good, but downstream consumers won't automatically pick that up.Please ensure this change is documented in release notes or a migration guide so SDK consumers can adjust. Based on learnings: "Ensure no breaking changes to public API or document them properly."
app/src/utils/devUtils.ts-135-137 (1)
135-137:⚠️ Potential issue | 🟡 MinorPII logged in console output.
Lines 135–137 log
firstNameandlastNameto the console. Even though this is dev-only test data today, the pattern sets a precedent and the log could surface real-looking names in crash reporters or log aggregators on debug builds.Redact or omit the names:
Proposed fix
- console.log( - `Created ${doc.type} for ${doc.firstName} ${doc.lastName} (${doc.country})`, - ); + console.log( + `Created ${doc.type} for ${doc.country}`, + );As per coding guidelines: "NEVER log sensitive data including PII (names, DOB, passport numbers, addresses)."
contracts/deployments/registry.json-290-304 (1)
290-304:⚠️ Potential issue | 🟡 MinorMissing metadata for IdentityRegistryKyc v1.0.0 deployment.
gitTag(line 295) anddeployedBy(line 300) are empty strings. This reduces traceability for the initial KYC registry deployment. Consider populating these before merging to staging.packages/mobile-sdk-alpha/src/documents/utils.ts-244-244 (1)
244-244:⚠️ Potential issue | 🟡 Minor
datafield will be empty string for KYC documents.With the new
'kyc'category flowing throughstoreDocumentWithDeduplication, this line casts toAadhaarDataand accesses.qrData, which won't exist on a KYC document. The|| ''fallback prevents a crash, but if downstream code relies onmetadata.databeing meaningful, KYC documents will silently have an empty string. Worth confirming this is acceptable or if KYC docs should populate a more representative value.packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tsx-83-86 (1)
83-86:⚠️ Potential issue | 🟡 MinorUpdate the description text to include KYC documents.
Line 127's description mentions "passport, biometric ID or Aadhaar card" but KYC is now a valid document type flowing through this screen. The user-facing text should be updated to reflect all supported document categories.
The concern about
signatureAlgorithmandcurveOrExponentbeingundefinedfor KYC documents is not an issue—these fields are explicitly optional in the event payload type definition, and the listener correctly handlesundefinedvalues. This is intentional: KYC documents don't have CSCA signature metadata.common/src/utils/kyc/api.ts-90-94 (1)
90-94:⚠️ Potential issue | 🟡 MinorNo validation on signature format before destructuring.
deserializeSignaturesplits on,and destructures exactly 3 elements. If the base64-decoded string doesn't contain exactly 3 comma-separated BigInt-parseable values:
- Fewer than 3:
BigInt(undefined)throws an unhelpfulTypeError- More than 3: silently ignores extra values
For a cryptographic primitive, a defensive check with a clear error message would be appropriate:
🛡️ Proposed defensive check
export function deserializeSignature(signature: string): { R: Point<bigint>; s: bigint } { - const [Rx, Ry, s] = Buffer.from(signature, 'base64').toString('utf-8').split(',').map(BigInt); + const parts = Buffer.from(signature, 'base64').toString('utf-8').split(','); + if (parts.length !== 3) { + throw new Error(`Invalid signature format: expected 3 components, got ${parts.length}`); + } + const [Rx, Ry, s] = parts.map(BigInt); return { R: [Rx, Ry] as Point<bigint>, s }; }app/src/components/homescreen/cardSecurityBadge.ts-11-38 (1)
11-38:⚠️ Potential issue | 🟡 MinorRemove misleading docstring claim about KYC documents — they are excluded from this function and handled separately.
The
getSecurityLevel()function's parameter type isPassportData | AadhaarData, which explicitly excludesKycData. In the only call site (IdCard.tsx), KYC documents are filtered out with an early return (lines 351–357) beforegetSecurityLevel()is called on line 393.The docstring incorrectly claims "KYC documents -> STANDARD (always)" but this code path never executes. The
'STANDARD'value is defined in theSecurityLeveltype but never returned. Update the JSDoc to document only the actual supported document types (Aadhaar and MRZ), or refactor to include KYC handling in the function if it should return'STANDARD'for those documents.app/src/components/homescreen/EmptyIdCard.tsx-84-104 (1)
84-104:⚠️ Potential issue | 🟡 MinorDuplicate text: both header title and subtitle display "NO IDENTITY FOUND".
Both the title (line 93) and the subtitle (line 102) render the identical string
"NO IDENTITY FOUND". Comparing with sibling components —ExpiredIdCarduses"EXPIRED ID"/"TIME TO REGISTER A VALID COPY"andPendingIdCarduses"IDENTITY UNDER REVIEW"/"NO IDENTITY FOUND"— this looks like a copy-paste oversight. The subtitle likely needs different text (e.g.,"REGISTER A NEW DOCUMENT"or similar per the Figma spec).common/src/utils/kyc/generateInputs.ts-283-283 (1)
283-283:⚠️ Potential issue | 🟡 MinorPrecision loss when generating mock secret key from
subOrder.
Number(subOrder - 2n)truncates the BabyJubJub suborder (~2^253) toNumber.MAX_SAFE_INTEGER(~2^53), so the random key space is vastly reduced. This is in a mock/test generator, so it's not a production security issue, but worth noting for correctness in test scenarios.app/src/screens/dev/DevSettingsScreen.tsx-60-60 (1)
60-60:⚠️ Potential issue | 🟡 MinorMinor typo in alert message.
The message ends with
?."— there's an extra period after the question mark.- 'Are you sure you want to remove the expiration date flag for the current (selected) document?.', + 'Are you sure you want to remove the expiration date flag for the current (selected) document?',common/src/utils/passports/passport.ts-58-63 (1)
58-63:⚠️ Potential issue | 🟡 MinorContent hash: field concatenation without delimiters risks collisions.
The stable fields are concatenated without any separator:
${fullName}${dob}${country}${idType}. Different field-value combinations can produce the same concatenated string (e.g.,fullName="AB"+country="CD"vsfullName="ABC"+country="D"). Use a delimiter consistent with the other branches.Proposed fix
- const stableFields = `${parsedApplicantInfo.fullName}${parsedApplicantInfo.dob}${parsedApplicantInfo.country}${parsedApplicantInfo.idType}`; + const stableFields = `${parsedApplicantInfo.fullName}|${parsedApplicantInfo.dob}|${parsedApplicantInfo.country}|${parsedApplicantInfo.idType}`;common/src/utils/kyc/utils.ts-14-23 (1)
14-23:⚠️ Potential issue | 🟡 MinorAdd explicit return type annotation for clarity.
The function lacks a return type annotation and implicitly returns
string | undefined. Add an explicit return typestring | undefinedto clarify the contract. Additionally, line 19 contains a redundant.map(Number)conversion sinceArray.from(...charCodeAt(0))already producesnumber[].app/src/screens/verification/ProveScreen.tsx-320-320 (1)
320-320:⚠️ Potential issue | 🟡 MinorDebug log leaks
selectedAppobject to console.
console.log('useEffect selectedApp', selectedApp)will serialize the entireselectedAppobject, which may includeuserId,endpoint,sessionId, and other fields. Even if not strict PII, this is noisy in production and could expose session identifiers. Consider removing or gating behind a debug flag.As per coding guidelines: "Do not include sensitive data in logs - avoid logging PII, credentials, and tokens."
app/src/screens/home/HomeScreen.tsx-292-299 (1)
292-299:⚠️ Potential issue | 🟡 MinorBare
return;inside.map()producesundefined— usereturn null;instead.Static analysis (Biome) flagged this: a
.map()callback should always return a value. The barereturn;at line 298 returnsundefined, which React handles gracefully but violates the linter rule and harms readability. Usereturn null;for consistency with the pattern at line 288.Proposed fix
if ( !documentData.metadata.isRegistered && activePendingVerifications.some( doc => doc.documentId === documentData.metadata.id, ) ) { - return; + return null; }
🧹 Nitpick comments (25)
app/Gemfile (1)
16-16: Nice addition ofbundler-audit— solid move for catching vulnerable gem versions in CI. Therequire: falseis appropriate for a CLI-only tool.RuboCop flags that
bundler-auditshould appear beforenokogirialphabetically. Low priority, but easy to fix if you want a clean lint run.Optional: fix gem ordering
group :development do + gem "bundler-audit", "~> 0.9", require: false gem "dotenv" gem "nokogiri", "~> 1.18", platform: :ruby - gem "bundler-audit", "~> 0.9", require: false endapp/src/utils/devUtils.ts (2)
83-94:clearAllDocumentsis callable without a dev-mode guard.The function is
async function(not exported), so today it's only reachable via the guardedgenerateTestDocuments. However, it unconditionally deletes every document in the catalog—a destructive operation. Adding a lightweightIS_DEV_MODEcheck inside it would provide defense-in-depth against accidental future misuse.Suggested safeguard
async function clearAllDocuments(): Promise<void> { + if (!IS_DEV_MODE) { + return; + } const catalog = await loadDocumentCatalogDirectlyFromKeychain();
128-129: Settingmock = falseon mock documents is intentional but risky.Overriding
mockDoc.mock = falsemakes generated test docs indistinguishable from real ones. If a tester forgets to clear them, downstream verification flows, analytics, or on-chain interactions could treat them as genuine. Consider adding a comment or a separate field (e.g.,devGenerated: true) that the UI can key off for background rendering without losing traceability.common/src/utils/proving.ts (1)
76-84: Consider a lookup map instead of deepening ternary chains.The ternary chain for mapping
circuitName → typeis now 4 levels deep and will keep growing as new document types are added. A simple map would be more maintainable, self-documenting, and would let you throw on unexpected circuit names rather than silently falling back to'disclose_id'.♻️ Optional refactor
- const type = - circuitName === 'vc_and_disclose' - ? 'disclose' - : circuitName === 'vc_and_disclose_aadhaar' - ? 'disclose_aadhaar' - : circuitName === 'vc_and_disclose_kyc' - ? 'disclose_kyc' - : 'disclose_id'; + const discloseTypeMap: Record<string, DiscloseProofType> = { + vc_and_disclose: 'disclose', + vc_and_disclose_aadhaar: 'disclose_aadhaar', + vc_and_disclose_kyc: 'disclose_kyc', + vc_and_disclose_id: 'disclose_id', + }; + const type = discloseTypeMap[circuitName]; + if (!type) { + throw new Error(`Unknown disclose circuit name: ${circuitName}`); + }Same pattern could be applied to the register branch (lines 100-105). This would also catch mismatches at runtime instead of silently defaulting.
contracts/tasks/upgrade/upgrade.ts (2)
406-425: Libraries are redeployed on every upgrade run — consider reuse for cost savings.Each invocation deploys all 7 libraries fresh, even if identical bytecode is already on-chain. For testnet this is fine, but on Celo mainnet this could burn non-trivial gas. Consider optionally caching library addresses (e.g., in the registry) and skipping redeployment when bytecode matches.
That said, for an operator-facing script with infrequent execution, this is pragmatic.
882-906: Dead code: theisOwnableContractblock on Lines 887–906 is unreachable.The same
isOwnableContractcondition is already handled at Lines 821–847, which returns early. The code here can never execute. There's also a duplicate "Step 15" comment (Lines 753 and 883).Remove the unreachable block
- // ======================================================================== - // Step 15: Handle Safe proposal or owner direct execution - // ======================================================================== - - // Skip Safe proposal if this is an Ownable contract (first upgrade) - if (isOwnableContract) { - log.step("Skipping Safe proposal - owner must execute directly"); - log.box([ - "OWNER DIRECT EXECUTION REQUIRED", - "═".repeat(70), - "", - "Transaction Data:", - ` To: ${proxyAddress}`, - ` Data: ${upgradeData}`, - "", - "Execute this transaction from the owner account:", - ` ${currentOwner}`, - "", - "After upgrade completes, grant SECURITY_ROLE to Safe:", - ` ${governance.securityMultisig || "Not configured"}`, - "", - "Future upgrades can use Safe proposals.", - ]); - return; - } - - log.step("Checking if you're a multisig signer..."); + log.step("Checking if you're a multisig signer...");app/src/utils/documentUtils.ts (1)
9-24: Naming collision withgetDocumentTypeNameindocumentAttributes.ts.There's another
getDocumentTypeNamefunction inapp/src/utils/documentAttributes.ts(lines 330-341) that maps short codes ('p'→'Passport','i'→'ID'). Both are potentially exported from@/utils. Having two functions with the same name but different input semantics (category string vs. short code) is a maintenance hazard — callers can easily import the wrong one.Consider renaming one to disambiguate, e.g.
getDocumentCategoryDisplayNamevsgetDocumentTypeCodeDisplayName.#!/bin/bash # Check how both getDocumentTypeName functions are exported and imported rg -n --type=ts 'getDocumentTypeName' -g '!node_modules'app/src/screens/verification/DocumentSelectorForProvingScreen.tsx (1)
272-272: Note: Two identically-namedgetDocumentTypeNamefunctions exist in the codebase.
app/src/utils/documentUtils.tsexportsgetDocumentTypeName(category, idType?)whileapp/src/utils/documentAttributes.tsexportsgetDocumentTypeName(documentType). The import here correctly referencesdocumentUtils.ts, but the name collision across modules could trip up future contributors.packages/mobile-sdk-alpha/src/stores/protocolStore.ts (3)
562-583: KYCfetch_deployed_circuitsandfetch_circuits_dns_mappinglack timeout protection.These fetchers use bare
fetch()withoutfetchWithTimeout(), unlike the passport and id_card counterparts. On degraded infrastructure, these calls can hang indefinitely. The same issue exists for aadhaar's fetchers, but since this is new code, it's worth hardening from the start.♻️ Suggested fix for fetch_deployed_circuits (same pattern for fetch_circuits_dns_mapping)
fetch_deployed_circuits: async (environment: 'prod' | 'stg') => { const url = `${environment === 'prod' ? API_URL : API_URL_STAGING}/deployed-circuits`; - const response = await fetch(url); + const response = await fetchWithTimeout(url); if (!response.ok) { throw new Error(`HTTP error fetching ${url}! status: ${response.status}`); }
584-586:fetch_public_keysis a no-op stub — consider adding a comment explaining why.Setting
public_keystonullwithout fetching is presumably intentional (KYC verification doesn't use traditional CSCA/DSC public keys), but a brief comment would prevent future contributors from treating this as unfinished work. The stale comment on line 541 (//add empty functions for kyc) also no longer reflects reality.
602-641: KYC OFAC tree fetcher also uses barefetch()insidefetchTree— same timeout concern.The nested
fetchTreehelper at lines 608-623 callsfetch()directly. For consistency and resilience, consider usingfetchWithTimeout()here as well, matching the pattern infetch_identity_treejust above.app/src/hooks/useSumsubWebSocket.ts (1)
79-81: No connection timeout or reconnection control on the WebSocket.
io(SUMSUB_TEE_URL, { transports: ['websocket', 'polling'] })uses socket.io defaults: auto-reconnect with exponential backoff, no hard connection timeout. On mobile, if the TEE server is unreachable, this will retry indefinitely, draining battery and resources.Consider adding
timeoutandreconnectionAttemptsto bound the connection lifecycle:Suggested socket options
const socket = io(SUMSUB_TEE_URL, { transports: ['websocket', 'polling'], + timeout: 15000, + reconnectionAttempts: 5, });app/tests/src/screens/kyc/KYCVerifiedScreen.test.tsx (1)
173-177: Test "should use the documentId from route params" doesn't actually assert documentId usage.This test only verifies the component renders truthy — identical to the "should render the screen without errors" test on line 115. Consider asserting something that actually depends on
documentId(e.g., verifying it's passed to a downstream call or appears in rendered output) to make this test meaningful.app/src/screens/documents/management/ManageDocumentsScreen.tsx (1)
219-222: KYC data deserialized twice per document in list rendering.For KYC documents,
deserializeApplicantInfo(data)is called once ingetDisplayName→getKYCDisplayName(line 149) and again inextractCountryFromData(line 220), both invoked per document in the.map()at line 308. The operation is lightweight (base64 decode + slicing), so this isn't a performance blocker, but extracting the deserialized result once and passing it through would be cleaner.packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
907-914: DuplicatedcircuitNameresolution logic between_reconnectTeeWebSocketandinitTeeConnection.Lines 907–913 and 1348–1354 contain identical ternary chains for determining
circuitNamebased ondocumentCategory. This is a maintenance risk — if a new document category is added, both must be updated in lockstep.Consider extracting a shared helper, e.g.
getDiscloseCircuitName(documentCategory).app/src/stores/pendingKycStore.ts (1)
14-14: Open TODO:VERIFICATION_TIMEOUT_MSvalue needs finalization.The comment
TODO seshanthsuggests this 48-hour timeout is a placeholder. Per learnings, this is UX-only (server-side is authoritative), but the value should still be intentional before shipping to staging.Would you like me to open a tracking issue for this TODO?
common/src/utils/kyc/generateInputs.ts (1)
248-271:generateKycRegisterInputis well-structured.Deserializes applicant info and signature, constructs the register input with proper field mapping. The
asynckeyword is present despite noawait— the function returns a plain value, so this is harmless but technically unnecessary.app/tests/src/utils/documents.test.ts (1)
23-131: Consider adding a test case forkycdocument category.Given the broader KYC changes in this PR (new
isKycDocumenttype guard, KYC-specific circuit paths, etc.), it would be valuable to verifyisDocumentInactivebehavior fordocumentCategory: 'kyc'as well.app/src/hooks/usePendingKycRecovery.ts (1)
106-106:unsubscribeAllin dependency array but unused in effect body.
unsubscribeAllappears in the dependency array but is never called inside this effect. If its reference identity ever changes, it will cause an unnecessary re-execution. Consider removing it from the deps array (with an eslint-disable comment if needed) or moving it to the cleanup-only effect on line 50.Suggested fix
- }, [pendingVerifications, subscribe, unsubscribeAll]); + }, [pendingVerifications, subscribe]);app/src/components/homescreen/KycIdCard.tsx (1)
107-111:hiddenprop is accepted but unused.The
hiddenprop is destructured as_hiddenand never referenced. If the card should support a hidden/collapsed state (like other card variants), the rendering logic should honor it. If not needed, remove it from the interface to keep the API surface clean.common/src/utils/kyc/utils.ts (1)
25-42:generateKycNullifieris the canonical implementation — ensuregenerateNullifierinpassport.tsdelegates here.As noted in the passport.ts review, the KYC branch in
generateNullifier(passport.ts:211-227) duplicates this exact logic. This file should be the single source of truth for KYC nullifier generation.common/src/utils/passports/passport.ts (1)
211-227: Extract the KYC nullifier logic to use the shared utilitygenerateKycNullifier.The deserialization → serialization → slicing → Poseidon hashing logic in the
isKycDocumentbranch (lines 211–227) is identical togenerateKycNullifierincommon/src/utils/kyc/utils.ts. Changes to either copy must be synchronized to avoid divergence.Suggested refactor
if (isKycDocument(passportData)) { - const applicantInfo = deserializeApplicantInfo(passportData.serializedApplicantInfo); - const serializedData = serializeKycData(applicantInfo); - const msgPadded = Array.from(serializedData, (x) => x.charCodeAt(0)); - const dataPadded = msgPadded.map((x) => Number(x)); - const idNumber = dataPadded.slice( - KYC_ID_NUMBER_INDEX, - KYC_ID_NUMBER_INDEX + KYC_ID_NUMBER_LENGTH - ); - const nullifierInputs = [ - ...'sumsub'.split('').map((x) => x.charCodeAt(0)), - ...idNumber, - ...dataPadded.slice(KYC_ID_TYPE_INDEX, KYC_ID_TYPE_INDEX + KYC_ID_TYPE_LENGTH), - ]; - const nullifier = packBytesAndPoseidon(nullifierInputs); - return nullifier; + return generateKycNullifier(passportData); }packages/mobile-sdk-alpha/src/documents/validation.ts (1)
109-189: IdenticalgetKycAttributesimplementation exists in bothapp/src/utils/documentAttributes.tsandpackages/mobile-sdk-alpha/src/documents/validation.ts.Both functions deserialize KYC data, format name/date fields, and return the same
DocumentAttributesstructure using identical logic. This creates a maintenance risk—any future changes to deserialization, date parsing, or field mappings must be synchronized across both files.Consider extracting the shared logic to a common utilities module (either within an existing shared package or a new one) to establish a single source of truth.
app/src/providers/passportDataProvider.tsx (1)
898-911: Silent catch when deserializing KYCidType— consider logging a warning.The empty catch block at line 907 swallows deserialization errors completely. While this is intentionally defensive for metadata enrichment, a
console.warnwould help diagnose issues with malformed KYC data during development.Proposed fix
} catch { - return {}; + console.warn('Failed to deserialize KYC applicantInfo for idType extraction'); + return {}; }app/src/components/homescreen/IdCard.tsx (1)
61-267: Consider extractingCOUNTRY_DEMONYMSto a shared utility.This 200+ line static mapping bloats the component file. Moving it to a utility (e.g.,
@/utils/countryDemonyms.ts) would improve readability and make it reusable across components.
| const getCountryDemonym = (code: string): string => { | ||
| if (!code) return ''; | ||
| const upperCode = code.toUpperCase().replace(/</g, '').trim(); | ||
| if (!upperCode) return ''; | ||
| // Fallback for any remaining special codes with < | ||
| if (code.includes('<')) { | ||
| return COUNTRY_DEMONYMS['D<<'] || 'GERMAN'; | ||
| } | ||
| return COUNTRY_DEMONYMS[upperCode] || upperCode; | ||
| }; |
There was a problem hiding this comment.
getCountryDemonym incorrectly treats any code containing < as German.
Line 279 checks code.includes('<') after stripping < from upperCode on line 276. Since code (the original, unstripped value) is checked, any MRZ nationality code with padding (e.g., 'GBR<<<', 'F<<') would incorrectly return 'GERMAN'. The special D<< handling at lines 366-369 (where nationality is normalized before calling this function) makes the fallback here redundant and harmful.
Proposed fix
const getCountryDemonym = (code: string): string => {
if (!code) return '';
const upperCode = code.toUpperCase().replace(/</g, '').trim();
if (!upperCode) return '';
- // Fallback for any remaining special codes with <
- if (code.includes('<')) {
- return COUNTRY_DEMONYMS['D<<'] || 'GERMAN';
- }
return COUNTRY_DEMONYMS[upperCode] || upperCode;
};🤖 Prompt for AI Agents
In `@app/src/components/homescreen/IdCard.tsx` around lines 274 - 283, The
getCountryDemonym function is incorrectly treating any input containing '<' as
German; remove the special-case check that inspects the original code
(code.includes('<')) and its D<< fallback. Instead use the already-sanitized
upperCode (from uppercasing, stripping '<', and trimming) to look up
COUNTRY_DEMONYMS and return that value or upperCode as the fallback; keep the
early guards (if !code / if !upperCode) intact and reference getCountryDemonym
and COUNTRY_DEMONYMS when making the change.
| const navigateToDocumentOnboarding = useCallback(() => { | ||
| switch (idDocument?.documentCategory) { | ||
| case 'passport': | ||
| case 'id_card': | ||
| navigation.navigate('DocumentOnboarding'); | ||
| break; | ||
| case 'aadhaar': | ||
| navigation.navigate('AadhaarUpload', { countryCode: 'IND' }); | ||
| break; | ||
| } | ||
| }, [idDocument?.documentCategory, navigation]); |
There was a problem hiding this comment.
Missing 'kyc' case in navigateToDocumentOnboarding — same issue as ProveScreen.
The switch handles passport, id_card, and aadhaar but not kyc. If a KYC document becomes inactive and the user taps the inactive warning, the modal's "Continue" callback calls navigateToDocumentOnboarding which silently no-ops for KYC.
🤖 Prompt for AI Agents
In `@app/src/components/homescreen/IdCard.tsx` around lines 307 - 317, The switch
in navigateToDocumentOnboarding (inside IdCard.tsx) omits the 'kyc' case so
tapping the inactive-warning "Continue" silently no-ops for KYC documents; add a
case 'kyc' that routes to the same onboarding flow as passport/id_card (e.g.,
navigation.navigate('DocumentOnboarding')), mirroring the fix applied in
ProveScreen, so KYC documents correctly navigate to the onboarding screen.
| const getTruncatedId = (): string => { | ||
| if (idNumber && idNumber.length > 10) { | ||
| return `0x${idNumber.slice(0, 4)}..${idNumber.slice(-3)}`; | ||
| } | ||
| return idNumber ? `0x${idNumber}` : ''; | ||
| }; |
There was a problem hiding this comment.
"0x" prefix on KYC document ID numbers is misleading.
getTruncatedId prepends "0x" to the document ID (e.g., a driver's license number). The 0x prefix conventionally denotes hexadecimal or blockchain addresses, which doesn't apply to government-issued document numbers. This will display something like 0xDL12..678 for a driver's license — confusing for users.
Proposed fix — remove 0x prefix or use a contextual prefix
const getTruncatedId = (): string => {
if (idNumber && idNumber.length > 10) {
- return `0x${idNumber.slice(0, 4)}..${idNumber.slice(-3)}`;
+ return `${idNumber.slice(0, 4)}..${idNumber.slice(-3)}`;
}
- return idNumber ? `0x${idNumber}` : '';
+ return idNumber || '';
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const getTruncatedId = (): string => { | |
| if (idNumber && idNumber.length > 10) { | |
| return `0x${idNumber.slice(0, 4)}..${idNumber.slice(-3)}`; | |
| } | |
| return idNumber ? `0x${idNumber}` : ''; | |
| }; | |
| const getTruncatedId = (): string => { | |
| if (idNumber && idNumber.length > 10) { | |
| return `${idNumber.slice(0, 4)}..${idNumber.slice(-3)}`; | |
| } | |
| return idNumber || ''; | |
| }; |
🤖 Prompt for AI Agents
In `@app/src/components/homescreen/KycIdCard.tsx` around lines 148 - 153, The
getTruncatedId function is prepending "0x" to KYC document IDs which is
misleading for non-hex IDs; update getTruncatedId to stop adding the "0x" prefix
and return either a truncated plain ID (e.g.,
`${idNumber.slice(0,4)}..${idNumber.slice(-3)}`) when length > 10 or the raw
idNumber when shorter, ensuring both branches use the same no-prefix format and
preserving the existing idNumber/null checks.
| const navigateToDocumentOnboarding = useCallback( | ||
| (documentMetadata: DocumentMetadata) => { | ||
| switch (documentMetadata.documentCategory) { | ||
| case 'passport': | ||
| case 'id_card': | ||
| navigate('DocumentOnboarding'); | ||
| break; | ||
| case 'aadhaar': | ||
| navigate('AadhaarUpload', { countryCode: 'IND' }); | ||
| break; | ||
| } | ||
| }, | ||
| [navigate], | ||
| ); |
There was a problem hiding this comment.
Missing 'kyc' case in navigateToDocumentOnboarding.
The switch handles passport, id_card, and aadhaar but silently does nothing for kyc documents. If a KYC document is detected as inactive and the user taps "Continue" in the modal (line 154), the callback fires navigateToDocumentOnboarding which falls through the switch with no action — the user is stuck.
Proposed fix
const navigateToDocumentOnboarding = useCallback(
(documentMetadata: DocumentMetadata) => {
switch (documentMetadata.documentCategory) {
case 'passport':
case 'id_card':
navigate('DocumentOnboarding');
break;
case 'aadhaar':
navigate('AadhaarUpload', { countryCode: 'IND' });
break;
+ case 'kyc':
+ // TODO: navigate to appropriate KYC re-verification flow
+ navigate('Home' as never);
+ break;
}
},
[navigate],
);🤖 Prompt for AI Agents
In `@app/src/screens/verification/ProveScreen.tsx` around lines 123 - 136, The
switch in navigateToDocumentOnboarding ignores documentCategory === 'kyc',
causing no navigation; add a 'kyc' case to the switch that calls navigate(...)
to the correct onboarding screen (e.g., navigate('DocumentOnboarding') or
navigate('KycOnboarding') depending on your routing), so
documentMetadata.documentCategory === 'kyc' will route the user when the modal
"Continue" triggers navigateToDocumentOnboarding; update the switch inside
navigateToDocumentOnboarding accordingly and keep the dependency array
[navigate] unchanged.
| const mapDisclosuresToKycFields = (config: SelfAppDisclosureConfig): KycField[] => { | ||
| const mapping: [keyof SelfAppDisclosureConfig, KycField][] = [ | ||
| ['issuing_state', 'ADDRESS'], | ||
| ['nationality', 'COUNTRY'], | ||
| ['name', 'FULL_NAME'], | ||
| ['passport_number', 'ID_NUMBER'], | ||
| ['date_of_birth', 'DOB'], | ||
| ['gender', 'GENDER'], | ||
| ['expiry_date', 'EXPIRY_DATE'], | ||
| ]; | ||
| return mapping.filter(([key]) => config[key]).map(([_, field]) => field); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find KycField enum/type definition
echo "=== Searching for KycField definition ==="
rg -n "enum\s+KycField|type\s+KycField|interface\s+KycField|KycField\s*=" --type=ts | head -20
echo ""
echo "=== Checking imports in registerInputs.ts ==="
head -40 common/src/utils/circuits/registerInputs.ts
echo ""
echo "=== Searching for ADDRESS field usage ==="
rg -n "ADDRESS" --type=ts -B2 -A2 | head -40Repository: selfxyz/self
Length of output: 3805
🏁 Script executed:
#!/bin/bash
# Find SelfAppDisclosureConfig definition
echo "=== Searching for SelfAppDisclosureConfig ==="
rg -n "SelfAppDisclosureConfig" --type=ts -B5 -A10
echo ""
echo "=== Check context around line 276 in registerInputs.ts ==="
sed -n '260,290p' common/src/utils/circuits/registerInputs.tsRepository: selfxyz/self
Length of output: 25803
🏁 Script executed:
#!/bin/bash
# Find KYC_FIELD_LENGTHS and all available KycFields
echo "=== KYC_FIELD_LENGTHS definition ==="
rg -n "KYC_FIELD_LENGTHS" --type=ts -A20 -B5
echo ""
echo "=== All KycField references ==="
rg -n "KYC_[A-Z_]*(?:INDEX|LENGTH|FIELD)" --type=ts | head -30Repository: selfxyz/self
Length of output: 6473
🏁 Script executed:
#!/bin/bash
# Check how KYC fields are actually used in generateKycDiscloseInputFromData and generateKycRegisterInput
echo "=== generateKycDiscloseInputFromData usage ==="
rg -n "generateKycDiscloseInputFromData" --type=ts -B2 -A10
echo ""
echo "=== Check kyc/generateInputs.ts ==="
rg -n "issuing" --type=ts -iRepository: selfxyz/self
Length of output: 12727
🏁 Script executed:
#!/bin/bash
# Check what data is actually in issuing_state from passport DG1
echo "=== issuing_state in dg1.ts ==="
sed -n '15,50p' common/src/utils/passports/dg1.ts
echo ""
echo "=== Check test examples of issuing_state ==="
rg "issuing_state|issuingCountry|issuingState" --type=ts -B1 -A1 | grep -E "(FRA|USA|UTO)" | head -10Repository: selfxyz/self
Length of output: 3218
The mapping 'issuing_state' → 'ADDRESS' is semantically and typologically incorrect.
issuing_state from passport DG1 is a 3-letter country code (e.g., "FRA", "UTO") representing the document's issuing country. The ADDRESS field in KYC expects a 100-character physical address string. This mismatch will disclose the wrong data—the issuing country will be incorrectly populated as a residential address field, violating both the semantic intent and type constraints.
The codebase has no dedicated ISSUING_COUNTRY field in KYC_FIELD_LENGTHS. Either map issuing_state to an appropriate field or remove it from KYC disclosure if no suitable equivalent exists.
🤖 Prompt for AI Agents
In `@common/src/utils/circuits/registerInputs.ts` around lines 274 - 284, In
mapDisclosuresToKycFields, the ['issuing_state','ADDRESS'] pair is wrong: update
the mapping array (the mapping variable inside mapDisclosuresToKycFields) so
that 'issuing_state' is not mapped to 'ADDRESS' — either remove the
['issuing_state', ...] entry entirely or map it to an appropriate field such as
'COUNTRY' (or add a new 'ISSUING_COUNTRY' KYC field and its length in
KYC_FIELD_LENGTHS and map to that) and ensure you adjust KYC_FIELD_LENGTHS if
you introduce 'ISSUING_COUNTRY'; do not leave issuing_state mapped to ADDRESS.
| if (isKycDocument(documentData)) { | ||
| commitment = generateKycCommitment(documentData, secret); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the kyc/utils file and examine generateKycCommitment implementation
fd -t f "utils" --path "*/kyc/*" | head -20Repository: selfxyz/self
Length of output: 289
🏁 Script executed:
# Search for generateKycCommitment definition and its return type
rg "generateKycCommitment" -A 15 -B 2 --type ts --type tsxRepository: selfxyz/self
Length of output: 83
🏁 Script executed:
# Get the actual validate.ts file content around lines 280-320
fd "validate.ts" | xargs -I {} sed -n '280,320p' {}Repository: selfxyz/self
Length of output: 1403
🏁 Script executed:
# Check for generateKycNullifier usage in validate.ts
rg "generateKycNullifier" -B 2 -A 2Repository: selfxyz/self
Length of output: 761
🏁 Script executed:
# Get the full generateKycCommitment implementation in kyc/utils.ts
rg "export const generateKycCommitment" -A 20Repository: selfxyz/self
Length of output: 1714
🏁 Script executed:
# Check the full context of isUserRegistered function
fd "validate.ts" | xargs -I {} sed -n '270,320p' {}Repository: selfxyz/self
Length of output: 1664
🏁 Script executed:
# Search for all uses of generateKycNullifier in validate.ts
rg "generateKycNullifier" --context 5Repository: selfxyz/self
Length of output: 1616
🏁 Script executed:
# Get the type signatures from the kyc/utils file
fd "kyc.*utils" -e ts | xargs -I {} grep -A 5 "generateKycCommitment\|generateKycNullifier" {}Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Check if generateKycCommitment has an explicit return type
rg "export const generateKycCommitment" -A 1 | head -5Repository: selfxyz/self
Length of output: 237
🏁 Script executed:
# Check complete kyc/utils file to see all function signatures with types
fd "kyc.*utils" -e ts | xargs -I {} grep -B 2 "export const generateKyc" {}Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Verify the context: isKycDocument is checked BEFORE calling generateKycCommitment
sed -n '285,295p' common/src/utils/passports/validate.tsRepository: selfxyz/self
Length of output: 495
🏁 Script executed:
# Check the full signature of generateKycCommitment - does it have explicit return type?
rg "export const generateKycCommitment.*=>|^}" -A 10 common/src/utils/kyc/utils.tsRepository: selfxyz/self
Length of output: 1156
🏁 Script executed:
# Verify that commitment is always assigned before line 312 - check all branches
sed -n '279,315p' common/src/utils/passports/validate.tsRepository: selfxyz/self
Length of output: 1318
🏁 Script executed:
# Look for any TypeScript tsconfig to understand type strictness
fd "tsconfig" -e json | xargs cat | head -50Repository: selfxyz/self
Length of output: 1439
generateKycCommitment lacks explicit return type and implicitly returns undefined, creating a type mismatch.
The function has no explicit return type annotation. It returns commitment.toString() only inside the if (isKycDocument(passportData)) guard, implicitly returning undefined otherwise. This gives it an inferred return type of string | undefined. However, at line 290, it's assigned to commitment: string, which is a type safety violation. While the nested isKycDocument check should protect against undefined in practice, the implicit return violates the declared type contract and creates a maintenance risk if the function is called from elsewhere.
Also remove the unused import: generateKycNullifier is imported on line 32 but never used in this file.
Suggested fixes:
- Add explicit return type:
generateKycCommitment = (...): string => { ... }and handle the non-KYC case explicitly (either throw or return a default). - Remove unused import
generateKycNullifier.
🤖 Prompt for AI Agents
In `@common/src/utils/passports/validate.ts` around lines 290 - 291, The function
generateKycCommitment currently lacks an explicit return type and only returns a
string inside the isKycDocument(passportData) branch, causing an inferred string
| undefined; update generateKycCommitment to have an explicit return type of
string and handle the non-KYC case explicitly (e.g., throw a clear error or
return a defined default) so it never returns undefined, ensuring assignment to
commitment: string is safe; additionally remove the unused import
generateKycNullifier from the top of the file.
| "2.13.0": { | ||
| "initializerVersion": 12, | ||
| "initializerFunction": "", | ||
| "changelog": "Upgrade to v2.13.0", | ||
| "gitTag": "identityverificationhub-v2.13.0", | ||
| "deployments": { | ||
| "celo-sepolia": { | ||
| "impl": "0x244c93516Abd58E1952452d3D8C4Ce7D454776B8", | ||
| "deployedAt": "2026-02-02T14:47:21.882Z", | ||
| "deployedBy": "0x82D8DaC3a386dec55a0a44DffBd3113e8A7D139B", | ||
| "gitCommit": "33bca485" | ||
| } | ||
| } | ||
| }, | ||
| "2.13.0": { | ||
| "initializerVersion": 12, | ||
| "initializerFunction": "", | ||
| "changelog": "Upgrade to v2.13.0", | ||
| "gitTag": "identityverificationhub-v2.13.0", | ||
| "deployments": { | ||
| "celo": { | ||
| "impl": "0x0D911083b2F2236D79EF20bb58AAf6009a1220B5", | ||
| "deployedAt": "2026-02-09T11:26:30.941Z", | ||
| "deployedBy": "0xC1C860804EFdA544fe79194d1a37e60b846CEdeb", | ||
| "gitCommit": "88ae00b1" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Duplicate "2.13.0" key — celo-sepolia deployment data will be silently lost.
JSON does not permit duplicate keys. The second "2.13.0" object (lines 169–182, celo deployment) overwrites the first (lines 155–168, celo-sepolia deployment). This means the celo-sepolia deployment record for v2.13.0 will be dropped at parse time.
Merge both network deployments under a single "2.13.0" key:
🐛 Proposed fix
- "2.13.0": {
- "initializerVersion": 12,
- "initializerFunction": "",
- "changelog": "Upgrade to v2.13.0",
- "gitTag": "identityverificationhub-v2.13.0",
- "deployments": {
- "celo-sepolia": {
- "impl": "0x244c93516Abd58E1952452d3D8C4Ce7D454776B8",
- "deployedAt": "2026-02-02T14:47:21.882Z",
- "deployedBy": "0x82D8DaC3a386dec55a0a44DffBd3113e8A7D139B",
- "gitCommit": "33bca485"
- }
- }
- },
- "2.13.0": {
- "initializerVersion": 12,
- "initializerFunction": "",
- "changelog": "Upgrade to v2.13.0",
- "gitTag": "identityverificationhub-v2.13.0",
- "deployments": {
- "celo": {
- "impl": "0x0D911083b2F2236D79EF20bb58AAf6009a1220B5",
- "deployedAt": "2026-02-09T11:26:30.941Z",
- "deployedBy": "0xC1C860804EFdA544fe79194d1a37e60b846CEdeb",
- "gitCommit": "88ae00b1"
- }
- }
- }
+ "2.13.0": {
+ "initializerVersion": 12,
+ "initializerFunction": "",
+ "changelog": "Upgrade to v2.13.0",
+ "gitTag": "identityverificationhub-v2.13.0",
+ "deployments": {
+ "celo-sepolia": {
+ "impl": "0x244c93516Abd58E1952452d3D8C4Ce7D454776B8",
+ "deployedAt": "2026-02-02T14:47:21.882Z",
+ "deployedBy": "0x82D8DaC3a386dec55a0a44DffBd3113e8A7D139B",
+ "gitCommit": "33bca485"
+ },
+ "celo": {
+ "impl": "0x0D911083b2F2236D79EF20bb58AAf6009a1220B5",
+ "deployedAt": "2026-02-09T11:26:30.941Z",
+ "deployedBy": "0xC1C860804EFdA544fe79194d1a37e60b846CEdeb",
+ "gitCommit": "88ae00b1"
+ }
+ }
+ }🧰 Tools
🪛 Biome (2.3.14)
[error] 155-155: The key 2.13.0 was already declared.
This is where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
🤖 Prompt for AI Agents
In `@contracts/deployments/registry.json` around lines 155 - 182, The JSON
contains duplicate "2.13.0" keys causing the celo-sepolia deployment to be lost;
locate the "2.13.0" entries in the registry JSON and merge their "deployments"
objects so a single "2.13.0" entry contains both "celo-sepolia" and "celo" under
the same "deployments" map (preserve each deployment's impl, deployedAt,
deployedBy, gitCommit, initializerVersion, initializerFunction, changelog and
gitTag fields).
| if (!noNewInitializer) { | ||
| const targetVersionInfoForInit = getVersionInfo(contractId, newVersion); | ||
| const initializerName = targetVersionInfoForInit?.initializerFunction || `initializeV${newInitializerVersion}`; | ||
|
|
||
| try { | ||
| const iface = proxyContract.interface; | ||
| const initFragment = iface.getFunction(initializerName); | ||
| if (initFragment && initFragment.inputs.length === 0) { | ||
| initData = iface.encodeFunctionData(initializerName, []); | ||
| log.detail("Initializer", initializerName); | ||
| } | ||
| } catch { | ||
| log.detail("Initializer", "None"); | ||
| } | ||
| } catch { | ||
| log.detail("Initializer", "None"); | ||
| } else { | ||
| log.detail("Initializer", "None (code-only upgrade)"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.ts" -path "*/contracts/tasks/upgrade/*" | head -5Repository: selfxyz/self
Length of output: 237
🏁 Script executed:
wc -l contracts/tasks/upgrade/upgrade.ts 2>/dev/null || echo "File not found, searching for upgrade files..."
fd -e ts -p "*upgrade*" contracts/Repository: selfxyz/self
Length of output: 401
🏁 Script executed:
# Check if versioned initializer functions exist in contracts
rg -n 'function\s+initializeV\d+' --type=sol 2>/dev/null | head -20Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Also check TypeScript for versioned initializers
rg -n 'initializeV\d+' contracts/tasks/upgrade/upgrade.ts | head -20Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
sed -n '625,640p' contracts/tasks/upgrade/upgrade.tsRepository: selfxyz/self
Length of output: 540
🏁 Script executed:
sed -n '705,730p' contracts/tasks/upgrade/upgrade.tsRepository: selfxyz/self
Length of output: 1074
🏁 Script executed:
# Search more broadly for initializer functions - any pattern
rg -n 'initializer' contracts/tasks/upgrade/upgrade.ts | head -30Repository: selfxyz/self
Length of output: 2041
🏁 Script executed:
# Check if there are any initialize functions in contracts (not versioned)
rg -n 'function\s+initialize\s*\(' --type=sol | head -20Repository: selfxyz/self
Length of output: 83
🏁 Script executed:
# Search for any .sol files first
find . -name "*.sol" -type f 2>/dev/null | head -10Repository: selfxyz/self
Length of output: 585
🏁 Script executed:
# Look for initialize function definitions (not just the word initialize)
rg -n 'function.*initialize' contracts/ --type ts | head -20Repository: selfxyz/self
Length of output: 330
🏁 Script executed:
# Check how getVersionInfo works and what it returns
rg -n 'function getVersionInfo\|const getVersionInfo' contracts/tasks/upgrade/upgrade.ts -A 10Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Check what happens when targetVersionInfoForInit is undefined
rg -n 'getVersionInfo\(' contracts/tasks/upgrade/upgrade.ts -B 2 -A 2Repository: selfxyz/self
Length of output: 615
🏁 Script executed:
# Now search for initialize functions in .sol files
find contracts/contracts -name "*.sol" -type f -exec grep -l "function.*initialize" {} \; 2>/dev/nullRepository: selfxyz/self
Length of output: 957
🏁 Script executed:
# Check a specific contract to see what initializer it has
head -100 contracts/contracts/IdentityVerificationHubImplV1.sol | grep -n "function.*initialize\|reinitializer"Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Look for reinitializer decorator pattern (which is what's being used)
rg -n 'reinitializer' contracts/contracts/ --glob="*.sol" | head -20Repository: selfxyz/self
Length of output: 2646
🏁 Script executed:
# Check the actual initialization logic in contracts
rg -n '@.*initializer|function.*initialize' contracts/contracts/ --glob="*.sol" -A 3 | head -40Repository: selfxyz/self
Length of output: 3682
🏁 Script executed:
# Check the getVersionInfo implementation
rg -n 'function getVersionInfo\|const getVersionInfo\|export.*getVersionInfo' contracts/tasks/upgrade/ -A 15 | head -50Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Also verify: when does getVersionInfo return undefined?
rg -n 'getVersionInfo' contracts/tasks/upgrade/upgrade.ts -B 1 -A 1 | head -30Repository: selfxyz/self
Length of output: 779
🏁 Script executed:
# Check if the fallback initializeV is ever actually used
rg -n 'initializeV' contracts/tasks/upgrade/upgrade.tsRepository: selfxyz/self
Length of output: 177
🏁 Script executed:
# Verify the actual contract structure - do we have functions with parameters?
rg -n 'function initialize\(' contracts/contracts/ --glob="*.sol" -A 2 | head -30Repository: selfxyz/self
Length of output: 2540
🏁 Script executed:
# Check what getVersionInfo is imported from and how it works
rg -n 'getVersionInfo' contracts/tasks/upgrade/utils.ts -B 2 -A 15Repository: selfxyz/self
Length of output: 759
🏁 Script executed:
# Understand the registry structure and when getVersionInfo returns undefined
rg -n 'export.*getVersionInfo\|function.*getVersionInfo' contracts/tasks/upgrade/ -A 10Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Check the test cases to see how initializer encoding is tested
fd -e ts -path "*/test*" -path "*upgrade*" | head -5Repository: selfxyz/self
Length of output: 224
🏁 Script executed:
# Look at the actual initialization call site - is initData used?
sed -n '700,740p' contracts/tasks/upgrade/upgrade.tsRepository: selfxyz/self
Length of output: 1668
🏁 Script executed:
# Check if there are any tests for the initializer encoding logic
find contracts -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "initializeV\|encodeFunctionData" 2>/dev/null | head -5Repository: selfxyz/self
Length of output: 248
🏁 Script executed:
# Look at how initialization is actually done - do we pass parameters?
rg -n 'upgradeToAndCall\|initialize.*address' contracts/tasks/upgrade/upgrade.ts -B 2 -A 2 | head -30Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Check what happens with contracts that have initialize(address)
rg -n 'initialize\(' contracts/contracts/registry/IdentityRegistryImplV1.sol -B 2 -A 5Repository: selfxyz/self
Length of output: 340
🏁 Script executed:
# Verify if the code ever handles parameterized initializers anywhere
rg -n 'inputs.length\|\.inputs\|parameters' contracts/tasks/upgrade/upgrade.ts | head -20Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Check how the initializer is supposed to get its parameters (if any)
rg -n 'encodeFunctionData.*initialize' contracts/tasks/upgrade/upgrade.ts -B 5 -A 5Repository: selfxyz/self
Length of output: 487
🏁 Script executed:
# Verify: do ANY real contracts have parameterized initialize?
rg -n 'function initialize\(' contracts/contracts/ --glob="*.sol" | grep -v 'reinitializer\|initializer)'Repository: selfxyz/self
Length of output: 1056
🏁 Script executed:
# What happens if initialize has parameters - can we pass them somewhere?
sed -n '630,650p' contracts/tasks/upgrade/upgrade.tsRepository: selfxyz/self
Length of output: 902
🏁 Script executed:
# Check the full context of how initialization data gets passed
sed -n '706,745p' contracts/tasks/upgrade/upgrade.tsRepository: selfxyz/self
Length of output: 1519
🏁 Script executed:
# Check which contracts are actually deployed and upgraded
rg -n 'contractId.*IdentityRegistry\|contractId.*IdentityVerification' contracts/tasks/upgrade/upgrade.ts | head -10Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# See what contracts have NO-parameter initialize functions
rg -n 'function initialize\(\)\s*external' contracts/contracts/ --glob="*.sol"Repository: selfxyz/self
Length of output: 476
🏁 Script executed:
# Check if parameterized initializers cause an issue in the upgrade flow
rg -n 'initializerVersion\|newInitializerVersion' contracts/tasks/upgrade/upgrade.ts | head -20Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Verify: when getVersionInfo returns null, does fallback initializeVN actually work?
sed -n '709,730p' contracts/tasks/upgrade/upgrade.ts | cat -nRepository: selfxyz/self
Length of output: 1076
🏁 Script executed:
# Critical check: do parameterized initializers get called anywhere?
rg -n 'initialize.*address.*external.*initializer' contracts/contracts/ --glob="*.sol" -B 2 -A 5 | head -30Repository: selfxyz/self
Length of output: 2728
🏁 Script executed:
# Are there ANY tests that verify initializer is called with parameters?
rg -n 'encodeFunctionData.*initialize' contracts/test/ --glob="*.ts" -B 3 -A 3Repository: selfxyz/self
Length of output: 10461
🏁 Script executed:
# What's the actual behavior when initialize has parameters?
sed -n '714,722p' contracts/tasks/upgrade/upgrade.tsRepository: selfxyz/self
Length of output: 430
Parameterized initializers are silently skipped during upgrades.
The encoding logic on lines 716–721 only encodes initializers that have zero parameters: if (initFragment && initFragment.inputs.length === 0). Contracts like IdentityRegistryKycImplV1 have parameterized initializers—e.g., initialize(address hubAddress, address pcr0ManagerAddress)—which do not match this condition. When upgrading such contracts, the code silently skips encoding the initializer, leaving initData as "0x" and never calling the initializer on the upgraded contract. No error is logged; the upgrade completes with an uninitialized contract state.
Either:
- Support parameterized initializers by storing and retrieving init parameters from the registry, or
- Fail loudly if the initializer has required parameters rather than silently skipping it.
🤖 Prompt for AI Agents
In `@contracts/tasks/upgrade/upgrade.ts` around lines 709 - 725, The upgrade
currently skips parameterized initializers (the block using
proxyContract.interface.getFunction(initializerName) only encodes when
initFragment.inputs.length === 0), leaving initData as "0x" and never calling
required initializers; fix by either (A) supporting parameterized initializers:
read initializer arguments from the target version metadata (e.g., enhance
getVersionInfo(contractId, newVersion) to return a field like initializerArgs or
initializerParams and then call iface.encodeFunctionData(initializerName,
initializerArgs) when initFragment.inputs.length > 0), or (B) fail loudly: if
initFragment.inputs.length > 0 and no initializer args are present, throw or log
an error and abort the upgrade (instead of silently setting initData), making
use of symbols initializerName, initFragment, proxyContract.interface,
getVersionInfo, noNewInitializer, and initData to locate and implement the
change.
4 participants
🚀 Weekly Release to Staging
Release Date: February 12, 2026
Release Branch:
release/staging-2026-02-12This automated PR promotes a snapshot of
devtostagingfor testing.What's Included
All commits merged to
devup to the branch creation time.Note: This PR uses a dedicated release branch, so new commits to
devwill NOT automatically appear here.Review Checklist
Next Steps
After merging, the staging environment will be updated. A production release PR will be created on Sunday.
This PR was automatically created by the Release Calendar workflow on February 12, 2026
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores