-
Notifications
You must be signed in to change notification settings - Fork 264
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
Calculate min and max ssz lengths #2358
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.
Looks good 👍 Just make sure that variableFieldCount
is used correctly
TBH overall SSZ code looks not too good :) Another chance to think on SSZ refactoring.
case "boolean": | ||
return BOOLEAN_SIZE; | ||
default: | ||
throw new IllegalArgumentException("Unable to deserialize " + classInfo.getSimpleName()); |
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.
May be change exception wording?
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.
Done.
private static LengthBounds calculateBitlistLength( | ||
final ReflectionInformation reflectionInfo, final int variableFieldCount) { | ||
final LengthBounds fieldLengthBounds; | ||
final long maxSize = reflectionInfo.getBitlistElementMaxSizes().get(variableFieldCount); |
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.
Not sure here: getBitlistElementMaxSizes()
seems to return only Bitlist
instance sizes in the referred class. Why is it indexed by variableFieldCount
which is a total counter of lists and bitfields?
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.
That is highly suspicious but exactly matches what SimpleOffsetSerializer
does. It also works for every type we know how to SSZ serialise, but I strongly suspect that's a coincidence and there just aren't any objects that have both SSZList
and Bitlist
.
Created a test class with both and sure enough it fails with IndexOutOfBoundsException
. So have fixed both LengthBoundsCalculator
and SimpleOffsetSerializer
.
final Class<?> listElementType = reflectionInfo.getListElementTypes().get(variableFieldCount); | ||
final long listElementMaxSize = reflectionInfo.getListElementMaxSizes().get(variableFieldCount); |
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.
The same question as for calculateBitlistLength
PR Description
In preparation for validating rpc message lengths more strictly, calculate the minimum and maximum lengths for SSZ serialised objects. Not currently used anywhere but changes to
SimpleOffsetSerializer
are so complex it's worth reviewing them separately.Fixed Issue(s)
Part of #2077
Documentation
documentation
label to this PR if updates are required.