-
Notifications
You must be signed in to change notification settings - Fork 0
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
Openzeppelin synth fixes #1
Conversation
L-03 Missing Docstrings
CR-02 Redeem and Withdraw Might Be Prevented by Enabled Controllers
Signed-off-by: Mick de Graaf <[email protected]>
CR-01 Assets Can be Lost if gulp is Called With Empty Deposit
N-11 Functions Are Updating the State Without Event Emissions
N-09 Incorrect Docstrings
N-08 Gas and Code Improvements
N-06 Non-Explicit Imports Are Used
Signed-off-by: Mick de Graaf <[email protected]>
N-05 Missing Named Parameters in Mapping
N-03 Inconsistent Use of Named Returns
Signed-off-by: Mick de Graaf <[email protected]>
N-01 Function Visibility Overly Permissive
L-02 Incomplete Docstrings
…yz/openzeppelin-synth-fixes-private into openzeppelin-synth-fixes
_withdraw(_msgSender(), receiver, owner, assets, shares); | ||
_totalAssets = _totalAssets - assets; |
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.
Shouldn't _deposit()
also be updated to match the pattern here?
function _deposit(address caller, address receiver, uint256 assets, uint256 shares) internal override { |
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.
it seems that OZ missed that when reviewing. this was Mick's PR for the issue: https://github.com/euler-xyz/openzeppelin-synth-fixes-private/pull/13/files
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.
Yeah I noticed that, I guess we either do the full fix or they should mark it as partially fixed in the report.
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.
fixed here: ca5fca5
No description provided.