-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix createSlice for reference type #4
Conversation
The problem was found when trying to serialize field `v []Uint256`, where Uint256 is a struct with SSZ interface.
@KlonD90 could you please check this changes in JS? So we can be sure that it won't break anything. |
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.
Anyway let's wait @KlonD90's decision about compatibility with JS library
@@ -340,7 +340,7 @@ func (v *Value) createSlice(useNumVariable bool) string { | |||
// []int uses the Extend functions in the fastssz package | |||
return fmt.Sprintf("::.%s = ssz.Extend%s(::.%s, %s)", v.name, uintVToName(v.e), v.name, size) | |||
|
|||
case TypeContainer: | |||
case TypeContainer, TypeReference: |
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.
I'd add some tests to current test suite (it will be checked inside our project but anyway...)
Also seems we need to fix error messages for this case...
Lines 303 to 309 in 93b9cf0
num, ok := DivideInt(a, b) | |
if !ok { | |
return 0, fmt.Errorf("xx") | |
} | |
if num > max { | |
return 0, fmt.Errorf("yy") | |
} |
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.
What is the error message?
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.
Seems in case if user passes malformed input or adds more elements than limit allows it will get "xx" or "yy" as an error message.
It's not related to your patch directly, it's just interesting moment.
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.
I see, let's do it later.
About tests, I tried to find a place in fastssz where to make them, but no luck. Let's push it as is and see what we'll need.
IDK how it would effect. Do you have an example of encoding generated? If it's plain data and also hash level plain too it wouldn't affect |
I think that you could merge it. We don't have a []Uint256 in external message. It shouldn't break anything except check that data field could be read another way? |
The problem was found when trying to serialize field
v []Uint256
, where Uint256 is a struct with SSZ interface.