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.

Sign up for GitHub

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

Jump to bottom

[MPT - stack trie] fix witness generation #1752

Open
KimiWu123 opened this issue Feb 5, 2024 · 2 comments · Fixed by #1784 · May be fixed by #1789
Open

[MPT - stack trie] fix witness generation #1752

KimiWu123 opened this issue Feb 5, 2024 · 2 comments · Fixed by #1784 · May be fixed by #1789
Assignees
@KimiWu123
Labels
T-feature Type: new features
Milestone
MPT

Comments

@KimiWu123
Copy link
Contributor

Describe the feature you would like

There was an implementation under mpt-witness-generator/trie/stacktrie.go already, but still has some bugs. This issue is mainly to fix it.
See Miha's doc for more details.

Additional context

No response
@KimiWu123 KimiWu123 added the T-feature Type: new features label Feb 5, 2024
@KimiWu123 KimiWu123 added this to the MPT milestone Feb 5, 2024
@KimiWu123 KimiWu123 self-assigned this Feb 5, 2024
@KimiWu123 KimiWu123 mentioned this issue Feb 5, 2024
Open
@miha-stopar miha-stopar mentioned this issue Feb 23, 2024
Merged
4 tasks
@miha-stopar
Copy link
Collaborator

This is partially resolved by #1779, but would be good to add some further tests which would test GetProof more extensively, especially for the hashed trie (like TestGetProof). Also, it would be nice to have some tests that check whether the number of proof elements (for elements with a bigger depth) is as expected and the leaf (the last proof element) is as expected...

github-merge-queue bot pushed a commit that referenced this issue Feb 26, 2024
@miha-stopar
Stacktrie getproof (#1779)
3bbc757 
### Description

- Fix in the stack trie `GetProof` method
- Added description for `TestTransactions`

### Issue Link


#1752

### Type of change

- [x ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update
- [ ] Refactor (no updates to logic)

### How Has This Been Tested?

`TestTransactions` and `TestGetProof` in
`zkevm-circuits/geth-utils/gethutil/mpt/witness/gen_witness_transactions_test.go`.
@KimiWu123 KimiWu123 mentioned this issue Feb 27, 2024
Merged
1 task
KimiWu123 added a commit that referenced this issue Mar 6, 2024
@KimiWu123
Merge branch 'main' into feat/#1752-fix-witness-generation-test
414e228
github-merge-queue bot pushed a commit that referenced this issue Mar 7, 2024
@KimiWu123
Feat/#1752 fix witness generation - adding testing (#1784)
b82ea41 
### Description

closed #1752 

- Adding different numbers of txs to test different trie status (e.g.
one leaf only, one ext. , one branch and one leaf ...etc)
- Fixing `GetProof` (if input is an ext node, it always assign first
child of the root ext. node, `st.children[0]`)
- Fixing `getNodeFromBranchRLP` (if input is an leaf, it could throw an
exception)
- Refactoring `getNodeFromBranchRLP`

### Issue Link

#1752 

### Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
@KimiWu123 KimiWu123 reopened this Mar 8, 2024
@KimiWu123 KimiWu123 linked a pull request Mar 8, 2024 that will close this issue
Open
1 task
@KimiWu123
Copy link
Contributor Author

Update:
#1784 was to add testing for GetProof
#1789 was to export json file for circuit

KimiWu123 added a commit that referenced this issue Apr 19, 2024
@KimiWu123
Merge branch 'extension-into-branch' (only .go files) into feat/#1752-…
a5b0b2e 
…fix-witness-generation-json-export
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@KimiWu123 KimiWu123

Labels
T-feature Type: new features
Projects
zkEVM Community Edition
Status: 👀 In review
Milestone
MPT
2 participants
@miha-stopar @KimiWu123