-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Disallow indexed reference types in events when using ABIEncoderV2 #4820
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
Conversation
libsolidity/analysis/TypeChecker.cpp
Outdated
numIndexed++; | ||
if ( | ||
_eventDef.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2) | ||
&& dynamic_cast<ReferenceType const*>(type(*var).get()) |
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.
&&
to the previous line
libsolidity/analysis/TypeChecker.cpp
Outdated
) | ||
m_errorReporter.typeError( | ||
var->location(), | ||
"Reference types cannot be 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.
Indexed reference types cannot yet be used with ABIEncoderV2
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.
Please add an endtoend test for dynamically-sized arrays both indexed and non-indexed and for both encoders (if supported) and a non-indexed struct with the new encoder.
Oh and the struct should be taken from storage. |
Ah and I think we need an entry in the bug list don't we? |
337d63a
to
d6a8b94
Compare
d6a8b94
to
c00db3c
Compare
Codecov Report
@@ Coverage Diff @@
## develop #4820 +/- ##
===========================================
+ Coverage 87.79% 87.82% +0.03%
===========================================
Files 314 314
Lines 31299 31383 +84
Branches 3702 3702
===========================================
+ Hits 27478 27562 +84
Misses 2561 2561
Partials 1260 1260
|
@chriseth Let's merge this without the json path test and I'll create a separate PR for it. |
I think this was working for strings, but not for structs. Because the actual hashing is in done in As it turned out in #5102 some tests rely on it. |
Closes #4687