Tags in accounts abi #16737

gballet · 2018-05-14T10:49:04Z

Because of a timing issue, #16648 didn't get merged properly. This is the fix.

holiman

LGTM if travis goes green

jebek29 · 2018-05-14T13:01:06Z

Duplicate of #16603

jebek29 · 2018-05-14T13:02:39Z

diff --git a/accounts/abi/argument.go b/accounts/abi/argument.go
index 512d8fdfa7..93b513c346 100644
--- a/accounts/abi/argument.go
+++ b/accounts/abi/argument.go
@@ -111,9 +111,14 @@ func (arguments Arguments) unpackTuple(v interface{}, marshalledValues []interfa
if err := requireUnpackKind(value, typ, kind, arguments); err != nil {
return err
}

// If the output interface is a struct, make sure names don't collide

// If the interface is a struct, get of abi->struct_field mapping
var abi2struct map[string]string
if kind == reflect.Struct {

  if err := requireUniqueStructFieldNames(arguments); err != nil {

```
  var err error
```

  abi2struct, err = mapAbiToStructFields(arguments, value)

```
  if err != nil {
  	return err
  }
```
}
@@ -123,9 +128,10 @@ func (arguments Arguments) unpackTuple(v interface{}, marshalledValues []interfa
```
  switch kind {
  case reflect.Struct:
```

  	err := unpackStruct(value, reflectValue, arg)

```
  	if err != nil {
```
```
  		return err
```

  	if structField, ok := abi2struct[arg.Name]; ok {

  		if err := set(value.FieldByName(structField), reflectValue, arg); err != nil {

```
  			return err
```

  		}
  	}
  case reflect.Slice, reflect.Array:
  	if value.Len() < i {

@@ -151,17 +157,22 @@ func (arguments Arguments) unpackAtomic(v interface{}, marshalledValues []interf
if len(marshalledValues) != 1 {
return fmt.Errorf("abi: wrong length, expected single value, got %d", len(marshalledValues))
}
+
elem := reflect.ValueOf(v).Elem()
kind := elem.Kind()
reflectValue := reflect.ValueOf(marshalledValues[0])

var abi2struct map[string]string
if kind == reflect.Struct {

```
  //make sure names don't collide
```

  if err := requireUniqueStructFieldNames(arguments); err != nil {

```
  var err error
```

  if abi2struct, err = mapAbiToStructFields(arguments, elem); err != nil {
  	return err
  }

  return unpackStruct(elem, reflectValue, arguments[0])

```
  arg := arguments.NonIndexed()[0]
```

  if structField, ok := abi2struct[arg.Name]; ok {

  	return set(elem.FieldByName(structField), reflectValue, arg)

```
  }
```
```
  return nil
```
}

return set(elem, reflectValue, arguments.NonIndexed()[0])
@@ -277,18 +288,3 @@ func capitalise(input string) string {
}
return strings.ToUpper(input[:1]) + input[1:]
}

-//unpackStruct extracts each argument into its corresponding struct field
-func unpackStruct(value, reflectValue reflect.Value, arg Argument) error {

name := capitalise(arg.Name)
typ := value.Type()
for j := 0; j < typ.NumField(); j++ {
```
  // TODO read tags: `abi:"fieldName"`
```
```
  if typ.Field(j).Name == name {
```

  	if err := set(value.Field(j), reflectValue, arg); err != nil {

```
  		return err
```
```
  	}
```
```
  }
```
}
return nil
-}
diff --git a/accounts/abi/event_test.go b/accounts/abi/event_test.go
index cca61e433d..3bfdd7c0ab 100644
--- a/accounts/abi/event_test.go
+++ b/accounts/abi/event_test.go
@@ -58,12 +58,28 @@ var jsonEventPledge = []byte({ "type": "event" })

+var jsonEventMixedCase = []byte(`{

"anonymous": false,
"inputs": [{

  "indexed": false, "name": "value", "type": "uint256"

```
}, {
```

  "indexed": false, "name": "_value", "type": "uint256"

```
}, {
```

  "indexed": false, "name": "Value", "type": "uint256"

}],
"name": "MixedCase",
"type": "event"
}`)

// 1000000
var transferData1 = "00000000000000000000000000000000000000000000000000000000000f4240"

// "0x00Ce0d46d924CC8437c806721496599FC3FFA268", 2218516807680, "usd"
var pledgeData1 = "00000000000000000000000000ce0d46d924cc8437c806721496599fc3ffa2680000000000000000000000000000000000000000000000000000020489e800007573640000000000000000000000000000000000000000000000000000000000"

+// 1000000,2218516807680,1000001
+var mixedCaseData1 = "00000000000000000000000000000000000000000000000000000000000f42400000000000000000000000000000000000000000000000000000020489e8000000000000000000000000000000000000000000000000000000000000000f4241"
+
func TestEventId(t *testing.T) {
var table = []struct {
definition string
@@ -121,6 +137,27 @@ func TestEventTupleUnpack(t *testing.T) {
Value *big.Int
}

type EventTransferWithTag struct {

  // this is valid because `value` is not exportable,

  // so value is only unmarshalled into `Value1`.

```
  value  *big.Int
```
```
  Value1 *big.Int `abi:"value"`
```
}
type BadEventTransferWithSameFieldAndTag struct {
```
  Value  *big.Int
```
```
  Value1 *big.Int `abi:"value"`
```
}
type BadEventTransferWithDuplicatedTag struct {
```
  Value1 *big.Int `abi:"value"`
```
```
  Value2 *big.Int `abi:"value"`
```
}
type BadEventTransferWithEmptyTag struct {
```
  Value *big.Int `abi:""`
```
}
type EventPledge struct {
Who common.Address
Wad *big.Int
@@ -133,9 +170,16 @@ func TestEventTupleUnpack(t *testing.T) {
Currency [3]byte
}
type EventMixedCase struct {
```
  Value1 *big.Int `abi:"value"`
```
```
  Value2 *big.Int `abi:"_value"`
```
```
  Value3 *big.Int `abi:"Value"`
```
}
bigint := new(big.Int)
bigintExpected := big.NewInt(1000000)
bigintExpected2 := big.NewInt(2218516807680)
bigintExpected3 := big.NewInt(1000001)
addr := common.HexToAddress("0x00Ce0d46d924CC8437c806721496599FC3FFA268")
var testCases = []struct {
data string
@@ -158,6 +202,34 @@ func TestEventTupleUnpack(t *testing.T) {
jsonEventTransfer,
"",
"Can unpack ERC20 Transfer event into slice",
}, {
```
  transferData1,
```
```
  &EventTransferWithTag{},
```

  &EventTransferWithTag{Value1: bigintExpected},

```
  jsonEventTransfer,
```
```
  "",
```

  "Can unpack ERC20 Transfer event into structure with abi: tag",

}, {
```
  transferData1,
```

  &BadEventTransferWithDuplicatedTag{},

  &BadEventTransferWithDuplicatedTag{},

```
  jsonEventTransfer,
```

  "struct: abi tag in 'Value2' already mapped",

  "Can not unpack ERC20 Transfer event with duplicated abi tag",

}, {
```
  transferData1,
```

  &BadEventTransferWithSameFieldAndTag{},

  &BadEventTransferWithSameFieldAndTag{},

```
  jsonEventTransfer,
```

  "abi: multiple variables maps to the same abi field 'value'",

  "Can not unpack ERC20 Transfer event with a field and a tag mapping to the same abi variable",

}, {
```
  transferData1,
```
```
  &BadEventTransferWithEmptyTag{},
```
```
  &BadEventTransferWithEmptyTag{},
```
```
  jsonEventTransfer,
```

  "struct: abi tag in 'Value' is empty",

```
  "Can not unpack ERC20 Transfer event with an empty tag",
```
}, {
pledgeData1,
&EventPledge{},
@@ -216,6 +288,13 @@ func TestEventTupleUnpack(t *testing.T) {
jsonEventPledge,
"abi: cannot unmarshal tuple into map[string]interface {}",
"Can not unpack Pledge event into map",
}, {
```
  mixedCaseData1,
```
```
  &EventMixedCase{},
```

  &EventMixedCase{Value1: bigintExpected, Value2: bigintExpected2, Value3: bigintExpected3},

```
  jsonEventMixedCase,
```
```
  "",
```
```
  "Can unpack abi variables with mixed case",
```
}}

for _, tc := range testCases {
@@ -227,7 +306,7 @@ func TestEventTupleUnpack(t *testing.T) {
assert.Nil(err, "Should be able to unpack event data.")
assert.Equal(tc.expected, tc.dest, tc.name)
} else {

```
  		assert.EqualError(err, tc.error)
```

```
  		assert.EqualError(err, tc.error, tc.name)
  	}
  })
```
}
diff --git a/accounts/abi/reflect.go b/accounts/abi/reflect.go
index 5620a70845..0193517a42 100644
--- a/accounts/abi/reflect.go
+++ b/accounts/abi/reflect.go
@@ -19,6 +19,7 @@ package abi
import (
"fmt"
"reflect"
"strings"
)

// indirect recursively dereferences the value until it either gets the value
@@ -111,18 +112,101 @@ func requireUnpackKind(v reflect.Value, t reflect.Type, k reflect.Kind,
return nil
}

-// requireUniqueStructFieldNames makes sure field names don't collide
-func requireUniqueStructFieldNames(args Arguments) error {

exists := make(map[string]bool)
+// mapAbiToStringField maps abi to struct fields.
+// first round: for each Exportable field that contains a abi:"" tag
+// and this field name exists in the arguments, pair them together.
+// second round: for each argument field that has not been already linked,
+// find what variable is expected to be mapped into, if it exists and has not been
+// used, pair them.
+func mapAbiToStructFields(args Arguments, value reflect.Value) (map[string]string, error) {

typ := value.Type()
abi2struct := make(map[string]string)
struct2abi := make(map[string]string)
// first round ~~~
for i := 0; i < typ.NumField(); i++ {
```
  structFieldName := typ.Field(i).Name
```
```
  // skip private struct fields.
```

  if structFieldName[:1] != strings.ToUpper(structFieldName[:1]) {

```
  	continue
```
```
  }
```

  // skip fields that have no abi:"" tag.

```
  var ok bool
```
```
  var tagName string
```

  if tagName, ok = typ.Field(i).Tag.Lookup("abi"); !ok {

```
  	continue
```
```
  }
```
```
  // check if tag is empty.
```
```
  if tagName == "" {
```

  	return nil, fmt.Errorf("struct: abi tag in '%s' is empty", structFieldName)

```
  }
```

  // check which argument field matches with the abi tag.

```
  found := false
```

  for _, abiField := range args.NonIndexed() {

```
  	if abiField.Name == tagName {
```

  		if abi2struct[abiField.Name] != "" {

  			return nil, fmt.Errorf("struct: abi tag in '%s' already mapped", structFieldName)

```
  		}
```
```
  		// pair them
```

  		abi2struct[abiField.Name] = structFieldName

  		struct2abi[structFieldName] = abiField.Name

```
  		found = true
```
```
  	}
```
```
  }
```

  // check if this tag has been mapped.

```
  if !found {
```

  	return nil, fmt.Errorf("struct: abi tag '%s' defined but not found in abi", tagName)

```
  }
```
}
// second round ~~~
for _, arg := range args {

```
  field := capitalise(arg.Name)
```
```
  if field == "" {
```

  	return fmt.Errorf("abi: purely underscored output cannot unpack to struct")

```
  abiFieldName := arg.Name
```

  structFieldName := capitalise(abiFieldName)

```
  if structFieldName == "" {
```

  	return nil, fmt.Errorf("abi: purely underscored output cannot unpack to struct")
  }

```
  if exists[field] {
```

  	return fmt.Errorf("abi: multiple outputs mapping to the same struct field '%s'", field)

  // this abi has already been paired, skip it... unless there exists another, yet unassigned

  // struct field with the same field name. If so, raise an error:

```
  //    abi: [ { "name": "value" } ]
```

  //    struct { Value  *big.Int , Value1 *big.Int `abi:"value"`}

```
  if abi2struct[abiFieldName] != "" {
```

  	if abi2struct[abiFieldName] != structFieldName &&

  		struct2abi[structFieldName] == "" &&

  		value.FieldByName(structFieldName).IsValid() {

  		return nil, fmt.Errorf("abi: multiple variables maps to the same abi field '%s'", abiFieldName)

```
  	}
```
```
  	continue
  }
```

```
  exists[field] = true
```

  // return an error if this struct field has already been paired.

  if struct2abi[structFieldName] != "" {

  	return nil, fmt.Errorf("abi: multiple outputs mapping to the same struct field '%s'", structFieldName)

```
  }
```

  if value.FieldByName(structFieldName).IsValid() {

```
  	// pair them
```

  	abi2struct[abiFieldName] = structFieldName

  	struct2abi[structFieldName] = abiFieldName

```
  } else {
```

  	// not paired, but annotate as used, to detect cases like

  	//   abi : [ { "name": "value" }, { "name": "_value" } ]

```
  	//   struct { Value *big.Int }
```

  	struct2abi[structFieldName] = abiFieldName

```
  }
```
}

return nil

return abi2struct, nil
}

adriamb and others added 3 commits May 14, 2018 12:47

accounts/abi: allow abi: tags when unpacking structs

434f627

accounts/abi: fix comments

25b4e88

accounts/abi: Fix the last grammar comment

0862a58

gballet requested a review from holiman May 14, 2018 10:49

holiman approved these changes May 14, 2018

View reviewed changes

gballet merged commit 247b5f0 into ethereum:master May 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tags in accounts abi #16737

Tags in accounts abi #16737

gballet commented May 14, 2018

holiman left a comment

jebek29 commented May 14, 2018

jebek29 commented May 14, 2018

Tags in accounts abi #16737

Tags in accounts abi #16737

Conversation

gballet commented May 14, 2018

holiman left a comment

Choose a reason for hiding this comment

jebek29 commented May 14, 2018

jebek29 commented May 14, 2018