-
Notifications
You must be signed in to change notification settings - Fork 20.1k
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
Add packing for dynamic array and slice types #18051
Changes from 5 commits
ded4e8a
dabca31
fc490af
758feae
e024c16
b65b3d2
9810e35
a5c4c6b
da590e6
0f9afde
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -243,12 +243,9 @@ func (arguments Arguments) Pack(args ...interface{}) ([]byte, error) { | |
// input offset is the bytes offset for packed output | ||
inputOffset := 0 | ||
for _, abiArg := range abiArgs { | ||
if abiArg.Type.T == ArrayTy { | ||
inputOffset += 32 * abiArg.Type.Size | ||
} else { | ||
inputOffset += 32 | ||
} | ||
inputOffset += getOffset(abiArg.Type) | ||
} | ||
|
||
var ret []byte | ||
for i, a := range args { | ||
input := abiArgs[i] | ||
|
@@ -257,14 +254,15 @@ func (arguments Arguments) Pack(args ...interface{}) ([]byte, error) { | |
if err != nil { | ||
return nil, err | ||
} | ||
// check for a slice type (string, bytes, slice) | ||
if input.Type.requiresLengthPrefix() { | ||
// calculate the offset | ||
offset := inputOffset + len(variableInput) | ||
// check for dynamic types)= | ||
vedhavyas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if offsetRequired(input.Type) { | ||
// set the offset | ||
ret = append(ret, packNum(reflect.ValueOf(offset))...) | ||
// Append the packed output to the variable input. The variable input | ||
// will be appended at the end of the input. | ||
ret = append(ret, packNum(reflect.ValueOf(inputOffset))...) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove empty line |
||
// calculate next offset | ||
inputOffset += len(packed) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove empty line |
||
// append to variable input | ||
variableInput = append(variableInput, packed...) | ||
} else { | ||
// append the packed value to the input | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -183,27 +183,69 @@ func (t Type) pack(v reflect.Value) ([]byte, error) { | |
return nil, err | ||
} | ||
|
||
if t.T == SliceTy || t.T == ArrayTy { | ||
var packed []byte | ||
switch t.T { | ||
case SliceTy, ArrayTy: | ||
var ret []byte | ||
|
||
if t.requiresLengthPrefix() { | ||
// append length | ||
ret = append(ret, packNum(reflect.ValueOf(v.Len()))...) | ||
} | ||
|
||
// calculate offset if any | ||
offset := 0 | ||
offsetReq := offsetRequired(*t.Elem) | ||
if offsetReq { | ||
offset = getOffset(*t.Elem) * v.Len() | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove empty line |
||
var tail []byte | ||
for i := 0; i < v.Len(); i++ { | ||
val, err := t.Elem.pack(v.Index(i)) | ||
if err != nil { | ||
return nil, err | ||
} | ||
packed = append(packed, val...) | ||
} | ||
if t.T == SliceTy { | ||
return packBytesSlice(packed, v.Len()), nil | ||
} else if t.T == ArrayTy { | ||
return packed, nil | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove empty line |
||
if !offsetReq { | ||
ret = append(ret, val...) | ||
continue | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove empty line |
||
ret = append(ret, packNum(reflect.ValueOf(offset))...) | ||
offset += len(val) | ||
tail = append(tail, val...) | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove empty line |
||
return append(ret, tail...), nil | ||
default: | ||
return packElement(t, v), nil | ||
} | ||
return packElement(t, v), nil | ||
} | ||
|
||
// requireLengthPrefix returns whether the type requires any sort of length | ||
// prefixing. | ||
func (t Type) requiresLengthPrefix() bool { | ||
return t.T == StringTy || t.T == BytesTy || t.T == SliceTy | ||
} | ||
|
||
// offsetRequired returns true if the type is considered dynamic | ||
func offsetRequired(t Type) bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as comment below: I would like the function name to explicitly reflect what kind of offset that is. |
||
// dynamic types | ||
// array is also a dynamic type if the array type is dynamic | ||
if t.T == StringTy || t.T == BytesTy || t.T == SliceTy || (t.T == ArrayTy && offsetRequired(*t.Elem)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. perhaps simplify
|
||
return true | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove empty line |
||
return false | ||
} | ||
|
||
// getOffset returns the offset to be added for t | ||
func getOffset(t Type) int { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you please give it a more explicit name: what kind of offset that is? And update the comment to be more explicit, too. |
||
// if it is an array and there are no dynamic types | ||
// then the array is static type | ||
if t.T == ArrayTy && !offsetRequired(*t.Elem) { | ||
return 32 * t.Size | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove empty line |
||
return 32 | ||
} |
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.
remove empty line