-
Notifications
You must be signed in to change notification settings - Fork 81
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
implement lazy root computation #18
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.
So the only difference is that we extend the API to be able to compute specific row/col roots on demand. Looks like a good change anyway.
return &ByzantineColumnError{j, edsBackup} | ||
} | ||
} else if mode == column { | ||
adjMask := mask.RowView(int(j)) | ||
if vecNumTrue(adjMask) == adjMask.Len()-1 && !bytes.Equal(eds.RowRoots()[j], rowRoots[j]) { | ||
if vecNumTrue(adjMask) == adjMask.Len()-1 && !bytes.Equal(eds.RowRoot(j), rowRoots[j]) { |
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.
Orthogonal to this PR but we should really refactor this. The massive indentation / nesting alone cries for splitting this up into multiple methods...
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.
🟢 👍🏼 🤸🏼♂️ 🙌🏼
this PR is a slight refactor, that adds the ability to compute the root of a single row or column. This functionality is useful repairing the square without feeding the underlying
rsmt2d.Tree
nil values when not needed.While the current root caching mechanism is used, no attempt was made to cache individual row or column roots. This means that if any element of the square changes, then like the current implementation, each root must be recomputed. 🤷♂️
I'm keeping this as a draft for now, as more changes could be needed to finish #235
closes #17