accounts/abi: ABI explicit difference between Unpack and UnpackIntoIn…

…terface (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 <gballet@gmail.com>

accounts/abi/abi.go

@@ -80,36 +80,56 @@ func (abi ABI) Pack(name string, args ...interface{}) ([]byte, error) {
 	return append(method.ID, arguments...), nil
 }
 
-// Unpack output in v according to the abi specification.
-func (abi ABI) Unpack(v interface{}, name string, data []byte) (err error) {
+func (abi ABI) getArguments(name string, data []byte) (Arguments, error) {
 	// since there can't be naming collisions with contracts and events,
 	// we need to decide whether we're calling a method or an event
+	var args Arguments
 	if method, ok := abi.Methods[name]; ok {
 		if len(data)%32 != 0 {
-			return fmt.Errorf("abi: improperly formatted output: %s - Bytes: [%+v]", string(data), data)
+			return nil, fmt.Errorf("abi: improperly formatted output: %s - Bytes: [%+v]", string(data), data)
 		}
-		return method.Outputs.Unpack(v, data)
+		args = method.Outputs
 	}
 	if event, ok := abi.Events[name]; ok {
-		return event.Inputs.Unpack(v, data)
+		args = event.Inputs
 	}
-	return fmt.Errorf("abi: could not locate named method or event")
+	if args == nil {
+		return nil, errors.New("abi: could not locate named method or event")
+	}
+	return args, nil
+}
+
+// Unpack unpacks the output according to the abi specification.
+func (abi ABI) Unpack(name string, data []byte) ([]interface{}, error) {
+	args, err := abi.getArguments(name, data)
+	if err != nil {
+		return nil, err
+	}
+	return args.Unpack(data)
+}
+
+// UnpackIntoInterface unpacks the output in v according to the abi specification.
+// It performs an additional copy. Please only use, if you want to unpack into a
+// structure that does not strictly conform to the abi structure (e.g. has additional arguments)
+func (abi ABI) UnpackIntoInterface(v interface{}, name string, data []byte) error {
+	args, err := abi.getArguments(name, data)
+	if err != nil {
+		return err
+	}
+	unpacked, err := args.Unpack(data)
+	if err != nil {
+		return err
+	}
+	return args.Copy(v, unpacked)
 }
 
 // UnpackIntoMap unpacks a log into the provided map[string]interface{}.
 func (abi ABI) UnpackIntoMap(v map[string]interface{}, name string, data []byte) (err error) {
-	// since there can't be naming collisions with contracts and events,
-	// we need to decide whether we're calling a method or an event
-	if method, ok := abi.Methods[name]; ok {
-		if len(data)%32 != 0 {
-			return fmt.Errorf("abi: improperly formatted output")
-		}
-		return method.Outputs.UnpackIntoMap(v, data)
-	}
-	if event, ok := abi.Events[name]; ok {
-		return event.Inputs.UnpackIntoMap(v, data)
+	args, err := abi.getArguments(name, data)
+	if err != nil {
+		return err
 	}
-	return fmt.Errorf("abi: could not locate named method or event")
+	return args.UnpackIntoMap(v, data)
 }
 
 // UnmarshalJSON implements json.Unmarshaler interface.
@@ -250,10 +270,10 @@ func UnpackRevert(data []byte) (string, error) {
 	if !bytes.Equal(data[:4], revertSelector) {
 		return "", errors.New("invalid data for unpacking")
 	}
-	var reason string
 	typ, _ := NewType("string", "", nil)
-	if err := (Arguments{{Type: typ}}).Unpack(&reason, data[4:]); err != nil {
+	unpacked, err := (Arguments{{Type: typ}}).Unpack(data[4:])
+	if err != nil {
 		return "", err
 	}
-	return reason, nil
+	return unpacked[0].(string), nil
 }

accounts/abi/abi_test.go

@@ -181,18 +181,15 @@ func TestConstructor(t *testing.T) {
 	if err != nil {
 		t.Error(err)
 	}
-	v := struct {
-		A *big.Int
-		B *big.Int
-	}{new(big.Int), new(big.Int)}
-	//abi.Unpack(&v, "", packed)
-	if err := abi.Constructor.Inputs.Unpack(&v, packed); err != nil {
+	unpacked, err := abi.Constructor.Inputs.Unpack(packed)
+	if err != nil {
 		t.Error(err)
 	}
-	if !reflect.DeepEqual(v.A, big.NewInt(1)) {
+
+	if !reflect.DeepEqual(unpacked[0], big.NewInt(1)) {
 		t.Error("Unable to pack/unpack from constructor")
 	}
-	if !reflect.DeepEqual(v.B, big.NewInt(2)) {
+	if !reflect.DeepEqual(unpacked[1], big.NewInt(2)) {
 		t.Error("Unable to pack/unpack from constructor")
 	}
 }
@@ -743,7 +740,7 @@ func TestUnpackEvent(t *testing.T) {
 	}
 	var ev ReceivedEvent
 
-	err = abi.Unpack(&ev, "received", data)
+	err = abi.UnpackIntoInterface(&ev, "received", data)
 	if err != nil {
 		t.Error(err)
 	}
@@ -752,7 +749,7 @@ func TestUnpackEvent(t *testing.T) {
 		Sender common.Address
 	}
 	var receivedAddrEv ReceivedAddrEvent
-	err = abi.Unpack(&receivedAddrEv, "receivedAddr", data)
+	err = abi.UnpackIntoInterface(&receivedAddrEv, "receivedAddr", data)
 	if err != nil {
 		t.Error(err)
 	}

accounts/abi/argument.go

@@ -76,28 +76,20 @@ func (arguments Arguments) isTuple() bool {
 }
 
 // Unpack performs the operation hexdata -> Go format.
-func (arguments Arguments) Unpack(v interface{}, data []byte) error {
+func (arguments Arguments) Unpack(data []byte) ([]interface{}, error) {
 	if len(data) == 0 {
 		if len(arguments) != 0 {
-			return fmt.Errorf("abi: attempting to unmarshall an empty string while arguments are expected")
+			return nil, fmt.Errorf("abi: attempting to unmarshall an empty string while arguments are expected")
 		}
-		return nil // Nothing to unmarshal, return
-	}
-	// make sure the passed value is arguments pointer
-	if reflect.Ptr != reflect.ValueOf(v).Kind() {
-		return fmt.Errorf("abi: Unpack(non-pointer %T)", v)
-	}
-	marshalledValues, err := arguments.UnpackValues(data)
-	if err != nil {
-		return err
-	}
-	if len(marshalledValues) == 0 {
-		return fmt.Errorf("abi: Unpack(no-values unmarshalled %T)", v)
-	}
-	if arguments.isTuple() {
-		return arguments.unpackTuple(v, marshalledValues)
+		// Nothing to unmarshal, return default variables
+		nonIndexedArgs := arguments.NonIndexed()
+		defaultVars := make([]interface{}, len(nonIndexedArgs))
+		for index, arg := range nonIndexedArgs {
+			defaultVars[index] = reflect.New(arg.Type.GetType())
+		}
+		return defaultVars, nil
 	}
-	return arguments.unpackAtomic(v, marshalledValues[0])
+	return arguments.UnpackValues(data)
 }
 
 // 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)
 	return nil
 }
 
-// unpackAtomic unpacks ( hexdata -> go ) a single value.
-func (arguments Arguments) unpackAtomic(v interface{}, marshalledValues interface{}) error {
+// Copy performs the operation go format -> provided struct.
+func (arguments Arguments) Copy(v interface{}, values []interface{}) error {
+	// make sure the passed value is arguments pointer
+	if reflect.Ptr != reflect.ValueOf(v).Kind() {
+		return fmt.Errorf("abi: Unpack(non-pointer %T)", v)
+	}
+	if len(values) == 0 {
+		if len(arguments) != 0 {
+			return fmt.Errorf("abi: attempting to copy no values while %d arguments are expected", len(arguments))
+		}
+		return nil // Nothing to copy, return
+	}
+	if arguments.isTuple() {
+		return arguments.copyTuple(v, values)
+	}
+	return arguments.copyAtomic(v, values[0])
+}
+
+// unpackAtomic unpacks ( hexdata -> go ) a single value
+func (arguments Arguments) copyAtomic(v interface{}, marshalledValues interface{}) error {
 	dst := reflect.ValueOf(v).Elem()
 	src := reflect.ValueOf(marshalledValues)
 
@@ -133,8 +143,8 @@ func (arguments Arguments) unpackAtomic(v interface{}, marshalledValues interfac
 	return set(dst, src)
 }
 
-// unpackTuple unpacks ( hexdata -> go ) a batch of values.
-func (arguments Arguments) unpackTuple(v interface{}, marshalledValues []interface{}) error {
+// copyTuple copies a batch of values from marshalledValues to v.
+func (arguments Arguments) copyTuple(v interface{}, marshalledValues []interface{}) error {
 	value := reflect.ValueOf(v).Elem()
 	nonIndexedArgs := arguments.NonIndexed()
 

accounts/abi/bind/base.go

@@ -117,11 +117,14 @@ func DeployContract(opts *TransactOpts, abi abi.ABI, bytecode []byte, backend Co
 // sets the output to result. The result type might be a single field for simple
 // returns, a slice of interfaces for anonymous returns and a struct for named
 // returns.
-func (c *BoundContract) Call(opts *CallOpts, result interface{}, method string, params ...interface{}) error {
+func (c *BoundContract) Call(opts *CallOpts, results *[]interface{}, method string, params ...interface{}) error {
 	// Don't crash on a lazy user
 	if opts == nil {
 		opts = new(CallOpts)
 	}
+	if results == nil {
+		results = new([]interface{})
+	}
 	// Pack the input, call and unpack the results
 	input, err := c.abi.Pack(method, params...)
 	if err != nil {
@@ -158,10 +161,14 @@ func (c *BoundContract) Call(opts *CallOpts, result interface{}, method string,
 			}
 		}
 	}
-	if err != nil {
+
+	if len(*results) == 0 {
+		res, err := c.abi.Unpack(method, output)
+		*results = res
 		return err
 	}
-	return c.abi.Unpack(result, method, output)
+	res := *results
+	return c.abi.UnpackIntoInterface(res[0], method, output)
 }
 
 // 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
 // UnpackLog unpacks a retrieved log into the provided output structure.
 func (c *BoundContract) UnpackLog(out interface{}, event string, log types.Log) error {
 	if len(log.Data) > 0 {
-		if err := c.abi.Unpack(out, event, log.Data); err != nil {
+		if err := c.abi.UnpackIntoInterface(out, event, log.Data); err != nil {
 			return err
 		}
 	}

accounts/abi/bind/base_test.go

@@ -71,11 +71,10 @@ func TestPassingBlockNumber(t *testing.T) {
 			},
 		},
 	}, mc, nil, nil)
-	var ret string
 
 	blockNumber := big.NewInt(42)
 
-	bc.Call(&bind.CallOpts{BlockNumber: blockNumber}, &ret, "something")
+	bc.Call(&bind.CallOpts{BlockNumber: blockNumber}, nil, "something")
 
 	if mc.callContractBlockNumber != blockNumber {
 		t.Fatalf("CallContract() was not passed the block number")
@@ -85,7 +84,7 @@ func TestPassingBlockNumber(t *testing.T) {
 		t.Fatalf("CodeAt() was not passed the block number")
 	}
 
-	bc.Call(&bind.CallOpts{}, &ret, "something")
+	bc.Call(&bind.CallOpts{}, nil, "something")
 
 	if mc.callContractBlockNumber != nil {
 		t.Fatalf("CallContract() was passed a block number when it should not have been")
@@ -95,7 +94,7 @@ func TestPassingBlockNumber(t *testing.T) {
 		t.Fatalf("CodeAt() was passed a block number when it should not have been")
 	}
 
-	bc.Call(&bind.CallOpts{BlockNumber: blockNumber, Pending: true}, &ret, "something")
+	bc.Call(&bind.CallOpts{BlockNumber: blockNumber, Pending: true}, nil, "something")
 
 	if !mc.pendingCallContractCalled {
 		t.Fatalf("CallContract() was not passed the block number")

accounts/abi/bind/bind_test.go

@@ -1696,11 +1696,11 @@ func TestGolangBindings(t *testing.T) {
 		t.Skip("go sdk not found for testing")
 	}
 	// Create a temporary workspace for the test suite
-	ws, err := ioutil.TempDir("", "")
+	ws, err := ioutil.TempDir("", "binding-test")
 	if err != nil {
 		t.Fatalf("failed to create temporary workspace: %v", err)
 	}
-	defer os.RemoveAll(ws)
+	//defer os.RemoveAll(ws)
 
 	pkg := filepath.Join(ws, "bindtest")
 	if err = os.MkdirAll(pkg, 0700); err != nil {

accounts/abi/bind/template.go

@@ -261,7 +261,7 @@ var (
 	// sets the output to result. The result type might be a single field for simple
 	// returns, a slice of interfaces for anonymous returns and a struct for named
 	// returns.
-	func (_{{$contract.Type}} *{{$contract.Type}}Raw) Call(opts *bind.CallOpts, result interface{}, method string, params ...interface{}) error {
+	func (_{{$contract.Type}} *{{$contract.Type}}Raw) Call(opts *bind.CallOpts, result *[]interface{}, method string, params ...interface{}) error {
 		return _{{$contract.Type}}.Contract.{{$contract.Type}}Caller.contract.Call(opts, result, method, params...)
 	}
 
@@ -280,7 +280,7 @@ var (
 	// sets the output to result. The result type might be a single field for simple
 	// returns, a slice of interfaces for anonymous returns and a struct for named
 	// returns.
-	func (_{{$contract.Type}} *{{$contract.Type}}CallerRaw) Call(opts *bind.CallOpts, result interface{}, method string, params ...interface{}) error {
+	func (_{{$contract.Type}} *{{$contract.Type}}CallerRaw) Call(opts *bind.CallOpts, result *[]interface{}, method string, params ...interface{}) error {
 		return _{{$contract.Type}}.Contract.contract.Call(opts, result, method, params...)
 	}
 
@@ -300,19 +300,23 @@ var (
 		//
 		// Solidity: {{.Original.String}}
 		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) {
-			{{if .Structured}}ret := new(struct{
-				{{range .Normalized.Outputs}}{{.Name}} {{bindtype .Type $structs}}
-				{{end}}
-			}){{else}}var (
-				{{range $i, $_ := .Normalized.Outputs}}ret{{$i}} = new({{bindtype .Type $structs}})
-				{{end}}
-			){{end}}
-			out := {{if .Structured}}ret{{else}}{{if eq (len .Normalized.Outputs) 1}}ret0{{else}}&[]interface{}{
-				{{range $i, $_ := .Normalized.Outputs}}ret{{$i}},
-				{{end}}
-			}{{end}}{{end}}
-			err := _{{$contract.Type}}.contract.Call(opts, out, "{{.Original.Name}}" {{range .Normalized.Inputs}}, {{.Name}}{{end}})
-			return {{if .Structured}}*ret,{{else}}{{range $i, $_ := .Normalized.Outputs}}*ret{{$i}},{{end}}{{end}} err
+			var out []interface{}
+			err := _{{$contract.Type}}.contract.Call(opts, &out, "{{.Original.Name}}" {{range .Normalized.Inputs}}, {{.Name}}{{end}})
+			{{if .Structured}}
+			outstruct := new(struct{ {{range .Normalized.Outputs}} {{.Name}} {{bindtype .Type $structs}}; {{end}} })
+			{{range $i, $t := .Normalized.Outputs}} 
+			outstruct.{{.Name}} = out[{{$i}}].({{bindtype .Type $structs}}){{end}}
+
+			return *outstruct, err
+			{{else}}
+			if err != nil {
+				return {{range $i, $_ := .Normalized.Outputs}}*new({{bindtype .Type $structs}}), {{end}} err
+			}
+			{{range $i, $t := .Normalized.Outputs}}
+			out{{$i}} := *abi.ConvertType(out[{{$i}}], new({{bindtype .Type $structs}})).(*{{bindtype .Type $structs}}){{end}}
+			
+			return {{range $i, $t := .Normalized.Outputs}}out{{$i}}, {{end}} err
+			{{end}}
 		}
 
 		// {{.Normalized.Name}} is a free data retrieval call binding the contract method 0x{{printf "%x" .Original.ID}}.