Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: substring matching #17

Merged
merged 15 commits into from
Aug 23, 2024
Merged

feat: substring matching #17

merged 15 commits into from
Aug 23, 2024

Conversation

lonerapier
Copy link
Collaborator

Adds a substring matching circuit using random linear combination.

  • removes circuits from circomlib and import from circomlib directly using circom's -l flag.
  • asymptotic complexity: $O(m+n)$, where $m,n$ = data,key length respectively
  • Uses an underconstrained function SubstringSearch to compute probable position of key in data. So, a malicious prover can set invalid position.
  • position is verified by a properly constrained SubstringMatchWithIndex function
    • calculates linear combination of key and data at position specified
    • validates that sum should be equal.

Constraint count for dataLength: 787 and keyLength: 10

template instances: 293
non-linear constraints: 54707
linear constraints: 0
public inputs: 0
private inputs: 797
public outputs: 1
wires: 56282
labels: 313180

Most of these constraints are due to Poseidon hash used to calculate a random number for linear combination. r can't be public constant or even private input, because then the verification is prone to failure, and should be properly initiated using a random oracle.

[!NOTE] search circuit somehow doesn't compiles with circomkit (mentioned in README). You might have to compile manually using circom circuits/main/search.circom --r1cs --wasm -l node_modules/ -o build/search/

@devloper
Copy link

Nice! I'll give this a longer review tomorrow!

Copy link
Contributor

@Autoparallel Autoparallel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! I'm just looking to have these comments/questions resolved, then can approve.

Nothing major! Looks good!

circuits.json Show resolved Hide resolved
circuits/extract.circom Outdated Show resolved Hide resolved
circuits/search.circom Show resolved Hide resolved
# Profile
9 * `dataLen` constraints

NOTE: Modified from https://github.com/zkemail/zk-email-verify/tree/main/packages/circuits
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should look through the audit to see if this was hit then: https://www.zksecurity.xyz/reports/zkemail

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's currently not in upstream zkemail code, have a PR open.

circuits/search.circom Show resolved Hide resolved
circuits/test/search.test.ts Show resolved Hide resolved
circuits/test/search.test.ts Show resolved Hide resolved
circuits/utils/array.circom Outdated Show resolved Hide resolved
circuits/utils/hash.circom Outdated Show resolved Hide resolved
circuits/utils/hash.circom Show resolved Hide resolved
@lonerapier
Copy link
Collaborator Author

Thanks for the amazing review @Autoparallel. Will incorporate these changes.

Copy link
Contributor

@Autoparallel Autoparallel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now! Just bring it up to date with main.

If you want multiple files in utils/, I'm happy with that choice :)

@lonerapier
Copy link
Collaborator Author

rebased with main.

In the end, I decided to move utils circuits in separate files :)

@lonerapier lonerapier merged commit 71b4470 into main Aug 23, 2024
1 check passed
@lonerapier lonerapier deleted the feat/substring-matching branch August 23, 2024 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants