-
Notifications
You must be signed in to change notification settings - Fork 20.5k
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
trie: reject deletions when verifying range proofs #23960
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.
LGTM
// Ensure the received batch is monotonic increasing and contains no deletions | ||
for i := 0; i < len(keys)-1; i++ { | ||
if bytes.Compare(keys[i], keys[i+1]) >= 0 { | ||
return false, errors.New("range is not monotonically increasing") | ||
} | ||
} | ||
for _, value := range values { | ||
if len(value) == 0 { | ||
return false, errors.New("range contains deletion") | ||
} | ||
} |
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.
len(keys) == len(values)
, so you could use the same loop, with a slight modification
// Ensure the received batch is monotonic increasing and contains no deletions | |
for i := 0; i < len(keys)-1; i++ { | |
if bytes.Compare(keys[i], keys[i+1]) >= 0 { | |
return false, errors.New("range is not monotonically increasing") | |
} | |
} | |
for _, value := range values { | |
if len(value) == 0 { | |
return false, errors.New("range contains deletion") | |
} | |
} | |
// Ensure the received batch is monotonic increasing and contains no deletions | |
for i := 0; i < len(keys); i++ { | |
if i > 0 && bytes.Compare(keys[i-1], keys[i]) >= 0 { | |
return false, errors.New("range is not monotonically increasing") | |
} | |
if len(values[i]) == 0 { | |
return false, errors.New("range contains deletion") | |
} | |
} |
Feel free to ignore
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.
👍
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.
It did occur to me, but the off by one and extra clause check seemed wonky. The code is cleaner this way, we're not saving anything by optimizing out an extra addition per array item. Unless you guys feel very strongly, I'd leave it as is.
Reject trie range proofs if they contain deletions, otherwise these would be noop entries that just bloat the response and may potentially cause trouble down the line since a semi-invalid proof passes verification.