-
Notifications
You must be signed in to change notification settings - Fork 71
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
fix: prevent out of bounds on lookup inners access #365
fix: prevent out of bounds on lookup inners access #365
Conversation
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.
General question on if it makes sense to just skip if we are out of bounds or return some error
…out-of-bounds-lookup-inners
also added a unit test for decompressExist to cover new error handling
if exist == nil { | ||
return nil | ||
return nil, nil |
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.
tests broke with an error return here. I don't have a great understanding of when this case arises? Is there a comment we can add to elaborate on it? @AdityaSripal
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.
If a nonexistence proof is proving the leftmost key wouldn't the leftExist be nil? So decompressExist returns nil and we don't error out on line 149
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.
ah yes, thank you! That makes sense to me, added a comment
Is this pr still needed if we're removing batch proofs? |
Nope, changes are no longer relevant! |
When decompressing a compressed proof. If the compressed proof has a path value (index) which is out of bounds of the inner op lookup list, return an error (previously would panic)