From e7cd627d93d43ff52d13c66b336e12b96d9bfdef Mon Sep 17 00:00:00 2001 From: croath Date: Fri, 29 Dec 2017 19:46:40 +0800 Subject: [PATCH 01/11] accounts/abi: fix for one output interface crashing --- accounts/abi/argument.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/accounts/abi/argument.go b/accounts/abi/argument.go index ad17fbf2b1..65f79c9d4c 100644 --- a/accounts/abi/argument.go +++ b/accounts/abi/argument.go @@ -169,6 +169,16 @@ func (arguments Arguments) unpackAtomic(v interface{}, output []byte) error { if err != nil { return err } + + // if we reach this part, there is only one output member from the contract event. + // for mobile, the result type is always a slice. + if reflect.Slice == value.Kind() && value.Len() >= 1 { + //check if it's not a byte slice + if reflect.TypeOf([]byte{}) != value.Type() { + value = value.Index(0).Elem() + } + } + return set(value, reflect.ValueOf(marshalledValue), arg) } From 88e67c552e5da4d114d80be3ff471f4f2ed030f5 Mon Sep 17 00:00:00 2001 From: croath Date: Sun, 31 Dec 2017 19:12:55 +0800 Subject: [PATCH 02/11] accounts/abi: add a test case for unpacking mobile interfaces --- accounts/abi/unpack_test.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/accounts/abi/unpack_test.go b/accounts/abi/unpack_test.go index 3910950732..a7901896ab 100644 --- a/accounts/abi/unpack_test.go +++ b/accounts/abi/unpack_test.go @@ -287,6 +287,42 @@ func TestUnpack(t *testing.T) { } } +var unpackMobileTests = []unpackTest{ + { + def: `[{"type": "uint256"}]`, + enc: "0000000000000000000000000000000000000000000000000000000000000001", + want: big.NewInt(1), + }, +} + +func TestUnpackMobileOnly(t *testing.T) { + for i, test := range unpackMobileTests { + 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)) + results := make([]interface{}, 1) + copy(results, []interface{}{outptr.Interface()}) + err = abi.Unpack(&results, "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) + } + }) + } +} + type methodMultiOutput struct { Int *big.Int String string From a6787a6308d2109006b036c8a6a331afa938912d Mon Sep 17 00:00:00 2001 From: croath Date: Tue, 23 Jan 2018 19:10:23 +0800 Subject: [PATCH 03/11] accounts/abi: fix the `output` at the beginning instead of making a workaround --- accounts/abi/argument.go | 10 ---------- accounts/abi/unpack_test.go | 36 ------------------------------------ mobile/bind.go | 18 +++++++++++++----- 3 files changed, 13 insertions(+), 51 deletions(-) diff --git a/accounts/abi/argument.go b/accounts/abi/argument.go index 65f79c9d4c..ad17fbf2b1 100644 --- a/accounts/abi/argument.go +++ b/accounts/abi/argument.go @@ -169,16 +169,6 @@ func (arguments Arguments) unpackAtomic(v interface{}, output []byte) error { if err != nil { return err } - - // if we reach this part, there is only one output member from the contract event. - // for mobile, the result type is always a slice. - if reflect.Slice == value.Kind() && value.Len() >= 1 { - //check if it's not a byte slice - if reflect.TypeOf([]byte{}) != value.Type() { - value = value.Index(0).Elem() - } - } - return set(value, reflect.ValueOf(marshalledValue), arg) } diff --git a/accounts/abi/unpack_test.go b/accounts/abi/unpack_test.go index a7901896ab..3910950732 100644 --- a/accounts/abi/unpack_test.go +++ b/accounts/abi/unpack_test.go @@ -287,42 +287,6 @@ func TestUnpack(t *testing.T) { } } -var unpackMobileTests = []unpackTest{ - { - def: `[{"type": "uint256"}]`, - enc: "0000000000000000000000000000000000000000000000000000000000000001", - want: big.NewInt(1), - }, -} - -func TestUnpackMobileOnly(t *testing.T) { - for i, test := range unpackMobileTests { - 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)) - results := make([]interface{}, 1) - copy(results, []interface{}{outptr.Interface()}) - err = abi.Unpack(&results, "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) - } - }) - } -} - type methodMultiOutput struct { Int *big.Int String string diff --git a/mobile/bind.go b/mobile/bind.go index 7b79bedafd..e8164d523e 100644 --- a/mobile/bind.go +++ b/mobile/bind.go @@ -154,12 +154,20 @@ func (c *BoundContract) GetDeployer() *Transaction { // Call invokes the (constant) contract method with params as input values and // sets the output to result. func (c *BoundContract) Call(opts *CallOpts, out *Interfaces, method string, args *Interfaces) error { - results := make([]interface{}, len(out.objects)) - copy(results, out.objects) - if err := c.contract.Call(&opts.opts, &results, method, args.objects...); err != nil { - return err + if len(out.objects) == 1 { + result := out.objects[0] + if err := c.contract.Call(&opts.opts, result, method, args.objects...); err != nil { + return err + } + out.objects[0] = result + } else { + results := make([]interface{}, len(out.objects)) + copy(results, out.objects) + if err := c.contract.Call(&opts.opts, &results, method, args.objects...); err != nil { + return err + } + copy(out.objects, results) } - copy(out.objects, results) return nil } From 1ede68355db6adbf468d198a8d1ecb0ad1a3ea31 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 28 Dec 2017 11:17:45 +0100 Subject: [PATCH 04/11] accounts/abi: add another unpack interface --- accounts/abi/abi.go | 6 +- accounts/abi/abi_test.go | 6 +- accounts/abi/argument.go | 59 +++++- accounts/abi/unpack_test.go | 2 +- accounts/abi/unpackv2_test.go | 336 ++++++++++++++++++++++++++++++++++ 5 files changed, 395 insertions(+), 14 deletions(-) create mode 100644 accounts/abi/unpackv2_test.go diff --git a/accounts/abi/abi.go b/accounts/abi/abi.go index abcb403db2..32f041890b 100644 --- a/accounts/abi/abi.go +++ b/accounts/abi/abi.go @@ -136,11 +136,11 @@ func (abi *ABI) UnmarshalJSON(data []byte) error { // MethodById looks up a method by the 4-byte id // returns nil if none found -func (abi *ABI) MethodById(sigdata []byte) *Method { +func (abi *ABI) MethodById(sigdata []byte) (*Method, error){ for _, method := range abi.Methods { if bytes.Equal(method.Id(), sigdata[:4]) { - return &method + return &method, nil } } - return nil + return nil, fmt.Errorf("ABI spec does not contain method signature in data: 0x%x", sigdata[:4]) } diff --git a/accounts/abi/abi_test.go b/accounts/abi/abi_test.go index 2d43b631c2..3bef6add5c 100644 --- a/accounts/abi/abi_test.go +++ b/accounts/abi/abi_test.go @@ -689,7 +689,11 @@ func TestABI_MethodById(t *testing.T) { } for name, m := range abi.Methods { a := fmt.Sprintf("%v", m) - b := fmt.Sprintf("%v", abi.MethodById(m.Id())) + m2,err := abi.MethodById(m.Id()) + if err != nil { + t.Fatal(err) + } + b := fmt.Sprintf("%v", m2) if a != b { t.Errorf("Method %v (id %v) not 'findable' by id in ABI", name, common.ToHex(m.Id())) } diff --git a/accounts/abi/argument.go b/accounts/abi/argument.go index 04ca6150ad..b9b5371211 100644 --- a/accounts/abi/argument.go +++ b/accounts/abi/argument.go @@ -67,6 +67,16 @@ func (arguments Arguments) LengthNonIndexed() int { return out } +func (arguments Arguments) NonIndexed() Arguments{ + var ret []Argument + for _,arg := range arguments{ + if !arg.Indexed{ + ret = append(ret, arg) + } + } + return ret +} + // isTuple returns true for non-atomic constructs, like (uint,uint) or uint[] func (arguments Arguments) isTuple() bool { return len(arguments) > 1 @@ -114,14 +124,9 @@ func (arguments Arguments) unpackTuple(v interface{}, output []byte) error { // `j` counts the number of complex types. // both `i` and `j` are used to to correctly compute `data` offset. - i, j := -1, 0 - for _, arg := range arguments { + j := 0 + for i, arg := range arguments.NonIndexed() { - if arg.Indexed { - // can't read, continue - continue - } - i++ marshalledValue, err := toGoType((i+j)*32, arg.Type, output) if err != nil { return err @@ -178,7 +183,6 @@ func (arguments Arguments) unpackAtomic(v interface{}, output []byte) error { } value := valueOf.Elem() - marshalledValue, err := toGoType(0, arg.Type, output) if err != nil { return err @@ -186,7 +190,44 @@ func (arguments Arguments) unpackAtomic(v interface{}, output []byte) error { return set(value, reflect.ValueOf(marshalledValue), arg) } -// Unpack performs the operation Go format -> Hexdata +// UnpackValues can be used to unpack ABI-encoded hexdata according to the ABI-specification, +// without supplying a struct to unpack into. Instead, this method returns a list containing the +// values. An atomic argument will be a list with one element. +func (arguments Arguments) UnpackValues(data []byte) ([]interface{}, error){ + + retval := make([]interface{},0,arguments.LengthNonIndexed()) + + virtualArgs := 0 + + for index,arg:= range arguments.NonIndexed(){ + + marshalledValue, err := toGoType((index + virtualArgs) * 32, arg.Type, data) + + if arg.Type.T == ArrayTy { + //If we have a static array, like [3]uint256, these are coded as + // just like uint256,uint256,uint256. + // This means that we need to add two 'virtual' arguments when + // we count the index from now on + + virtualArgs += arg.Type.Size - 1 + } + + if err != nil{ + return nil, err + } + retval = append(retval, marshalledValue) + } + return retval, nil +} + +// UnpackValues performs the operation Go format -> Hexdata +// It is the semantic opposite of UnpackValues +func (arguments Arguments) PackValues(args []interface{}) ([]byte, error) { + return arguments.Pack(args...) +} + + +// Pack performs the operation Go format -> Hexdata func (arguments Arguments) Pack(args ...interface{}) ([]byte, error) { // Make sure arguments match up and pack them abiArgs := arguments diff --git a/accounts/abi/unpack_test.go b/accounts/abi/unpack_test.go index a65426a30b..e9f9108125 100644 --- a/accounts/abi/unpack_test.go +++ b/accounts/abi/unpack_test.go @@ -130,7 +130,7 @@ var unpackTests = []unpackTest{ { def: `[{"type": "bytes32"}]`, enc: "0100000000000000000000000000000000000000000000000000000000000000", - want: common.HexToHash("0100000000000000000000000000000000000000000000000000000000000000"), + want: [32]byte{1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, }, { def: `[{"type": "function"}]`, diff --git a/accounts/abi/unpackv2_test.go b/accounts/abi/unpackv2_test.go new file mode 100644 index 0000000000..d0074ff7b1 --- /dev/null +++ b/accounts/abi/unpackv2_test.go @@ -0,0 +1,336 @@ +// Copyright 2015 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package abi + +import ( + "bytes" + "encoding/hex" + "fmt" + "math/big" + "reflect" + "strconv" + "strings" + "testing" + + "github.com/ethereum/go-ethereum/common" +) + +func TestUnpackV2(t *testing.T) { + for i, test := range unpackTests { + 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) + } + out, err := abi.Methods["method"].Outputs.UnpackValues(encb) + + if err != nil { + t.Fatal(err) + } + if len(test.err) != 0 { + // The new stuff doesn't have these types of errors + return + } + if !reflect.DeepEqual(test.want, out[0]) { + t.Errorf("test %d (%v) failed: expected %v, got %v", i, test.def, test.want, out[0]) + } + }) + } +} + + +func TestMultiReturnWithArrayV2(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("000000000000000000000000000000000000000000000000000000000000000900000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000007")) + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000006")) + + out, err := abi.Methods["multi"].Outputs.UnpackValues(buff.Bytes()) + + ret1Exp := [3]uint64{9, 8, 7} + ret2Exp := uint64(6) + + if !reflect.DeepEqual(out[0], ret1Exp) { + t.Error("array result", out[0], "!= Expected", ret1Exp) + } + if out[1] != ret2Exp { + t.Error("int result", out[1], "!= Expected", ret2Exp) + } +} + +func TestUnmarshalV2(t *testing.T) { + const definition = `[ + { "name" : "int", "constant" : false, "outputs": [ { "type": "uint256" } ] }, + { "name" : "bool", "constant" : false, "outputs": [ { "type": "bool" } ] }, + { "name" : "bytes", "constant" : false, "outputs": [ { "type": "bytes" } ] }, + { "name" : "fixed", "constant" : false, "outputs": [ { "type": "bytes32" } ] }, + { "name" : "multi", "constant" : false, "outputs": [ { "type": "bytes" }, { "type": "bytes" } ] }, + { "name" : "intArraySingle", "constant" : false, "outputs": [ { "type": "uint256[3]" } ] }, + { "name" : "addressSliceSingle", "constant" : false, "outputs": [ { "type": "address[]" } ] }, + { "name" : "addressSliceDouble", "constant" : false, "outputs": [ { "name": "a", "type": "address[]" }, { "name": "b", "type": "address[]" } ] }, + { "name" : "mixedBytes", "constant" : true, "outputs": [ { "name": "a", "type": "bytes" }, { "name": "b", "type": "bytes32" } ] }]` + + abi, err := JSON(strings.NewReader(definition)) + if err != nil { + t.Fatal(err) + } + buff := new(bytes.Buffer) + + // marshall mixed bytes (mixedBytes) + p0Exp := common.Hex2Bytes("01020000000000000000") + p1Exp := common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000ddeeff") + + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000040")) + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000ddeeff")) + buff.Write(common.Hex2Bytes("000000000000000000000000000000000000000000000000000000000000000a")) + buff.Write(common.Hex2Bytes("0102000000000000000000000000000000000000000000000000000000000000")) + + mixedBytes, err := abi.Methods["mixedBytes"].Outputs.UnpackValues(buff.Bytes()) + if err != nil { + t.Error(err) + } else { + p0 := mixedBytes[0].([]byte) + p1 := mixedBytes[1].([32]byte) + if !bytes.Equal(p0, p0Exp) { + t.Errorf("unexpected value unpacked: want %x, got %x", p0Exp, p0) + } + + if !bytes.Equal(p1[:], p1Exp) { + t.Errorf("unexpected value unpacked: want %x, got %x", p1Exp, p1) + } + } + + // marshal int + integer, err := abi.Methods["int"].Outputs.UnpackValues(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000001")) + if err != nil { + t.Error(err) + } + if len(integer) == 0 { + t.Error("Expected one integer") + } + intval := integer[0].(*big.Int) + if intval == nil || intval.Cmp(big.NewInt(1)) != 0 { + t.Error("expected Int to be 1 got", intval) + } + + // marshal bool + boolreturns, err := abi.Methods["bool"].Outputs.UnpackValues(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000001")) + if err != nil { + t.Error(err) + } + boolval := boolreturns[0].(bool) + if !boolval { + t.Error("expected Bool to be true") + } + + // marshal dynamic bytes max length 32 + buff.Reset() + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000020")) + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000020")) + bytesOut := common.RightPadBytes([]byte("hello"), 32) + buff.Write(bytesOut) + + bytesreturns, err := abi.Methods["bytes"].Outputs.UnpackValues(buff.Bytes()) + + if err != nil { + t.Error(err) + } + bytesval := bytesreturns[0].([]byte) + if !bytes.Equal(bytesval, bytesOut) { + t.Errorf("expected %x got %x", bytesOut, bytesval) + } + + // marshall dynamic bytes max length 64 + buff.Reset() + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000020")) + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000040")) + bytesOut = common.RightPadBytes([]byte("hello"), 64) + buff.Write(bytesOut) + + bytesreturns, err = abi.Methods["bytes"].Outputs.UnpackValues(buff.Bytes()) + if err != nil { + t.Error(err) + } + bytesval = bytesreturns[0].([]byte) + if !bytes.Equal(bytesval, bytesOut) { + t.Errorf("expected %x got %x", bytesOut, bytesval) + } + + // marshall dynamic bytes max length 64 + buff.Reset() + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000020")) + buff.Write(common.Hex2Bytes("000000000000000000000000000000000000000000000000000000000000003f")) + bytesOut = common.RightPadBytes([]byte("hello"), 64) + buff.Write(bytesOut) + + bytesreturns, err = abi.Methods["bytes"].Outputs.UnpackValues(buff.Bytes()) + if err != nil { + t.Error(err) + } + bytesval = bytesreturns[0].([]byte) + + if !bytes.Equal(bytesval, bytesOut[:len(bytesOut)-1]) { + t.Errorf("expected %x got %x", bytesOut[:len(bytesOut)-1], bytesval) + } + // marshal dynamic bytes output empty (nil) + bytesreturns, err = abi.Methods["bytes"].Outputs.UnpackValues(nil) + if err == nil { + t.Error("expected error") + } + // marshal dynamic bytes output empty + buff.Reset() + bytesreturns, err = abi.Methods["bytes"].Outputs.UnpackValues(buff.Bytes()) + if err == nil { + t.Error("expected error") + } + + // marshal dynamic bytes length 5 + buff.Reset() + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000020")) + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000005")) + buff.Write(common.RightPadBytes([]byte("hello"), 32)) + + bytesreturns, err = abi.Methods["bytes"].Outputs.UnpackValues(buff.Bytes()) + if err != nil { + t.Error(err) + } + bytesval = bytesreturns[0].([]byte) + + if !bytes.Equal(bytesval, []byte("hello")) { + t.Errorf("expected %x got %x", bytesOut, bytesval) + } + + // marshal dynamic bytes length 5 + buff.Reset() + buff.Write(common.RightPadBytes([]byte("hello"), 32)) + + hashreturns, err := abi.Methods["fixed"].Outputs.UnpackValues(buff.Bytes()) + if err != nil { + t.Error(err) + } + hashval := hashreturns[0].([32]byte) + + helloHash := common.BytesToHash(common.RightPadBytes([]byte("hello"), 32)) + if common.Hash(hashval) != helloHash { + t.Errorf("Expected %x to equal %x", hashval, helloHash) + } + + // marshal error + buff.Reset() + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000020")) + + bytesreturns, err = abi.Methods["bytes"].Outputs.UnpackValues(buff.Bytes()) + if err == nil { + // Error abi: cannot marshal in to go slice: offset 32 would go over slice boundary (len=64) + t.Error("expected error") + } + bytesreturns, err = abi.Methods["multi"].Outputs.UnpackValues(make([]byte, 64)) + + buff.Reset() + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000001")) + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000002")) + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000003")) + // marshal int array + + intArrayReturns, err := abi.Methods["intArraySingle"].Outputs.UnpackValues(buff.Bytes()) + if err != nil { + t.Error(err) + } + intArray := intArrayReturns[0].([3]*big.Int) + + var testAgainstIntArray = [3]*big.Int{big.NewInt(1), big.NewInt(2), big.NewInt(3)} + + for i, intval := range intArray { + if intval.Cmp(testAgainstIntArray[i]) != 0 { + t.Errorf("expected %v, got %v", testAgainstIntArray[i], intval) + } + } + // marshal address slice + buff.Reset() + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000020")) // offset + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000001")) // size + buff.Write(common.Hex2Bytes("0000000000000000000000000100000000000000000000000000000000000000")) + + outAddrReturns, err := abi.Methods["addressSliceSingle"].Outputs.UnpackValues(buff.Bytes()) + if err != nil { + t.Fatal("didn't expect error:", err) + } + outAddr := outAddrReturns[0].([]common.Address) + if len(outAddr) != 1 { + t.Fatal("expected 1 item, got", len(outAddr)) + } + + if outAddr[0] != (common.Address{1}) { + t.Errorf("expected %x, got %x", common.Address{1}, outAddr[0]) + } + + // marshal multiple address slice + buff.Reset() + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000040")) // offset + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000080")) // offset + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000001")) // size + buff.Write(common.Hex2Bytes("0000000000000000000000000100000000000000000000000000000000000000")) + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000002")) // size + buff.Write(common.Hex2Bytes("0000000000000000000000000200000000000000000000000000000000000000")) + buff.Write(common.Hex2Bytes("0000000000000000000000000300000000000000000000000000000000000000")) + + outAddrStructReturns, err := abi.Methods["addressSliceDouble"].Outputs.UnpackValues(buff.Bytes()) + if err != nil { + t.Fatal("didn't expect error:", err) + } + A := outAddrStructReturns[0].([]common.Address) + B := outAddrStructReturns[1].([]common.Address) + + if len(A) != 1 { + t.Fatal("expected 1 item, got", len(A)) + } + + if A[0] != (common.Address{1}) { + t.Errorf("expected %x, got %x", common.Address{1}, A[0]) + } + + if len(B) != 2 { + t.Fatal("expected 1 item, got", len(B)) + } + + if B[0] != (common.Address{2}) { + t.Errorf("expected %x, got %x", common.Address{2}, B[0]) + } + if B[1] != (common.Address{3}) { + t.Errorf("expected %x, got %x", common.Address{3}, B[1]) + } + + // marshal invalid address slice + buff.Reset() + buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000100")) + + err = abi.Unpack(&outAddr, "addressSliceSingle", buff.Bytes()) + _, err = abi.Methods["addressSliceSingle"].Outputs.UnpackValues(buff.Bytes()) + if err == nil { + t.Fatal("expected error:", err) + } + +} From f0f594d0453c6f53eaeeac6187785daf12044f58 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Sat, 30 Dec 2017 16:07:12 +0100 Subject: [PATCH 05/11] accounts/abi: Deduplicate code in unpacker --- accounts/abi/abi.go | 2 +- accounts/abi/abi_test.go | 2 +- accounts/abi/argument.go | 81 ++++++++++++++--------------------- accounts/abi/unpackv2_test.go | 1 - 4 files changed, 33 insertions(+), 53 deletions(-) diff --git a/accounts/abi/abi.go b/accounts/abi/abi.go index 32f041890b..fd286c2c22 100644 --- a/accounts/abi/abi.go +++ b/accounts/abi/abi.go @@ -136,7 +136,7 @@ func (abi *ABI) UnmarshalJSON(data []byte) error { // MethodById looks up a method by the 4-byte id // returns nil if none found -func (abi *ABI) MethodById(sigdata []byte) (*Method, error){ +func (abi *ABI) MethodById(sigdata []byte) (*Method, error) { for _, method := range abi.Methods { if bytes.Equal(method.Id(), sigdata[:4]) { return &method, nil diff --git a/accounts/abi/abi_test.go b/accounts/abi/abi_test.go index 3bef6add5c..e66828240a 100644 --- a/accounts/abi/abi_test.go +++ b/accounts/abi/abi_test.go @@ -689,7 +689,7 @@ func TestABI_MethodById(t *testing.T) { } for name, m := range abi.Methods { a := fmt.Sprintf("%v", m) - m2,err := abi.MethodById(m.Id()) + m2, err := abi.MethodById(m.Id()) if err != nil { t.Fatal(err) } diff --git a/accounts/abi/argument.go b/accounts/abi/argument.go index b9b5371211..bdd0894f17 100644 --- a/accounts/abi/argument.go +++ b/accounts/abi/argument.go @@ -67,10 +67,10 @@ func (arguments Arguments) LengthNonIndexed() int { return out } -func (arguments Arguments) NonIndexed() Arguments{ +func (arguments Arguments) NonIndexed() Arguments { var ret []Argument - for _,arg := range arguments{ - if !arg.Indexed{ + for _, arg := range arguments { + if !arg.Indexed { ret = append(ret, arg) } } @@ -84,21 +84,27 @@ func (arguments Arguments) isTuple() bool { // Unpack performs the operation hexdata -> Go format func (arguments Arguments) Unpack(v interface{}, data []byte) error { - if arguments.isTuple() { - return arguments.unpackTuple(v, data) - } - return arguments.unpackAtomic(v, data) -} -func (arguments Arguments) unpackTuple(v interface{}, output []byte) error { // make sure the passed value is arguments pointer - valueOf := reflect.ValueOf(v) - if reflect.Ptr != valueOf.Kind() { + if reflect.Ptr != reflect.ValueOf(v).Kind() { return fmt.Errorf("abi: Unpack(non-pointer %T)", v) } + marshalledValues, err := arguments.UnpackValues(data) + if err != nil { + return err + } + + if arguments.isTuple() { + return arguments.unpackTuple(v, marshalledValues) + } + return arguments.unpackAtomic(v, marshalledValues) +} + +func (arguments Arguments) unpackTuple(v interface{}, marshalledValues []interface{}) error { + var ( - value = valueOf.Elem() + value = reflect.ValueOf(v).Elem() typ = value.Type() kind = value.Kind() ) @@ -120,25 +126,9 @@ func (arguments Arguments) unpackTuple(v interface{}, output []byte) error { exists[field] = true } } - // `i` counts the nonindexed arguments. - // `j` counts the number of complex types. - // both `i` and `j` are used to to correctly compute `data` offset. - - j := 0 for i, arg := range arguments.NonIndexed() { - marshalledValue, err := toGoType((i+j)*32, arg.Type, output) - if err != nil { - return err - } - - if arg.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 += arg.Type.Size - 1 - } - - reflectValue := reflect.ValueOf(marshalledValue) + reflectValue := reflect.ValueOf(marshalledValues[i]) switch kind { case reflect.Struct: @@ -171,37 +161,29 @@ func (arguments Arguments) unpackTuple(v interface{}, output []byte) error { } // unpackAtomic unpacks ( hexdata -> go ) a single value -func (arguments Arguments) unpackAtomic(v interface{}, output []byte) error { - // make sure the passed value is arguments pointer - valueOf := reflect.ValueOf(v) - if reflect.Ptr != valueOf.Kind() { - return fmt.Errorf("abi: Unpack(non-pointer %T)", v) - } - arg := arguments[0] - if arg.Indexed { - return fmt.Errorf("abi: attempting to unpack indexed variable into element.") +func (arguments Arguments) unpackAtomic(v interface{}, marshalledValues []interface{}) error { + + if len(marshalledValues) != 1 { + return fmt.Errorf("abi: wrong length, expected single value, got %d", len(marshalledValues)) } - value := valueOf.Elem() - marshalledValue, err := toGoType(0, arg.Type, output) - if err != nil { - return err - } - return set(value, reflect.ValueOf(marshalledValue), arg) + elem := reflect.ValueOf(v).Elem() + reflectValue := reflect.ValueOf(marshalledValues[0]) + return set(elem, reflectValue, arguments.NonIndexed()[0]) } // UnpackValues can be used to unpack ABI-encoded hexdata according to the ABI-specification, // without supplying a struct to unpack into. Instead, this method returns a list containing the // values. An atomic argument will be a list with one element. -func (arguments Arguments) UnpackValues(data []byte) ([]interface{}, error){ +func (arguments Arguments) UnpackValues(data []byte) ([]interface{}, error) { - retval := make([]interface{},0,arguments.LengthNonIndexed()) + retval := make([]interface{}, 0, arguments.LengthNonIndexed()) virtualArgs := 0 - for index,arg:= range arguments.NonIndexed(){ + for index, arg := range arguments.NonIndexed() { - marshalledValue, err := toGoType((index + virtualArgs) * 32, arg.Type, data) + marshalledValue, err := toGoType((index+virtualArgs)*32, arg.Type, data) if arg.Type.T == ArrayTy { //If we have a static array, like [3]uint256, these are coded as @@ -212,7 +194,7 @@ func (arguments Arguments) UnpackValues(data []byte) ([]interface{}, error){ virtualArgs += arg.Type.Size - 1 } - if err != nil{ + if err != nil { return nil, err } retval = append(retval, marshalledValue) @@ -226,7 +208,6 @@ func (arguments Arguments) PackValues(args []interface{}) ([]byte, error) { return arguments.Pack(args...) } - // Pack performs the operation Go format -> Hexdata func (arguments Arguments) Pack(args ...interface{}) ([]byte, error) { // Make sure arguments match up and pack them diff --git a/accounts/abi/unpackv2_test.go b/accounts/abi/unpackv2_test.go index d0074ff7b1..364a097625 100644 --- a/accounts/abi/unpackv2_test.go +++ b/accounts/abi/unpackv2_test.go @@ -57,7 +57,6 @@ func TestUnpackV2(t *testing.T) { } } - func TestMultiReturnWithArrayV2(t *testing.T) { const definition = `[{"name" : "multi", "outputs": [{"type": "uint64[3]"}, {"type": "uint64"}]}]` abi, err := JSON(strings.NewReader(definition)) From 08c5d4dd271c58385df94842f1b5700ca6ef181c Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Sat, 13 Jan 2018 15:12:52 +0100 Subject: [PATCH 06/11] accounts/abi: address review concerns --- accounts/abi/abi.go | 2 +- accounts/abi/abi_test.go | 2 +- accounts/abi/argument.go | 18 +- accounts/abi/unpackv2_test.go | 335 ---------------------------------- 4 files changed, 5 insertions(+), 352 deletions(-) delete mode 100644 accounts/abi/unpackv2_test.go diff --git a/accounts/abi/abi.go b/accounts/abi/abi.go index fd286c2c22..254b1f7fb4 100644 --- a/accounts/abi/abi.go +++ b/accounts/abi/abi.go @@ -142,5 +142,5 @@ func (abi *ABI) MethodById(sigdata []byte) (*Method, error) { return &method, nil } } - return nil, fmt.Errorf("ABI spec does not contain method signature in data: 0x%x", sigdata[:4]) + return nil, fmt.Errorf("no method with id: %#x", sigdata[:4]) } diff --git a/accounts/abi/abi_test.go b/accounts/abi/abi_test.go index e66828240a..35e0094ddd 100644 --- a/accounts/abi/abi_test.go +++ b/accounts/abi/abi_test.go @@ -691,7 +691,7 @@ func TestABI_MethodById(t *testing.T) { a := fmt.Sprintf("%v", m) m2, err := abi.MethodById(m.Id()) if err != nil { - t.Fatal(err) + t.Fatalf("Failed to look up ABI method: %v", err) } b := fmt.Sprintf("%v", m2) if a != b { diff --git a/accounts/abi/argument.go b/accounts/abi/argument.go index bdd0894f17..f171f4cc63 100644 --- a/accounts/abi/argument.go +++ b/accounts/abi/argument.go @@ -67,6 +67,7 @@ func (arguments Arguments) LengthNonIndexed() int { return out } +// NonIndexed returns the arguments with indexed arguments filtered out func (arguments Arguments) NonIndexed() Arguments { var ret []Argument for _, arg := range arguments { @@ -89,12 +90,10 @@ func (arguments Arguments) Unpack(v interface{}, data []byte) error { if reflect.Ptr != reflect.ValueOf(v).Kind() { return fmt.Errorf("abi: Unpack(non-pointer %T)", v) } - marshalledValues, err := arguments.UnpackValues(data) if err != nil { return err } - if arguments.isTuple() { return arguments.unpackTuple(v, marshalledValues) } @@ -162,11 +161,9 @@ func (arguments Arguments) unpackTuple(v interface{}, marshalledValues []interfa // unpackAtomic unpacks ( hexdata -> go ) a single value func (arguments Arguments) unpackAtomic(v interface{}, marshalledValues []interface{}) error { - if len(marshalledValues) != 1 { return fmt.Errorf("abi: wrong length, expected single value, got %d", len(marshalledValues)) } - elem := reflect.ValueOf(v).Elem() reflectValue := reflect.ValueOf(marshalledValues[0]) return set(elem, reflectValue, arguments.NonIndexed()[0]) @@ -176,24 +173,18 @@ func (arguments Arguments) unpackAtomic(v interface{}, marshalledValues []interf // without supplying a struct to unpack into. Instead, this method returns a list containing the // values. An atomic argument will be a list with one element. func (arguments Arguments) UnpackValues(data []byte) ([]interface{}, error) { - retval := make([]interface{}, 0, arguments.LengthNonIndexed()) - virtualArgs := 0 - for index, arg := range arguments.NonIndexed() { - marshalledValue, err := toGoType((index+virtualArgs)*32, arg.Type, data) - if arg.Type.T == ArrayTy { - //If we have a static array, like [3]uint256, these are coded as + // If we have a static array, like [3]uint256, these are coded as // just like uint256,uint256,uint256. // This means that we need to add two 'virtual' arguments when // we count the index from now on virtualArgs += arg.Type.Size - 1 } - if err != nil { return nil, err } @@ -202,7 +193,7 @@ func (arguments Arguments) UnpackValues(data []byte) ([]interface{}, error) { return retval, nil } -// UnpackValues performs the operation Go format -> Hexdata +// PackValues performs the operation Go format -> Hexdata // It is the semantic opposite of UnpackValues func (arguments Arguments) PackValues(args []interface{}) ([]byte, error) { return arguments.Pack(args...) @@ -215,7 +206,6 @@ func (arguments Arguments) Pack(args ...interface{}) ([]byte, error) { if len(args) != len(abiArgs) { return nil, fmt.Errorf("argument count mismatch: %d for %d", len(args), len(abiArgs)) } - // variable input is the output appended at the end of packed // output. This is used for strings and bytes types input. var variableInput []byte @@ -229,7 +219,6 @@ func (arguments Arguments) Pack(args ...interface{}) ([]byte, error) { inputOffset += 32 } } - var ret []byte for i, a := range args { input := abiArgs[i] @@ -238,7 +227,6 @@ 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 diff --git a/accounts/abi/unpackv2_test.go b/accounts/abi/unpackv2_test.go deleted file mode 100644 index 364a097625..0000000000 --- a/accounts/abi/unpackv2_test.go +++ /dev/null @@ -1,335 +0,0 @@ -// Copyright 2015 The go-ethereum Authors -// This file is part of the go-ethereum library. -// -// The go-ethereum library is free software: you can redistribute it and/or modify -// it under the terms of the GNU Lesser General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// The go-ethereum library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Lesser General Public License for more details. -// -// You should have received a copy of the GNU Lesser General Public License -// along with the go-ethereum library. If not, see . - -package abi - -import ( - "bytes" - "encoding/hex" - "fmt" - "math/big" - "reflect" - "strconv" - "strings" - "testing" - - "github.com/ethereum/go-ethereum/common" -) - -func TestUnpackV2(t *testing.T) { - for i, test := range unpackTests { - 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) - } - out, err := abi.Methods["method"].Outputs.UnpackValues(encb) - - if err != nil { - t.Fatal(err) - } - if len(test.err) != 0 { - // The new stuff doesn't have these types of errors - return - } - if !reflect.DeepEqual(test.want, out[0]) { - t.Errorf("test %d (%v) failed: expected %v, got %v", i, test.def, test.want, out[0]) - } - }) - } -} - -func TestMultiReturnWithArrayV2(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("000000000000000000000000000000000000000000000000000000000000000900000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000007")) - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000006")) - - out, err := abi.Methods["multi"].Outputs.UnpackValues(buff.Bytes()) - - ret1Exp := [3]uint64{9, 8, 7} - ret2Exp := uint64(6) - - if !reflect.DeepEqual(out[0], ret1Exp) { - t.Error("array result", out[0], "!= Expected", ret1Exp) - } - if out[1] != ret2Exp { - t.Error("int result", out[1], "!= Expected", ret2Exp) - } -} - -func TestUnmarshalV2(t *testing.T) { - const definition = `[ - { "name" : "int", "constant" : false, "outputs": [ { "type": "uint256" } ] }, - { "name" : "bool", "constant" : false, "outputs": [ { "type": "bool" } ] }, - { "name" : "bytes", "constant" : false, "outputs": [ { "type": "bytes" } ] }, - { "name" : "fixed", "constant" : false, "outputs": [ { "type": "bytes32" } ] }, - { "name" : "multi", "constant" : false, "outputs": [ { "type": "bytes" }, { "type": "bytes" } ] }, - { "name" : "intArraySingle", "constant" : false, "outputs": [ { "type": "uint256[3]" } ] }, - { "name" : "addressSliceSingle", "constant" : false, "outputs": [ { "type": "address[]" } ] }, - { "name" : "addressSliceDouble", "constant" : false, "outputs": [ { "name": "a", "type": "address[]" }, { "name": "b", "type": "address[]" } ] }, - { "name" : "mixedBytes", "constant" : true, "outputs": [ { "name": "a", "type": "bytes" }, { "name": "b", "type": "bytes32" } ] }]` - - abi, err := JSON(strings.NewReader(definition)) - if err != nil { - t.Fatal(err) - } - buff := new(bytes.Buffer) - - // marshall mixed bytes (mixedBytes) - p0Exp := common.Hex2Bytes("01020000000000000000") - p1Exp := common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000ddeeff") - - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000040")) - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000ddeeff")) - buff.Write(common.Hex2Bytes("000000000000000000000000000000000000000000000000000000000000000a")) - buff.Write(common.Hex2Bytes("0102000000000000000000000000000000000000000000000000000000000000")) - - mixedBytes, err := abi.Methods["mixedBytes"].Outputs.UnpackValues(buff.Bytes()) - if err != nil { - t.Error(err) - } else { - p0 := mixedBytes[0].([]byte) - p1 := mixedBytes[1].([32]byte) - if !bytes.Equal(p0, p0Exp) { - t.Errorf("unexpected value unpacked: want %x, got %x", p0Exp, p0) - } - - if !bytes.Equal(p1[:], p1Exp) { - t.Errorf("unexpected value unpacked: want %x, got %x", p1Exp, p1) - } - } - - // marshal int - integer, err := abi.Methods["int"].Outputs.UnpackValues(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000001")) - if err != nil { - t.Error(err) - } - if len(integer) == 0 { - t.Error("Expected one integer") - } - intval := integer[0].(*big.Int) - if intval == nil || intval.Cmp(big.NewInt(1)) != 0 { - t.Error("expected Int to be 1 got", intval) - } - - // marshal bool - boolreturns, err := abi.Methods["bool"].Outputs.UnpackValues(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000001")) - if err != nil { - t.Error(err) - } - boolval := boolreturns[0].(bool) - if !boolval { - t.Error("expected Bool to be true") - } - - // marshal dynamic bytes max length 32 - buff.Reset() - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000020")) - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000020")) - bytesOut := common.RightPadBytes([]byte("hello"), 32) - buff.Write(bytesOut) - - bytesreturns, err := abi.Methods["bytes"].Outputs.UnpackValues(buff.Bytes()) - - if err != nil { - t.Error(err) - } - bytesval := bytesreturns[0].([]byte) - if !bytes.Equal(bytesval, bytesOut) { - t.Errorf("expected %x got %x", bytesOut, bytesval) - } - - // marshall dynamic bytes max length 64 - buff.Reset() - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000020")) - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000040")) - bytesOut = common.RightPadBytes([]byte("hello"), 64) - buff.Write(bytesOut) - - bytesreturns, err = abi.Methods["bytes"].Outputs.UnpackValues(buff.Bytes()) - if err != nil { - t.Error(err) - } - bytesval = bytesreturns[0].([]byte) - if !bytes.Equal(bytesval, bytesOut) { - t.Errorf("expected %x got %x", bytesOut, bytesval) - } - - // marshall dynamic bytes max length 64 - buff.Reset() - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000020")) - buff.Write(common.Hex2Bytes("000000000000000000000000000000000000000000000000000000000000003f")) - bytesOut = common.RightPadBytes([]byte("hello"), 64) - buff.Write(bytesOut) - - bytesreturns, err = abi.Methods["bytes"].Outputs.UnpackValues(buff.Bytes()) - if err != nil { - t.Error(err) - } - bytesval = bytesreturns[0].([]byte) - - if !bytes.Equal(bytesval, bytesOut[:len(bytesOut)-1]) { - t.Errorf("expected %x got %x", bytesOut[:len(bytesOut)-1], bytesval) - } - // marshal dynamic bytes output empty (nil) - bytesreturns, err = abi.Methods["bytes"].Outputs.UnpackValues(nil) - if err == nil { - t.Error("expected error") - } - // marshal dynamic bytes output empty - buff.Reset() - bytesreturns, err = abi.Methods["bytes"].Outputs.UnpackValues(buff.Bytes()) - if err == nil { - t.Error("expected error") - } - - // marshal dynamic bytes length 5 - buff.Reset() - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000020")) - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000005")) - buff.Write(common.RightPadBytes([]byte("hello"), 32)) - - bytesreturns, err = abi.Methods["bytes"].Outputs.UnpackValues(buff.Bytes()) - if err != nil { - t.Error(err) - } - bytesval = bytesreturns[0].([]byte) - - if !bytes.Equal(bytesval, []byte("hello")) { - t.Errorf("expected %x got %x", bytesOut, bytesval) - } - - // marshal dynamic bytes length 5 - buff.Reset() - buff.Write(common.RightPadBytes([]byte("hello"), 32)) - - hashreturns, err := abi.Methods["fixed"].Outputs.UnpackValues(buff.Bytes()) - if err != nil { - t.Error(err) - } - hashval := hashreturns[0].([32]byte) - - helloHash := common.BytesToHash(common.RightPadBytes([]byte("hello"), 32)) - if common.Hash(hashval) != helloHash { - t.Errorf("Expected %x to equal %x", hashval, helloHash) - } - - // marshal error - buff.Reset() - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000020")) - - bytesreturns, err = abi.Methods["bytes"].Outputs.UnpackValues(buff.Bytes()) - if err == nil { - // Error abi: cannot marshal in to go slice: offset 32 would go over slice boundary (len=64) - t.Error("expected error") - } - bytesreturns, err = abi.Methods["multi"].Outputs.UnpackValues(make([]byte, 64)) - - buff.Reset() - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000001")) - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000002")) - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000003")) - // marshal int array - - intArrayReturns, err := abi.Methods["intArraySingle"].Outputs.UnpackValues(buff.Bytes()) - if err != nil { - t.Error(err) - } - intArray := intArrayReturns[0].([3]*big.Int) - - var testAgainstIntArray = [3]*big.Int{big.NewInt(1), big.NewInt(2), big.NewInt(3)} - - for i, intval := range intArray { - if intval.Cmp(testAgainstIntArray[i]) != 0 { - t.Errorf("expected %v, got %v", testAgainstIntArray[i], intval) - } - } - // marshal address slice - buff.Reset() - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000020")) // offset - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000001")) // size - buff.Write(common.Hex2Bytes("0000000000000000000000000100000000000000000000000000000000000000")) - - outAddrReturns, err := abi.Methods["addressSliceSingle"].Outputs.UnpackValues(buff.Bytes()) - if err != nil { - t.Fatal("didn't expect error:", err) - } - outAddr := outAddrReturns[0].([]common.Address) - if len(outAddr) != 1 { - t.Fatal("expected 1 item, got", len(outAddr)) - } - - if outAddr[0] != (common.Address{1}) { - t.Errorf("expected %x, got %x", common.Address{1}, outAddr[0]) - } - - // marshal multiple address slice - buff.Reset() - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000040")) // offset - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000080")) // offset - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000001")) // size - buff.Write(common.Hex2Bytes("0000000000000000000000000100000000000000000000000000000000000000")) - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000002")) // size - buff.Write(common.Hex2Bytes("0000000000000000000000000200000000000000000000000000000000000000")) - buff.Write(common.Hex2Bytes("0000000000000000000000000300000000000000000000000000000000000000")) - - outAddrStructReturns, err := abi.Methods["addressSliceDouble"].Outputs.UnpackValues(buff.Bytes()) - if err != nil { - t.Fatal("didn't expect error:", err) - } - A := outAddrStructReturns[0].([]common.Address) - B := outAddrStructReturns[1].([]common.Address) - - if len(A) != 1 { - t.Fatal("expected 1 item, got", len(A)) - } - - if A[0] != (common.Address{1}) { - t.Errorf("expected %x, got %x", common.Address{1}, A[0]) - } - - if len(B) != 2 { - t.Fatal("expected 1 item, got", len(B)) - } - - if B[0] != (common.Address{2}) { - t.Errorf("expected %x, got %x", common.Address{2}, B[0]) - } - if B[1] != (common.Address{3}) { - t.Errorf("expected %x, got %x", common.Address{3}, B[1]) - } - - // marshal invalid address slice - buff.Reset() - buff.Write(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000100")) - - err = abi.Unpack(&outAddr, "addressSliceSingle", buff.Bytes()) - _, err = abi.Methods["addressSliceSingle"].Outputs.UnpackValues(buff.Bytes()) - if err == nil { - t.Fatal("expected error:", err) - } - -} From bd6ed23899c8a3b4b6d0db29f0f6298e492cedd6 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Sat, 13 Jan 2018 16:03:24 +0100 Subject: [PATCH 07/11] accounts/abi: harden unpacking against malicious input --- accounts/abi/unpack.go | 17 ++++++--- accounts/abi/unpack_test.go | 70 +++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/accounts/abi/unpack.go b/accounts/abi/unpack.go index 3342456618..51fb9ab9b6 100644 --- a/accounts/abi/unpack.go +++ b/accounts/abi/unpack.go @@ -95,6 +95,9 @@ func readFixedBytes(t Type, word []byte) (interface{}, error) { // iteratively unpack elements func forEachUnpack(t Type, output []byte, start, size int) (interface{}, error) { + if size < 0 { + return nil, fmt.Errorf("cannot marshal input to array, size is negative (%d)", size) + } if start+32*size > len(output) { return nil, fmt.Errorf("abi: cannot marshal in to go array: offset %d would go over slice boundary (len=%d)", len(output), start+32*size) } @@ -181,16 +184,22 @@ func toGoType(index int, t Type, output []byte) (interface{}, error) { // interprets a 32 byte slice as an offset and then determines which indice to look to decode the type. func lengthPrefixPointsTo(index int, output []byte) (start int, length int, err error) { - offset := int(binary.BigEndian.Uint64(output[index+24 : index+32])) + offsetBig := big.NewInt(0).SetBytes(output[index : index+32]) + if !offsetBig.IsInt64() { + return 0, 0, fmt.Errorf("abi offset larger than int64: %v", offsetBig) + } + offset := int(offsetBig.Int64()) if offset+32 > len(output) { return 0, 0, fmt.Errorf("abi: cannot marshal in to go slice: offset %d would go over slice boundary (len=%d)", len(output), offset+32) } - length = int(binary.BigEndian.Uint64(output[offset+24 : offset+32])) + lengthBig := big.NewInt(0).SetBytes(output[offset : offset+32]) + if !lengthBig.IsInt64() { + return 0, 0, fmt.Errorf("abi length larger than int64: %v", lengthBig) + } + length = int(lengthBig.Int64()) if offset+32+length > len(output) { return 0, 0, fmt.Errorf("abi: cannot marshal in to go type: length insufficient %d require %d", len(output), offset+32+length) } start = offset + 32 - - //fmt.Printf("LENGTH PREFIX INFO: \nsize: %v\noffset: %v\nstart: %v\n", length, offset, start) return } diff --git a/accounts/abi/unpack_test.go b/accounts/abi/unpack_test.go index e9f9108125..742211244b 100644 --- a/accounts/abi/unpack_test.go +++ b/accounts/abi/unpack_test.go @@ -683,3 +683,73 @@ func TestUnmarshal(t *testing.T) { t.Fatal("expected error:", err) } } + +func TestOOMMaliciousInput(t *testing.T) { + oomTests := []unpackTest{ + { + def: `[{"type": "uint8[]"}]`, + enc: "0000000000000000000000000000000000000000000000000000000000000020" + // offset + "0000000000000000000000000000000000000000000000000000000000000003" + // num elems + "0000000000000000000000000000000000000000000000000000000000000001" + // elem 1 + "0000000000000000000000000000000000000000000000000000000000000002", // elem 2 + }, + { // Length larger than 64 bits + def: `[{"type": "uint8[]"}]`, + enc: "0000000000000000000000000000000000000000000000000000000000000020" + // offset + "00ffffffffffffffffffffffffffffffffffffffffffffff0000000000000002" + // num elems + "0000000000000000000000000000000000000000000000000000000000000001" + // elem 1 + "0000000000000000000000000000000000000000000000000000000000000002", // elem 2 + }, + { // Offset very large (over 64 bits) + def: `[{"type": "uint8[]"}]`, + enc: "00ffffffffffffffffffffffffffffffffffffffffffffff0000000000000020" + // offset + "0000000000000000000000000000000000000000000000000000000000000002" + // num elems + "0000000000000000000000000000000000000000000000000000000000000001" + // elem 1 + "0000000000000000000000000000000000000000000000000000000000000002", // elem 2 + }, + { // Offset very large (below 64 bits) + def: `[{"type": "uint8[]"}]`, + enc: "0000000000000000000000000000000000000000000000007ffffffffff00020" + // offset + "0000000000000000000000000000000000000000000000000000000000000002" + // num elems + "0000000000000000000000000000000000000000000000000000000000000001" + // elem 1 + "0000000000000000000000000000000000000000000000000000000000000002", // elem 2 + }, + { // Offset negative (as 64 bit) + def: `[{"type": "uint8[]"}]`, + enc: "000000000000000000000000000000000000000000000000f000000000000020" + // offset + "0000000000000000000000000000000000000000000000000000000000000002" + // num elems + "0000000000000000000000000000000000000000000000000000000000000001" + // elem 1 + "0000000000000000000000000000000000000000000000000000000000000002", // elem 2 + }, + + { // Negative length + def: `[{"type": "uint8[]"}]`, + enc: "0000000000000000000000000000000000000000000000000000000000000020" + // offset + "000000000000000000000000000000000000000000000000f000000000000002" + // num elems + "0000000000000000000000000000000000000000000000000000000000000001" + // elem 1 + "0000000000000000000000000000000000000000000000000000000000000002", // elem 2 + }, + { // Very large length + def: `[{"type": "uint8[]"}]`, + enc: "0000000000000000000000000000000000000000000000000000000000000020" + // offset + "0000000000000000000000000000000000000000000000007fffffffff000002" + // num elems + "0000000000000000000000000000000000000000000000000000000000000001" + // elem 1 + "0000000000000000000000000000000000000000000000000000000000000002", // elem 2 + }, + } + for i, test := range oomTests { + 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) + } + _, err = abi.Methods["method"].Outputs.UnpackValues(encb) + if err == nil { + t.Fatalf("Expected error on malicious input, test %d", i) + } + } +} From 61f2279bdeac595a4607080028715a8222db6cd4 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 2 Feb 2018 14:03:58 +0100 Subject: [PATCH 08/11] abi: fix missing method on go 1.7/1.8 --- accounts/abi/unpack.go | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/accounts/abi/unpack.go b/accounts/abi/unpack.go index 51fb9ab9b6..761c80edfd 100644 --- a/accounts/abi/unpack.go +++ b/accounts/abi/unpack.go @@ -184,22 +184,32 @@ func toGoType(index int, t Type, output []byte) (interface{}, error) { // interprets a 32 byte slice as an offset and then determines which indice to look to decode the type. func lengthPrefixPointsTo(index int, output []byte) (start int, length int, err error) { - offsetBig := big.NewInt(0).SetBytes(output[index : index+32]) - if !offsetBig.IsInt64() { - return 0, 0, fmt.Errorf("abi offset larger than int64: %v", offsetBig) + bigOffsetEnd := big.NewInt(0).SetBytes(output[index : index+32]) + bigOffsetEnd.Add(bigOffsetEnd, common.Big32) + outputLength := big.NewInt(int64(len(output))) + + if bigOffsetEnd.Cmp(outputLength) > 0 { + return 0, 0, fmt.Errorf("abi: cannot marshal in to go slice: offset %v would go over slice boundary (len=%v)", bigOffsetEnd, outputLength) } - offset := int(offsetBig.Int64()) - if offset+32 > len(output) { - return 0, 0, fmt.Errorf("abi: cannot marshal in to go slice: offset %d would go over slice boundary (len=%d)", len(output), offset+32) + + if bigOffsetEnd.BitLen() > 63 { + return 0, 0, fmt.Errorf("abi offset larger than int64: %v", bigOffsetEnd) } - lengthBig := big.NewInt(0).SetBytes(output[offset : offset+32]) - if !lengthBig.IsInt64() { - return 0, 0, fmt.Errorf("abi length larger than int64: %v", lengthBig) + + offsetEnd := int(bigOffsetEnd.Uint64()) + lengthBig := big.NewInt(0).SetBytes(output[offsetEnd-32 : offsetEnd]) + + totalSize := big.NewInt(0) + totalSize.Add(totalSize, bigOffsetEnd) + totalSize.Add(totalSize, lengthBig) + if totalSize.BitLen() > 63 { + return 0, 0, fmt.Errorf("abi length larger than int64: %v", totalSize) } - length = int(lengthBig.Int64()) - if offset+32+length > len(output) { - return 0, 0, fmt.Errorf("abi: cannot marshal in to go type: length insufficient %d require %d", len(output), offset+32+length) + + if totalSize.Cmp(outputLength) > 0 { + return 0, 0, fmt.Errorf("abi: cannot marshal in to go type: length insufficient %v require %v", outputLength, totalSize) } - start = offset + 32 + start = int(bigOffsetEnd.Uint64()) + length = int(lengthBig.Uint64()) return } From 01507d9b9d872aa6dfaf3ad704b8c2b65e378a41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Tue, 20 Feb 2018 14:33:34 +0200 Subject: [PATCH 09/11] cmd, console: support all termination signals --- cmd/geth/consolecmd.go | 3 ++- cmd/utils/cmd.go | 5 +++-- console/console.go | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/cmd/geth/consolecmd.go b/cmd/geth/consolecmd.go index 9d5cc38a1b..7eca4d59f6 100644 --- a/cmd/geth/consolecmd.go +++ b/cmd/geth/consolecmd.go @@ -22,6 +22,7 @@ import ( "os/signal" "path/filepath" "strings" + "syscall" "github.com/ethereum/go-ethereum/cmd/utils" "github.com/ethereum/go-ethereum/console" @@ -207,7 +208,7 @@ func ephemeralConsole(ctx *cli.Context) error { } // Wait for pending callbacks, but stop for Ctrl-C. abort := make(chan os.Signal, 1) - signal.Notify(abort, os.Interrupt) + signal.Notify(abort, syscall.SIGINT, syscall.SIGTERM) go func() { <-abort diff --git a/cmd/utils/cmd.go b/cmd/utils/cmd.go index 53cdf7861c..186d18d8f5 100644 --- a/cmd/utils/cmd.go +++ b/cmd/utils/cmd.go @@ -25,6 +25,7 @@ import ( "os/signal" "runtime" "strings" + "syscall" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" @@ -64,7 +65,7 @@ func StartNode(stack *node.Node) { } go func() { sigc := make(chan os.Signal, 1) - signal.Notify(sigc, os.Interrupt) + signal.Notify(sigc, syscall.SIGINT, syscall.SIGTERM) defer signal.Stop(sigc) <-sigc log.Info("Got interrupt, shutting down...") @@ -85,7 +86,7 @@ func ImportChain(chain *core.BlockChain, fn string) error { // If a signal is received, the import will stop at the next batch. interrupt := make(chan os.Signal, 1) stop := make(chan struct{}) - signal.Notify(interrupt, os.Interrupt) + signal.Notify(interrupt, syscall.SIGINT, syscall.SIGTERM) defer signal.Stop(interrupt) defer close(interrupt) go func() { diff --git a/console/console.go b/console/console.go index 52fe1f542a..b280d4e65d 100644 --- a/console/console.go +++ b/console/console.go @@ -26,6 +26,7 @@ import ( "regexp" "sort" "strings" + "syscall" "github.com/ethereum/go-ethereum/internal/jsre" "github.com/ethereum/go-ethereum/internal/web3ext" @@ -332,7 +333,7 @@ func (c *Console) Interactive() { }() // Monitor Ctrl-C too in case the input is empty and we need to bail abort := make(chan os.Signal, 1) - signal.Notify(abort, os.Interrupt) + signal.Notify(abort, syscall.SIGINT, syscall.SIGTERM) // Start sending prompts to the user and reading back inputs for { From 14c76371bab105a99c796a652df86df3e3e7d958 Mon Sep 17 00:00:00 2001 From: Dmitry Shulyak Date: Wed, 21 Feb 2018 16:03:26 +0200 Subject: [PATCH 10/11] p2p: when peer is removed remove it also from dial history (#16060) This change removes a peer information from dialing history when peer is removed from static list. It allows to force a server to re-dial concrete peer if it is needed. In our case we are running geth node on mobile devices, and it is common for a network connection to flap on mobile. Almost every time it flaps or network connection is changed from cellular to wifi peers are disconnected with read timeout. And usually it takes 30 seconds (default expiration timeout) to recover connection with static peers after connectivity is restored. This change allows us to reconnect with peers almost immediately and it seems harmless enough. --- p2p/dial.go | 13 +++++++++++++ p2p/dial_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/p2p/dial.go b/p2p/dial.go index f5ff2c2117..d8feceb9f3 100644 --- a/p2p/dial.go +++ b/p2p/dial.go @@ -154,6 +154,9 @@ func (s *dialstate) addStatic(n *discover.Node) { func (s *dialstate) removeStatic(n *discover.Node) { // This removes a task so future attempts to connect will not be made. delete(s.static, n.ID) + // This removes a previous dial timestamp so that application + // can force a server to reconnect with chosen peer immediately. + s.hist.remove(n.ID) } func (s *dialstate) newTasks(nRunning int, peers map[discover.NodeID]*Peer, now time.Time) []task { @@ -390,6 +393,16 @@ func (h dialHistory) min() pastDial { } func (h *dialHistory) add(id discover.NodeID, exp time.Time) { heap.Push(h, pastDial{id, exp}) + +} +func (h *dialHistory) remove(id discover.NodeID) bool { + for i, v := range *h { + if v.id == id { + heap.Remove(h, i) + return true + } + } + return false } func (h dialHistory) contains(id discover.NodeID) bool { for _, v := range h { diff --git a/p2p/dial_test.go b/p2p/dial_test.go index ad18ef9abe..2a7941fc65 100644 --- a/p2p/dial_test.go +++ b/p2p/dial_test.go @@ -515,6 +515,50 @@ func TestDialStateStaticDial(t *testing.T) { }) } +// This test checks that static peers will be redialed immediately if they were re-added to a static list. +func TestDialStaticAfterReset(t *testing.T) { + wantStatic := []*discover.Node{ + {ID: uintID(1)}, + {ID: uintID(2)}, + } + + rounds := []round{ + // Static dials are launched for the nodes that aren't yet connected. + { + peers: nil, + new: []task{ + &dialTask{flags: staticDialedConn, dest: &discover.Node{ID: uintID(1)}}, + &dialTask{flags: staticDialedConn, dest: &discover.Node{ID: uintID(2)}}, + }, + }, + // No new dial tasks, all peers are connected. + { + peers: []*Peer{ + {rw: &conn{flags: staticDialedConn, id: uintID(1)}}, + {rw: &conn{flags: staticDialedConn, id: uintID(2)}}, + }, + done: []task{ + &dialTask{flags: staticDialedConn, dest: &discover.Node{ID: uintID(1)}}, + &dialTask{flags: staticDialedConn, dest: &discover.Node{ID: uintID(2)}}, + }, + new: []task{ + &waitExpireTask{Duration: 30 * time.Second}, + }, + }, + } + dTest := dialtest{ + init: newDialState(wantStatic, nil, fakeTable{}, 0, nil), + rounds: rounds, + } + runDialTest(t, dTest) + for _, n := range wantStatic { + dTest.init.removeStatic(n) + dTest.init.addStatic(n) + } + // without removing peers they will be considered recently dialed + runDialTest(t, dTest) +} + // This test checks that past dials are not retried for some time. func TestDialStateCache(t *testing.T) { wantStatic := []*discover.Node{ From b585f761283957dcd1783a134449a01423145df3 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 21 Feb 2018 15:10:18 +0100 Subject: [PATCH 11/11] ethapi: prevent creating contract if no data is provided (#16108) * ethapi: prevent creating contract if no data is provided * internal/ethapi: downcase error for no data on contract creation --- internal/ethapi/api.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 314086335c..636d0bfe26 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1135,6 +1135,18 @@ func (args *SendTxArgs) setDefaults(ctx context.Context, b Backend) error { if args.Data != nil && args.Input != nil && !bytes.Equal(*args.Data, *args.Input) { return errors.New(`Both "data" and "input" are set and not equal. Please use "input" to pass transaction call data.`) } + if args.To == nil { + // Contract creation + var input []byte + if args.Data != nil { + input = *args.Data + } else if args.Input != nil { + input = *args.Input + } + if len(input) == 0 { + return errors.New(`contract creation without any data provided`) + } + } return nil }