-
Notifications
You must be signed in to change notification settings - Fork 33
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
Parameterize asset balance number #29
Conversation
The PR looks good to me @alxkzmn. One thing that I noticed is that moving from vectors to arrays have downgraded the performance in building the merkle sum tree, contrary to what I thought. In particular, building a mst with 24 levels takes 592 s using array, while it took 12s less before. Not a huge deal, but I suggest reverting these changes. Sorry for the wrong heads up 🥲 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks great!
Just a small note - we have to revise the csv_parser
if we are going to process CSV files with multiple asset data in the future. But as of now, we're good to go!
Merginn |
Resolves #18 by parameterizing the number of assets for the MST node and the Poseidon hasher dimensions. Hasher configs and the circuit generic parameters can be generated using the Python script that takes the number of assets as an argument.