-
-
Notifications
You must be signed in to change notification settings - Fork 810
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: improve error message for conflicting methods IDs #3134
fix: improve error message for conflicting methods IDs #3134
Conversation
…cting methods IDs
Codecov Report
@@ Coverage Diff @@
## master #3134 +/- ##
==========================================
- Coverage 88.49% 88.39% -0.11%
==========================================
Files 95 95
Lines 10760 10761 +1
Branches 2266 2266
==========================================
- Hits 9522 9512 -10
- Misses 796 806 +10
- Partials 442 443 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
interesting -- i think we should output the full function signatures (including the arguments), and also the colliding method id. ex.
|
- the 4-byte selector - their full signature
Made the changes you mentioned, one can verify that functions with default arguments are handled correctly with the following contract: @external
def OwnerTransferV7b711143(a: uint256) :
pass
@external
def withdraw(a: uint256, b: uint256 = 0):
pass
@external
def foo(a: uint256):
pass |
vyper/semantics/validation/utils.py
Outdated
x for i in functions for x in i.method_ids.keys() if i.method_ids[x] == collision | ||
) | ||
raise StructureException( | ||
f"Methods produce colliding method ID `{hex(collision)}`: {collision_str}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hex(collision)
might produce a badly formatted method ID if collision
requires fewer than 4 bytes (e.g. collision == 1
).
…n if they require fewer than 4 bytes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome, nice work. thanks!
Head branch was pushed to by a user without write access
Forgot to run mypy, sorry for that! |
What I did
Fix #3133
How I did it
I modified
collision_str
invalidate_unique_method_ids
so that we look for the collision ID in lists of IDs instead of looking for it in lists of function signatureHow to verify it
Trying to compile the following contract will now output the name of the functions having a collision:
Commit message
fix: improve error message for conflicting methods IDs
Description for the changelog
Improve error message for conflicting methods IDs
Cute Animal Picture