ci: nightly proving key generation workflow #804
Conversation
baumstern
force-pushed
the
pbuild
branch
4 times, most recently
from
86790ce to
52d25ac
Compare
|
@baumstern I assume this is aimed to solve #803 ? Moving forward, please provide PR descriptions & reference the relevant issue |
|
FYI @baumstern looks like this PR includes a bunch of miscellaneous commits. Why? Perhaps you didn't branch of the latest |
| - name: Use Node.js 16 | ||
| uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: 16 |
There was a problem hiding this comment.
I believe we should use Node 18 now, correct? See #775
There was a problem hiding this comment.
Yes for unit tests. But e2e test has problem with Node 18. See #775 (comment)
baumstern
force-pushed
the
pbuild
branch
3 times, most recently
from
9ad667e to
d4b2703
Compare
baumstern
force-pushed
the
pbuild
branch
2 times, most recently
from
19b0dd4 to
8c28133
Compare
- Test generated proving key for correctness
- Run tests for each sub-package:
- maci-circuits
- maci-contracts
- maci-core
- maci-crypto
- maci-domainobjs
The reason for this is that the PR was created from the previous default branch, which was the |
baumstern
changed the title
| @@ -2,28 +2,26 @@ name: Circuit | |||
|
|
|||
| on: | |||
| push: | |||
| branches: [ master, audit, dev ] | |||
| branches: [ master, dev ] | |||
.github/workflows/nightly.yml
Outdated
|
|
||
|
|
||
| jobs: | ||
| generate-proving-key: |
There was a problem hiding this comment.
small nit: would call it "generate-proving-keys" as it's > 1 key
| libgmp-dev \ | ||
| libsodium-dev \ | ||
| nasm \ | ||
| nlohmann-json3-dev |
There was a problem hiding this comment.
nlohmann-json3-dev is not needed anymore
There was a problem hiding this comment.
It appears that removing nlohmann-json3-dev leads to a compilation failure. Here's the error we encounter:
main.cpp:9:10: fatal error: nlohmann/json.hpp: No such file or directory
9 | #include <nlohmann/json.hpp>
| ^~~~~~~~~~~~~~~~~~~
compilation terminated.
There was a problem hiding this comment.
mm it'd be nice to do some checks on a fresh VM. Looks like rapidnsark does not need this, but zkey-manager does, however where does it use it? is it here: https://github.com/privacy-scaling-explorations/zkey-manager/blob/master/ts/compile.ts#L102?
There was a problem hiding this comment.
It's needed when computing witness with c++: https://docs.circom.io/getting-started/computing-the-witness/
.github/workflows/nightly.yml
Outdated
|
|
||
| # To prevent `npm install` failure of circuit package, | ||
| # it has to checkout manually because version of `circomlib` has pinned to a specific commit | ||
| - name: Checkout circomlib |
There was a problem hiding this comment.
how come this is not something we do while working locally but it's needed here?
There was a problem hiding this comment.
The step to manually checkout circomlib was introduced in April 2022, as part of a pull request (see PR #403). The reason for this step was due to a specific requirement at the time. maci-circuits imports circomlib and pins it to a specific commit (as indicated here in the package.json). Back then, the direct pulling of this dependency with a pinned commit like https://github.com/weijiekoh/circomlib#ac85e82c1914d47789e2032fb11ceb2cfdd38a2b failed.
This issue could have been related to the behavior of npm.js or changes in GitHub's security policy, as outlined in this GitHub blog post or specific to the GitHub Actions Runner environment.
However, I've re-triggered the CI run without this manual checkout step and it worked smoothly, indicating that this step may no longer be necessary. It seems that the underlying issue has been resolved, either through updates to npm.js or GitHub's handling of dependencies.
.github/workflows/nightly.yml
Outdated
|
|
||
| # # To prevent `npm install` failure of circuit package, | ||
| # # it has to checkout manually because version of `circomlib` has pinned to a specific commit | ||
| # - name: Checkout circomlib |
There was a problem hiding this comment.
Why is this now commented out?
If not needed, can we delete this?
This PR introduces a nightly proving key generation workflow. It will help us maintain the ability to generate the proving key and ensure that it remains functional even if no new changes are made to the circuits. It provides an additional layer of assurance for the codebase and allows others to build their own proving keys without any issues.