-
Notifications
You must be signed in to change notification settings - Fork 10
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
refactor: respond to PR comment by 0xbok #198
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #198 +/- ##
==========================================
- Coverage 85.85% 85.69% -0.16%
==========================================
Files 14 14
Lines 933 923 -10
Branches 275 251 -24
==========================================
- Hits 801 791 -10
Misses 117 117
Partials 15 15
Continue to review full report in Codecov by Sentry.
|
refactor: respond to PR comment by 0xbok
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
🤖 Slither Analysis Report 🔎Slither report
# Slither report
_This comment was automatically generated by the GitHub Actions workflow._
THIS CHECKLIST IS NOT COMPLETE. Use
locked-ether🟡 Impact: Medium
utils/NexusBootstrap.sol#L33-L165 unused-return🟡 Impact: Medium
base/ERC7739Validator.sol#L279-L309 constable-statesImpact: Optimization
|
// If recovery fails, we'll return false | ||
} | ||
|
||
// owner can not be zero address in this contract |
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.
can't the owner be zero here: https://github.com/bcnmy/nexus/pull/198/files#diff-0de611a725083823f818063145fae9df78b92f67f62ed9528d5337dd17aa7e10R74-R76 ?
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.
oh, I thought we had zero address check in place. I think we can enforce this in onInstall and create informational issue. @0xbok
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.
No description provided.