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

accounts/abi: fix setStruct if src is pointer (e.g. big.Int) #23557

Conversation

MariusVanDerWijden
Copy link
Member

closes #23551

The issue here was that we were trying to setStruct from a struct to a pointer to a struct, so we need to dereference the pointer first

@rjl493456442
Copy link
Member

rjl493456442 commented Sep 13, 2021

I tried to reproduce the issue. So it's buggy when we try to assign the resolved value into the receiver.

More specifically, we want to assign struct { A *big.Int} into *contract.ContractStructEvent. Since there is only a single returned value, we enter this path https://github.com/ethereum/go-ethereum/blob/master/accounts/abi/argument.go#L132
Because the receiver is a struct, we should get the first field of struct as the nested value receiver.

So I think the buggy part is here

if dst.Kind() == reflect.Struct && src.Kind() != reflect.Struct {
	return set(dst.Field(0), src)
}

We should use the field(0) anyway no matter the type of resolved value.
Should we fix this issue in the above approach?

if dst.Kind() == reflect.Struct {
	return set(dst.Field(0), src)
}

@rjl493456442
Copy link
Member

diff --git a/accounts/abi/argument.go b/accounts/abi/argument.go
index e6d524559..261b4d1b8 100644
--- a/accounts/abi/argument.go
+++ b/accounts/abi/argument.go
@@ -137,7 +137,7 @@ func (arguments Arguments) copyAtomic(v interface{}, marshalledValues interface{
 	dst := reflect.ValueOf(v).Elem()
 	src := reflect.ValueOf(marshalledValues)
 
-	if dst.Kind() == reflect.Struct && src.Kind() != reflect.Struct {
+	if dst.Kind() == reflect.Struct {
 		return set(dst.Field(0), src)
 	}
 	return set(dst, src)
diff --git a/accounts/abi/reflect.go b/accounts/abi/reflect.go
index 11248e073..7af0a1b26 100644
--- a/accounts/abi/reflect.go
+++ b/accounts/abi/reflect.go
@@ -159,7 +159,7 @@ func setArray(dst, src reflect.Value) error {
 		dst.Set(array)
 		return nil
 	}
-	return errors.New("Cannot set array, destination not settable")
+	return errors.New("cannot set array, destination not settable")
 }
 
 func setStruct(dst, src reflect.Value) error {
@@ -167,7 +167,7 @@ func setStruct(dst, src reflect.Value) error {
 		srcField := src.Field(i)
 		dstField := dst.Field(i)
 		if !dstField.IsValid() || !srcField.IsValid() {
-			return fmt.Errorf("Could not find src field: %v value: %v in destination", srcField.Type().Name(), srcField)
+			return fmt.Errorf("could not find src field: %v value: %v in destination", srcField.Type().Name(), srcField)
 		}
 		if err := set(dstField, srcField); err != nil {
 			return err
diff --git a/accounts/abi/unpack_test.go b/accounts/abi/unpack_test.go
index b88f77805..76e43b598 100644
--- a/accounts/abi/unpack_test.go
+++ b/accounts/abi/unpack_test.go
@@ -762,20 +762,23 @@ func TestUnpackTuple(t *testing.T) {
 	buff.Write(common.Hex2Bytes("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")) // ret[b] = -1
 
 	// If the result is single tuple, use struct as return value container directly.
-	v := struct {
+	type v struct {
 		A *big.Int
 		B *big.Int
-	}{new(big.Int), new(big.Int)}
-
-	err = abi.UnpackIntoInterface(&v, "tuple", buff.Bytes())
+	}
+	type r struct {
+		Result v
+	}
+	var ret0 = new(r)
+	err = abi.UnpackIntoInterface(ret0, "tuple", buff.Bytes())
 	if err != nil {
 		t.Error(err)
 	} else {
-		if v.A.Cmp(big.NewInt(1)) != 0 {
-			t.Errorf("unexpected value unpacked: want %x, got %x", 1, v.A)
+		if ret0.Result.A.Cmp(big.NewInt(1)) != 0 {
+			t.Errorf("unexpected value unpacked: want %x, got %x", 1, ret0.Result.A)
 		}
-		if v.B.Cmp(big.NewInt(-1)) != 0 {
-			t.Errorf("unexpected value unpacked: want %x, got %x", -1, v.B)
+		if ret0.Result.B.Cmp(big.NewInt(-1)) != 0 {
+			t.Errorf("unexpected value unpacked: want %x, got %x", -1, ret0.Result.B)
 		}
 	}

@MariusVanDerWijden
Copy link
Member Author

@rjl493456442 If that works, that would be great. I'm not super great with go reflection.
You can just create a pr with your fix and we close this one

@MariusVanDerWijden
Copy link
Member Author

closed in favor of #23573

@MariusVanDerWijden MariusVanDerWijden deleted the fix-event-struct branch November 30, 2021 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

abigen cannot parse events with struct arguments
2 participants