-
Notifications
You must be signed in to change notification settings - Fork 31
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
Various improvements to VIDPF #1177
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cjpatton
commented
Dec 27, 2024
cjpatton
commented
Dec 27, 2024
cjpatton
force-pushed
the
cjpatton/947-working
branch
from
December 28, 2024 20:11
2a2e9e8
to
4e90f46
Compare
divergentdave
approved these changes
Jan 2, 2025
The "cs" field is merely a correction word for the node proof.
Clean up comments a bit and move a couple of things out of the public API.
This simplifies the API a bit and improves parity with the idpf module.
Create an iterator over a `VidpfInput` that consolidates some error handling common to VIDPF generation and evaluation. This allows us to reduce the amount of error handling necessary and improve control flow in a few places.
We're currently computing the onehot proof via the following tree traversal: for each prefix, visit each node along the path from the root to the node labeled by the prefix. The Mastic spec calls for a breadth-first traversal of the sub-tree composed of the nodes of the prefixes. This commit makes three improvements towards aligning Mastic with the spec. First, de-couple the onehot proof computation from `eval_next()` completely. At the moment, the "latest onehot proof" is stored in the evaluation state, a value that is dependent on the tree traversal. This forces us to handle Mastic-specific details in VIDPF. Instead we store the node proof in the evaluation state, which is independent of the traversal. Second, modify `eval()` and `eval_cache()` to return a onehot proof for the single prefix they traverse. Third, consolidate the `eval_root()` APIs into a single function.
Rename `VidpfEvalCache` to `VidpfEvalResult` and have `eval_enxt()` output it.
This is needed in order to properly compute the path and payload checks.
cjpatton
force-pushed
the
cjpatton/947-working
branch
from
January 3, 2025 15:12
4e90f46
to
9ea8c7b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Partially addresses #947.
I'm working on completing Mastic. This first PR cleans up several aspects of VIDPF that should make it easier to bring it up to spec.
It's best to review this PR commit-by-commit.