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

Fix tuples handled incorrectly in decode_function_input #1410

Merged
merged 2 commits into from
Aug 5, 2019

Conversation

davesque
Copy link
Contributor

@davesque davesque commented Aug 5, 2019

What was wrong?

Fixes #1409 .

How was it fixed?

Canonical tuple type strings weren't being properly generated in decode_function_input.

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@davesque
Copy link
Contributor Author

davesque commented Aug 5, 2019

This should fix the immediate issue. Going to see if the tests pass and also look for any other places where type strings aren't being generated correctly.

@davesque davesque changed the title [WIP] Fix tuples handled incorrectly in decode_function_input Fix tuples handled incorrectly in decode_function_input Aug 5, 2019
@davesque davesque requested review from kclowes and carver August 5, 2019 19:42
@davesque
Copy link
Contributor Author

davesque commented Aug 5, 2019

I think this is good to merge with approvals.

@pipermerriam
Copy link
Member

Any chance there's a test that could have caught this?

@davesque
Copy link
Contributor Author

davesque commented Aug 5, 2019

@pipermerriam Yeah, good point. Let me add one.

@kclowes
Copy link
Collaborator

kclowes commented Aug 5, 2019

Ack, that parity failure is a flaky test. You should be able to re-run and get a pass

@davesque
Copy link
Contributor Author

davesque commented Aug 5, 2019

@pipermerriam I thought about adding more complex tests, but I think that functionality is already covered by the tests that were added when tuple support was merged. It seems like the failing example from #1409 is good enough to ensure that get_abi_input_types is being called, which is what we should be testing for.

Not sure what that failure's all about. Seems unrelated to this change.

Copy link
Collaborator

@carver carver left a comment

Choose a reason for hiding this comment

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

I'm not deep in this right now, but it looks fine to me! 👍

@ankitchiplunkar
Copy link
Contributor

thanks a lot @davesque

@davesque
Copy link
Contributor Author

davesque commented Aug 5, 2019

@ankitchiplunkar No prob, cheers!

@kclowes kclowes merged commit b899050 into ethereum:master Aug 5, 2019
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.

Cannot decode tuple types
5 participants