feat: stack machine #18

Autoparallel · 2024-08-13T00:09:55Z

Stack machine JSON parser

This pull request is essentially complete barring feedback from you all. I'm aware this could use more documentation but the code is shorter and cleaner now.

To Review

For reviewing (other than just reading the code), I suggest going through the tests specifically for the parser and seeing what's going on there. The main idea is how the stack functions.

Stack

The stack tracks the history of how we have traversed a JSON file. For instance, we start a JSON by reading a { and when we see this token, we push [1,0] onto the stack. This tells us we are now inside an object. The parser can also be in a state of parsing_string or parsing_number. Why? For example, we want to know when we are in one of these values. Furthermore, if we are parsing a string, then tokens like { should not cause stack push (there is a test inside of parser/values.test.ts that shows this with a ,).

Here's the summary of the rules:

When not parsing a string

{ : pushes [1,0] to the stack in the next unallocated position
} : removes [1,0] from the stack if the value there is [1,0] or [1,1] (TODO: This should be properly tested.)
: : updates top from [1,0] to [1,1] to signify we are now in the "value territory"
[ : pushes [2,0] to the stack as like the brace
] : pops [2,n] off the stack if the value there is indeed [2,n] (TODO: Also needs testing)
, : more nuanced:
- If [1,x] was on top of stack, this resets the x (I believe this is tested)
- If [2,n] is on top of stack, this rewrites to [2,n+1] indicating we've traversed through another element of an array
0, ..., 9 : enables parsing_number which can then be disabled by any non-numeric character. (TODO: except for the nuance of feat(json): handle additional numeric characters such as e and . #31.)

Else

" : toggles parsing_string (TODO: escapes like "this is a quote: \"I am hungry\"." are not supported yet. See feat(json): handle escape characters \ within strings #28).

To Test

npx mocha will run all tests. You can npx mocha -g State to see ones for the StateUpdate function, for example.
If you check out circuits.json you will see a list of a bunch of circuits. There are corresponding raw JSON files for these in json_examples/test/. To work with this (work from root dir):
- run cargo install --path . which installs the witness binary.
- With this binary, you can:

witness -j json_examples/test/value_array.json -o inputs/value_array -f input.json

and this will place input.json in inputs/value_array/ which circomkit will see when you run:

npx circomkit compile value_array
npx witness value_array input

Feel free to do these for as many files as you want so you can see what the machine looks like. You can also see how we can choose MAX_STACK_HEIGHT to be as small as possible for a given JSON format we expect from a server.

Final words

Please help me make issues for any weird edge cases and things you can think of here. It's not hard to modify at this point, and in writing this up, I discovered a few things that need testing. You could also be a champ and attempt to write a test for those such cases written here or that you think of!

* wip: start with bitmask * WIP: time to start testing * tests: `ArrayAdd` and `ArrayMul` * tests passing * update comments

Autoparallel · 2024-08-16T22:32:49Z

Tagged relevant issues.

0xJepsen

really cool stuff, great cooking here.

mattes · 2024-08-20T15:39:27Z

epic!

commit 9b6f694 Author: Colin Roberts <[email protected]> Date: Thu Aug 15 16:36:19 2024 -0600 feat: upgrade input JSON creation (#14) * add: `reddit_response.json` * refactor tests + add failing case * easier fix * test: parse to key * tests: key parsing * bug: `next_end_of_kv` on read `:` * fix: `end_of_kv` bug * test: find value * tests: `inside_value` and `inside_value_to_exit` * test: parse to NEXT key * parses JSON with two string keys * WIP: value inside value * comment * refactor (#10) * wip: start with bitmask * WIP: time to start testing * tests: `ArrayAdd` and `ArrayMul` * tests passing * update comments * feat: 2 key depth 1 json * 2 kv json and all tests passing * nested json works!!! * reduce constraints * cleanup * rename variables * more cleaning * more cleanup * make comments clean * WAYLON NITPICKING ME LOL * feat: improved CLI for witness * gitignore input.json * Update main.rs * Squashed commit of the following: commit ed2c440 Author: Colin Roberts <[email protected]> Date: Thu Aug 15 16:32:44 2024 -0600 tests/refactor: state update and improved JSON parsing (#11) * add: `reddit_response.json` * refactor tests + add failing case * easier fix * test: parse to key * tests: key parsing * bug: `next_end_of_kv` on read `:` * fix: `end_of_kv` bug * test: find value * tests: `inside_value` and `inside_value_to_exit` * test: parse to NEXT key * parses JSON with two string keys * WIP: value inside value * comment * refactor (#10) * wip: start with bitmask * WIP: time to start testing * tests: `ArrayAdd` and `ArrayMul` * tests passing * update comments * feat: 2 key depth 1 json * 2 kv json and all tests passing * nested json works!!! * reduce constraints * cleanup * rename variables * more cleaning * more cleanup * make comments clean * WAYLON NITPICKING ME LOL Merge branch 'main' into feat/stack-machine

devloper

Very nice work! No blocking comments for me. I added some inline thoughts to stoke future discussion and cleanups

devloper · 2024-08-21T15:43:24Z

circuits/utils.circom

+        matchChecker.array[i] <== 1 - indicator[i].out;
+        for(var j = 0; j < n; j++) {
+            component_out[i][j] <== indicator[i].out * vals[i][j];
+            sum[j] += component_out[i][j];


Slight update to my last review, vars do "accumulate" the sub expressions and then the final constraint will check all of those sub expressions, so I'm now more convinced this sum is actually correct.

Yeah, i think this is correct because you can still get warnings about non-quadratic constraints through vars!

circuits/parser.circom

devloper · 2024-08-21T21:58:14Z

circuits/parser.circom

+    component isPush       = IsEqual();
+    isPush.in            <== [readStartBrace + readStartBracket, 1];
+    component isPop        = IsEqual();
+    isPop.in             <== [readEndBrace + readEndBracket, 1];


nit: replace readEndBrace + readEndBracket with readEndChar (does this work?)

Oh... yeah I think we can do that.

circuits/parser.circom

thor314 · 2024-08-21T22:58:13Z

doc comments in modules would help readibility
improvement of reader clarity in readme also
make sure reader knows how to run logging examples

super cracked PR, good work

Autoparallel · 2024-08-22T15:26:49Z

doc comments in modules would help readibility

improvement of reader clarity in readme also

make sure reader knows how to run logging examples

super cracked PR, good work

Will add and take @devloper's comments into account too.

Then will merge.

Decreased `StateUpdate` from 332 constraints to 311

Cut down to 252 constraints now for StateUpdate

Autoparallel added 30 commits August 7, 2024 16:11

add: reddit_response.json

d2be624

refactor tests + add failing case

6d4b7c3

easier fix

afcacda

test: parse to key

99e0e5c

tests: key parsing

3a20a47

bug: next_end_of_kv on read :

98a3b51

fix: end_of_kv bug

44bcc18

test: find value

fb0fb34

tests: inside_value and inside_value_to_exit

0f82ab0

test: parse to NEXT key

0b166ea

parses JSON with two string keys

fc81c78

WIP: value inside value

f1ebb51

comment

07fd671

refactor (#10)

a62dd35

* wip: start with bitmask * WIP: time to start testing * tests: `ArrayAdd` and `ArrayMul` * tests passing * update comments

feat: 2 key depth 1 json

63a00d5

2 kv json and all tests passing

9528248

nested json works!!!

86bb82d

reduce constraints

4142ab0

cleanup

1d425e5

rename variables

ec6ec0e

more cleaning

ba8888f

more cleanup

887786a

make comments clean

621e130

WAYLON NITPICKING ME LOL

18c68d8

feat: improved CLI for witness

52fa826

gitignore input.json

331f218

Update main.rs

7eef73e

feat: update rust

ca148b5

feat: parse with array as value

c95b72f

feat; InRange template

5998757

Autoparallel added 4 commits August 15, 2024 19:19

formatting

6d10e1b

cut down on constraints

2cd22ce

save state

019fcd8

lots of cleanup

6cfe96b

Autoparallel requested review from devloper, thor314, lonerapier and 0xJepsen August 16, 2024 22:32

Autoparallel added 2 commits August 16, 2024 16:35

Delete mask.test.ts

65e3a6a

example test

957922a

Autoparallel added enhancement New feature or request json tests Make sure thing work good labels Aug 16, 2024

Autoparallel added this to the JSON Selective Disclosure Template milestone Aug 16, 2024

0xJepsen approved these changes Aug 19, 2024

View reviewed changes

Autoparallel mentioned this pull request Aug 20, 2024

fix: breaking out of parsing_number state #42

Closed

devloper approved these changes Aug 21, 2024

View reviewed changes

devloper assigned Autoparallel Aug 21, 2024

thor314 approved these changes Aug 21, 2024

View reviewed changes

Autoparallel added 4 commits August 22, 2024 13:20

docs + refactor: parser

8a7d334

Decreased `StateUpdate` from 332 constraints to 311

redo instruction

91c3564

Cut down to 252 constraints now for StateUpdate

add documentation

167da25

Update parser.circom

9c71481

Autoparallel merged commit 9ffb44f into main Aug 22, 2024
1 check passed

Autoparallel deleted the feat/stack-machine branch August 22, 2024 21:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: stack machine #18

feat: stack machine #18

Autoparallel commented Aug 13, 2024 •

edited

Loading

Autoparallel commented Aug 16, 2024

0xJepsen left a comment

mattes commented Aug 20, 2024

devloper left a comment

devloper Aug 21, 2024

Autoparallel Aug 22, 2024

devloper Aug 21, 2024

Autoparallel Aug 22, 2024

thor314 commented Aug 21, 2024

Autoparallel commented Aug 22, 2024

feat: stack machine #18

feat: stack machine #18

Conversation

Autoparallel commented Aug 13, 2024 • edited Loading

Stack machine JSON parser

To Review

Stack

When not parsing a string

Else

To Test

Final words

Autoparallel commented Aug 16, 2024

0xJepsen left a comment

Choose a reason for hiding this comment

mattes commented Aug 20, 2024

devloper left a comment

Choose a reason for hiding this comment

devloper Aug 21, 2024

Choose a reason for hiding this comment

Autoparallel Aug 22, 2024

Choose a reason for hiding this comment

devloper Aug 21, 2024

Choose a reason for hiding this comment

Autoparallel Aug 22, 2024

Choose a reason for hiding this comment

thor314 commented Aug 21, 2024

Autoparallel commented Aug 22, 2024

Autoparallel commented Aug 13, 2024 •

edited

Loading