Clean up SSZ multiproofs spec: unused function #2715
Closed
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
ralexstokes
force-pushed
the
ralexstokes-patch-1
branch
from
a9c62d8 to
3e4a64f
Compare
|
What are the chances we need this in light client R&D? |
|
Concatenation is an important thing for constructing nested path. E.g. with a container of 5 fields, and access to field index 4, with 7 fields and access to field index 5, you get: |
|
yeah, similar feeling. Although it's not used in the spec, it's good to be defined for others |
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.
The function
concat_generalized_indicesis not currently used anywhere in this repo (and by extension the helperget_power_of_two_floor) and a quick search only yields reference from the past PR: #1323We could use this functionality in the future, but currently don't as we don't need any receipt consumption logic in the consensus protocol as it stands today or in the near term.
Given that this code will be in the git history in the event we do need it, I'd suggest removing it for now so that the spec is "lighter" and that implementers of multiproofs don't bother implementing an unnecessary function (if they do what I just did and just mechanically implement the spec from the markdown file line by line ;) )