-
Notifications
You must be signed in to change notification settings - Fork 422
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
Remediate Trevor's comments #2835
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## v3 #2835 +/- ##
==========================================
+ Coverage 62.07% 62.53% +0.46%
==========================================
Files 99 99
Lines 1002 1017 +15
Branches 104 106 +2
==========================================
+ Hits 622 636 +14
- Misses 343 344 +1
Partials 37 37
|
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 populate the description with more specific explanations
solidity/contracts/isms/hook/AbstractMessageIdAuthorizedIsm.sol
Outdated
Show resolved
Hide resolved
solidity/contracts/isms/hook/AbstractMessageIdAuthorizedIsm.sol
Outdated
Show resolved
Hide resolved
solidity/contracts/isms/hook/AbstractMessageIdAuthorizedIsm.sol
Outdated
Show resolved
Hide resolved
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 populate the description and title with more specific explanations of changes
feel free to copy in @tkporter's comments
Description
Addressing @tkporter 's comments from here: #2733
msg.value
exploit onAbstractMessageIdAuthorizedIsm
Drive-by changes
none
Related issues
none
Backward compatibility
No
Testing
Unit