Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

[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
Labels
T-feature Type: new features
Milestone

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
@miha-stopar miha-stopar mentioned this issue Feb 23, 2024
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
### 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`.
github-merge-queue bot pushed a commit that referenced this issue Mar 7, 2024
### 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
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.