Skip to content

Conversation

@fselmo
Copy link
Collaborator

@fselmo fselmo commented Jul 5, 2022

What was wrong?

Contract function signatures in our error messages were only ever read as <class 'tuple'> for tuple / struct argument types. This makes it very difficult to compare user-provided arguments to the function signature.

How was it fixed?

  • Collapse tuple argument types in the identified function signature(s)
  • Collapse user-provided argument types to better compare directly to the identified function signature(s)

Todo:

Cute Animal Picture

moog_camping

@fselmo fselmo force-pushed the better-messaging-for-tuple-types branch 4 times, most recently from 4f19e5e to 59edecc Compare July 5, 2022 20:01
@fselmo fselmo marked this pull request as ready for review July 5, 2022 20:02
@fselmo fselmo requested review from kclowes and pacrob July 5, 2022 20:02
@fselmo fselmo force-pushed the better-messaging-for-tuple-types branch 2 times, most recently from 4295991 to 1a46200 Compare July 6, 2022 01:40
@fselmo fselmo changed the title Better messaging for tuple types Better messaging for wrong arguments to tuple types Jul 7, 2022
@fselmo fselmo force-pushed the better-messaging-for-tuple-types branch 3 times, most recently from 48c1e62 to ea50d26 Compare July 7, 2022 17:02
Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for updating the newsfragment README too 💥

fselmo added 2 commits July 7, 2022 14:12
- collapse tuple types into their nested types for the recognized function signature(s)
- collapse user argument types to better compare against the recognized function signature(s)
@fselmo fselmo force-pushed the better-messaging-for-tuple-types branch from ea50d26 to 0df507e Compare July 7, 2022 20:15
@fselmo fselmo merged commit d683ada into ethereum:master Jul 7, 2022
@fselmo fselmo deleted the better-messaging-for-tuple-types branch April 3, 2024 20:51
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.

2 participants