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

Revert index range check on SelectRegexReveal #218

Merged
merged 8 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
packages/helpers/src/lib
5 changes: 5 additions & 0 deletions .prettierrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"semi": true,
"tabWidth": 2,
"printWidth": 120
}
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"license": "MIT",
"private": true,
"scripts": {
"lint": "yarn prettier --write packages/**/**.ts",
"test": "jest"
},
"workspaces": [
Expand All @@ -12,5 +13,8 @@
"packageManager": "[email protected]",
"engines": {
"node": ">=14.0.0"
},
"devDependencies": {
"prettier": "^3.3.3"
}
}
2 changes: 2 additions & 0 deletions packages/circuits/tests/byte-mask.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,7 @@ describe("ByteMask Circuit", () => {
} catch (error) {
expect(error).toBeTruthy();
}

expect.assertions(1);
});
});
2 changes: 2 additions & 0 deletions packages/circuits/tests/remove-soft-line-breaks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { wasm as wasm_tester } from 'circom_tester';
import path from 'path';

describe('RemoveSoftLineBreaks', () => {
jest.setTimeout(20_1000);

let circuit: any;

beforeAll(async () => {
Expand Down
54 changes: 11 additions & 43 deletions packages/circuits/tests/select-regex-reveal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,7 @@ describe("Select Regex Reveal", () => {
});

it("should reveal the substring with maximum revealed length", async function () {
let input = [
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0,
];
let input = new Array(34).fill(0);
const startIndex = Math.floor(Math.random() * 24);
const revealed = Array.from("zk email").map((char) =>
char.charCodeAt(0)
Expand All @@ -40,10 +37,7 @@ describe("Select Regex Reveal", () => {
});

it("should reveal the substring with non-maximum revealed length", async function () {
let input = [
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0,
];
let input = new Array(34).fill(0);
const startIndex = 30;
const revealed = Array.from("zk").map((char) => char.charCodeAt(0));
for (let i = 0; i < revealed.length; i++) {
Expand All @@ -60,11 +54,8 @@ describe("Select Regex Reveal", () => {
});

it("should fail when all zero", async function () {
let input = [
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0,
];
const startIndex = Math.floor(Math.random() * 32);
let input = new Array(34).fill(0);
const startIndex = Math.floor(Math.random() * 34);
try {
const witness = await circuit.calculateWitness({
in: input,
Expand All @@ -74,13 +65,12 @@ describe("Select Regex Reveal", () => {
} catch (error) {
expect((error as Error).message).toMatch("Assert Failed");
}

expect.assertions(1);
});

it("should fail when startIndex is 0", async function () {
let input = [
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0,
];
let input = new Array(34).fill(0);
const startIndex = 1 + Math.floor(Math.random() * 24);
const revealed = Array.from("zk email").map((char) =>
char.charCodeAt(0)
Expand All @@ -97,13 +87,12 @@ describe("Select Regex Reveal", () => {
} catch (error) {
expect((error as Error).message).toMatch("Assert Failed");
}

expect.assertions(1);
Copy link
Member Author

Choose a reason for hiding this comment

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

@SoraSuegami Just FYI, we need to add this in negative testing to let jest know we expect an assertion - this ensures the catch block was executed (as our assertion is in catch). Otherwise the test would pass even if the code didn't fail as expected.

});

it("should fail when startIndex is not before 0", async function () {
let input = [
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0,
];
let input = new Array(34).fill(0);
const startIndex = Math.floor(Math.random() * 23);
const revealed = Array.from("zk email").map((char) =>
char.charCodeAt(0)
Expand All @@ -120,28 +109,7 @@ describe("Select Regex Reveal", () => {
} catch (error) {
expect((error as Error).message).toMatch("Assert Failed");
}
});

it("should fail when startIndex is larger than max length", async function () {
Copy link
Member

Choose a reason for hiding this comment

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

Wait why did we remove this test

Copy link
Member Author

Choose a reason for hiding this comment

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

By our pattern we are constraining indices where it originates (main circuits). Adding them to each utils might be redundant as main circuits would be using multiple utils. We are not doing these checks on other circuits as well. This was added recently by another PR but we already have comments saying "assumes valid index"

let input = [
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0,
];
const startIndex = Math.floor(Math.random() * 24);
const revealed = Array.from("zk email").map((char) =>
char.charCodeAt(0)
);
for (let i = 0; i < revealed.length; i++) {
input[startIndex + i] = revealed[i];
}
try {
const witness = await circuit.calculateWitness({
in: input,
startIndex: 32,
});
await circuit.checkConstraints(witness);
} catch (error) {
expect((error as Error).message).toMatch("Assert Failed");
}
expect.assertions(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe("SplitBytesToWords Helper unit test", () => {

beforeAll(async () => {
circuit = await wasm_tester(
path.join(__dirname, "./test-circuits/splitBytesToWords-test.circom"),
path.join(__dirname, "./test-circuits/split-bytes-to-words-test.circom"),
{
recompile: true,
include: path.join(__dirname, "../../../node_modules"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ pragma circom 2.1.6;

include "../../utils/regex.circom";

component main = SelectRegexReveal(32,8);
component main = SelectRegexReveal(34, 8);
4 changes: 0 additions & 4 deletions packages/circuits/utils/regex.circom
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ template SelectRegexReveal(maxArrayLen, maxRevealLen) {
signal isPreviousZero[maxArrayLen];
signal isAboveMaxRevealLen[maxArrayLen];

// Assert startIndex < maxArrayLen
signal isValidStartIndex <== LessThan(bitLength)([startIndex, maxArrayLen]);
isValidStartIndex === 1;

isPreviousZero[0] <== 1;
for(var i = 0; i < maxArrayLen; i++) {
isStartIndex[i] <== IsEqual()([i, startIndex]);
Expand Down
11 changes: 11 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7168,6 +7168,15 @@ __metadata:
languageName: node
linkType: hard

"prettier@npm:^3.3.3":
version: 3.3.3
resolution: "prettier@npm:3.3.3"
bin:
prettier: bin/prettier.cjs
checksum: bc8604354805acfdde6106852d14b045bb20827ad76a5ffc2455b71a8257f94de93f17f14e463fe844808d2ccc87248364a5691488a3304f1031326e62d9276e
languageName: node
linkType: hard

"pretty-format@npm:^29.0.0, pretty-format@npm:^29.7.0":
version: 29.7.0
resolution: "pretty-format@npm:29.7.0"
Expand Down Expand Up @@ -7455,6 +7464,8 @@ __metadata:
"root-workspace-0b6124@workspace:.":
version: 0.0.0-use.local
resolution: "root-workspace-0b6124@workspace:."
dependencies:
prettier: ^3.3.3
languageName: unknown
linkType: soft

Expand Down
Loading