-
Notifications
You must be signed in to change notification settings - Fork 858
Fixing problems in MPT found by light client #1610
Conversation
ffdb1a7
to
47ffed1
Compare
…ented by assert_eq in prepare_witness.go
Could you add some unit tests to check that adding/updating/removing is done correctly perhaps? |
You mean to the trie? The geth code is used, so it should be ok I think? |
Yes, I meant for the trie. I thought not all cases might be covered an error was found (hence the PR) but I guess it is not trie based per se then. |
Yeah, the trie should be ok I think. What this PR solves are some bugs related to the conversion of the getProof result to the MPT witness + MPT table fix for a case when the leaf is deleted and there were just two leaves in the branch. |
I'm not sure to understand everything you're doing, but here are a few fixes and questions I have. |
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.
Unable to verify the correctess since I did not work on this part.
Did a very lightweight review in order to merge into main.
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.
Tested with zklight branch and works ok
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!
Description
Witness generator fixes:
CreateObject
called when there is no storage trie for an account.Circuit fix:
When value is set to 0 (which deletes the key) and there are two nodes in the branch, then the node is deleted and the other node moves up one level (replaces the branch). The returned node is then this moved node which doesn't have the value 0 and the lookup into MPT tables fails.
Now the circuit is fixed to ensure the new value in the table is 0.
Besides that, some other problems are fixed which were found by @adria0 when integrating the light client and MPT (these were temporary hacks in MPT for testing purposes that were forgotten to be removed):
SetState
calls removed inprepare_witness.go
calls fromoracle.PrefetchStorage
call uncommented inprepare_witness.go
oracle.PrefetchStorage
call uncommented instate/state_object.go updateTrie
prepare_witness.go
.Type of change
How Has This Been Tested?
No test until now covered this scenario, now
TestNeighbourNodeInHashedBranch
has been added.