-
Notifications
You must be signed in to change notification settings - Fork 43
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
Info tree updater tests #1418
base: zkevm
Are you sure you want to change the base?
Info tree updater tests #1418
Conversation
5c0a1b8
to
080bb36
Compare
080bb36
to
9ffc90c
Compare
@afa7789 @hexoscott how is it going with this one? |
I'm gonna return to this PR and answer scott, as soon as I finish my investigation on the unwind. |
@hexoscott I made some updates here, I think it's better now. |
retrievedUpdateGer, err := db.GetL1InfoTreeUpdateByGer(l1infotreeupdate.GER) | ||
assert.NoError(t, err) | ||
assert.NotNil(t, retrievedUpdateGer) | ||
|
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.
I think for checking the handler updating the info tree we should check the new tree info root hash. We could use a network like Bali or integration network 8 where have known L1 info roots already that we know are valid. We can just replicate those updates with mock logs etc. to ensure the tree comes back with the same root as we're expecting. We don't need to go crazy, just a couple of updates would be enough.
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.
an extra test for this case then.
to check with real networks.
instead of changing the current one would be good.
( TODO )
internal_tests.go are related to the the internal functions we can't call in a outside package
_tests.go is the external functions, I wanted to do the unit test for those, that's why I had to do it this way.