Skip to content
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

accounts/abi: include fixed array size in offset for dynamic type #15285

Merged
merged 3 commits into from
Dec 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 182 additions & 0 deletions accounts/abi/abi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,188 @@ func TestInputVariableInputLength(t *testing.T) {
}
}

func TestInputFixedArrayAndVariableInputLength(t *testing.T) {
const definition = `[
{ "type" : "function", "name" : "fixedArrStr", "constant" : true, "inputs" : [ { "name" : "str", "type" : "string" }, { "name" : "fixedArr", "type" : "uint256[2]" } ] },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would make sense to test more functions, accepting a mixed amount of arguments. For example, put the fixed array first, have several arrays (some dynamic, others not) and so on. You never know what kind of change will come after you.

{ "type" : "function", "name" : "fixedArrBytes", "constant" : true, "inputs" : [ { "name" : "str", "type" : "bytes" }, { "name" : "fixedArr", "type" : "uint256[2]" } ] },
{ "type" : "function", "name" : "mixedArrStr", "constant" : true, "inputs" : [ { "name" : "str", "type" : "string" }, { "name" : "fixedArr", "type": "uint256[2]" }, { "name" : "dynArr", "type": "uint256[]" } ] },
{ "type" : "function", "name" : "doubleFixedArrStr", "constant" : true, "inputs" : [ { "name" : "str", "type" : "string" }, { "name" : "fixedArr1", "type": "uint256[2]" }, { "name" : "fixedArr2", "type": "uint256[3]" } ] },
{ "type" : "function", "name" : "multipleMixedArrStr", "constant" : true, "inputs" : [ { "name" : "str", "type" : "string" }, { "name" : "fixedArr1", "type": "uint256[2]" }, { "name" : "dynArr", "type" : "uint256[]" }, { "name" : "fixedArr2", "type" : "uint256[3]" } ] }
]`

abi, err := JSON(strings.NewReader(definition))
if err != nil {
t.Error(err)
}

// test string, fixed array uint256[2]
strin := "hello world"
arrin := [2]*big.Int{big.NewInt(1), big.NewInt(2)}
fixedArrStrPack, err := abi.Pack("fixedArrStr", strin, arrin)
if err != nil {
t.Error(err)
}

// generate expected output
offset := make([]byte, 32)
offset[31] = 96
length := make([]byte, 32)
length[31] = byte(len(strin))
strvalue := common.RightPadBytes([]byte(strin), 32)
arrinvalue1 := common.LeftPadBytes(arrin[0].Bytes(), 32)
arrinvalue2 := common.LeftPadBytes(arrin[1].Bytes(), 32)
exp := append(offset, arrinvalue1...)
exp = append(exp, arrinvalue2...)
exp = append(exp, append(length, strvalue...)...)

// ignore first 4 bytes of the output. This is the function identifier
fixedArrStrPack = fixedArrStrPack[4:]
if !bytes.Equal(fixedArrStrPack, exp) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing the output bytes is useful, but ultimately you should also check that it is possible to unpack what has been packed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that the unpacker interface currently supports unpacking method outputs, but not unpacking method inputs (feel free to correct me if I'm wrong here). Seems like adding support would be a bigger update and not sure if that should be a part of this PR or not?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, did you see my comment here:#15702 (comment) ?

Yes, unpacking method inputs would be a very useful addition. I'm not sure what the ABI-package status is now, haven't kept track of the various (three?) PR:s that are in progress.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it doesn't have to be part of this PR, though. If we merge the current PR:s, I believe that making abi-unpacking more generic (supporting both input and output) can be added afterwards.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me, happy to help out with making abi unpacking more generic in a separate PR if needed as well

t.Errorf("expected %x, got %x\n", exp, fixedArrStrPack)
}

// test byte array, fixed array uint256[2]
bytesin := []byte(strin)
arrin = [2]*big.Int{big.NewInt(1), big.NewInt(2)}
fixedArrBytesPack, err := abi.Pack("fixedArrBytes", bytesin, arrin)
if err != nil {
t.Error(err)
}

// generate expected output
offset = make([]byte, 32)
offset[31] = 96
length = make([]byte, 32)
length[31] = byte(len(strin))
strvalue = common.RightPadBytes([]byte(strin), 32)
arrinvalue1 = common.LeftPadBytes(arrin[0].Bytes(), 32)
arrinvalue2 = common.LeftPadBytes(arrin[1].Bytes(), 32)
exp = append(offset, arrinvalue1...)
exp = append(exp, arrinvalue2...)
exp = append(exp, append(length, strvalue...)...)

// ignore first 4 bytes of the output. This is the function identifier
fixedArrBytesPack = fixedArrBytesPack[4:]
if !bytes.Equal(fixedArrBytesPack, exp) {
t.Errorf("expected %x, got %x\n", exp, fixedArrBytesPack)
}

// test string, fixed array uint256[2], dynamic array uint256[]
strin = "hello world"
fixedarrin := [2]*big.Int{big.NewInt(1), big.NewInt(2)}
dynarrin := []*big.Int{big.NewInt(1), big.NewInt(2), big.NewInt(3)}
mixedArrStrPack, err := abi.Pack("mixedArrStr", strin, fixedarrin, dynarrin)
if err != nil {
t.Error(err)
}

// generate expected output
stroffset := make([]byte, 32)
stroffset[31] = 128
strlength := make([]byte, 32)
strlength[31] = byte(len(strin))
strvalue = common.RightPadBytes([]byte(strin), 32)
fixedarrinvalue1 := common.LeftPadBytes(fixedarrin[0].Bytes(), 32)
fixedarrinvalue2 := common.LeftPadBytes(fixedarrin[1].Bytes(), 32)
dynarroffset := make([]byte, 32)
dynarroffset[31] = byte(160 + ((len(strin)/32)+1)*32)
dynarrlength := make([]byte, 32)
dynarrlength[31] = byte(len(dynarrin))
dynarrinvalue1 := common.LeftPadBytes(dynarrin[0].Bytes(), 32)
dynarrinvalue2 := common.LeftPadBytes(dynarrin[1].Bytes(), 32)
dynarrinvalue3 := common.LeftPadBytes(dynarrin[2].Bytes(), 32)
exp = append(stroffset, fixedarrinvalue1...)
exp = append(exp, fixedarrinvalue2...)
exp = append(exp, dynarroffset...)
exp = append(exp, append(strlength, strvalue...)...)
dynarrarg := append(dynarrlength, dynarrinvalue1...)
dynarrarg = append(dynarrarg, dynarrinvalue2...)
dynarrarg = append(dynarrarg, dynarrinvalue3...)
exp = append(exp, dynarrarg...)

// ignore first 4 bytes of the output. This is the function identifier
mixedArrStrPack = mixedArrStrPack[4:]
if !bytes.Equal(mixedArrStrPack, exp) {
t.Errorf("expected %x, got %x\n", exp, mixedArrStrPack)
}

// test string, fixed array uint256[2], fixed array uint256[3]
strin = "hello world"
fixedarrin1 := [2]*big.Int{big.NewInt(1), big.NewInt(2)}
fixedarrin2 := [3]*big.Int{big.NewInt(1), big.NewInt(2), big.NewInt(3)}
doubleFixedArrStrPack, err := abi.Pack("doubleFixedArrStr", strin, fixedarrin1, fixedarrin2)
if err != nil {
t.Error(err)
}

// generate expected output
stroffset = make([]byte, 32)
stroffset[31] = 192
strlength = make([]byte, 32)
strlength[31] = byte(len(strin))
strvalue = common.RightPadBytes([]byte(strin), 32)
fixedarrin1value1 := common.LeftPadBytes(fixedarrin1[0].Bytes(), 32)
fixedarrin1value2 := common.LeftPadBytes(fixedarrin1[1].Bytes(), 32)
fixedarrin2value1 := common.LeftPadBytes(fixedarrin2[0].Bytes(), 32)
fixedarrin2value2 := common.LeftPadBytes(fixedarrin2[1].Bytes(), 32)
fixedarrin2value3 := common.LeftPadBytes(fixedarrin2[2].Bytes(), 32)
exp = append(stroffset, fixedarrin1value1...)
exp = append(exp, fixedarrin1value2...)
exp = append(exp, fixedarrin2value1...)
exp = append(exp, fixedarrin2value2...)
exp = append(exp, fixedarrin2value3...)
exp = append(exp, append(strlength, strvalue...)...)

// ignore first 4 bytes of the output. This is the function identifier
doubleFixedArrStrPack = doubleFixedArrStrPack[4:]
if !bytes.Equal(doubleFixedArrStrPack, exp) {
t.Errorf("expected %x, got %x\n", exp, doubleFixedArrStrPack)
}

// test string, fixed array uint256[2], dynamic array uint256[], fixed array uint256[3]
strin = "hello world"
fixedarrin1 = [2]*big.Int{big.NewInt(1), big.NewInt(2)}
dynarrin = []*big.Int{big.NewInt(1), big.NewInt(2)}
fixedarrin2 = [3]*big.Int{big.NewInt(1), big.NewInt(2), big.NewInt(3)}
multipleMixedArrStrPack, err := abi.Pack("multipleMixedArrStr", strin, fixedarrin1, dynarrin, fixedarrin2)
if err != nil {
t.Error(err)
}

// generate expected output
stroffset = make([]byte, 32)
stroffset[31] = 224
strlength = make([]byte, 32)
strlength[31] = byte(len(strin))
strvalue = common.RightPadBytes([]byte(strin), 32)
fixedarrin1value1 = common.LeftPadBytes(fixedarrin1[0].Bytes(), 32)
fixedarrin1value2 = common.LeftPadBytes(fixedarrin1[1].Bytes(), 32)
dynarroffset = U256(big.NewInt(int64(256 + ((len(strin)/32)+1)*32)))
dynarrlength = make([]byte, 32)
dynarrlength[31] = byte(len(dynarrin))
dynarrinvalue1 = common.LeftPadBytes(dynarrin[0].Bytes(), 32)
dynarrinvalue2 = common.LeftPadBytes(dynarrin[1].Bytes(), 32)
fixedarrin2value1 = common.LeftPadBytes(fixedarrin2[0].Bytes(), 32)
fixedarrin2value2 = common.LeftPadBytes(fixedarrin2[1].Bytes(), 32)
fixedarrin2value3 = common.LeftPadBytes(fixedarrin2[2].Bytes(), 32)
exp = append(stroffset, fixedarrin1value1...)
exp = append(exp, fixedarrin1value2...)
exp = append(exp, dynarroffset...)
exp = append(exp, fixedarrin2value1...)
exp = append(exp, fixedarrin2value2...)
exp = append(exp, fixedarrin2value3...)
exp = append(exp, append(strlength, strvalue...)...)
dynarrarg = append(dynarrlength, dynarrinvalue1...)
dynarrarg = append(dynarrarg, dynarrinvalue2...)
exp = append(exp, dynarrarg...)

// ignore first 4 bytes of the output. This is the function identifier
multipleMixedArrStrPack = multipleMixedArrStrPack[4:]
if !bytes.Equal(multipleMixedArrStrPack, exp) {
t.Errorf("expected %x, got %x\n", exp, multipleMixedArrStrPack)
}
}

func TestDefaultFunctionParsing(t *testing.T) {
const definition = `[{ "name" : "balance" }]`

Expand Down
13 changes: 12 additions & 1 deletion accounts/abi/method.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ func (method Method) pack(args ...interface{}) ([]byte, error) {
// output. This is used for strings and bytes types input.
var variableInput []byte

// input offset is the bytes offset for packed output
inputOffset := 0
for _, input := range method.Inputs {
if input.Type.T == ArrayTy {
inputOffset += (32 * input.Type.Size)
} else {
inputOffset += 32
}
}

var ret []byte
for i, a := range args {
input := method.Inputs[i]
Expand All @@ -60,7 +70,8 @@ func (method Method) pack(args ...interface{}) ([]byte, error) {
// check for a slice type (string, bytes, slice)
if input.Type.requiresLengthPrefix() {
// calculate the offset
offset := len(method.Inputs)*32 + len(variableInput)
offset := inputOffset + len(variableInput)

// set the offset
ret = append(ret, packNum(reflect.ValueOf(offset))...)
// Append the packed output to the variable input. The variable input
Expand Down