Refactor proving machine so it's easier to grok#1526
Closed
transphorm wants to merge 13 commits intodevfrom
Closed
Refactor proving machine so it's easier to grok#1526transphorm wants to merge 13 commits intodevfrom
transphorm wants to merge 13 commits intodevfrom
Conversation
Contributor
📝 WalkthroughWalkthroughModularizes the proving pipeline: extracts document processing, payload generation/encryption, WebSocket/Socket.IO listeners, TEE connection handling, typed state machine, dependency factories, constants, and tests; renames a public circuit type export. Changes
Sequence Diagram(s)mermaid App->>Actor: PARSE_ID_DOCUMENT Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (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 |
* Fix websocket handler test mocks * Limit keychain modal to Android (#1591) * fix formatting
![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: f73b27ff07
ℹ️ 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".
transphorm
changed the title
transphorm
changed the title
Member
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
421-423: Add explicit platform check at enableKeychainErrorModal call site for clarity.The keychain error modal platform gating exists in
showKeychainErrorModal()(line 352:if (Platform.OS !== 'android') return;), but theenableKeychainErrorModal()call on line 423 is unconditional. Per the coding guidelines ("Implement platform-specific handling withPlatform.OSchecks before platform-specific code"), add an explicit platform guard here to make the intent clear:if (Platform.OS === 'android') { selfClient.navigation?.enableKeychainErrorModal?.(); }This improves readability and consistency with the guideline pattern of platform checks preceding platform-specific operations.
🤖 Fix all issues with AI agents
In @packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts:
- Around line 232-237: In the 'disclose' branch where isUserRegistered(...) is
called, avoid the unsafe cast `secret as string`; validate that `secret` is a
non-empty string (or handle null/undefined/Buffer types) before passing it to
isUserRegistered, and if invalid throw or return an explicit error; locate the
call to isUserRegistered(passportData, secret as string, (documentCategory) =>
getCommitmentTree(selfClient, documentCategory)) in documentProcessor.ts and
replace the cast with a checked variable (e.g., ensure typeof secret ===
'string' && secret.length > 0) or convert safely and handle the failure path.
In @packages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts:
- Around line 81-88: In the 'register' branch where generateTEEInputsRegister is
called, you are force-casting secret to string; instead add a null/undefined
guard for the secret variable before calling generateTEEInputsRegister (e.g., if
(!secret) throw or return a handled error), then pass the validated string to
generateTEEInputsRegister; update the case 'register' block around the secret
usage (and keep circuitTypeWithDocumentExtension assignment) to ensure no "null"
string is forwarded.
- Around line 101-106: In the 'disclose' case branch, add a null/undefined check
for the secret variable before passing it to generateTEEInputsDiscloseStateless;
currently only selfApp is validated, so if secret is missing this will throw or
produce invalid inputs — validate secret (e.g., if (!secret) throw new
Error('Secret is required for disclose');) and then call
generateTEEInputsDiscloseStateless(secret as string, ...) to ensure a clear,
early error and prevent downstream failures.
- Line 216: The code uses a non-null assertion on uuid when calling
_buildSubmitRequest(uuid!, encryptedPayload), which can throw or produce invalid
params if uuid is null; update the caller in payloadGenerator.ts to validate
uuid before calling _buildSubmitRequest (e.g., check if uuid is null/undefined
and either throw a clear error or generate/return an appropriate response), or
adjust _buildSubmitRequest to accept a nullable uuid and handle it safely;
reference the uuid variable and the _buildSubmitRequest(...) call to locate
where to add the null check and error handling.
In @packages/mobile-sdk-alpha/src/proving/internal/socketIOListener.ts:
- Around line 114-118: The code in socketIOListener.ts logs the entire incoming
data object on PROVE_FAILURE (console.error(data)), which risks exposing
sensitive proof information; remove that full-object log and instead log only
non-sensitive identifiers or statuses (e.g., result.actorEvent.type, a short
failure code or reason if safe) using the existing logger or console.error, or
redact fields before logging; locate the PROVE_FAILURE branch that checks
result.actorEvent.type === 'PROVE_FAILURE' and replace the console.error(data)
with a minimal, non-sensitive error message referencing result.actorEvent (not
the full data payload).
- Line 95: The code unsafely asserts getState().circuitType as a string when
calling handleStatusCode; replace the assertion with a null-safe guard: read
const circuitType = getState().circuitType and if it is null/undefined either
return/handle the error early or provide a safe default before calling
handleStatusCode(circuitType, ...), or update handleStatusCode to accept string
| null and handle null inside; ensure you reference the getState().circuitType
and handleStatusCode symbols so the change is applied where the result is
computed.
In @packages/mobile-sdk-alpha/src/proving/internal/websocketUrlResolver.ts:
- Line 22: The ternary return currently maps any non-'passport' category to
'DSC_ID', which silently treats 'aadhaar' as DSC_ID; update the resolver
(websocketUrlResolver function) to explicitly handle 'aadhaar' by throwing the
same error as payloadGenerator does, and also validate other unsupported
categories: if documentCategory === 'passport' return 'DSC', if documentCategory
=== 'id' return 'DSC_ID', if documentCategory === 'aadhaar' throw an explicit
error, otherwise throw a generic unsupported documentCategory error so behavior
is consistent with payloadGenerator.
🧹 Nitpick comments (4)
packages/mobile-sdk-alpha/src/proving/internal/socketIOListener.ts (1)
40-44: Consider adding connection timeout for resilience.The Socket.IO connection lacks a timeout configuration. On poor network conditions, this could hang indefinitely, degrading user experience.
Suggested enhancement
const socket: Socket = socketIo(url, { path: '/', transports: ['websocket'], + timeout: 30000, // 30 second connection timeout + reconnection: false, // Explicit - let state machine handle retry });packages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.ts (1)
196-202: Unusual pattern: Synthetic MessageEvent for error propagation.Creating a synthetic
MessageEventwith error JSON to reuse_handleWebSocketMessageis clever but may confuse future maintainers. Consider extracting the error handling into a separate helper for clarity.packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts (1)
273-279: Fire-and-forget async operation may silently fail.The IIFE at lines 273-279 runs
markCurrentDocumentAsRegisteredwithout awaiting. While the error is logged, a failure here won't propagate or affect the flow. Consider if this side-effect is critical enough to warrant proper error handling.Alternative: Log structured event on failure
(async () => { try { await markCurrentDocumentAsRegistered(selfClient); } catch (error) { console.error('Error marking document as registered:', error); + selfClient.trackEvent(ProofEvents.MARK_REGISTERED_FAILED, { + error: error instanceof Error ? error.message : String(error), + }); } })();packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
344-352: RedundantgetStateassignment increateWebSocketDeps.The
socketDepsobject (created viacreateSocketDeps) already includesgetState: get. Spreading it and then immediately re-assigninggetState: getis unnecessary.♻️ Suggested cleanup
const createWebSocketDeps = (selfClient: SelfClient): WebSocketHandlerDeps => { const socketDeps = createSocketDeps(selfClient); return { ...socketDeps, - getState: get, startSocketIOStatusListener: (receivedUuid: string, endpointType: EndpointType, client: SelfClient) => startSocketIOStatusListener(receivedUuid, endpointType, client, socketDeps), }; };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/src/providers/selfClientProvider.tsxpackages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.tspackages/mobile-sdk-alpha/src/proving/internal/socketIOListener.tspackages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tspackages/mobile-sdk-alpha/src/proving/internal/websocketUrlResolver.tspackages/mobile-sdk-alpha/src/proving/provingMachine.tspackages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.ts
🧰 Additional context used
📓 Path-based instructions (16)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,tsx}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
packages/mobile-sdk-alpha/src/proving/internal/websocketUrlResolver.tspackages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tspackages/mobile-sdk-alpha/src/proving/internal/socketIOListener.tspackages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.tsapp/src/providers/selfClientProvider.tsxpackages/mobile-sdk-alpha/src/proving/provingMachine.tspackages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Use React Navigation withcreateStaticNavigationfor type-safe navigation in React Native applications.
Implement platform-specific handling withPlatform.OS === 'ios' ? 'iOS' : 'Android'checks before platform-specific code in React Native.
Initialize native modules withinitializeNativeModules()before any native operations in React Native.
Implement lazy loading for screens usingReact.lazy()in React Native applications.
Implement custom modal system withuseModalhook and callback registry in React Native.
Integrate haptic feedback usinguseHapticNavigationhook in React Native navigation.
Use platform-specific initial routes: web uses 'Home', mobile uses 'Splash' in React Navigation.
Use Zustand for global state management in React Native applications.
Use custom hooks for complex state (useModal,useHapticNavigation) instead of inline logic.
Use AsyncStorage for simple data, SQLite for complex data, and Keychain for sensitive data in React Native.
Use@/alias for src imports and@tests/alias for test imports in TypeScript/JavaScript files.
Use conditional rendering with Platform.OS for platform-specific code in React Native.
Use Tamagui for UI components in React Native applications.
Do not log sensitive data in production, including identity verification and passport information.
Use Keychain for secure storage of sensitive data in React Native.
Implement proper cleanup of sensitive data after use.
Implement certificate validation for passport data verification.
Always use try-catch for async operations in React Native and TypeScript code.
Implement graceful degradation when native modules fail in React Native.
Provide user-friendly error messages in UI and error handlers.
Lazy load screens and components to optimize bundle size in React Native.
Prevent memory leaks in native modules in React Native.
Files:
packages/mobile-sdk-alpha/src/proving/internal/websocketUrlResolver.tspackages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tspackages/mobile-sdk-alpha/src/proving/internal/socketIOListener.tspackages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.tsapp/src/providers/selfClientProvider.tsxpackages/mobile-sdk-alpha/src/proving/provingMachine.tspackages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts
**/*.{tsx,jsx,ts,js}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper cleanup in useEffect and component unmount hooks in React.
Files:
packages/mobile-sdk-alpha/src/proving/internal/websocketUrlResolver.tspackages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tspackages/mobile-sdk-alpha/src/proving/internal/socketIOListener.tspackages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.tsapp/src/providers/selfClientProvider.tsxpackages/mobile-sdk-alpha/src/proving/provingMachine.tspackages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/*.{ts,tsx}: Check types across the codebase by runningyarn types
Runyarn typesoryarn typecheckto check TypeScript types across the codebase
Runyarn buildto build the package for distribution
Before committing changes, ensure TypeScript types are valid by runningyarn types
Before committing changes, ensure the build succeeds by runningyarn build
Before creating a PR, ensureyarn typespasses (TypeScript validation)
Before creating a PR, ensureyarn buildsucceeds (package builds correctly)
Ensure no breaking changes to public API or document them properly
Verify cross-platform compatibility for both React Native and Web environments
This package uses TypeScript with strict type checking
Use ESLint with TypeScript-specific rules
Use platform detectionPlatform.OS === 'web'when adding platform-specific code
Maintain type definitions that are complete and accurate
Files:
packages/mobile-sdk-alpha/src/proving/internal/websocketUrlResolver.tspackages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tspackages/mobile-sdk-alpha/src/proving/internal/socketIOListener.tspackages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.tspackages/mobile-sdk-alpha/src/proving/provingMachine.tspackages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts
packages/mobile-sdk-alpha/**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
Run
yarn lintto check for linting issues oryarn lint:fixto automatically fix them
Files:
packages/mobile-sdk-alpha/src/proving/internal/websocketUrlResolver.tspackages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tspackages/mobile-sdk-alpha/src/proving/internal/socketIOListener.tspackages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.tspackages/mobile-sdk-alpha/src/proving/provingMachine.tspackages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/src/proving/internal/websocketUrlResolver.tspackages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tspackages/mobile-sdk-alpha/src/proving/internal/socketIOListener.tspackages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.tspackages/mobile-sdk-alpha/src/proving/provingMachine.tspackages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.test.{ts,tsx,js,jsx}: UserenderHookfor testing custom React hooks instead of rendering components.
Mock console.error in tests to avoid test output clutter while testing error scenarios.
Test error boundaries and recovery mechanisms in React components.
Mock SQLite operations withexecuteSqlmethod in database tests using utilities fromtests/__setup__/databaseMocks.ts.
**/*.test.{ts,tsx,js,jsx}: Never userequire('react-native')in test files; use ES6importstatements instead to avoid nested require() calls that cause out-of-memory errors in CI/CD pipelines
Never userequire('react')in test files; use ES6import React from 'react'instead to avoid nested require() calls that cause out-of-memory errors
Files:
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.ts
{app,packages/mobile-sdk-alpha}/**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never create nested require('react-native') calls in tests as this causes out-of-memory errors in CI/CD pipelines; use ES6 import statements instead and avoid dynamic require() calls in beforeEach/afterEach hooks
Files:
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.ts
packages/mobile-sdk-alpha/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/*.test.{ts,tsx}: Use real imports from@selfxyz/mobile-sdk-alphain tests instead of mocking the entire package
Write integration tests that exercise real validation logic from the SDK
TestisPassportDataValid()with realistic, synthetic passport data (never use real user data)
VerifyextractMRZInfo()using published sample MRZ strings (e.g., ICAO examples)
EnsureparseNFCResponse()works with representative, synthetic NFC data
Avoid mocking the entire package in Jest setup or replacing real functions with mock implementations
Never use real user PII (personally identifiable information) in tests - use only synthetic, anonymized, or approved test vectors
Use ES6importstatements instead ofrequire()when possible
Avoid dynamicrequire()calls inbeforeEach/afterEachhooks to prevent out-of-memory errors
Prefer top-level imports over nested requires in test files
Use Vitest for testing, not Jest, with setup configuration intests/setup.ts
React Native is mocked intests/setup.tsusingvi.mock()- use imports in test files instead of require()
Test both React Native and Web environments when adding platform-specific code
Files:
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.ts
**/*.{test,spec}.{ts,js,tsx,jsx}
⚙️ CodeRabbit configuration file
**/*.{test,spec}.{ts,js,tsx,jsx}: Review test files for:
- Test coverage completeness
- Test case quality and edge cases
- Mock usage appropriateness
- Test readability and maintainability
Files:
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
Implement comprehensive error boundaries in React components.
Files:
app/src/providers/selfClientProvider.tsx
**/{mobile,client,app,time,verification}/**/*.{ts,tsx,js,swift,kt}
📄 CodeRabbit inference engine (.cursor/rules/compliance-verification.mdc)
Use server-signed time tokens or chain block timestamps for trusted time in mobile clients, do not trust device wall-clock alone
Files:
app/src/providers/selfClientProvider.tsx
**/{mobile,client,app,proof,zk}/**/*.{ts,tsx,js,swift,kt}
📄 CodeRabbit inference engine (.cursor/rules/compliance-verification.mdc)
**/{mobile,client,app,proof,zk}/**/*.{ts,tsx,js,swift,kt}: Include trusted time anchor in proof generation and verify time anchor authenticity before proof generation in mobile implementations
Achieve proof generation in <60 seconds on mid-tier mobile devices
Files:
app/src/providers/selfClientProvider.tsx
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Ensure
yarn typespasses (TypeScript validation) before creating a PR
Files:
app/src/providers/selfClientProvider.tsx
app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
app/**/*.{ts,tsx,js,jsx}: Ensure web build succeeds withyarn webbefore creating a PR
Do not include sensitive data in logs - avoid logging PII, credentials, and tokens
Usereact-native-dotenvfor environment configuration via@envimport
Confirm no sensitive data exposed before PR merge
Files:
app/src/providers/selfClientProvider.tsx
app/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:
- Component architecture and reusability
- State management patterns
- Performance optimizations
- TypeScript type safety
- React hooks usage and dependencies
- Navigation patterns
Files:
app/src/providers/selfClientProvider.tsx
🧠 Learnings (35)
📓 Common learnings
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Address CodeRabbitAI feedback during PR review process
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Document complex native module changes in PRs
Learnt from: CR
Repo: selfxyz/self PR: 0
File: noir/AGENTS.md:0-0
Timestamp: 2025-11-25T14:08:34.041Z
Learning: Verify cryptographic properties and confirm no logical errors are introduced after PR reviews
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-11-25T14:07:28.188Z
Learning: Applies to **/{mobile,client,app,proof,zk}/**/*.{ts,tsx,js,swift,kt} : Achieve proof generation in <60 seconds on mid-tier mobile devices
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Explain platform-specific code paths (iOS/Android/Web) in PR descriptions
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-11-25T14:07:28.188Z
Learning: Applies to **/{mobile,client,app,proof,zk}/**/*.{ts,tsx,js,swift,kt} : Include trusted time anchor in proof generation and verify time anchor authenticity before proof generation in mobile implementations
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Ensure no breaking changes to public API or document them properly
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/websocketUrlResolver.tspackages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tspackages/mobile-sdk-alpha/src/proving/internal/socketIOListener.tspackages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.tsapp/src/providers/selfClientProvider.tsxpackages/mobile-sdk-alpha/src/proving/provingMachine.tspackages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts
📚 Learning: 2025-08-25T14:25:57.586Z
Learnt from: aaronmgdr
Repo: selfxyz/self PR: 951
File: app/src/providers/authProvider.web.tsx:17-18
Timestamp: 2025-08-25T14:25:57.586Z
Learning: The selfxyz/mobile-sdk-alpha/constants/analytics import path is properly configured with SDK exports, Metro aliases, and TypeScript resolution. Import changes from @/consts/analytics to this path are part of valid analytics migration, not TypeScript resolution issues.
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/websocketUrlResolver.ts
📚 Learning: 2025-12-25T19:19:04.954Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Document complex native module changes in PRs
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/src/proving/provingMachine.ts
📚 Learning: 2025-11-18T12:17:14.819Z
Learnt from: seshanthS
Repo: selfxyz/self PR: 1337
File: packages/mobile-sdk-alpha/src/processing/mrz.ts:189-194
Timestamp: 2025-11-18T12:17:14.819Z
Learning: In packages/mobile-sdk-alpha/src/processing/mrz.ts, the checkScannedInfo function and related TD1 extraction/validation logic are only reached on Android. iOS uses native Swift parsing (LiveMRZScannerView.swift) that bypasses this TypeScript layer.
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Verify `extractMRZInfo()` using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Test `isPassportDataValid()` with realistic, synthetic passport data (never use real user data)
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Maintain type definitions that are complete and accurate
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tspackages/mobile-sdk-alpha/src/proving/provingMachine.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Use real imports from `selfxyz/mobile-sdk-alpha` in tests instead of mocking the entire package
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tspackages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.tspackages/mobile-sdk-alpha/src/proving/provingMachine.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/index.{ts,tsx} : Run `yarn validate:exports` to verify that exports are properly configured
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/src/proving/provingMachine.ts
📚 Learning: 2025-11-25T14:06:55.970Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T14:06:55.970Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Implement certificate validation for passport data verification.
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/src/proving/provingMachine.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Write integration tests that exercise real validation logic from the SDK
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.ts
📚 Learning: 2025-11-25T14:06:55.970Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T14:06:55.970Z
Learning: Identity Verification Hub manages multi-step verification process for passports and EU ID cards, handles document attestation through zero-knowledge proofs, and implements verification paths (E-PASSPORT and EU_ID_CARD) in `contracts/contracts/IdentityVerificationHubImplV2.sol`.
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts
📚 Learning: 2025-12-13T18:00:46.963Z
Learnt from: seshanthS
Repo: selfxyz/self PR: 1497
File: app/src/screens/verification/ProveScreen.tsx:125-161
Timestamp: 2025-12-13T18:00:46.963Z
Learning: In app/src/screens/verification/ProveScreen.tsx: The document expiration check using checkDocumentExpiration() is UX-only to prevent wasted gas and provide better user experience. The authoritative expiration validation is enforced in the circuits and smart contracts using trusted time sources (block timestamps), not device clock.
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tspackages/mobile-sdk-alpha/src/proving/provingMachine.ts
📚 Learning: 2025-11-25T14:07:28.188Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-11-25T14:07:28.188Z
Learning: Applies to **/{compliance,ofac,verification,identity}/**/*.{ts,tsx,js,py} : Implement three-tier OFAC verification system: Passport Number Check (direct passport validation), Name + DOB Check (full name with exact date of birth), and Name + Year Check (name with year of birth, defaulting to Jan-01)
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts
📚 Learning: 2025-09-10T14:47:40.945Z
Learnt from: shazarre
Repo: selfxyz/self PR: 1041
File: app/src/providers/passportDataProvider.tsx:297-301
Timestamp: 2025-09-10T14:47:40.945Z
Learning: In app/src/providers/passportDataProvider.tsx: The deleteDocumentDirectlyFromKeychain function is a low-level utility used by the DocumentsAdapter and should not include error handling since callers like deleteDocument() already implement appropriate try/catch with logging for Keychain operations.
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.tsapp/src/providers/selfClientProvider.tsxpackages/mobile-sdk-alpha/src/proving/provingMachine.ts
📚 Learning: 2025-11-25T14:07:28.188Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-11-25T14:07:28.188Z
Learning: Applies to **/{mobile,client,app,proof,zk}/**/*.{ts,tsx,js,swift,kt} : Include trusted time anchor in proof generation and verify time anchor authenticity before proof generation in mobile implementations
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tspackages/mobile-sdk-alpha/src/proving/provingMachine.tspackages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : This package uses TypeScript with strict type checking
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tspackages/mobile-sdk-alpha/src/proving/provingMachine.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Verify cross-platform compatibility for both React Native and Web environments
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tsapp/src/providers/selfClientProvider.tsxpackages/mobile-sdk-alpha/src/proving/provingMachine.ts
📚 Learning: 2025-11-25T14:07:28.188Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-11-25T14:07:28.188Z
Learning: Applies to **/{mobile,client,app,proof,zk}/**/*.{ts,tsx,js,swift,kt} : Achieve proof generation in <60 seconds on mid-tier mobile devices
Applied to files:
packages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tspackages/mobile-sdk-alpha/src/proving/provingMachine.tspackages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Avoid mocking the entire package in Jest setup or replacing real functions with mock implementations
Applied to files:
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : React Native is mocked in `tests/setup.ts` using `vi.mock()` - use imports in test files instead of require()
Applied to files:
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Prefer top-level imports over nested requires in test files
Applied to files:
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.tspackages/mobile-sdk-alpha/src/proving/provingMachine.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Use ES6 `import` statements instead of `require()` when possible
Applied to files:
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.tspackages/mobile-sdk-alpha/src/proving/provingMachine.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Use Vitest for testing, not Jest, with setup configuration in `tests/setup.ts`
Applied to files:
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Avoid dynamic `require()` calls in `beforeEach`/`afterEach` hooks to prevent out-of-memory errors
Applied to files:
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.ts
📚 Learning: 2025-12-25T19:18:22.033Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/test-memory-optimization.mdc:0-0
Timestamp: 2025-12-25T19:18:22.033Z
Learning: Applies to **/tests/setup.ts : Use `vi.mock()` instead of `require()` calls in Vitest setup files; ensure React Native mocks are established once at setup time, not during test execution
Applied to files:
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.ts
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
Repo: selfxyz/self PR: 936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: SelfClientProvider is wrapped in app/App.tsx, providing context for useSelfClient() hook usage throughout the React Native app navigation stacks.
Applied to files:
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.tsapp/src/providers/selfClientProvider.tsxpackages/mobile-sdk-alpha/src/proving/provingMachine.ts
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
Repo: selfxyz/self PR: 936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: The main App.tsx file is located at app/App.tsx (not in app/src), and it properly wraps the entire app with SelfClientProvider at the top of the provider hierarchy, enabling useSelfClient() hook usage throughout all navigation screens.
Applied to files:
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.tsapp/src/providers/selfClientProvider.tsx
📚 Learning: 2025-08-26T14:49:15.210Z
Learnt from: shazarre
Repo: selfxyz/self PR: 936
File: app/src/screens/passport/PassportNFCScanScreen.web.tsx:8-11
Timestamp: 2025-08-26T14:49:15.210Z
Learning: The main App.tsx file is located at app/App.tsx (at the app root), not at app/src/App.tsx, and contains the SelfClientProvider wrapping the entire application.
Applied to files:
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.ts
📚 Learning: 2025-08-26T14:42:45.297Z
Learnt from: aaronmgdr
Repo: selfxyz/self PR: 936
File: app/src/utils/proving/validateDocument.ts:53-54
Timestamp: 2025-08-26T14:42:45.297Z
Learning: In the Self app, native modules (including Keychain) are initialized early in the app startup sequence, before functions like checkAndUpdateRegistrationStates() are called, so additional native module readiness guards in document-related functions are not needed.
Applied to files:
app/src/providers/selfClientProvider.tsx
📚 Learning: 2025-11-25T14:06:55.970Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T14:06:55.970Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Implement platform-specific handling with `Platform.OS === 'ios' ? 'iOS' : 'Android'` checks before platform-specific code in React Native.
Applied to files:
app/src/providers/selfClientProvider.tsx
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Use platform detection `Platform.OS === 'web'` when adding platform-specific code
Applied to files:
app/src/providers/selfClientProvider.tsx
📚 Learning: 2025-11-25T14:06:55.970Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T14:06:55.970Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use Keychain for secure storage of sensitive data in React Native.
Applied to files:
app/src/providers/selfClientProvider.tsx
📚 Learning: 2025-12-25T19:19:04.954Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Explain platform-specific code paths (iOS/Android/Web) in PR descriptions
Applied to files:
app/src/providers/selfClientProvider.tsx
🧬 Code graph analysis (4)
packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts (2)
packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
ProvingState(42-94)common/src/utils/passports/validate.ts (4)
checkDocumentSupported(75-126)isUserRegistered(272-309)isUserRegisteredWithAlternativeCSCA(311-391)isDocumentNullified(236-270)
packages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.ts (2)
packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
ProvingState(42-94)common/src/utils/attest.ts (2)
validatePKIToken(140-201)checkPCR0Mapping(111-138)
packages/mobile-sdk-alpha/src/proving/internal/socketIOListener.ts (2)
packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
ProvingState(42-94)packages/mobile-sdk-alpha/src/proving/internal/statusHandlers.ts (2)
parseStatusMessage(91-105)handleStatusCode(34-86)
app/src/providers/selfClientProvider.tsx (1)
packages/mobile-sdk-demo/tests/mocks/react-native.ts (1)
Platform(14-25)
🔇 Additional comments (13)
app/src/providers/selfClientProvider.tsx (1)
349-352: LGTM! Clean platform gate for Android-only keychain modal.The early return pattern is appropriate here. Consider adding a brief inline comment explaining why this is Android-only (e.g., iOS handles keychain auth differently) for future maintainers.
packages/mobile-sdk-alpha/tests/proving/internal/websocketHandlers.test.ts (1)
67-69: LGTM! Correct module mocking pattern for dependency-injected architecture.The mock correctly targets only
_startSocketIOStatusListenerrather than the entire package, aligning with the coding guidelines.packages/mobile-sdk-alpha/src/proving/internal/websocketUrlResolver.ts (1)
30-35: LGTM! Safe optional chaining for URL resolution.The optional chaining on line 35 handles missing data gracefully by returning
undefined. The calling code should handle this case appropriately.packages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.ts (2)
229-261: LGTM! Clean WebSocket open handler with proper UUID generation and tracking.The hello message construction and event tracking are well-structured.
66-74: DEV flag is properly controlled by the build system; ensure production builds set it to false.The code at line 70 correctly enforces PCR0 validation in production (
if (!__DEV__ && !pcr0Mapping)). However, the direct use of__DEV__at line 66 without a defensive check is inconsistent with the pattern used elsewhere in the codebase (seenativeEvents.native.ts). Consider usingtypeof __DEV__ !== 'undefined' && __DEV__for consistency, or document that the build system guarantees__DEV__ = falsein production bundles.packages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts (1)
63-66: LGTM! Encryption implementation using established utility.The
_encryptPayloadproperly delegates toencryptAES256GCMfrom the common utils.packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts (2)
74-101: Review passport metadata tracking for sensitive data concerns.The analytics tracking includes extensive passport metadata. While most fields appear to be technical metadata (hash functions, algorithms),
country_codeand DSC identifiers could be considered sensitive in aggregate. Verify this aligns with privacy requirements.
41-124: LGTM! Well-structured document parsing with comprehensive error handling and telemetry.The parsing flow properly handles errors, tracks events, and updates state through the dependency-injected interface.
packages/mobile-sdk-alpha/src/proving/provingMachine.ts (5)
24-38: LGTM — Clean modular import structure.The aliased imports for internal functions (
_generatePayload as generatePayload, etc.) clearly distinguish internal implementations from the store's public API methods. Type-only imports are correctly separated.
386-411: LGTM — Handler delegation is clean and consistent.The delegation pattern correctly wires the internal handlers with their dependencies. Creating deps per-call is acceptable given the low frequency of WebSocket lifecycle events during proof generation.
492-497: LGTM — Document processing delegation follows the established pattern.
708-708: LGTM — Payload generation delegation is consistent with the refactoring pattern.
329-367: Well-structured dependency injection architecture.The factory functions (
createSocketDeps,createWebSocketDeps,createPayloadDeps,createDocumentDeps) provide a clean DI pattern that:
- Makes the proving machine testable by allowing mock injection
- Keeps the store focused on state machine orchestration
- Maintains consistent access to
get,set, andgetActorRefThis aligns with the refactoring goal of making the proving machine easier to understand. Based on learnings, ensure no breaking changes to the public API are introduced.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/screens/dev/DevLoadingScreen.tsx (1)
241-254: Type mismatch: selector values don't matchProvingMachineCircuitType.The selector offers
['dsc', 'register', 'aadhaar']butProvingMachineCircuitTypeis defined as'register' | 'dsc' | 'disclose'. This will cause TypeScript errors and incorrect behavior.Suggested fix
{useMemo( () => - (['dsc', 'register', 'aadhaar'] as const).map((item, i) => { + (['dsc', 'register', 'disclose'] as const satisfies readonly ProvingMachineCircuitType[]).map((item, i) => { return ( <Select.Item index={i} key={item} value={item}>
🤖 Fix all issues with AI agents
In `@packages/mobile-sdk-alpha/src/index.ts`:
- Line 46: The public type export was renamed from provingMachineCircuitType to
ProvingMachineCircuitType, which breaks consumers; restore a
backwards-compatible (deprecated) alias by re-exporting the old name from the
new type (e.g., export type { ProvingMachineCircuitType as
provingMachineCircuitType } ) so both ProvingMachineCircuitType and
provingMachineCircuitType are available, and mark the alias as deprecated in a
comment or JSDoc to indicate it will be removed in a future release.
In `@packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts`:
- Line 162: The call to fetchAllTreesAndCircuits uses a non-null assertion on
env (env!) which can throw if env is null; update the surrounding code in
documentProcessor.ts to validate or provide a default for env before calling
fetchAllTreesAndCircuits: check the env variable (or throw a clear error) and
only call fetchAllTreesAndCircuits(selfClient, document, env,
passportData.dsc_parsed!.authorityKeyIdentifier) when env is defined, or
compute/pass a safe default value; also ensure any upstream callers of the
function containing this call handle missing env appropriately so passportData
and selfClient references remain valid.
- Around line 65-72: The code is using a non-null assertion on
parsedPassportData.passportMetadata in documentProcessor.ts (the
passportMetadata! usage) which can throw at runtime; update the logic in the
relevant function (where passportMetadata is read) to defensively check if
parsedPassportData.passportMetadata is present before accessing .dsc, e.g., if
passportMetadata is undefined log a clear error or handle the absence and set
dscObject = {} (or surface a typed error), otherwise extract dsc into dscObject;
ensure you remove the `!` and use a guard that covers both normal and error
paths so documentProcessor continues to behave safely.
In `@packages/mobile-sdk-alpha/src/proving/internal/teeConnectionHandler.ts`:
- Around line 95-139: The returned Promise can hang because actor.subscribe may
never emit 'ready_to_prove' or 'error'; add a connection timeout (e.g., start a
timer when creating the WebSocket) that after a fixed interval calls
handleConnectError with a descriptive message (like 'timeout'), resolves the
promise, and performs cleanup; ensure you clear the timer when
handleConnectSuccess or handleConnectError runs, and also remove the ws event
listeners, close the ws, and unsubscribe from actor in the timeout branch to
mirror the existing success/error cleanup; reference the Promise body, ws,
wsHandlers, handleConnectSuccess, handleConnectError, and the actor.subscribe
unsubscribe logic when implementing this.
In `@packages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.ts`:
- Around line 66-74: The code uses __DEV__ directly in the validation flow
(validatePKIToken, checkPCR0Mapping) which can be undefined in browser bundles;
update the conditional to first check typeof __DEV__ !== 'undefined' before
using it (e.g., replace occurrences of !__DEV__ with typeof __DEV__ !==
'undefined' && !__DEV__), so the PCR0 mapping guard (`if (!__DEV__ &&
!pcr0Mapping) { actor.send({ type: 'CONNECT_ERROR' }); return; }`) behaves
safely across platforms and avoids undefined behavior when __DEV__ is not
present.
In `@packages/mobile-sdk-alpha/src/proving/provingMachine.ts`:
- Around line 223-230: The analytics context hard-codes platform: 'ios' in
provingMachine (context object), causing Android sessions to be mislabeled;
replace that value with the getPlatform() helper from internal/helpers.ts (or
derive via Platform.OS) and ensure getPlatform is imported/available in this
module; update the context assignment (the object with sessionId, userId,
circuitType, currentState, stage) to use platform: getPlatform() so the platform
is computed dynamically.
In
`@packages/mobile-sdk-alpha/tests/proving/internal/teeConnectionHandler.test.ts`:
- Around line 297-299: The test calls wsEventListeners['open']() without the
required Event argument (wsEventListeners is typed as Record<string, (event:
Event) => void>), causing TS2554; update the test to invoke
wsEventListeners['open'] with a mock Event object (e.g., an empty object cast as
Event or a new Event('open')) so the call signature matches, then assert
mockDeps.handleWsOpen was called with mockSelfClient as before (referencing
wsEventListeners['open'], handleWsOpen, and mockSelfClient).
- Around line 49-52: The mockSelfClient used in tests lacks the getSelfAppState
method causing TypeError failures; update the mock object named mockSelfClient
(typed as SelfClient) in teeConnectionHandler.test.ts to include a
getSelfAppState stub (e.g., a vi.fn() that returns the expected shape or a
resolved promise matching production usage) so tests that call getSelfAppState
succeed; ensure the stubbed return value matches how TeeConnectionHandler or
related code consumes it.
🧹 Nitpick comments (7)
packages/mobile-sdk-alpha/src/proving/internal/helpers.ts (1)
42-45:getPlatformomits web platform support.Per the SDK's cross-platform requirement, this function returns only
'ios' | 'android'but doesn't account for web environments wherePlatform.OS === 'web'. If the SDK is used in a web context, callers will incorrectly receive'android'.Consider expanding the return type and handling:
Suggested fix
-export const getPlatform = (): 'ios' | 'android' => (Platform.OS === 'ios' ? 'ios' : 'android'); +export const getPlatform = (): 'ios' | 'android' | 'web' => { + if (Platform.OS === 'ios') return 'ios'; + if (Platform.OS === 'web') return 'web'; + return 'android'; +};Based on learnings, verify cross-platform compatibility for both React Native and Web environments.
packages/mobile-sdk-alpha/src/proving/internal/dependencyFactory.ts (2)
43-44: Minor inefficiency: redundant factory creation on each call.The pattern
createContextFactory(selfClient)(stage, overrides)creates a new curried function every timecreateProofContextis called. Since you're not caching the factory, you could simplify by callingcreateProofContextdirectly.♻️ Suggested simplification
export const createDocumentDeps = ( selfClient: SelfClient, get: GetStateFn, set: SetStateFn, getActor: GetActorFn, ): DocumentProcessorDeps => ({ getState: get, setState: set, getActor, - createProofContext: (stage: string, overrides: Partial<ProofContext> = {}) => - createContextFactory(selfClient)(stage, overrides), + createProofContext: (stage: string, overrides: Partial<ProofContext> = {}) => + createProofContext(selfClient, stage, overrides), });Apply the same pattern to
createPayloadDepsandcreateSocketDeps.
82-88: Minor:getStateis redundantly reassigned.
socketDepsalready containsgetState: getfromcreateSocketDeps, so re-assigning it at line 85 is unnecessary.♻️ Cleaner spread
export const createWebSocketDeps = ( selfClient: SelfClient, get: GetStateFn, set: SetStateFn, getActor: GetActorFn, ): WebSocketHandlerDeps => { const socketDeps = createSocketDeps(selfClient, get, set, getActor); return { ...socketDeps, - getState: get, startSocketIOStatusListener: (receivedUuid: string, endpointType: EndpointType, client: SelfClient) => startSocketIOStatusListener(receivedUuid, endpointType, client, socketDeps), }; };packages/mobile-sdk-alpha/src/proving/internal/actorSubscriptions.ts (2)
110-117: Fire-and-forget async pattern without structured error handling.The IIFE catches the error but only logs it. Consider whether a retry mechanism or user notification would be appropriate since this marks the document registration state persistently.
The
console.errorat line 115 could potentially include sensitive document details depending on whatmarkCurrentDocumentAsRegisteredthrows.♻️ Consider sanitizing error output
(async () => { try { await markCurrentDocumentAsRegistered(selfClient); } catch (error) { //This will be checked and updated when the app launches the next time - console.error('Error marking document as registered:', error); + console.error('Error marking document as registered'); + // Avoid logging full error which may contain document details } })();Based on coding guidelines: avoid logging PII or sensitive identity verification data.
152-158: Hardcoded error values lose diagnostic context.Line 154 passes hardcoded
'error', 'error'strings, while theFAILUREcase at line 148 properly propagateserror_codeandreason. This inconsistency loses potentially useful diagnostic information.♻️ Propagate actual error info
if (state.value === PROVING_STATES.ERROR) { if (currentState.circuitType === 'disclose') { - selfClient.getSelfAppState().handleProofResult(false, 'error', 'error'); + const { error_code, reason } = currentState; + selfClient.getSelfAppState().handleProofResult(false, error_code ?? 'error', reason ?? 'unknown_error'); } // Disable keychain error modal when proving flow ends selfClient.navigation?.disableKeychainErrorModal?.(); }packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts (1)
102-104: Remove debug console.log statements from production code.These log statements expose internal flow details. Use
selfClient.logProofEventfor structured logging instead, or remove them.♻️ Remove or convert to structured logging
- console.log('passport data parsed successfully, storing in keychain'); await storePassportData(selfClient, parsedPassportData); - console.log('passport data stored in keychain'); + selfClient.logProofEvent('info', 'Passport data stored in keychain', context);packages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.ts (1)
229-261: UUID generation and hello message handling looks secure.The
_handleWsOpenfunction:
- Generates a fresh UUID per connection using
v4()- Properly serializes public key for transmission
- Updates state before sending
One consideration: there's no timeout handling if the WebSocket never receives a response to the hello message. The connection could hang indefinitely.
Consider adding a connection timeout that triggers
CONNECT_ERRORif no attestation is received within a reasonable window (e.g., 30 seconds).
|
|
||
| export type { PassportValidationCallbacks } from './validation/document'; | ||
|
|
||
| export type { ProvingMachineCircuitType } from './proving/provingMachine'; |
Contributor
There was a problem hiding this comment.
Keep a backwards-compatible alias for the renamed public type.
Dropping the previous provingMachineCircuitType export is a public API break for SDK consumers. Please keep a deprecated alias (or document the breaking change explicitly). Based on learnings, avoid undocumented public API breaks.
♻️ Suggested compatibility alias
export type { ProvingMachineCircuitType } from './proving/provingMachine';
+/** `@deprecated` Use ProvingMachineCircuitType */
+export type { ProvingMachineCircuitType as provingMachineCircuitType } from './proving/provingMachine';📝 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.
Suggested change
| export type { ProvingMachineCircuitType } from './proving/provingMachine'; | |
| export type { ProvingMachineCircuitType } from './proving/provingMachine'; | |
| /** `@deprecated` Use ProvingMachineCircuitType */ | |
| export type { ProvingMachineCircuitType as provingMachineCircuitType } from './proving/provingMachine'; |
🤖 Prompt for AI Agents
In `@packages/mobile-sdk-alpha/src/index.ts` at line 46, The public type export
was renamed from provingMachineCircuitType to ProvingMachineCircuitType, which
breaks consumers; restore a backwards-compatible (deprecated) alias by
re-exporting the old name from the new type (e.g., export type {
ProvingMachineCircuitType as provingMachineCircuitType } ) so both
ProvingMachineCircuitType and provingMachineCircuitType are available, and mark
the alias as deprecated in a comment or JSDoc to indicate it will be removed in
a future release.
Comment on lines
+65
to
+72
| const passportMetadata = parsedPassportData.passportMetadata!; | ||
| let dscObject; | ||
| try { | ||
| dscObject = { dsc: passportMetadata.dsc }; | ||
| } catch (error) { | ||
| console.error('Failed to parse dsc:', error); | ||
| dscObject = {}; | ||
| } |
Contributor
There was a problem hiding this comment.
Non-null assertion on passportMetadata could cause runtime error.
Line 65 uses passportMetadata! without verifying it exists. If initPassportDataParsing returns data without passportMetadata, this will throw an unclear error.
🔧 Add defensive check
- const passportMetadata = parsedPassportData.passportMetadata!;
+ const passportMetadata = parsedPassportData.passportMetadata;
+ if (!passportMetadata) {
+ throw new Error('Parsed passport data missing metadata');
+ }
let dscObject;🤖 Prompt for AI Agents
In `@packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts` around
lines 65 - 72, The code is using a non-null assertion on
parsedPassportData.passportMetadata in documentProcessor.ts (the
passportMetadata! usage) which can throw at runtime; update the logic in the
relevant function (where passportMetadata is read) to defensively check if
parsedPassportData.passportMetadata is present before accessing .dsc, e.g., if
passportMetadata is undefined log a clear error or handle the absence and set
dscObject = {} (or surface a typed error), otherwise extract dsc into dscObject;
ensure you remove the `!` and use a guard that covers both normal and error
paths so documentProcessor continues to behave safely.
| step: 'protocol_store_fetch', | ||
| document, | ||
| }); | ||
| await fetchAllTreesAndCircuits(selfClient, document, env!, passportData.dsc_parsed!.authorityKeyIdentifier); |
Contributor
There was a problem hiding this comment.
Non-null assertion on env parameter.
Line 162 uses env! which will throw if env is null. Add validation or provide a default.
🔧 Add validation
+ if (!env) {
+ throw new Error('Environment not set');
+ }
await fetchAllTreesAndCircuits(selfClient, document, env!, passportData.dsc_parsed!.authorityKeyIdentifier);📝 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.
Suggested change
| await fetchAllTreesAndCircuits(selfClient, document, env!, passportData.dsc_parsed!.authorityKeyIdentifier); | |
| if (!env) { | |
| throw new Error('Environment not set'); | |
| } | |
| await fetchAllTreesAndCircuits(selfClient, document, env!, passportData.dsc_parsed!.authorityKeyIdentifier); |
🤖 Prompt for AI Agents
In `@packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts` at line
162, The call to fetchAllTreesAndCircuits uses a non-null assertion on env
(env!) which can throw if env is null; update the surrounding code in
documentProcessor.ts to validate or provide a default for env before calling
fetchAllTreesAndCircuits: check the env variable (or throw a clear error) and
only call fetchAllTreesAndCircuits(selfClient, document, env,
passportData.dsc_parsed!.authorityKeyIdentifier) when env is defined, or
compute/pass a safe default value; also ensure any upstream callers of the
function containing this call handle missing env appropriately so passportData
and selfClient references remain valid.
Comment on lines
+95
to
+139
| return new Promise(resolve => { | ||
| const ws = new WebSocket(wsRpcUrl); | ||
|
|
||
| const handleConnectSuccess = () => { | ||
| selfClient.logProofEvent('info', 'TEE connection succeeded', baseContext, { | ||
| duration_ms: Date.now() - startTime, | ||
| }); | ||
| selfClient.trackEvent(ProofEvents.TEE_CONN_SUCCESS); | ||
| resolve(true); | ||
| }; | ||
| const handleConnectError = (msg: string = 'connect_error') => { | ||
| selfClient.logProofEvent('error', 'TEE connection failed', baseContext, { | ||
| failure: 'PROOF_FAILED_CONNECTION', | ||
| error: msg, | ||
| duration_ms: Date.now() - startTime, | ||
| }); | ||
| selfClient.trackEvent(ProofEvents.TEE_CONN_FAILED, { message: msg }); | ||
| resolve(false); | ||
| }; | ||
|
|
||
| // Create stable handler functions | ||
| const wsHandlers: WsHandlers = { | ||
| message: (event: MessageEvent) => handleWebSocketMessage(event, selfClient), | ||
| open: () => handleWsOpen(selfClient), | ||
| error: (error: Event) => handleWsError(error, selfClient), | ||
| close: (event: CloseEvent) => handleWsClose(event, selfClient), | ||
| }; | ||
|
|
||
| setState({ wsConnection: ws, wsHandlers }); | ||
|
|
||
| ws.addEventListener('message', wsHandlers.message); | ||
| ws.addEventListener('open', wsHandlers.open); | ||
| ws.addEventListener('error', wsHandlers.error); | ||
| ws.addEventListener('close', wsHandlers.close); | ||
|
|
||
| const unsubscribe = actor.subscribe(state => { | ||
| if (state.matches('ready_to_prove')) { | ||
| handleConnectSuccess(); | ||
| unsubscribe.unsubscribe(); | ||
| } else if (state.matches('error')) { | ||
| handleConnectError(); | ||
| unsubscribe.unsubscribe(); | ||
| } | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Promise lacks timeout—connection can hang indefinitely.
If the actor never transitions to ready_to_prove or error, this promise never resolves. On mobile, this could leave the user stuck on a loading screen with no feedback.
Consider adding a connection timeout:
Suggested fix
+const TEE_CONNECTION_TIMEOUT_MS = 30000;
+
return new Promise(resolve => {
const ws = new WebSocket(wsRpcUrl);
+
+ const timeoutId = setTimeout(() => {
+ selfClient.logProofEvent('error', 'TEE connection timeout', baseContext, {
+ failure: 'PROOF_FAILED_CONNECTION',
+ duration_ms: Date.now() - startTime,
+ });
+ selfClient.trackEvent(ProofEvents.TEE_CONN_FAILED, { message: 'timeout' });
+ unsubscribe.unsubscribe();
+ resolve(false);
+ }, TEE_CONNECTION_TIMEOUT_MS);
const handleConnectSuccess = () => {
+ clearTimeout(timeoutId);
selfClient.logProofEvent('info', 'TEE connection succeeded', baseContext, {
duration_ms: Date.now() - startTime,
});
selfClient.trackEvent(ProofEvents.TEE_CONN_SUCCESS);
resolve(true);
};
const handleConnectError = (msg: string = 'connect_error') => {
+ clearTimeout(timeoutId);
selfClient.logProofEvent('error', 'TEE connection failed', baseContext, {🤖 Prompt for AI Agents
In `@packages/mobile-sdk-alpha/src/proving/internal/teeConnectionHandler.ts`
around lines 95 - 139, The returned Promise can hang because actor.subscribe may
never emit 'ready_to_prove' or 'error'; add a connection timeout (e.g., start a
timer when creating the WebSocket) that after a fixed interval calls
handleConnectError with a descriptive message (like 'timeout'), resolves the
promise, and performs cleanup; ensure you clear the timer when
handleConnectSuccess or handleConnectError runs, and also remove the ws event
listeners, close the ws, and unsubscribe from actor in the timeout branch to
mirror the existing success/error cleanup; reference the Promise body, ws,
wsHandlers, handleConnectSuccess, handleConnectError, and the actor.subscribe
unsubscribe logic when implementing this.
Comment on lines
+66
to
+74
| const { userPubkey, serverPubkey, imageHash, verified } = validatePKIToken(attestationToken, __DEV__); | ||
|
|
||
| const pcr0Mapping = await checkPCR0Mapping(imageHash); | ||
|
|
||
| if (!__DEV__ && !pcr0Mapping) { | ||
| console.error('PCR0 mapping not found'); | ||
| actor.send({ type: 'CONNECT_ERROR' }); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find build configuration files in the mobile SDK
find packages/mobile-sdk-alpha -maxdepth 2 -type f \( -name "package.json" -o -name "tsconfig.json" -o -name "*.config.*" -o -name ".env*" \) | head -20Repository: selfxyz/self
Length of output: 268
🏁 Script executed:
# Check for __DEV__ usage throughout the mobile SDK
rg "__DEV__" packages/mobile-sdk-alpha --type ts --type tsx -B 2 -A 2Repository: selfxyz/self
Length of output: 83
🏁 Script executed:
# Look at the websocket handlers file to understand context better
head -80 packages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.tsRepository: selfxyz/self
Length of output: 2824
🏁 Script executed:
# Search for __DEV__ definition or import in the mobile SDK (correct file type syntax)
rg "__DEV__" packages/mobile-sdk-alpha -t tsRepository: selfxyz/self
Length of output: 845
🏁 Script executed:
# Read the tsup build config to understand how __DEV__ is handled
cat packages/mobile-sdk-alpha/tsup.config.tsRepository: selfxyz/self
Length of output: 4344
🏁 Script executed:
# Check if __DEV__ is defined globally or in a polyfill file
rg "^(const|let|var|global|define).*__DEV__" packages/mobile-sdk-alpha -t tsRepository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Check package.json to understand SDK exports and platform specificity
cat packages/mobile-sdk-alpha/package.jsonRepository: selfxyz/self
Length of output: 7222
🏁 Script executed:
# Look for any documentation about SDK platform requirements
cat packages/mobile-sdk-alpha/AGENTS.mdRepository: selfxyz/self
Length of output: 7417
Add defensive checks for __DEV__ in cross-platform code to prevent undefined behavior.
The PCR0 mapping validation at line 70 relies on __DEV__ being defined, but in web/browser environments where this SDK is consumed as a bundled package, __DEV__ may be undefined. This could either cause the validation to fail silently (if undefined → falsy) or behave unexpectedly.
Align with the defensive pattern used in nativeEvents.native.ts by checking typeof __DEV__ !== 'undefined' before using it. Alternatively, ensure the build configuration explicitly defines __DEV__ via tsup's esbuildOptions for non-React Native targets.
🤖 Prompt for AI Agents
In `@packages/mobile-sdk-alpha/src/proving/internal/websocketHandlers.ts` around
lines 66 - 74, The code uses __DEV__ directly in the validation flow
(validatePKIToken, checkPCR0Mapping) which can be undefined in browser bundles;
update the conditional to first check typeof __DEV__ !== 'undefined' before
using it (e.g., replace occurrences of !__DEV__ with typeof __DEV__ !==
'undefined' && !__DEV__), so the PCR0 mapping guard (`if (!__DEV__ &&
!pcr0Mapping) { actor.send({ type: 'CONNECT_ERROR' }); return; }`) behaves
safely across platforms and avoids undefined behavior when __DEV__ is not
present.
Comment on lines
+223
to
+230
| context: { | ||
| sessionId: uuid, | ||
| userId: selfClient.getSelfAppState().selfApp?.userId, | ||
| circuitType: get().circuitType || null, | ||
| currentState: get().currentState || 'unknown-state', | ||
| stage: 'startProving', | ||
| platform: 'ios', | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the target file
fd provingMachine.ts packages/mobile-sdk-alphaRepository: selfxyz/self
Length of output: 111
🏁 Script executed:
# Read the file to see the context around lines 223-230
head -n 250 packages/mobile-sdk-alpha/src/proving/provingMachine.ts | tail -n 50Repository: selfxyz/self
Length of output: 2017
🏁 Script executed:
# Check for Platform imports in the file
rg -n "import.*Platform" packages/mobile-sdk-alpha/src/proving/provingMachine.tsRepository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Search for how the platform field is used in analytics/events
rg -n "PROVING_BEGIN_GENERATION|platform.*ios|platform.*android" packages/mobile-sdk-alpha -i --type ts --type tsxRepository: selfxyz/self
Length of output: 83
🏁 Script executed:
# Check if there are other references to the analytics context in this file
rg -n "context:.*{" packages/mobile-sdk-alpha/src/proving/provingMachine.ts -A 10 | head -40Repository: selfxyz/self
Length of output: 141
🏁 Script executed:
# Search for how PROVING_BEGIN_GENERATION event is handled
rg -n "PROVING_BEGIN_GENERATION" packages/mobile-sdk-alpha --type ts -A 5 -B 2Repository: selfxyz/self
Length of output: 2206
🏁 Script executed:
# Check if Platform is imported or used elsewhere in mobile-sdk-alpha
rg -n "Platform\.OS" packages/mobile-sdk-alpha --type tsRepository: selfxyz/self
Length of output: 3186
🏁 Script executed:
# Look at the imports section of provingMachine.ts
head -n 30 packages/mobile-sdk-alpha/src/proving/provingMachine.tsRepository: selfxyz/self
Length of output: 1542
🏁 Script executed:
# Search for platform value expectations in event processing
rg -n "platform.*==|platform.*:.*ios|platform.*:.*android" packages/mobile-sdk-alpha -i --type tsRepository: selfxyz/self
Length of output: 2587
🏁 Script executed:
# Check if this file targets mobile only or cross-platform
rg -n "Platform\.OS|web|isWeb" packages/mobile-sdk-alpha/src/proving/provingMachine.tsRepository: selfxyz/self
Length of output: 97
🏁 Script executed:
# Find ProofContext type definition
rg -n "type ProofContext|interface ProofContext" packages/mobile-sdk-alpha --type ts -A 5Repository: selfxyz/self
Length of output: 457
🏁 Script executed:
# Check how the platform field from events is actually used downstream
rg -n "ProofContext|\.platform" packages/mobile-sdk-alpha/src --type ts -B 2 -A 2 | head -50Repository: selfxyz/self
Length of output: 4111
🏁 Script executed:
# Verify the getPlatform helper is exported and available for import
head -n 50 packages/mobile-sdk-alpha/src/proving/internal/helpers.tsRepository: selfxyz/self
Length of output: 1381
Use getPlatform() helper instead of hard-coding platform: 'ios'.
Android sessions are mislabeled in analytics. The codebase provides a getPlatform() helper in internal/helpers.ts that returns the correct platform value. Use that instead, or derive it dynamically from Platform.OS.
🔧 Proposed fix
-import { PROVING_EVENTS, TIMING } from './internal/constants';
+import { PROVING_EVENTS, TIMING } from './internal/constants';
+import { getPlatform } from './internal/helpers'; context: {
sessionId: uuid,
userId: selfClient.getSelfAppState().selfApp?.userId,
circuitType: get().circuitType || null,
currentState: get().currentState || 'unknown-state',
stage: 'startProving',
- platform: 'ios',
+ platform: getPlatform(),
},🤖 Prompt for AI Agents
In `@packages/mobile-sdk-alpha/src/proving/provingMachine.ts` around lines 223 -
230, The analytics context hard-codes platform: 'ios' in provingMachine (context
object), causing Android sessions to be mislabeled; replace that value with the
getPlatform() helper from internal/helpers.ts (or derive via Platform.OS) and
ensure getPlatform is imported/available in this module; update the context
assignment (the object with sessionId, userId, circuitType, currentState, stage)
to use platform: getPlatform() so the platform is computed dynamically.
…ity-issues Add null guards and secure logging in proving internals
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts`:
- Around line 280-298: The block uses an unsafe cast of csca when calling
reStorePassportDataWithRightCSCA and fires markCurrentDocumentAsRegistered in a
detached IIFE; validate that csca (from isUserRegisteredWithAlternativeCSCA) is
a non-empty string before calling reStorePassportDataWithRightCSCA (handle
null/undefined by branching or early return) and remove the fire-and-forget
pattern: await markCurrentDocumentAsRegistered(selfClient) inside a try/catch,
log/report failures and only proceed to setState({ circuitType: 'register' }),
call selfClient.trackEvent/selfClient.logProofEvent, and actor.send({ type:
'ALREADY_REGISTERED' }) after successful marking (or implement a clear
retry/error path if marking fails) so state and stored data remain consistent.
- Around line 206-209: The call to checkDocumentSupported is passing
deployed_circuits with a non-null assertion (deployed_circuits!) which can hide
an undefined and produce an opaque runtime error; update the getDeployedCircuits
implementation (the arrow passed into checkDocumentSupported) to defensively
handle missing deployed_circuits from
selfClient.getProtocolState()[documentCategory] — either return a clear fallback
(e.g., empty array) or throw an explicit Error with context including the
documentCategory and protocol state when deployed_circuits is undefined — so
checkDocumentSupported receives a predictable value and failures surface with a
clear message; adjust any callers/typing of getDeployedCircuits if needed to
reflect the chosen behavior.
In `@packages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts`:
- Around line 194-203: The call to getPayload is using an incomplete type cast
on circuitTypeWithDocumentExtension; update the call to pass a correctly typed
value by either removing the explicit cast on circuitTypeWithDocumentExtension
so TypeScript infers its full type, or extend the union in the cast to include
'disclose' to match the getPayload signature (RegisterProofType | DscProofType |
DiscloseProofType). Locate the getPayload invocation in payloadGenerator.ts and
change the cast/argument handling for circuitTypeWithDocumentExtension
accordingly so it aligns with the accepted proof types.
- Around line 172-180: Add an explicit null/undefined guard for the circuitType
value before casting it to ProvingMachineCircuitType: check that the local
variable circuitType is non-null and throw or return a clear error (e.g.,
"Missing circuitType" or "Invalid circuit type: null/undefined") instead of
using the blind cast in the call to _generateCircuitInputs; update the same
pattern in teeConnectionHandler where circuitType is cast on line 59. Reference
the circuitType variable and the _generateCircuitInputs call so you locate and
replace the `as ProvingMachineCircuitType` cast with a proper guard and
descriptive error.
- Around line 68-149: The function _generateCircuitInputs currently leaves
variables (inputs, circuitName, endpointType, endpoint,
circuitTypeWithDocumentExtension) untyped which propagates any; define a proper
return type (e.g., interface GenerateCircuitInputsResult { inputs: /*type*/,
circuitName: string, endpointType: /*type*/, endpoint: /*type*/,
circuitTypeWithDocumentExtension: string }) and annotate _generateCircuitInputs
with that return type, adjust the local let declarations or rely on destructured
assignment so each variable is typed, and then remove the downstream as casts in
_generatePayload that were compensating for the implicit any.
In `@packages/mobile-sdk-alpha/src/proving/internal/socketIOListener.ts`:
- Around line 34-38: The setup currently captures const actor = getActor() and
uses that stale closure inside the connect_error handler (and other handlers in
the 57-68 range), causing wrong/disposed actor usage; update the connect_error
handler (and any handlers that reference the outer actor variable) to call
getActor() at event time instead of using the captured actor, then null-check
the returned actor and bail or log consistently (same pattern as disconnect and
status handlers) so the current actor is always used.
- Around line 41-44: The Socket created by socketIo(url, ...) uses default
infinite reconnection; update the options passed to socketIo (the socket
constant in socketIOListener.ts) to include bounded reconnection settings such
as reconnectionAttempts and a connection timeout (and optionally
reconnectionDelay/reconnectionDelayMax) appropriate for mobile so the client
stops retrying indefinitely and avoids battery drain — modify the options object
passed to socketIo(...) to add these keys with sensible mobile-oriented values.
- Line 117: The code uses an unsafe double-cast at selfClient.trackEvent(event
as unknown as keyof typeof ProofEvents) which masks type mismatches; update
handleStatusCode to return analytics typed as (keyof typeof ProofEvents)[] (or a
discriminated union) so callers get proper compile-time checks, or add a runtime
guard before calling trackEvent that checks the event value against
Object.keys(ProofEvents) and only calls selfClient.trackEvent when the value is
a valid keyof typeof ProofEvents; reference symbols: handleStatusCode,
selfClient.trackEvent, and ProofEvents to locate and fix the code paths emitting
and consuming the analytics events.
🧹 Nitpick comments (3)
🤖 Fix all nitpicks with AI agents
Verify each finding against the current code and only fix it if needed. In `@packages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts`: - Around line 68-149: The function _generateCircuitInputs currently leaves variables (inputs, circuitName, endpointType, endpoint, circuitTypeWithDocumentExtension) untyped which propagates any; define a proper return type (e.g., interface GenerateCircuitInputsResult { inputs: /*type*/, circuitName: string, endpointType: /*type*/, endpoint: /*type*/, circuitTypeWithDocumentExtension: string }) and annotate _generateCircuitInputs with that return type, adjust the local let declarations or rely on destructured assignment so each variable is typed, and then remove the downstream as casts in _generatePayload that were compensating for the implicit any. In `@packages/mobile-sdk-alpha/src/proving/internal/socketIOListener.ts`: - Around line 41-44: The Socket created by socketIo(url, ...) uses default infinite reconnection; update the options passed to socketIo (the socket constant in socketIOListener.ts) to include bounded reconnection settings such as reconnectionAttempts and a connection timeout (and optionally reconnectionDelay/reconnectionDelayMax) appropriate for mobile so the client stops retrying indefinitely and avoids battery drain — modify the options object passed to socketIo(...) to add these keys with sensible mobile-oriented values. - Line 117: The code uses an unsafe double-cast at selfClient.trackEvent(event as unknown as keyof typeof ProofEvents) which masks type mismatches; update handleStatusCode to return analytics typed as (keyof typeof ProofEvents)[] (or a discriminated union) so callers get proper compile-time checks, or add a runtime guard before calling trackEvent that checks the event value against Object.keys(ProofEvents) and only calls selfClient.trackEvent when the value is a valid keyof typeof ProofEvents; reference symbols: handleStatusCode, selfClient.trackEvent, and ProofEvents to locate and fix the code paths emitting and consuming the analytics events.packages/mobile-sdk-alpha/src/proving/internal/socketIOListener.ts (2)
41-44: No reconnection limits configured — risky on mobile.socket.io-client defaults to infinite reconnection attempts. On a mobile device with flaky connectivity, this can drain battery and keep the app busy indefinitely. Consider setting
reconnectionAttemptsand/ortimeoutto bounded values appropriate for mobile.Suggested configuration
const socket: Socket = socketIo(url, { path: '/', transports: ['websocket'], + reconnectionAttempts: 5, + reconnectionDelay: 2000, + timeout: 15000, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mobile-sdk-alpha/src/proving/internal/socketIOListener.ts` around lines 41 - 44, The Socket created by socketIo(url, ...) uses default infinite reconnection; update the options passed to socketIo (the socket constant in socketIOListener.ts) to include bounded reconnection settings such as reconnectionAttempts and a connection timeout (and optionally reconnectionDelay/reconnectionDelayMax) appropriate for mobile so the client stops retrying indefinitely and avoids battery drain — modify the options object passed to socketIo(...) to add these keys with sensible mobile-oriented values.
117-117: Unsafe double type assertion masks potential type mismatches.
event as unknown as keyof typeof ProofEventsbypasses TypeScript's type system entirely. IfhandleStatusCodereturns analytics with event names that don't matchProofEventskeys, this will silently pass at compile time and potentially fail at runtime. Consider typing theanalyticsarray inhandleStatusCode's return type withkeyof typeof ProofEventsdirectly, or at minimum use a runtime guard.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mobile-sdk-alpha/src/proving/internal/socketIOListener.ts` at line 117, The code uses an unsafe double-cast at selfClient.trackEvent(event as unknown as keyof typeof ProofEvents) which masks type mismatches; update handleStatusCode to return analytics typed as (keyof typeof ProofEvents)[] (or a discriminated union) so callers get proper compile-time checks, or add a runtime guard before calling trackEvent that checks the event value against Object.keys(ProofEvents) and only calls selfClient.trackEvent when the value is a valid keyof typeof ProofEvents; reference symbols: handleStatusCode, selfClient.trackEvent, and ProofEvents to locate and fix the code paths emitting and consuming the analytics events.packages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts (1)
68-149: Untyped return values weaken downstream type safety.All five variables on line 79 are declared without type annotations, making them implicitly
any. This propagatesanythrough the return value, which is why lines 196–199 in_generatePayloadrequire explicitascasts. Typing the return properly here eliminates those downstream casts.♻️ Suggested approach
Define an explicit return type for
_generateCircuitInputs:+type CircuitInputsResult = { + inputs: Record<string, unknown>; + circuitName: string; + endpointType: EndpointType; + endpoint: string; + circuitTypeWithDocumentExtension: string; +}; + export const _generateCircuitInputs = async ( selfClient: SelfClient, circuitType: ProvingMachineCircuitType, secret: string | undefined | null, passportData: IDDocument, env: 'prod' | 'stg', selfApp: SelfApp | null, -) => { +): Promise<CircuitInputsResult> => {Then the
letdeclarations on line 79 get inferred types and theascasts in_generatePayloadbecome unnecessary.Based on learnings: "Maintain type definitions that are complete and accurate."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts` around lines 68 - 149, The function _generateCircuitInputs currently leaves variables (inputs, circuitName, endpointType, endpoint, circuitTypeWithDocumentExtension) untyped which propagates any; define a proper return type (e.g., interface GenerateCircuitInputsResult { inputs: /*type*/, circuitName: string, endpointType: /*type*/, endpoint: /*type*/, circuitTypeWithDocumentExtension: string }) and annotate _generateCircuitInputs with that return type, adjust the local let declarations or rely on destructured assignment so each variable is typed, and then remove the downstream as casts in _generatePayload that were compensating for the implicit any.
Comment on lines
+206
to
+209
| const isSupported = await checkDocumentSupported(passportData, { | ||
| getDeployedCircuits: (documentCategory: DocumentCategory) => | ||
| selfClient.getProtocolState()[documentCategory].deployed_circuits!, | ||
| }); |
Contributor
There was a problem hiding this comment.
deployed_circuits! non-null assertion may produce an opaque runtime error.
If deployed_circuits is undefined for a given document category, the assertion passes undefined into checkDocumentSupported, likely causing a confusing downstream error rather than a clear failure message.
Proposed fix
const isSupported = await checkDocumentSupported(passportData, {
getDeployedCircuits: (documentCategory: DocumentCategory) => {
+ const circuits = selfClient.getProtocolState()[documentCategory].deployed_circuits;
+ if (!circuits) {
+ throw new Error(`No deployed circuits found for ${documentCategory}`);
+ }
+ return circuits;
- selfClient.getProtocolState()[documentCategory].deployed_circuits!,
},
});📝 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.
Suggested change
| const isSupported = await checkDocumentSupported(passportData, { | |
| getDeployedCircuits: (documentCategory: DocumentCategory) => | |
| selfClient.getProtocolState()[documentCategory].deployed_circuits!, | |
| }); | |
| const isSupported = await checkDocumentSupported(passportData, { | |
| getDeployedCircuits: (documentCategory: DocumentCategory) => { | |
| const circuits = selfClient.getProtocolState()[documentCategory].deployed_circuits; | |
| if (!circuits) { | |
| throw new Error(`No deployed circuits found for ${documentCategory}`); | |
| } | |
| return circuits; | |
| }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts` around
lines 206 - 209, The call to checkDocumentSupported is passing deployed_circuits
with a non-null assertion (deployed_circuits!) which can hide an undefined and
produce an opaque runtime error; update the getDeployedCircuits implementation
(the arrow passed into checkDocumentSupported) to defensively handle missing
deployed_circuits from selfClient.getProtocolState()[documentCategory] — either
return a clear fallback (e.g., empty array) or throw an explicit Error with
context including the documentCategory and protocol state when deployed_circuits
is undefined — so checkDocumentSupported receives a predictable value and
failures surface with a clear message; adjust any callers/typing of
getDeployedCircuits if needed to reflect the chosen behavior.
Comment on lines
+280
to
+298
| if (isRegistered) { | ||
| await reStorePassportDataWithRightCSCA(selfClient, passportData, csca as string); | ||
|
|
||
| (async () => { | ||
| try { | ||
| await markCurrentDocumentAsRegistered(selfClient); | ||
| } catch (error) { | ||
| console.error('Error marking document as registered:', error); | ||
| } | ||
| })(); | ||
| setState({ circuitType: 'register' }); // Update circuit type to 'register' to reflect full registration completion | ||
|
|
||
| selfClient.trackEvent(ProofEvents.ALREADY_REGISTERED); | ||
| selfClient.logProofEvent('info', 'Document already registered', context, { | ||
| duration_ms: Date.now() - startTime, | ||
| }); | ||
| actor.send({ type: 'ALREADY_REGISTERED' }); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Unsafe csca as string cast (line 281) and fire-and-forget async IIFE (lines 283–289).
Two concerns in this block:
-
csca as string— No validation thatcscais a non-empty string. TheisUserRegisteredWithAlternativeCSCAreturn type may allownull/undefinedforcsca, which would silently corrupt the stored passport data. -
Fire-and-forget IIFE —
markCurrentDocumentAsRegisteredruns detached. If it fails, the state machine has already moved toALREADY_REGISTERED, but the document isn't actually marked. This creates a silent inconsistency that could cause repeated re-registration attempts or stale state on next launch.
Proposed fix
if (isRegistered) {
+ if (!csca || typeof csca !== 'string') {
+ throw new Error('Alternative CSCA value is missing or invalid');
+ }
- await reStorePassportDataWithRightCSCA(selfClient, passportData, csca as string);
+ await reStorePassportDataWithRightCSCA(selfClient, passportData, csca);
- (async () => {
- try {
- await markCurrentDocumentAsRegistered(selfClient);
- } catch (error) {
- console.error('Error marking document as registered:', error);
- }
- })();
+ try {
+ await markCurrentDocumentAsRegistered(selfClient);
+ } catch (error) {
+ console.error('Error marking document as registered:', error);
+ // Non-fatal: continue with state transition
+ }
setState({ circuitType: 'register' });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/mobile-sdk-alpha/src/proving/internal/documentProcessor.ts` around
lines 280 - 298, The block uses an unsafe cast of csca when calling
reStorePassportDataWithRightCSCA and fires markCurrentDocumentAsRegistered in a
detached IIFE; validate that csca (from isUserRegisteredWithAlternativeCSCA) is
a non-empty string before calling reStorePassportDataWithRightCSCA (handle
null/undefined by branching or early return) and remove the fire-and-forget
pattern: await markCurrentDocumentAsRegistered(selfClient) inside a try/catch,
log/report failures and only proceed to setState({ circuitType: 'register' }),
call selfClient.trackEvent/selfClient.logProofEvent, and actor.send({ type:
'ALREADY_REGISTERED' }) after successful marking (or implement a clear
retry/error path if marking fails) so state and stored data remain consistent.
Comment on lines
+172
to
+180
| const { inputs, circuitName, endpointType, endpoint, circuitTypeWithDocumentExtension } = | ||
| await _generateCircuitInputs( | ||
| selfClient, | ||
| circuitType as ProvingMachineCircuitType, | ||
| secret, | ||
| passportData, | ||
| env, | ||
| selfClient.getSelfAppState().selfApp, | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the ProvingState type definition
rg -n "type ProvingState\s*=" --type=ts packages/mobile-sdk-alpha/Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Look for PayloadState type definition
rg -n "type PayloadState\s*=" --type=ts packages/mobile-sdk-alpha/Repository: selfxyz/self
Length of output: 151
🏁 Script executed:
# Find the payloadGenerator.ts file and examine context around lines 172-180
cat -n packages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts | head -200 | tail -80Repository: selfxyz/self
Length of output: 2989
🏁 Script executed:
# Find _generateCircuitInputs function to see how it handles circuitType
ast-grep --pattern '_generateCircuitInputs($$$) {
$$$
}'Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Also check for where circuitType comes from in the code
rg -n "circuitType\s*=" --type=ts -B2 -A2 packages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts | head -100Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Get the full PayloadState type definition
sed -n '39,60p' packages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.tsRepository: selfxyz/self
Length of output: 643
🏁 Script executed:
# Search for ProvingMachineCircuitType definition
rg -n "type ProvingMachineCircuitType\|ProvingMachineCircuitType\s*=" --type=ts packages/mobile-sdk-alpha/ | head -20Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Get the full _generateCircuitInputs function signature and initial lines
rg -n "const _generateCircuitInputs\|export const _generateCircuitInputs\|function _generateCircuitInputs" --type=ts -A 10 packages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.tsRepository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Check the ProvingState type in the types file
fd -t f "*.ts" packages/mobile-sdk-alpha/src/proving/ -name "*types*" -exec cat -n {} \;Repository: selfxyz/self
Length of output: 286
🏁 Script executed:
# Search for ProvingState type definition
rg -n "type ProvingState\s*=" --type=ts packages/mobile-sdk-alpha/Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
# Search for ProvingMachineCircuitType definition
rg -n "ProvingMachineCircuitType" --type=ts packages/mobile-sdk-alpha/ | head -20Repository: selfxyz/self
Length of output: 1227
🏁 Script executed:
# List all files in proving directory to find types
find packages/mobile-sdk-alpha/src/proving -name "*.ts" -type fRepository: selfxyz/self
Length of output: 1011
🏁 Script executed:
# Get the full ProvingState type definition from types.ts
cat -n packages/mobile-sdk-alpha/src/proving/types.ts | head -40Repository: selfxyz/self
Length of output: 1785
🏁 Script executed:
# Check if there are other places where circuitType is cast without guard
rg -n "circuitType as ProvingMachineCircuitType" --type=ts packages/mobile-sdk-alpha/Repository: selfxyz/self
Length of output: 325
🏁 Script executed:
# Check line 59 in teeConnectionHandler.ts context
sed -n '50,70p' packages/mobile-sdk-alpha/src/proving/internal/teeConnectionHandler.tsRepository: selfxyz/self
Length of output: 975
Add explicit null check before casting circuitType to ProvingMachineCircuitType.
The type definition confirms circuitType: ProvingMachineCircuitType | null in ProvingState (line 26 of types.ts). While other required fields (passportData, env, sharedKey) have explicit guards at lines 162–170, circuitType lacks one. The as ProvingMachineCircuitType cast on line 175 silently bypasses the type system and will produce a less helpful error message ("Invalid circuit type:null") if the value is nullish.
🛡️ Proposed fix
+ if (!circuitType) {
+ throw new Error('Circuit type is not set');
+ }
const { inputs, circuitName, endpointType, endpoint, circuitTypeWithDocumentExtension } =
await _generateCircuitInputs(
selfClient,
- circuitType as ProvingMachineCircuitType,
+ circuitType,Also apply the same fix to teeConnectionHandler.ts line 59 where the identical pattern occurs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts` around
lines 172 - 180, Add an explicit null/undefined guard for the circuitType value
before casting it to ProvingMachineCircuitType: check that the local variable
circuitType is non-null and throw or return a clear error (e.g., "Missing
circuitType" or "Invalid circuit type: null/undefined") instead of using the
blind cast in the call to _generateCircuitInputs; update the same pattern in
teeConnectionHandler where circuitType is cast on line 59. Reference the
circuitType variable and the _generateCircuitInputs call so you locate and
replace the `as ProvingMachineCircuitType` cast with a proper guard and
descriptive error.
Comment on lines
+194
to
+203
| const payload = getPayload( | ||
| inputs, | ||
| circuitTypeWithDocumentExtension as 'register_id' | 'dsc_id' | 'register' | 'dsc', | ||
| circuitName as string, | ||
| endpointType as EndpointType, | ||
| endpoint as string, | ||
| selfApp?.version, | ||
| userDefinedData, | ||
| selfApp?.selfDefinedData ?? '', | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'function getPayload|export.*getPayload|const getPayload' --type=ts -A10 -B2 packages/mobile-sdk-alpha/ | head -80Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
cat -n packages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts | sed -n '130,210p'Repository: selfxyz/self
Length of output: 2953
🏁 Script executed:
rg -n "circuitType.*=.*'disclose'" --type=ts packages/mobile-sdk-alpha/ -B3 -A3Repository: selfxyz/self
Length of output: 6679
🏁 Script executed:
rg -n 'export.*function getPayload|export const getPayload' --type=ts -A15 packages/mobile-sdk-alpha/Repository: selfxyz/self
Length of output: 38
🏁 Script executed:
rg -n 'export.*getPayload|const getPayload' --type=ts -B5 -A15 | grep -v node_modules | head -100Repository: selfxyz/self
Length of output: 1307
🏁 Script executed:
fd 'payloadGenerator' --type=f packages/mobile-sdk-alpha/ | xargs rg -n 'getPayload' -B2 -A10Repository: selfxyz/self
Length of output: 12267
Remove incomplete type cast on circuitTypeWithDocumentExtension.
The cast to 'register_id' | 'dsc_id' | 'register' | 'dsc' is incomplete. The getPayload function signature at common/src/utils/proving.ts:62-71 accepts RegisterProofType | DscProofType | DiscloseProofType, which includes 'disclose' as a valid value. Line 135 sets this variable to 'disclose' for disclosure flows, and the test at packages/mobile-sdk-alpha/tests/proving/internal/payloadGenerator.test.ts:241-250 confirms the function is called with this value. Either remove the cast entirely to rely on proper type inference, or update the union to include all accepted proof types.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/mobile-sdk-alpha/src/proving/internal/payloadGenerator.ts` around
lines 194 - 203, The call to getPayload is using an incomplete type cast on
circuitTypeWithDocumentExtension; update the call to pass a correctly typed
value by either removing the explicit cast on circuitTypeWithDocumentExtension
so TypeScript infers its full type, or extend the union in the cast to include
'disclose' to match the getPayload signature (RegisterProofType | DscProofType |
DiscloseProofType). Locate the getPayload invocation in payloadGenerator.ts and
change the cast/argument handling for circuitTypeWithDocumentExtension
accordingly so it aligns with the accepted proof types.
Comment on lines
+34
to
+38
| const actor = getActor(); | ||
| if (!actor) { | ||
| console.error('Cannot start Socket.IO listener: Actor not available.'); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Stale actor reference used in connect_error — inconsistent with other handlers.
Line 34 captures actor at setup time, and it's used directly on line 66 inside the connect_error handler. Meanwhile, disconnect (line 71) and status (line 98, 127) correctly call getActor() to get the current actor. If the actor is swapped between setup and the error event firing, the stale closure sends to the wrong (or disposed) actor.
Suggested fix
socket.on('connect_error', error => {
console.error('SocketIO connection error:', error);
+ const currentActor = getActor();
selfClient.trackEvent(ProofEvents.SOCKETIO_CONNECT_ERROR, {
message: error instanceof Error ? error.message : String(error),
});
selfClient.logProofEvent('error', 'Socket.IO connection error', context, {
failure: 'PROOF_FAILED_CONNECTION',
error: error instanceof Error ? error.message : String(error),
});
- actor.send({ type: 'PROVE_ERROR' });
+ currentActor?.send({ type: 'PROVE_ERROR' });
setState({ socketConnection: null });
});Also applies to: 57-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/mobile-sdk-alpha/src/proving/internal/socketIOListener.ts` around
lines 34 - 38, The setup currently captures const actor = getActor() and uses
that stale closure inside the connect_error handler (and other handlers in the
57-68 range), causing wrong/disposed actor usage; update the connect_error
handler (and any handlers that reference the outer actor variable) to call
getActor() at event time instead of using the captured actor, then null-check
the returned actor and bail or log consistently (same pattern as disconnect and
status handlers) so the current actor is always used.
Member
Author
|
closing in favor of this updated pr |
transphorm
deleted the
codex/refactor-provingmachine.ts-for-maintainability
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Related
Testing
Codex Task
Summary by CodeRabbit
New Features
Refactor
Tests
Chores