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

Update circuits.js tests and run in CI #167

Merged
merged 11 commits into from
Apr 4, 2023
Merged

Conversation

spalladino
Copy link
Collaborator

Fixes some circuits tests, skips others, and adds them to CI

@spalladino spalladino requested review from spypsy and benesjan April 4, 2023 14:23
@spypsy
Copy link
Member

spypsy commented Apr 4, 2023

Have some similar changes + fixes in #172 maybe we should merge into here?

@spalladino
Copy link
Collaborator Author

If you think this one is good, I'd merge to master first, and then get yours on top of it on master.

Copy link
Contributor

@benesjan benesjan left a comment

Choose a reason for hiding this comment

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

Other than the formatting, looks good to me.

@@ -103,7 +103,7 @@ jobs:
name: "Build and test"
command: build foundation

aztec-js:
aztec-js:
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be a typo. The space should not be there.

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Prettier should put a line at the end of files so this change seems to be caused by incorrect formatting. We should probably configure prettier so that it throws an error instead of warning so that we keep the formatting coherent across codebase.
image

Copy link
Member

Choose a reason for hiding this comment

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

should probably add yarn formatting as a preCommit step

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm I'm running the formatter in circuits.js locally and it's not complaining, and the CI is not reporting any warnings either (see here).

~/Projects/aztec3-packages/yarn-project/circuits.js (palla/circuits-tests-ci)$ yarn formatting
Checking formatting...
All matched files use Prettier code style!

@spalladino spalladino merged commit ff02583 into master Apr 4, 2023
@spalladino spalladino deleted the palla/circuits-tests-ci branch April 4, 2023 14:43
ludamad pushed a commit that referenced this pull request Apr 14, 2023
* feat: partial logic to check existence of vk in contract tree (missing merkle tree membership calls)

* fix private call data

* perform sibling-path traversal/hashing to get from VKhash/function-leaf all the way to function root and then to contract root

---------

Co-authored-by: dbanks12 <[email protected]>
ludamad pushed a commit that referenced this pull request Apr 17, 2023
* feat: partial logic to check existence of vk in contract tree (missing merkle tree membership calls)

* fix private call data

* perform sibling-path traversal/hashing to get from VKhash/function-leaf all the way to function root and then to contract root

---------

Co-authored-by: dbanks12 <[email protected]>
ludamad pushed a commit that referenced this pull request Apr 17, 2023
* feat: partial logic to check existence of vk in contract tree (missing merkle tree membership calls)

* fix private call data

* perform sibling-path traversal/hashing to get from VKhash/function-leaf all the way to function root and then to contract root

---------

Co-authored-by: dbanks12 <[email protected]>
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