-
Notifications
You must be signed in to change notification settings - Fork 187
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
Ensure event emits are type checked #202
Ensure event emits are type checked #202
Conversation
6e83637
to
d2fcc99
Compare
Codecov Report
@@ Coverage Diff @@
## master #202 +/- ##
==========================================
+ Coverage 77.51% 77.73% +0.21%
==========================================
Files 46 46
Lines 3322 3323 +1
==========================================
+ Hits 2575 2583 +8
+ Misses 747 740 -7
Continue to review full report at Codecov.
|
@@ -9,7 +9,7 @@ contract Foo: | |||
s5: string100 | |||
|
|||
pub def __init__(s1: string42, a: address, s2: string26, u: u256, s3: string100): | |||
emit MyEvent(s2, u, s1, s3, a, "static string", "foo") | |||
emit MyEvent(s2, u, s1, s3, a, "static string", string100("foo")) |
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.
This came to my attention after I merged #201 so it sparked this PR to tighten our type checks.
contract Foo: | ||
event MyEvent: | ||
val_1: string100 | ||
val_2: u8 |
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.
for sanity it might be good to mark one of these as indexed
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.
Good point 👍
d2fcc99
to
afb812a
Compare
What was wrong?
On master, the following would not raise an error even though it should:
How was it fixed?
Basically reuse the
call_arg
that is defined inexpression
and a little bit of refactoring the event to expose all field types.To-Do
OPTIONAL: Update Spec if applicable
Add entry to the release notes (may forgo for trivial changes)
Clean up commit history