feat: add TEE input helpers #888
Conversation
WalkthroughAdds zk-kit and shared workspace dependencies. Extends the mobile SDK public API by exporting two new TEE input generators. Implements generateTEEInputsDisclose and generateTEEInputsRegister to assemble circuit inputs and endpoint metadata. Introduces unit tests for both functions with mocked dependencies. No removals or breaking API changes. Changes
Sequence Diagram(s)sequenceDiagram
actor App
participant Disclose as generateTEEInputsDisclose
participant Utils as Hash/Selectors Utils
participant Trees as SMT/LeanIMT
participant Circuit as generateCircuitInputsVCandDisclose
App->>Disclose: (secret, passportData, selfApp, ofacTrees, commitmentTree)
Disclose->>Utils: calculateUserIdentifierHash / hashEndpointWithScope
Disclose->>Utils: build DG1 selectors (doc-type specific)
Disclose->>Trees: Import OFAC SMTs + commitment LeanIMT
Disclose->>Circuit: generateCircuitInputsVCandDisclose(...)
Circuit-->>Disclose: inputs
Disclose-->>App: { inputs, circuitName, endpointType, endpoint }
sequenceDiagram
actor App
participant Register as generateTEEInputsRegister
participant Circuit as generateCircuitInputsRegister
participant Utils as getCircuitNameFromPassportData
App->>Register: (secret, passportData, dscTree, env)
Register->>Circuit: generateCircuitInputsRegister(...)
Circuit-->>Register: inputs
Register->>Utils: getCircuitNameFromPassportData(passportData, "register")
Register-->>App: { inputs, circuitName, endpointType(env), endpoint }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
packages/mobile-sdk-alpha/src/index.ts (1)
40-41: Optional: Export typed return aliases for API stability.Consider exporting a shared return type (e.g., TEEInputsResponse) from the helpers and re-export it here. It hardens API contracts and improves DX.
packages/mobile-sdk-alpha/tests/proving/discloseInputs.test.ts (1)
26-53: Expand coverage: test non-passport flow, age/OFAC selectors, and tree/SMT wiring.Current test validates the happy-path for passport. Add:
- An id_card path asserting circuitName = 'vc_and_disclose_id'.
- Minimum age selector toggling (selector_older_than and majority).
- OFAC selector on/off behavior.
- Basic assertions that LeanIMT.import is invoked with a hash function and commitmentTree and SMT instances import provided trees.
Apply this diff to enhance coverage (imports plus additional tests):
@@ -import { generateTEEInputsDisclose } from '../../src'; +import { generateTEEInputsDisclose } from '../../src'; +import { LeanIMT } from '@openpassport/zk-kit-lean-imt'; +import { SMT } from '@openpassport/zk-kit-smt'; @@ it('builds disclose inputs with endpoint info', () => { @@ }); + + it('builds disclose inputs for id_card (no passport SMT)', () => { + const passportData = { documentCategory: 'id_card' } as PassportData; + const selfApp = { + scope: 's', + disclosures: {}, + endpoint: 'https://e', + endpointType: 'https', + userId: 'u', + userDefinedData: '0x0', + chainID: 1, + } as any; + const ofacTrees = { nameAndDob: [], nameAndYob: [] } as any; + const commitmentTree = [['0']]; + const result = generateTEEInputsDisclose('sec', passportData, selfApp as SelfApp, ofacTrees, commitmentTree as any); + expect(result.circuitName).toBe('vc_and_disclose_id'); + expect(result.endpointType).toBe('https'); + expect(result.endpoint).toBe('https://e'); + }); + + it('respects minimumAge and ofac disclosures', () => { + const passportData = { documentCategory: 'passport' } as PassportData; + const selfApp = { + scope: 's', + disclosures: { minimumAge: 21, ofac: true, excludedCountries: ['IR'] }, + endpoint: 'https://e', + endpointType: 'https', + userId: 'u', + userDefinedData: '0x0', + chainID: 1, + } as any; + const ofacTrees = { + passportNoAndNationality: [], + nameAndDob: [], + nameAndYob: [], + } as any; + const commitmentTree = [['0']]; + const result = generateTEEInputsDisclose('sec', passportData, selfApp as SelfApp, ofacTrees, commitmentTree as any); + expect(result.circuitName).toBe('vc_and_disclose'); + // Lightly assert internals wiring without over-coupling: + expect(LeanIMT.import).toHaveBeenCalledWith(expect.any(Function), commitmentTree); + expect((SMT as any).mock.calls.length).toBeGreaterThanOrEqual(3); + });packages/mobile-sdk-alpha/tests/proving/registerInputs.test.ts (1)
13-26: Add prod-path test to cover endpointType mapping.Currently only 'stg' is covered. Add a test with env 'prod' expecting endpointType 'celo'.
Apply this diff to add the extra case:
@@ describe('generateTEEInputsRegister', () => { it('builds register inputs with endpoint info', () => { @@ }); + + it('maps env=prod to celo endpointType', () => { + const passportData = { documentCategory: 'passport' } as PassportData; + const result = generateTEEInputsRegister('sec', passportData, 'tree', 'prod'); + expect(result.endpointType).toBe('celo'); + expect(result.endpoint).toBe('https://self.xyz'); + }); });packages/mobile-sdk-alpha/src/proving/registerInputs.ts (1)
10-14: Consider centralizing endpoint/env mapping.If other helpers (e.g., disclose) expose the same metadata, factor endpoint and endpointType resolution into a shared util to avoid drift.
packages/mobile-sdk-alpha/src/proving/discloseInputs.ts (2)
88-100: Consider extracting the special attributes list as a constantThe list of special attributes
['ofac', 'excludedCountries', 'minimumAge']is duplicated in bothgetSelectorDg1PassportandgetSelectorDg1IdCard. This violates the DRY principle.+const SPECIAL_DISCLOSURE_ATTRIBUTES = ['ofac', 'excludedCountries', 'minimumAge'] as const; + function getSelectorDg1Passport(disclosures: SelfAppDisclosureConfig) { const selector_dg1 = Array(88).fill('0'); Object.entries(disclosures).forEach(([attribute, reveal]) => { - if (['ofac', 'excludedCountries', 'minimumAge'].includes(attribute)) { + if (SPECIAL_DISCLOSURE_ATTRIBUTES.includes(attribute as any)) { return; }And apply the same change to
getSelectorDg1IdCardon line 105.
44-52: Memory optimization opportunity for SMT initializationThe SMT objects are always created even when not needed (e.g.,
passportNoAndNationalitySMTfor ID cards). Consider lazy initialization.- let passportNoAndNationalitySMT: SMT | null = null; - const nameAndDobSMT = new SMT(poseidon2, true); - const nameAndYobSMT = new SMT(poseidon2, true); - if (document === 'passport') { - passportNoAndNationalitySMT = new SMT(poseidon2, true); - passportNoAndNationalitySMT.import(ofacTrees.passportNoAndNationality); - } - nameAndDobSMT.import(ofacTrees.nameAndDob); - nameAndYobSMT.import(ofacTrees.nameAndYob); + // Initialize SMTs only when needed + const nameAndDobSMT = new SMT(poseidon2, true); + nameAndDobSMT.import(ofacTrees.nameAndDob); + + const nameAndYobSMT = new SMT(poseidon2, true); + nameAndYobSMT.import(ofacTrees.nameAndYob); + + let passportNoAndNationalitySMT: SMT | null = null; + if (document === 'passport') { + passportNoAndNationalitySMT = new SMT(poseidon2, true); + passportNoAndNationalitySMT.import(ofacTrees.passportNoAndNationality); + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
packages/mobile-sdk-alpha/package.json(1 hunks)packages/mobile-sdk-alpha/src/index.ts(1 hunks)packages/mobile-sdk-alpha/src/proving/discloseInputs.ts(1 hunks)packages/mobile-sdk-alpha/src/proving/registerInputs.ts(1 hunks)packages/mobile-sdk-alpha/tests/proving/discloseInputs.test.ts(1 hunks)packages/mobile-sdk-alpha/tests/proving/registerInputs.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/registerInputs.test.tspackages/mobile-sdk-alpha/tests/proving/discloseInputs.test.ts
🧬 Code Graph Analysis (4)
packages/mobile-sdk-alpha/tests/proving/registerInputs.test.ts (2)
packages/mobile-sdk-alpha/src/index.ts (1)
generateTEEInputsRegister(41-41)packages/mobile-sdk-alpha/src/proving/registerInputs.ts (1)
generateTEEInputsRegister(4-15)
packages/mobile-sdk-alpha/src/proving/registerInputs.ts (1)
packages/mobile-sdk-alpha/src/index.ts (1)
generateTEEInputsRegister(41-41)
packages/mobile-sdk-alpha/tests/proving/discloseInputs.test.ts (2)
packages/mobile-sdk-alpha/src/index.ts (1)
generateTEEInputsDisclose(40-40)packages/mobile-sdk-alpha/src/proving/discloseInputs.ts (1)
generateTEEInputsDisclose(21-77)
packages/mobile-sdk-alpha/src/proving/discloseInputs.ts (1)
packages/mobile-sdk-alpha/src/index.ts (1)
generateTEEInputsDisclose(40-40)
🪛 GitHub Check: lint
packages/mobile-sdk-alpha/tests/proving/discloseInputs.test.ts
[warning] 44-44:
Delete ··
[warning] 37-37:
Delete ··
[warning] 36-36:
Delete ··
[warning] 35-35:
Delete ··
[warning] 34-34:
Delete ··
[warning] 33-33:
Delete ··
[warning] 32-32:
Delete ··
[warning] 31-31:
Replace ········ with ······
[warning] 30-30:
Delete ··
[warning] 29-29:
Delete ··
🪛 GitHub Actions: Mobile SDK CI
packages/mobile-sdk-alpha/tests/proving/discloseInputs.test.ts
[warning] 1-1: Prettier formatting issues detected. Run 'prettier --write' to fix.
🔇 Additional comments (3)
packages/mobile-sdk-alpha/package.json (1)
56-56: Confirm workspace dependency publishability."@selfxyz/common": "workspace:*" will be rewritten on publish only if it’s part of the same release and versioned in lockstep. Ensure:
- @selfxyz/common is published publicly alongside this package.
- validate-exports/verify-conditions scripts catch mismatches in version publishing.
Would you like a script that checks for "workspace:*" across publishable packages and ensures corresponding public versions exist?
packages/mobile-sdk-alpha/src/index.ts (1)
40-41: LGTM: Publicly re-exporting the TEE input helpers.The barrel now surfaces generateTEEInputsDisclose and generateTEEInputsRegister as part of the public API. This aligns with the PR objective.
packages/mobile-sdk-alpha/tests/proving/discloseInputs.test.ts (1)
29-37: Fix Prettier/ESLint formatting to unblock CI.The pipeline flagged spacing/indentation. Run:
- yarn workspace @selfxyz/mobile-sdk-alpha nice
- or: yarn lint:fix && yarn fmt:fix
Here’s a minimal diff reducing indentation around the selfApp and result lines:
- const selfApp = { + const selfApp = { @@ - const result = generateTEEInputsDisclose('sec', passportData, selfApp as SelfApp, ofacTrees, commitmentTree as any); + const result = generateTEEInputsDisclose('sec', passportData, selfApp as SelfApp, ofacTrees, commitmentTree as any);Also applies to: 44-44
| "@openpassport/zk-kit-lean-imt": "^0.0.6", | ||
| "@openpassport/zk-kit-smt": "^0.0.1", | ||
| "@selfxyz/common": "workspace:*", | ||
| "poseidon-lite": "^0.2.0", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify RN/Hermes compatibility and bundling for new crypto-heavy deps (poseidon-lite, zk-kit IMT/SMT).
These packages are BigInt- and hashing-heavy. In React Native, some crypto libs target Node or require specific polyfills. Confirm they are ESM-friendly for tsup, tree-shakeable for web, and don’t rely on Node built-ins at runtime.
- Risk areas:
- poseidon-lite runtime in RN (Hermes BigInt support and any WASM/native bindings).
- @openpassport/zk-kit-lean-imt and @openpassport/zk-kit-smt bundling in RN/Web (no Node APIs).
- Side-effect-free modules for tree-shaking under "sideEffects": false.
Please run the test matrix on both RN and web targets. If helpful, here are web queries to cross-check RN support:
🌐 Web query:
Is the npm package "poseidon-lite" compatible with React Native (Hermes) without Node polyfills?
💡 Result:
The npm package poseidon-lite is not confirmed to be compatible with React Native running on Hermes without Node polyfills; available information indicates likely incompatibility without extra configuration.
Key details:
- The npm documentation[6] for poseidon-lite does not specify React Native (Hermes) support or note whether it avoids dependencies on Node.js built-in modules. This is critical because most npm cryptography libraries—Poseidon is a hashing algorithm—commonly depend on modules like
Buffer,crypto, or others that are not available in React Native or Hermes environments without polyfills. - Hermes is a JavaScript engine optimized for React Native and is designed for improved launch times and memory usage, but it intentionally omits Node.js APIs and browser shims[1][3][4][5][7]. React Native (with Hermes) does not provide standard Node.js modules natively.
- If a package like poseidon-lite references any Node core modules (e.g.,
crypto,stream,Bufferin Node mode), it will not work out-of-the-box with React Native unless you install and correctly configure suitable polyfills or shims for these modules. Doing so is possible, but can be complex and may not always guarantee security or performance. - Search results and the official React Native and Hermes documentation do not list poseidon-lite as a compatible or officially supported package[1][2][3][4][5][7].
Recommended next steps:
- Inspect poseidon-lite dependencies and source code: If it directly or indirectly imports any Node.js built-in modules, polyfills are required. Without them, it is incompatible with React Native (Hermes).
- Alternative: Look for cryptographic libraries explicitly written for (and tested on) React Native with Hermes, such as those based purely on JavaScript or using the
expo-cryptomodule or other React Native-specific cryptography solutions. - If you must use poseidon-lite, prepare for significant manual configuration of polyfills, and note that this may impact your app’s security, performance, or maintenance burden.
Summary:
poseidon-lite is not natively compatible with React Native (Hermes) without Node polyfills, and there is no official support or ready-to-use solution indicated in upstream documentation[6]. Before using it, audit its code for Node dependencies and evaluate alternatives designed for React Native/Hermes environments.
Citations:
- 1: https://reactnative.dev/architecture/bundled-hermes
- 2: https://www.infoq.com/news/2021/03/react-native-064-hermes/
- 3: https://reactnative.dev/docs/0.73/hermes
- 4: https://reactnative.dev/docs/hermes
- 5: https://www.itpathsolutions.com/react-native-hermes-the-default-engine/
- 6: https://npmjs.com/poseidon-lite
- 7: https://retool.com/blog/hermes-react-native-same-league-swift-kotlin
- 8: https://reactnative.dev/blog/2021/10/26/toward-hermes-being-the-default
- 9: https://github.com/0i0/deepme-crawler/blob/master/db/db.p.csv
- 10: [android] Hermes compatibility invertase/react-native-firebase#5424
🌐 Web query:
Do "@openpassport/zk-kit-lean-imt" and "@openpassport/zk-kit-smt" support bundling in React Native environments?
💡 Result:
Short answer
- There’s no published statement that either package explicitly targets React Native. Both are TypeScript / JavaScript libraries published to npm, and LeanIMT is generally usable in JS environments (so it will usually bundle in React Native). (npmjs.com, github.com)
- Sparse Merkle Tree (smt) in the zk‑kit repo is flagged in the repo metadata as having bundling issues (the repo lists some packages as not bundle‑friendly). That means smt is more likely to require Metro polyfills or extra configuration in RN. (github.com)
What that practically means
- @openpassport/zk-kit-lean-imt appears to be a pure TypeScript/JS implementation and will most often work out-of-the-box in a React Native app (no native binaries in the package). Try importing it — most likely it will bundle fine. (npmjs.com)
- @openpassport/zk-kit-smt may import hashing/crypto code (the README examples show usage of crypto-js / poseidon-lite). If one of its transitive deps expects Node built-ins (crypto, fs, etc.) Metro will fail to resolve them and you’ll need polyfills/config. The zk‑kit repo’s package list marks smt as problematic for bundling, so expect to do extra bundler work if you hit errors. (github.com)
How to test quickly in your React Native app
- yarn add @openpassport/zk-kit-lean-imt @openpassport/zk-kit-smt
- In your RN project run Metro / start the app and import a minimal usage (create an instance and call a simple method). If Metro succeeds and the JS runs, the package bundled successfully.
If Metro fails (common error: “Unable to resolve module 'crypto'” or similar)
- Typical fixes:
- Use node-libs-react-native or explicit extraNodeModules to polyfill Node built-ins. Example metro.config.js pattern:
- resolver.extraNodeModules -> map builtins (crypto, stream, buffer, etc.) to RN-friendly shims.
- Install the recommended polyfills (examples used in the community): node-libs-react-native, buffer, react-native-get-random-values (or react-native-crypto + react-native-randombytes depending on approach). Then add any required global shims (e.g. global.Buffer = require('buffer').Buffer). (stackoverflow.com, gist.github.com)
- Use node-libs-react-native or explicit extraNodeModules to polyfill Node built-ins. Example metro.config.js pattern:
- If you use Expo, prefer the Expo crypto APIs (expo-crypto) or customize metro via the Expo docs; Expo also has guidance for node builtins. (docs.expo.dev)
Concrete quick troubleshooting recipe (one of the common community approaches)
- npm/yarn add node-libs-react-native buffer react-native-get-random-values
- Edit metro.config.js:
- resolver.extraNodeModules = require('node-libs-react-native')
- add watchFolders if you need to include linked packages
- In your app entry (e.g. index.js):
- require('react-native-get-random-values')
- global.Buffer = require('buffer').Buffer
- Restart Metro / clear cache (react-native start --reset-cache) and rebuild. (If you still see missing modules, add targeted extraNodeModules mappings for the problematic module.) (stackoverflow.com, gist.github.com)
Summary / recommendation
- Try lean-imt first — it’s very likely to bundle without work. (npmjs.com)
- Expect smt to possibly need extra Metro polyfills (the repo metadata suggests it’s more likely to be bundle-problematic). If you get Metro errors, follow the polyfill/metro.config approaches above. (github.com)
If you want, I can:
- Check a specific error you’re seeing (paste the Metro error output) and give an exact metro.config.js + list of packages to install; or
- Produce a ready-to-drop metro.config.js and index.js bootstrap snippet tailored to Expo vs. bare React Native.
Citations:
- 1: https://www.npmjs.com/package/%40openpassport/zk-kit-lean-imt?utm_source=chatgpt.com
- 2: https://github.com/zk-passport/openpassport/issues/151?utm_source=chatgpt.com
- 3: https://github.com/zk-passport/zk-kit
- 4: https://www.npmjs.com/package/%40openpassport/zk-kit-lean-imt?utm_source=chatgpt.com
- 5: https://github.com/zk-passport/openpassport/issues/151?utm_source=chatgpt.com
- 6: https://stackoverflow.com/questions/75215120/unable-to-resolve-module-crypto?utm_source=chatgpt.com
- 7: https://gist.github.com/parshap/e3063d9bf6058041b34b26b7166fd6bd?utm_source=chatgpt.com
- 8: https://docs.expo.dev/versions/v52.0.0/config/metro?utm_source=chatgpt.com
- 9: https://stackoverflow.com/questions/75215120/unable-to-resolve-module-crypto?utm_source=chatgpt.com
- 10: https://gist.github.com/parshap/e3063d9bf6058041b34b26b7166fd6bd?utm_source=chatgpt.com
- 11: https://www.npmjs.com/package/%40openpassport/zk-kit-lean-imt?utm_source=chatgpt.com
- 12: https://github.com/zk-passport/openpassport/issues/151?utm_source=chatgpt.com
Ensure React Native (Hermes) compatibility and bundler config for crypto dependencies
Based on upstream docs and community feedback, these new packages will need extra configuration in a React Native (Hermes) environment:
• poseidon-lite
– Not confirmed to run out-of-the-box on RN/Hermes (Hermes omits Node.js built-ins like crypto, Buffer).
– Action: Either swap for an RN-compatible Poseidon implementation (e.g. expo-crypto based) or add Node polyfills:
• yarn add node-libs-react-native buffer react-native-get-random-values
• metro.config.js → resolver.extraNodeModules = require('node-libs-react-native')
• In your entry (index.js):
js import 'react-native-get-random-values'; global.Buffer = require('buffer').Buffer;
• Clear Metro cache and rebuild.
• @openpassport/zk-kit-lean-imt
– Pure TypeScript/JS, no native binaries. Should bundle cleanly under Metro.
– Action: Import a minimal API call in RN and confirm Metro starts without errors.
• @openpassport/zk-kit-smt
– Repo metadata flags this as “bundle-problematic.” It pulls in hashing libs (crypto-js, poseidon-lite), so Metro may throw “Unable to resolve module 'crypto'” or similar.
– Action: Treat like poseidon-lite above—polyfill Node built-ins via node-libs-react-native, buffer, etc., or follow the troubleshooting recipe from the zk-kit issue tracker.
Additional web bundling checks:
• Confirm each package’s package.json exports an ESM entrypoint (module/exports) so tsup can tree-shake under sideEffects: false.
• On your web target, run a production build and inspect bundle size to ensure dead code is pruned.
Next Steps
- In a demo RN project,
yarn add poseidon-lite @openpassport/zk-kit-lean-imt @openpassport/zk-kit-smt - Attempt a minimal import/use of each. Note any Metro errors.
- Apply polyfills and Metro tweaks as above.
- Re-run Metro and Web (tsup) builds; verify no Node-built-ins errors and that tree-shaking works.
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/package.json around lines 54-57, adding
poseidon-lite and the zk-kit packages can break React Native/Hermes bundling
because Hermes omits Node built-ins (crypto, Buffer) and
@openpassport/zk-kit-smt pulls hashing deps; fix by either swapping
poseidon-lite for an RN-compatible Poseidon implementation or adding Node
polyfills and bundler tweaks: install node-libs-react-native, buffer and
react-native-get-random-values; set metro.config.js resolver.extraNodeModules =
require('node-libs-react-native'); import 'react-native-get-random-values' and
set global.Buffer = require('buffer').Buffer in the app entry; verify
@openpassport/zk-kit-lean-imt imports cleanly in RN and treat
@openpassport/zk-kit-smt like poseidon-lite (polyfill or replace), then clear
Metro cache and rebuild; additionally confirm package.json for these packages
exposes ESM entrypoints (module/exports) for web tsup tree-shaking and run
production web build to inspect bundle size.
| export function generateTEEInputsDisclose( | ||
| secret: string, | ||
| passportData: PassportData, | ||
| selfApp: SelfApp, | ||
| ofacTrees: { | ||
| passportNoAndNationality: any; | ||
| nameAndDob: any; | ||
| nameAndYob: any; | ||
| }, | ||
| commitmentTree: any, | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding input validation for the TEE input generator parameters
The function accepts several critical parameters without validating their structure or required properties. For a security-sensitive TEE context, consider adding validation to ensure:
secretis not emptypassportDatacontains required fields and validdocumentCategoryselfApphas all required properties- Tree structures are properly formatted
Add validation at the beginning of the function:
export function generateTEEInputsDisclose(
secret: string,
passportData: PassportData,
selfApp: SelfApp,
ofacTrees: {
passportNoAndNationality: any;
nameAndDob: any;
nameAndYob: any;
},
commitmentTree: any,
) {
+ // Validate critical inputs
+ if (!secret || typeof secret !== 'string') {
+ throw new Error('Invalid secret: must be a non-empty string');
+ }
+ if (!passportData?.documentCategory || !['passport', 'id_card'].includes(passportData.documentCategory)) {
+ throw new Error('Invalid passportData: documentCategory must be "passport" or "id_card"');
+ }
+ if (!selfApp?.scope || !selfApp?.endpoint || !selfApp?.disclosures) {
+ throw new Error('Invalid selfApp: missing required properties');
+ }
+ if (!ofacTrees?.nameAndDob || !ofacTrees?.nameAndYob) {
+ throw new Error('Invalid ofacTrees: missing required trees');
+ }
+
const { scope, disclosures, endpoint, userId, userDefinedData, chainID } = selfApp;📝 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.
| export function generateTEEInputsDisclose( | |
| secret: string, | |
| passportData: PassportData, | |
| selfApp: SelfApp, | |
| ofacTrees: { | |
| passportNoAndNationality: any; | |
| nameAndDob: any; | |
| nameAndYob: any; | |
| }, | |
| commitmentTree: any, | |
| ) { | |
| export function generateTEEInputsDisclose( | |
| secret: string, | |
| passportData: PassportData, | |
| selfApp: SelfApp, | |
| ofacTrees: { | |
| passportNoAndNationality: any; | |
| nameAndDob: any; | |
| nameAndYob: any; | |
| }, | |
| commitmentTree: any, | |
| ) { | |
| // Validate critical inputs | |
| if (!secret || typeof secret !== 'string') { | |
| throw new Error('Invalid secret: must be a non-empty string'); | |
| } | |
| if (!passportData?.documentCategory || !['passport', 'id_card'].includes(passportData.documentCategory)) { | |
| throw new Error('Invalid passportData: documentCategory must be "passport" or "id_card"'); | |
| } | |
| if (!selfApp?.scope || !selfApp?.endpoint || !selfApp?.disclosures) { | |
| throw new Error('Invalid selfApp: missing required properties'); | |
| } | |
| if (!ofacTrees?.nameAndDob || !ofacTrees?.nameAndYob) { | |
| throw new Error('Invalid ofacTrees: missing required trees'); | |
| } | |
| const { scope, disclosures, endpoint, userId, userDefinedData, chainID } = selfApp; | |
| // ...rest of the function... | |
| } |
| ofacTrees: { | ||
| passportNoAndNationality: any; | ||
| nameAndDob: any; | ||
| nameAndYob: any; | ||
| }, | ||
| commitmentTree: any, |
There was a problem hiding this comment.
Type safety concern: Using any type for tree parameters
The OFAC trees and commitment tree parameters use any type, which bypasses TypeScript's type checking. This could lead to runtime errors if incorrect data structures are passed.
Consider defining proper types for the tree structures:
+import type { SMTData, IMTData } from '@openpassport/zk-kit-smt';
+
export function generateTEEInputsDisclose(
secret: string,
passportData: PassportData,
selfApp: SelfApp,
ofacTrees: {
- passportNoAndNationality: any;
- nameAndDob: any;
- nameAndYob: any;
+ passportNoAndNationality: SMTData;
+ nameAndDob: SMTData;
+ nameAndYob: SMTData;
},
- commitmentTree: any,
+ commitmentTree: IMTData,
) {📝 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.
| ofacTrees: { | |
| passportNoAndNationality: any; | |
| nameAndDob: any; | |
| nameAndYob: any; | |
| }, | |
| commitmentTree: any, | |
| import type { SMTData, IMTData } from '@openpassport/zk-kit-smt'; | |
| export function generateTEEInputsDisclose( | |
| secret: string, | |
| passportData: PassportData, | |
| selfApp: SelfApp, | |
| ofacTrees: { | |
| passportNoAndNationality: SMTData; | |
| nameAndDob: SMTData; | |
| nameAndYob: SMTData; | |
| }, | |
| commitmentTree: IMTData, | |
| ) { | |
| // … | |
| } |
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/proving/discloseInputs.ts around lines 25 to
30, the OFAC trees and commitmentTree are typed as `any`, which disables
TypeScript checks; replace `any` with precise interfaces/types that model the
tree nodes and APIs (e.g., node value type, children, proof/serialize methods)
or use a generic Tree<T> type used across the codebase, update the function and
related signatures to use those types, import or declare the interfaces near the
top of the file (or in a shared types file) and run the typechecker to fix all
resulting errors and ensure callers are updated to provide the correctly typed
trees.
| function getSelectorDg1(document: DocumentCategory, disclosures: SelfAppDisclosureConfig) { | ||
| switch (document) { | ||
| case 'passport': | ||
| return getSelectorDg1Passport(disclosures); | ||
| case 'id_card': | ||
| return getSelectorDg1IdCard(disclosures); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add exhaustive check for document category switch
The switch statement doesn't have a default case, which could lead to undefined behavior if new document categories are added in the future.
function getSelectorDg1(document: DocumentCategory, disclosures: SelfAppDisclosureConfig) {
switch (document) {
case 'passport':
return getSelectorDg1Passport(disclosures);
case 'id_card':
return getSelectorDg1IdCard(disclosures);
+ default:
+ // Exhaustive check - this should never be reached if DocumentCategory is properly typed
+ const exhaustiveCheck: never = document;
+ throw new Error(`Unsupported document category: ${exhaustiveCheck}`);
}
}📝 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.
| function getSelectorDg1(document: DocumentCategory, disclosures: SelfAppDisclosureConfig) { | |
| switch (document) { | |
| case 'passport': | |
| return getSelectorDg1Passport(disclosures); | |
| case 'id_card': | |
| return getSelectorDg1IdCard(disclosures); | |
| } | |
| } | |
| function getSelectorDg1(document: DocumentCategory, disclosures: SelfAppDisclosureConfig) { | |
| switch (document) { | |
| case 'passport': | |
| return getSelectorDg1Passport(disclosures); | |
| case 'id_card': | |
| return getSelectorDg1IdCard(disclosures); | |
| default: | |
| // Exhaustive check - this should never be reached if DocumentCategory is properly typed | |
| const exhaustiveCheck: never = document; | |
| throw new Error(`Unsupported document category: ${exhaustiveCheck}`); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/proving/discloseInputs.ts around lines 79 to
86, the switch over DocumentCategory is missing a default/exhaustive branch
which can return undefined if a new category is added; update the switch to be
exhaustive by adding a default that either throws a descriptive Error (e.g.,
`throw new Error("Unsupported document category: " + document)`) or use the
TypeScript exhaustive-check pattern (assign `document` to a `never` typed
variable and throw) so the compiler errors on new categories and runtime
behavior is explicit.
| const [start, end] = attributeToPosition[attribute as keyof typeof attributeToPosition]; | ||
| selector_dg1.fill('1', start, end + 1); |
There was a problem hiding this comment.
Add boundary validation for attribute positions
The code assumes attributeToPosition and attributeToPosition_ID will always return valid array indices, but doesn't validate the boundaries against the selector array length.
Add validation to prevent potential array out-of-bounds errors:
function getSelectorDg1Passport(disclosures: SelfAppDisclosureConfig) {
const selector_dg1 = Array(88).fill('0');
Object.entries(disclosures).forEach(([attribute, reveal]) => {
if (['ofac', 'excludedCountries', 'minimumAge'].includes(attribute)) {
return;
}
if (reveal) {
const [start, end] = attributeToPosition[attribute as keyof typeof attributeToPosition];
+ if (start < 0 || end >= selector_dg1.length) {
+ throw new Error(`Invalid position range for attribute ${attribute}: [${start}, ${end}]`);
+ }
selector_dg1.fill('1', start, end + 1);
}
});
return selector_dg1;
}Apply similar validation to getSelectorDg1IdCard:
function getSelectorDg1IdCard(disclosures: SelfAppDisclosureConfig) {
const selector_dg1 = Array(90).fill('0');
Object.entries(disclosures).forEach(([attribute, reveal]) => {
if (['ofac', 'excludedCountries', 'minimumAge'].includes(attribute)) {
return;
}
if (reveal) {
const [start, end] = attributeToPosition_ID[attribute as keyof typeof attributeToPosition_ID];
+ if (start < 0 || end >= selector_dg1.length) {
+ throw new Error(`Invalid position range for attribute ${attribute}: [${start}, ${end}]`);
+ }
selector_dg1.fill('1', start, end + 1);
}
});
return selector_dg1;
}Also applies to: 109-110
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/proving/discloseInputs.ts around lines 95-96
(and similarly for lines 109-110 / getSelectorDg1IdCard), the code reads
start/end from attributeToPosition (and attributeToPosition_ID) and uses them to
fill selector_dg1 without validating boundaries; add explicit validation that
start and end are numbers, start >= 0, end >= start, and end <
selector_dg1.length before calling selector_dg1.fill(...); if validation fails,
throw or return a clear error (or skip filling) to avoid out-of-bounds access;
apply the same checks to the getSelectorDg1IdCard usage and any other places
that use positions from those maps.
| export function generateTEEInputsRegister( | ||
| secret: string, | ||
| passportData: PassportData, | ||
| dscTree: string, | ||
| env: 'prod' | 'stg', | ||
| ) { | ||
| const inputs = generateCircuitInputsRegister(secret, passportData, dscTree); | ||
| const circuitName = getCircuitNameFromPassportData(passportData, 'register'); | ||
| const endpointType = env === 'stg' ? 'staging_celo' : 'celo'; | ||
| const endpoint = 'https://self.xyz'; | ||
| return { inputs, circuitName, endpointType, endpoint }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strengthen types and defaults; keep API shape stable.
- Add an explicit return type alias for stability and clearer DX.
- Default env to 'prod' to reduce call-site footguns.
- Optional: Document with JSDoc.
Apply this diff:
@@
-import type { PassportData } from '@selfxyz/common/types';
+import type { PassportData } from '@selfxyz/common/types';
@@
-export function generateTEEInputsRegister(
+export type TEEInputsResponse = {
+ inputs: unknown;
+ circuitName: string;
+ endpointType: 'staging_celo' | 'celo';
+ endpoint: string;
+};
+
+/**
+ * Build TEE inputs for the register circuit.
+ */
+export function generateTEEInputsRegister(
secret: string,
passportData: PassportData,
dscTree: string,
- env: 'prod' | 'stg',
-) {
+ env: 'prod' | 'stg' = 'prod',
+): TEEInputsResponse {
const inputs = generateCircuitInputsRegister(secret, passportData, dscTree);
const circuitName = getCircuitNameFromPassportData(passportData, 'register');
const endpointType = env === 'stg' ? 'staging_celo' : 'celo';
const endpoint = 'https://self.xyz';
return { inputs, circuitName, endpointType, endpoint };
}📝 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.
| export function generateTEEInputsRegister( | |
| secret: string, | |
| passportData: PassportData, | |
| dscTree: string, | |
| env: 'prod' | 'stg', | |
| ) { | |
| const inputs = generateCircuitInputsRegister(secret, passportData, dscTree); | |
| const circuitName = getCircuitNameFromPassportData(passportData, 'register'); | |
| const endpointType = env === 'stg' ? 'staging_celo' : 'celo'; | |
| const endpoint = 'https://self.xyz'; | |
| return { inputs, circuitName, endpointType, endpoint }; | |
| } | |
| import type { PassportData } from '@selfxyz/common/types'; | |
| export type TEEInputsResponse = { | |
| inputs: unknown; | |
| circuitName: string; | |
| endpointType: 'staging_celo' | 'celo'; | |
| endpoint: string; | |
| }; | |
| /** | |
| * Build TEE inputs for the register circuit. | |
| */ | |
| export function generateTEEInputsRegister( | |
| secret: string, | |
| passportData: PassportData, | |
| dscTree: string, | |
| env: 'prod' | 'stg' = 'prod', | |
| ): TEEInputsResponse { | |
| const inputs = generateCircuitInputsRegister(secret, passportData, dscTree); | |
| const circuitName = getCircuitNameFromPassportData(passportData, 'register'); | |
| const endpointType = env === 'stg' ? 'staging_celo' : 'celo'; | |
| const endpoint = 'https://self.xyz'; | |
| return { inputs, circuitName, endpointType, endpoint }; | |
| } |
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/proving/registerInputs.ts around lines 4 to 15,
the function lacks an explicit return type and the env parameter is required;
update the function signature to use a named return type alias (define and
export a suitable type/interface capturing inputs, circuitName, endpointType,
endpoint), set env to default to 'prod' (env: 'prod' | 'stg' = 'prod'), and add
a brief JSDoc above the function describing parameters and return shape to
preserve API stability and improve DX.
|
closing this pr in favor of - #917 |
Summary
Testing
yarn workspace @selfxyz/mobile-sdk-alpha niceyarn workspace @selfxyz/mobile-sdk-alpha buildyarn workspace @selfxyz/mobile-sdk-alpha typesyarn workspace @selfxyz/mobile-sdk-alpha testyarn lintyarn buildhttps://chatgpt.com/codex/tasks/task_b_689c068145d8832d9c9c7425db710527
Summary by CodeRabbit
New Features
Tests
Chores