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.

Sign up for GitHub

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

Jump to bottom

tests/refactor: state update and improved JSON parsing #11

Merged
merged 24 commits into from
Aug 15, 2024
Merged

tests/refactor: state update and improved JSON parsing #11

merged 24 commits into from
Aug 15, 2024

Conversation

Autoparallel
Copy link
Contributor

@Autoparallel Autoparallel commented Aug 9, 2024 •
edited
Loading

A lot went on here. I refactored the StateUpdate quite heavily and now take full advantage of the 2d array of instructions (given some ascii) by piping them through a mask that depends on the current state of the parser. This allowed me to reduce constraints while gaining better parsing capabilities.

I added a bunch of tests and more test JSON files. Currently able to properly parse:

  • test.json
  • test_two_key.json
  • test_depth.json
  • sambhav_example.json (lol)

Next steps are:

  • Allow inside_value to be trigged by brackets and numbers. This can likely be combined into a single instruction with a wider match case, so I may need to rework the switch/match to do so.
  • Allow for ansi escape inside string-based values

Closes #5

@Autoparallel
add: reddit_response.json
d2be624
@Autoparallel
refactor tests + add failing case
6d4b7c3
@Autoparallel
easier fix
afcacda
@Autoparallel
test: parse to key
99e0e5c
@Autoparallel
tests: key parsing
3a20a47
@Autoparallel
bug: next_end_of_kv on read :
98a3b51
@Autoparallel
fix: end_of_kv bug
44bcc18
@Autoparallel
test: find value
fb0fb34
@Autoparallel
tests: inside_value and inside_value_to_exit
0f82ab0
@Autoparallel
test: parse to NEXT key
0b166ea
@Autoparallel
parses JSON with two string keys
fc81c78
@Autoparallel
WIP: value inside value
f1ebb51
@Autoparallel
comment
07fd671
@Autoparallel
refactor (#10)
a62dd35 
* wip: start with bitmask

* WIP: time to start testing

* tests: `ArrayAdd` and `ArrayMul`

* tests passing

* update comments
@Autoparallel
feat: 2 key depth 1 json
63a00d5
@Autoparallel
2 kv json and all tests passing
9528248
@Autoparallel
nested json works!!!
86bb82d
@Autoparallel
reduce constraints
4142ab0
@Autoparallel
cleanup
1d425e5
@Autoparallel
rename variables
ec6ec0e
@Autoparallel
more cleaning
ba8888f
@Autoparallel
more cleanup
887786a
@Autoparallel
make comments clean
621e130
0xJepsen
0xJepsen approved these changes Aug 9, 2024
View reviewed changes
Copy link

@0xJepsen 0xJepsen left a comment

Choose a reason for hiding this comment

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

very horny sir

@0xJepsen
Copy link

0xJepsen commented Aug 9, 2024

Can you explain to me how you are using the mask?

devloper
devloper reviewed Aug 10, 2024
View reviewed changes
Copy link

@devloper devloper left a comment

Choose a reason for hiding this comment

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

Some comments on potential areas of underconstraint

circuits/parser.circom Show resolved Hide resolved
circuits/parser.circom Show resolved Hide resolved
circuits/parser.circom Show resolved Hide resolved
circuits/test/operators.test.ts Show resolved Hide resolved
lonerapier
lonerapier approved these changes Aug 12, 2024
View reviewed changes
Copy link
Collaborator

@lonerapier lonerapier left a comment

Choose a reason for hiding this comment

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

Michelin star cooking 🔥

@Autoparallel
Copy link
Contributor Author

I'm resolving comments that won't carry over to the stack machine version so i can clean this up

This was referenced Aug 15, 2024
Open
Open
Closed
@Autoparallel
Copy link
Contributor Author

@devloper stellar review on this.

I turned your relevant comments into issues. I'm going to merge this in to clean up the pipeline a bit to stage for the stack machine impl

@Autoparallel Autoparallel merged commit ed2c440 into main Aug 15, 2024
1 check passed
@Autoparallel Autoparallel deleted the tests/state-update branch August 15, 2024 22:32
Autoparallel added a commit that referenced this pull request Aug 15, 2024
@Autoparallel
Squashed commit of the following:
cd46c62 
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
Autoparallel added a commit that referenced this pull request Aug 15, 2024
@Autoparallel
Squashed commit of the following:
e84b196 
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

clean up merge commit
Autoparallel added a commit that referenced this pull request Aug 20, 2024
@Autoparallel
Squashed commit of the following:
2e229c7 
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
lonerapier added a commit that referenced this pull request Aug 28, 2024
@lonerapier @Autoparallel
feat: value extractor (#37)
6a48ff5 
* 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

* feat: update rust

* feat: parse with array as value

* feat; `InRange` template

* WIP: number parsing

* good stopping point

* compiling again

* stack hard

* save progress

* save wip

* getting through tests

* add optimised
substring search

* resolve random input bug using poseidon hash

* import circomlib and remove circuits

* add array and hash circuits

* add search

* add js poseidon

* update tests

* add search tests

* wip: stack

* big ints killing me

* use strings for tests

* reduce and fix some tests!

* refactor tests

* reduce and fix some tests!

* moar tests

* cosmetics

* continuing with tests

* save state

* renumbered stack entries

* wip: handle end of arr/obj

* maybe progress

* save

* almost!

* tests passing

* cleaning

* wip: big example

* bug: fails to parse example.json

* add basic switch template again

* feat: `GetTopOfStack`

Gotta work with this to make life easier. We can make easier conditionals for this function and perhaps not even need to store a pointer and just store the stack!

Other notes:
The issue is that parsing commas in arrays will return back a -3 value to overwrite a stack position, but this will fail when inside of an array. So, if we know that the top of stack is a 2 (for inside of an array) we can make conditionals on this. We could return even something like -4 for the comma so we have to transform it depending on what the top of stack is

* IT"S LIVING

* refactor: use `circomlib` directly

* refactor: JSONs

* redo all in `circuits.json`

* changeable stack height

* save state

* reorganize tests

* small cleanup

* continuing test cleanup

* remove antiquated tests

* cleanup

* refactor: `language.circom`

* todo note

* good save state

* good state!

* Update notes.md

* 2d stack

* basic array tracking

* tests passing

but still not properly indexing in arrays

* small cleanup

* almost there!

* arrays!

* satisfying!

* another example

* Update notes.md

* save notes

* 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

clean up merge commit

* WIP: remove `pointer` state

* WIP: cleaning further

Many stack tests passing. I want to now not push to stack when we read a colon and instead enable use of the second stack position. This will break more things temporarily.

* WIP: reduced many variables

Lot's of complexity cleared away and many important tests are passing again. Still WIP, but getting close.

* all tests pass

* updated circuits

* formatting

* add zkemail

* extracting string yayyy!!!!!

* add tests for string extractor

* parse numbers

* convert unicode number array to number

* extracting array index

* yay!! multi-depth key extraction done. onto more nested structures noww

* extract nested array working

* add nested structure example circuit

* delete utils

* move input generation to separate binary

* initial codegen

* fix: correct hardcoded signals

* add codegen example

* remove old code and rename to interpreter

* codegen changes

* add test files

* feat(codegen): add output filename

* complete tests

* chore(tests): remove duplicate

* chore(docs): interpreter

* tests: add interpreter

* fix(test): add tests failing in parallel

* fix(codegen): add poseidon hasher

* Merge branch 'main' into temp-fetcher

* refactor: code reorganisation

* experiment with mocha parallel tests

* review nits

* handle conflicting files

---------

Co-authored-by: Colin Roberts <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@devloper devloper devloper left review comments

@0xJepsen 0xJepsen 0xJepsen approved these changes

@lonerapier lonerapier lonerapier approved these changes

@thor314 thor314 thor314 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tests(disclosure): valid state transitions for StateUpdate
5 participants
@Autoparallel @0xJepsen @devloper @thor314 @lonerapier