-
Notifications
You must be signed in to change notification settings - Fork 20.2k
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
core/state: tests on the binary iterator #30754
Conversation
3318909
to
e9fc202
Compare
This PR now contains additional test coverage, and fails as expected two tests concerning deletes. @rjl493456442 want to push your fixes on top here? |
@rjl493456442 I pushed a different fix now. Sorry for the noise :) |
if !it.next() { | ||
return false | ||
} | ||
if len(it.Account()) == 0 && len(it.Slot()) == 0 { |
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.
We should also check the it.Fail.
it.Fail
might be set if any error occurs by calling it.Account()
or it.Slot()
. If so, we should return false and stop iteration.
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.
one issue, otherwise lgtm
We can go with this pull request and I will rebase my stuff on top |
Work in progress
cc @rjl493456442