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.
Implement 7739 #140
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 7739 #140
Changes from 9 commits
6ad87b2
af19d9d
f8e2234
93c00cd
1e4402c
6fc3163
46c6d39
7d32e1a
1b6c6df
cd6a70e
420a2f9
ac2be66
57d9245
d7a7455
e63d105
3e04315
2050fcb
0be11c3
14b5e41
23f622c
97029f0
c4a0f65
4e8f640
b8a2db8
f0b4bf0
6730adb
17f02c3
756a119
08b9da4
4e5ba50
11be832
a00ac81
973cbdf
5b5fcfa
a9da1f3
587ce1c
3857251
3e62772
d23b4ed
b143ce2
61353fe
a8896af
c147c16
3b3d61c
2a6bcf2
f38d5c4
7de8aed
e32e94a
ce1ccac
807d2fe
e004269
3ccab25
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
why does the spec require sending in the contentsDescr length?
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 think because it assumes you are decoding in calldata, but can double check
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.
Yea the spec actually doesn't say what its used for at all
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.
do we not need to length check the type as well?
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.
Could these length checks be all done inside .decodeContentsDescr() ?
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.
Yes, I have that implemented in #146
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.
Ok I changed this to check both, we don't need to check
contentsDescr
however because that's done indecodeContentsDescr