plume-sig / zk-nullifier-sig

Implementation of PLUME: nullifier friendly signature scheme on ECDSA
MIT License
128 stars 22 forks source link

chore: clean up #74

Closed 0xmad closed 7 months ago

0xmad commented 8 months ago
Divide-By-0 commented 7 months ago

This looks great! Until @skaunov reviews it, do you want to send your wallet address and I can send a bounty?

0xmad commented 7 months ago

@Divide-By-0 sorry, but I work at PSE :)

Divide-By-0 commented 7 months ago

@Divide-By-0 sorry, but I work at PSE :)

Oh you know what, that makes a lot of sense LOL

0xmad commented 7 months ago

@skaunov thanks for the review. I added pnpm just because it's faster and it's easier to use it as monorepo tool (I'll add workspaces later to get rid of relative paths inside circuits/circom).

skaunov commented 7 months ago

I just noticed that @Divide-By-0 asked actually to also merge this one. %)

skaunov commented 7 months ago

Just noticed that we have path with repetition now: <circuits/circom/test/circuits>. I personally try to avoid this due to minor discomfort it brings, so want to attract your attention to this. Though if everybody fine with it, then...

0xmad commented 7 months ago

@skaunov what's the problem with it? It's not going to be exported and it uses internally inside test folder.

skaunov commented 7 months ago

Path at https://github.com/plume-sig/zk-nullifier-sig/blob/79c602670b8448c9a85d552ba8a8d8aa5bbede04/README.md?plain=1#L44 should also corrected, right? @0xmad

skaunov commented 7 months ago

I also received when was quickly running the instructions.

devbox@pop-os:~/zk-nullifier-sig/circuits/circom$ pnpm run flatten-deps && pnpm run test
 ERR_PNPM_NO_SCRIPT  Missing script: flatten-deps

Command "flatten-deps" not found.
0xmad commented 7 months ago

circuits/ path yes, should be corrected. You don't need to run flatten-deps with test. It's automatically run after pnpm install.