Skip to content
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

RoundUp for mint, previewMint, withdraw and previewWithdraw #102

Merged
merged 9 commits into from
Dec 28, 2022

Conversation

tushargargght
Copy link
Contributor

@tushargargght tushargargght commented Dec 14, 2022

ERC-4626 expects the result returned from the following functions of Vaults to be rounded up:

function _previewWithdraw(uint256 assets) 
	internal view virtual 
	returns (uint256 shares)

function _previewMint(uint256 shares) 
	internal view virtual 
	returns (uint256 assets)

Due to how Solidity division works, the current implementation of those two functions currently rounds the result down - thus not conforming to ERC-4626 standard.

Included in this PR:

  • Round up for mint, withdraw and corresponding tests for all vaults. Note that Curve and Convex Liquidity vaults do not need round up for mint as we are anyways doing it through MINT_ADJUST in the calc library.
  • Fix for the bug where the first deposit to CurveMetaVault (or ConvexLiquidatorVaults) can lead to very large amounts of shares minted to user because of decimals mismatch between shares and _metaVaultShares
function _getSharesFromMetaVaultShares(
        uint256 _metaVaultShares,
        uint256 _totalMetaVaultShares,
        uint256 _totalShares
    ) internal pure returns (uint256 shares) {
        if (_totalMetaVaultShares == 0) {
            shares = _metaVaultShares;
        } else {
            shares = (_metaVaultShares * _totalShares) / _totalMetaVaultShares;
        }
    }
  • Several decimals value fixes in the savePlus and saveFraxPlus fork-tests

@tushargargght tushargargght self-assigned this Dec 14, 2022
@tushargargght tushargargght added the v2 mStable v2 roadmap label Dec 14, 2022
@tushargargght tushargargght marked this pull request as ready for review December 28, 2022 07:23
contracts/vault/AbstractVault.sol Show resolved Hide resolved
contracts/vault/AbstractVault.sol Show resolved Hide resolved
if (_totalShares == 0) {
metapoolTokens = _shares;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good pickup

test/shared/BaseVault.behaviour.ts Outdated Show resolved Hide resolved
@tushargargght tushargargght merged commit daf7f42 into develop Dec 28, 2022
@tushargargght tushargargght deleted the macro_222_roundUp branch December 28, 2022 09:40
@naddison36 naddison36 mentioned this pull request Jan 10, 2023
naddison36 added a commit that referenced this pull request Jan 10, 2023
* fix: npm publish action (#86)
* chore: adds publish web to publish action
* feat: fraxBP vaults (#71)
* feat: test for update assetsPerShare after removal of underlyingVault
* feat: ConvexFrax based vaults
* chore: Refactor Curve lib tests (#74)
* chore: using Hardhat network helpers for setBalance, setStorageAt and stopImpersonateAccount (#82)
* fix: missing awaits in savePlus fork tests
* refact: reduce gas consumption while donating tokens (#87)
* chore: updated docs for second audit
* chore: adds deployment address (#93)
* fix: removeVault() will block because of incorrect vaults length (#96)
* fix: modified require to use total vaults length instead of active vaults
* feat: test for nth vault removal
* fix: outdated assets amount in _redeem and _mint for PeriodicAllocationAbstractVault (#99)
* feat: recalculate assets amount of successful assetsPerShare update
* feat: tests for recalculating assetsPerShare for redeem and mint
* fix: Meta vaults cannot re-add underlying vaults after removal (#100)
* feat: removed allowance in remove vault
* fix: test for remove and re-add vault
* fix: liq-donate-tokens task with multiple rewards (#101)
* fix: liq-settle-swap task (#103)
* fix: removes death code from LiquidatorVaults (#104)
* fix: RoundUp for `mint`, `previewMint`, `withdraw` and `previewWithdraw` (#102)
* feat: contract changes for roundUp
* feat: LightBasicVault roundUp
* feat: roundUp unit-tests
* feat: 3Crv based vault changes
* feat: FraxBp based vault changes
* feat: roundUp tests and scaling bug fix
* fix: reset network before running test
* fix: [Macro] Medium and lower criticality fixes (#106)
* feat: [M-4] added check for performance fees too high during initialization
* feat: [L-2] cannot remove cached vault
* feat: [Q-1] removed zero-address check for code consistency
* feat: [Q-2] removed unused imports
* feat: [Q-3] corrected comment
* feat: [G-1] No check if performanceFee is 0
* feat: [G-3] Unnecessary control variable initialization of for loops And removed zero address check vaultManagerRole
* fix: fork-tests for sameAssetsUnderlyingVaults behavior for removeVault
* feat: [G-2] uint24 for performanceFee
* fix: uint24 performanceFee
* fix: unit-test errors
* fix: removed toNumber()
* chore: toNumber fix
* fix: minor fixes
* feat: updated diagrams
* feat: get historical token balances using token-balance HH task (#107)
* fix: liquidator retry approvals (#110)
* fix: liquidator retry approvals by pre-approving the async swapper to transfer reward tokens
* fix: typos in README (#112)

Co-authored-by: doncesarts <doncesarts@gmail.com>
Co-authored-by: tushargargght <tushargargght@gmail.com>
Co-authored-by: omahs <73983677+omahs@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2 mStable v2 roadmap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants