Skip to content

accounts/abi: add array length after parsing array#15618

Merged
fjl merged 3 commits into
ethereum:masterfrom
dshulyak:arrayunpack
Dec 20, 2017
Merged

accounts/abi: add array length after parsing array#15618
fjl merged 3 commits into
ethereum:masterfrom
dshulyak:arrayunpack

Conversation

@dshulyak
Copy link
Copy Markdown
Contributor

@dshulyak dshulyak commented Dec 6, 2017

solves: #15617

In current codebase array fields are counted before unpacking output.
So, if we have array with 2 fields: toGoType function will try to unpack
values from 64 till 64 + 2*32 which is not correct in my opinion.

I shifted counting to be done after processing array,
so in case array is the first field,
and it has 2 elements: we will read it from 0 till 2*32.

Additionally array doesnt require length prefix, so field that follows array
should be read from 64 byte.
This is why i made additional substitution in this change.
@dshulyak
Copy link
Copy Markdown
Contributor Author

@karalabe @fjl folks, perhaps you can help with review?

@holiman
Copy link
Copy Markdown
Contributor

holiman commented Dec 19, 2017

@dshulyak I'm looking into this.. but how does this PR relate to #15568 ? They at least partially overlap, right? And if so, do you still want this one merged?

@dshulyak
Copy link
Copy Markdown
Contributor Author

@holiman yes, I want this one to be merged. And I will abandon #15568 in favor of #15452.

@fjl fjl requested a review from gballet December 19, 2017 13:11
@fjl fjl merged commit da58afc into ethereum:master Dec 20, 2017
@karalabe karalabe added this to the 1.8.0 milestone Dec 20, 2017
b00ris pushed a commit to b00ris/go-ethereum that referenced this pull request Jan 19, 2018
mariameda pushed a commit to NiluPlatform/go-nilu that referenced this pull request Aug 23, 2018
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.

5 participants