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

feat: upgrade zktrie to v0.6.0 #403

Merged
merged 19 commits into from
Aug 2, 2023
Merged

feat: upgrade zktrie to v0.6.0 #403

merged 19 commits into from
Aug 2, 2023

Conversation

noel2004
Copy link
Member

@noel2004 noel2004 commented Jul 20, 2023

This PR upgrade zktrie to v0.6.0

It should be tested against trace-dumper

  • without any panic in running
  • the storage proof contains new data structure (the new node type) and applied new hash scheme
  • without any error in witness generator
    - [ ] update zktrie to v0.6.1 (the endianness of zktrie key in storage layer would be reversed)

Should not be merged until the depended zktrie has been stable

@noel2004 noel2004 changed the title Upgrade zktrie to v0.6 [DO NOT MERGE] Upgrade zktrie to v0.6 Jul 20, 2023
@noel2004 noel2004 marked this pull request as ready for review July 20, 2023 07:02
@noel2004
Copy link
Member Author

Some unittests which hard-coded a expected state hash are pending for update

@icemelon icemelon requested a review from vyzo July 25, 2023 05:37
Copy link

@vyzo vyzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a little concerned with all the panics, can't we just return errors?

@noel2004 noel2004 changed the base branch from poseidon_domain_spec to develop July 28, 2023 02:44
@noel2004 noel2004 changed the title [DO NOT MERGE] Upgrade zktrie to v0.6 Upgrade zktrie to v0.6 Jul 28, 2023
@noel2004 noel2004 changed the title Upgrade zktrie to v0.6 [DO NOT MERGE] Upgrade zktrie to v0.6 Jul 28, 2023
@noel2004 noel2004 changed the title [DO NOT MERGE] Upgrade zktrie to v0.6 [DO NOT MERGE] Upgrade zktrie to v0.6.1 Jul 28, 2023
@lispc
Copy link

lispc commented Jul 28, 2023

the new poseidon hash code is frozen. Can @mask-pp help to generate some traces in traces repo (not all traces need like bridge if it is too complex, several traces like swap transfer sushi will be ok) so we can have more tests?

@noel2004 noel2004 changed the title [DO NOT MERGE] Upgrade zktrie to v0.6.1 Upgrade zktrie to v0.6.0 Jul 28, 2023
@noel2004
Copy link
Member Author

Everything for new hash scheme is done. The endianess issue would be resolved in l2geth, not zktrie module, with another PR

@Thegaram
Copy link

I am a little concerned with all the panics, can't we just return errors?

Agreed, but we can leave this as a refactoring step after the release.

Thegaram
Thegaram previously approved these changes Jul 31, 2023
crypto/poseidon/poseidon.go Show resolved Hide resolved
params/config.go Outdated Show resolved Hide resolved
trie/zk_trie.go Show resolved Hide resolved
Thegaram
Thegaram previously approved these changes Aug 1, 2023
@0xmountaintop 0xmountaintop changed the title Upgrade zktrie to v0.6.0 feat: upgrade zktrie to v0.6.0 Aug 2, 2023
@0xmountaintop 0xmountaintop merged commit 11b35cd into develop Aug 2, 2023
5 checks passed
@0xmountaintop 0xmountaintop deleted the update/zktrie branch August 2, 2023 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants