From 73d4a57d47d3381faa0516b319fa5598e71681f9 Mon Sep 17 00:00:00 2001
From: Martin Holst Swende <martin@swende.se>
Date: Thu, 21 Dec 2017 10:26:30 +0100
Subject: [PATCH] acounts/abi: refactor abi, generalize abi pack/unpack to
 Arguments

---
 accounts/abi/abi.go        |  39 ++++-----
 accounts/abi/abi_test.go   |  10 ++-
 accounts/abi/argument.go   | 161 +++++++++++++++++++++++++++++++++++++
 accounts/abi/event.go      |  89 +-------------------
 accounts/abi/event_test.go |  75 +++++++++++++++++
 accounts/abi/method.go     | 128 +----------------------------
 accounts/abi/unpack.go     |   9 ---
 7 files changed, 260 insertions(+), 251 deletions(-)

diff --git a/accounts/abi/abi.go b/accounts/abi/abi.go
index 6170c7062..7229a67bf 100644
--- a/accounts/abi/abi.go
+++ b/accounts/abi/abi.go
@@ -50,25 +50,25 @@ func JSON(reader io.Reader) (ABI, error) {
 // methods string signature. (signature = baz(uint32,string32))
 func (abi ABI) Pack(name string, args ...interface{}) ([]byte, error) {
 	// Fetch the ABI of the requested method
-	var method Method
-
 	if name == "" {
-		method = abi.Constructor
-	} else {
-		m, exist := abi.Methods[name]
-		if !exist {
-			return nil, fmt.Errorf("method '%s' not found", name)
+		// constructor
+		arguments, err := abi.Constructor.Inputs.Pack(args...)
+		if err != nil {
+			return nil, err
 		}
-		method = m
+		return arguments, nil
+
 	}
-	arguments, err := method.pack(args...)
+	method, exist := abi.Methods[name]
+	if !exist {
+		return nil, fmt.Errorf("method '%s' not found", name)
+	}
+
+	arguments, err := method.Inputs.Pack(args...)
 	if err != nil {
 		return nil, err
 	}
 	// Pack up the method ID too if not a constructor and return
-	if name == "" {
-		return arguments, nil
-	}
 	return append(method.Id(), arguments...), nil
 }
 
@@ -77,26 +77,17 @@ func (abi ABI) Unpack(v interface{}, name string, output []byte) (err error) {
 	if len(output) == 0 {
 		return fmt.Errorf("abi: unmarshalling empty output")
 	}
-
 	// since there can't be naming collisions with contracts and events,
 	// we need to decide whether we're calling a method or an event
-	var unpack unpacker
 	if method, ok := abi.Methods[name]; ok {
 		if len(output)%32 != 0 {
 			return fmt.Errorf("abi: improperly formatted output")
 		}
-		unpack = method
+		return method.Outputs.Unpack(v, output)
 	} else if event, ok := abi.Events[name]; ok {
-		unpack = event
-	} else {
-		return fmt.Errorf("abi: could not locate named method or event")
-	}
-
-	// requires a struct to unpack into for a tuple return...
-	if unpack.isTupleReturn() {
-		return unpack.tupleUnpack(v, output)
+		return event.Inputs.Unpack(v, output)
 	}
-	return unpack.singleUnpack(v, output)
+	return fmt.Errorf("abi: could not locate named method or event")
 }
 
 // UnmarshalJSON implements json.Unmarshaler interface
diff --git a/accounts/abi/abi_test.go b/accounts/abi/abi_test.go
index 644a388e3..1ae351730 100644
--- a/accounts/abi/abi_test.go
+++ b/accounts/abi/abi_test.go
@@ -22,7 +22,6 @@ import (
 	"fmt"
 	"log"
 	"math/big"
-	"reflect"
 	"strings"
 	"testing"
 
@@ -75,9 +74,12 @@ func TestReader(t *testing.T) {
 	}
 
 	// deep equal fails for some reason
-	t.Skip()
-	if !reflect.DeepEqual(abi, exp) {
-		t.Errorf("\nabi: %v\ndoes not match exp: %v", abi, exp)
+	//t.Skip()
+	// Check with String() instead
+	expS := fmt.Sprintf("%v",exp)
+	gotS := fmt.Sprintf("%v", abi)
+	if expS != gotS {
+		t.Errorf("\nGot abi: \n%v\ndoes not match expected \n%v", abi, exp)
 	}
 }
 
diff --git a/accounts/abi/argument.go b/accounts/abi/argument.go
index 59bcc117c..c41c2c6b0 100644
--- a/accounts/abi/argument.go
+++ b/accounts/abi/argument.go
@@ -19,6 +19,8 @@ package abi
 import (
 	"encoding/json"
 	"fmt"
+	"reflect"
+	"strings"
 )
 
 // Argument holds the name of the argument and the corresponding type.
@@ -29,6 +31,8 @@ type Argument struct {
 	Indexed bool // indexed is only used by events
 }
 
+type Arguments []Argument
+
 // UnmarshalJSON implements json.Unmarshaler interface
 func (a *Argument) UnmarshalJSON(data []byte) error {
 	var extarg struct {
@@ -60,3 +64,160 @@ func countNonIndexedArguments(args []Argument) int {
 	}
 	return out
 }
+func (a *Arguments) isTuple() bool {
+	return a != nil && len(*a) > 1
+}
+
+func (a *Arguments) Unpack(v interface{}, data []byte) error {
+	if a.isTuple() {
+		return a.unpackTuple(v, data)
+	}
+	return a.unpackAtomic(v, data)
+}
+
+func (a *Arguments) unpackTuple(v interface{}, output []byte) error {
+	// make sure the passed value is a pointer
+	valueOf := reflect.ValueOf(v)
+	if reflect.Ptr != valueOf.Kind() {
+		return fmt.Errorf("abi: Unpack(non-pointer %T)", v)
+	}
+
+	var (
+		value = valueOf.Elem()
+		typ   = value.Type()
+		kind  = value.Kind()
+	)
+/* !TODO add this back
+	if err := requireUnpackKind(value, typ, kind, (*a), false); err != nil {
+		return err
+	}
+*/
+	// `i` counts the nonindexed arguments.
+	// `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(*a) {
+
+		if arg.Indexed {
+			// can't read, continue
+			continue
+		}
+		i++
+		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)
+
+		switch kind {
+		case reflect.Struct:
+			for j := 0; j < typ.NumField(); j++ {
+				field := typ.Field(j)
+				// TODO read tags: `abi:"fieldName"`
+				if field.Name == strings.ToUpper(arg.Name[:1])+arg.Name[1:] {
+					if err := set(value.Field(j), reflectValue, arg); err != nil {
+						return err
+					}
+				}
+			}
+		case reflect.Slice, reflect.Array:
+			if value.Len() < i {
+				return fmt.Errorf("abi: insufficient number of arguments for unpack, want %d, got %d", len(*a), value.Len())
+			}
+			v := value.Index(i)
+			if err := requireAssignable(v, reflectValue); err != nil {
+				return err
+			}
+			reflectValue := reflect.ValueOf(marshalledValue)
+			return set(v.Elem(), reflectValue, arg)
+		default:
+			return fmt.Errorf("abi:[2] cannot unmarshal tuple in to %v", typ)
+		}
+	}
+	return nil
+}
+
+func (a *Arguments) unpackAtomic(v interface{}, output []byte) error {
+	// make sure the passed value is a pointer
+	valueOf := reflect.ValueOf(v)
+	if reflect.Ptr != valueOf.Kind() {
+		return fmt.Errorf("abi: Unpack(non-pointer %T)", v)
+	}
+	arg := (*a)[0]
+	if arg.Indexed {
+		return fmt.Errorf("abi: attempting to unpack indexed variable into element.")
+	}
+
+	value := valueOf.Elem()
+
+	marshalledValue, err := toGoType(0, arg.Type, output)
+	if err != nil {
+		return err
+	}
+	if err := set(value, reflect.ValueOf(marshalledValue), arg); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (arguments *Arguments) Pack(args ...interface{}) ([]byte, error) {
+	// Make sure arguments match up and pack them
+	if arguments == nil {
+		return nil, fmt.Errorf("arguments are nil, programmer error!")
+	}
+
+	abiArgs := *arguments
+	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
+
+	// 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
+		}
+	}
+
+	var ret []byte
+	for i, a := range args {
+		input := abiArgs[i]
+		// pack the input
+		packed, err := input.Type.pack(reflect.ValueOf(a))
+		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)
+			// 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.
+			variableInput = append(variableInput, packed...)
+		} else {
+			// append the packed value to the input
+			ret = append(ret, packed...)
+		}
+	}
+	// append the variable input at the end of the packed input
+	ret = append(ret, variableInput...)
+
+	return ret, nil
+}
diff --git a/accounts/abi/event.go b/accounts/abi/event.go
index 3d4e0b63c..726bac90e 100644
--- a/accounts/abi/event.go
+++ b/accounts/abi/event.go
@@ -18,7 +18,6 @@ package abi
 
 import (
 	"fmt"
-	"reflect"
 	"strings"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -31,7 +30,7 @@ import (
 type Event struct {
 	Name      string
 	Anonymous bool
-	Inputs    []Argument
+	Inputs    Arguments
 }
 
 // Id returns the canonical representation of the event's signature used by the
@@ -45,89 +44,3 @@ func (e Event) Id() common.Hash {
 	}
 	return common.BytesToHash(crypto.Keccak256([]byte(fmt.Sprintf("%v(%v)", e.Name, strings.Join(types, ",")))))
 }
-
-// unpacks an event return tuple into a struct of corresponding go types
-//
-// Unpacking can be done into a struct or a slice/array.
-func (e Event) tupleUnpack(v interface{}, output []byte) error {
-	// make sure the passed value is a pointer
-	valueOf := reflect.ValueOf(v)
-	if reflect.Ptr != valueOf.Kind() {
-		return fmt.Errorf("abi: Unpack(non-pointer %T)", v)
-	}
-
-	var (
-		value = valueOf.Elem()
-		typ   = value.Type()
-		kind  = value.Kind()
-	)
-	if err := requireUnpackKind(value, typ, kind, e.Inputs, true); err != nil {
-		return err
-	}
-
-	// `i` counts the nonindexed arguments.
-	// `j` counts the number of complex types.
-	// both `i` and `j` are used to to correctly compute `data` offset.
-	i, j := -1, 0
-	for _, input := range e.Inputs {
-		if input.Indexed {
-			// Indexed arguments are not packed into data
-			continue
-		}
-		i++
-		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 kind {
-		case reflect.Struct:
-			for j := 0; j < typ.NumField(); j++ {
-				field := typ.Field(j)
-				// TODO read tags: `abi:"fieldName"`
-				if field.Name == strings.ToUpper(input.Name[:1])+input.Name[1:] {
-					if err := set(value.Field(j), reflectValue, input); err != nil {
-						return err
-					}
-				}
-			}
-		case reflect.Slice, reflect.Array:
-			v := value.Index(i)
-			if err := requireAssignable(v, reflectValue); err != nil {
-				return err
-			}
-			if err := set(v.Elem(), reflectValue, input); err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
-func (e Event) isTupleReturn() bool { return len(e.Inputs) > 1 }
-
-func (e Event) singleUnpack(v interface{}, output []byte) error {
-	// make sure the passed value is a pointer
-	valueOf := reflect.ValueOf(v)
-	if reflect.Ptr != valueOf.Kind() {
-		return fmt.Errorf("abi: Unpack(non-pointer %T)", v)
-	}
-
-	if e.Inputs[0].Indexed {
-		return fmt.Errorf("abi: attempting to unpack indexed variable into element")
-	}
-
-	value := valueOf.Elem()
-
-	marshalledValue, err := toGoType(0, e.Inputs[0].Type, output)
-	if err != nil {
-		return err
-	}
-	return set(value, reflect.ValueOf(marshalledValue), e.Inputs[0])
-}
diff --git a/accounts/abi/event_test.go b/accounts/abi/event_test.go
index 34c7eebde..93740f6a5 100644
--- a/accounts/abi/event_test.go
+++ b/accounts/abi/event_test.go
@@ -241,3 +241,78 @@ func unpackTestEventData(dest interface{}, hexData string, jsonEvent []byte, ass
 	a := ABI{Events: map[string]Event{"e": e}}
 	return a.Unpack(dest, "e", data)
 }
+
+/*
+!TODO enable these when the fix is in. Taken from
+https://github.com/ethereum/go-ethereum/pull/15568
+
+*/
+/*
+type testResult struct {
+	Values [2]*big.Int
+	Value1 *big.Int
+	Value2 *big.Int
+}
+
+type testCase struct {
+	definition string
+	want       testResult
+}
+
+func (tc testCase) encoded(intType, arrayType Type) []byte {
+	var b bytes.Buffer
+	if tc.want.Value1 != nil {
+		val, _ := intType.pack(reflect.ValueOf(tc.want.Value1))
+		b.Write(val)
+	}
+
+	if !reflect.DeepEqual(tc.want.Values, [2]*big.Int{nil, nil}) {
+		val, _ := arrayType.pack(reflect.ValueOf(tc.want.Values))
+		b.Write(val)
+	}
+	if tc.want.Value2 != nil {
+		val, _ := intType.pack(reflect.ValueOf(tc.want.Value2))
+		b.Write(val)
+	}
+	return b.Bytes()
+}
+
+// TestEventUnpackIndexed verifies that indexed field will be skipped by event decoder.
+func TestEventUnpackIndexed(t *testing.T) {
+	definition := `[{"name": "test", "type": "event", "inputs": [{"indexed": true, "name":"value1", "type":"uint8"},{"indexed": false, "name":"value2", "type":"uint8"}]}]`
+	type testStruct struct {
+		Value1 uint8
+		Value2 uint8
+	}
+	abi, err := JSON(strings.NewReader(definition))
+	require.NoError(t, err)
+	var b bytes.Buffer
+	b.Write(packNum(reflect.ValueOf(uint8(8))))
+	var rst testStruct
+	require.NoError(t, abi.Unpack(&rst, "test", b.Bytes()))
+	require.Equal(t, uint8(0), rst.Value1)
+	require.Equal(t, uint8(8), rst.Value2)
+}
+
+// TestEventIndexedWithArrayUnpack verifies that decoder will not overlow when static array is indexed input.
+func TestEventIndexedWithArrayUnpack(t *testing.T) {
+	definition := `[{"name": "test", "type": "event", "inputs": [{"indexed": true, "name":"value1", "type":"uint8[2]"},{"indexed": false, "name":"value2", "type":"string"}]}]`
+	type testStruct struct {
+		Value1 [2]uint8
+		Value2 string
+	}
+	abi, err := JSON(strings.NewReader(definition))
+	require.NoError(t, err)
+	var b bytes.Buffer
+	stringOut := "abc"
+	// number of fields that will be encoded * 32
+	b.Write(packNum(reflect.ValueOf(32)))
+	b.Write(packNum(reflect.ValueOf(len(stringOut))))
+	b.Write(common.RightPadBytes([]byte(stringOut), 32))
+	fmt.Println(b.Bytes())
+	var rst testStruct
+	require.NoError(t, abi.Unpack(&rst, "test", b.Bytes()))
+	require.Equal(t, [2]uint8{0, 0}, rst.Value1)
+	require.Equal(t, stringOut, rst.Value2)
+}
+*/
diff --git a/accounts/abi/method.go b/accounts/abi/method.go
index 0df7f618e..f434ffdbe 100644
--- a/accounts/abi/method.go
+++ b/accounts/abi/method.go
@@ -18,7 +18,6 @@ package abi
 
 import (
 	"fmt"
-	"reflect"
 	"strings"
 
 	"github.com/ethereum/go-ethereum/crypto"
@@ -35,131 +34,8 @@ import (
 type Method struct {
 	Name    string
 	Const   bool
-	Inputs  []Argument
-	Outputs []Argument
-}
-
-func (method Method) pack(args ...interface{}) ([]byte, error) {
-	// Make sure arguments match up and pack them
-	if len(args) != len(method.Inputs) {
-		return nil, fmt.Errorf("argument count mismatch: %d for %d", len(args), len(method.Inputs))
-	}
-	// variable input is the output appended at the end of packed
-	// 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]
-		// pack the input
-		packed, err := input.Type.pack(reflect.ValueOf(a))
-		if err != nil {
-			return nil, fmt.Errorf("`%s` %v", method.Name, err)
-		}
-
-		// check for a slice type (string, bytes, slice)
-		if input.Type.requiresLengthPrefix() {
-			// calculate the offset
-			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
-			// will be appended at the end of the input.
-			variableInput = append(variableInput, packed...)
-		} else {
-			// append the packed value to the input
-			ret = append(ret, packed...)
-		}
-	}
-	// append the variable input at the end of the packed input
-	ret = append(ret, variableInput...)
-
-	return ret, nil
-}
-
-// unpacks a method return tuple into a struct of corresponding go types
-//
-// Unpacking can be done into a struct or a slice/array.
-func (method Method) tupleUnpack(v interface{}, outputSlice []byte) error {
-	// make sure the passed value is a pointer
-	valueOf := reflect.ValueOf(v)
-	if reflect.Ptr != valueOf.Kind() {
-		return fmt.Errorf("abi: Unpack(non-pointer %T)", v)
-	}
-
-	var (
-		value = valueOf.Elem()
-		typ   = value.Type()
-		kind  = value.Kind()
-	)
-	if err := requireUnpackKind(value, typ, kind, method.Outputs, false); err != nil {
-		return err
-	}
-
-	j := 0
-	for i, output := range method.Outputs {
-		marshalledValue, err := toGoType((i+j)*32, ouptut.Type, outputSlice)
-		if err != nil {
-			return err
-		}
-		if output.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 += output.Type.Size - 1
-		}
-		reflectValue := reflect.ValueOf(marshalledValue)
-
-		switch kind {
-		case reflect.Struct:
-			for j := 0; j < typ.NumField(); j++ {
-				field := typ.Field(j)
-				// TODO read tags: `abi:"fieldName"`
-				if field.Name == strings.ToUpper(output.Name[:1])+output.Name[1:] {
-					if err := set(value.Field(j), reflectValue, output); err != nil {
-						return err
-					}
-				}
-			}
-		case reflect.Slice, reflect.Array:
-			v := value.Index(i)
-			if err := requireAssignable(v, reflectValue); err != nil {
-				return err
-			}
-			if err := set(v.Elem(), reflectValue, output); err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
-func (method Method) isTupleReturn() bool { return len(method.Outputs) > 1 }
-
-func (method Method) singleUnpack(v interface{}, output []byte) error {
-	// make sure the passed value is a pointer
-	valueOf := reflect.ValueOf(v)
-	if reflect.Ptr != valueOf.Kind() {
-		return fmt.Errorf("abi: Unpack(non-pointer %T)", v)
-	}
-
-	value := valueOf.Elem()
-
-	marshalledValue, err := toGoType(0, method.Outputs[0].Type, output)
-	if err != nil {
-		return err
-	}
-	return set(value, reflect.ValueOf(marshalledValue), method.Outputs[0])
+	Inputs  Arguments
+	Outputs Arguments
 }
 
 // Sig returns the methods string signature according to the ABI spec.
diff --git a/accounts/abi/unpack.go b/accounts/abi/unpack.go
index 377aee874..80efb3f7e 100644
--- a/accounts/abi/unpack.go
+++ b/accounts/abi/unpack.go
@@ -25,15 +25,6 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 )
 
-// unpacker is a utility interface that enables us to have
-// abstraction between events and methods and also to properly
-// "unpack" them; e.g. events use Inputs, methods use Outputs.
-type unpacker interface {
-	tupleUnpack(v interface{}, output []byte) error
-	singleUnpack(v interface{}, output []byte) error
-	isTupleReturn() bool
-}
-
 // reads the integer based on its kind
 func readInteger(kind reflect.Kind, b []byte) interface{} {
 	switch kind {
-- 
GitLab