From 547ab356ca0cbd5b206011f78398ba48a71ea0b5 Mon Sep 17 00:00:00 2001 From: sam bacha Date: Sun, 19 Dec 2021 22:40:30 -0800 Subject: [PATCH 1/6] chore(deps): update dependencies and CI (#26) * chore(deps): update dependencies This updates the contract vault mapping to the new github repo location * ci(gh): update workflow update nodejs version, ensure github actions processes are not running multiple processes. --- .github/workflows/test.yaml | 16 ++++++++++++---- brownie-config.yml | 4 ++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index e69f77e..164d678 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -7,12 +7,20 @@ on: - develop pull_request: + jobs: test: - runs-on: ubuntu-latest + name: Node ${{ matrix.node }} on ${{ matrix.os }} + runs-on: ${{ matrix.os }} + + strategy: + fail-fast: false + matrix: + node: ['14.x'] + os: ['ubuntu-latest'] steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - name: Cache compiler installations uses: actions/cache@v2 @@ -23,9 +31,9 @@ jobs: key: ${{ runner.os }}-compiler-cache - name: Setup node.js - uses: actions/setup-node@v1 + uses: actions/setup-node@v2 with: - node-version: '12.x' + node-version: '14.x' - name: Install ganache run: npm install -g ganache-cli@6.12.1 diff --git a/brownie-config.yml b/brownie-config.yml index 03e3ea9..d386fdb 100644 --- a/brownie-config.yml +++ b/brownie-config.yml @@ -8,7 +8,7 @@ autofetch_sources: True # require OpenZepplin Contracts dependencies: - - iearn-finance/yearn-vaults@0.4.3 + - yearn/yearn-vaults@0.4.3 - OpenZeppelin/openzeppelin-contracts@3.1.0 # path remapping to support imports from GitHub/NPM @@ -16,7 +16,7 @@ compiler: solc: version: 0.6.12 remappings: - - "@yearnvaults=iearn-finance/yearn-vaults@0.4.3" + - "@yearnvaults=yearn/yearn-vaults@0.4.3" - "@openzeppelin=OpenZeppelin/openzeppelin-contracts@3.1.0" reports: From 29de97eddd46c46e4280fed32161e80f1bc854ac Mon Sep 17 00:00:00 2001 From: panda <87183122+pandadefi@users.noreply.github.com> Date: Fri, 2 Dec 2022 14:06:41 +0200 Subject: [PATCH 2/6] include basewrapper in the repo --- brownie-config.yml | 4 +- contracts/AffiliateToken.sol | 3 +- contracts/BaseWrapper.sol | 311 +++++++++++++++++++++++++++++++++++ requirements-dev.txt | 4 +- 4 files changed, 316 insertions(+), 6 deletions(-) create mode 100644 contracts/BaseWrapper.sol diff --git a/brownie-config.yml b/brownie-config.yml index d386fdb..5c77119 100644 --- a/brownie-config.yml +++ b/brownie-config.yml @@ -9,7 +9,7 @@ autofetch_sources: True # require OpenZepplin Contracts dependencies: - yearn/yearn-vaults@0.4.3 - - OpenZeppelin/openzeppelin-contracts@3.1.0 + - OpenZeppelin/openzeppelin-contracts@3.4.2 # path remapping to support imports from GitHub/NPM compiler: @@ -17,7 +17,7 @@ compiler: version: 0.6.12 remappings: - "@yearnvaults=yearn/yearn-vaults@0.4.3" - - "@openzeppelin=OpenZeppelin/openzeppelin-contracts@3.1.0" + - "@openzeppelin=OpenZeppelin/openzeppelin-contracts@3.4.2" reports: exclude_contracts: diff --git a/contracts/AffiliateToken.sol b/contracts/AffiliateToken.sol index a6d7f4e..0e6200b 100644 --- a/contracts/AffiliateToken.sol +++ b/contracts/AffiliateToken.sol @@ -2,8 +2,7 @@ pragma solidity ^0.6.12; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; - -import {VaultAPI, BaseWrapper} from "@yearnvaults/contracts/BaseWrapper.sol"; +import {VaultAPI, BaseWrapper} from "./BaseWrapper.sol"; contract AffiliateToken is ERC20, BaseWrapper { /// @notice The EIP-712 typehash for the contract's domain diff --git a/contracts/BaseWrapper.sol b/contracts/BaseWrapper.sol new file mode 100644 index 0000000..57ff028 --- /dev/null +++ b/contracts/BaseWrapper.sol @@ -0,0 +1,311 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.6.12; +pragma experimental ABIEncoderV2; + +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/SafeERC20.sol"; +import {Math} from "@openzeppelin/contracts/math/Math.sol"; +import {SafeMath} from "@openzeppelin/contracts/math/SafeMath.sol"; + +import {VaultAPI} from "@yearnvaults/contracts/BaseStrategy.sol"; + +interface RegistryAPI { + function governance() external view returns (address); + + function latestVault(address token) external view returns (address); + + function numVaults(address token) external view returns (uint256); + + function vaults(address token, uint256 deploymentId) external view returns (address); +} + +/** + * @title Yearn Base Wrapper + * @author yearn.finance + * @notice + * BaseWrapper implements all of the required functionality to interoperate + * closely with the Vault contract. This contract should be inherited and the + * abstract methods implemented to adapt the Wrapper. + * A good starting point to build a wrapper is https://github.com/yearn/brownie-wrapper-mix + * + */ +abstract contract BaseWrapper { + using Math for uint256; + using SafeMath for uint256; + using SafeERC20 for IERC20; + + IERC20 public token; + + // Reduce number of external calls (SLOADs stay the same) + VaultAPI[] private _cachedVaults; + + RegistryAPI public registry; + + // ERC20 Unlimited Approvals (short-circuits VaultAPI.transferFrom) + uint256 constant UNLIMITED_APPROVAL = type(uint256).max; + // Sentinal values used to save gas on deposit/withdraw/migrate + // NOTE: DEPOSIT_EVERYTHING == WITHDRAW_EVERYTHING == MIGRATE_EVERYTHING + uint256 constant DEPOSIT_EVERYTHING = type(uint256).max; + uint256 constant WITHDRAW_EVERYTHING = type(uint256).max; + uint256 constant MIGRATE_EVERYTHING = type(uint256).max; + // VaultsAPI.depositLimit is unlimited + uint256 constant UNCAPPED_DEPOSITS = type(uint256).max; + + constructor(address _token, address _registry) public { + // Recommended to use a token with a `Registry.latestVault(_token) != address(0)` + token = IERC20(_token); + // Recommended to use `v2.registry.ychad.eth` + registry = RegistryAPI(_registry); + } + + /** + * @notice + * Used to update the yearn registry. + * @param _registry The new _registry address. + */ + function setRegistry(address _registry) external { + require(msg.sender == registry.governance()); + // In case you want to override the registry instead of re-deploying + registry = RegistryAPI(_registry); + // Make sure there's no change in governance + // NOTE: Also avoid bricking the wrapper from setting a bad registry + require(msg.sender == registry.governance()); + } + + /** + * @notice + * Used to get the most revent vault for the token using the registry. + * @return An instance of a VaultAPI + */ + function bestVault() public view virtual returns (VaultAPI) { + return VaultAPI(registry.latestVault(address(token))); + } + + /** + * @notice + * Used to get all vaults from the registery for the token + * @return An array containing instances of VaultAPI + */ + function allVaults() public view virtual returns (VaultAPI[] memory) { + uint256 cache_length = _cachedVaults.length; + uint256 num_vaults = registry.numVaults(address(token)); + + // Use cached + if (cache_length == num_vaults) { + return _cachedVaults; + } + + VaultAPI[] memory vaults = new VaultAPI[](num_vaults); + + for (uint256 vault_id = 0; vault_id < cache_length; vault_id++) { + vaults[vault_id] = _cachedVaults[vault_id]; + } + + for (uint256 vault_id = cache_length; vault_id < num_vaults; vault_id++) { + vaults[vault_id] = VaultAPI(registry.vaults(address(token), vault_id)); + } + + return vaults; + } + + function _updateVaultCache(VaultAPI[] memory vaults) internal { + // NOTE: even though `registry` is update-able by Yearn, the intended behavior + // is that any future upgrades to the registry will replay the version + // history so that this cached value does not get out of date. + if (vaults.length > _cachedVaults.length) { + _cachedVaults = vaults; + } + } + + /** + * @notice + * Used to get the balance of an account accross all the vaults for a token. + * @dev will be used to get the wrapper balance using totalVaultBalance(address(this)). + * @param account The address of the account. + * @return balance of token for the account accross all the vaults. + */ + function totalVaultBalance(address account) public view returns (uint256 balance) { + VaultAPI[] memory vaults = allVaults(); + + for (uint256 id = 0; id < vaults.length; id++) { + balance = balance.add(vaults[id].balanceOf(account).mul(vaults[id].pricePerShare()).div(10**uint256(vaults[id].decimals()))); + } + } + + /** + * @notice + * Used to get the TVL on the underlying vaults. + * @return assets the sum of all the assets managed by the underlying vaults. + */ + function totalAssets() public view returns (uint256 assets) { + VaultAPI[] memory vaults = allVaults(); + + for (uint256 id = 0; id < vaults.length; id++) { + assets = assets.add(vaults[id].totalAssets()); + } + } + + function _deposit( + address depositor, + address receiver, + uint256 amount, // if `MAX_UINT256`, just deposit everything + bool pullFunds // If true, funds need to be pulled from `depositor` via `transferFrom` + ) internal returns (uint256 deposited) { + VaultAPI _bestVault = bestVault(); + + if (pullFunds) { + if (amount != DEPOSIT_EVERYTHING) { + token.safeTransferFrom(depositor, address(this), amount); + } else { + token.safeTransferFrom(depositor, address(this), token.balanceOf(depositor)); + } + } + + if (token.allowance(address(this), address(_bestVault)) < amount) { + token.safeApprove(address(_bestVault), 0); // Avoid issues with some tokens requiring 0 + token.safeApprove(address(_bestVault), UNLIMITED_APPROVAL); // Vaults are trusted + } + + // Depositing returns number of shares deposited + // NOTE: Shortcut here is assuming the number of tokens deposited is equal to the + // number of shares credited, which helps avoid an occasional multiplication + // overflow if trying to adjust the number of shares by the share price. + uint256 beforeBal = token.balanceOf(address(this)); + if (receiver != address(this)) { + _bestVault.deposit(amount, receiver); + } else if (amount != DEPOSIT_EVERYTHING) { + _bestVault.deposit(amount); + } else { + _bestVault.deposit(); + } + + uint256 afterBal = token.balanceOf(address(this)); + deposited = beforeBal.sub(afterBal); + // `receiver` now has shares of `_bestVault` as balance, converted to `token` here + // Issue a refund if not everything was deposited + if (depositor != address(this) && afterBal > 0) token.safeTransfer(depositor, afterBal); + } + + function _withdraw( + address sender, + address receiver, + uint256 amount, // if `MAX_UINT256`, just withdraw everything + bool withdrawFromBest // If true, also withdraw from `_bestVault` + ) internal returns (uint256 withdrawn) { + VaultAPI _bestVault = bestVault(); + + VaultAPI[] memory vaults = allVaults(); + _updateVaultCache(vaults); + + // NOTE: This loop will attempt to withdraw from each Vault in `allVaults` that `sender` + // is deposited in, up to `amount` tokens. The withdraw action can be expensive, + // so it if there is a denial of service issue in withdrawing, the downstream usage + // of this wrapper contract must give an alternative method of withdrawing using + // this function so that `amount` is less than the full amount requested to withdraw + // (e.g. "piece-wise withdrawals"), leading to less loop iterations such that the + // DoS issue is mitigated (at a tradeoff of requiring more txns from the end user). + for (uint256 id = 0; id < vaults.length; id++) { + if (!withdrawFromBest && vaults[id] == _bestVault) { + continue; // Don't withdraw from the best + } + + // Start with the total shares that `sender` has + uint256 availableShares = vaults[id].balanceOf(sender); + + // Restrict by the allowance that `sender` has to this contract + // NOTE: No need for allowance check if `sender` is this contract + if (sender != address(this)) { + availableShares = Math.min(availableShares, vaults[id].allowance(sender, address(this))); + } + + // Limit by maximum withdrawal size from each vault + availableShares = Math.min(availableShares, vaults[id].maxAvailableShares()); + + if (availableShares > 0) { + // Intermediate step to move shares to this contract before withdrawing + // NOTE: No need for share transfer if this contract is `sender` + if (sender != address(this)) vaults[id].transferFrom(sender, address(this), availableShares); + + if (amount != WITHDRAW_EVERYTHING) { + // Compute amount to withdraw fully to satisfy the request + uint256 estimatedShares = amount + .sub(withdrawn) // NOTE: Changes every iteration + .mul(10**uint256(vaults[id].decimals())) + .div(vaults[id].pricePerShare()); // NOTE: Every Vault is different + + // Limit amount to withdraw to the maximum made available to this contract + // NOTE: Avoid corner case where `estimatedShares` isn't precise enough + // NOTE: If `0 < estimatedShares < 1` but `availableShares > 1`, this will withdraw more than necessary + if (estimatedShares > 0 && estimatedShares < availableShares) { + withdrawn = withdrawn.add(vaults[id].withdraw(estimatedShares)); + } else { + withdrawn = withdrawn.add(vaults[id].withdraw(availableShares)); + } + } else { + withdrawn = withdrawn.add(vaults[id].withdraw()); + } + + // Check if we have fully satisfied the request + // NOTE: use `amount = WITHDRAW_EVERYTHING` for withdrawing everything + if (amount <= withdrawn) break; // withdrawn as much as we needed + } + } + + // If we have extra, deposit back into `_bestVault` for `sender` + // NOTE: Invariant is `withdrawn <= amount` + if (withdrawn > amount && withdrawn.sub(amount) > _bestVault.pricePerShare().div(10**_bestVault.decimals())) { + // Don't forget to approve the deposit + if (token.allowance(address(this), address(_bestVault)) < withdrawn.sub(amount)) { + token.safeApprove(address(_bestVault), UNLIMITED_APPROVAL); // Vaults are trusted + } + + _bestVault.deposit(withdrawn.sub(amount), sender); + withdrawn = amount; + } + + // `receiver` now has `withdrawn` tokens as balance + if (receiver != address(this)) token.safeTransfer(receiver, withdrawn); + } + + function _migrate(address account) internal returns (uint256) { + return _migrate(account, MIGRATE_EVERYTHING); + } + + function _migrate(address account, uint256 amount) internal returns (uint256) { + // NOTE: In practice, it was discovered that <50 was the maximum we've see for this variance + return _migrate(account, amount, 0); + } + + function _migrate( + address account, + uint256 amount, + uint256 maxMigrationLoss + ) internal returns (uint256 migrated) { + VaultAPI _bestVault = bestVault(); + + // NOTE: Only override if we aren't migrating everything + uint256 _depositLimit = _bestVault.depositLimit(); + uint256 _totalAssets = _bestVault.totalAssets(); + if (_depositLimit <= _totalAssets) return 0; // Nothing to migrate (not a failure) + + uint256 _amount = amount; + if (_depositLimit < UNCAPPED_DEPOSITS && _amount < WITHDRAW_EVERYTHING) { + // Can only deposit up to this amount + uint256 _depositLeft = _depositLimit.sub(_totalAssets); + if (_amount > _depositLeft) _amount = _depositLeft; + } + + if (_amount > 0) { + // NOTE: `false` = don't withdraw from `_bestVault` + uint256 withdrawn = _withdraw(account, address(this), _amount, false); + if (withdrawn == 0) return 0; // Nothing to migrate (not a failure) + + // NOTE: `false` = don't do `transferFrom` because it's already local + migrated = _deposit(address(this), account, withdrawn, false); + // NOTE: Due to the precision loss of certain calculations, there is a small inefficency + // on how migrations are calculated, and this could lead to a DoS issue. Hence, this + // value is made to be configurable to allow the user to specify how much is acceptable + require(withdrawn.sub(migrated) <= maxMigrationLoss); + } // else: nothing to migrate! (not a failure) + } +} diff --git a/requirements-dev.txt b/requirements-dev.txt index ddcd023..3b29818 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,2 +1,2 @@ -black==21.8b0 -eth-brownie>=1.16.3,<2.0.0 +black==22.10.0 +eth-brownie>=1.19.2,<2.0.0 From cb371541d7647653eba5250f437f545a8b0e8ea0 Mon Sep 17 00:00:00 2001 From: panda <87183122+pandadefi@users.noreply.github.com> Date: Fri, 2 Dec 2022 14:11:08 +0200 Subject: [PATCH 3/6] fix: lint --- tests/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index e5f32fe..e6e41ec 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -75,7 +75,7 @@ def create_vault(token, releaseDelta=0, governance=gov): ) vault = Vault.at(tx.return_value) - vault.setDepositLimit(2 ** 256 - 1, {"from": governance}) + vault.setDepositLimit(2**256 - 1, {"from": governance}) return vault yield create_vault @@ -99,7 +99,7 @@ def sign_token_permit( token, owner: Account, # NOTE: Must be a eth_key account, not Brownie spender: str, - allowance: int = 2 ** 256 - 1, # Allowance to set with `permit` + allowance: int = 2**256 - 1, # Allowance to set with `permit` deadline: int = 0, # 0 means no time limit override_nonce: int = None, ): From b186d1bf18efd833bef416b743bb561026c2caa5 Mon Sep 17 00:00:00 2001 From: panda <87183122+pandadefi@users.noreply.github.com> Date: Fri, 2 Dec 2022 14:12:09 +0200 Subject: [PATCH 4/6] fix: lint --- contracts/BaseWrapper.sol | 93 +++++++++++++++++++++++++++++++-------- 1 file changed, 75 insertions(+), 18 deletions(-) diff --git a/contracts/BaseWrapper.sol b/contracts/BaseWrapper.sol index 57ff028..02c454c 100644 --- a/contracts/BaseWrapper.sol +++ b/contracts/BaseWrapper.sol @@ -16,7 +16,10 @@ interface RegistryAPI { function numVaults(address token) external view returns (uint256); - function vaults(address token, uint256 deploymentId) external view returns (address); + function vaults(address token, uint256 deploymentId) + external + view + returns (address); } /** @@ -101,8 +104,14 @@ abstract contract BaseWrapper { vaults[vault_id] = _cachedVaults[vault_id]; } - for (uint256 vault_id = cache_length; vault_id < num_vaults; vault_id++) { - vaults[vault_id] = VaultAPI(registry.vaults(address(token), vault_id)); + for ( + uint256 vault_id = cache_length; + vault_id < num_vaults; + vault_id++ + ) { + vaults[vault_id] = VaultAPI( + registry.vaults(address(token), vault_id) + ); } return vaults; @@ -124,11 +133,20 @@ abstract contract BaseWrapper { * @param account The address of the account. * @return balance of token for the account accross all the vaults. */ - function totalVaultBalance(address account) public view returns (uint256 balance) { + function totalVaultBalance(address account) + public + view + returns (uint256 balance) + { VaultAPI[] memory vaults = allVaults(); for (uint256 id = 0; id < vaults.length; id++) { - balance = balance.add(vaults[id].balanceOf(account).mul(vaults[id].pricePerShare()).div(10**uint256(vaults[id].decimals()))); + balance = balance.add( + vaults[id] + .balanceOf(account) + .mul(vaults[id].pricePerShare()) + .div(10**uint256(vaults[id].decimals())) + ); } } @@ -157,7 +175,11 @@ abstract contract BaseWrapper { if (amount != DEPOSIT_EVERYTHING) { token.safeTransferFrom(depositor, address(this), amount); } else { - token.safeTransferFrom(depositor, address(this), token.balanceOf(depositor)); + token.safeTransferFrom( + depositor, + address(this), + token.balanceOf(depositor) + ); } } @@ -183,7 +205,8 @@ abstract contract BaseWrapper { deposited = beforeBal.sub(afterBal); // `receiver` now has shares of `_bestVault` as balance, converted to `token` here // Issue a refund if not everything was deposited - if (depositor != address(this) && afterBal > 0) token.safeTransfer(depositor, afterBal); + if (depositor != address(this) && afterBal > 0) + token.safeTransfer(depositor, afterBal); } function _withdraw( @@ -215,16 +238,27 @@ abstract contract BaseWrapper { // Restrict by the allowance that `sender` has to this contract // NOTE: No need for allowance check if `sender` is this contract if (sender != address(this)) { - availableShares = Math.min(availableShares, vaults[id].allowance(sender, address(this))); + availableShares = Math.min( + availableShares, + vaults[id].allowance(sender, address(this)) + ); } // Limit by maximum withdrawal size from each vault - availableShares = Math.min(availableShares, vaults[id].maxAvailableShares()); + availableShares = Math.min( + availableShares, + vaults[id].maxAvailableShares() + ); if (availableShares > 0) { // Intermediate step to move shares to this contract before withdrawing // NOTE: No need for share transfer if this contract is `sender` - if (sender != address(this)) vaults[id].transferFrom(sender, address(this), availableShares); + if (sender != address(this)) + vaults[id].transferFrom( + sender, + address(this), + availableShares + ); if (amount != WITHDRAW_EVERYTHING) { // Compute amount to withdraw fully to satisfy the request @@ -236,10 +270,16 @@ abstract contract BaseWrapper { // Limit amount to withdraw to the maximum made available to this contract // NOTE: Avoid corner case where `estimatedShares` isn't precise enough // NOTE: If `0 < estimatedShares < 1` but `availableShares > 1`, this will withdraw more than necessary - if (estimatedShares > 0 && estimatedShares < availableShares) { - withdrawn = withdrawn.add(vaults[id].withdraw(estimatedShares)); + if ( + estimatedShares > 0 && estimatedShares < availableShares + ) { + withdrawn = withdrawn.add( + vaults[id].withdraw(estimatedShares) + ); } else { - withdrawn = withdrawn.add(vaults[id].withdraw(availableShares)); + withdrawn = withdrawn.add( + vaults[id].withdraw(availableShares) + ); } } else { withdrawn = withdrawn.add(vaults[id].withdraw()); @@ -253,9 +293,16 @@ abstract contract BaseWrapper { // If we have extra, deposit back into `_bestVault` for `sender` // NOTE: Invariant is `withdrawn <= amount` - if (withdrawn > amount && withdrawn.sub(amount) > _bestVault.pricePerShare().div(10**_bestVault.decimals())) { + if ( + withdrawn > amount && + withdrawn.sub(amount) > + _bestVault.pricePerShare().div(10**_bestVault.decimals()) + ) { // Don't forget to approve the deposit - if (token.allowance(address(this), address(_bestVault)) < withdrawn.sub(amount)) { + if ( + token.allowance(address(this), address(_bestVault)) < + withdrawn.sub(amount) + ) { token.safeApprove(address(_bestVault), UNLIMITED_APPROVAL); // Vaults are trusted } @@ -271,7 +318,10 @@ abstract contract BaseWrapper { return _migrate(account, MIGRATE_EVERYTHING); } - function _migrate(address account, uint256 amount) internal returns (uint256) { + function _migrate(address account, uint256 amount) + internal + returns (uint256) + { // NOTE: In practice, it was discovered that <50 was the maximum we've see for this variance return _migrate(account, amount, 0); } @@ -289,7 +339,9 @@ abstract contract BaseWrapper { if (_depositLimit <= _totalAssets) return 0; // Nothing to migrate (not a failure) uint256 _amount = amount; - if (_depositLimit < UNCAPPED_DEPOSITS && _amount < WITHDRAW_EVERYTHING) { + if ( + _depositLimit < UNCAPPED_DEPOSITS && _amount < WITHDRAW_EVERYTHING + ) { // Can only deposit up to this amount uint256 _depositLeft = _depositLimit.sub(_totalAssets); if (_amount > _depositLeft) _amount = _depositLeft; @@ -297,7 +349,12 @@ abstract contract BaseWrapper { if (_amount > 0) { // NOTE: `false` = don't withdraw from `_bestVault` - uint256 withdrawn = _withdraw(account, address(this), _amount, false); + uint256 withdrawn = _withdraw( + account, + address(this), + _amount, + false + ); if (withdrawn == 0) return 0; // Nothing to migrate (not a failure) // NOTE: `false` = don't do `transferFrom` because it's already local From becc15e225775541e15a7913a9abebe9f67b0e6f Mon Sep 17 00:00:00 2001 From: pandadefi <87183122+pandadefi@users.noreply.github.com> Date: Fri, 2 Dec 2022 16:42:41 +0200 Subject: [PATCH 5/6] chore: solidity 0.8 (#27) --- brownie-config.yml | 13 +++---- contracts/AffiliateToken.sol | 25 +++++++------- contracts/BaseWrapper.sol | 66 +++++++++++++++++------------------- 3 files changed, 49 insertions(+), 55 deletions(-) diff --git a/brownie-config.yml b/brownie-config.yml index 5c77119..5f809db 100644 --- a/brownie-config.yml +++ b/brownie-config.yml @@ -8,17 +8,14 @@ autofetch_sources: True # require OpenZepplin Contracts dependencies: - - yearn/yearn-vaults@0.4.3 - - OpenZeppelin/openzeppelin-contracts@3.4.2 + - yearn/yearn-vaults@0.4.5 + - OpenZeppelin/openzeppelin-contracts@4.8.0 # path remapping to support imports from GitHub/NPM compiler: solc: - version: 0.6.12 + version: 0.8.17 remappings: - - "@yearnvaults=yearn/yearn-vaults@0.4.3" - - "@openzeppelin=OpenZeppelin/openzeppelin-contracts@3.4.2" + - "@yearnvaults=yearn/yearn-vaults@0.4.5" + - "@openzeppelin=OpenZeppelin/openzeppelin-contracts@4.8.0" -reports: - exclude_contracts: - - SafeMath diff --git a/contracts/AffiliateToken.sol b/contracts/AffiliateToken.sol index 0e6200b..f1a7f00 100644 --- a/contracts/AffiliateToken.sol +++ b/contracts/AffiliateToken.sol @@ -1,10 +1,11 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.6.12; +pragma solidity ^0.8.17; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import {VaultAPI, BaseWrapper} from "./BaseWrapper.sol"; contract AffiliateToken is ERC20, BaseWrapper { + uint8 public immutable DECIMALS; /// @notice The EIP-712 typehash for the contract's domain bytes32 public constant DOMAIN_TYPEHASH = keccak256( @@ -35,7 +36,7 @@ contract AffiliateToken is ERC20, BaseWrapper { address _registry, string memory name, string memory symbol - ) public BaseWrapper(_token, _registry) ERC20(name, symbol) { + ) BaseWrapper(_token, _registry) ERC20(name, symbol) { DOMAIN_SEPARATOR = keccak256( abi.encode( DOMAIN_TYPEHASH, @@ -46,7 +47,7 @@ contract AffiliateToken is ERC20, BaseWrapper { ) ); affiliate = msg.sender; - _setupDecimals(uint8(ERC20(address(token)).decimals())); + DECIMALS = uint8(ERC20(address(token)).decimals()); } function _getChainId() internal view returns (uint256) { @@ -70,10 +71,7 @@ contract AffiliateToken is ERC20, BaseWrapper { uint256 totalShares = totalSupply(); if (totalShares > 0) { - return - totalVaultBalance(address(this)).mul(numShares).div( - totalShares - ); + return (totalVaultBalance(address(this)) * numShares) / totalShares; } else { return numShares; } @@ -81,23 +79,22 @@ contract AffiliateToken is ERC20, BaseWrapper { function pricePerShare() external view returns (uint256) { return - totalVaultBalance(address(this)).mul(10**uint256(decimals())).div( - totalSupply() - ); + (totalVaultBalance(address(this)) * 10**uint256(decimals())) / + totalSupply(); } function _sharesForValue(uint256 amount) internal view returns (uint256) { // total wrapper assets before deposit (assumes deposit already occured) uint256 totalBalance = totalVaultBalance(address(this)); if (totalBalance > amount) { - return totalSupply().mul(amount).div(totalBalance.sub(amount)); + return (totalSupply() * amount) / (totalBalance - amount); } else { return amount; } } function deposit() external returns (uint256) { - return deposit(uint256(-1)); // Deposit everything + return deposit(type(uint256).max); // Deposit everything } function deposit(uint256 amount) public returns (uint256 deposited) { @@ -177,4 +174,8 @@ contract AffiliateToken is ERC20, BaseWrapper { _approve(owner, spender, amount); } + + function decimals() public view virtual override returns (uint8) { + return DECIMALS; + } } diff --git a/contracts/BaseWrapper.sol b/contracts/BaseWrapper.sol index 02c454c..25d5a4a 100644 --- a/contracts/BaseWrapper.sol +++ b/contracts/BaseWrapper.sol @@ -1,12 +1,9 @@ // SPDX-License-Identifier: GPL-3.0 -pragma solidity ^0.6.12; +pragma solidity ^0.8.17; pragma experimental ABIEncoderV2; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; -import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/SafeERC20.sol"; -import {Math} from "@openzeppelin/contracts/math/Math.sol"; -import {SafeMath} from "@openzeppelin/contracts/math/SafeMath.sol"; - +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import {VaultAPI} from "@yearnvaults/contracts/BaseStrategy.sol"; interface RegistryAPI { @@ -33,8 +30,6 @@ interface RegistryAPI { * */ abstract contract BaseWrapper { - using Math for uint256; - using SafeMath for uint256; using SafeERC20 for IERC20; IERC20 public token; @@ -54,7 +49,7 @@ abstract contract BaseWrapper { // VaultsAPI.depositLimit is unlimited uint256 constant UNCAPPED_DEPOSITS = type(uint256).max; - constructor(address _token, address _registry) public { + constructor(address _token, address _registry) { // Recommended to use a token with a `Registry.latestVault(_token) != address(0)` token = IERC20(_token); // Recommended to use `v2.registry.ychad.eth` @@ -141,12 +136,10 @@ abstract contract BaseWrapper { VaultAPI[] memory vaults = allVaults(); for (uint256 id = 0; id < vaults.length; id++) { - balance = balance.add( - vaults[id] - .balanceOf(account) - .mul(vaults[id].pricePerShare()) - .div(10**uint256(vaults[id].decimals())) - ); + balance = + balance + + ((vaults[id].balanceOf(account) * vaults[id].pricePerShare()) / + 10**uint256(vaults[id].decimals())); } } @@ -159,7 +152,7 @@ abstract contract BaseWrapper { VaultAPI[] memory vaults = allVaults(); for (uint256 id = 0; id < vaults.length; id++) { - assets = assets.add(vaults[id].totalAssets()); + assets = assets + vaults[id].totalAssets(); } } @@ -202,7 +195,7 @@ abstract contract BaseWrapper { } uint256 afterBal = token.balanceOf(address(this)); - deposited = beforeBal.sub(afterBal); + deposited = beforeBal - afterBal; // `receiver` now has shares of `_bestVault` as balance, converted to `token` here // Issue a refund if not everything was deposited if (depositor != address(this) && afterBal > 0) @@ -238,14 +231,14 @@ abstract contract BaseWrapper { // Restrict by the allowance that `sender` has to this contract // NOTE: No need for allowance check if `sender` is this contract if (sender != address(this)) { - availableShares = Math.min( + availableShares = min( availableShares, vaults[id].allowance(sender, address(this)) ); } // Limit by maximum withdrawal size from each vault - availableShares = Math.min( + availableShares = min( availableShares, vaults[id].maxAvailableShares() ); @@ -262,10 +255,9 @@ abstract contract BaseWrapper { if (amount != WITHDRAW_EVERYTHING) { // Compute amount to withdraw fully to satisfy the request - uint256 estimatedShares = amount - .sub(withdrawn) // NOTE: Changes every iteration - .mul(10**uint256(vaults[id].decimals())) - .div(vaults[id].pricePerShare()); // NOTE: Every Vault is different + uint256 estimatedShares = ((amount - withdrawn) * + 10**uint256(vaults[id].decimals())) / // NOTE: Changes every iteration + vaults[id].pricePerShare(); // NOTE: Every Vault is different // Limit amount to withdraw to the maximum made available to this contract // NOTE: Avoid corner case where `estimatedShares` isn't precise enough @@ -273,16 +265,16 @@ abstract contract BaseWrapper { if ( estimatedShares > 0 && estimatedShares < availableShares ) { - withdrawn = withdrawn.add( - vaults[id].withdraw(estimatedShares) - ); + withdrawn = + withdrawn + + vaults[id].withdraw(estimatedShares); } else { - withdrawn = withdrawn.add( - vaults[id].withdraw(availableShares) - ); + withdrawn = + withdrawn + + vaults[id].withdraw(availableShares); } } else { - withdrawn = withdrawn.add(vaults[id].withdraw()); + withdrawn = withdrawn + vaults[id].withdraw(); } // Check if we have fully satisfied the request @@ -295,18 +287,18 @@ abstract contract BaseWrapper { // NOTE: Invariant is `withdrawn <= amount` if ( withdrawn > amount && - withdrawn.sub(amount) > - _bestVault.pricePerShare().div(10**_bestVault.decimals()) + withdrawn - amount > + _bestVault.pricePerShare() / 10**_bestVault.decimals() ) { // Don't forget to approve the deposit if ( token.allowance(address(this), address(_bestVault)) < - withdrawn.sub(amount) + withdrawn - amount ) { token.safeApprove(address(_bestVault), UNLIMITED_APPROVAL); // Vaults are trusted } - _bestVault.deposit(withdrawn.sub(amount), sender); + _bestVault.deposit(withdrawn - amount, sender); withdrawn = amount; } @@ -343,7 +335,7 @@ abstract contract BaseWrapper { _depositLimit < UNCAPPED_DEPOSITS && _amount < WITHDRAW_EVERYTHING ) { // Can only deposit up to this amount - uint256 _depositLeft = _depositLimit.sub(_totalAssets); + uint256 _depositLeft = _depositLimit - _totalAssets; if (_amount > _depositLeft) _amount = _depositLeft; } @@ -362,7 +354,11 @@ abstract contract BaseWrapper { // NOTE: Due to the precision loss of certain calculations, there is a small inefficency // on how migrations are calculated, and this could lead to a DoS issue. Hence, this // value is made to be configurable to allow the user to specify how much is acceptable - require(withdrawn.sub(migrated) <= maxMigrationLoss); + require(withdrawn - migrated <= maxMigrationLoss); } // else: nothing to migrate! (not a failure) } + + function min(uint256 a, uint256 b) internal pure returns (uint256) { + return a < b ? a : b; + } } From 6256100fcfd25a1d8128d6eab2d98dedabcbf3e2 Mon Sep 17 00:00:00 2001 From: pandadefi <87183122+pandadefi@users.noreply.github.com> Date: Fri, 2 Dec 2022 16:46:03 +0200 Subject: [PATCH 6/6] doc: add warning about the codebase. --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index df3893c..740f8be 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,9 @@ # Yearn Affiliate Wrapper Brownie Mix +## Warning + +This repo is for inspiration purposes, this code isn't audited. + ## What you'll find here - Basic Solidity Smart Contract for creating your own Yearn Affiliate Wrapper ([`contracts/AffiliateToken.sol`](contracts/AffiliateToken.sol))