From c93b90563380e168546c043214788dfec5a38b29 Mon Sep 17 00:00:00 2001 From: lwh Date: Tue, 7 Jun 2022 14:38:54 +0800 Subject: [PATCH] accounts/abi/bind: fix duplicate field names in the generated go struct (#24924) * accounts/abi/bind: fix duplicate field names in the generated go struct #24627 * accounts, cmd/abigen: resolve name conflicts * ci lint, accounts/abi: remove unused function overloadedArgName Co-authored-by: Gary Rong --- accounts/abi/abi.go | 23 +++----------- accounts/abi/bind/bind.go | 25 +++++++++++++-- accounts/abi/bind/bind_test.go | 48 +++++++++++++++++++++++++++++ accounts/abi/error.go | 6 ++-- accounts/abi/event.go | 7 +++-- accounts/abi/type.go | 56 +++++++++++++++++++++++----------- accounts/abi/utils.go | 41 +++++++++++++++++++++++++ cmd/abigen/main.go | 6 +++- 8 files changed, 168 insertions(+), 44 deletions(-) create mode 100644 accounts/abi/utils.go diff --git a/accounts/abi/abi.go b/accounts/abi/abi.go index cd2f4d7978bd..ed5b6e92ef9c 100644 --- a/accounts/abi/abi.go +++ b/accounts/abi/abi.go @@ -164,7 +164,7 @@ func (abi *ABI) UnmarshalJSON(data []byte) error { case "constructor": abi.Constructor = NewMethod("", "", Constructor, field.StateMutability, field.Constant, field.Payable, field.Inputs, nil) case "function": - name := overloadedName(field.Name, func(s string) bool { _, ok := abi.Methods[s]; return ok }) + name := ResolveNameConflict(field.Name, func(s string) bool { _, ok := abi.Methods[s]; return ok }) abi.Methods[name] = NewMethod(name, field.Name, Function, field.StateMutability, field.Constant, field.Payable, field.Inputs, field.Outputs) case "fallback": // New introduced function type in v0.6.0, check more detail @@ -184,9 +184,11 @@ func (abi *ABI) UnmarshalJSON(data []byte) error { } abi.Receive = NewMethod("", "", Receive, field.StateMutability, field.Constant, field.Payable, nil, nil) case "event": - name := overloadedName(field.Name, func(s string) bool { _, ok := abi.Events[s]; return ok }) + name := ResolveNameConflict(field.Name, func(s string) bool { _, ok := abi.Events[s]; return ok }) abi.Events[name] = NewEvent(name, field.Name, field.Anonymous, field.Inputs) case "error": + // Errors cannot be overloaded or overridden but are inherited, + // no need to resolve the name conflict here. abi.Errors[field.Name] = NewError(field.Name, field.Inputs) default: return fmt.Errorf("abi: could not recognize type %v of field %v", field.Type, field.Name) @@ -251,20 +253,3 @@ func UnpackRevert(data []byte) (string, error) { } return unpacked[0].(string), nil } - -// overloadedName returns the next available name for a given thing. -// Needed since solidity allows for overloading. -// -// e.g. if the abi contains Methods send, send1 -// overloadedName would return send2 for input send. -// -// overloadedName works for methods, events and errors. -func overloadedName(rawName string, isAvail func(string) bool) string { - name := rawName - ok := isAvail(name) - for idx := 0; ok; idx++ { - name = fmt.Sprintf("%s%d", rawName, idx) - ok = isAvail(name) - } - return name -} diff --git a/accounts/abi/bind/bind.go b/accounts/abi/bind/bind.go index ff69a78c642f..2b02a6344ec3 100644 --- a/accounts/abi/bind/bind.go +++ b/accounts/abi/bind/bind.go @@ -99,6 +99,7 @@ func Bind(types []string, abis []string, bytecodes []string, fsigs []map[string] // Normalize the method for capital cases and non-anonymous inputs/outputs normalized := original normalizedName := methodNormalizer[lang](alias(aliases, original.Name)) + // Ensure there is no duplicated identifier var identifiers = callIdentifiers if !original.IsConstant() { @@ -108,6 +109,7 @@ func Bind(types []string, abis []string, bytecodes []string, fsigs []map[string] return "", fmt.Errorf("duplicated identifier \"%s\"(normalized \"%s\"), use --alias for renaming", original.Name, normalizedName) } identifiers[normalizedName] = true + normalized.Name = normalizedName normalized.Inputs = make([]abi.Argument, len(original.Inputs)) copy(normalized.Inputs, original.Inputs) @@ -152,12 +154,22 @@ func Bind(types []string, abis []string, bytecodes []string, fsigs []map[string] eventIdentifiers[normalizedName] = true normalized.Name = normalizedName + used := make(map[string]bool) normalized.Inputs = make([]abi.Argument, len(original.Inputs)) copy(normalized.Inputs, original.Inputs) for j, input := range normalized.Inputs { if input.Name == "" { normalized.Inputs[j].Name = fmt.Sprintf("arg%d", j) } + // Event is a bit special, we need to define event struct in binding, + // ensure there is no camel-case-style name conflict. + for index := 0; ; index++ { + if !used[capitalise(normalized.Inputs[j].Name)] { + used[capitalise(normalized.Inputs[j].Name)] = true + break + } + normalized.Inputs[j].Name = fmt.Sprintf("%s%d", normalized.Inputs[j].Name, index) + } if hasStruct(input.Type) { bindStructType[lang](input.Type, structs) } @@ -432,15 +444,22 @@ func bindStructTypeGo(kind abi.Type, structs map[string]*tmplStruct) string { if s, exist := structs[id]; exist { return s.Name } - var fields []*tmplField + var ( + names = make(map[string]bool) + fields []*tmplField + ) for i, elem := range kind.TupleElems { - field := bindStructTypeGo(*elem, structs) - fields = append(fields, &tmplField{Type: field, Name: capitalise(kind.TupleRawNames[i]), SolKind: *elem}) + name := capitalise(kind.TupleRawNames[i]) + name = abi.ResolveNameConflict(name, func(s string) bool { return names[s] }) + names[name] = true + fields = append(fields, &tmplField{Type: bindStructTypeGo(*elem, structs), Name: name, SolKind: *elem}) } name := kind.TupleRawName if name == "" { name = fmt.Sprintf("Struct%d", len(structs)) } + name = capitalise(name) + structs[id] = &tmplStruct{ Name: name, Fields: fields, diff --git a/accounts/abi/bind/bind_test.go b/accounts/abi/bind/bind_test.go index 6b72cc456357..03d760076894 100644 --- a/accounts/abi/bind/bind_test.go +++ b/accounts/abi/bind/bind_test.go @@ -1949,6 +1949,54 @@ var bindTests = []struct { } sim.Commit() + if _, err = bind.WaitDeployed(nil, sim, tx); err != nil { + t.Logf("Deployment tx: %+v", tx) + t.Errorf("bind.WaitDeployed(nil, %T, ) got err %v; want nil err", sim, err) + } + `, + }, + { + name: `NameConflict`, + contract: ` + // SPDX-License-Identifier: GPL-3.0 + pragma solidity >=0.4.22 <0.9.0; + contract oracle { + struct request { + bytes data; + bytes _data; + } + event log (int msg, int _msg); + function addRequest(request memory req) public pure {} + function getRequest() pure public returns (request memory) { + return request("", ""); + } + } + `, + bytecode: []string{`0x608060405234801561001057600080fd5b5061042b806100206000396000f3fe608060405234801561001057600080fd5b50600436106100365760003560e01c8063c2bb515f1461003b578063cce7b04814610059575b600080fd5b610043610075565b60405161005091906101af565b60405180910390f35b610073600480360381019061006e91906103ac565b6100b5565b005b61007d6100b8565b604051806040016040528060405180602001604052806000815250815260200160405180602001604052806000815250815250905090565b50565b604051806040016040528060608152602001606081525090565b600081519050919050565b600082825260208201905092915050565b60005b8381101561010c5780820151818401526020810190506100f1565b8381111561011b576000848401525b50505050565b6000601f19601f8301169050919050565b600061013d826100d2565b61014781856100dd565b93506101578185602086016100ee565b61016081610121565b840191505092915050565b600060408301600083015184820360008601526101888282610132565b915050602083015184820360208601526101a28282610132565b9150508091505092915050565b600060208201905081810360008301526101c9818461016b565b905092915050565b6000604051905090565b600080fd5b600080fd5b600080fd5b7f4e487b7100000000000000000000000000000000000000000000000000000000600052604160045260246000fd5b61022282610121565b810181811067ffffffffffffffff82111715610241576102406101ea565b5b80604052505050565b60006102546101d1565b90506102608282610219565b919050565b600080fd5b600080fd5b600080fd5b600067ffffffffffffffff82111561028f5761028e6101ea565b5b61029882610121565b9050602081019050919050565b82818337600083830152505050565b60006102c76102c284610274565b61024a565b9050828152602081018484840111156102e3576102e261026f565b5b6102ee8482856102a5565b509392505050565b600082601f83011261030b5761030a61026a565b5b813561031b8482602086016102b4565b91505092915050565b60006040828403121561033a576103396101e5565b5b610344604061024a565b9050600082013567ffffffffffffffff81111561036457610363610265565b5b610370848285016102f6565b600083015250602082013567ffffffffffffffff81111561039457610393610265565b5b6103a0848285016102f6565b60208301525092915050565b6000602082840312156103c2576103c16101db565b5b600082013567ffffffffffffffff8111156103e0576103df6101e0565b5b6103ec84828501610324565b9150509291505056fea264697066735822122033bca1606af9b6aeba1673f98c52003cec19338539fb44b86690ce82c51483b564736f6c634300080e0033`}, + abi: []string{`[ { "anonymous": false, "inputs": [ { "indexed": false, "internalType": "int256", "name": "msg", "type": "int256" }, { "indexed": false, "internalType": "int256", "name": "_msg", "type": "int256" } ], "name": "log", "type": "event" }, { "inputs": [ { "components": [ { "internalType": "bytes", "name": "data", "type": "bytes" }, { "internalType": "bytes", "name": "_data", "type": "bytes" } ], "internalType": "struct oracle.request", "name": "req", "type": "tuple" } ], "name": "addRequest", "outputs": [], "stateMutability": "pure", "type": "function" }, { "inputs": [], "name": "getRequest", "outputs": [ { "components": [ { "internalType": "bytes", "name": "data", "type": "bytes" }, { "internalType": "bytes", "name": "_data", "type": "bytes" } ], "internalType": "struct oracle.request", "name": "", "type": "tuple" } ], "stateMutability": "pure", "type": "function" } ]`}, + imports: ` + "math/big" + + "github.com/ethereum/go-ethereum/accounts/abi/bind" + "github.com/ethereum/go-ethereum/accounts/abi/bind/backends" + "github.com/ethereum/go-ethereum/core" + "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/eth/ethconfig" + `, + tester: ` + var ( + key, _ = crypto.GenerateKey() + user, _ = bind.NewKeyedTransactorWithChainID(key, big.NewInt(1337)) + sim = backends.NewSimulatedBackend(core.GenesisAlloc{user.From: {Balance: big.NewInt(1000000000000000000)}}, ethconfig.Defaults.Miner.GasCeil) + ) + defer sim.Close() + + _, tx, _, err := DeployNameConflict(user, sim) + if err != nil { + t.Fatalf("DeployNameConflict() got err %v; want nil err", err) + } + sim.Commit() + if _, err = bind.WaitDeployed(nil, sim, tx); err != nil { t.Logf("Deployment tx: %+v", tx) t.Errorf("bind.WaitDeployed(nil, %T, ) got err %v; want nil err", sim, err) diff --git a/accounts/abi/error.go b/accounts/abi/error.go index 3d1a4877ddcd..f2c44d2de8dd 100644 --- a/accounts/abi/error.go +++ b/accounts/abi/error.go @@ -30,11 +30,13 @@ type Error struct { Name string Inputs Arguments str string + // Sig contains the string signature according to the ABI spec. - // e.g. event foo(uint32 a, int b) = "foo(uint32,int256)" + // e.g. error foo(uint32 a, int b) = "foo(uint32,int256)" // Please note that "int" is substitute for its canonical representation "int256" Sig string - // ID returns the canonical representation of the event's signature used by the + + // ID returns the canonical representation of the error's signature used by the // abi definition to identify event names and types. ID common.Hash } diff --git a/accounts/abi/event.go b/accounts/abi/event.go index b238a36d7cea..f9457b86afeb 100644 --- a/accounts/abi/event.go +++ b/accounts/abi/event.go @@ -29,24 +29,27 @@ import ( // don't get the signature canonical representation as the first LOG topic. type Event struct { // Name is the event name used for internal representation. It's derived from - // the raw name and a suffix will be added in the case of a event overload. + // the raw name and a suffix will be added in the case of event overloading. // // e.g. // These are two events that have the same name: // * foo(int,int) // * foo(uint,uint) - // The event name of the first one wll be resolved as foo while the second one + // The event name of the first one will be resolved as foo while the second one // will be resolved as foo0. Name string + // RawName is the raw event name parsed from ABI. RawName string Anonymous bool Inputs Arguments str string + // Sig contains the string signature according to the ABI spec. // e.g. event foo(uint32 a, int b) = "foo(uint32,int256)" // Please note that "int" is substitute for its canonical representation "int256" Sig string + // ID returns the canonical representation of the event's signature used by the // abi definition to identify event names and types. ID common.Hash diff --git a/accounts/abi/type.go b/accounts/abi/type.go index ffa3acafe9c2..7d4e6308398c 100644 --- a/accounts/abi/type.go +++ b/accounts/abi/type.go @@ -23,6 +23,8 @@ import ( "regexp" "strconv" "strings" + "unicode" + "unicode/utf8" "github.com/ethereum/go-ethereum/common" ) @@ -161,19 +163,26 @@ func NewType(t string, internalType string, components []ArgumentMarshaling) (ty elems []*Type names []string expression string // canonical parameter expression + used = make(map[string]bool) ) expression += "(" - overloadedNames := make(map[string]string) for idx, c := range components { cType, err := NewType(c.Type, c.InternalType, c.Components) if err != nil { return Type{}, err } - fieldName, err := overloadedArgName(c.Name, overloadedNames) + name := ToCamelCase(c.Name) + if name == "" { + return Type{}, errors.New("abi: purely anonymous or underscored field is not supported") + } + fieldName := ResolveNameConflict(name, func(s string) bool { return used[s] }) if err != nil { return Type{}, err } - overloadedNames[fieldName] = fieldName + used[fieldName] = true + if !isValidFieldName(fieldName) { + return Type{}, fmt.Errorf("field %d has invalid name", idx) + } fields = append(fields, reflect.StructField{ Name: fieldName, // reflect.StructOf will panic for any exported field. Type: cType.GetType(), @@ -250,20 +259,6 @@ func (t Type) GetType() reflect.Type { } } -func overloadedArgName(rawName string, names map[string]string) (string, error) { - fieldName := ToCamelCase(rawName) - if fieldName == "" { - return "", errors.New("abi: purely anonymous or underscored field is not supported") - } - // Handle overloaded fieldNames - _, ok := names[fieldName] - for idx := 0; ok; idx++ { - fieldName = fmt.Sprintf("%s%d", ToCamelCase(rawName), idx) - _, ok = names[fieldName] - } - return fieldName, nil -} - // String implements Stringer. func (t Type) String() (out string) { return t.stringKind @@ -399,3 +394,30 @@ func getTypeSize(t Type) int { } return 32 } + +// isLetter reports whether a given 'rune' is classified as a Letter. +// This method is copied from reflect/type.go +func isLetter(ch rune) bool { + return 'a' <= ch && ch <= 'z' || 'A' <= ch && ch <= 'Z' || ch == '_' || ch >= utf8.RuneSelf && unicode.IsLetter(ch) +} + +// isValidFieldName checks if a string is a valid (struct) field name or not. +// +// According to the language spec, a field name should be an identifier. +// +// identifier = letter { letter | unicode_digit } . +// letter = unicode_letter | "_" . +// This method is copied from reflect/type.go +func isValidFieldName(fieldName string) bool { + for i, c := range fieldName { + if i == 0 && !isLetter(c) { + return false + } + + if !(isLetter(c) || unicode.IsDigit(c)) { + return false + } + } + + return len(fieldName) > 0 +} diff --git a/accounts/abi/utils.go b/accounts/abi/utils.go new file mode 100644 index 000000000000..e24df5b7003d --- /dev/null +++ b/accounts/abi/utils.go @@ -0,0 +1,41 @@ +// Copyright 2022 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 "fmt" + +// ResolveNameConflict returns the next available name for a given thing. +// This helper can be used for lots of purposes: +// +// - In solidity function overloading is supported, this function can fix +// the name conflicts of overloaded functions. +// - In golang binding generation, the parameter(in function, event, error, +// and struct definition) name will be converted to camelcase style which +// may eventually lead to name conflicts. +// +// Name conflicts are mostly resolved by adding number suffix. +// e.g. if the abi contains Methods send, send1 +// ResolveNameConflict would return send2 for input send. +func ResolveNameConflict(rawName string, used func(string) bool) string { + name := rawName + ok := used(name) + for idx := 0; ok; idx++ { + name = fmt.Sprintf("%s%d", rawName, idx) + ok = used(name) + } + return name +} diff --git a/cmd/abigen/main.go b/cmd/abigen/main.go index 7b3b35e4e54f..9547101eff50 100644 --- a/cmd/abigen/main.go +++ b/cmd/abigen/main.go @@ -237,7 +237,11 @@ func abigen(c *cli.Context) error { nameParts := strings.Split(name, ":") types = append(types, nameParts[len(nameParts)-1]) - libPattern := crypto.Keccak256Hash([]byte(name)).String()[2:36] + // Derive the library placeholder which is a 34 character prefix of the + // hex encoding of the keccak256 hash of the fully qualified library name. + // Note that the fully qualified library name is the path of its source + // file and the library name separated by ":". + libPattern := crypto.Keccak256Hash([]byte(name)).String()[2:36] // the first 2 chars are 0x libs[libPattern] = nameParts[len(nameParts)-1] } }