feat: selfrica circuit and tests#1171
Conversation
Vishalkulkarni45
had a problem deploying
to
development
GitHub Actions
Failure
Vishalkulkarni45
temporarily deployed
to
development
GitHub Actions
Inactive
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR introduces comprehensive Selfrica identity verification support alongside existing Aadhaar functionality. It adds new Circom circuits for Selfrica registration and disclosure with OFAC SMT verification, smart contract integration for verification flows and identity registry management, TypeScript utilities for input generation and ECDSA signing, and extensive test coverage across circuit and contract layers. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CircomCircuit as Circom (VC & Disclose)
participant SmartContract as Contract Hub
participant Registry as Selfrica Registry
participant Tree as Merkle Tree / SMT
User->>CircomCircuit: SmileData, ECDSA sig, Merkle proof, scope, date
CircomCircuit->>Tree: Verify Merkle root<br/>(secret + msg → leaf)
Tree-->>CircomCircuit: Root valid
CircomCircuit->>CircomCircuit: Verify OFAC SMT<br/>(name+DOB, name+YOB)
CircomCircuit->>CircomCircuit: Validate age<br/>(IsOlderThan)
CircomCircuit->>CircomCircuit: Pack revealed data<br/>(selector bits)
CircomCircuit-->>User: Proof + public signals<br/>(nullifier, revealedData_packed, OFAC roots)
User->>SmartContract: Submit proof
SmartContract->>Registry: Check pubkey commitment
Registry-->>SmartContract: Valid
SmartContract->>SmartContract: Decode public signals
SmartContract->>SmartContract: Verify OFAC/forbidden countries<br/>Verify age/expiry
SmartContract->>Registry: Register commitment<br/>(nullifier, commitment)
Registry->>Tree: Update IMT
Tree-->>Registry: New root
Registry-->>SmartContract: Registered
SmartContract-->>User: VerificationCompleted event
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Rationale: This PR introduces substantial new cryptographic logic (ECDSA signing/verification, OFAC SMT verification, Baby Jubjub operations) across multiple layers (circuits, contracts, utilities). While the changes follow established patterns and remain cohesive in scope (Selfrica integration), the density of new logic—particularly around signature verification, field layout handling, and state management in the registry—requires careful scrutiny. The interaction between circuit outputs and contract verification paths, plus OFAC root management, demands thorough review. Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Vishalkulkarni45
had a problem deploying
to
development
GitHub Actions
Failure
Vishalkulkarni45
temporarily deployed
to
development
GitHub Actions
Inactive
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
circuits/package.json (1)
1-1: Fix formatting issues flagged by lint.The pipeline indicates
yarn lintfailed. Runyarn formatorprettier --write .to resolve formatting issues across the affected files.circuits/tests/disclose/vc_and_disclose_aadhaar.test.ts (3)
52-62: Mocha hook misuse: use before(), not this.beforeAll().this.beforeAll is not a Mocha API. The suite setup may never run.
- this.beforeAll(async function () { + before(async function () { this.timeout(0); circuit = await wasmTester(
119-123: Incorrect getOutput selection: requesting only [4] but reading [0]-[3].You read revealData_packed[0..3] without fetching them, leading to undefined values.
-const revealedData = await circuit.getOutput(w, [`revealData_packed[4]`]); +const revealedData = await circuit.getOutput( + w, + [`revealData_packed[0]`,`revealData_packed[1]`,`revealData_packed[2]`,`revealData_packed[3]`] +);
164-168: Same issue: fetch all indices you consume.You later read revealData_packed[0..4]; fetch them explicitly.
-const revealedData = await circuit.getOutput(w, [`revealData_packed[4]`, 'reveal_photoHash']); +const revealedData = await circuit.getOutput( + w, + [ + `revealData_packed[0]`, + `revealData_packed[1]`, + `revealData_packed[2]`, + `revealData_packed[3]`, + `revealData_packed[4]`, + 'reveal_photoHash' + ] +);common/src/utils/trees.ts (2)
56-58: Remove or redact PII in logs (names, passport/country, DOB, arbitrary entry dumps)Several console.log calls emit sensitive data (names, countries, raw entries). This violates logging guidelines and can leak PII in CI/telemetry.
Replace with opt‑in debug logs and redaction:
- console.log('Processing', treetype, 'number', i, 'out of', field.length); + if (process.env.DEBUG_TREES === '1') { + console.debug('[SMT]', treetype, 'i=', i, '/', field.length); + } ... - console.log('This entry already exists in the tree, skipping...'); + if (process.env.DEBUG_TREES === '1') console.debug('[SMT] duplicate leaf, skipping'); ... - console.log('nationality and countryCode', nationality, countryCode); + if (process.env.DEBUG_TREES === '1') console.debug('[normalizeCountry]', 'alpha3 resolved'); ... - console.log('arr', arr, 'arr.length', arr.length); + if (process.env.DEBUG_TREES === '1') console.debug('[name MRZ] len=', arr.length); ... - console.log('dob is null', i, entry); + console.warn('[Aadhaar] dob missing at i=', i); ... - console.log('year is null', i, entry); + console.warn('[Aadhaar] year missing at i=', i); ... - console.log('Error creating leaf value', i, country1, country2); + console.warn('[country leaf] invalid input at i=', i);Also consider centralizing logging with a redact(name) helper (e.g., “J*** D***”). As per coding guidelines.
Also applies to: 70-73, 77-82, 336-337, 469-471, 620-623, 640-647, 518-523
693-708: Normalize error returns to BigInt(0) to prevent undefined propagationgetPassportNumberAndNationalityLeaf returns undefined on invalid lengths, while callers expect bigint. This can cause runtime/type errors.
Apply:
if (passport.length !== 9) { - console.log('parsed passport length is not 9:', i, passport); - return; + console.warn('parsed passport length is not 9:', i); + return BigInt(0); } if (nationality.length !== 3) { - console.log('parsed nationality length is not 3:', i, nationality); - return; + console.warn('parsed nationality length is not 3:', i); + return BigInt(0); } try { const fullHash = poseidon12(passport.concat(nationality)); return generateSmallKey(fullHash); } catch (err) { - console.log('err : passport', err, i, passport); + console.error('err : passport', err, i); + return BigInt(0); }
🧹 Nitpick comments (2)
common/src/utils/selfrica/decodePublicSignals.ts (1)
21-36: Add bounds validation forpublicSignalsarray.The function directly indexes
publicSignalswithout verifying it has the expected length. If a malformed or truncated array is passed, this will throw an error. Consider adding a guard:export const decodePublicSignals = (publicSignals: string[]) => { + const expectedLength = SELFRICA_PUBLIC_SIGNALS_CURRENT_DATE + SELFRICA_PUBLIC_SIGNALS_CURRENT_DATE_LENGTH; + if (publicSignals.length < expectedLength) { + throw new Error(`Invalid publicSignals length: expected at least ${expectedLength}, got ${publicSignals.length}`); + } + const revealedDataPacked = publicSignals.slice(SELFRICA_PUBLIC_SIGNALS_REVEALED_DATA_PACKED, ...);common/src/utils/trees.ts (1)
710-744: Avoid noisy per-entry logs; guard with env flag to keep perf predictablebuildSelfricaSMT logs each iteration; this tanks throughput on large datasets and floods CI.
Wrap progress logs behind DEBUG_TREES and throttle to every N items (e.g., i % 1000 === 0).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
circuits/circuits/disclose/vc_and_disclose_selfrica.circom(1 hunks)circuits/circuits/utils/selfrica/constants.circom(1 hunks)circuits/circuits/utils/selfrica/date/dateIsLess.circom(1 hunks)circuits/circuits/utils/selfrica/date/isOlderThan.circom(1 hunks)circuits/circuits/utils/selfrica/date/isValid.circom(1 hunks)circuits/circuits/utils/selfrica/disclose/country_not_in_list.circom(1 hunks)circuits/circuits/utils/selfrica/disclose/disclose.circom(1 hunks)circuits/circuits/utils/selfrica/disclose/ofac/ofac_name_dob_selfrica.circom(1 hunks)circuits/circuits/utils/selfrica/disclose/ofac/ofac_name_yob_selfrica.circom(1 hunks)circuits/package.json(1 hunks)circuits/tests/disclose/vc_and_disclose_aadhaar.test.ts(2 hunks)circuits/tests/disclose/vc_and_disclose_selfrica.test.ts(1 hunks)common/src/utils/selfrica/constants.ts(1 hunks)common/src/utils/selfrica/decodePublicSignals.ts(1 hunks)common/src/utils/selfrica/generateInputs.ts(1 hunks)common/src/utils/selfrica/rsa.ts(1 hunks)common/src/utils/selfrica/types.ts(1 hunks)common/src/utils/trees.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: 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:
common/src/utils/selfrica/decodePublicSignals.tscommon/src/utils/selfrica/types.tscommon/src/utils/selfrica/rsa.tscircuits/tests/disclose/vc_and_disclose_aadhaar.test.tscommon/src/utils/selfrica/generateInputs.tscommon/src/utils/trees.tscommon/src/utils/selfrica/constants.tscircuits/tests/disclose/vc_and_disclose_selfrica.test.ts
common/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
common/src/**/*.{ts,tsx,js,jsx}: Review shared utilities for:
- Reusability and modular design
- Type safety and error handling
- Side-effect management
- Documentation and naming clarity
Files:
common/src/utils/selfrica/decodePublicSignals.tscommon/src/utils/selfrica/types.tscommon/src/utils/selfrica/rsa.tscommon/src/utils/selfrica/generateInputs.tscommon/src/utils/trees.tscommon/src/utils/selfrica/constants.ts
circuits/**/*.circom
⚙️ CodeRabbit configuration file
circuits/**/*.circom: Review ZK circuit code for:
- Circuit correctness and completeness
- Constraint efficiency
- Input validation
- Security considerations for zero-knowledge proofs
Files:
circuits/circuits/utils/selfrica/disclose/country_not_in_list.circomcircuits/circuits/utils/selfrica/constants.circomcircuits/circuits/utils/selfrica/disclose/ofac/ofac_name_yob_selfrica.circomcircuits/circuits/utils/selfrica/date/dateIsLess.circomcircuits/circuits/utils/selfrica/date/isOlderThan.circomcircuits/circuits/disclose/vc_and_disclose_selfrica.circomcircuits/circuits/utils/selfrica/disclose/ofac/ofac_name_dob_selfrica.circomcircuits/circuits/utils/selfrica/date/isValid.circomcircuits/circuits/utils/selfrica/disclose/disclose.circom
**/*.{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:
circuits/tests/disclose/vc_and_disclose_aadhaar.test.tscircuits/tests/disclose/vc_and_disclose_selfrica.test.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Integrate forbidden-country non-inclusion proof into the disclosure proof circuit
📚 Learning: 2025-08-24T18:53:11.221Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Integrate forbidden-country non-inclusion proof into the disclosure proof circuit
Applied to files:
circuits/circuits/utils/selfrica/disclose/country_not_in_list.circomcircuits/circuits/disclose/vc_and_disclose_selfrica.circomcircuits/circuits/utils/selfrica/disclose/disclose.circom
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to noir/crates/dg1/src/ofac/*.nr : OFAC Compliance System implements three-tier verification (name-based, name + DOB, passport number), and Merkle tree-based sanctions list verification.
Applied to files:
circuits/circuits/utils/selfrica/disclose/ofac/ofac_name_yob_selfrica.circomcircuits/circuits/utils/selfrica/disclose/ofac/ofac_name_dob_selfrica.circom
📚 Learning: 2025-08-24T18:53:11.221Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Ensure the circuit performs day-level comparison with correct date arithmetic
Applied to files:
circuits/circuits/utils/selfrica/date/dateIsLess.circom
📚 Learning: 2025-08-24T18:53:11.221Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Implement the age verification circuit at circuits/circuits/disclose/disclose.circom with private DOB input, public minimum-age input, and boolean output
Applied to files:
circuits/circuits/utils/selfrica/date/isOlderThan.circomcircuits/circuits/disclose/vc_and_disclose_selfrica.circomcircuits/circuits/utils/selfrica/disclose/disclose.circom
🧬 Code graph analysis (5)
common/src/utils/selfrica/decodePublicSignals.ts (1)
common/src/utils/selfrica/constants.ts (16)
SELFRICA_PUBLIC_SIGNALS_REVEALED_DATA_PACKED(42-42)SELFRICA_PUBLIC_SIGNALS_REVEALED_DATA_PACKED_LENGTH(43-43)SELFRICA_MAX_LENGTH(37-37)SELFRICA_PUBLIC_SIGNALS_FORBIDDEN_COUNTRIES_PACKED(45-45)SELFRICA_PUBLIC_SIGNALS_FORBIDDEN_COUNTRIES_PACKED_LENGTH(46-46)SELFRICA_PUBLIC_SIGNALS_IDENTITY_COMMITMENT(48-48)SELFRICA_PUBLIC_SIGNALS_NULLIFIER(49-49)SELFRICA_PUBLIC_SIGNALS_PUBKEY_X(50-50)SELFRICA_PUBLIC_SIGNALS_PUBKEY_Y(51-51)SELFRICA_PUBLIC_SIGNALS_SCOPE(52-52)SELFRICA_PUBLIC_SIGNALS_OFAC_NAME_DOB_SMT_ROOT(53-53)SELFRICA_PUBLIC_SIGNALS_OFAC_NAME_YOB_SMT_ROOT(54-54)SELFRICA_PUBLIC_SIGNALS_ATTESTATION_ID(55-55)SELFRICA_PUBLIC_SIGNALS_CURRENT_DATE(57-57)SELFRICA_PUBLIC_SIGNALS_CURRENT_DATE_LENGTH(58-58)SELFRICA_PUBLIC_SIGNALS_USER_IDENTIFIER(56-56)
common/src/utils/selfrica/types.ts (1)
app/scripts/analyze-tree-shaking.cjs (1)
constants(164-180)
common/src/utils/selfrica/generateInputs.ts (6)
common/src/utils/selfrica/constants.ts (5)
SELFRICA_MAX_LENGTH(37-37)SELFRICA_FULL_NAME_INDEX(16-16)SELFRICA_FULL_NAME_LENGTH(17-17)SELFRICA_DOB_INDEX(19-19)SELFRICA_DOB_LENGTH(20-20)common/src/utils/selfrica/types.ts (3)
SmileData(3-20)serializeSmileData(22-99)SelfricaCircuitInput(101-120)common/src/utils/trees.ts (3)
getNameDobLeafSelfrica(767-772)getNameYobLeafSelfrica(809-815)generateSMTProof(176-225)common/src/utils/circuits/generateInputs.ts (1)
formatInput(58-78)common/src/utils/selfrica/rsa.ts (3)
generateRSAKeyPair(50-50)signRSA(50-50)verifyRSA(50-50)common/src/utils/bytes.ts (1)
splitToWords(140-153)
common/src/utils/trees.ts (1)
common/src/utils/hash.ts (1)
packBytesAndPoseidon(185-188)
circuits/tests/disclose/vc_and_disclose_selfrica.test.ts (3)
common/src/utils/selfrica/generateInputs.ts (3)
generateCircuitInput(98-158)NON_OFAC_DUMMY_INPUT(55-72)OFAC_DUMMY_INPUT(36-53)common/src/utils/selfrica/constants.ts (1)
SELFRICA_MAX_LENGTH(37-37)common/src/utils/selfrica/types.ts (1)
serializeSmileData(22-99)
🪛 GitHub Actions: Circuits CI
circuits/tests/disclose/vc_and_disclose_selfrica.test.ts
[warning] 1-1: Code style issues found in file. Run 'Prettier --write' to fix.
🪛 GitHub Actions: Common CI
common/src/utils/selfrica/decodePublicSignals.ts
[warning] 1-1: Code style issues found in this file. Run 'Prettier --write' to fix.
common/src/utils/selfrica/types.ts
[warning] 1-1: Code style issues found in this file. Run 'Prettier --write' to fix.
common/src/utils/selfrica/rsa.ts
[warning] 1-1: Code style issues found in this file. Run 'Prettier --write' to fix.
common/src/utils/selfrica/generateInputs.ts
[warning] 1-1: Code style issues found in this file. Run 'Prettier --write' to fix.
common/src/utils/trees.ts
[warning] 1-1: Code style issues found in this file. Run 'Prettier --write' to fix.
common/src/utils/selfrica/constants.ts
[warning] 1-1: Code style issues found in this file. Run 'Prettier --write' to fix.
🪛 GitHub Actions: Workspace CI
common/src/utils/selfrica/decodePublicSignals.ts
[error] 1-1: Command 'yarn lint' failed with exit code 1.
circuits/circuits/utils/selfrica/disclose/country_not_in_list.circom
[error] 1-1: Command 'yarn lint' failed with exit code 1.
circuits/circuits/utils/selfrica/constants.circom
[error] 1-1: Command 'yarn lint' failed with exit code 1.
common/src/utils/selfrica/types.ts
[error] 1-1: Command 'yarn lint' failed with exit code 1.
circuits/circuits/utils/selfrica/disclose/ofac/ofac_name_yob_selfrica.circom
[error] 1-1: Command 'yarn lint' failed with exit code 1.
common/src/utils/selfrica/rsa.ts
[error] 1-1: Command 'yarn lint' failed with exit code 1.
circuits/circuits/utils/selfrica/date/dateIsLess.circom
[error] 1-1: Command 'yarn lint' failed with exit code 1.
circuits/package.json
[error] 1-1: Command 'yarn lint' failed with exit code 1.
circuits/tests/disclose/vc_and_disclose_aadhaar.test.ts
[error] 1-1: Command 'yarn lint' failed with exit code 1.
circuits/circuits/utils/selfrica/date/isOlderThan.circom
[error] 1-1: Command 'yarn lint' failed with exit code 1.
circuits/circuits/disclose/vc_and_disclose_selfrica.circom
[error] 1-1: Command 'yarn lint' failed with exit code 1.
circuits/circuits/utils/selfrica/disclose/ofac/ofac_name_dob_selfrica.circom
[error] 1-1: Command 'yarn lint' failed with exit code 1.
common/src/utils/selfrica/generateInputs.ts
[error] 1-1: Command 'yarn lint' failed with exit code 1.
common/src/utils/trees.ts
[error] 1-1: Command 'yarn lint' failed with exit code 1.
common/src/utils/selfrica/constants.ts
[error] 1-1: Command 'yarn lint' failed with exit code 1.
circuits/circuits/utils/selfrica/date/isValid.circom
[error] 1-1: Command 'yarn lint' failed with exit code 1.
circuits/circuits/utils/selfrica/disclose/disclose.circom
[error] 1-1: Command 'yarn lint' failed with exit code 1.
circuits/tests/disclose/vc_and_disclose_selfrica.test.ts
[error] 1-1: Command 'yarn lint' failed with exit code 1.
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: build-deps
🔇 Additional comments (8)
circuits/package.json (1)
24-24: LGTM! Script follows existing pattern.The new test script is consistent with other disclose test scripts and correctly targets the selfrica test file.
circuits/circuits/utils/selfrica/disclose/ofac/ofac_name_yob_selfrica.circom (2)
26-31: Verify YoB extraction assumes correct DOB format.The code extracts the first 4 bytes of
DOBas the year of birth, assuming DOB is in YYYYMMDD format. Ensure upstream validation (e.g.,IsValidFullYearor data provider guarantees) enforces this format—otherwise, malformed dates could produce incorrect YoB hashes and bypass OFAC checks.
33-33: Hash input order is consistent in circuit and SMT builder.circuits/circuits/utils/selfrica/date/dateIsLess.circom (2)
16-65: Component-wise comparison does not handle calendar arithmetic.The circuit performs component-wise comparison (year, month, day) without accounting for varying month lengths or leap years. For example, 2024-02-30 < 2024-03-01 would evaluate as true even though Feb 30 is invalid. As per the learnings, ensure "day-level comparison with correct date arithmetic" by validating dates with
IsValidFullYearbefore comparing, or document this limitation clearly.Based on learnings
16-65: Add an assertion in isOlderThan.circom to enforce the age check.isValid.circom correctly constrains
DateIsLessFullYear.outvia1 === is_valid.out;, but isOlderThan.circom merely exportsout <== is_older_than.outwithout asserting it. After thesignal output out <== is_older_than.out;line, add:1 === is_older_than.out;to guarantee the circuit fails when the comparison is false.
⛔ Skipped due to learnings
Learnt from: CR PR: selfxyz/self#0 File: .cursor/rules/compliance-verification.mdc:0-0 Timestamp: 2025-08-24T18:53:11.221Z Learning: Applies to circuits/circuits/disclose/disclose.circom : Ensure the circuit performs day-level comparison with correct date arithmeticLearnt from: CR PR: selfxyz/self#0 File: .cursor/rules/compliance-verification.mdc:0-0 Timestamp: 2025-08-24T18:53:11.221Z Learning: Applies to circuits/circuits/disclose/disclose.circom : Implement the age verification circuit at circuits/circuits/disclose/disclose.circom with private DOB input, public minimum-age input, and boolean outputcircuits/circuits/utils/selfrica/constants.circom (1)
3-101: Constants alignment verified. All Circom index and length constants match their TypeScript counterparts.common/src/utils/selfrica/generateInputs.ts (1)
132-139: SELFRICA_MAX_LENGTH matches expected 266 No action required.common/src/utils/selfrica/constants.ts (1)
1-37: Add schema documentation and verify field lengths
- SELFRICA_PHONE_NUMBER_LENGTH = 12 may not support full E.164 phone numbers (up to 15 digits).
- SELFRICA_FULL_NAME_LENGTH = 40 could truncate longer international names—consider increasing to 60–80 characters.
- No inline documentation describes each field’s encoding, length rationale, padding, or total layout.
Could you confirm your expected phone-number and full-name formats and update lengths or docs accordingly?
| signal input user_identifier; | ||
| signal input current_date[8]; |
There was a problem hiding this comment.
Bind the public user_identifier to the derived commitment.
user_identifier is declared as a public input but never constrained, letting a prover claim any value while still satisfying the circuit, which breaks the link between the proof and the user identity.
Add an equality constraint before exposing the commitment so the public value matches the hashed signature:
signal output identity_commitment <== idCommCal.out;
+ user_identifier === idCommCal.out;Also applies to: 156-157
🤖 Prompt for AI Agents
In circuits/circuits/disclose/vc_and_disclose_selfrica.circom around lines 43-44
(and similarly at 156-157), the public input signal user_identifier is declared
but never constrained; add an equality constraint that binds user_identifier to
the derived value (the hash/derived commitment of the signature or identifier
computed inside the circuit) before the commitment is exposed so the public
input cannot be arbitrarily chosen by the prover; place the constraint
immediately after computing the derived_hash/commitment (e.g., derivedHash ===
user_identifier) and before any signal is marked public or emitted.
| // component id_num_hasher = Sha256Bytes(SMILE_ID_PADDED()); | ||
| id_num_hasher.paddedIn <== id_num; | ||
| id_num_hasher.paddedInLength <== SMILE_ID_PADDED(); |
There was a problem hiding this comment.
Instantiate the ID number hasher.
id_num_hasher is referenced but never instantiated (the only declaration is commented out), so the circuit will not compile and the ID-number SHA-256 constraints never materialize.
Apply this diff so the hasher exists before wiring it:
- // component id_num_hasher = Sha256Bytes(SMILE_ID_PADDED());
+ component id_num_hasher = Sha256Bytes(SMILE_ID_PADDED());📝 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.
| // component id_num_hasher = Sha256Bytes(SMILE_ID_PADDED()); | |
| id_num_hasher.paddedIn <== id_num; | |
| id_num_hasher.paddedInLength <== SMILE_ID_PADDED(); | |
| component id_num_hasher = Sha256Bytes(SMILE_ID_PADDED()); | |
| id_num_hasher.paddedIn <== id_num; | |
| id_num_hasher.paddedInLength <== SMILE_ID_PADDED(); |
🤖 Prompt for AI Agents
In circuits/circuits/disclose/vc_and_disclose_selfrica.circom around lines 107
to 109, the id_num_hasher is wired but not instantiated (the component
declaration is commented out), so add back a component instantiation like:
declare id_num_hasher as Sha256Bytes(SMILE_ID_PADDED()) before assigning its
inputs, ensuring the component name matches the wiring (id_num_hasher) and
SMILE_ID_PADDED() is used for the constructor parameter so the SHA-256
constraints are generated.
| template DateIsLessFullYear() { | ||
| signal input day_1; | ||
| signal input day_2; | ||
|
|
||
| signal input month_1; | ||
| signal input month_2; | ||
|
|
||
| signal input year_1; | ||
| signal input year_2; | ||
|
|
||
| signal output out; | ||
|
|
||
| // ---- | ||
| component year_less = LessThan(8); | ||
| year_less.in[0] <== year_1; | ||
| year_less.in[1] <== year_2; | ||
| signal is_year_less <== year_less.out; | ||
|
|
||
| component month_less = LessThan(8); | ||
| month_less.in[0] <== month_1; | ||
| month_less.in[1] <== month_2; | ||
| signal is_month_less <== month_less.out; | ||
|
|
||
| component day_less = LessThan(8); | ||
| day_less.in[0] <== day_1; | ||
| day_less.in[1] <== day_2; | ||
| signal is_day_less <== day_less.out; | ||
|
|
||
| // ---- | ||
| component year_equal = IsEqual(); | ||
| year_equal.in[0] <== year_1; | ||
| year_equal.in[1] <== year_2; | ||
| signal is_year_equal <== year_equal.out; | ||
|
|
||
| component month_equal = IsEqual(); | ||
| month_equal.in[0] <== month_1; | ||
| month_equal.in[1] <== month_2; | ||
| signal is_month_equal <== month_equal.out; | ||
|
|
||
| // ---- | ||
| signal is_year_equal_and_month_less <== (is_year_equal * is_month_less); | ||
| signal is_year_equal_and_month_equal <== (is_year_equal * is_month_equal); | ||
| signal is_year_equal_and_month_equal_and_day_less <== (is_year_equal_and_month_equal * is_day_less); | ||
|
|
||
| component greater_than = GreaterThan(3); | ||
| greater_than.in[0] <== is_year_less + is_year_equal_and_month_less + is_year_equal_and_month_equal_and_day_less; | ||
| greater_than.in[1] <== 0; | ||
|
|
||
| out <== greater_than.out; | ||
| } |
There was a problem hiding this comment.
No input range validation for day, month, year.
The circuit does not constrain inputs to valid ranges (day 1-31, month 1-12, year > 0). Invalid dates like 2024-02-30 or 2024-13-01 could produce incorrect comparison results. Ensure the caller (e.g., IsValidFullYear) validates date components before invoking this template, or add range checks here.
Consider adding range constraints or verify that IsValidFullYear enforces them:
// Example range constraints (add to template)
component day1_valid = LessThan(8);
day1_valid.in[0] <== day_1;
day1_valid.in[1] <== 32; // day must be < 32
day1_valid.out === 1;
component month1_valid = LessThan(8);
month1_valid.in[0] <== month_1;
month1_valid.in[1] <== 13; // month must be < 13
month1_valid.out === 1;
// Similar for day_2, month_2🤖 Prompt for AI Agents
In circuits/circuits/utils/selfrica/date/dateIsLess.circom around lines 16-65,
the template accepts day/month/year inputs without validating ranges; add
explicit range checks for both date inputs (day in 1..31, month in 1..12, year >
0) or call/require the existing IsValidFullYear verifier before using this
template. Implement these checks by adding comparator components (e.g.,
LessThan/GreaterThan or custom range check) for day_1, month_1, year_1 and
day_2, month_2, year_2 and assert their outputs (e.g., out === 1) so invalid
values fail the circuit; alternatively, document and enforce that callers must
run IsValidFullYear on both dates before invoking DateIsLessFullYear.
| for (var i = 0; i < 8; i++) { | ||
| validity_date_num[i] <== validity_date_ascii[i] - ASCII_rotation; | ||
| } |
There was a problem hiding this comment.
Constrain ASCII digits before date comparison
validity_date_ascii[i] - 48 is unconstrained, so a prover can feed values outside '0'..'9' (e.g., 48 + 500) and fabricate arbitrary years/months to bypass the expiry gate. Please enforce both ASCII range checks (48 ≤ char ≤ 57) and digit bounds (0 ≤ digit ≤ 9) before wiring into DateIsLessFullYear. Example patch:
validity_date_num[i] <== validity_date_ascii[i] - ASCII_rotation;
+
+ component digit_lt_10 = LessThan(4);
+ digit_lt_10.in[0] <== validity_date_num[i];
+ digit_lt_10.in[1] <== TEN;
+ digit_lt_10.out === 1;
+
+ component ascii_ge_48 = GreaterEqThan(8);
+ ascii_ge_48.in[0] <== validity_date_ascii[i];
+ ascii_ge_48.in[1] <== ASCII_rotation;
+ ascii_ge_48.out === 1;
+
+ signal ASCII_upper_bound <== ASCII_rotation + TEN;
+ component ascii_lt_58 = LessThan(8);
+ ascii_lt_58.in[0] <== validity_date_ascii[i];
+ ascii_lt_58.in[1] <== ASCII_upper_bound;
+ ascii_lt_58.out === 1;As per coding guidelines
📝 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.
| for (var i = 0; i < 8; i++) { | |
| validity_date_num[i] <== validity_date_ascii[i] - ASCII_rotation; | |
| } | |
| for (var i = 0; i < 8; i++) { | |
| // Convert ASCII codepoint to digit | |
| validity_date_num[i] <== validity_date_ascii[i] - ASCII_rotation; | |
| // Enforce digit is in [0..9] | |
| component digit_lt_10 = LessThan(4); | |
| digit_lt_10.in[0] <== validity_date_num[i]; | |
| digit_lt_10.in[1] <== TEN; | |
| digit_lt_10.out === 1; | |
| // Enforce ASCII code is ≥ '0' (48) | |
| component ascii_ge_48 = GreaterEqThan(8); | |
| ascii_ge_48.in[0] <== validity_date_ascii[i]; | |
| ascii_ge_48.in[1] <== ASCII_rotation; | |
| ascii_ge_48.out === 1; | |
| // Enforce ASCII code is < '0' + 10 (i.e. ≤ '9' (57)) | |
| signal ASCII_upper_bound <== ASCII_rotation + TEN; | |
| component ascii_lt_58 = LessThan(8); | |
| ascii_lt_58.in[0] <== validity_date_ascii[i]; | |
| ascii_lt_58.in[1] <== ASCII_upper_bound; | |
| ascii_lt_58.out === 1; | |
| } |
🤖 Prompt for AI Agents
In circuits/circuits/utils/selfrica/date/isValid.circom around lines 21-23, the
subtraction validity_date_ascii[i] - 48 is currently unconstrained allowing
non-digit ASCII values; add explicit constraints that each
validity_date_ascii[i] is between 48 and 57 inclusive, then compute the digit
(validity_date_num[i]) and constrain that digit between 0 and 9 before passing
digits into DateIsLessFullYear. Concretely: for each i enforce
validity_date_ascii[i] >= 48 and <= 57, wire validity_date_num[i] <==
validity_date_ascii[i] - 48, and enforce validity_date_num[i] >= 0 and <= 9 (or
equivalent range constraints) prior to the date-comparison wiring.
| signal input selector_smile_data[selfrica_max_length]; | ||
|
|
There was a problem hiding this comment.
Enforce selector_smile_data as boolean bits to keep PackBytes sound and prevent malformed reveals
selector_smile_data isn’t constrained to {0,1} but is multiplied into bytes and later packed. Non‑boolean values can break byte bounds and weaken gating semantics. Constrain each selector bit.
Apply this diff near the selectors:
signal input selector_smile_data[selfrica_max_length];
+// Ensure all selectors are boolean
+for (var i = 0; i < selfrica_max_length; i++) {
+ selector_smile_data[i] * (selector_smile_data[i] - 1) === 0;
+}Also applies to: 74-76
🤖 Prompt for AI Agents
In circuits/circuits/utils/selfrica/disclose/disclose.circom around lines 19-20
(and similarly update lines 74-76), the signal input
selector_smile_data[selfrica_max_length] is not constrained to boolean values;
add constraints that force each selector bit to be 0 or 1 (e.g., for each i
enforce selector_smile_data[i] * (selector_smile_data[i] - 1) == 0) so packing
and byte multiplications remain sound and prevent malformed reveals. Ensure you
apply the same boolean constraint pattern to any other selector arrays mentioned
at lines 74-76.
| // Sign nullifier with RSA | ||
| const idNumber = Buffer.from(msgArray.slice(30, 30 + 20)); | ||
|
|
||
| const id_num_rsaSig = signRSA(idNumber, privateKey); | ||
| console.assert(verifyRSA(idNumber, id_num_rsaSig, publicKey) == true, "Invalid nullifier RSA signature"); | ||
|
|
||
| // Convert nullifier RSA signature to limbs | ||
| const nullifierSigBigInt = BigInt('0x' + id_num_rsaSig.toString('hex')); | ||
|
|
There was a problem hiding this comment.
Avoid hardcoded byte offsets when signing the nullifier; derive from constants
Slicing idNumber as msgArray.slice(30, 30 + 20) is brittle. If field ordering or sizes change, you’ll sign the wrong segment and break nullifier integrity.
Refactor to compute offsets from Selfrica constants:
-import { SELFRICA_DOB_INDEX, SELFRICA_DOB_LENGTH, SELFRICA_FULL_NAME_INDEX, SELFRICA_FULL_NAME_LENGTH, SELFRICA_MAX_LENGTH } from "./constants.js";
+import {
+ SELFRICA_DOB_INDEX, SELFRICA_DOB_LENGTH,
+ SELFRICA_FULL_NAME_INDEX, SELFRICA_FULL_NAME_LENGTH,
+ SELFRICA_ID_NUMBER_INDEX, SELFRICA_ID_NUMBER_LENGTH,
+ SELFRICA_MAX_LENGTH
+} from "./constants.js";
...
- const idNumber = Buffer.from(msgArray.slice(30, 30 + 20));
+ const idNumber = Buffer.from(
+ msgArray.slice(SELFRICA_ID_NUMBER_INDEX, SELFRICA_ID_NUMBER_INDEX + SELFRICA_ID_NUMBER_LENGTH)
+ );As per coding guidelines.
Also applies to: 185-192
🤖 Prompt for AI Agents
In common/src/utils/selfrica/generateInputs.ts around lines 118 to 126 (and
similarly lines 185 to 192), the code uses hardcoded slice indices
msgArray.slice(30, 30 + 20) to extract the idNumber; instead compute the start
and length from the Selfrica field-size and field-order constants (e.g.,
BASE_OFFSET + offsets.nullifier or equivalent), or use a helper that returns the
nullifier byte-range, then replace the hardcoded numbers with those computed
values so the slice always targets the correct bytes even if field sizes or
ordering change.
| export const serializeSmileData = (smileData: SmileData) => { | ||
| //ensure max length of each field | ||
| let serializedData = ''; | ||
| smileData.country = smileData.country.toUpperCase().padEnd(constants.SELFRICA_COUNTRY_LENGTH, '\0'); | ||
| smileData.idType = smileData.idType.toUpperCase().padEnd(constants.SELFRICA_ID_TYPE_LENGTH, '\0'); | ||
| smileData.idNumber = smileData.idNumber.padEnd(constants.SELFRICA_ID_NUMBER_LENGTH, '\0'); | ||
| smileData.issuanceDate = smileData.issuanceDate.padEnd(constants.SELFRICA_ISSUANCE_DATE_LENGTH, '\0'); | ||
| smileData.expiryDate = smileData.expiryDate.padEnd(constants.SELFRICA_EXPIRY_DATE_LENGTH, '\0'); | ||
| smileData.fullName = smileData.fullName.padEnd(constants.SELFRICA_FULL_NAME_LENGTH, '\0'); | ||
| smileData.dob = smileData.dob.padEnd(constants.SELFRICA_DOB_LENGTH, '\0'); | ||
| smileData.photoHash = smileData.photoHash.padEnd(constants.SELFRICA_PHOTO_HASH_LENGTH, '\0'); | ||
| smileData.phoneNumber = smileData.phoneNumber.padEnd(constants.SELFRICA_PHONE_NUMBER_LENGTH, '\0'); | ||
| smileData.document = smileData.document.padEnd(constants.SELFRICA_DOCUMENT_LENGTH, '\0'); | ||
| smileData.gender = smileData.gender.padEnd(constants.SELFRICA_GENDER_LENGTH, '\0'); | ||
| smileData.address = smileData.address.padEnd(constants.SELFRICA_ADDRESS_LENGTH, '\0'); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid mutating input in serializeSmileData to prevent cross-test/data pollution.
serializeSmileData pads/uppercases fields in-place on the passed object. This can corrupt shared constants (e.g., NON_OFAC_DUMMY_INPUT) used elsewhere.
export const serializeSmileData = (smileData: SmileData) => {
- //ensure max length of each field
- let serializedData = '';
- smileData.country = smileData.country.toUpperCase().padEnd(constants.SELFRICA_COUNTRY_LENGTH, '\0');
- smileData.idType = smileData.idType.toUpperCase().padEnd(constants.SELFRICA_ID_TYPE_LENGTH, '\0');
+ // ensure max length of each field (no mutation)
+ let serializedData = '';
+ const country = smileData.country.toUpperCase().padEnd(constants.SELFRICA_COUNTRY_LENGTH, '\0');
+ const idType = smileData.idType.toUpperCase().padEnd(constants.SELFRICA_ID_TYPE_LENGTH, '\0');
/* repeat for others as locals: idNumber, issuanceDate, ... address */
- if (smileData.country.length !== constants.SELFRICA_COUNTRY_LENGTH) {
+ if (country.length !== constants.SELFRICA_COUNTRY_LENGTH) {
throw new Error(`Country must be ${constants.SELFRICA_COUNTRY_LENGTH} characters`);
}
- serializedData += smileData.country;
+ serializedData += country;
- if (smileData.idType.length !== constants.SELFRICA_ID_TYPE_LENGTH) {
+ if (idType.length !== constants.SELFRICA_ID_TYPE_LENGTH) {
throw new Error(`ID type must be ${constants.SELFRICA_ID_TYPE_LENGTH} characters`);
}
- serializedData += smileData.idType;
+ serializedData += idType;
// ...apply same pattern for all remaining fieldsAlso applies to: 41-97
🤖 Prompt for AI Agents
common/src/utils/selfrica/types.ts around lines 22-37 (also applies to 41-97):
the function currently mutates the input SmileData by uppercasing/padding fields
in-place; instead create a non-mutating local copy or read each field into local
variables and apply toUpperCase()/padEnd() there, build the serialized string
from those locals and return it without modifying the original object; ensure
all fields are normalized using the constants lengths and that the function does
not change smileData properties.
| export type SelfricaPublicInput = { | ||
| pubKeyX: string, | ||
| pubKeyY: string, | ||
| scope: string, | ||
| ofac_name_dob_smt_root: string[], | ||
| ofac_name_yob_smt_root: string[], | ||
| attestation_id: string[], | ||
| } |
There was a problem hiding this comment.
Public input type/shape mismatch (pubKeyX/pubKeyY vs pubKey). Align the API.
SelfricaPublicInput declares pubKeyX/pubKeyY, but getPublicInput returns pubKey (RSA modulus limbs). This inconsistency will break consumers and is confusing.
Option A (RSA: keep limbs array):
-export type SelfricaPublicInput = {
- pubKeyX: string,
- pubKeyY: string,
- scope: string,
- ofac_name_dob_smt_root: string[],
- ofac_name_yob_smt_root: string[],
- attestation_id: string[],
-}
+export type SelfricaPublicInput = {
+ pubKey: string[],
+ msg_sig: string[],
+ id_num_sig: string[],
+ scope: string,
+ ofac_name_dob_smt_root: string[],
+ ofac_name_yob_smt_root: string[],
+ attestation_id: string[],
+}
-export const getPublicInput = (input: SelfricaCircuitInput) => {
+export const getPublicInput = (input: SelfricaCircuitInput): SelfricaPublicInput => {
return {
pubKey: input.pubKey,
msg_sig: input.msg_sig,
id_num_sig: input.id_num_sig,
ofac_name_dob_smt_root: input.ofac_name_dob_smt_root,
ofac_name_yob_smt_root: input.ofac_name_yob_smt_root,
attestation_id: ['4'],
}
}If elliptic keys are actually intended, split and rename the source accordingly. Based on learnings.
Also applies to: 131-140
🤖 Prompt for AI Agents
In common/src/utils/selfrica/types.ts around lines 122-129 (and also update
lines 131-140), the declared SelfricaPublicInput shape uses pubKeyX/pubKeyY but
the runtime/getPublicInput returns an RSA-style pubKey (array of limbs); change
the type to match the runtime by replacing pubKeyX and pubKeyY with a single
pubKey: string[] field (and update the related type entries at lines 131-140
accordingly), then update any references/tests to use input.pubKey array instead
of pubKeyX/pubKeyY.
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (3)
common/src/utils/selfrica/types.ts (1)
122-139: RealignSelfricaPublicInputwith the actual return payload
SelfricaPublicInputadvertisespubKeyX,pubKeyY, andscope, butgetPublicInputreturnspubKey(array), omitsscope, and adds signatures that are not part of the declared type. Downstream callers relying on the declared shape will either see runtime failures or silently drop the data they need. Please make the type and function agree (and includescope) so public-input derivation is reliable.-export type SelfricaPublicInput = { - pubKeyX: string, - pubKeyY: string, - scope: string, - ofac_name_dob_smt_root: string[], - ofac_name_yob_smt_root: string[], - attestation_id: string[], -} - -export const getPublicInput = (input: SelfricaCircuitInput) => { - return { - pubKey: input.pubKey, - msg_sig: input.msg_sig, - id_num_sig: input.id_num_sig, - ofac_name_dob_smt_root: input.ofac_name_dob_smt_root, - ofac_name_yob_smt_root: input.ofac_name_yob_smt_root, - attestation_id: ['4'], - } -} +export type SelfricaPublicInput = { + pubKey: string[], + msg_sig: string[], + id_num_sig: string[], + scope: string, + ofac_name_dob_smt_root: string[], + ofac_name_yob_smt_root: string[], + attestation_id: string[], +}; + +export const getPublicInput = (input: SelfricaCircuitInput): SelfricaPublicInput => ({ + pubKey: input.pubKey, + msg_sig: input.msg_sig, + id_num_sig: input.id_num_sig, + scope: input.scope, + ofac_name_dob_smt_root: input.ofac_name_dob_smt_root, + ofac_name_yob_smt_root: input.ofac_name_yob_smt_root, + attestation_id: ['4'], +});common/src/utils/selfrica/generateInputs.ts (2)
137-155: Fix hardcoded majority age and current date.
current_dateis still set to "20240101" andmajority_age_ASCIIencodes ASCII for “001”. Both values contradict the dummy payload (20250101 / age 20), so the circuit will fail date/age checks. Please update them to the correct values (or compute them dynamically).
200-220: Replace stale current_date in real-data path.The real-data helper still emits "20240101" for
current_date, so proofs generated on/after 2025-10-01 will fail the circuit’s freshness check. Align this with the actual current date (or thread it from the caller).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
circuits/circuits/disclose/vc_and_disclose_selfrica.circom(1 hunks)circuits/circuits/utils/selfrica/constants.circom(1 hunks)circuits/circuits/utils/selfrica/date/dateIsLess.circom(1 hunks)circuits/circuits/utils/selfrica/date/isOlderThan.circom(1 hunks)circuits/circuits/utils/selfrica/date/isValid.circom(1 hunks)circuits/circuits/utils/selfrica/disclose/disclose.circom(1 hunks)circuits/circuits/utils/selfrica/disclose/ofac/ofac_name_dob_selfrica.circom(1 hunks)circuits/circuits/utils/selfrica/disclose/ofac/ofac_name_yob_selfrica.circom(1 hunks)circuits/package.json(1 hunks)circuits/tests/disclose/vc_and_disclose_aadhaar.test.ts(2 hunks)circuits/tests/disclose/vc_and_disclose_selfrica.test.ts(1 hunks)common/src/utils/selfrica/constants.ts(1 hunks)common/src/utils/selfrica/decodePublicSignals.ts(1 hunks)common/src/utils/selfrica/generateInputs.ts(1 hunks)common/src/utils/selfrica/rsa.ts(1 hunks)common/src/utils/selfrica/types.ts(1 hunks)common/src/utils/trees.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: 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:
common/src/utils/selfrica/decodePublicSignals.tscommon/src/utils/selfrica/rsa.tscommon/src/utils/selfrica/generateInputs.tscommon/src/utils/selfrica/constants.tscircuits/tests/disclose/vc_and_disclose_aadhaar.test.tscommon/src/utils/trees.tscommon/src/utils/selfrica/types.tscircuits/tests/disclose/vc_and_disclose_selfrica.test.ts
common/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
common/src/**/*.{ts,tsx,js,jsx}: Review shared utilities for:
- Reusability and modular design
- Type safety and error handling
- Side-effect management
- Documentation and naming clarity
Files:
common/src/utils/selfrica/decodePublicSignals.tscommon/src/utils/selfrica/rsa.tscommon/src/utils/selfrica/generateInputs.tscommon/src/utils/selfrica/constants.tscommon/src/utils/trees.tscommon/src/utils/selfrica/types.ts
circuits/**/*.circom
⚙️ CodeRabbit configuration file
circuits/**/*.circom: Review ZK circuit code for:
- Circuit correctness and completeness
- Constraint efficiency
- Input validation
- Security considerations for zero-knowledge proofs
Files:
circuits/circuits/utils/selfrica/disclose/ofac/ofac_name_yob_selfrica.circomcircuits/circuits/utils/selfrica/date/isValid.circomcircuits/circuits/utils/selfrica/date/dateIsLess.circomcircuits/circuits/utils/selfrica/date/isOlderThan.circomcircuits/circuits/utils/selfrica/disclose/disclose.circomcircuits/circuits/utils/selfrica/constants.circomcircuits/circuits/disclose/vc_and_disclose_selfrica.circomcircuits/circuits/utils/selfrica/disclose/ofac/ofac_name_dob_selfrica.circom
**/*.{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:
circuits/tests/disclose/vc_and_disclose_aadhaar.test.tscircuits/tests/disclose/vc_and_disclose_selfrica.test.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Integrate forbidden-country non-inclusion proof into the disclosure proof circuit
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Implement the age verification circuit at circuits/circuits/disclose/disclose.circom with private DOB input, public minimum-age input, and boolean output
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to noir/crates/dg1/src/ofac/*.nr : OFAC Compliance System implements three-tier verification (name-based, name + DOB, passport number), and Merkle tree-based sanctions list verification.
Applied to files:
circuits/circuits/utils/selfrica/disclose/ofac/ofac_name_yob_selfrica.circomcircuits/circuits/utils/selfrica/disclose/ofac/ofac_name_dob_selfrica.circom
📚 Learning: 2025-08-24T18:53:11.221Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Ensure the circuit performs day-level comparison with correct date arithmetic
Applied to files:
circuits/circuits/utils/selfrica/date/dateIsLess.circom
📚 Learning: 2025-08-24T18:53:11.221Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Integrate forbidden-country non-inclusion proof into the disclosure proof circuit
Applied to files:
circuits/circuits/utils/selfrica/disclose/disclose.circomcircuits/circuits/disclose/vc_and_disclose_selfrica.circom
📚 Learning: 2025-08-24T18:53:11.221Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Implement the age verification circuit at circuits/circuits/disclose/disclose.circom with private DOB input, public minimum-age input, and boolean output
Applied to files:
circuits/circuits/utils/selfrica/disclose/disclose.circomcircuits/circuits/disclose/vc_and_disclose_selfrica.circom
🧬 Code graph analysis (4)
common/src/utils/selfrica/decodePublicSignals.ts (1)
common/src/utils/selfrica/constants.ts (16)
SELFRICA_PUBLIC_SIGNALS_REVEALED_DATA_PACKED(42-42)SELFRICA_PUBLIC_SIGNALS_REVEALED_DATA_PACKED_LENGTH(43-43)SELFRICA_MAX_LENGTH(37-37)SELFRICA_PUBLIC_SIGNALS_FORBIDDEN_COUNTRIES_PACKED(45-45)SELFRICA_PUBLIC_SIGNALS_FORBIDDEN_COUNTRIES_PACKED_LENGTH(46-46)SELFRICA_PUBLIC_SIGNALS_IDENTITY_COMMITMENT(48-48)SELFRICA_PUBLIC_SIGNALS_NULLIFIER(49-49)SELFRICA_PUBLIC_SIGNALS_PUBKEY_X(50-50)SELFRICA_PUBLIC_SIGNALS_PUBKEY_Y(51-51)SELFRICA_PUBLIC_SIGNALS_SCOPE(52-52)SELFRICA_PUBLIC_SIGNALS_OFAC_NAME_DOB_SMT_ROOT(53-53)SELFRICA_PUBLIC_SIGNALS_OFAC_NAME_YOB_SMT_ROOT(54-54)SELFRICA_PUBLIC_SIGNALS_ATTESTATION_ID(55-55)SELFRICA_PUBLIC_SIGNALS_CURRENT_DATE(57-57)SELFRICA_PUBLIC_SIGNALS_CURRENT_DATE_LENGTH(58-58)SELFRICA_PUBLIC_SIGNALS_USER_IDENTIFIER(56-56)
common/src/utils/selfrica/generateInputs.ts (5)
common/src/utils/selfrica/constants.ts (5)
SELFRICA_MAX_LENGTH(37-37)SELFRICA_FULL_NAME_INDEX(16-16)SELFRICA_FULL_NAME_LENGTH(17-17)SELFRICA_DOB_INDEX(19-19)SELFRICA_DOB_LENGTH(20-20)common/src/utils/selfrica/types.ts (3)
SmileData(3-20)serializeSmileData(22-99)SelfricaCircuitInput(101-120)common/src/utils/trees.ts (3)
getNameDobLeafSelfrica(767-772)getNameYobLeafSelfrica(809-815)generateSMTProof(176-225)common/src/utils/circuits/generateInputs.ts (1)
formatInput(58-78)common/src/utils/bytes.ts (1)
splitToWords(140-153)
common/src/utils/trees.ts (1)
common/src/utils/hash.ts (1)
packBytesAndPoseidon(185-188)
circuits/tests/disclose/vc_and_disclose_selfrica.test.ts (3)
common/src/utils/selfrica/generateInputs.ts (3)
generateCircuitInput(98-158)NON_OFAC_DUMMY_INPUT(55-72)OFAC_DUMMY_INPUT(36-53)common/src/utils/selfrica/constants.ts (1)
SELFRICA_MAX_LENGTH(37-37)common/src/utils/selfrica/types.ts (1)
serializeSmileData(22-99)
🪛 GitHub Actions: Circuits CI
circuits/tests/disclose/vc_and_disclose_selfrica.test.ts
[warning] Code style issues found in this file. Run 'Prettier --write' to fix.
🪛 GitHub Actions: Common CI
common/src/utils/selfrica/decodePublicSignals.ts
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
common/src/utils/selfrica/rsa.ts
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
common/src/utils/selfrica/generateInputs.ts
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
common/src/utils/selfrica/constants.ts
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
common/src/utils/trees.ts
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
common/src/utils/selfrica/types.ts
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
⏰ 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: test
- GitHub Check: build-android
- GitHub Check: build-ios
🔇 Additional comments (8)
circuits/package.json (1)
24-24: LGTM! Test script follows established patterns.The new test script is consistent with existing disclose test scripts and properly configured.
circuits/circuits/utils/selfrica/date/dateIsLess.circom (1)
56-64: Date comparison logic is correct.The three-tier comparison correctly implements date ordering: first by year, then by month (if years equal), then by day (if both year and month equal).
circuits/tests/disclose/vc_and_disclose_aadhaar.test.ts (2)
15-22: LGTM! Path resolution modernized correctly.The addition of
__dirnameand file loading updates properly handle ES module path resolution and make the test more maintainable.
59-59: Include path update is appropriate.Adding 'node_modules' to the include array ensures circomlib circuits can be properly resolved during compilation.
circuits/circuits/utils/selfrica/date/isValid.circom (2)
29-47: Year/month/day construction is correct.The digit-to-number conversion using powers of 10 correctly builds the full year, month, and day values from individual digits.
48-48: Validity constraint is properly enforced.The constraint
1 === is_valid.outcorrectly ensures the passport is valid (current date is before validity date).circuits/circuits/utils/selfrica/disclose/ofac/ofac_name_dob_selfrica.circom (2)
19-34: Hash composition approach is sound for OFAC verification.The two-tier hashing (separate name and DOB hashes, then combined) provides a clean structure that matches the OFAC name+DOB verification tier. Using Poseidon throughout is efficient for ZK circuits.
34-36: SMTVerify mode is correct for OFAC non-inclusion checks — all OFAC circuits consistently pass0(mode 0 = non-inclusion, mode 1 = inclusion) to assert the subject is not on the sanctions list.
| signal input selector_ofac; | ||
| signal input user_identifier; | ||
| signal input current_date[8]; | ||
| signal input majority_age_ASCII[3]; | ||
| signal input selector_older_than; | ||
|
|
||
| signal output attestation_id <== 4; | ||
|
|
||
| // Convert the two decimal inputs back to bit array | ||
| signal disclose_sel[selfrica_length]; | ||
|
|
||
| // Convert disclose_sel_low (first 133 bits) to bit array | ||
| component low_bits = Num2Bits(compressed_bit_len); | ||
| low_bits.in <== compressed_disclose_sel[0]; | ||
|
|
||
| // Convert disclose_sel_high (next 133 bits) to bit array | ||
| component high_bits = Num2Bits(compressed_bit_len); | ||
| high_bits.in <== compressed_disclose_sel[1]; | ||
|
|
||
| // Combine the bit arrays (little-endian format) | ||
| for(var i = 0; i < compressed_bit_len; i++){ | ||
| disclose_sel[i] <== low_bits.out[i]; | ||
| } | ||
| for(var i = 0; i < compressed_bit_len; i++){ | ||
| disclose_sel[compressed_bit_len + i] <== high_bits.out[i]; | ||
| } | ||
|
|
||
|
|
||
| //skiped paddedInLength to be within `ceil(log2(8 * maxByteLength))` bits bez we are using hardcoded values | ||
| component msg_hasher = Sha256Bytes(SMILE_DATA_PADDED()); | ||
| msg_hasher.paddedIn <== SmileID_data_padded; | ||
| msg_hasher.paddedInLength <== SMILE_DATA_PADDED(); | ||
|
|
||
| //verify Hash(smiledata) signatur | ||
| component msg_sig_verify = SignatureVerifier(1, n, k); | ||
| msg_sig_verify.hash <== msg_hasher.out; | ||
| msg_sig_verify.pubKey <== pubKey; | ||
| msg_sig_verify.signature <== msg_sig; | ||
|
|
||
|
|
||
| //Calculate IDNUMBER hash | ||
| signal id_num[SMILE_ID_PADDED()]; | ||
| var idNumberIdx = ID_NUMBER_INDEX(); | ||
|
|
||
| // Fill the first 20 bytes with actual ID number data | ||
| for (var i = 0; i < ID_NUMBER_LENGTH(); i++) { | ||
| id_num[i] <== SmileID_data_padded[idNumberIdx + i]; | ||
| } | ||
|
|
||
| // Add SHA-256 padding for 20-byte message | ||
| // Add padding bit '1' (0x80) | ||
| id_num[ID_NUMBER_LENGTH()] <== 128; // 0x80 in decimal | ||
|
|
||
| // Fill with zeros up to position 56 (64 - 8 for length field) | ||
| for (var i = ID_NUMBER_LENGTH() + 1; i < SMILE_ID_PADDED() - 8; i++) { | ||
| id_num[i] <== 0; | ||
| } | ||
|
|
||
| // Add 64-bit length field (20 bytes * 8 = 160 bits) | ||
| // Length in bits as 64-bit big-endian integer | ||
| for (var i = SMILE_ID_PADDED() - 8; i < SMILE_ID_PADDED() - 1; i++) { | ||
| id_num[i] <== 0; // High bytes are 0 for small lengths | ||
| } | ||
| id_num[SMILE_ID_PADDED() - 1] <== 160; // 20 * 8 = 160 bits | ||
|
|
||
| component id_num_hasher = Sha256Bytes(SMILE_ID_PADDED()); | ||
| id_num_hasher.paddedIn <== id_num; | ||
| id_num_hasher.paddedInLength <== SMILE_ID_PADDED(); | ||
|
|
||
| //verify Hash(IdNumber) signature | ||
| component id_num_sig_verify = SignatureVerifier(1, n, k); | ||
| id_num_sig_verify.hash <== id_num_hasher.out; | ||
| id_num_sig_verify.pubKey <== pubKey; | ||
| id_num_sig_verify.signature <== id_num_sig; | ||
|
|
||
|
|
||
| // Identity Commitment = Hash( IdNumCommit sig ) | ||
| component idCommCal = CustomHasher(k); | ||
| idCommCal.in <== msg_sig; | ||
|
|
||
| //Nullifier = HASH( nullifier sig , scope ) | ||
| component nullifierCal = CustomHasher(k + 1); | ||
| for (var i = 0; i < k; i++) { | ||
| nullifierCal.in[i] <== id_num_sig[i]; | ||
| } | ||
| nullifierCal.in[k] <== scope; | ||
|
|
||
| component disclose_circuit = DISCLOSE_SELFRICA(MAX_FORBIDDEN_COUNTRIES_LIST_LENGTH, namedobTreeLevels, nameyobTreeLevels); | ||
|
|
||
| for (var i = 0; i < selfrica_length; i++) { | ||
| disclose_circuit.smile_data[i] <== SmileID_data_padded[i]; | ||
| } | ||
| disclose_circuit.selector_smile_data <== disclose_sel; | ||
| disclose_circuit.forbidden_countries_list <== forbidden_countries_list; | ||
|
|
||
| disclose_circuit.ofac_name_dob_smt_leaf_key <== ofac_name_dob_smt_leaf_key; | ||
| disclose_circuit.ofac_name_dob_smt_root <== ofac_name_dob_smt_root; | ||
| disclose_circuit.ofac_name_dob_smt_siblings <== ofac_name_dob_smt_siblings; | ||
|
|
||
| disclose_circuit.ofac_name_yob_smt_leaf_key <== ofac_name_yob_smt_leaf_key; | ||
| disclose_circuit.ofac_name_yob_smt_root <== ofac_name_yob_smt_root; | ||
| disclose_circuit.ofac_name_yob_smt_siblings <== ofac_name_yob_smt_siblings; | ||
|
|
||
| disclose_circuit.selector_ofac <== selector_ofac; | ||
| disclose_circuit.current_date <== current_date; | ||
| disclose_circuit.majority_age_ASCII <== majority_age_ASCII; | ||
| disclose_circuit.selector_older_than <== selector_older_than; | ||
|
|
||
| var revealed_data_packed_chunk_length = computeIntChunkLength(selfrica_length + 2 + 3); | ||
| signal output revealedData_packed[revealed_data_packed_chunk_length] <== disclose_circuit.revealedData_packed; | ||
|
|
||
| var forbidden_countries_list_packed_chunk_length = computeIntChunkLength(MAX_FORBIDDEN_COUNTRIES_LIST_LENGTH * country_length); | ||
| signal output forbidden_countries_list_packed[forbidden_countries_list_packed_chunk_length] <== disclose_circuit.forbidden_countries_list_packed; | ||
|
|
||
| signal output identity_commitment <== idCommCal.out; | ||
| signal output nullifier <== nullifierCal.out; | ||
|
|
There was a problem hiding this comment.
Bind user_identifier to the derived identity commitment
user_identifier is declared as a public input but never constrained, so a prover can emit any identifier while the circuit still verifies, defeating downstream identity binding. Tie it to idCommCal.out (or whichever commitment you expect verifiers to trust).
component idCommCal = CustomHasher(k);
idCommCal.in <== msg_sig;
+ user_identifier === idCommCal.out;📝 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.
| signal input selector_ofac; | |
| signal input user_identifier; | |
| signal input current_date[8]; | |
| signal input majority_age_ASCII[3]; | |
| signal input selector_older_than; | |
| signal output attestation_id <== 4; | |
| // Convert the two decimal inputs back to bit array | |
| signal disclose_sel[selfrica_length]; | |
| // Convert disclose_sel_low (first 133 bits) to bit array | |
| component low_bits = Num2Bits(compressed_bit_len); | |
| low_bits.in <== compressed_disclose_sel[0]; | |
| // Convert disclose_sel_high (next 133 bits) to bit array | |
| component high_bits = Num2Bits(compressed_bit_len); | |
| high_bits.in <== compressed_disclose_sel[1]; | |
| // Combine the bit arrays (little-endian format) | |
| for(var i = 0; i < compressed_bit_len; i++){ | |
| disclose_sel[i] <== low_bits.out[i]; | |
| } | |
| for(var i = 0; i < compressed_bit_len; i++){ | |
| disclose_sel[compressed_bit_len + i] <== high_bits.out[i]; | |
| } | |
| //skiped paddedInLength to be within `ceil(log2(8 * maxByteLength))` bits bez we are using hardcoded values | |
| component msg_hasher = Sha256Bytes(SMILE_DATA_PADDED()); | |
| msg_hasher.paddedIn <== SmileID_data_padded; | |
| msg_hasher.paddedInLength <== SMILE_DATA_PADDED(); | |
| //verify Hash(smiledata) signatur | |
| component msg_sig_verify = SignatureVerifier(1, n, k); | |
| msg_sig_verify.hash <== msg_hasher.out; | |
| msg_sig_verify.pubKey <== pubKey; | |
| msg_sig_verify.signature <== msg_sig; | |
| //Calculate IDNUMBER hash | |
| signal id_num[SMILE_ID_PADDED()]; | |
| var idNumberIdx = ID_NUMBER_INDEX(); | |
| // Fill the first 20 bytes with actual ID number data | |
| for (var i = 0; i < ID_NUMBER_LENGTH(); i++) { | |
| id_num[i] <== SmileID_data_padded[idNumberIdx + i]; | |
| } | |
| // Add SHA-256 padding for 20-byte message | |
| // Add padding bit '1' (0x80) | |
| id_num[ID_NUMBER_LENGTH()] <== 128; // 0x80 in decimal | |
| // Fill with zeros up to position 56 (64 - 8 for length field) | |
| for (var i = ID_NUMBER_LENGTH() + 1; i < SMILE_ID_PADDED() - 8; i++) { | |
| id_num[i] <== 0; | |
| } | |
| // Add 64-bit length field (20 bytes * 8 = 160 bits) | |
| // Length in bits as 64-bit big-endian integer | |
| for (var i = SMILE_ID_PADDED() - 8; i < SMILE_ID_PADDED() - 1; i++) { | |
| id_num[i] <== 0; // High bytes are 0 for small lengths | |
| } | |
| id_num[SMILE_ID_PADDED() - 1] <== 160; // 20 * 8 = 160 bits | |
| component id_num_hasher = Sha256Bytes(SMILE_ID_PADDED()); | |
| id_num_hasher.paddedIn <== id_num; | |
| id_num_hasher.paddedInLength <== SMILE_ID_PADDED(); | |
| //verify Hash(IdNumber) signature | |
| component id_num_sig_verify = SignatureVerifier(1, n, k); | |
| id_num_sig_verify.hash <== id_num_hasher.out; | |
| id_num_sig_verify.pubKey <== pubKey; | |
| id_num_sig_verify.signature <== id_num_sig; | |
| // Identity Commitment = Hash( IdNumCommit sig ) | |
| component idCommCal = CustomHasher(k); | |
| idCommCal.in <== msg_sig; | |
| //Nullifier = HASH( nullifier sig , scope ) | |
| component nullifierCal = CustomHasher(k + 1); | |
| for (var i = 0; i < k; i++) { | |
| nullifierCal.in[i] <== id_num_sig[i]; | |
| } | |
| nullifierCal.in[k] <== scope; | |
| component disclose_circuit = DISCLOSE_SELFRICA(MAX_FORBIDDEN_COUNTRIES_LIST_LENGTH, namedobTreeLevels, nameyobTreeLevels); | |
| for (var i = 0; i < selfrica_length; i++) { | |
| disclose_circuit.smile_data[i] <== SmileID_data_padded[i]; | |
| } | |
| disclose_circuit.selector_smile_data <== disclose_sel; | |
| disclose_circuit.forbidden_countries_list <== forbidden_countries_list; | |
| disclose_circuit.ofac_name_dob_smt_leaf_key <== ofac_name_dob_smt_leaf_key; | |
| disclose_circuit.ofac_name_dob_smt_root <== ofac_name_dob_smt_root; | |
| disclose_circuit.ofac_name_dob_smt_siblings <== ofac_name_dob_smt_siblings; | |
| disclose_circuit.ofac_name_yob_smt_leaf_key <== ofac_name_yob_smt_leaf_key; | |
| disclose_circuit.ofac_name_yob_smt_root <== ofac_name_yob_smt_root; | |
| disclose_circuit.ofac_name_yob_smt_siblings <== ofac_name_yob_smt_siblings; | |
| disclose_circuit.selector_ofac <== selector_ofac; | |
| disclose_circuit.current_date <== current_date; | |
| disclose_circuit.majority_age_ASCII <== majority_age_ASCII; | |
| disclose_circuit.selector_older_than <== selector_older_than; | |
| var revealed_data_packed_chunk_length = computeIntChunkLength(selfrica_length + 2 + 3); | |
| signal output revealedData_packed[revealed_data_packed_chunk_length] <== disclose_circuit.revealedData_packed; | |
| var forbidden_countries_list_packed_chunk_length = computeIntChunkLength(MAX_FORBIDDEN_COUNTRIES_LIST_LENGTH * country_length); | |
| signal output forbidden_countries_list_packed[forbidden_countries_list_packed_chunk_length] <== disclose_circuit.forbidden_countries_list_packed; | |
| signal output identity_commitment <== idCommCal.out; | |
| signal output nullifier <== nullifierCal.out; | |
| // Identity Commitment = Hash( IdNumCommit sig ) | |
| component idCommCal = CustomHasher(k); | |
| idCommCal.in <== msg_sig; | |
| user_identifier === idCommCal.out; |
🤖 Prompt for AI Agents
In circuits/circuits/disclose/vc_and_disclose_selfrica.circom around lines 42 to
158, user_identifier is declared as a public input but never constrained; add a
binding after idCommCal is computed to force the public input to equal the
derived identity commitment (i.e., constrain user_identifier to idCommCal.out).
Ensure the types/sizes match (convert or index if idCommCal.out is an array) so
the circuit enforces user_identifier <== idCommCal.out (or element-wise
equality) immediately after idCommCal is available.
| /// @param year_1 is the year of the first date | ||
| /// @param year_2 is the year of the second date | ||
| /// @output out is the result of the comparison | ||
| /// @dev output is not constrained — verifier has to handle this check |
There was a problem hiding this comment.
Clarify constraint documentation.
The comment states "output is not constrained — verifier has to handle this check" but the output is actually constrained through the GreaterThan component at line 64. This documentation is misleading.
Update the comment to accurately reflect that the output is constrained:
-/// @dev output is not constrained — verifier has to handle this check
+/// @dev output is constrained via GreaterThan component📝 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.
| /// @dev output is not constrained — verifier has to handle this check | |
| /// @dev output is constrained via GreaterThan component |
🤖 Prompt for AI Agents
In circuits/circuits/utils/selfrica/date/dateIsLess.circom around line 14, the
current comment "output is not constrained — verifier has to handle this check"
is misleading because the output is constrained by the GreaterThan component at
line 64; update the comment to accurately state that the output is constrained
(mentioning the GreaterThan check at line 64) and remove the claim that the
verifier must handle it, so the documentation reflects the actual constraint
enforcement.
| template DateIsLessFullYear() { | ||
| signal input day_1; | ||
| signal input day_2; | ||
|
|
||
| signal input month_1; | ||
| signal input month_2; | ||
|
|
||
| signal input year_1; | ||
| signal input year_2; | ||
|
|
||
| signal output out; |
There was a problem hiding this comment.
Add input validation for date components.
The template accepts year/month/day inputs without validating they represent valid dates. Invalid values (e.g., month=13, day=32) will produce incorrect comparison results.
Based on learnings: Ensure the circuit performs day-level comparison with correct date arithmetic.
Consider adding range constraints:
// Add after signal declarations
component month_1_valid = LessThan(8);
month_1_valid.in[0] <== month_1;
month_1_valid.in[1] <== 13;
month_1_valid.out === 1;
component day_1_valid = LessThan(8);
day_1_valid.in[0] <== day_1;
day_1_valid.in[1] <== 32;
day_1_valid.out === 1;
// Repeat for month_2 and day_2| for (var i=0; i<8; i++) { | ||
| birthdateNum[i] <== birthDateASCII[i] - ASCII_rotation; | ||
| } | ||
|
|
||
| signal TEN <== 10; | ||
| signal CENTURY <== 100; | ||
| signal MILLENIA <== 1000; | ||
|
|
||
| signal currDateMillenia <== currDate[0] * MILLENIA; | ||
| signal currDateCentury <== currDate[1] * CENTURY; | ||
| signal currDateDecade <== currDate[2] * TEN; | ||
| signal currDateYear <== currDateMillenia + currDateCentury + currDateDecade + currDate[3]; | ||
|
|
||
| signal birthDateMillenia <== birthdateNum[0] * MILLENIA; | ||
| signal birthDateCentury <== birthdateNum[1] * CENTURY; | ||
| signal birthDateDecade <== birthdateNum[2] * TEN; | ||
| signal birthDateYear <== birthDateMillenia + birthDateCentury + birthDateDecade + birthdateNum[3]; | ||
|
|
There was a problem hiding this comment.
Clamp birthDateASCII digits to ASCII range
We never constrain birthDateASCII[i] to '0'..'9'. A prover can drop non-digit field elements here, shape birthdateNum however they want, and bypass the age gate while still satisfying downstream constraints. Please mirror the guard you already apply to majorityASCII so the birth date derivation can’t be spoofed.
@@
- for (var i=0; i<8; i++) {
+ component birth_ascii_lo[8];
+ component birth_ascii_hi[8];
+ signal birth_ascii_guard[8];
+ for (var i=0; i<8; i++) {
+ birth_ascii_lo[i] = LessThan(8);
+ birth_ascii_hi[i] = LessThan(8);
+ birth_ascii_lo[i].in[0] <== 47;
+ birth_ascii_lo[i].in[1] <== birthDateASCII[i];
+ birth_ascii_hi[i].in[0] <== birthDateASCII[i];
+ birth_ascii_hi[i].in[1] <== 58;
birthdateNum[i] <== birthDateASCII[i] - ASCII_rotation;
+ birth_ascii_guard[i] <== birth_ascii_lo[i].out * birth_ascii_hi[i].out;
}
+ for (var i=1; i<8; i++) {
+ birth_ascii_guard[i] <== birth_ascii_guard[i-1] * birth_ascii_guard[i];
+ }
+ birth_ascii_guard[7] === 1;📝 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.
| for (var i=0; i<8; i++) { | |
| birthdateNum[i] <== birthDateASCII[i] - ASCII_rotation; | |
| } | |
| signal TEN <== 10; | |
| signal CENTURY <== 100; | |
| signal MILLENIA <== 1000; | |
| signal currDateMillenia <== currDate[0] * MILLENIA; | |
| signal currDateCentury <== currDate[1] * CENTURY; | |
| signal currDateDecade <== currDate[2] * TEN; | |
| signal currDateYear <== currDateMillenia + currDateCentury + currDateDecade + currDate[3]; | |
| signal birthDateMillenia <== birthdateNum[0] * MILLENIA; | |
| signal birthDateCentury <== birthdateNum[1] * CENTURY; | |
| signal birthDateDecade <== birthdateNum[2] * TEN; | |
| signal birthDateYear <== birthDateMillenia + birthDateCentury + birthDateDecade + birthdateNum[3]; | |
| // --- enforce ASCII '0'..'9' on each birthDateASCII digit --- | |
| component birth_ascii_lo[8]; | |
| component birth_ascii_hi[8]; | |
| signal birth_ascii_guard[8]; | |
| for (var i = 0; i < 8; i++) { | |
| birth_ascii_lo[i] = LessThan(8); | |
| birth_ascii_hi[i] = LessThan(8); | |
| // birthDateASCII[i] > 47 <==> ascii >= '0' | |
| birth_ascii_lo[i].in[0] <== 47; | |
| birth_ascii_lo[i].in[1] <== birthDateASCII[i]; | |
| // birthDateASCII[i] < 58 <==> ascii <= '9' | |
| birth_ascii_hi[i].in[0] <== birthDateASCII[i]; | |
| birth_ascii_hi[i].in[1] <== 58; | |
| // derive numeric digit | |
| birthdateNum[i] <== birthDateASCII[i] - ASCII_rotation; | |
| // both guards must pass | |
| birth_ascii_guard[i] <== birth_ascii_lo[i].out * birth_ascii_hi[i].out; | |
| } | |
| // accumulate all guards and require them all to be true | |
| for (var i = 1; i < 8; i++) { | |
| birth_ascii_guard[i] <== birth_ascii_guard[i - 1] * birth_ascii_guard[i]; | |
| } | |
| birth_ascii_guard[7] === 1; | |
| // --- existing date‐to‐number logic remains unchanged --- | |
| signal TEN <== 10; | |
| signal CENTURY <== 100; | |
| signal MILLENIA <== 1000; | |
| signal currDateMillenia <== currDate[0] * MILLENIA; | |
| signal currDateCentury <== currDate[1] * CENTURY; | |
| signal currDateDecade <== currDate[2] * TEN; | |
| signal currDateYear <== currDateMillenia + currDateCentury + currDateDecade + currDate[3]; | |
| signal birthDateMillenia <== birthdateNum[0] * MILLENIA; | |
| signal birthDateCentury <== birthdateNum[1] * CENTURY; | |
| signal birthDateDecade <== birthdateNum[2] * TEN; | |
| signal birthDateYear <== birthDateMillenia + birthDateCentury + birthDateDecade + birthdateNum[3]; |
🤖 Prompt for AI Agents
In circuits/circuits/utils/selfrica/date/isOlderThan.circom around lines 23 to
40, the birthDateASCII digits are never constrained to the ASCII '0'..'9' range
so a prover can supply non-digit values; mirror the same guard used for
majorityASCII by adding constraints for each birthDateASCII[i] (i=0..7) that
enforce they are within the ASCII digit range (>= '0' and <= '9') using the same
comparator/boolean logic or range-check component already used for
majorityASCII, so birthdateNum derivation cannot be spoofed.
| signal ASCII_rotation <== 48; | ||
|
|
||
| for (var i = 0; i < 8; i++) { | ||
| validity_date_num[i] <== validity_date_ascii[i] - ASCII_rotation; | ||
| } |
There was a problem hiding this comment.
Add validation for ASCII digit inputs.
The template assumes validity_date_ascii contains valid ASCII digits (48-57) but doesn't validate this. Invalid inputs could cause incorrect date calculations or constraint failures.
Add range constraints to ensure inputs are valid ASCII digits:
// Add after line 18
for (var i = 0; i < 8; i++) {
component ascii_valid = LessThan(8);
ascii_valid.in[0] <== validity_date_ascii[i];
ascii_valid.in[1] <== 58; // ASCII '9' + 1
ascii_valid.out === 1;
component ascii_min = GreaterThan(8);
ascii_min.in[0] <== validity_date_ascii[i];
ascii_min.in[1] <== 47; // ASCII '0' - 1
ascii_min.out === 1;
}🤖 Prompt for AI Agents
In circuits/circuits/utils/selfrica/date/isValid.circom around lines 19 to 23,
the code subtracts ASCII_rotation from validity_date_ascii but doesn't validate
that each input is an ASCII digit (48-57); add per-byte range checks before the
subtraction by instantiating two comparator components for each i: a LessThan
comparing validity_date_ascii[i] < 58 with out===1 and a GreaterThan comparing
validity_date_ascii[i] > 47 with out===1, so together they enforce 48 <=
validity_date_ascii[i] <= 57 for i in 0..7, then proceed with the existing
ASCII_rotation subtraction.
| signal input selector_smile_data[selfrica_max_length]; | ||
|
|
||
| signal input forbidden_countries_list[MAX_FORBIDDEN_COUNTRIES_LIST_LENGTH * country_length]; | ||
|
|
||
| signal input ofac_name_dob_smt_leaf_key; | ||
| signal input ofac_name_dob_smt_root; | ||
| signal input ofac_name_dob_smt_siblings[name_dob_tree_levels]; | ||
|
|
||
| signal input ofac_name_yob_smt_leaf_key; | ||
| signal input ofac_name_yob_smt_root; | ||
| signal input ofac_name_yob_smt_siblings[name_yob_tree_levels]; | ||
|
|
||
| signal input selector_ofac; | ||
| signal input current_date[8]; | ||
| signal input majority_age_ASCII[3]; | ||
| signal input selector_older_than; | ||
|
|
||
| selector_ofac * (selector_ofac - 1) === 0; | ||
| selector_older_than * (selector_older_than - 1) === 0; | ||
|
|
||
| signal validity_ASCII[8]; | ||
| for (var i = 0; i < EXPIRATION_DATE_LENGTH(); i++) { | ||
| validity_ASCII[i] <== smile_data[EXPIRATION_DATE_INDEX() + i]; | ||
| } | ||
| IsValidFullYear()(current_date, validity_ASCII); | ||
|
|
||
| signal birth_date_ASCII[8]; | ||
| for (var i = 0; i < DOB_LENGTH(); i++) { | ||
| birth_date_ASCII[i] <== smile_data[DOB_INDEX() + i]; | ||
| } | ||
|
|
||
| component is_older_than = IsOlderThan(); | ||
| is_older_than.majorityASCII <== majority_age_ASCII; | ||
| is_older_than.currDate <== current_date; | ||
| is_older_than.birthDateASCII <== birth_date_ASCII; | ||
| signal is_older_than_result <== is_older_than.out; | ||
|
|
||
| component ofac_name_dob_circuit = OFAC_NAME_DOB_SELFRICA(name_dob_tree_levels); | ||
| ofac_name_dob_circuit.smile_data <== smile_data; | ||
| ofac_name_dob_circuit.smt_leaf_key <== ofac_name_dob_smt_leaf_key; | ||
| ofac_name_dob_circuit.smt_root <== ofac_name_dob_smt_root; | ||
| ofac_name_dob_circuit.smt_siblings <== ofac_name_dob_smt_siblings; | ||
|
|
||
| component ofac_name_yob_circuit = OFAC_NAME_YOB_SELFRICA(name_yob_tree_levels); | ||
| ofac_name_yob_circuit.smile_data <== smile_data; | ||
| ofac_name_yob_circuit.smt_leaf_key <== ofac_name_yob_smt_leaf_key; | ||
| ofac_name_yob_circuit.smt_root <== ofac_name_yob_smt_root; | ||
| ofac_name_yob_circuit.smt_siblings <== ofac_name_yob_smt_siblings; | ||
|
|
||
| signal older_than_verified[3]; | ||
| older_than_verified[0] <== is_older_than_result * majority_age_ASCII[0]; | ||
| older_than_verified[1] <== is_older_than_result * majority_age_ASCII[1]; | ||
| older_than_verified[2] <== is_older_than_result * majority_age_ASCII[2]; | ||
|
|
||
| signal revealed_data[selfrica_max_length + 2 + 3]; | ||
| for (var i = 0; i < selfrica_max_length; i++) { | ||
| revealed_data[i] <== smile_data[i] * selector_smile_data[i]; | ||
| } | ||
|
|
||
| revealed_data[selfrica_max_length] <== ofac_name_dob_circuit.ofacCheckResult * selector_ofac; | ||
| revealed_data[selfrica_max_length + 1] <== ofac_name_yob_circuit.ofacCheckResult * selector_ofac; | ||
| revealed_data[selfrica_max_length + 2] <== older_than_verified[0] * selector_older_than; | ||
| revealed_data[selfrica_max_length + 3] <== older_than_verified[1] * selector_older_than; | ||
| revealed_data[selfrica_max_length + 4] <== older_than_verified[2] * selector_older_than; | ||
|
|
There was a problem hiding this comment.
Constrain selector_smile_data to boolean values
selector_smile_data drives which bytes are revealed, but nothing here forces each selector entry to be 0/1. A prover can pick arbitrary field elements (e.g., 2, p−1) and still satisfy the circuit while disclosing tampered values, breaking relying-party expectations. Please add a booleanity constraint per selector entry before using it as a mask.
signal revealed_data[selfrica_max_length + 2 + 3];
- for (var i = 0; i < selfrica_max_length; i++) {
+ for (var i = 0; i < selfrica_max_length; i++) {
+ selector_smile_data[i] * (selector_smile_data[i] - 1) === 0;
revealed_data[i] <== smile_data[i] * selector_smile_data[i];
}📝 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.
| signal input selector_smile_data[selfrica_max_length]; | |
| signal input forbidden_countries_list[MAX_FORBIDDEN_COUNTRIES_LIST_LENGTH * country_length]; | |
| signal input ofac_name_dob_smt_leaf_key; | |
| signal input ofac_name_dob_smt_root; | |
| signal input ofac_name_dob_smt_siblings[name_dob_tree_levels]; | |
| signal input ofac_name_yob_smt_leaf_key; | |
| signal input ofac_name_yob_smt_root; | |
| signal input ofac_name_yob_smt_siblings[name_yob_tree_levels]; | |
| signal input selector_ofac; | |
| signal input current_date[8]; | |
| signal input majority_age_ASCII[3]; | |
| signal input selector_older_than; | |
| selector_ofac * (selector_ofac - 1) === 0; | |
| selector_older_than * (selector_older_than - 1) === 0; | |
| signal validity_ASCII[8]; | |
| for (var i = 0; i < EXPIRATION_DATE_LENGTH(); i++) { | |
| validity_ASCII[i] <== smile_data[EXPIRATION_DATE_INDEX() + i]; | |
| } | |
| IsValidFullYear()(current_date, validity_ASCII); | |
| signal birth_date_ASCII[8]; | |
| for (var i = 0; i < DOB_LENGTH(); i++) { | |
| birth_date_ASCII[i] <== smile_data[DOB_INDEX() + i]; | |
| } | |
| component is_older_than = IsOlderThan(); | |
| is_older_than.majorityASCII <== majority_age_ASCII; | |
| is_older_than.currDate <== current_date; | |
| is_older_than.birthDateASCII <== birth_date_ASCII; | |
| signal is_older_than_result <== is_older_than.out; | |
| component ofac_name_dob_circuit = OFAC_NAME_DOB_SELFRICA(name_dob_tree_levels); | |
| ofac_name_dob_circuit.smile_data <== smile_data; | |
| ofac_name_dob_circuit.smt_leaf_key <== ofac_name_dob_smt_leaf_key; | |
| ofac_name_dob_circuit.smt_root <== ofac_name_dob_smt_root; | |
| ofac_name_dob_circuit.smt_siblings <== ofac_name_dob_smt_siblings; | |
| component ofac_name_yob_circuit = OFAC_NAME_YOB_SELFRICA(name_yob_tree_levels); | |
| ofac_name_yob_circuit.smile_data <== smile_data; | |
| ofac_name_yob_circuit.smt_leaf_key <== ofac_name_yob_smt_leaf_key; | |
| ofac_name_yob_circuit.smt_root <== ofac_name_yob_smt_root; | |
| ofac_name_yob_circuit.smt_siblings <== ofac_name_yob_smt_siblings; | |
| signal older_than_verified[3]; | |
| older_than_verified[0] <== is_older_than_result * majority_age_ASCII[0]; | |
| older_than_verified[1] <== is_older_than_result * majority_age_ASCII[1]; | |
| older_than_verified[2] <== is_older_than_result * majority_age_ASCII[2]; | |
| signal revealed_data[selfrica_max_length + 2 + 3]; | |
| for (var i = 0; i < selfrica_max_length; i++) { | |
| revealed_data[i] <== smile_data[i] * selector_smile_data[i]; | |
| } | |
| revealed_data[selfrica_max_length] <== ofac_name_dob_circuit.ofacCheckResult * selector_ofac; | |
| revealed_data[selfrica_max_length + 1] <== ofac_name_yob_circuit.ofacCheckResult * selector_ofac; | |
| revealed_data[selfrica_max_length + 2] <== older_than_verified[0] * selector_older_than; | |
| revealed_data[selfrica_max_length + 3] <== older_than_verified[1] * selector_older_than; | |
| revealed_data[selfrica_max_length + 4] <== older_than_verified[2] * selector_older_than; | |
| signal revealed_data[selfrica_max_length + 2 + 3]; | |
| for (var i = 0; i < selfrica_max_length; i++) { | |
| // Ensure each selector is boolean (0 or 1) | |
| selector_smile_data[i] * (selector_smile_data[i] - 1) === 0; | |
| revealed_data[i] <== smile_data[i] * selector_smile_data[i]; | |
| } |
🤖 Prompt for AI Agents
In circuits/circuits/utils/selfrica/disclose/disclose.circom around lines 19 to
83, each element of selector_smile_data is used as a mask but is not constrained
to be boolean; add a per-entry booleanity constraint to prevent non-{0,1} values
by asserting selector_smile_data[i] * (selector_smile_data[i] - 1) === 0 for
every i before the revealed_data loop (i.e., add a for loop over i from 0 to
selfrica_max_length applying that constraint).
| var selfrica_max_length = SELFRICA_MAX_LENGTH(); | ||
| signal input smile_data[selfrica_max_length]; | ||
|
|
||
| signal input smt_leaf_key; | ||
| signal input smt_root; | ||
| signal input smt_siblings[n_levels]; | ||
|
|
||
| var name_length = FULL_NAME_LENGTH(); | ||
| var name_index = FULL_NAME_INDEX(); | ||
| //name hash | ||
| component name_hash = PackBytesAndPoseidon(name_length); | ||
| for (var i = name_index; i < name_index + name_length; i++) { | ||
| name_hash.in[i - name_index] <== smile_data[i]; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add bounds validation for name extraction.
The template extracts name data from smile_data using constants but doesn't validate that the array access is within bounds. If constants are misconfigured, this could cause undefined behavior.
Add an assertion or constraint to validate array bounds:
// Add after line 17
signal name_end <== name_index + name_length;
// Ensure name_end <= selfrica_max_length
component bounds_check = LessThan(16);
bounds_check.in[0] <== name_end;
bounds_check.in[1] <== selfrica_max_length + 1;
bounds_check.out === 1;🤖 Prompt for AI Agents
In circuits/circuits/utils/selfrica/disclose/ofac/ofac_name_dob_selfrica.circom
around lines 9 to 22, the code slices smile_data using name_index and
name_length without validating bounds; add a bounds check immediately after the
name_end calculation (name_end = name_index + name_length) and enforce name_end
<= selfrica_max_length by instantiating a LessThan comparator (compare name_end
and selfrica_max_length + 1) and asserting its output equals 1 so the circuit
fails when the extracted range would exceed smile_data.
| const processNameSelfrica = (firstName: string, lastName: string, i: number): bigint => { | ||
| firstName = firstName.replace(/'/g, ''); | ||
| firstName = firstName.replace(/\./g, ''); | ||
| firstName = firstName.replace(/[- ]/g, '<'); | ||
| lastName = lastName.replace(/'/g, ''); | ||
| lastName = lastName.replace(/[- ]/g, '<'); | ||
| lastName = lastName.replace(/\./g, ''); | ||
|
|
||
| //TODO: check if smile id does first name and last name || last name and first name | ||
| const nameArr = (lastName + ' ' + firstName).padEnd(40, '\0').split('').map(char => char.charCodeAt(0)); | ||
| return BigInt(packBytesAndPoseidon(nameArr)); | ||
| } |
There was a problem hiding this comment.
Guard Selfrica name inputs before string ops
processNameSelfrica calls .replace on firstName/lastName without verifying they’re non-empty strings. When the dataset has missing name fields (common in sanction feeds), this will throw and abort tree construction, blocking circuit input generation. Please short-circuit to BigInt(0) (or otherwise skip the record) when either name is absent.
-const processNameSelfrica = (firstName: string, lastName: string, i: number): bigint => {
+const processNameSelfrica = (firstName: string, lastName: string, i: number): bigint => {
+ if (typeof firstName !== 'string' || typeof lastName !== 'string') {
+ console.log('Name data missing for Selfrica entry', i, { firstName, lastName });
+ return BigInt(0);
+ }
firstName = firstName.replace(/'/g, '');Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In common/src/utils/trees.ts around lines 774 to 785, processNameSelfrica
currently calls string methods on firstName/lastName without validation which
will throw if names are null/undefined; update the function to first guard that
both firstName and lastName are non-empty strings (e.g. typeof === 'string' and
trimmed length > 0) and immediately return BigInt(0) when either is
missing/invalid; otherwise proceed with the existing replace/pad/split/pack
logic. Ensure the early-return uses BigInt(0) to keep downstream tree
construction safe.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
circuits/tests/utils/selfrica/date/is_valid.test.circom (1)
1-4: Add pragma directive for Circom version consistency.The file is missing a
pragma circomversion directive at the top. The sibling test fileis_older_than.test.circomdeclarespragma circom 2.1.9;. Adding the pragma ensures consistent compiler behavior across test files.Apply this diff:
+pragma circom 2.1.9; + include "../../../../circuits/utils/selfrica/date/isValid.circom";circuits/tests/utils/selfrica/date/isOlderThan.test.ts (1)
1-60: Fix Prettier formatting issues.CI pipeline detected code style issues. Run
prettier --write circuits/tests/utils/selfrica/date/isOlderThan.test.tsto fix formatting.circuits/tests/utils/selfrica/date/date.test.ts (1)
1-119: Fix Prettier formatting issues.CI pipeline detected code style issues. Run
prettier --write circuits/tests/utils/selfrica/date/date.test.tsto fix formatting.
🧹 Nitpick comments (3)
circuits/tests/utils/selfrica/date/isOlderThan.test.ts (3)
32-44: Clarify test description (double negative).The test description "should not return false if the user is younger than the majority" uses a double negative and is confusing. It should be "should return false if the user is younger than the majority" for clarity.
Apply this diff:
- it("should not return false if the user is younger than the majority", async () => { + it("should return false if the user is younger than the majority", async () => {
46-58: Clarify test description (double negative).The test description uses a double negative and is confusing. Simplify to "should return false if the user birthdate is in the majority year but the current date is not" for clarity.
Apply this diff:
- it('should not return true if the user birthdate is in the majority year but the current date is not', async () => { + it('should return false if the user birthdate is in the majority year but the current date is not', async () => {
5-59: Expand test coverage for edge cases.The test suite covers basic scenarios but misses several critical edge cases for date validation:
- Boundary condition (user exactly at majority age on the same day)
- Invalid date inputs (month > 12, day > 31)
- Leap year handling
- ASCII input validation (non-digit characters should fail gracefully)
Consider adding tests for these scenarios to ensure circuit robustness.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
circuits/tests/utils/selfrica/date/date.test.ts(1 hunks)circuits/tests/utils/selfrica/date/isOlderThan.test.ts(1 hunks)circuits/tests/utils/selfrica/date/is_older_than.test.circom(1 hunks)circuits/tests/utils/selfrica/date/is_valid.test.circom(1 hunks)circuits/tests/utils/selfrica/ofac/ofac.test.ts(1 hunks)circuits/tests/utils/selfrica/ofac/ofac_name_dob_selfrica.test.circom(1 hunks)circuits/tests/utils/selfrica/ofac/ofac_name_yob_selfrica.test.circom(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- circuits/tests/utils/selfrica/ofac/ofac_name_dob_selfrica.test.circom
🧰 Additional context used
📓 Path-based instructions (3)
circuits/**/*.circom
⚙️ CodeRabbit configuration file
circuits/**/*.circom: Review ZK circuit code for:
- Circuit correctness and completeness
- Constraint efficiency
- Input validation
- Security considerations for zero-knowledge proofs
Files:
circuits/tests/utils/selfrica/date/is_valid.test.circomcircuits/tests/utils/selfrica/ofac/ofac_name_yob_selfrica.test.circomcircuits/tests/utils/selfrica/date/is_older_than.test.circom
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: 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:
circuits/tests/utils/selfrica/ofac/ofac.test.tscircuits/tests/utils/selfrica/date/isOlderThan.test.tscircuits/tests/utils/selfrica/date/date.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:
circuits/tests/utils/selfrica/ofac/ofac.test.tscircuits/tests/utils/selfrica/date/isOlderThan.test.tscircuits/tests/utils/selfrica/date/date.test.ts
🧠 Learnings (2)
📚 Learning: 2025-08-24T18:53:11.221Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Ensure the circuit performs day-level comparison with correct date arithmetic
Applied to files:
circuits/tests/utils/selfrica/date/is_older_than.test.circomcircuits/tests/utils/selfrica/date/date.test.ts
📚 Learning: 2025-08-24T18:53:11.221Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Implement the age verification circuit at circuits/circuits/disclose/disclose.circom with private DOB input, public minimum-age input, and boolean output
Applied to files:
circuits/tests/utils/selfrica/date/is_older_than.test.circom
🧬 Code graph analysis (1)
circuits/tests/utils/selfrica/ofac/ofac.test.ts (2)
common/src/utils/selfrica/types.ts (1)
serializeSmileData(22-99)common/src/utils/selfrica/generateInputs.ts (2)
OFAC_DUMMY_INPUT(36-53)NON_OFAC_DUMMY_INPUT(55-72)
🪛 GitHub Actions: Circuits CI
circuits/tests/utils/selfrica/ofac/ofac.test.ts
[warning] 1-1: Code style issues found. Run 'Prettier --write' to fix.
circuits/tests/utils/selfrica/date/isOlderThan.test.ts
[warning] 1-1: Code style issues found. Run 'Prettier --write' to fix.
circuits/tests/utils/selfrica/date/date.test.ts
[warning] 1-1: Code style issues found. Run 'Prettier --write' to fix.
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: build-deps
🔇 Additional comments (2)
circuits/tests/utils/selfrica/date/is_older_than.test.circom (1)
1-6: LGTM!Standard Circom test wrapper structure. The file correctly includes the IsOlderThan circuit and declares it as the main component for testing.
circuits/tests/utils/selfrica/date/isOlderThan.test.ts (1)
8-15: LGTM!Test setup correctly configures the WASM tester with appropriate include paths for circuit dependencies.
| before(async () => { | ||
| circuit = await wasmTester( | ||
| path.join(__dirname, "is_valid.test.circom"), | ||
| { | ||
| include: ['node_modules', "node_modules/@zk-kit/binary-merkle-root.circom/src"], | ||
| } | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify include paths are complete.
The isOlderThan.test.ts file includes circomlib/circuits in its include paths, but this test file does not. Verify whether the IsValidFullYear circuit requires circomlib dependencies. If it uses circomlib comparators or utilities, add the missing include path.
Run the following script to check if the circuit depends on circomlib:
🏁 Script executed:
#!/bin/bash
# Check if IsValidFullYear circuit includes any circomlib dependencies
rg -nP "include.*circomlib" circuits/circuits/utils/selfrica/date/isValid.circomLength of output: 96
| validity_date_ascii: "19950412".split("").map((x) => x.charCodeAt(0)), | ||
| } | ||
|
|
||
| console.log(inputs); |
There was a problem hiding this comment.
Remove PII logging.
Console.log on line 23 logs date inputs. Per coding guidelines, NEVER log PII including DOB or dates that could be associated with individuals.
Apply this diff:
- console.log(inputs);📝 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.
| console.log(inputs); |
🤖 Prompt for AI Agents
In circuits/tests/utils/selfrica/date/date.test.ts around line 23, a
console.log(inputs) call is printing date inputs (PII such as DOB); remove this
log or replace it with a non-PII-safe assertion or masked debug output.
Specifically, delete the console.log(inputs) line from the test (or if you need
audit info, log only a sanitized summary like input length or a hash) so no raw
date/PII is written to logs.
| it('should return false if the year is equal and month is equal and day is greater', async () => { | ||
| const inputs = { | ||
| current_date: [1, 9, 9, 6, 0, 4, 0, 3], | ||
| validity_date_ascii: "19960412".split("").map((x) => x.charCodeAt(0)), | ||
| } | ||
|
|
||
| try { | ||
| const witness = await circuit.calculateWitness(inputs); | ||
| await circuit.checkConstraints(witness); | ||
|
|
||
| throw new Error("should return false if the year is equal and month is equal and day is greater: FAILED"); | ||
| } catch (error) { | ||
| expect(error).to.exist; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Fix incorrect test input.
The test description states "day is greater" but the input shows current_date: [1, 9, 9, 6, 0, 4, 0, 3] (day 03) compared to validity_date_ascii: "19960412" (day 12). Day 03 is less than day 12, not greater. This test does not validate the scenario it claims to test.
Apply this diff to correct the input:
const inputs = {
- current_date: [1, 9, 9, 6, 0, 4, 0, 3],
+ current_date: [1, 9, 9, 6, 0, 4, 1, 3],
validity_date_ascii: "19960412".split("").map((x) => x.charCodeAt(0)),
}📝 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.
| it('should return false if the year is equal and month is equal and day is greater', async () => { | |
| const inputs = { | |
| current_date: [1, 9, 9, 6, 0, 4, 0, 3], | |
| validity_date_ascii: "19960412".split("").map((x) => x.charCodeAt(0)), | |
| } | |
| try { | |
| const witness = await circuit.calculateWitness(inputs); | |
| await circuit.checkConstraints(witness); | |
| throw new Error("should return false if the year is equal and month is equal and day is greater: FAILED"); | |
| } catch (error) { | |
| expect(error).to.exist; | |
| } | |
| }); | |
| it('should return false if the year is equal and month is equal and day is greater', async () => { | |
| const inputs = { | |
| current_date: [1, 9, 9, 6, 0, 4, 1, 3], | |
| validity_date_ascii: "19960412".split("").map((x) => x.charCodeAt(0)), | |
| } | |
| try { | |
| const witness = await circuit.calculateWitness(inputs); | |
| await circuit.checkConstraints(witness); | |
| throw new Error("should return false if the year is equal and month is equal and day is greater: FAILED"); | |
| } catch (error) { | |
| expect(error).to.exist; | |
| } | |
| }); |
🤖 Prompt for AI Agents
In circuits/tests/utils/selfrica/date/date.test.ts around lines 87 to 101, the
test claims "day is greater" but current_date is "19960403" while
validity_date_ascii is "19960412"; update the current_date input so its day is
greater than 12 (for example change the array from [1,9,9,6,0,4,0,3] to
[1,9,9,6,0,4,1,3] representing "19960413") so the test actually exercises the
described condition.
| it ('should return false if the year is equal and month is equal and day is equal', async () => { | ||
| const inputs = { | ||
| current_date: [1, 9, 9, 6, 0, 4, 0, 2], | ||
| validity_date_ascii: "19960412".split("").map((x) => x.charCodeAt(0)), | ||
| } | ||
|
|
||
| try { | ||
| const witness = await circuit.calculateWitness(inputs); | ||
| await circuit.checkConstraints(witness); | ||
|
|
||
| throw new Error("should return false if the year is equal and month is equal and day is equal: FAILED"); | ||
| } catch (error) { | ||
| expect(error).to.exist; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Fix incorrect test input for equal dates.
The test description states "year is equal and month is equal and day is equal" but the inputs show current_date: [1, 9, 9, 6, 0, 4, 0, 2] (day 02) compared to validity_date_ascii: "19960412" (day 12). The dates are not equal. This test does not validate the scenario it claims to test.
Apply this diff to correct the input:
const inputs = {
- current_date: [1, 9, 9, 6, 0, 4, 0, 2],
+ current_date: [1, 9, 9, 6, 0, 4, 1, 2],
validity_date_ascii: "19960412".split("").map((x) => x.charCodeAt(0)),
}📝 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.
| it ('should return false if the year is equal and month is equal and day is equal', async () => { | |
| const inputs = { | |
| current_date: [1, 9, 9, 6, 0, 4, 0, 2], | |
| validity_date_ascii: "19960412".split("").map((x) => x.charCodeAt(0)), | |
| } | |
| try { | |
| const witness = await circuit.calculateWitness(inputs); | |
| await circuit.checkConstraints(witness); | |
| throw new Error("should return false if the year is equal and month is equal and day is equal: FAILED"); | |
| } catch (error) { | |
| expect(error).to.exist; | |
| } | |
| }); | |
| it ('should return false if the year is equal and month is equal and day is equal', async () => { | |
| const inputs = { | |
| current_date: [1, 9, 9, 6, 0, 4, 1, 2], | |
| validity_date_ascii: "19960412".split("").map((x) => x.charCodeAt(0)), | |
| } | |
| try { | |
| const witness = await circuit.calculateWitness(inputs); | |
| await circuit.checkConstraints(witness); | |
| throw new Error("should return false if the year is equal and month is equal and day is equal: FAILED"); | |
| } catch (error) { | |
| expect(error).to.exist; | |
| } | |
| }); |
🤖 Prompt for AI Agents
In circuits/tests/utils/selfrica/date/date.test.ts around lines 103 to 117, the
test claims dates are equal but current_date is set to 19960402
([1,9,9,6,0,4,0,2]) while validity_date_ascii is "19960412"; update current_date
to match 19960412 by changing the array to [1,9,9,6,0,4,1,2] so the day bytes
equal 12 and the test validates the intended equal-date scenario.
| }); | ||
|
|
||
| it('should return true if the user is older than the majority', async () => { | ||
| console.log(['1', '9', '9', '4', '0', '4', '0', '2'].map((x) => x.charCodeAt(0))); |
There was a problem hiding this comment.
Remove PII logging.
Console.log on line 18 logs birth date data. Per coding guidelines, NEVER log PII including DOB.
Apply this diff:
- console.log(['1', '9', '9', '4', '0', '4', '0', '2'].map((x) => x.charCodeAt(0)));📝 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.
| console.log(['1', '9', '9', '4', '0', '4', '0', '2'].map((x) => x.charCodeAt(0))); |
🤖 Prompt for AI Agents
In circuits/tests/utils/selfrica/date/isOlderThan.test.ts around line 18, the
console.log call prints birth date data (PII) and must be removed; delete the
console.log invocation that maps and logs the date digits, and if any runtime
insight is required for debugging replace it with non-PII-safe assertions or a
mocked, non-sensitive value (do not log actual DOB or derived characters), then
run tests to ensure no other references depend on that log.
| const dummy_smile_input = serializeSmileData(OFAC_DUMMY_INPUT); | ||
| const ofacInputs = generateCircuitInputsOfac(OFAC_DUMMY_INPUT, namedob_smt, proofLevel); | ||
| const inputs = { | ||
| smile_data: dummy_smile_input.split('').map((x) => x.charCodeAt(0)), | ||
| ...ofacInputs, | ||
| }; | ||
|
|
||
| const witness = await circuit.calculateWitness(inputs); | ||
| const ofacCheckResult = (await circuit.getOutput(witness, ['ofacCheckResult'])).ofacCheckResult; | ||
| expect(ofacCheckResult).to.equal('0'); | ||
| }); | ||
|
|
||
| it('should return 1 if the person is not in the ofac list', async () => { | ||
| const dummy_smile_input = serializeSmileData(NON_OFAC_DUMMY_INPUT); | ||
| const ofacInputs = generateCircuitInputsOfac(NON_OFAC_DUMMY_INPUT, namedob_smt, proofLevel); | ||
| const inputs = { | ||
| smile_data: dummy_smile_input.split('').map((x) => x.charCodeAt(0)), | ||
| ...ofacInputs, | ||
| }; |
There was a problem hiding this comment.
Avoid mutating shared OFAC fixtures.
serializeSmileData uppercases/pads fields in place. Calling it with the exported OFAC_DUMMY_INPUT / NON_OFAC_DUMMY_INPUT objects mutates the shared fixtures, so any other test importing them will now observe NUL‑padded values and derive different hashes/SMT leaves. This cross-test data contamination is already biting reliability. Please clone the fixture (e.g., { ...OFAC_DUMMY_INPUT }) before passing it into serializeSmileData or adjust serializeSmileData to work on a copy.
🤖 Prompt for AI Agents
In circuits/tests/utils/selfrica/ofac/ofac.test.ts around lines 37 to 55, the
tests call serializeSmileData with the shared OFAC_DUMMY_INPUT /
NON_OFAC_DUMMY_INPUT fixtures which serializeSmileData mutates in place, causing
cross-test contamination; fix by passing a shallow copy of the fixture into
serializeSmileData (e.g., serializeSmileData({ ...OFAC_DUMMY_INPUT })) wherever
used in these tests, or alter serializeSmileData to operate on and return a copy
instead of mutating its input so the exported fixtures remain unchanged.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
common/src/utils/selfrica/constants.ts (2)
64-64: Fix formatting issue: missing newline at EOF.Pipeline failure indicates a Prettier formatting issue, likely the missing newline at the end of the file.
Run these commands to fix:
#!/bin/bash # Fix formatting yarn prettier --write common/src/utils/selfrica/constants.ts # Verify linting passes yarn lintBased on learnings: Run linting/formatting (yarn nice, yarn lint) before PRs.
39-63: Document the public signals interface contract.As noted in the previous review, these constants define the circuit's output interface but lack explanatory comments. This remains critical for circuit developers, frontend integrators, and auditors to understand the proof structure.
Add documentation covering:
- What each signal represents and its purpose
- Data type/format (field element, packed bytes, etc.)
- Packing layout for multi-element signals
- Whether CURRENT_DATE is 8 separate components or a single packed value
Based on learnings: Run linting/formatting (yarn nice, yarn lint) before PRs.
🧹 Nitpick comments (1)
common/src/utils/selfrica/constants.ts (1)
1-37: Consider adding brief inline comments for maintainability.The sequential field layout is correct, but the constants lack context on what each field represents and why specific lengths were chosen. Adding concise comments would help future maintainers understand the SELFRICA data schema without referencing external documentation.
Example pattern:
+// SELFRICA data layout: sequential byte-packed fields +// Country code (ISO 3166-1 alpha-3, 3 chars) export const SELFRICA_COUNTRY_INDEX = 0; export const SELFRICA_COUNTRY_LENGTH = 3; +// ID type (e.g., "PASSPORT", up to 27 chars) export const SELFRICA_ID_TYPE_INDEX = SELFRICA_COUNTRY_INDEX + SELFRICA_COUNTRY_LENGTH; export const SELFRICA_ID_TYPE_LENGTH = 27;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
common/src/utils/selfrica/constants.ts(1 hunks)common/src/utils/selfrica/decodePublicSignals.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- common/src/utils/selfrica/decodePublicSignals.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: 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:
common/src/utils/selfrica/constants.ts
common/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
common/src/**/*.{ts,tsx,js,jsx}: Review shared utilities for:
- Reusability and modular design
- Type safety and error handling
- Side-effect management
- Documentation and naming clarity
Files:
common/src/utils/selfrica/constants.ts
🧠 Learnings (1)
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Run linting/formatting and ensure no new issues (yarn nice, yarn lint) before PRs
Applied to files:
common/src/utils/selfrica/constants.ts
🪛 GitHub Actions: Common CI
common/src/utils/selfrica/constants.ts
[warning] 1-1: Code style issues found in file. Run Prettier with --write to fix.
⏰ 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). (1)
- GitHub Check: build-deps
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
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)
contracts/test/utils/types.ts (1)
6-36: Import the Selfrica registry type
DeployedActorsV2now exposesregistrySelfrica/registrySelfricaImpl, butIdentityRegistrySelfricaImplV1is never imported. TypeScript will throw “Cannot find name 'IdentityRegistrySelfricaImplV1'”, breaking the test helpers and any consumer of this type. Please add the missing import.IdentityRegistryIdCardImplV1, + IdentityRegistrySelfricaImplV1, TestSelfVerificationRoot,
♻️ Duplicate comments (7)
circuits/tests/disclose/vc_and_disclose_selfrica.test.ts (4)
2-21: Use an ESM-safe test directory helperRedeclaring
__dirnameblows up under CommonJS (Identifier '__dirname' has already been declared) and staysundefinedunder ESM, so fixture reads break. Switch to the TEST_DIR fallback we use elsewhere.-import * as path from 'path'; +import * as path from 'path'; +import { fileURLToPath } from 'url'; … -const __dirname = path.dirname(__filename); +const TEST_DIR = + typeof __dirname === 'undefined' + ? path.dirname(fileURLToPath(import.meta.url)) + : __dirname; … - fs.readFileSync(path.join(__dirname, '../consts/ofac/nameAndDobSelfricaSMT.json'), 'utf8') + fs.readFileSync(path.join(TEST_DIR, '../consts/ofac/nameAndDobSelfricaSMT.json'), 'utf8')(Apply the same TEST_DIR substitution for every
path.join(__dirname, …)in this file.)
124-137: Request every revealedData slot you consume
getOutputonly returns the signals you ask for. Requesting just'revealedData_packed[9]'and then reading indices 0–8 leaves themundefined, sounpackRevealworks on garbage. Pull the full slice instead.- const revealedData = (await circuit.getOutput(witness, ['revealedData_packed[9]'])); + const revealedData = await circuit.getOutput( + witness, + Array.from({ length: 9 }, (_, i) => `revealedData_packed[${i}]`), + );(Apply the same fix to every
getOutputcall that readsrevealedData_packed[0..8].)
70-78: Fail the test without logging raw circuit errorsThis catch logs the full constraint failure (potential PII) and then lets the test pass whenever the message doesn’t match exactly, masking regressions. Assert on the expected substring and drop the log.
- } catch (e) { - const errMsg = e?.message || e?.toString?.() || ""; - if (!errMsg.includes("Num2Bits")) { - console.log('errMsg', errMsg); - throw new Error(`Expected error message to include "Num2Bits", but got:\n${errMsg}`); - } - } + } catch (e) { + const errMsg = e?.message || e?.toString?.() || ""; + expect(errMsg).to.include("Num2Bits"); + }Please remove the other
console.log(e.message)catch blocks later in the file and make them fail fast instead of swallowing errors.
139-172: Do not swallow OFAC circuit failuresThese try/catch blocks log the error and continue, so any witness/constraint regression slips through CI. Drop the catch (or rethrow) and avoid printing the raw error payload.
- try { - const witness = await circuit.calculateWitness(input); - await circuit.checkConstraints(witness); - … - } catch (e) { - console.log(e.message); - } + const witness = await circuit.calculateWitness(input); + await circuit.checkConstraints(witness); + …Apply the same treatment to the other OFAC/non-OFAC tests below.
circuits/circuits/disclose/vc_and_disclose_selfrica.circom (1)
118-157: Binduser_identifierto the computed commitment
user_identifieris a public input but it's never constrained against the identity commitment derived inside the circuit. A prover can therefore choose an arbitrary identifier while still satisfying the rest of the constraints, breaking identity binding for downstream verifiers. Add an equality check right after computingidCommCal.outso the public value is forced to match the hashed output.component idCommCal = CustomHasher(k); idCommCal.in <== msg_sig; //Nullifier = HASH( nullifier sig , scope ) @@ - signal output identity_commitment <== idCommCal.out; + user_identifier === idCommCal.out; + signal output identity_commitment <== idCommCal.out;common/src/utils/selfrica/constants.ts (2)
1-1: Fix formatting and linting issues before merge.Pipeline failures indicate missing newline at EOF and other style issues.
Run:
#!/bin/bash yarn prettier --write common/src/utils/selfrica/constants.ts yarn lint --fix
95-120: Document the public signals interface.The public signal constants define the circuit's output interface but lack explanatory comments. This is critical for circuit developers, frontend consumers, and auditors.
Add documentation covering:
- What each signal represents and its purpose
- Data type/format (field element, packed bytes, etc.)
- For packed signals: packing layout and what LENGTH represents
- Whether CURRENT_DATE is 8 separate elements or packed representation
🧹 Nitpick comments (3)
common/src/constants/constants.ts (1)
6-6: Group attestation ID constants at the top of constants.ts. Attestation IDs (lines 4, 6, 106, 163) should be consolidated into a single block for better maintainability. SELFRICA_ATTESTATION_ID is referenced in your SelfRICA tests, so consolidating won’t introduce unused exports.common/src/utils/selfrica/constants.ts (2)
132-137: Add input validation to prevent silent failures.The function doesn't validate:
serializedDatalength matches expectedSELFRICA_MAX_LENGTH(266 bytes)fieldis a validSelfricaFieldkey- Extracted slice is within bounds
This could lead to silent data corruption or incorrect extraction.
Apply this diff:
export function extractSelfricaField(serializedData: string, field: SelfricaField): string { + if (serializedData.length !== SELFRICA_MAX_LENGTH) { + throw new Error(`Invalid serialized data length: expected ${SELFRICA_MAX_LENGTH}, got ${serializedData.length}`); + } const startIndex = SELFRICA_REVEAL_DATA_INDICES[field]; const length = SELFRICA_FIELD_LENGTHS[field]; + if (startIndex + length > serializedData.length) { + throw new Error(`Field ${field} extraction would exceed data bounds`); + } const extracted = serializedData.slice(startIndex, startIndex + length); return extracted.replace(/\0+$/, ''); }
144-169: Document the 133-bit split and add validation.The function splits the selector at index 133 without explanation. While this appears correct (133 + 133 = 266 total bits), the magic number and rationale should be documented.
Additionally, no validation is performed on the
fieldsToRevealinput.Apply this diff:
/** * Helper function to create a selector field for revealing specific Selfrica data * @param fieldsToReveal - Array of field names to reveal - * @returns Selector value as bigint + * @returns Tuple of [highBits, lowBits] representing the 266-bit selector + * The selector is split at bit 133 to fit in two circuit signals (lowBits: 0-132, highBits: 133-265) */ export function createSelfricaSelector(fieldsToReveal: SelfricaField[]): [bigint, bigint] { + if (!fieldsToReveal.length) { + throw new Error('fieldsToReveal cannot be empty'); + } + // Validate all fields exist + for (const field of fieldsToReveal) { + if (!(field in SELFRICA_FIELD_LENGTHS)) { + throw new Error(`Invalid field: ${field}`); + } + } + const bits = Array(SELFRICA_MAX_LENGTH).fill(0); for (const field of fieldsToReveal) { const selectorBits = SELFRICA_SELECTOR_BITS[field]; for (const bit of selectorBits) { bits[bit] = 1; } } let lowResult = 0n; let highResult = 0n; + // Split at 133 to create two circuit signals (total 266 bits = SELFRICA_MAX_LENGTH) for (let i = 0; i < 133; i++) {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
circuits/circuits/disclose/vc_and_disclose_selfrica.circom(1 hunks)circuits/scripts/build/build_disclose_circuits.sh(1 hunks)circuits/scripts/build/common.sh(2 hunks)circuits/tests/disclose/vc_and_disclose_selfrica.test.ts(1 hunks)common/index.ts(1 hunks)common/src/constants/constants.ts(1 hunks)common/src/utils/selfrica/constants.ts(1 hunks)common/src/utils/selfrica/generateInputs.ts(1 hunks)common/src/utils/selfrica/rsa.ts(1 hunks)common/src/utils/selfrica/types.ts(1 hunks)contracts/contracts/IdentityVerificationHubImplV2.sol(8 hunks)contracts/contracts/constants/AttestationId.sol(1 hunks)contracts/contracts/constants/CircuitConstantsV2.sol(2 hunks)contracts/contracts/interfaces/IIdentityRegistrySelfricaV1.sol(1 hunks)contracts/contracts/interfaces/IVcAndDiscloseCircuitVerifier.sol(1 hunks)contracts/contracts/libraries/CircuitAttributeHandlerV2.sol(5 hunks)contracts/contracts/libraries/CustomVerifier.sol(2 hunks)contracts/contracts/libraries/Formatter.sol(1 hunks)contracts/contracts/libraries/SelfStructs.sol(1 hunks)contracts/contracts/registry/IdentityRegistrySelfricaImplV1.sol(1 hunks)contracts/test/utils/deploymentV2.ts(10 hunks)contracts/test/utils/generateProof.ts(5 hunks)contracts/test/utils/types.ts(3 hunks)contracts/test/v2/discloseSelfrica.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- common/src/utils/selfrica/types.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: 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:
common/src/constants/constants.tscontracts/contracts/libraries/SelfStructs.solcontracts/contracts/libraries/Formatter.solcontracts/test/utils/deploymentV2.tscontracts/contracts/libraries/CustomVerifier.solcommon/index.tscommon/src/utils/selfrica/rsa.tscontracts/contracts/constants/CircuitConstantsV2.solcontracts/test/utils/generateProof.tscontracts/contracts/IdentityVerificationHubImplV2.solcontracts/contracts/libraries/CircuitAttributeHandlerV2.solcommon/src/utils/selfrica/generateInputs.tscontracts/test/utils/types.tscircuits/tests/disclose/vc_and_disclose_selfrica.test.tscontracts/contracts/interfaces/IIdentityRegistrySelfricaV1.solcontracts/contracts/registry/IdentityRegistrySelfricaImplV1.solcontracts/contracts/constants/AttestationId.solcommon/src/utils/selfrica/constants.tscontracts/contracts/interfaces/IVcAndDiscloseCircuitVerifier.solcontracts/test/v2/discloseSelfrica.test.ts
common/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
common/src/**/*.{ts,tsx,js,jsx}: Review shared utilities for:
- Reusability and modular design
- Type safety and error handling
- Side-effect management
- Documentation and naming clarity
Files:
common/src/constants/constants.tscommon/src/utils/selfrica/rsa.tscommon/src/utils/selfrica/generateInputs.tscommon/src/utils/selfrica/constants.ts
contracts/**
📄 CodeRabbit inference engine (AGENTS.md)
Compile Solidity contracts using
yarn workspace @selfxyz/contracts build
Files:
contracts/contracts/libraries/SelfStructs.solcontracts/contracts/libraries/Formatter.solcontracts/test/utils/deploymentV2.tscontracts/contracts/libraries/CustomVerifier.solcontracts/contracts/constants/CircuitConstantsV2.solcontracts/test/utils/generateProof.tscontracts/contracts/IdentityVerificationHubImplV2.solcontracts/contracts/libraries/CircuitAttributeHandlerV2.solcontracts/test/utils/types.tscontracts/contracts/interfaces/IIdentityRegistrySelfricaV1.solcontracts/contracts/registry/IdentityRegistrySelfricaImplV1.solcontracts/contracts/constants/AttestationId.solcontracts/contracts/interfaces/IVcAndDiscloseCircuitVerifier.solcontracts/test/v2/discloseSelfrica.test.ts
contracts/**/*.sol
⚙️ CodeRabbit configuration file
contracts/**/*.sol: Review Solidity smart contracts for:
- Security vulnerabilities (reentrancy, overflow, etc.)
- Gas optimization opportunities
- Access control patterns
- Event emission for important state changes
- Code documentation and NatSpec comments
Files:
contracts/contracts/libraries/SelfStructs.solcontracts/contracts/libraries/Formatter.solcontracts/contracts/libraries/CustomVerifier.solcontracts/contracts/constants/CircuitConstantsV2.solcontracts/contracts/IdentityVerificationHubImplV2.solcontracts/contracts/libraries/CircuitAttributeHandlerV2.solcontracts/contracts/interfaces/IIdentityRegistrySelfricaV1.solcontracts/contracts/registry/IdentityRegistrySelfricaImplV1.solcontracts/contracts/constants/AttestationId.solcontracts/contracts/interfaces/IVcAndDiscloseCircuitVerifier.sol
contracts/contracts/IdentityVerificationHubImplV2.sol
📄 CodeRabbit inference engine (.cursorrules)
Identity Verification Hub should manage multi-step verification for passports and EU ID cards, handle document attestation with zero-knowledge proofs, and implement verification paths: E-PASSPORT and EU_ID_CARD.
Files:
contracts/contracts/IdentityVerificationHubImplV2.sol
circuits/**/*.circom
⚙️ CodeRabbit configuration file
circuits/**/*.circom: Review ZK circuit code for:
- Circuit correctness and completeness
- Constraint efficiency
- Input validation
- Security considerations for zero-knowledge proofs
Files:
circuits/circuits/disclose/vc_and_disclose_selfrica.circom
**/*.{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:
circuits/tests/disclose/vc_and_disclose_selfrica.test.tscontracts/test/v2/discloseSelfrica.test.ts
🧠 Learnings (7)
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to contracts/contracts/IdentityVerificationHubImplV2.sol : Identity Verification Hub should manage multi-step verification for passports and EU ID cards, handle document attestation with zero-knowledge proofs, and implement verification paths: E-PASSPORT and EU_ID_CARD.
Applied to files:
contracts/test/utils/deploymentV2.tscontracts/contracts/IdentityVerificationHubImplV2.solcontracts/test/utils/types.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to contracts/contracts/registry/IdentityRegistryImplV1.sol : Identity Registry Management maintains separate registries for passports and ID cards, handles DSC key commitment registration, and tracks nullifiers for duplicate prevention.
Applied to files:
contracts/test/utils/deploymentV2.tscontracts/test/utils/types.tscontracts/contracts/interfaces/IIdentityRegistrySelfricaV1.solcontracts/contracts/registry/IdentityRegistrySelfricaImplV1.sol
📚 Learning: 2025-08-24T18:53:11.221Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Integrate forbidden-country non-inclusion proof into the disclosure proof circuit
Applied to files:
circuits/scripts/build/build_disclose_circuits.shcircuits/circuits/disclose/vc_and_disclose_selfrica.circom
📚 Learning: 2025-08-24T18:53:11.221Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Ensure the circuit performs day-level comparison with correct date arithmetic
Applied to files:
contracts/contracts/IdentityVerificationHubImplV2.sol
📚 Learning: 2025-08-24T18:53:11.221Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Implement the age verification circuit at circuits/circuits/disclose/disclose.circom with private DOB input, public minimum-age input, and boolean output
Applied to files:
circuits/circuits/disclose/vc_and_disclose_selfrica.circom
📚 Learning: 2025-08-25T11:20:43.865Z
Learnt from: Vishalkulkarni45
PR: selfxyz/self#949
File: circuits/circuits/disclose/vc_and_disclose_aadhaar.circom:223-223
Timestamp: 2025-08-25T11:20:43.865Z
Learning: In Aadhaar disclosure circuits, the user_identifier signal is used for replay attack prevention. Even when constrained in a "dummy" operation like `signal dummy <== user_identifier + user_identifier;`, this ensures the identifier is part of the witness and contributes to proof uniqueness, preventing replay attacks.
Applied to files:
circuits/circuits/disclose/vc_and_disclose_selfrica.circom
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Run linting/formatting and ensure no new issues (yarn nice, yarn lint) before PRs
Applied to files:
common/src/utils/selfrica/constants.ts
🧬 Code graph analysis (7)
contracts/test/utils/deploymentV2.ts (1)
contracts/test/utils/generateProof.ts (1)
getSMTs(537-565)
common/src/utils/selfrica/rsa.ts (1)
common/index.ts (3)
generateRSAKeyPair(145-145)signRSA(146-146)verifyRSA(147-147)
contracts/test/utils/generateProof.ts (2)
contracts/test/utils/types.ts (1)
CircuitArtifacts(115-125)common/src/utils/selfrica/generateInputs.ts (1)
generateCircuitInputWithRealData(166-248)
common/src/utils/selfrica/generateInputs.ts (6)
common/src/utils/selfrica/constants.ts (7)
SELFRICA_MAX_LENGTH(37-37)SelfricaField(93-93)createSelfricaSelector(144-169)SELFRICA_FULL_NAME_INDEX(16-16)SELFRICA_FULL_NAME_LENGTH(17-17)SELFRICA_DOB_INDEX(19-19)SELFRICA_DOB_LENGTH(20-20)common/src/utils/selfrica/types.ts (3)
SmileData(3-20)serializeSmileData(22-39)SelfricaCircuitInput(41-60)common/src/utils/trees.ts (3)
getNameDobLeafSelfrica(767-772)getNameYobLeafSelfrica(809-815)generateSMTProof(176-225)common/src/utils/circuits/generateInputs.ts (1)
formatInput(58-78)common/src/utils/selfrica/rsa.ts (3)
generateRSAKeyPair(45-45)signRSA(45-45)verifyRSA(45-45)common/src/utils/bytes.ts (1)
splitToWords(140-153)
circuits/tests/disclose/vc_and_disclose_selfrica.test.ts (4)
common/index.ts (6)
generateCircuitInput(129-129)customHasher(110-110)SELFRICA_MAX_LENGTH(137-137)serializeSmileData(141-141)NON_OFAC_DUMMY_INPUT(132-132)OFAC_DUMMY_INPUT(133-133)common/src/utils/selfrica/generateInputs.ts (3)
generateCircuitInput(104-164)NON_OFAC_DUMMY_INPUT(61-78)OFAC_DUMMY_INPUT(42-59)common/src/utils/selfrica/constants.ts (1)
SELFRICA_MAX_LENGTH(37-37)common/src/utils/selfrica/types.ts (1)
serializeSmileData(22-39)
common/src/utils/selfrica/constants.ts (1)
common/index.ts (1)
SELFRICA_MAX_LENGTH(137-137)
contracts/test/v2/discloseSelfrica.test.ts (5)
common/index.ts (9)
generateRSAKeyPair(145-145)Country3LetterCode(22-22)hashEndpointWithScope(88-88)calculateUserIdentifierHash(66-66)generateCircuitInputWithRealData(130-130)serializeSmileData(141-141)NON_OFAC_DUMMY_INPUT(132-132)customHasher(110-110)pubkeyCommitment(131-131)common/src/utils/selfrica/rsa.ts (1)
generateRSAKeyPair(45-45)contracts/test/utils/generateProof.ts (2)
getSMTs(537-565)generateVcAndDiscloseSelfricaProof(491-514)common/src/utils/selfrica/generateInputs.ts (3)
generateCircuitInputWithRealData(166-248)NON_OFAC_DUMMY_INPUT(61-78)pubkeyCommitment(250-260)common/src/utils/selfrica/types.ts (1)
serializeSmileData(22-39)
🔇 Additional comments (2)
common/src/constants/constants.ts (1)
6-6: LGTM! Sequential attestation ID added correctly.The new
SELFRICA_ATTESTATION_IDfollows the existing naming convention and maintains the sequential numbering pattern (after '3'). The string type is consistent with other attestation IDs.common/src/utils/selfrica/constants.ts (1)
39-91: Well-structured constant definitions.The field length mappings, reveal indices, and selector bit arrays are well-organized with clear patterns. The use of
as constfor type narrowing and inline comments showing computed values improves maintainability.
| -l node_modules \ | ||
| -l node_modules/@zk-kit/binary-merkle-root.circom/src \ | ||
| -l node_modules/circomlib/circuits \ |
There was a problem hiding this comment.
Include path now breaks circom builds
These -l node_modules paths only resolve if we invoke the build scripts from the repo root. However, the rest of this script still assumes we run from circuits/ (e.g., ../contracts/... later on). From that working directory the new include path resolves to circuits/node_modules, so circom can’t load binary-merkle-root or circomlib and the build fails. Please restore the previous relative path (or compute an absolute path) so the includes resolve correctly.
- circom ${CIRCUIT_PATH} \
- -l node_modules \
- -l node_modules/@zk-kit/binary-merkle-root.circom/src \
- -l node_modules/circomlib/circuits \
+ circom ${CIRCUIT_PATH} \
+ -l ../node_modules \
+ -l ../node_modules/@zk-kit/binary-merkle-root.circom/src \
+ -l ../node_modules/circomlib/circuits \🤖 Prompt for AI Agents
In circuits/scripts/build/common.sh around lines 75 to 77, the added "-l
node_modules" include paths assume the script runs from the repo root and break
when the script runs from the circuits/ directory; restore the previous relative
include paths (or compute absolute paths from the script location) so circom can
find binary-merkle-root and circomlib. Change the three include flags to point
to ../node_modules and ../node_modules/<package>/... (or derive an absolute path
using the script's directory and use that for all -l flags) so the rest of the
script's ../contracts/... references remain correct and builds succeed
regardless of CWD.
| //TODO: FIX THESE | ||
| export const SELFRICA_PUBLIC_SIGNALS_PUBKEY = 16; | ||
| export const SELFRICA_PUBLIC_SIGNALS_PUBKEY_LENGTH = 17; |
There was a problem hiding this comment.
Resolve TODO before merge.
The TODO comment indicates incomplete work on PUBKEY constants. These public signals are part of the circuit's output interface and must be verified.
Verify:
- Are the index values (16, 17) correct for the circuit's public signal layout?
- Should PUBKEY_LENGTH be a count (like other
_LENGTHconstants) rather than another index? - Does the circuit actually output PUBKEY as multiple elements requiring a length constant?
🤖 Prompt for AI Agents
In common/src/utils/selfrica/constants.ts around lines 109-111, the two
constants labeled as TODO are wrong: verify the circuit's public signal layout
to determine the correct start index for SELFRICA_PUBLIC_SIGNALS_PUBKEY and
whether SELFRICA_PUBLIC_SIGNALS_PUBKEY_LENGTH should be a length (count of
elements) rather than an index. Inspect the circuit's exported public signal
ordering (circuit definition / witness schema / verifier input mapping) and set
SELFRICA_PUBLIC_SIGNALS_PUBKEY to the actual starting index of the pubkey in
that ordering, set SELFRICA_PUBLIC_SIGNALS_PUBKEY_LENGTH to the number of
elements the pubkey occupies (matching other _LENGTH constants), remove the
TODO, and add a short comment referencing the circuit file/commit used to derive
these values; run existing tests or add a small unit test asserting these
constants match the circuit's publicSignals shape.
| current_date: ['2', '0', '2', '4', '0', '1', '0', '1'], | ||
| majority_age_ASCII: ['0', '0', '1'].map((x) => x.charCodeAt(0)), | ||
| selector_older_than: ['1'], |
There was a problem hiding this comment.
Provide ASCII-coded dates/ages to match circuit expectations
Both generators feed plain digits ('2', '0', …) into current_date, so formatInput converts them to the numeric values 2, 0, … instead of the ASCII codes the date validators compare against. In the dummy generator you also encode "001" for majority_age_ASCII, which undermines the “older than 20” guard. Encode these fields as ASCII code points and keep the age at "020" so the age/date checks behave correctly.
+const asciiDigits = (value: string) => value.split("").map((c) => c.charCodeAt(0).toString());
@@
- current_date: ['2', '0', '2', '4', '0', '1', '0', '1'],
- majority_age_ASCII: ['0', '0', '1'].map((x) => x.charCodeAt(0)),
+ current_date: asciiDigits("20240101"),
+ majority_age_ASCII: "020".split("").map((x) => x.charCodeAt(0)),
@@
- const currentDate = new Date().toISOString().split('T')[0].replace(/-/g, '').split("");
+ const currentDate = asciiDigits(new Date().toISOString().split("T")[0].replace(/-/g, ""));
@@
- current_date: currentDate,
+ current_date: currentDate,Also applies to: 242-244
🤖 Prompt for AI Agents
In common/src/utils/selfrica/generateInputs.ts around lines 158-160 (also apply
same fix to lines 242-244), the generator supplies plain digit characters for
current_date and an incorrect majority_age_ASCII value causing formatInput to
yield numeric digits instead of ASCII code points and misrepresenting the age;
replace those entries with arrays of ASCII code points (use .charCodeAt for each
character or hardcode the codepoint numbers) for current_date and set
majority_age_ASCII to represent "020" (ASCII codes for '0','2','0') so the
circuit compares against ASCII-encoded dates/ages and the "older than 20" guard
works correctly.
| return | ||
| FieldPositions({ | ||
| issuingStateStart: 50, | ||
| issuingStateEnd: 57, | ||
| nameStart: 66, | ||
| nameEnd: 105, | ||
| documentNumberStart: 30, | ||
| documentNumberEnd: 49, | ||
| nationalityStart: 0, | ||
| nationalityEnd: 2, | ||
| dateOfBirthStart: 106, | ||
| dateOfBirthEnd: 113, | ||
| genderStart: 160, | ||
| genderEnd: 165, | ||
| expiryDateStart: 58, | ||
| expiryDateEnd: 65, | ||
| olderThanStart: 268, | ||
| olderThanEnd: 270, | ||
| ofacStart: 266, | ||
| ofacEnd: 267 | ||
| }); |
There was a problem hiding this comment.
Fix SelfRICA date formatting before it ships
The new SelfRICA field positions expose 8-character (YYYYMMDD) slices for both DOB and expiry, but getDateOfBirth / getExpiryDate still funnel that payload through Formatter.formatDate, which hard-reverts unless the slice is exactly 6 chars. Any consumer that keeps using the existing getters with SELFRICA_ID_CARD will now blow up at runtime (and we already added a formatDateFullYear helper, so we clearly expect the longer width). Please branch on the attestation and call the full-year formatter instead.
function getDateOfBirth(bytes32 attestationId, bytes memory charcodes) internal pure returns (string memory) {
FieldPositions memory positions = getFieldPositions(attestationId);
- return
- Formatter.formatDate(
- extractStringAttribute(charcodes, positions.dateOfBirthStart, positions.dateOfBirthEnd)
- );
+ string memory raw = extractStringAttribute(charcodes, positions.dateOfBirthStart, positions.dateOfBirthEnd);
+ if (attestationId == AttestationId.SELFRICA_ID_CARD) {
+ return Formatter.formatDateFullYear(raw);
+ }
+ return Formatter.formatDate(raw);
}
@@
function getExpiryDate(bytes32 attestationId, bytes memory charcodes) internal pure returns (string memory) {
FieldPositions memory positions = getFieldPositions(attestationId);
- return
- Formatter.formatDate(extractStringAttribute(charcodes, positions.expiryDateStart, positions.expiryDateEnd));
+ string memory raw = extractStringAttribute(charcodes, positions.expiryDateStart, positions.expiryDateEnd);
+ if (attestationId == AttestationId.SELFRICA_ID_CARD) {
+ return Formatter.formatDateFullYear(raw);
+ }
+ return Formatter.formatDate(raw);
}📝 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.
| return | |
| FieldPositions({ | |
| issuingStateStart: 50, | |
| issuingStateEnd: 57, | |
| nameStart: 66, | |
| nameEnd: 105, | |
| documentNumberStart: 30, | |
| documentNumberEnd: 49, | |
| nationalityStart: 0, | |
| nationalityEnd: 2, | |
| dateOfBirthStart: 106, | |
| dateOfBirthEnd: 113, | |
| genderStart: 160, | |
| genderEnd: 165, | |
| expiryDateStart: 58, | |
| expiryDateEnd: 65, | |
| olderThanStart: 268, | |
| olderThanEnd: 270, | |
| ofacStart: 266, | |
| ofacEnd: 267 | |
| }); | |
| function getDateOfBirth(bytes32 attestationId, bytes memory charcodes) internal pure returns (string memory) { | |
| FieldPositions memory positions = getFieldPositions(attestationId); | |
| string memory raw = extractStringAttribute(charcodes, positions.dateOfBirthStart, positions.dateOfBirthEnd); | |
| if (attestationId == AttestationId.SELFRICA_ID_CARD) { | |
| return Formatter.formatDateFullYear(raw); | |
| } | |
| return Formatter.formatDate(raw); | |
| } | |
| function getExpiryDate(bytes32 attestationId, bytes memory charcodes) internal pure returns (string memory) { | |
| FieldPositions memory positions = getFieldPositions(attestationId); | |
| string memory raw = extractStringAttribute(charcodes, positions.expiryDateStart, positions.expiryDateEnd); | |
| if (attestationId == AttestationId.SELFRICA_ID_CARD) { | |
| return Formatter.formatDateFullYear(raw); | |
| } | |
| return Formatter.formatDate(raw); | |
| } |
🤖 Prompt for AI Agents
In contracts/contracts/libraries/CircuitAttributeHandlerV2.sol around lines 116
to 136, the getters for dateOfBirth and expiryDate still call
Formatter.formatDate which expects 6-char dates but SelfRICA slices are 8-char
(YYYYMMDD); update those getters to detect when attestation == SELFRICA_ID_CARD
(or the SelfRICA attestation constant used here) and call
Formatter.formatDateFullYear for those cases, falling back to
Formatter.formatDate for other attestations, ensuring both getDateOfBirth and
getExpiryDate use the branched formatter so 8-character dates parse correctly.
| function initialize(address _hub) external initializer { | ||
| __ImplRoot_init(); | ||
| _hub = _hub; | ||
| emit RegistryInitialized(_hub); | ||
| } |
There was a problem hiding this comment.
Fix storage shadowing in initialize
The initializer assigns the parameter _hub back to itself, leaving the storage slot _hub at zero. As a result, every onlyHub call reverts (HUB_NOT_SET()), bricking the registry. Rename the parameter (or use explicit storage reference) so the assignment targets the storage variable.
- function initialize(address _hub) external initializer {
+ function initialize(address hub_) external initializer {
__ImplRoot_init();
- _hub = _hub;
- emit RegistryInitialized(_hub);
+ _hub = hub_;
+ emit RegistryInitialized(hub_);
}📝 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 initialize(address _hub) external initializer { | |
| __ImplRoot_init(); | |
| _hub = _hub; | |
| emit RegistryInitialized(_hub); | |
| } | |
| function initialize(address hub_) external initializer { | |
| __ImplRoot_init(); | |
| _hub = hub_; | |
| emit RegistryInitialized(hub_); | |
| } |
🤖 Prompt for AI Agents
In contracts/contracts/registry/IdentityRegistrySelfricaImplV1.sol around lines
121-125, the initializer mistakenly assigns the function parameter back to
itself, leaving the contract storage variable `_hub` unset; rename the incoming
parameter (e.g. `address hub_`) or otherwise avoid shadowing, then set the
storage slot (`_hub = hub_`) and emit the event with the stored value so
`onlyHub` checks succeed.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
common/src/utils/types.ts (1)
55-61: Include 'selfrica' in DocumentType union to match new document categoryWithout this, SelfricaData.documentType cannot be 'selfrica' and may cause TS errors.
export type DocumentType = | 'passport' | 'id_card' | 'aadhaar' + | 'selfrica' | 'mock_passport' | 'mock_id_card' | 'mock_aadhaar';
♻️ Duplicate comments (9)
circuits/tests/disclose/vc_and_disclose_selfrica.test.ts (4)
14-21: ESM-safe path resolution: replace __dirname redeclaration with TEST_DIR fallback.Avoid redeclaring __dirname; use fileURLToPath fallback and update path joins.
Apply this diff within this file:
+import { fileURLToPath } from 'url'; @@ -const __dirname = path.dirname(__filename); +const TEST_DIR = + typeof __dirname === 'undefined' + ? path.dirname(fileURLToPath(import.meta.url)) + : __dirname; @@ - fs.readFileSync(path.join(__dirname, '../consts/ofac/nameAndDobSelfricaSMT.json'), 'utf8') + fs.readFileSync(path.join(TEST_DIR, '../consts/ofac/nameAndDobSelfricaSMT.json'), 'utf8') @@ - fs.readFileSync(path.join(__dirname, '../consts/ofac/nameAndYobSelfricaSMT.json'), 'utf8') + fs.readFileSync(path.join(TEST_DIR, '../consts/ofac/nameAndYobSelfricaSMT.json'), 'utf8')As per coding guidelines.
Also applies to: 16-21
124-136: Fix getOutput selection: request all indices you consume.Only requesting [9] but reading [0..8] yields undefined.
Apply this pattern for each call:
-const revealedData = (await circuit.getOutput(witness, ['revealedData_packed[9]'])); +const revealedData = await circuit.getOutput( + witness, + Array.from({ length: 10 }, (_, i) => `revealedData_packed[${i}]`) +);Do this at Lines 124, 153, 184, 245 (and keep the subsequent reads 0..8 as-is).
Also applies to: 153-165, 184-195, 245-256
71-78: Remove verbose error logging in negative assertion.Do not print full errMsg; assert concisely to avoid leaking inputs.
- const errMsg = e?.message || e?.toString?.() || ""; - if (!errMsg.includes("Num2Bits")) { - console.log('errMsg', errMsg); - throw new Error(`Expected error message to include "Num2Bits", but got:\n${errMsg}`); - } + const errMsg = typeof e?.message === 'string' ? e.message : String(e ?? ''); + if (!errMsg.includes('Num2Bits')) { + throw new Error('Expected error message to include "Num2Bits".'); + }As per coding guidelines.
139-143: Do not swallow failures or log raw errors; rethrow to fail tests.console.log leaks PII and hides regressions.
- } catch (e) { - console.log(e.message); - } + } catch (e) { + throw e; + }Apply at all listed catch blocks. As per coding guidelines.
Also applies to: 168-171, 224-227, 285-288
circuits/circuits/utils/selfrica/disclose/disclose.circom (1)
72-76: Constrain selector_smile_data to boolean {0,1} before masking.Without booleanity, provers can use non-binary masks and produce malformed reveals.
signal revealed_data[selfrica_max_length + 2 + 3]; - for (var i = 0; i < selfrica_max_length; i++) { + for (var i = 0; i < selfrica_max_length; i++) { + // enforce boolean mask + selector_smile_data[i] * (selector_smile_data[i] - 1) === 0; revealed_data[i] <== smile_data[i] * selector_smile_data[i]; }circuits/circuits/disclose/vc_and_disclose_selfrica.circom (1)
154-161: Bind public user_identifier to the derived identity commitment.Currently unconstrained; a prover can claim any identifier. Enforce equality to the computed commitment.
- signal output identity_commitment <== idCommCal.out; + signal output identity_commitment <== idCommCal.out; + user_identifier === idCommCal.out;This ties the public input to the commitment and prevents misuse. Based on learnings.
common/src/utils/selfrica/generateInputs.ts (3)
36-39: Fix compressed_disclose_sel word order; remove ad‑hoc .reverse()Circuit expects [low, high] but createDiscloseSelFromFields returns [high, low]. Return [low, high] from the helper and drop .reverse() at call sites for consistency.
@@ -export const createDiscloseSelFromFields = (fieldsToReveal: SelfricaField[]): string[] => { - const [highResult, lowResult] = createSelfricaSelector(fieldsToReveal); - return [highResult.toString(), lowResult.toString()]; -}; +export const createDiscloseSelFromFields = (fieldsToReveal: SelfricaField[]): string[] => { + const [highResult, lowResult] = createSelfricaSelector(fieldsToReveal); + // Circuit expects [low, high] + return [lowResult.toString(), highResult.toString()]; +}; @@ - const fieldsToRevealFinal = fieldsToReveal || []; - const compressed_disclose_sel = createDiscloseSelFromFields(fieldsToRevealFinal).reverse(); + const fieldsToRevealFinal = fieldsToReveal || []; + const compressed_disclose_sel = createDiscloseSelFromFields(fieldsToRevealFinal); @@ - const compressed_disclose_sel = createDiscloseSelFromFields(fieldsToRevealFinal).reverse(); + const compressed_disclose_sel = createDiscloseSelFromFields(fieldsToRevealFinal);Also applies to: 195-197, 281-282
6-6: Use schema constants for ID-number slice (avoid magic numbers when signing nullifier)Hardcoded slice(30, 30 + 20) is brittle; sign the exact ID-number byte-range from constants to preserve nullifier integrity.
@@ -import { createSelfricaSelector, SELFRICA_DOB_INDEX, SELFRICA_DOB_LENGTH, SELFRICA_FULL_NAME_INDEX, SELFRICA_FULL_NAME_LENGTH, SELFRICA_MAX_LENGTH, SelfricaField } from "./constants.js"; +import { createSelfricaSelector, SELFRICA_DOB_INDEX, SELFRICA_DOB_LENGTH, SELFRICA_FULL_NAME_INDEX, SELFRICA_FULL_NAME_LENGTH, SELFRICA_ID_NUMBER_INDEX, SELFRICA_ID_NUMBER_LENGTH, SELFRICA_MAX_LENGTH, SelfricaField } from "./constants.js"; @@ - // Sign nullifier with RSA - const idNumber = Buffer.from(msgArray.slice(30, 30 + 20)); + // Sign nullifier with RSA + const idNumber = Buffer.from( + msgArray.slice(SELFRICA_ID_NUMBER_INDEX, SELFRICA_ID_NUMBER_INDEX + SELFRICA_ID_NUMBER_LENGTH) + ); @@ - // Sign nullifier with RSA - const idNumber = Buffer.from(msgArray.slice(30, 30 + 20)); + // Sign nullifier with RSA + const idNumber = Buffer.from( + msgArray.slice(SELFRICA_ID_NUMBER_INDEX, SELFRICA_ID_NUMBER_INDEX + SELFRICA_ID_NUMBER_LENGTH) + );Also applies to: 124-131, 261-267
12-12: Encode dates/ages as ASCII codepoints; fix dummy majority age and remove stray logCircuits compare ASCII digits. Provide ASCII codes for current_date and majority_age, keep majority age at 20 by default, and drop console.log.
@@ export const splitDiscloseSel = (disclose_sel: string[]): string[] => { @@ } +const asciiDigits = (s: string) => s.split("").map((c) => c.charCodeAt(0).toString()); + @@ - user_identifier: '1234567890', - current_date: ['2', '0', '2', '4', '0', '1', '0', '1'], - majority_age_ASCII: ['0', '0', '1'].map((x) => x.charCodeAt(0)), + user_identifier: '1234567890', + current_date: asciiDigits(smileData.current_date), + majority_age_ASCII: "020".split("").map((x) => x.charCodeAt(0)), @@ - //generate the current date - const currentDate = new Date().toISOString().split('T')[0].replace(/-/g, '').split(""); + // generate the current date (ASCII codepoints) + const currentDateStr = new Date().toISOString().split('T')[0].replace(/-/g, ''); + const currentDate = asciiDigits(currentDateStr); @@ - current_date: currentDate, + current_date: currentDate, @@ - //generate the current date - const currentDate = new Date().toISOString().split('T')[0].replace(/-/g, '').split(""); + // generate the current date (ASCII codepoints) + const currentDateStr = new Date().toISOString().split('T')[0].replace(/-/g, ''); + const currentDate = asciiDigits(currentDateStr); @@ - console.log('currentDate', currentDate); + // (removed debug logging) @@ - current_date: currentDate, - majority_age_ASCII: ['0', '2', '0'].map((x) => x.charCodeAt(0)), + current_date: currentDate, + majority_age_ASCII: ((20).toString().padStart(3, '0')) + .split("") + .map((x) => x.charCodeAt(0)),Optionally, pad a provided minimumAge in generateSelfricaInputWithOutSig:
- const majorityAgeASCII = minimumAge ? minimumAge.toString().split("").map((x) => x.charCodeAt(0)) : ['0', '0', '0'].map((x) => x.charCodeAt(0)); + const majorityAgeASCII = ((minimumAge ?? 20).toString().padStart(3, '0')) + .split("") + .map((x) => x.charCodeAt(0));Also applies to: 160-163, 198-201, 217-219, 283-286, 304-306
🧹 Nitpick comments (3)
circuits/circuits/utils/selfrica/disclose/disclose.circom (2)
31-36: Add ASCII-digit validation for current_date and majority_age_ASCII (optional but recommended).Guard against non-digit bytes to keep date/age logic sound.
Suggestion: use existing byte/digit checks (e.g., a ByteIsDigit helper) on current_date[8] and majority_age_ASCII[3], or add small range-check components to ensure each is within ASCII '0'..'9'. Based on learnings.
Also applies to: 38-53
77-82: Optionally gate age bytes by a selector.If you intend conditional disclosure, multiply older_than_verified bytes by a selector (e.g., selector_older_than) similar to selector_ofac.
common/src/utils/circuits/registerInputs.ts (1)
73-84: Ensure excludedCountries format matches circuit forbidden_countries_listConfirm disclosures.excludedCountries is the exact string[] shape (length/order) expected by the circuit; otherwise, OFAC checks may misbehave.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
circuits/circuits/disclose/vc_and_disclose_selfrica.circom(1 hunks)circuits/circuits/utils/selfrica/disclose/disclose.circom(1 hunks)circuits/tests/disclose/vc_and_disclose_selfrica.test.ts(1 hunks)common/src/utils/circuits/registerInputs.ts(3 hunks)common/src/utils/selfrica/generateInputs.ts(1 hunks)common/src/utils/selfrica/types.ts(1 hunks)common/src/utils/types.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- common/src/utils/selfrica/types.ts
🧰 Additional context used
📓 Path-based instructions (4)
circuits/**/*.circom
⚙️ CodeRabbit configuration file
circuits/**/*.circom: Review ZK circuit code for:
- Circuit correctness and completeness
- Constraint efficiency
- Input validation
- Security considerations for zero-knowledge proofs
Files:
circuits/circuits/disclose/vc_and_disclose_selfrica.circomcircuits/circuits/utils/selfrica/disclose/disclose.circom
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: 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:
common/src/utils/types.tscommon/src/utils/circuits/registerInputs.tscommon/src/utils/selfrica/generateInputs.tscircuits/tests/disclose/vc_and_disclose_selfrica.test.ts
common/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
common/src/**/*.{ts,tsx,js,jsx}: Review shared utilities for:
- Reusability and modular design
- Type safety and error handling
- Side-effect management
- Documentation and naming clarity
Files:
common/src/utils/types.tscommon/src/utils/circuits/registerInputs.tscommon/src/utils/selfrica/generateInputs.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:
circuits/tests/disclose/vc_and_disclose_selfrica.test.ts
🧠 Learnings (4)
📚 Learning: 2025-08-24T18:53:11.221Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Integrate forbidden-country non-inclusion proof into the disclosure proof circuit
Applied to files:
circuits/circuits/disclose/vc_and_disclose_selfrica.circomcircuits/circuits/utils/selfrica/disclose/disclose.circom
📚 Learning: 2025-08-24T18:53:11.221Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Implement the age verification circuit at circuits/circuits/disclose/disclose.circom with private DOB input, public minimum-age input, and boolean output
Applied to files:
circuits/circuits/disclose/vc_and_disclose_selfrica.circomcircuits/circuits/utils/selfrica/disclose/disclose.circom
📚 Learning: 2025-08-25T11:20:43.865Z
Learnt from: Vishalkulkarni45
PR: selfxyz/self#949
File: circuits/circuits/disclose/vc_and_disclose_aadhaar.circom:223-223
Timestamp: 2025-08-25T11:20:43.865Z
Learning: In Aadhaar disclosure circuits, the user_identifier signal is used for replay attack prevention. Even when constrained in a "dummy" operation like `signal dummy <== user_identifier + user_identifier;`, this ensures the identifier is part of the witness and contributes to proof uniqueness, preventing replay attacks.
Applied to files:
circuits/circuits/disclose/vc_and_disclose_selfrica.circom
📚 Learning: 2025-08-24T18:53:11.221Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.221Z
Learning: Applies to circuits/circuits/disclose/disclose.circom : Ensure the circuit performs day-level comparison with correct date arithmetic
Applied to files:
circuits/circuits/utils/selfrica/disclose/disclose.circom
🧬 Code graph analysis (4)
common/src/utils/types.ts (4)
common/src/utils/selfrica/constants.ts (1)
SelfricaField(93-93)common/index.ts (4)
DocumentCategory(7-7)IDDocument(9-9)AadhaarData(3-3)PassportData(12-12)common/src/utils/index.ts (4)
DocumentCategory(5-5)IDDocument(7-7)AadhaarData(2-2)PassportData(9-9)packages/mobile-sdk-alpha/src/types/public.ts (2)
IDDocument(16-16)PassportData(16-16)
common/src/utils/circuits/registerInputs.ts (4)
common/src/utils/types.ts (3)
SelfricaData(25-29)DocumentCategory(44-44)OfacTree(67-71)common/src/utils/hash/custom.ts (1)
calculateUserIdentifierHash(2-2)common/src/utils/selfrica/constants.ts (1)
SelfricaField(93-93)common/src/utils/selfrica/generateInputs.ts (1)
generateSelfricaInputWithOutSig(168-223)
common/src/utils/selfrica/generateInputs.ts (5)
common/src/utils/selfrica/constants.ts (7)
SELFRICA_MAX_LENGTH(37-37)SelfricaField(93-93)createSelfricaSelector(144-169)SELFRICA_FULL_NAME_INDEX(16-16)SELFRICA_FULL_NAME_LENGTH(17-17)SELFRICA_DOB_INDEX(19-19)SELFRICA_DOB_LENGTH(20-20)common/src/utils/selfrica/types.ts (3)
SmileData(3-20)serializeSmileData(22-39)SelfricaCircuitInput(41-59)common/src/utils/trees.ts (3)
getNameDobLeafSelfrica(767-772)getNameYobLeafSelfrica(809-815)generateSMTProof(176-225)common/src/utils/circuits/generateInputs.ts (1)
formatInput(58-78)common/src/utils/bytes.ts (1)
splitToWords(140-153)
circuits/tests/disclose/vc_and_disclose_selfrica.test.ts (3)
common/src/utils/selfrica/generateInputs.ts (3)
generateCircuitInput(104-166)NON_OFAC_DUMMY_INPUT(61-78)OFAC_DUMMY_INPUT(42-59)common/src/utils/selfrica/constants.ts (1)
SELFRICA_MAX_LENGTH(37-37)common/src/utils/selfrica/types.ts (1)
serializeSmileData(22-39)
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
export function logic to external libs, reduce compiler runs to 200, update deploy scripts to link new libs
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
4 participants
Note
Adds SelfRICA selective‑disclosure circuit with OFAC checks and age/date validation, supporting utilities (constants, input gen, RSA, trees, decoding) and comprehensive tests; minor Aadhaar test path fixes.
circuits/disclose/vc_and_disclose_selfrica.circomwith selective disclosure, identity commitment, and nullifier.utils/selfrica/disclose/ofac/ofac_name_dob_selfrica.circom,ofac_name_yob_selfrica.circom.utils/selfrica/date/isValid.circom,isOlderThan.circom,dateIsLess.circom.common/src/utils/selfrica/constants.ts,types.ts.generateInputs.ts(incl. RSA signing, disclose bits),decodePublicSignals.ts.rsa.ts(RSA-2048 PKCS#1 v1.5 sign/verify).trees.tsadditions for SelfRICA SMT building and leaf hashing (getNameDobLeafSelfrica,getNameYobLeafSelfrica).tests/disclose/vc_and_disclose_selfrica.test.ts(OFAC/non-OFAC, signatures, selectors).tests/utils/selfrica/date/*circom + TS tests.tests/utils/selfrica/ofac/*circom + TS tests with SMT fixtures.vc_and_disclose_aadhaar.test.ts.Written by Cursor Bugbot for commit 968a74c. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores