Skip to content

Commit 420b786

Browse files
MariusVanDerWijdengballet
MariusVanDerWijden
and
gballet
authored
accounts/abi: ABI explicit difference between Unpack and UnpackIntoInterface (ethereum#21091)
* accounts/abi: refactored abi.Unpack * accounts/abi/bind: fixed error * accounts/abi/bind: modified template * accounts/abi/bind: added ToStruct for conversion * accounts/abi: reenabled tests * accounts/abi: fixed tests * accounts/abi: fixed tests for packing/unpacking * accounts/abi: fixed tests * accounts/abi: added more logic to ToStruct * accounts/abi/bind: fixed template * accounts/abi/bind: fixed ToStruct conversion * accounts/abi/: removed unused code * accounts/abi: updated template * accounts/abi: refactored unused code * contracts/checkpointoracle: updated contracts to sol ^0.6.0 * accounts/abi: refactored reflection logic * accounts/abi: less code duplication in Unpack* * accounts/abi: fixed rebasing bug * fix a few typos in comments * rebase on master Co-authored-by: Guillaume Ballet <[email protected]>
1 parent c995914 commit 420b786

File tree

18 files changed

+325
-201
lines changed

18 files changed

+325
-201
lines changed

‎accounts/abi/abi.go

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -80,36 +80,56 @@ func (abi ABI) Pack(name string, args ...interface{}) ([]byte, error) {
8080
return append(method.ID, arguments...), nil
8181
}
8282

83-
// Unpack output in v according to the abi specification.
84-
func (abi ABI) Unpack(v interface{}, name string, data []byte) (err error) {
83+
func (abi ABI) getArguments(name string, data []byte) (Arguments, error) {
8584
// since there can't be naming collisions with contracts and events,
8685
// we need to decide whether we're calling a method or an event
86+
var args Arguments
8787
if method, ok := abi.Methods[name]; ok {
8888
if len(data)%32 != 0 {
89-
return fmt.Errorf("abi: improperly formatted output: %s - Bytes: [%+v]", string(data), data)
89+
return nil, fmt.Errorf("abi: improperly formatted output: %s - Bytes: [%+v]", string(data), data)
9090
}
91-
return method.Outputs.Unpack(v, data)
91+
args = method.Outputs
9292
}
9393
if event, ok := abi.Events[name]; ok {
94-
return event.Inputs.Unpack(v, data)
94+
args = event.Inputs
9595
}
96-
return fmt.Errorf("abi: could not locate named method or event")
96+
if args == nil {
97+
return nil, errors.New("abi: could not locate named method or event")
98+
}
99+
return args, nil
100+
}
101+
102+
// Unpack unpacks the output according to the abi specification.
103+
func (abi ABI) Unpack(name string, data []byte) ([]interface{}, error) {
104+
args, err := abi.getArguments(name, data)
105+
if err != nil {
106+
return nil, err
107+
}
108+
return args.Unpack(data)
109+
}
110+
111+
// UnpackIntoInterface unpacks the output in v according to the abi specification.
112+
// It performs an additional copy. Please only use, if you want to unpack into a
113+
// structure that does not strictly conform to the abi structure (e.g. has additional arguments)
114+
func (abi ABI) UnpackIntoInterface(v interface{}, name string, data []byte) error {
115+
args, err := abi.getArguments(name, data)
116+
if err != nil {
117+
return err
118+
}
119+
unpacked, err := args.Unpack(data)
120+
if err != nil {
121+
return err
122+
}
123+
return args.Copy(v, unpacked)
97124
}
98125

99126
// UnpackIntoMap unpacks a log into the provided map[string]interface{}.
100127
func (abi ABI) UnpackIntoMap(v map[string]interface{}, name string, data []byte) (err error) {
101-
// since there can't be naming collisions with contracts and events,
102-
// we need to decide whether we're calling a method or an event
103-
if method, ok := abi.Methods[name]; ok {
104-
if len(data)%32 != 0 {
105-
return fmt.Errorf("abi: improperly formatted output")
106-
}
107-
return method.Outputs.UnpackIntoMap(v, data)
108-
}
109-
if event, ok := abi.Events[name]; ok {
110-
return event.Inputs.UnpackIntoMap(v, data)
128+
args, err := abi.getArguments(name, data)
129+
if err != nil {
130+
return err
111131
}
112-
return fmt.Errorf("abi: could not locate named method or event")
132+
return args.UnpackIntoMap(v, data)
113133
}
114134

115135
// UnmarshalJSON implements json.Unmarshaler interface.
@@ -250,10 +270,10 @@ func UnpackRevert(data []byte) (string, error) {
250270
if !bytes.Equal(data[:4], revertSelector) {
251271
return "", errors.New("invalid data for unpacking")
252272
}
253-
var reason string
254273
typ, _ := NewType("string", "", nil)
255-
if err := (Arguments{{Type: typ}}).Unpack(&reason, data[4:]); err != nil {
274+
unpacked, err := (Arguments{{Type: typ}}).Unpack(data[4:])
275+
if err != nil {
256276
return "", err
257277
}
258-
return reason, nil
278+
return unpacked[0].(string), nil
259279
}

‎accounts/abi/abi_test.go

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -181,18 +181,15 @@ func TestConstructor(t *testing.T) {
181181
if err != nil {
182182
t.Error(err)
183183
}
184-
v := struct {
185-
A *big.Int
186-
B *big.Int
187-
}{new(big.Int), new(big.Int)}
188-
//abi.Unpack(&v, "", packed)
189-
if err := abi.Constructor.Inputs.Unpack(&v, packed); err != nil {
184+
unpacked, err := abi.Constructor.Inputs.Unpack(packed)
185+
if err != nil {
190186
t.Error(err)
191187
}
192-
if !reflect.DeepEqual(v.A, big.NewInt(1)) {
188+
189+
if !reflect.DeepEqual(unpacked[0], big.NewInt(1)) {
193190
t.Error("Unable to pack/unpack from constructor")
194191
}
195-
if !reflect.DeepEqual(v.B, big.NewInt(2)) {
192+
if !reflect.DeepEqual(unpacked[1], big.NewInt(2)) {
196193
t.Error("Unable to pack/unpack from constructor")
197194
}
198195
}
@@ -743,7 +740,7 @@ func TestUnpackEvent(t *testing.T) {
743740
}
744741
var ev ReceivedEvent
745742

746-
err = abi.Unpack(&ev, "received", data)
743+
err = abi.UnpackIntoInterface(&ev, "received", data)
747744
if err != nil {
748745
t.Error(err)
749746
}
@@ -752,7 +749,7 @@ func TestUnpackEvent(t *testing.T) {
752749
Sender common.Address
753750
}
754751
var receivedAddrEv ReceivedAddrEvent
755-
err = abi.Unpack(&receivedAddrEv, "receivedAddr", data)
752+
err = abi.UnpackIntoInterface(&receivedAddrEv, "receivedAddr", data)
756753
if err != nil {
757754
t.Error(err)
758755
}

‎accounts/abi/argument.go

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -76,28 +76,20 @@ func (arguments Arguments) isTuple() bool {
7676
}
7777

7878
// Unpack performs the operation hexdata -> Go format.
79-
func (arguments Arguments) Unpack(v interface{}, data []byte) error {
79+
func (arguments Arguments) Unpack(data []byte) ([]interface{}, error) {
8080
if len(data) == 0 {
8181
if len(arguments) != 0 {
82-
return fmt.Errorf("abi: attempting to unmarshall an empty string while arguments are expected")
82+
return nil, fmt.Errorf("abi: attempting to unmarshall an empty string while arguments are expected")
8383
}
84-
return nil // Nothing to unmarshal, return
85-
}
86-
// make sure the passed value is arguments pointer
87-
if reflect.Ptr != reflect.ValueOf(v).Kind() {
88-
return fmt.Errorf("abi: Unpack(non-pointer %T)", v)
89-
}
90-
marshalledValues, err := arguments.UnpackValues(data)
91-
if err != nil {
92-
return err
93-
}
94-
if len(marshalledValues) == 0 {
95-
return fmt.Errorf("abi: Unpack(no-values unmarshalled %T)", v)
96-
}
97-
if arguments.isTuple() {
98-
return arguments.unpackTuple(v, marshalledValues)
84+
// Nothing to unmarshal, return default variables
85+
nonIndexedArgs := arguments.NonIndexed()
86+
defaultVars := make([]interface{}, len(nonIndexedArgs))
87+
for index, arg := range nonIndexedArgs {
88+
defaultVars[index] = reflect.New(arg.Type.GetType())
89+
}
90+
return defaultVars, nil
9991
}
100-
return arguments.unpackAtomic(v, marshalledValues[0])
92+
return arguments.UnpackValues(data)
10193
}
10294

10395
// UnpackIntoMap performs the operation hexdata -> mapping of argument name to argument value.
@@ -122,8 +114,26 @@ func (arguments Arguments) UnpackIntoMap(v map[string]interface{}, data []byte)
122114
return nil
123115
}
124116

125-
// unpackAtomic unpacks ( hexdata -> go ) a single value.
126-
func (arguments Arguments) unpackAtomic(v interface{}, marshalledValues interface{}) error {
117+
// Copy performs the operation go format -> provided struct.
118+
func (arguments Arguments) Copy(v interface{}, values []interface{}) error {
119+
// make sure the passed value is arguments pointer
120+
if reflect.Ptr != reflect.ValueOf(v).Kind() {
121+
return fmt.Errorf("abi: Unpack(non-pointer %T)", v)
122+
}
123+
if len(values) == 0 {
124+
if len(arguments) != 0 {
125+
return fmt.Errorf("abi: attempting to copy no values while %d arguments are expected", len(arguments))
126+
}
127+
return nil // Nothing to copy, return
128+
}
129+
if arguments.isTuple() {
130+
return arguments.copyTuple(v, values)
131+
}
132+
return arguments.copyAtomic(v, values[0])
133+
}
134+
135+
// unpackAtomic unpacks ( hexdata -> go ) a single value
136+
func (arguments Arguments) copyAtomic(v interface{}, marshalledValues interface{}) error {
127137
dst := reflect.ValueOf(v).Elem()
128138
src := reflect.ValueOf(marshalledValues)
129139

@@ -133,8 +143,8 @@ func (arguments Arguments) unpackAtomic(v interface{}, marshalledValues interfac
133143
return set(dst, src)
134144
}
135145

136-
// unpackTuple unpacks ( hexdata -> go ) a batch of values.
137-
func (arguments Arguments) unpackTuple(v interface{}, marshalledValues []interface{}) error {
146+
// copyTuple copies a batch of values from marshalledValues to v.
147+
func (arguments Arguments) copyTuple(v interface{}, marshalledValues []interface{}) error {
138148
value := reflect.ValueOf(v).Elem()
139149
nonIndexedArgs := arguments.NonIndexed()
140150

‎accounts/abi/bind/base.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,11 +117,14 @@ func DeployContract(opts *TransactOpts, abi abi.ABI, bytecode []byte, backend Co
117117
// sets the output to result. The result type might be a single field for simple
118118
// returns, a slice of interfaces for anonymous returns and a struct for named
119119
// returns.
120-
func (c *BoundContract) Call(opts *CallOpts, result interface{}, method string, params ...interface{}) error {
120+
func (c *BoundContract) Call(opts *CallOpts, results *[]interface{}, method string, params ...interface{}) error {
121121
// Don't crash on a lazy user
122122
if opts == nil {
123123
opts = new(CallOpts)
124124
}
125+
if results == nil {
126+
results = new([]interface{})
127+
}
125128
// Pack the input, call and unpack the results
126129
input, err := c.abi.Pack(method, params...)
127130
if err != nil {
@@ -158,10 +161,14 @@ func (c *BoundContract) Call(opts *CallOpts, result interface{}, method string,
158161
}
159162
}
160163
}
161-
if err != nil {
164+
165+
if len(*results) == 0 {
166+
res, err := c.abi.Unpack(method, output)
167+
*results = res
162168
return err
163169
}
164-
return c.abi.Unpack(result, method, output)
170+
res := *results
171+
return c.abi.UnpackIntoInterface(res[0], method, output)
165172
}
166173

167174
// Transact invokes the (paid) contract method with params as input values.
@@ -339,7 +346,7 @@ func (c *BoundContract) WatchLogs(opts *WatchOpts, name string, query ...[]inter
339346
// UnpackLog unpacks a retrieved log into the provided output structure.
340347
func (c *BoundContract) UnpackLog(out interface{}, event string, log types.Log) error {
341348
if len(log.Data) > 0 {
342-
if err := c.abi.Unpack(out, event, log.Data); err != nil {
349+
if err := c.abi.UnpackIntoInterface(out, event, log.Data); err != nil {
343350
return err
344351
}
345352
}

‎accounts/abi/bind/base_test.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,10 @@ func TestPassingBlockNumber(t *testing.T) {
7171
},
7272
},
7373
}, mc, nil, nil)
74-
var ret string
7574

7675
blockNumber := big.NewInt(42)
7776

78-
bc.Call(&bind.CallOpts{BlockNumber: blockNumber}, &ret, "something")
77+
bc.Call(&bind.CallOpts{BlockNumber: blockNumber}, nil, "something")
7978

8079
if mc.callContractBlockNumber != blockNumber {
8180
t.Fatalf("CallContract() was not passed the block number")
@@ -85,7 +84,7 @@ func TestPassingBlockNumber(t *testing.T) {
8584
t.Fatalf("CodeAt() was not passed the block number")
8685
}
8786

88-
bc.Call(&bind.CallOpts{}, &ret, "something")
87+
bc.Call(&bind.CallOpts{}, nil, "something")
8988

9089
if mc.callContractBlockNumber != nil {
9190
t.Fatalf("CallContract() was passed a block number when it should not have been")
@@ -95,7 +94,7 @@ func TestPassingBlockNumber(t *testing.T) {
9594
t.Fatalf("CodeAt() was passed a block number when it should not have been")
9695
}
9796

98-
bc.Call(&bind.CallOpts{BlockNumber: blockNumber, Pending: true}, &ret, "something")
97+
bc.Call(&bind.CallOpts{BlockNumber: blockNumber, Pending: true}, nil, "something")
9998

10099
if !mc.pendingCallContractCalled {
101100
t.Fatalf("CallContract() was not passed the block number")

‎accounts/abi/bind/bind_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1696,11 +1696,11 @@ func TestGolangBindings(t *testing.T) {
16961696
t.Skip("go sdk not found for testing")
16971697
}
16981698
// Create a temporary workspace for the test suite
1699-
ws, err := ioutil.TempDir("", "")
1699+
ws, err := ioutil.TempDir("", "binding-test")
17001700
if err != nil {
17011701
t.Fatalf("failed to create temporary workspace: %v", err)
17021702
}
1703-
defer os.RemoveAll(ws)
1703+
//defer os.RemoveAll(ws)
17041704

17051705
pkg := filepath.Join(ws, "bindtest")
17061706
if err = os.MkdirAll(pkg, 0700); err != nil {

‎accounts/abi/bind/template.go

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ var (
261261
// sets the output to result. The result type might be a single field for simple
262262
// returns, a slice of interfaces for anonymous returns and a struct for named
263263
// returns.
264-
func (_{{$contract.Type}} *{{$contract.Type}}Raw) Call(opts *bind.CallOpts, result interface{}, method string, params ...interface{}) error {
264+
func (_{{$contract.Type}} *{{$contract.Type}}Raw) Call(opts *bind.CallOpts, result *[]interface{}, method string, params ...interface{}) error {
265265
return _{{$contract.Type}}.Contract.{{$contract.Type}}Caller.contract.Call(opts, result, method, params...)
266266
}
267267
@@ -280,7 +280,7 @@ var (
280280
// sets the output to result. The result type might be a single field for simple
281281
// returns, a slice of interfaces for anonymous returns and a struct for named
282282
// returns.
283-
func (_{{$contract.Type}} *{{$contract.Type}}CallerRaw) Call(opts *bind.CallOpts, result interface{}, method string, params ...interface{}) error {
283+
func (_{{$contract.Type}} *{{$contract.Type}}CallerRaw) Call(opts *bind.CallOpts, result *[]interface{}, method string, params ...interface{}) error {
284284
return _{{$contract.Type}}.Contract.contract.Call(opts, result, method, params...)
285285
}
286286
@@ -300,19 +300,23 @@ var (
300300
//
301301
// Solidity: {{.Original.String}}
302302
func (_{{$contract.Type}} *{{$contract.Type}}Caller) {{.Normalized.Name}}(opts *bind.CallOpts {{range .Normalized.Inputs}}, {{.Name}} {{bindtype .Type $structs}} {{end}}) ({{if .Structured}}struct{ {{range .Normalized.Outputs}}{{.Name}} {{bindtype .Type $structs}};{{end}} },{{else}}{{range .Normalized.Outputs}}{{bindtype .Type $structs}},{{end}}{{end}} error) {
303-
{{if .Structured}}ret := new(struct{
304-
{{range .Normalized.Outputs}}{{.Name}} {{bindtype .Type $structs}}
305-
{{end}}
306-
}){{else}}var (
307-
{{range $i, $_ := .Normalized.Outputs}}ret{{$i}} = new({{bindtype .Type $structs}})
308-
{{end}}
309-
){{end}}
310-
out := {{if .Structured}}ret{{else}}{{if eq (len .Normalized.Outputs) 1}}ret0{{else}}&[]interface{}{
311-
{{range $i, $_ := .Normalized.Outputs}}ret{{$i}},
312-
{{end}}
313-
}{{end}}{{end}}
314-
err := _{{$contract.Type}}.contract.Call(opts, out, "{{.Original.Name}}" {{range .Normalized.Inputs}}, {{.Name}}{{end}})
315-
return {{if .Structured}}*ret,{{else}}{{range $i, $_ := .Normalized.Outputs}}*ret{{$i}},{{end}}{{end}} err
303+
var out []interface{}
304+
err := _{{$contract.Type}}.contract.Call(opts, &out, "{{.Original.Name}}" {{range .Normalized.Inputs}}, {{.Name}}{{end}})
305+
{{if .Structured}}
306+
outstruct := new(struct{ {{range .Normalized.Outputs}} {{.Name}} {{bindtype .Type $structs}}; {{end}} })
307+
{{range $i, $t := .Normalized.Outputs}}
308+
outstruct.{{.Name}} = out[{{$i}}].({{bindtype .Type $structs}}){{end}}
309+
310+
return *outstruct, err
311+
{{else}}
312+
if err != nil {
313+
return {{range $i, $_ := .Normalized.Outputs}}*new({{bindtype .Type $structs}}), {{end}} err
314+
}
315+
{{range $i, $t := .Normalized.Outputs}}
316+
out{{$i}} := *abi.ConvertType(out[{{$i}}], new({{bindtype .Type $structs}})).(*{{bindtype .Type $structs}}){{end}}
317+
318+
return {{range $i, $t := .Normalized.Outputs}}out{{$i}}, {{end}} err
319+
{{end}}
316320
}
317321
318322
// {{.Normalized.Name}} is a free data retrieval call binding the contract method 0x{{printf "%x" .Original.ID}}.

0 commit comments

Comments
 (0)