Skip to content

accounts/abi: Abi binding support for nested arrays, fixes #15648, including nested array unpack fix #15676

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Mar 4, 2018
Merged

accounts/abi: Abi binding support for nested arrays, fixes #15648, including nested array unpack fix #15676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
faa190b
accounts/abi/bind: support for multi-dim arrays
protolambda Dec 14, 2017
b6f0296
accounts/abi/bind: Add some more documentation
protolambda Dec 14, 2017
2444b61
accounts/abi/bind: Improve code readability
protolambda Dec 28, 2017
4ed1e02
accounts/abi: bugfix for unpacking nested arrays
protolambda Dec 28, 2017
c4bf02a
accounts/abi: Fix unpacking of nested arrays
protolambda Dec 30, 2017
016928c
accounts/abi: deeply nested array unpack test
protolambda Dec 30, 2017
e2be9e9
accounts/abi: Add deeply nested array pack test
protolambda Dec 30, 2017
7da01ea
accounts/abi/bind: deeply nested arrays bind test
protolambda Dec 30, 2017
cc3a827
accounts/abi/bind: improve array binding
protolambda Jan 6, 2018
8929a3b
accounts/abi: Improve naming of element size func
protolambda Jan 6, 2018
1536b4c
accounts/abi: support nested nested array args
protolambda Jan 25, 2018
c788f13
accounts/abi: Test deep nested arr multi-arguments
protolambda Feb 21, 2018
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
26 changes: 23 additions & 3 deletions accounts/abi/argument.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,21 @@ func (arguments Arguments) unpackAtomic(v interface{}, marshalledValues []interf
return set(elem, reflectValue, arguments.NonIndexed()[0])
}

// Computes the full size of an array;
// i.e. counting nested arrays, which count towards size for unpacking.
func getArraySize(arr *Type) int {
Copy link
Contributor

Choose a reason for hiding this comment

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

I've done a rebase on master, and find that if don't even use this method, but leave it as is, I get no test failures. I'll push my rebased branch, with the method still present (but unused), and you can take a look and check if

  • The method is not needed, or,
  • An additional testcase is needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Check out my my branch: https://github.com/ethereum/go-ethereum/compare/master...holiman:pr_15676?expand=1 . It's rebased on master. PTAL that I didn't miss something, and why it's still working without using getArraySize (I guess the corresponding place in the master-code is here: https://github.com/ethereum/go-ethereum/compare/master...holiman:pr_15676?expand=1#diff-04b076f4fee5b5afe22e6efa614e4aceR200 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We probably need tests for the argument unpacking then, it may be very uncommon, but there was definitely a bug.

How to reproduce it (will test later with your rebase, this is what needed to be fixed in the original PR starting point):

  1. Some argument value needs to be a nested array. The abi format packs the array pointers in-place, hence no standard 32 byte argument length, but a variable length based on the array.
  2. There needs to be another argument value, after the nested array, that will be offset because of the miscalculated length of the previous argument (the nested array). If there isn't any, then there is (effectively) nothing wrong. But when there is, the argument is read from the wrong position, causing array data from the array argument to be read into content of following argument(s).

So it shows up very rarely, doesn't cause a panic, and only affects multi-valued arguments.

Also consider that there can be multiple levels of nested arrays, all packed together. Hence the for-loop that multiplies them. For completeness: multiplying in the size calculation works because of the array-size symmetry, no need to recursively sum all sizes. Other collections such as slices are just pointers, same size as any other argument, so nothing wrong there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/ethereum/go-ethereum/compare/master...holiman:pr_15676?expand=1#diff-04b076f4fee5b5afe22e6efa614e4aceR200
Is indeed the relevant part, but it doesn't cover arrays with multiple levels of nesting, only the surface. I'll rebase it with the multi-level size calculation, and add a test for it 👍

size := arr.Size
// Arrays can be nested, with each element being the same size
arr = arr.Elem
for arr.T == ArrayTy {
// Keep multiplying by elem.Size while the elem is an array.
size *= arr.Size
arr = arr.Elem
}
// Now we have the full array size, including its children.
return size
}

