From b479d6d2538e678ed4e7f15d497dbff37fff147c Mon Sep 17 00:00:00 2001 From: dantop114 Date: Fri, 1 Jul 2022 17:11:34 +0200 Subject: [PATCH 1/2] apply audit suggestions --- vaults/contracts/BaseStrategy.sol | 44 ++----------------- vaults/contracts/Vault.sol | 17 ++++++- .../auth/authorities/MerkleAuthority.sol | 3 -- vaults/interfaces/IStrategy.sol | 32 +++++++++++++- 4 files changed, 49 insertions(+), 47 deletions(-) diff --git a/vaults/contracts/BaseStrategy.sol b/vaults/contracts/BaseStrategy.sol index e829e66..73e1a46 100644 --- a/vaults/contracts/BaseStrategy.sol +++ b/vaults/contracts/BaseStrategy.sol @@ -53,43 +53,12 @@ abstract contract BaseStrategy is Initializable { /// @notice The Vault managing this strategy. IVault public vault; - /// @notice Deposited underlying. - uint256 depositedUnderlying; - /// @notice The strategy manager. address public manager; /// @notice The strategist. address public strategist; - /*/////////////////////////////////////////////////////////////// - EVENTS - //////////////////////////////////////////////////////////////*/ - - /// @notice Event emitted when a new manager is set for this strategy. - event UpdateManager(address indexed manager); - - /// @notice Event emitted when a new strategist is set for this strategy. - event UpdateStrategist(address indexed strategist); - - /// @notice Event emitted when rewards are sold. - event RewardsHarvested(address indexed reward, uint256 rewards, uint256 underlying); - - /// @notice Event emitted when underlying is deposited in this strategy. - event Deposit(IVault indexed vault, uint256 amount); - - /// @notice Event emitted when underlying is withdrawn from this strategy. - event Withdraw(IVault indexed vault, uint256 amount); - - /// @notice Event emitted when underlying is deployed. - event DepositUnderlying(uint256 deposited); - - /// @notice Event emitted when underlying is removed from other contracts and returned to the strategy. - event WithdrawUnderlying(uint256 amount); - - /// @notice Event emitted when tokens are sweeped from this strategy. - event Sweep(IERC20 indexed asset, uint256 amount); - /*/////////////////////////////////////////////////////////////// INITIALIZE //////////////////////////////////////////////////////////////*/ @@ -139,7 +108,6 @@ abstract contract BaseStrategy is Initializable { function deposit(uint256 amount) external virtual returns (uint8 success) { require(msg.sender == address(vault), "deposit::NOT_VAULT"); - depositedUnderlying += amount; underlying.safeTransferFrom(msg.sender, address(this), amount); emit Deposit(IVault(msg.sender), amount); @@ -148,24 +116,18 @@ abstract contract BaseStrategy is Initializable { } /// @notice Withdraw a specific amount of underlying tokens. + /// @dev This method uses the `float` amount to check for available amount. + /// @dev If a withdraw process is possible, override this. /// @param amount The amount of underlying to withdraw. function withdraw(uint256 amount) external virtual returns (uint8 success) { require(msg.sender == address(vault), "withdraw::NOT_VAULT"); - /// underflow should not stop vault from withdrawing - uint256 depositedUnderlying_ = depositedUnderlying; - if (depositedUnderlying_ >= amount) { - unchecked { - depositedUnderlying = depositedUnderlying_ - amount; - } - } - if (float() < amount) { success = NOT_ENOUGH_UNDERLYING; } else { underlying.transfer(msg.sender, amount); - emit Withdraw(IVault(msg.sender), amount); + success = SUCCESS; } } diff --git a/vaults/contracts/Vault.sol b/vaults/contracts/Vault.sol index 048b60a..7d54d42 100644 --- a/vaults/contracts/Vault.sol +++ b/vaults/contracts/Vault.sol @@ -186,6 +186,10 @@ contract Vault is ERC20, Pausable { /// @param newAuth The new Authority module. event AuthUpdated(Authority newAuth); + /// @notice Emitted when the number of blocks is updated. + /// @param newBlocksPerYear The new number of blocks per year. + event BlocksPerYearUpdated(uint256 blocks); + /// @notice Emitted when the fee percentage is updated. /// @param newFeePercent The new fee percentage. event HarvestFeePercentUpdated(uint256 newFeePercent); @@ -398,6 +402,7 @@ contract Vault is ERC20, Pausable { /// @param blocks Blocks in a given year. function setBlocksPerYear(uint256 blocks) external requiresAuth(msg.sender) { blocksPerYear = blocks; + emit BlocksPerYearUpdated(blocks); } /*/////////////////////////////////////////////////////////////// @@ -875,7 +880,17 @@ contract Vault is ERC20, Pausable { /// @notice Calculates the total amount of underlying tokens the Vault holds. /// @return totalUnderlyingHeld The total amount of underlying tokens the Vault holds. function totalUnderlying() public view virtual returns (uint256) { - return totalStrategyHoldings - lockedProfit() + totalFloat(); + uint256 float = totalFloat(); + uint256 locked = lockedProfit(); + uint256 holdings = totalStrategyHoldings; + + // If a withdrawal from a strategy occourred after an harvest + // `lockedProfit` may be greater than `totalStrategyHoldings`. + // So we have two cases: + // - if `holdings` > `locked`, `totalUnderlying` is `holdings - locked + float` + // - else if `holdings` < `locked`, we need to lock some funds from float (`totalUnderlying` is `float - locked`) + + return (holdings >= locked) ? holdings - locked + float : float - locked; } /// @notice Compute an estimated return given the auxoToken supply, initial exchange rate and locked profits. diff --git a/vaults/contracts/auth/authorities/MerkleAuthority.sol b/vaults/contracts/auth/authorities/MerkleAuthority.sol index 1686787..82f823b 100644 --- a/vaults/contracts/auth/authorities/MerkleAuthority.sol +++ b/vaults/contracts/auth/authorities/MerkleAuthority.sol @@ -34,9 +34,6 @@ contract MerkleAuth is MultiRolesAuthority { /// @notice Event emitted when the Merkle Root is set. event MerkleRootUpdate(bytes32 merkleRoot); - /// @notice Event emitted when VaultAuth admin is updated. - event AuthorityUpdate(address indexed admin); - /*/////////////////////////////////////////////////////////////// CONSTRUCTOR //////////////////////////////////////////////////////////////*/ diff --git a/vaults/interfaces/IStrategy.sol b/vaults/interfaces/IStrategy.sol index de344a6..5727b3a 100644 --- a/vaults/interfaces/IStrategy.sol +++ b/vaults/interfaces/IStrategy.sol @@ -20,6 +20,34 @@ import {IVault} from "./IVault.sol"; /// @title IStrategy /// @notice Basic Vault Strategy interface. interface IStrategy { + /*/////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ + + /// @notice Event emitted when a new manager is set for this strategy. + event UpdateManager(address indexed manager); + + /// @notice Event emitted when a new strategist is set for this strategy. + event UpdateStrategist(address indexed strategist); + + /// @notice Event emitted when rewards are sold. + event RewardsHarvested(address indexed reward, uint256 rewards, uint256 underlying); + + /// @notice Event emitted when underlying is deposited in this strategy. + event Deposit(IVault indexed vault, uint256 amount); + + /// @notice Event emitted when underlying is withdrawn from this strategy. + event Withdraw(IVault indexed vault, uint256 amount); + + /// @notice Event emitted when underlying is deployed. + event DepositUnderlying(uint256 deposited); + + /// @notice Event emitted when underlying is removed from other contracts and returned to the strategy. + event WithdrawUnderlying(uint256 amount); + + /// @notice Event emitted when tokens are sweeped from this strategy. + event Sweep(IERC20 indexed asset, uint256 amount); + /*/////////////////////////////////////////////////////////////// GENERAL INFO //////////////////////////////////////////////////////////////*/ @@ -47,8 +75,8 @@ interface IStrategy { /// @notice The underlying token the strategy accepts. function underlying() external view returns (IERC20); - /// @notice The amount deposited by the Vault in this strategy. - function depositedUnderlying() external returns (uint256); + /// @notice The float amount of underlying. + function float() external view returns (uint256); /// @notice An estimate amount of underlying managed by the strategy. function estimatedUnderlying() external returns (uint256); From 955eccff99d0f307e3325eba1219cee4dd360b26 Mon Sep 17 00:00:00 2001 From: dantop114 Date: Fri, 1 Jul 2022 17:21:58 +0200 Subject: [PATCH 2/2] amend: fix compile --- vaults/contracts/BaseStrategy.sol | 3 ++- vaults/contracts/Vault.sol | 2 +- vaults/contracts/mocks/MockStrategy.sol | 1 - 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vaults/contracts/BaseStrategy.sol b/vaults/contracts/BaseStrategy.sol index 73e1a46..c44b20c 100644 --- a/vaults/contracts/BaseStrategy.sol +++ b/vaults/contracts/BaseStrategy.sol @@ -24,8 +24,9 @@ import {IERC20Upgradeable as IERC20} from "@openzeppelin/contracts-upgradeable/t import {SafeERC20Upgradeable as SafeERC20} from "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol"; import {IVault} from "../interfaces/IVault.sol"; +import {IStrategy} from "../interfaces/IStrategy.sol"; -abstract contract BaseStrategy is Initializable { +abstract contract BaseStrategy is IStrategy, Initializable { using SafeERC20 for IERC20; /*/////////////////////////////////////////////////////////////// diff --git a/vaults/contracts/Vault.sol b/vaults/contracts/Vault.sol index 7d54d42..64e3e29 100644 --- a/vaults/contracts/Vault.sol +++ b/vaults/contracts/Vault.sol @@ -187,7 +187,7 @@ contract Vault is ERC20, Pausable { event AuthUpdated(Authority newAuth); /// @notice Emitted when the number of blocks is updated. - /// @param newBlocksPerYear The new number of blocks per year. + /// @param blocks The new number of blocks per year. event BlocksPerYearUpdated(uint256 blocks); /// @notice Emitted when the fee percentage is updated. diff --git a/vaults/contracts/mocks/MockStrategy.sol b/vaults/contracts/mocks/MockStrategy.sol index 6fd0c7c..802c1ed 100644 --- a/vaults/contracts/mocks/MockStrategy.sol +++ b/vaults/contracts/mocks/MockStrategy.sol @@ -41,7 +41,6 @@ contract MockStrategy is BaseStrategy { require(msg.sender == address(vault), "deposit::NOT_VAULT"); - depositedUnderlying += amount; underlying.safeTransferFrom(msg.sender, address(this), amount); emit Deposit(IVault(msg.sender), amount);