-
Notifications
You must be signed in to change notification settings - Fork 175
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(dec-2020-audit): [N10] Unnecessary type cast #2316
Conversation
Signed-off-by: Christopher Maree <christopher.maree@gmail.com>
Signed-off-by: Christopher Maree <christopher.maree@gmail.com>
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 catch LGTM
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.
Looks great! This one is interesting -- why was this here in the first place?
@@ -87,9 +87,9 @@ contract ExpiringMultiPartyCreator is ContractCreator, Testable, Lockable { | |||
|
|||
_registerContract(new address[](0), address(derivative)); |
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.
Should we remove the cast on this line as well?
@@ -99,9 +99,9 @@ contract PerpetualCreator is ContractCreator, Testable, Lockable { | |||
|
|||
_registerContract(new address[](0), address(derivative)); |
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.
Same as above.
Signed-off-by: Christopher Maree <christopher.maree@gmail.com>
Problem identified in audit:
In lines 102 and 104 of PerpetualCreator.sol the derivative variable is cast to the address type. Since it is defined as address type, the casts are unnecessary.
Solution in this PR
The redundant type casting was removed. This change was also made to the EMP creator, which was not audited but the change was made for consistency.