From da58afcea0525fab2d3f45f58e7e243a15407ab9 Mon Sep 17 00:00:00 2001 From: Dmitry Shulyak Date: Wed, 20 Dec 2017 16:09:23 +0200 Subject: [PATCH] accounts/abi: update array length after parsing array (#15618) Fixes #15617 --- accounts/abi/event.go | 8 +++-- accounts/abi/event_test.go | 23 +++++++++++++ accounts/abi/method.go | 9 +++--- accounts/abi/unpack_test.go | 64 ++++++++++++++++++++++++++----------- 4 files changed, 78 insertions(+), 26 deletions(-) diff --git a/accounts/abi/event.go b/accounts/abi/event.go index 44ed7b8df2..bd1098d878 100644 --- a/accounts/abi/event.go +++ b/accounts/abi/event.go @@ -71,14 +71,16 @@ func (e Event) tupleUnpack(v interface{}, output []byte) error { if input.Indexed { // can't read, continue continue - } else if input.Type.T == ArrayTy { - // need to move this up because they read sequentially - j += input.Type.Size } marshalledValue, err := toGoType((i+j)*32, input.Type, output) if err != nil { return err } + if input.Type.T == ArrayTy { + // combined index ('i' + 'j') need to be adjusted only by size of array, thus + // we need to decrement 'j' because 'i' was incremented + j += input.Type.Size - 1 + } reflectValue := reflect.ValueOf(marshalledValue) switch value.Kind() { diff --git a/accounts/abi/event_test.go b/accounts/abi/event_test.go index 7e2f13f763..a3899b4a6f 100644 --- a/accounts/abi/event_test.go +++ b/accounts/abi/event_test.go @@ -17,11 +17,14 @@ package abi import ( + "bytes" + "reflect" "strings" "testing" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" + "github.com/stretchr/testify/require" ) func TestEventId(t *testing.T) { @@ -54,3 +57,23 @@ func TestEventId(t *testing.T) { } } } + +// TestEventMultiValueWithArrayUnpack verifies that array fields will be counted after parsing array. +func TestEventMultiValueWithArrayUnpack(t *testing.T) { + definition := `[{"name": "test", "type": "event", "inputs": [{"indexed": false, "name":"value1", "type":"uint8[2]"},{"indexed": false, "name":"value2", "type":"uint8"}]}]` + type testStruct struct { + Value1 [2]uint8 + Value2 uint8 + } + abi, err := JSON(strings.NewReader(definition)) + require.NoError(t, err) + var b bytes.Buffer + var i uint8 = 1 + for ; i <= 3; i++ { + b.Write(packNum(reflect.ValueOf(i))) + } + var rst testStruct + require.NoError(t, abi.Unpack(&rst, "test", b.Bytes())) + require.Equal(t, [2]uint8{1, 2}, rst.Value1) + require.Equal(t, uint8(3), rst.Value2) +} diff --git a/accounts/abi/method.go b/accounts/abi/method.go index d8838e9ed6..6b9aa011ef 100644 --- a/accounts/abi/method.go +++ b/accounts/abi/method.go @@ -95,14 +95,15 @@ func (method Method) tupleUnpack(v interface{}, output []byte) error { j := 0 for i := 0; i < len(method.Outputs); i++ { toUnpack := method.Outputs[i] - if toUnpack.Type.T == ArrayTy { - // need to move this up because they read sequentially - j += toUnpack.Type.Size - } marshalledValue, err := toGoType((i+j)*32, toUnpack.Type, output) if err != nil { return err } + if toUnpack.Type.T == ArrayTy { + // combined index ('i' + 'j') need to be adjusted only by size of array, thus + // we need to decrement 'j' because 'i' was incremented + j += toUnpack.Type.Size - 1 + } reflectValue := reflect.ValueOf(marshalledValue) switch value.Kind() { diff --git a/accounts/abi/unpack_test.go b/accounts/abi/unpack_test.go index 1e21aafc05..14393d2309 100644 --- a/accounts/abi/unpack_test.go +++ b/accounts/abi/unpack_test.go @@ -22,6 +22,7 @@ import ( "fmt" "math/big" "reflect" + "strconv" "strings" "testing" @@ -261,25 +262,27 @@ var unpackTests = []unpackTest{ func TestUnpack(t *testing.T) { for i, test := range unpackTests { - def := fmt.Sprintf(`[{ "name" : "method", "outputs": %s}]`, test.def) - abi, err := JSON(strings.NewReader(def)) - if err != nil { - t.Fatalf("invalid ABI definition %s: %v", def, err) - } - encb, err := hex.DecodeString(test.enc) - if err != nil { - t.Fatalf("invalid hex: %s" + test.enc) - } - outptr := reflect.New(reflect.TypeOf(test.want)) - err = abi.Unpack(outptr.Interface(), "method", encb) - if err := test.checkError(err); err != nil { - t.Errorf("test %d (%v) failed: %v", i, test.def, err) - continue - } - out := outptr.Elem().Interface() - if !reflect.DeepEqual(test.want, out) { - t.Errorf("test %d (%v) failed: expected %v, got %v", i, test.def, test.want, out) - } + t.Run(strconv.Itoa(i), func(t *testing.T) { + def := fmt.Sprintf(`[{ "name" : "method", "outputs": %s}]`, test.def) + abi, err := JSON(strings.NewReader(def)) + if err != nil { + t.Fatalf("invalid ABI definition %s: %v", def, err) + } + encb, err := hex.DecodeString(test.enc) + if err != nil { + t.Fatalf("invalid hex: %s" + test.enc) + } + outptr := reflect.New(reflect.TypeOf(test.want)) + err = abi.Unpack(outptr.Interface(), "method", encb) + if err := test.checkError(err); err != nil { + t.Errorf("test %d (%v) failed: %v", i, test.def, err) + return + } + out := outptr.Elem().Interface() + if !reflect.DeepEqual(test.want, out) { + t.Errorf("test %d (%v) failed: expected %v, got %v", i, test.def, test.want, out) + } + }) } } @@ -336,6 +339,29 @@ func TestMultiReturnWithStruct(t *testing.T) { } } +func TestMultiReturnWithArray(t *testing.T) { + const definition = `[{"name" : "multi", "outputs": [{"type": "uint64[3]"}, {"type": "uint64"}]}]` + abi, err := JSON(strings.NewReader(definition)) + if err != nil { + t.Fatal(err) + } + buff := new(bytes.Buffer) + buff.Write(common.Hex2Bytes("000000000000000000000000000000000000000000000000000000000000000900000000000000000000000000000000000000000000000000000000000000090000000000000000000000000000000000000000000000000000000000000009")) + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000008")) + + ret1, ret1Exp := new([3]uint64), [3]uint64{9, 9, 9} + ret2, ret2Exp := new(uint64), uint64(8) + if err := abi.Unpack(&[]interface{}{ret1, ret2}, "multi", buff.Bytes()); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(*ret1, ret1Exp) { + t.Error("array result", *ret1, "!= Expected", ret1Exp) + } + if *ret2 != ret2Exp { + t.Error("int result", *ret2, "!= Expected", ret2Exp) + } +} + func TestUnmarshal(t *testing.T) { const definition = `[ { "name" : "int", "constant" : false, "outputs": [ { "type": "uint256" } ] },