From e530059c703982a8324890e31632be6d9e5daf4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Fri, 29 Dec 2017 15:12:21 +0200 Subject: [PATCH] accounts/abi: handle collinding outputs for struct unpacks --- accounts/abi/argument.go | 25 ++++++++++++++++++++--- accounts/abi/bind/bind.go | 10 ++++++++- accounts/abi/bind/bind_test.go | 37 ++++++++++++++++++++++++++++------ accounts/abi/unpack_test.go | 36 +++++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 10 deletions(-) diff --git a/accounts/abi/argument.go b/accounts/abi/argument.go index 4a5d03186e..5cfce61482 100644 --- a/accounts/abi/argument.go +++ b/accounts/abi/argument.go @@ -96,6 +96,17 @@ func (arguments Arguments) unpackTuple(v interface{}, output []byte) error { 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 kind == reflect.Struct { + exists := make(map[string]bool) + for _, arg := range arguments { + field := capitalise(arg.Name) + if exists[field] { + return fmt.Errorf("abi: multiple outputs mapping to the same struct field '%s'", field) + } + 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. @@ -123,11 +134,10 @@ func (arguments Arguments) unpackTuple(v interface{}, output []byte) error { switch kind { case reflect.Struct: + name := capitalise(arg.Name) for j := 0; j < typ.NumField(); j++ { - field := typ.Field(j) // TODO read tags: `abi:"fieldName"` - trimmed := strings.TrimLeft(arg.Name, "_") - if field.Name == strings.ToUpper(trimmed[:1])+trimmed[1:] { + if typ.Field(j).Name == name { if err := set(value.Field(j), reflectValue, arg); err != nil { return err } @@ -223,3 +233,12 @@ func (arguments Arguments) Pack(args ...interface{}) ([]byte, error) { return ret, nil } + +// capitalise makes the first character of a string upper case, also removing any +// prefixing underscores from the variable names. +func capitalise(input string) string { + for len(input) > 0 && input[0] == '_' { + input = input[1:] + } + return strings.ToUpper(input[:1]) + input[1:] +} diff --git a/accounts/abi/bind/bind.go b/accounts/abi/bind/bind.go index 5d2b61b870..366c983263 100644 --- a/accounts/abi/bind/bind.go +++ b/accounts/abi/bind/bind.go @@ -319,15 +319,23 @@ func decapitalise(input string) string { } // structured checks whether a method has enough information to return a proper -// Go struct ot if flat returns are needed. +// Go struct or if flat returns are needed. func structured(method abi.Method) bool { if len(method.Outputs) < 2 { return false } + exists := make(map[string]bool) for _, out := range method.Outputs { + // If the name is anonymous, we can't organize into a struct if out.Name == "" { return false } + // If the field name collides (var, Var, _var, _Var), we can't organize into a struct + field := capitalise(out.Name) + if exists[field] { + return false + } + exists[field] = true } return true } diff --git a/accounts/abi/bind/bind_test.go b/accounts/abi/bind/bind_test.go index b1b2b22c17..4099476223 100644 --- a/accounts/abi/bind/bind_test.go +++ b/accounts/abi/bind/bind_test.go @@ -126,6 +126,7 @@ var bindTests = []struct { {"type":"function","name":"namedOutput","constant":true,"inputs":[],"outputs":[{"name":"str","type":"string"}]}, {"type":"function","name":"anonOutput","constant":true,"inputs":[],"outputs":[{"name":"","type":"string"}]}, {"type":"function","name":"namedOutputs","constant":true,"inputs":[],"outputs":[{"name":"str1","type":"string"},{"name":"str2","type":"string"}]}, + {"type":"function","name":"collidingOutputs","constant":true,"inputs":[],"outputs":[{"name":"str","type":"string"},{"name":"Str","type":"string"}]}, {"type":"function","name":"anonOutputs","constant":true,"inputs":[],"outputs":[{"name":"","type":"string"},{"name":"","type":"string"}]}, {"type":"function","name":"mixedOutputs","constant":true,"inputs":[],"outputs":[{"name":"","type":"string"},{"name":"str","type":"string"}]} ] @@ -140,6 +141,7 @@ var bindTests = []struct { str1, err = b.NamedOutput(nil) str1, err = b.AnonOutput(nil) res, _ := b.NamedOutputs(nil) + str1, str2, err = b.CollidingOutputs(nil) str1, str2, err = b.AnonOutputs(nil) str1, str2, err = b.MixedOutputs(nil) @@ -450,13 +452,26 @@ var bindTests = []struct { { `Underscorer`, ` - contract Underscorer { - function UnderscoredOutput() constant returns (int _int, string _string) { - return (314, "pi"); - } + contract Underscorer { + function UnderscoredOutput() constant returns (int _int, string _string) { + return (314, "pi"); } - `, `6060604052341561000f57600080fd5b6101698061001e6000396000f300606060405260043610610041576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff16806367e6633d14610046575b600080fd5b341561005157600080fd5b6100596100db565b6040518083815260200180602001828103825283818151815260200191508051906020019080838360005b8381101561009f578082015181840152602081019050610084565b50505050905090810190601f1680156100cc5780820380516001836020036101000a031916815260200191505b50935050505060405180910390f35b60006100e5610129565b61013a8090506040805190810160405280600281526020017f7069000000000000000000000000000000000000000000000000000000000000815250915091509091565b6020604051908101604052806000815250905600a165627a7a72305820b38a996760144524e16187fefa4a8ec24520361614be164ab10a4676a5f196340029`, - `[{"constant": true,"inputs": [],"name": "UnderscoredOutput","outputs": [{"name": "_int","type": "int256"},{"name": "_string","type": "string"}],"payable": false,"stateMutability": "view","type": "function"}]`, + function LowerLowerCollision() constant returns (int _res, int res) { + return (1, 2); + } + function LowerUpperCollision() constant returns (int _res, int Res) { + return (1, 2); + } + function UpperLowerCollision() constant returns (int _Res, int res) { + return (1, 2); + } + function UpperUpperCollision() constant returns (int _Res, int Res) { + return (1, 2); + } + } + + `, `6060604052341561000f57600080fd5b6102a98061001e6000396000f30060606040526004361061006d576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff16806303a592131461007257806367e6633d146100a2578063af7486ab14610137578063e02ab24d14610167578063e409ca4514610197575b600080fd5b341561007d57600080fd5b6100856101c7565b604051808381526020018281526020019250505060405180910390f35b34156100ad57600080fd5b6100b56101dc565b6040518083815260200180602001828103825283818151815260200191508051906020019080838360005b838110156100fb5780820151818401526020810190506100e0565b50505050905090810190601f1680156101285780820380516001836020036101000a031916815260200191505b50935050505060405180910390f35b341561014257600080fd5b61014a61022a565b604051808381526020018281526020019250505060405180910390f35b341561017257600080fd5b61017a61023f565b604051808381526020018281526020019250505060405180910390f35b34156101a257600080fd5b6101aa610254565b604051808381526020018281526020019250505060405180910390f35b60008060016002819150809050915091509091565b60006101e6610269565b61013a8090506040805190810160405280600281526020017f7069000000000000000000000000000000000000000000000000000000000000815250915091509091565b60008060016002819150809050915091509091565b60008060016002819150809050915091509091565b60008060016002819150809050915091509091565b6020604051908101604052806000815250905600a165627a7a723058204b430a23ef5e12ced00d09d546fdc343d181a3229300c3dd1b6fabc0816d8ade0029`, + `[{"constant":true,"inputs":[],"name":"UpperUpperCollision","outputs":[{"name":"_Res","type":"int256"},{"name":"Res","type":"int256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[],"name":"LowerUpperCollision","outputs":[{"name":"_res","type":"int256"},{"name":"Res","type":"int256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[],"name":"UnderscoredOutput","outputs":[{"name":"_int","type":"int256"},{"name":"_string","type":"string"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[],"name":"UpperLowerCollision","outputs":[{"name":"_Res","type":"int256"},{"name":"res","type":"int256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[],"name":"LowerLowerCollision","outputs":[{"name":"_res","type":"int256"},{"name":"res","type":"int256"}],"payable":false,"stateMutability":"view","type":"function"}]`, ` // Generate a new random account and a funded simulator key, _ := crypto.GenerateKey() @@ -470,11 +485,21 @@ var bindTests = []struct { } sim.Commit() + // Verify that underscored return values correclty parse into structs if res, err := underscorer.UnderscoredOutput(nil); err != nil { t.Errorf("Failed to call constant function: %v", err) } else if res.Int.Cmp(big.NewInt(314)) != 0 || res.String != "pi" { t.Errorf("Invalid result, want: {314, \"pi\"}, got: %+v", res) } + // Verify that underscored and non-underscored name collisions force tuple outputs + var a, b *big.Int + + a, b, _ = underscorer.LowerLowerCollision(nil) + a, b, _ = underscorer.LowerUpperCollision(nil) + a, b, _ = underscorer.UpperLowerCollision(nil) + a, b, _ = underscorer.UpperUpperCollision(nil) + + fmt.Println(a, b, err) `, }, } diff --git a/accounts/abi/unpack_test.go b/accounts/abi/unpack_test.go index 3910950732..194d06586d 100644 --- a/accounts/abi/unpack_test.go +++ b/accounts/abi/unpack_test.go @@ -259,6 +259,42 @@ var unpackTests = []unpackTest{ enc: "000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000003", want: [3]*big.Int{big.NewInt(1), big.NewInt(2), big.NewInt(3)}, }, + // struct outputs + { + def: `[{"name":"int1","type":"int256"},{"name":"int2","type":"int256"}]`, + enc: "00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000002", + want: struct { + Int1 *big.Int + Int2 *big.Int + }{big.NewInt(1), big.NewInt(2)}, + }, + { + def: `[{"name":"int","type":"int256"},{"name":"Int","type":"int256"}]`, + enc: "00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000002", + want: struct { + Int1 *big.Int + Int2 *big.Int + }{}, + err: "abi: multiple outputs mapping to the same struct field 'Int'", + }, + { + def: `[{"name":"int","type":"int256"},{"name":"_int","type":"int256"}]`, + enc: "00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000002", + want: struct { + Int1 *big.Int + Int2 *big.Int + }{}, + err: "abi: multiple outputs mapping to the same struct field 'Int'", + }, + { + def: `[{"name":"Int","type":"int256"},{"name":"_int","type":"int256"}]`, + enc: "00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000002", + want: struct { + Int1 *big.Int + Int2 *big.Int + }{}, + err: "abi: multiple outputs mapping to the same struct field 'Int'", + }, } func TestUnpack(t *testing.T) {