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

chore(rln-v2): use lazyImt for gas estimates #39

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

rymnc
Copy link
Contributor

@rymnc rymnc commented Apr 30, 2024

Description

As requested by @alrevuelta, following are gas estimates for insertions (see the test "ValidRegistration").
This contract can be further optimized by aggregating all the mappings into one struct.

Checklist

Ensure you completed all of the steps below before submitting your pull request:

  • Added natspec comments?
  • Ran forge snapshot?
  • Ran pnpm lint?
  • Ran forge test?
  • Ran pnpm verify?

@alrevuelta
Copy link

Interesting. Can see we go from 135760 to 210499. What's the before/after? I mean, what does 135760 gas refer to? Current TWN contract? Or LazyIMT without RLNv2?

Mind elaborating on the differences we would have: RLNv2 vs RLNv2_LAZYIMT? Would like to understand where the gas increase is coming from.

@rymnc
Copy link
Contributor Author

rymnc commented Apr 30, 2024

the before is rln-v2, and after is rln-v2 with lazy imt.

rln-v2 vs rln-v1: rln-v2 uses an additional storage slot and hence costs more than rln-v1

@alrevuelta
Copy link

the before is rln-v2, and after is rln-v2 with lazy imt.

Interesting. And why the gas increase? Its quite significant. I'm comparing it with RLNv1 vs RLNv1_LazyImt that is around 90k vs 130k (44% more) vs 135760 to 210499 (55% more).

@rymnc
Copy link
Contributor Author

rymnc commented Apr 30, 2024

we can compare rln-v2 lazyimt to rln-v1 lazyimt by looking at the snapshot from the other pr: [PASS] test__ValidRegistration(uint256) (runs: 1000, μ: 137904, ~: 137904)
this is also due to the fact that we are using the poseidon hash to generate a rate commitment that is the leaf in the rln-v2 tree (as mentioned in yesterday's research call)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants