[SELF-656] feat: add proving helpers #917
Conversation
WalkthroughAdds two proving helpers—discloseInputs and registerInputs—to mobile-sdk-alpha and re-exports their types; moves disclose/register input generation out of the app into the SDK, updates provingMachine to call the new SDK functions, and adds unit tests validating selectors, env handling, and returned payloads. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant SDK as mobile-sdk-alpha
participant Utils as @selfxyz/common/utils
App->>SDK: registerInputs(secret, passportData, dscTree, env)
SDK->>Utils: generateCircuitInputsRegister(...)
Utils-->>SDK: inputs
SDK->>Utils: getCircuitNameFromPassportData(passportData,"register")
Utils-->>SDK: circuitName
SDK-->>App: { inputs, circuitName, endpointType(env), endpoint("https://self.xyz") }
sequenceDiagram
autonumber
actor App
participant SDK as mobile-sdk-alpha
participant Hash as hashing utilities
participant Trees as SMT/LeanIMT/poseidon
participant Utils as generateCircuitInputsVCandDisclose
App->>SDK: discloseInputs(secret, passportData, disclosuresCtx, ofacTrees, commitmentTree, env)
SDK->>Hash: calculateUserIdentifierHash(scope, chainID, userId, userDefinedData)
Hash-->>SDK: userIdentifierHash
SDK->>Trees: init OFAC SMTs and commitment IMT
Trees-->>SDK: prepared trees
SDK->>Utils: generateCircuitInputsVCandDisclose(...selectors, trees, userIdentifierHash...)
Utils-->>SDK: inputs
SDK-->>App: { inputs, circuitName(by document), endpointType(env), endpoint("https://self.xyz") }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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
|
Giga Summary• feat: add proving helpers
Quality Assessment• Strong test coverage and clean implementation. Quality Score: 8/10 (Threshold: 7/10) 💬 Detailed comments have been added to specific code changes. |
![giga-agent[bot]](https://avatars.githubusercontent.com/in/1202121?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/mobile-sdk-alpha/src/proving/registerInputs.ts (1)
7-9: Endpoint should be env-aware and centralized (hardcoded prod URL leaks into staging)Endpoint is always 'https://self.xyz' while endpointType toggles between prod/stg. This is inconsistent and likely breaks staging flows (e.g., scope hashing and routing). Centralize endpoint selection and derive it from env.
Apply this diff to make the endpoint env-aware:
@@ -import type { PassportData } from '@selfxyz/common/types'; +import type { PassportData } from '@selfxyz/common/types'; +import { endpointForEnv } from '../config/endpoints'; @@ - const endpointType = env === 'stg' ? 'staging_celo' : 'celo'; - const endpoint = 'https://self.xyz'; + const endpointType = env === 'stg' ? 'staging_celo' : 'celo'; + const endpoint = endpointForEnv(env); return { inputs, circuitName, endpointType, endpoint };And add a small helper (new file) to avoid duplication across helpers:
// packages/mobile-sdk-alpha/src/config/endpoints.ts export const ENDPOINTS = { prod: 'https://self.xyz', stg: 'https://staging.self.xyz', } as const; export type Env = keyof typeof ENDPOINTS; export function endpointForEnv(env: Env) { return ENDPOINTS[env]; } // Optional, if you want to centralize this too: export function endpointTypeForEnv(env: Env) { return env === 'stg' ? 'staging_celo' : 'celo'; }If you want, I can open a follow-up PR to refactor both proving helpers to use the same config and add tests for the staging endpoint value.
packages/mobile-sdk-alpha/src/proving/discloseInputs.ts (1)
45-46: Hardcoded prod URL; make endpoint env-aware and shared across helpersReturning a prod URL for staging will generate the wrong scope hash and can break proofs or route requests to the wrong backend.
Apply this diff to align endpoint with env:
@@ -import { LeanIMT } from '@openpassport/zk-kit-lean-imt'; +import { LeanIMT } from '@openpassport/zk-kit-lean-imt'; import { SMT } from '@openpassport/zk-kit-smt'; +import { endpointForEnv } from '../config/endpoints'; @@ - const endpoint = 'https://self.xyz'; + const endpoint = endpointForEnv(env); const scope_hash = hashEndpointWithScope(endpoint, scope);Optionally, you can centralize endpointType calculation similarly (via endpointTypeForEnv) to avoid duplication.
🧹 Nitpick comments (2)
packages/mobile-sdk-alpha/tests/proving/registerInputs.test.ts (1)
23-26: Clarify test for future env-aware endpoint logic
CurrentlyregisterInputsalways returns endpoint'https://self.xyz'. Once you update it to be env-aware, strengthen this test by asserting the host too:• In
packages/mobile-sdk-alpha/tests/proving/registerInputs.test.tsunder the “stg” case:it('uses staging endpoint type when env is stg', () => { const result = registerInputs('secret', passportData, 'tree', 'stg'); expect(result.endpointType).toBe('staging_celo'); + // After making endpoint env-aware: + expect(result.endpoint).toBe('https://staging.self.xyz'); });packages/mobile-sdk-alpha/src/proving/discloseInputs.ts (1)
84-87: Consider centralizing endpointType mapping (optional)Minor duplication with registerInputs; extracting a helper reduces drift risk if new envs are added.
Example:
- const circuitName = passportData.documentCategory === 'passport' ? 'vc_and_disclose' : 'vc_and_disclose_id'; - const endpointType = env === 'stg' ? 'staging_celo' : 'celo'; + const circuitName = passportData.documentCategory === 'passport' ? 'vc_and_disclose' : 'vc_and_disclose_id'; + const endpointType = env === 'stg' ? 'staging_celo' : 'celo'; // or endpointTypeForEnv(env)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/mobile-sdk-alpha/src/index.ts(2 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 (2)
**/*.{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
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/tests/proving/registerInputs.test.tspackages/mobile-sdk-alpha/src/proving/registerInputs.tspackages/mobile-sdk-alpha/src/index.tspackages/mobile-sdk-alpha/src/proving/discloseInputs.tspackages/mobile-sdk-alpha/tests/proving/discloseInputs.test.ts
🧬 Code Graph Analysis (3)
packages/mobile-sdk-alpha/tests/proving/registerInputs.test.ts (1)
packages/mobile-sdk-alpha/src/proving/registerInputs.ts (1)
registerInputs(4-10)
packages/mobile-sdk-alpha/src/proving/registerInputs.ts (1)
packages/mobile-sdk-alpha/src/index.ts (1)
registerInputs(64-64)
packages/mobile-sdk-alpha/tests/proving/discloseInputs.test.ts (1)
packages/mobile-sdk-alpha/src/proving/discloseInputs.ts (1)
discloseInputs(35-88)
🔇 Additional comments (2)
packages/mobile-sdk-alpha/src/index.ts (1)
51-52: Public API surface LGTMExporting discloseInputs and registerInputs here is consistent with the SDK’s entry point and keeps the API discoverable.
Also applies to: 63-65
packages/mobile-sdk-alpha/tests/proving/discloseInputs.test.ts (1)
39-59: Skip staging test: implement ‘stg’ env support firstIt looks like
discloseInputsdoesn’t currently handle the'stg'environment (there’s nocase 'stg'branch), so adding a test for staging will fail. Before covering staging in tests, first implement the staging‐endpoint logic inpackages/mobile-sdk-alpha/src/proving/discloseInputs.ts(endpointType and host for'stg'), then add this spec.Likely an incorrect or invalid review comment.
transphorm
changed the title
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
packages/mobile-sdk-alpha/src/proving/discloseInputs.ts (1)
61-63: Hardcoded endpoint should be configurable (repeat finding)Endpoint is hardcoded to https://self.xyz. This was previously flagged; consider moving this to a shared config or accepting it via function options to support non-prod/staging environments and future endpoint changes.
I can help thread an optional parameter through the API and wire it in the app if you want to support custom endpoints in tests and staging.
🧹 Nitpick comments (4)
packages/mobile-sdk-alpha/src/proving/discloseInputs.ts (3)
72-83: Gate OFAC tree imports behind the ofac selector to avoid unnecessary workYou import and hydrate all OFAC SMTs regardless of disclosures.ofac. On lower-end devices, SMT import can be non-trivial. Consider short-circuiting these imports when ofac isn’t requested to reduce CPU/memory.
- 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); - if (ofacTrees.passportNoAndNationality) { - passportNoAndNationalitySMT.import(ofacTrees.passportNoAndNationality); - } - } - nameAndDobSMT.import(ofacTrees.nameAndDob); - nameAndYobSMT.import(ofacTrees.nameAndYob); + let passportNoAndNationalitySMT: SMT | null = null; + let nameAndDobSMT: SMT | null = null; + let nameAndYobSMT: SMT | null = null; + if (disclosures.ofac) { + nameAndDobSMT = new SMT(poseidon2, true); + nameAndYobSMT = new SMT(poseidon2, true); + if (document === 'passport') { + passportNoAndNationalitySMT = new SMT(poseidon2, true); + if (ofacTrees.passportNoAndNationality) { + passportNoAndNationalitySMT.import(ofacTrees.passportNoAndNationality); + } + } + nameAndDobSMT.import(ofacTrees.nameAndDob); + nameAndYobSMT.import(ofacTrees.nameAndYob); + }Note: ensure generateCircuitInputsVCandDisclose tolerates nulls for these when ofac is off.
121-133: Validate attribute mapping existence to avoid runtime errorsIf disclosures contains a key not present in attributeToPosition, this will throw. Add a guard to fail fast with a clear message.
- if (reveal) { - const [start, end] = attributeToPosition[attribute as keyof typeof attributeToPosition]; + if (reveal) { + const pos = attributeToPosition[attribute as keyof typeof attributeToPosition]; + if (!pos) { + throw new Error(`Unknown passport attribute in disclosures: ${attribute}`); + } + const [start, end] = pos; selector_dg1.fill('1', start, end + 1); }
138-150: Same attribute existence validation for ID cardsMirror the guard for attributeToPosition_ID to avoid undefined access.
- if (reveal) { - const [start, end] = attributeToPosition_ID[attribute as keyof typeof attributeToPosition_ID]; + if (reveal) { + const pos = attributeToPosition_ID[attribute as keyof typeof attributeToPosition_ID]; + if (!pos) { + throw new Error(`Unknown ID card attribute in disclosures: ${attribute}`); + } + const [start, end] = pos; selector_dg1.fill('1', start, end + 1); }app/src/utils/proving/provingMachine.ts (1)
991-997: Propagate null checks for dsc_tree before registerInputsIn register flow, protocolStore[document].dsc_tree is passed directly. If fetch_all fails to load DSC tree, this will throw deep in the SDK. Add a guard with a clear error to improve debuggability.
- ({ inputs, circuitName, endpointType, endpoint } = registerInputs( + const dscTree = protocolStore[document].dsc_tree; + if (!dscTree) { + throw new Error('DSC tree not loaded'); + } + ({ inputs, circuitName, endpointType, endpoint } = registerInputs( secret as string, passportData, - protocolStore[document].dsc_tree, + dscTree, env, ));
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
app/src/utils/proving/provingInputs.ts(1 hunks)app/src/utils/proving/provingMachine.ts(4 hunks)app/tests/utils/proving/provingMachine.generatePayload.test.ts(6 hunks)packages/mobile-sdk-alpha/src/browser.ts(1 hunks)packages/mobile-sdk-alpha/src/index.ts(3 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)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/mobile-sdk-alpha/tests/proving/registerInputs.test.ts
- packages/mobile-sdk-alpha/src/index.ts
- packages/mobile-sdk-alpha/tests/proving/discloseInputs.test.ts
- packages/mobile-sdk-alpha/src/proving/registerInputs.ts
🧰 Additional context used
📓 Path-based instructions (3)
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/browser.tspackages/mobile-sdk-alpha/src/proving/discloseInputs.ts
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/utils/proving/provingInputs.tsapp/src/utils/proving/provingMachine.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:
app/tests/utils/proving/provingMachine.generatePayload.test.ts
🧬 Code Graph Analysis (2)
app/tests/utils/proving/provingMachine.generatePayload.test.ts (1)
packages/mobile-sdk-alpha/src/browser.ts (2)
registerInputs(58-58)discloseInputs(59-59)
packages/mobile-sdk-alpha/src/proving/discloseInputs.ts (1)
packages/mobile-sdk-alpha/src/index.ts (3)
DiscloseSelfApp(36-36)OfacTrees(36-36)discloseInputs(53-53)
🪛 GitHub Check: lint
packages/mobile-sdk-alpha/src/browser.ts
[failure] 60-60:
Expected DiscloseSelfApp before discloseInputs
[failure] 59-59:
Expected discloseInputs before registerInputs
[failure] 58-58:
Expected registerInputs before webScannerShim
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
- GitHub Check: e2e-ios
🔇 Additional comments (1)
app/src/utils/proving/provingInputs.ts (1)
3-3: LGTM on narrowing the scopeImport-only change aligns with the module’s new DSC-only responsibility. No issues.
transphorm
force-pushed
the
codex/port-tee-helper-functions
branch
from
20d3f5e to
5ca5aa5
Compare
|
rebased with #915 to unblock failing pipeline |
transphorm
force-pushed
the
codex/port-tee-helper-functions
branch
from
29f0b47 to
5ca5aa5
Compare
transphorm
force-pushed
the
codex/port-tee-helper-functions
branch
from
5ca5aa5 to
65385fa
Compare
| export type { QRProofOptions } from './qr'; | ||
|
|
||
| // Error handling | ||
| // NFC module |
|
need to update paths based on new |
|
closing due to new proposed work and architecture |
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 buildUsed testing-first approach guidance from mobile sdk migration rule.
https://chatgpt.com/codex/tasks/task_b_68a2aa9434a4832d9aa108e4232bb0de
Summary by CodeRabbit