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

audit: apply suggestions #24

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 5 additions & 42 deletions vaults/contracts/BaseStrategy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/*///////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -53,43 +54,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
//////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -139,7 +109,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);
Expand All @@ -148,24 +117,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;
}
}

Expand Down
17 changes: 16 additions & 1 deletion vaults/contracts/Vault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 blocks 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);
Expand Down Expand Up @@ -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);
}

/*///////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 0 additions & 3 deletions vaults/contracts/auth/authorities/MerkleAuthority.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
//////////////////////////////////////////////////////////////*/
Expand Down
1 change: 0 additions & 1 deletion vaults/contracts/mocks/MockStrategy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
32 changes: 30 additions & 2 deletions vaults/interfaces/IStrategy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
//////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -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);
Expand Down