// UnpackValues can be used to unpack ABI-encoded hexdata according to the ABI-specification,
// without supplying a struct to unpack into. Instead, this method returns a list containing the
// values. An atomic argument will be a list with one element.
Expand All @@ -181,9 +196,14 @@ func (arguments Arguments) UnpackValues(data []byte) ([]interface{}, error) {
// If we have a static array, like [3]uint256, these are coded as
// just like uint256,uint256,uint256.
// This means that we need to add two 'virtual' arguments when
// we count the index from now on

virtualArgs += arg.Type.Size - 1
// we count the index from now on.
//
// Array values nested multiple levels deep are also encoded inline:
// [2][3]uint256: uint256,uint256,uint256,uint256,uint256,uint256
//
// Calculate the full array size to get the correct offset for the next argument.
// Decrement it by 1, as the normal index increment is still applied.
virtualArgs += getArraySize(&arg.Type) - 1
}
if err != nil {
return nil, err
Expand Down
169 changes: 99 additions & 70 deletions accounts/abi/bind/bind.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,118 +164,147 @@ var bindType = map[Lang]func(kind abi.Type) string{
LangJava: bindTypeJava,
}

// Helper function for the binding generators.
// It reads the unmatched characters after the inner type-match,
// (since the inner type is a prefix of the total type declaration),
// looks for valid arrays (possibly a dynamic one) wrapping the inner type,
// and returns the sizes of these arrays.
//
// Returned array sizes are in the same order as solidity signatures; inner array size first.
// Array sizes may also be "", indicating a dynamic array.
func wrapArray(stringKind string, innerLen int, innerMapping string) (string, []string) {
Copy link
Member

Choose a reason for hiding this comment

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

It would be simpler, more readable and arguably more efficient to use the following regular expression \[(\d*)\] along with FindAllStringSubmatch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, probably a good idea. I'll simplify the implementation 👍

remainder := stringKind[innerLen:]
//find all the sizes
matches := regexp.MustCompile(`\[(\d*)\]`).FindAllStringSubmatch(remainder, -1)
parts := make([]string, 0, len(matches))
for _, match := range matches {
//get group 1 from the regex match
parts = append(parts, match[1])
}
return innerMapping, parts
}

// Translates the array sizes to a Go-lang declaration of a (nested) array of the inner type.
// Simply returns the inner type if arraySizes is empty.
func arrayBindingGo(inner string, arraySizes []string) string {
out := ""
//prepend all array sizes, from outer (end arraySizes) to inner (start arraySizes)
for i := len(arraySizes) - 1; i >= 0; i-- {
out += "[" + arraySizes[i] + "]"
}
out += inner
return out
}

// bindTypeGo converts a Solidity type to a Go one. Since there is no clear mapping
// from all Solidity types to Go ones (e.g. uint17), those that cannot be exactly
// mapped will use an upscaled type (e.g. *big.Int).
func bindTypeGo(kind abi.Type) string {
stringKind := kind.String()
innerLen, innerMapping := bindUnnestedTypeGo(stringKind)
return arrayBindingGo(wrapArray(stringKind, innerLen, innerMapping))
}

// The inner function of bindTypeGo, this finds the inner type of stringKind.
// (Or just the type itself if it is not an array or slice)
// The length of the matched part is returned, with the the translated type.
func bindUnnestedTypeGo(stringKind string) (int, string) {

switch {
case strings.HasPrefix(stringKind, "address"):
parts := regexp.MustCompile(`address(\[[0-9]*\])?`).FindStringSubmatch(stringKind)
if len(parts) != 2 {
return stringKind
}
return fmt.Sprintf("%scommon.Address", parts[1])
return len("address"), "common.Address"

case strings.HasPrefix(stringKind, "bytes"):
parts := regexp.MustCompile(`bytes([0-9]*)(\[[0-9]*\])?`).FindStringSubmatch(stringKind)
if len(parts) != 3 {
return stringKind
}
return fmt.Sprintf("%s[%s]byte", parts[2], parts[1])
parts := regexp.MustCompile(`bytes([0-9]*)`).FindStringSubmatch(stringKind)
return len(parts[0]), fmt.Sprintf("[%s]byte", parts[1])

case strings.HasPrefix(stringKind, "int") || strings.HasPrefix(stringKind, "uint"):
parts := regexp.MustCompile(`(u)?int([0-9]*)(\[[0-9]*\])?`).FindStringSubmatch(stringKind)
if len(parts) != 4 {
return stringKind
}
parts := regexp.MustCompile(`(u)?int([0-9]*)`).FindStringSubmatch(stringKind)
switch parts[2] {
case "8", "16", "32", "64":
return fmt.Sprintf("%s%sint%s", parts[3], parts[1], parts[2])
return len(parts[0]), fmt.Sprintf("%sint%s", parts[1], parts[2])
}
return fmt.Sprintf("%s*big.Int", parts[3])
return len(parts[0]), "*big.Int"

case strings.HasPrefix(stringKind, "bool") || strings.HasPrefix(stringKind, "string"):
parts := regexp.MustCompile(`([a-z]+)(\[[0-9]*\])?`).FindStringSubmatch(stringKind)
if len(parts) != 3 {
return stringKind
}
return fmt.Sprintf("%s%s", parts[2], parts[1])
case strings.HasPrefix(stringKind, "bool"):
return len("bool"), "bool"

case strings.HasPrefix(stringKind, "string"):
return len("string"), "string"

default:
return stringKind
return len(stringKind), stringKind
}
}

// Translates the array sizes to a Java declaration of a (nested) array of the inner type.
// Simply returns the inner type if arraySizes is empty.
func arrayBindingJava(inner string, arraySizes []string) string {
// Java array type declarations do not include the length.
return inner + strings.Repeat("[]", len(arraySizes))
}

// bindTypeJava converts a Solidity type to a Java one. Since there is no clear mapping
// from all Solidity types to Java ones (e.g. uint17), those that cannot be exactly
// mapped will use an upscaled type (e.g. BigDecimal).
func bindTypeJava(kind abi.Type) string {
stringKind := kind.String()
innerLen, innerMapping := bindUnnestedTypeJava(stringKind)
return arrayBindingJava(wrapArray(stringKind, innerLen, innerMapping))
}

// The inner function of bindTypeJava, this finds the inner type of stringKind.
// (Or just the type itself if it is not an array or slice)
// The length of the matched part is returned, with the the translated type.
func bindUnnestedTypeJava(stringKind string) (int, string) {

switch {
case strings.HasPrefix(stringKind, "address"):
parts := regexp.MustCompile(`address(\[[0-9]*\])?`).FindStringSubmatch(stringKind)
if len(parts) != 2 {
return stringKind
return len(stringKind), stringKind
}
if parts[1] == "" {
return fmt.Sprintf("Address")
return len("address"), "Address"
}
return fmt.Sprintf("Addresses")
return len(parts[0]), "Addresses"

case strings.HasPrefix(stringKind, "bytes"):
parts := regexp.MustCompile(`bytes([0-9]*)(\[[0-9]*\])?`).FindStringSubmatch(stringKind)
if len(parts) != 3 {
return stringKind
}
if parts[2] != "" {
return "byte[][]"
parts := regexp.MustCompile(`bytes([0-9]*)`).FindStringSubmatch(stringKind)
if len(parts) != 2 {
return len(stringKind), stringKind
}
return "byte[]"
return len(parts[0]), "byte[]"

case strings.HasPrefix(stringKind, "int") || strings.HasPrefix(stringKind, "uint"):
parts := regexp.MustCompile(`(u)?int([0-9]*)(\[[0-9]*\])?`).FindStringSubmatch(stringKind)
if len(parts) != 4 {
return stringKind
}
switch parts[2] {
case "8", "16", "32", "64":
if parts[1] == "" {
if parts[3] == "" {
return fmt.Sprintf("int%s", parts[2])
}
return fmt.Sprintf("int%s[]", parts[2])
}
//Note that uint and int (without digits) are also matched,
// these are size 256, and will translate to BigInt (the default).
parts := regexp.MustCompile(`(u)?int([0-9]*)`).FindStringSubmatch(stringKind)
if len(parts) != 3 {
return len(stringKind), stringKind
}
if parts[3] == "" {
return fmt.Sprintf("BigInt")

namedSize := map[string]string{
"8": "byte",
"16": "short",
"32": "int",
"64": "long",
}[parts[2]]

//default to BigInt
if namedSize == "" {
namedSize = "BigInt"
}
return fmt.Sprintf("BigInts")
return len(parts[0]), namedSize

case strings.HasPrefix(stringKind, "bool"):
parts := regexp.MustCompile(`bool(\[[0-9]*\])?`).FindStringSubmatch(stringKind)
if len(parts) != 2 {
return stringKind
}
if parts[1] == "" {
return fmt.Sprintf("bool")
}
return fmt.Sprintf("bool[]")
return len("bool"), "boolean"

case strings.HasPrefix(stringKind, "string"):
parts := regexp.MustCompile(`string(\[[0-9]*\])?`).FindStringSubmatch(stringKind)
if len(parts) != 2 {
return stringKind
}
if parts[1] == "" {
return fmt.Sprintf("String")
}
return fmt.Sprintf("String[]")
return len("string"), "String"

default:
return stringKind
return len(stringKind), stringKind
}
}

Expand Down Expand Up @@ -325,11 +354,13 @@ func namedTypeJava(javaKind string, solKind abi.Type) string {
return "String"
case "string[]":
return "Strings"
case "bool":
case "boolean":
return "Bool"
case "bool[]":
case "boolean[]":
return "Bools"
case "BigInt":
case "BigInt[]":
return "BigInts"
default:
parts := regexp.MustCompile(`(u)?int([0-9]*)(\[[0-9]*\])?`).FindStringSubmatch(solKind.String())
if len(parts) != 4 {
return javaKind
Expand All @@ -344,8 +375,6 @@ func namedTypeJava(javaKind string, solKind abi.Type) string {
default:
return javaKind
}
default:
return javaKind
}
}

Expand Down
66 changes: 66 additions & 0 deletions accounts/abi/bind/bind_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,72 @@ var bindTests = []struct {
}
`,
},
{
`DeeplyNestedArray`,
`
contract DeeplyNestedArray {
uint64[3][4][5] public deepUint64Array;
function storeDeepUintArray(uint64[3][4][5] arr) public {
deepUint64Array = arr;
}
function retrieveDeepArray() public view returns (uint64[3][4][5]) {
return deepUint64Array;
}
}
`,
`6060604052341561000f57600080fd5b6106438061001e6000396000f300606060405260043610610057576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff168063344248551461005c5780638ed4573a1461011457806398ed1856146101ab575b600080fd5b341561006757600080fd5b610112600480806107800190600580602002604051908101604052809291906000905b828210156101055783826101800201600480602002604051908101604052809291906000905b828210156100f25783826060020160038060200260405190810160405280929190826003602002808284378201915050505050815260200190600101906100b0565b505050508152602001906001019061008a565b5050505091905050610208565b005b341561011f57600080fd5b61012761021d565b604051808260056000925b8184101561019b578284602002015160046000925b8184101561018d5782846020020151600360200280838360005b8381101561017c578082015181840152602081019050610161565b505050509050019260010192610147565b925050509260010192610132565b9250505091505060405180910390f35b34156101b657600080fd5b6101de6004808035906020019091908035906020019091908035906020019091905050610309565b604051808267ffffffffffffffff1667ffffffffffffffff16815260200191505060405180910390f35b80600090600561021992919061035f565b5050565b6102256103b0565b6000600580602002604051908101604052809291906000905b8282101561030057838260040201600480602002604051908101604052809291906000905b828210156102ed578382016003806020026040519081016040528092919082600380156102d9576020028201916000905b82829054906101000a900467ffffffffffffffff1667ffffffffffffffff16815260200190600801906020826007010492830192600103820291508084116102945790505b505050505081526020019060010190610263565b505050508152602001906001019061023e565b50505050905090565b60008360058110151561031857fe5b600402018260048110151561032957fe5b018160038110151561033757fe5b6004918282040191900660080292509250509054906101000a900467ffffffffffffffff1681565b826005600402810192821561039f579160200282015b8281111561039e5782518290600461038e9291906103df565b5091602001919060040190610375565b5b5090506103ac919061042d565b5090565b610780604051908101604052806005905b6103c9610459565b8152602001906001900390816103c15790505090565b826004810192821561041c579160200282015b8281111561041b5782518290600361040b929190610488565b50916020019190600101906103f2565b5b5090506104299190610536565b5090565b61045691905b8082111561045257600081816104499190610562565b50600401610433565b5090565b90565b610180604051908101604052806004905b6104726105a7565b81526020019060019003908161046a5790505090565b82600380016004900481019282156105255791602002820160005b838211156104ef57835183826101000a81548167ffffffffffffffff021916908367ffffffffffffffff16021790555092602001926008016020816007010492830192600103026104a3565b80156105235782816101000a81549067ffffffffffffffff02191690556008016020816007010492830192600103026104ef565b505b50905061053291906105d9565b5090565b61055f91905b8082111561055b57600081816105529190610610565b5060010161053c565b5090565b90565b50600081816105719190610610565b50600101600081816105839190610610565b50600101600081816105959190610610565b5060010160006105a59190610610565b565b6060604051908101604052806003905b600067ffffffffffffffff168152602001906001900390816105b75790505090565b61060d91905b8082111561060957600081816101000a81549067ffffffffffffffff0219169055506001016105df565b5090565b90565b50600090555600a165627a7a7230582087e5a43f6965ab6ef7a4ff056ab80ed78fd8c15cff57715a1bf34ec76a93661c0029`,
`[{"constant":false,"inputs":[{"name":"arr","type":"uint64[3][4][5]"}],"name":"storeDeepUintArray","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[],"name":"retrieveDeepArray","outputs":[{"name":"","type":"uint64[3][4][5]"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[{"name":"","type":"uint256"},{"name":"","type":"uint256"},{"name":"","type":"uint256"}],"name":"deepUint64Array","outputs":[{"name":"","type":"uint64"}],"payable":false,"stateMutability":"view","type":"function"}]`,
`
// Generate a new random account and a funded simulator
key, _ := crypto.GenerateKey()
auth := bind.NewKeyedTransactor(key)
sim := backends.NewSimulatedBackend(core.GenesisAlloc{auth.From: {Balance: big.NewInt(10000000000)}})

//deploy the test contract
_, _, testContract, err := DeployDeeplyNestedArray(auth, sim)
if err != nil {
t.Fatalf("Failed to deploy test contract: %v", err)
}

// Finish deploy.
sim.Commit()

//Create coordinate-filled array, for testing purposes.
testArr := [5][4][3]uint64{}
for i := 0; i < 5; i++ {
testArr[i] = [4][3]uint64{}
for j := 0; j < 4; j++ {
testArr[i][j] = [3]uint64{}
for k := 0; k < 3; k++ {
//pack the coordinates, each array value will be unique, and can be validated easily.
testArr[i][j][k] = uint64(i) << 16 | uint64(j) << 8 | uint64(k)
}
}
}

if _, err := testContract.StoreDeepUintArray(&bind.TransactOpts{
From: auth.From,
Signer: auth.Signer,
}, testArr); err != nil {
t.Fatalf("Failed to store nested array in test contract: %v", err)
}

sim.Commit()

retrievedArr, err := testContract.RetrieveDeepArray(&bind.CallOpts{
From: auth.From,
Pending: false,
})
if err != nil {
t.Fatalf("Failed to retrieve nested array from test contract: %v", err)
}

//quick check to see if contents were copied
// (See accounts/abi/unpack_test.go for more extensive testing)
if retrievedArr[4][3][2] != testArr[4][3][2] {
t.Fatalf("Retrieved value does not match expected value! got: %d, expected: %d. %v", retrievedArr[4][3][2], testArr[4][3][2], err)
}`,
},
}

// Tests that packages generated by the binder can be successfully compiled and
Expand Down
5 changes: 5 additions & 0 deletions accounts/abi/pack_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,11 @@ func TestPack(t *testing.T) {
[32]byte{1},
common.Hex2Bytes("0100000000000000000000000000000000000000000000000000000000000000"),
},
{
"uint32[2][3][4]",
[4][3][2]uint32{{{1, 2}, {3, 4}, {5, 6}}, {{7, 8}, {9, 10}, {11, 12}}, {{13, 14}, {15, 16}, {17, 18}}, {{19, 20}, {21, 22}, {23, 24}}},
common.Hex2Bytes("000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000050000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000000000000000000000000000000700000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000009000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000b000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000d000000000000000000000000000000000000000000000000000000000000000e000000000000000000000000000000000000000000000000000000000000000f000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000110000000000000000000000000000000000000000000000000000000000000012000000000000000000000000000000000000000000000000000000000000001300000000000000000000000000000000000000000000000000000000000000140000000000000000000000000000000000000000000000000000000000000015000000000000000000000000000000000000000000000000000000000000001600000000000000000000000000000000000000000000000000000000000000170000000000000000000000000000000000000000000000000000000000000018"),
},
{
"address[]",
[]common.Address{{1}, {2}},
Expand Down
Loading