test(benchmarks): improve bechmarks to have complete performance data

0xarcano commented 2 months ago

Description

Added the missing Sparse Merkle tree, the missing operations:

Added Sparse Merkle Tree to the benchmark suites to complete the available merkle tree implementations in zk-kit.
Added operation to the benchmark suites (proof generation, proof verification, update, delete
Changed the "imt.ts" name to "benchmark-merkle-trees.ts"
Added README.md documentation about the benchmark suite
Added log managment with winston library to handle any error produced during the benchmark withtout break the benchmark execution

Related Issue(s)

Closes #311

Other information

The results folder is present in gitignore then the results are not included in the pull request, but you can use the README.md instructions to generate it.

Checklist

It would be helpful to write a comparison of of all them, answering questions like

[ ] "when not to use"/"when to use"
[ ] "used by?": protocol xyz, app xyz...
[x] include benchmarks: maybe one merkle tree is faster at generating a proof, but slower to update?

0xarcano commented 2 months ago

Dear @sripwoud, I have made the changes and run the recommended yarn commands before commit, but I have received the error: "YN0028: The lockfile would have been modified by this install, which is explicitly forbidden."

Please give me some clues about how to solve it, I used node 20.17.0 and yarn 4.1.0, when I run yarn to install the dependencies the yarn.lock has been changed but I didn't include it to the commit because I thought that is not convenient.

0xarcano commented 2 months ago

Hello, I have already fixed the problems in validation of the PR.

0xarcano commented 2 months ago

I have already took action in all the requested changes, please @sripwoud let me know how it looks

sripwoud commented 2 months ago

@prodasl

YN0028: The lockfile would have been modified by this install, which is explicitly forbidden.

is this solved, or do you still need help with this?

0xarcano commented 2 months ago

@prodasl

YN0028: The lockfile would have been modified by this install, which is explicitly forbidden.

is this solved, or do you still need help with this?

0xarcano commented 2 months ago

@prodasl

YN0028: The lockfile would have been modified by this install, which is explicitly forbidden.

is this solved, or do you still need help with this?

Hi, @sripwoud, thanks for the support. I have some doubts about it yet, it is related to my try to send the changes on yarn.lock, because the first time I did the PR, I haven't sent the changes of yarn.lock, but this time I have a lot of change reported in the yarn lock related to packages versions, the only real change was the winston lib added, then my questions are:

Should I include the changes of yarn.lock or definitely it should not be part of the PR? (when I not included I got the mentioned problem YN0028) If I include the yarn.lock, should I include the package version changes or just the new line for winston lib?

0xarcano commented 2 months ago

HI @sripwoud ,

Finally, I tried sharing the yarn.lock as it is in my local environment, sorry for all the trouble but I have not worked with those files before, all the checks in Github looks like are passed and all the change requests covered.

Please let me know if it is ready. (thanks again)

sripwoud commented 2 months ago

@prodasl

YN0028: The lockfile would have been modified by this install, which is explicitly forbidden.

is this solved, or do you still need help with this?

Hi, @sripwoud, thanks for the support.

I have some doubts about it yet, it is related to my try to send the changes on yarn.lock, because the first time I did the PR, I haven't sent the changes of yarn.lock, but this time I have a lot of change reported in the yarn lock related to packages versions, the only real change was the winston lib added, then my questions are:

Should I include the changes of yarn.lock or definitely it should not be part of the PR? (when I not included I got the mentioned problem YN0028)

If I include the yarn.lock, should I include the package version changes or just the new line for winston lib?

TLDR: you should commit any changes to yarn.lock every time you change deps in package.json

If there are more changes in yarn.lock than strictly related to your PR it is okay and normal: it is due to yarn bumping deps automatically according to semantic versioning rules.

In practice devs exceptionally review changes in yarn.lock because there are always so huge. Reviewing package.json changes is enough.

But avoid modifying yarn.lock yourself. Stick to the yarn add/remove commands.

If it happens that there a mismatch between the committed yarn.lock and package.json then you get the error:

YN0028: The lockfile would have been modified by this install, which is explicitly forbidden.

privacy-scaling-explorations / zk-kit