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

refactor: add @zk-kit/artifacts #788

Merged
merged 5 commits into from
May 20, 2024
Merged

refactor: add @zk-kit/artifacts #788

merged 5 commits into from
May 20, 2024

Conversation

sripwoud
Copy link
Contributor

@sripwoud sripwoud commented May 20, 2024

Description

privacy-scaling-explorations/snark-artifacts#45 and privacy-scaling-explorations/zk-kit#288 moved the functions to download the snark artifacts out of @zk-kit/utils and into a dedicated @zk-kit/artifacts package.
This PR adds and uses this package to download the artifacts in the @semaphore-protocol/proof package.

Other information

New release and bumping of @zk-kit/utils here is not necessary. Because the feature removed from it is now brought by @zk-kit/artifacts and this package is now added as a dep in the only semaphore package that uses it: @semaphore-protocol/proof.
Nonetheless once the new release of @zk-kit/utils is released (see privacy-scaling-explorations/zk-kit#290) , we should bump it in @semaphore-protocol/proof too (because now similar functions could theoretically imported from 2 different packages...): captured in #789

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation NA
  • My changes generate no new warnings
  • I have run yarn format and yarn lint without getting any errors
  • I have added tests that prove my fix is effective or that my feature works NA
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@sripwoud sripwoud marked this pull request as ready for review May 20, 2024 13:08
@sripwoud sripwoud requested a review from a team as a code owner May 20, 2024 13:08
@sripwoud sripwoud self-assigned this May 20, 2024
@sripwoud sripwoud added dependencies 📦 Upgrades or downgrades in dependencies refactoring ♻️ A code change that neither fixes a bug nor adds a feature labels May 20, 2024
@sripwoud sripwoud marked this pull request as draft May 20, 2024 13:11
@sripwoud sripwoud force-pushed the refactor/snark-artifacts branch from 5a7614e to 239a90a Compare May 20, 2024 13:17
@sripwoud
Copy link
Contributor Author

sripwoud commented May 20, 2024

setting back to draft although the checks pass.

there is a remaining bug related to jest picking the browser exports instead of node.

I am not sure why (especially because jest's default testEnvironment is node)

But it causes the circuits and contracts tests to fail locally (and they are skipped in the ci).

@cedoor
Copy link
Member

cedoor commented May 20, 2024

setting back to draft although the checks pass.

there is a remaining bug related to jest picking the browser exports instead of node.

I am not sure why (especially because jest's default testEnvironment is node)

But it causes the circuits and contracts tests to fail locally (and they are skipped in the ci).

This privacy-scaling-explorations/snark-artifacts#57 should fix it. I tested it locally.

@vplasencia
Copy link
Member

@sripwoud
Copy link
Contributor Author

Thanks @cedoor @vplasencia
yes all tests pass locally now

@cedoor cedoor merged commit 7b3621a into main May 20, 2024
5 checks passed
@cedoor cedoor deleted the refactor/snark-artifacts branch May 20, 2024 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies 📦 Upgrades or downgrades in dependencies refactoring ♻️ A code change that neither fixes a bug nor adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants