-
Notifications
You must be signed in to change notification settings - Fork 1
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
C4 audit mitigations #9
Conversation
reduce SLOAD calls in _withdrawFromVault
gas optimization
reduce SLOAD calls in _depositInVault
fix/90 prevent locking user funds in _withdrawFromVault
fix(contract): use generic proxy factory to deploy contract
@@ -261,7 +313,7 @@ contract YearnV2YieldSource is IYieldSource, ERC20Upgradeable, OwnableUpgradeabl | |||
/// @dev used to calculate how many shares to mint / burn when depositing / withdrawing | |||
/// @return tokens number of tokens equivalent (in value) to the amount of Yield Source shares | |||
function _sharesToToken(uint256 shares) internal view returns (uint256 tokens) { | |||
if(totalSupply() == 0) { | |||
if (totalSupply() == 0) { |
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.
let's pull this into a variable, as it's a public function call
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.
Done: 32b130d
@@ -248,7 +300,7 @@ contract YearnV2YieldSource is IYieldSource, ERC20Upgradeable, OwnableUpgradeabl | |||
/// @param tokens amount of tokens to be converted | |||
/// @return shares number of shares equivalent to the amount of tokens | |||
function _tokenToShares(uint256 tokens) internal view returns (uint256 shares) { | |||
if(totalSupply() == 0) { | |||
if (totalSupply() == 0) { |
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.
let's pull this into a variable
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.
Done: 32b130d
This PR fixes the following issues:
YearnV2YieldSource
wrong subtraction in withdraw code-423n4/2021-06-pooltogether-findings#90