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

ABI explicit difference between Unpack and UnpackIntoInterface #21091

Merged
merged 20 commits into from
Sep 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
199bfba
accounts/abi: refactored abi.Unpack
MariusVanDerWijden May 14, 2020
9b90470
accounts/abi/bind: fixed error
MariusVanDerWijden May 15, 2020
8946e6e
accounts/abi/bind: modified template
MariusVanDerWijden May 28, 2020
5006a51
accounts/abi/bind: added ToStruct for conversion
MariusVanDerWijden Jun 8, 2020
c2e7250
accounts/abi: reenabled tests
MariusVanDerWijden Jun 16, 2020
a2e0db2
accounts/abi: fixed tests
MariusVanDerWijden Jun 16, 2020
e87a6be
accounts/abi: fixed tests for packing/unpacking
MariusVanDerWijden Jun 30, 2020
f3e714c
accounts/abi: fixed tests
MariusVanDerWijden Jul 1, 2020
7e08e31
accounts/abi: added more logic to ToStruct
MariusVanDerWijden Jul 2, 2020
d2e5302
accounts/abi/bind: fixed template
MariusVanDerWijden Jul 2, 2020
e442a50
accounts/abi/bind: fixed ToStruct conversion
MariusVanDerWijden Jul 2, 2020
36439b9
accounts/abi/: removed unused code
MariusVanDerWijden Jul 6, 2020
5bbdaba
accounts/abi: updated template
MariusVanDerWijden Jul 13, 2020
9dd8e6d
accounts/abi: refactored unused code
MariusVanDerWijden Jul 13, 2020
e95dfd7
contracts/checkpointoracle: updated contracts to sol ^0.6.0
MariusVanDerWijden Jul 13, 2020
644bc14
accounts/abi: refactored reflection logic
MariusVanDerWijden Jul 13, 2020
f74d73c
accounts/abi: less code duplication in Unpack*
MariusVanDerWijden Jul 14, 2020
368a952
accounts/abi: fixed rebasing bug
MariusVanDerWijden Aug 7, 2020
56328d7
fix a few typos in comments
gballet Sep 15, 2020
6d1d680
rebase on master
MariusVanDerWijden Sep 24, 2020
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
60 changes: 40 additions & 20 deletions accounts/abi/abi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
17 changes: 7 additions & 10 deletions accounts/abi/abi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down
54 changes: 32 additions & 22 deletions accounts/abi/argument.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)

Expand All @@ -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()

Expand Down
15 changes: 11 additions & 4 deletions accounts/abi/bind/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

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

that doesn't seem necessary since you have another check for len(results) == 0 before results is used for the first time.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately not, as I have to dereference result in the length check. len(*results) == 0, If results is nil the dereference panics. It is valid for the user to pass in a valid slice pointer with length 0 which we will fill with our result. If he's lazy, he might pass nil which means he doesn't need the result.

results = new([]interface{})
}
// Pack the input, call and unpack the results
input, err := c.abi.Pack(method, params...)
if err != nil {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
}
Expand Down
7 changes: 3 additions & 4 deletions accounts/abi/bind/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand Down
4 changes: 2 additions & 2 deletions accounts/abi/bind/bind_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member Author

Choose a reason for hiding this comment

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

Remove after work is done


pkg := filepath.Join(ws, "bindtest")
if err = os.MkdirAll(pkg, 0700); err != nil {
Expand Down
34 changes: 19 additions & 15 deletions accounts/abi/bind/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
}

Expand All @@ -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...)
}

Expand All @@ -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}}.
Expand Down
Loading