Skip to content
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

merkledb -- verify range proof in fuzz test; fix bound error #1789

Merged
merged 16 commits into from
Aug 11, 2023

Conversation

danlaine
Copy link

@danlaine danlaine commented Aug 1, 2023

Why this should be merged

  1. Adds test coverage. Namely, checks that all proofs generated by a database are valid.
  2. Said test coverage fails without the changes in proof.go.

How this works

Fixes how we handle upper bound in range proof verification. Previously, if a proof contained one key-value pair whose key was the root, we would set largestPath to the empty path (""), causing insertChildrenGreaterThan to be Nothing. Then in addPathInfo we would have insertChildrenGreaterThan == Nothing, meaning we would not add proof node children "on the right", resulting in an invalid root ID. See similar issue https://github.com/ava-labs/avalanchego/pull/1629/files#r1235447981.

How this was tested

Added line in UT.

@danlaine danlaine changed the base branch from master to dev August 1, 2023 21:52
@danlaine danlaine changed the title sync -- verify range proof in fuzz test sync -- verify range proof in fuzz test; fix bound error Aug 2, 2023
@danlaine danlaine marked this pull request as ready for review August 2, 2023 16:24
@danlaine danlaine changed the title sync -- verify range proof in fuzz test; fix bound error merkledb -- verify range proof in fuzz test; fix bound error Aug 2, 2023
@dboehm-avalabs
Copy link
Contributor

I think this has been accomplished by #1793 except the fuzz test changes

Dan Laine added 2 commits August 8, 2023 11:06
@danlaine
Copy link
Author

danlaine commented Aug 8, 2023

I think this has been accomplished by #1793 except the fuzz test changes

Yeah there's no longer any semantic change in this PR, just rewording, changing comments, etc. I'd be down to include these non-test changes, but fine either way.

@danlaine danlaine merged commit b21e9b5 into dev Aug 11, 2023
14 checks passed
@danlaine danlaine deleted the sync-verify-range-proof-in-fuzz-test branch August 11, 2023 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants