Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

accounts/abi: allow abi: tags when unpacking structs #16648

Merged
merged 3 commits into from
May 14, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 20 additions & 24 deletions accounts/abi/argument.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand All @@ -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 {
Expand All @@ -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])
Expand Down Expand Up @@ -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
}
81 changes: 80 additions & 1 deletion accounts/abi/event_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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{},
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
})
}
Expand Down
104 changes: 94 additions & 10 deletions accounts/abi/reflect.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package abi
import (
"fmt"
"reflect"
"strings"
)

// indirect recursively dereferences the value until it either gets the value
Expand Down Expand Up @@ -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... unless exists another not still assigned
Copy link
Member

Choose a reason for hiding this comment

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

apparently I missed this one:
unless there exists another, yet unassigned...

Copy link
Author

Choose a reason for hiding this comment

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

Haha, thanks for the grammar checks @gballet , I'll change it! [ and I annotate to use a grammar checker next time for the comments :) ]

I have no strong opinion about the default value. No problem about implementing it. In which use case are you thinking about? A generic event / function argument parser?

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking about default values for API calls, but it doesn't really make sense actually because that would be a Go-only behavior. Just fix the last grammar thing. @holiman and I will discuss the default value topic, but I don't think it's necessary after all.

Copy link
Contributor

Choose a reason for hiding this comment

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

discuss the default value topic, but I don't think it's necessary after all.

I agree

Copy link
Member

Choose a reason for hiding this comment

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

indeed so @AdriaMB please fix the last grammar issue and we'll merge your PR.

Copy link
Member

Choose a reason for hiding this comment

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

nvm, I just did

// struct field with the same field name, in the case 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] != "" {
Copy link
Member

Choose a reason for hiding this comment

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

could you move this before line 181? It would simplify the condition check on line 184

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
}