semaphore-protocol / semaphore

A zero-knowledge protocol for anonymous interactions.
https://semaphore.pse.dev
MIT License
898 stars 194 forks source link

refactor: add `@zk-kit/artifacts` #788

Closed sripwoud closed 4 months ago

sripwoud commented 4 months ago

Description

https://github.com/privacy-scaling-explorations/snark-artifacts/pull/45 and https://github.com/privacy-scaling-explorations/zk-kit/pull/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 https://github.com/privacy-scaling-explorations/zk-kit/issues/290) , we should bump it in @semaphore-protocol/proof too (because now similar functions could theoretically imported from 2 different packages...): captured in https://github.com/semaphore-protocol/semaphore/issues/789

Checklist

sripwoud commented 4 months ago

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 commented 4 months ago

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 https://github.com/privacy-scaling-explorations/snark-artifacts/pull/57 should fix it. I tested it locally.

vplasencia commented 4 months ago

Yes, that new config works. Then I think this: https://github.com/semaphore-protocol/semaphore/pull/788/files#diff-860bd1f15d1e0bafcfc6f62560524f588e6d6bf56d4ab1b0f6f8146461558730R16

can be removed.

sripwoud commented 4 months ago

Thanks @cedoor @vplasencia yes all tests pass locally now