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

fix: minor changes #82

Merged
merged 6 commits into from
Feb 2, 2024
Merged
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
2 changes: 1 addition & 1 deletion SPECIFICATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ A strategies management can set another address to serve as an `emergencyAdmin`.
#### Setting Performance Fee
Setting the percent in terms of basis points for the amount of profit to be charged as a fee.

This has a minimum of 5% and a maximum of 50%.
There is a maximum of 50%.

#### Setting performance fee recipient
Setting the non-zero address that will receive any shares issued as a result of the performance fee.
Expand Down
156 changes: 68 additions & 88 deletions src/TokenizedStrategy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,9 @@ contract TokenizedStrategy {
address pendingManagement; // Address that is pending to take over `management`.
address emergencyAdmin; // Address to act in emergencies as well as `management`.


// Strategy status checks.
bool entered; // Bool to prevent reentrancy.
bool shutdown; // Bool that can be used to stop deposits into the strategy.

// Strategy Status
uint8 entered; // To prevent reentrancy. Use uint8 for gas savings.
bool shutdown; // Bool that can be used to stop deposits into the strategy.
}

/*//////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -287,15 +285,15 @@ contract TokenizedStrategy {
modifier nonReentrant() {
StrategyData storage S = _strategyStorage();
// On the first call to nonReentrant, `entered` will be false
require(!S.entered, "ReentrancyGuard: reentrant call");
require(S.entered != ENTERED, "ReentrancyGuard: reentrant call");

// Any calls to nonReentrant after this point will fail
S.entered = true;
S.entered = ENTERED;

_;

// Reset to false once call has finished
S.entered = false;
S.entered = NOT_ENTERED;
}

/**
Expand Down Expand Up @@ -349,25 +347,25 @@ contract TokenizedStrategy {
/// @notice API version this TokenizedStrategy implements.
string internal constant API_VERSION = "3.0.2";

/// @notice Value to set the `entered` flag to during a call.
uint8 internal constant ENTERED = 2;
/// @notice Value to set the `entered` flag to at the end of the call.
uint8 internal constant NOT_ENTERED = 1;

/// @notice Maximum in Basis Points the Performance Fee can be set to.
uint16 public constant MAX_FEE = 5_000; // 50%

/// @notice Used for fee calculations.
uint256 internal constant MAX_BPS = 10_000;
/// @notice Used for profit unlocking rate calculations.
uint256 internal constant MAX_BPS_EXTENDED = 1_000_000_000_000;

/// @notice Minimum in Basis points the Performance fee can be set to.
// Used to disincentive forking strategies just to lower fees.
uint16 public constant MIN_FEE = 500; // 5%
/// @notice Maximum in Basis Points the Performance Fee can be set to.
uint16 public constant MAX_FEE = 5_000; // 50%

/// @notice Seconds per year for max profit unlocking time.
uint256 internal constant SECONDS_PER_YEAR = 31_556_952; // 365.2425 days

/// @notice Address of the previously deployed Vault factory that the
// protocol fee config is retrieved from.
// NOTE: This will be set to deployed factory. deterministic address for testing is used now
address public constant FACTORY =
0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f;
address public immutable FACTORY;

/**
* @dev Custom storage slot that will be used to store the
Expand Down Expand Up @@ -504,7 +502,10 @@ contract TokenizedStrategy {
"ERC4626: deposit more than max"
);
// Check for rounding error.
require((shares = _previewDeposit(S, assets)) != 0, "ZERO_SHARES");
require(
(shares = _convertToShares(S, assets, Math.Rounding.Down)) != 0,
"ZERO_SHARES"
);

_deposit(S, receiver, assets, shares);
}
Expand All @@ -525,7 +526,10 @@ contract TokenizedStrategy {
// Checking max mint will also check if shutdown.
require(shares <= _maxMint(S, receiver), "ERC4626: mint more than max");
// Check for rounding error.
require((assets = _previewMint(S, shares)) != 0, "ZERO_ASSETS");
require(
(assets = _convertToAssets(S, shares, Math.Rounding.Up)) != 0,
"ZERO_ASSETS"
);

_deposit(S, receiver, assets, shares);
}
Expand Down Expand Up @@ -570,7 +574,10 @@ contract TokenizedStrategy {
"ERC4626: withdraw more than max"
);
// Check for rounding error or 0 value.
require((shares = _previewWithdraw(S, assets)) != 0, "ZERO_SHARES");
require(
(shares = _convertToShares(S, assets, Math.Rounding.Up)) != 0,
"ZERO_SHARES"
);

// Withdraw and track the actual amount withdrawn for loss check.
_withdraw(S, receiver, owner, assets, shares, maxLoss);
Expand Down Expand Up @@ -618,7 +625,10 @@ contract TokenizedStrategy {
);
uint256 assets;
// Check for rounding error or 0 value.
require((assets = _previewRedeem(S, shares)) != 0, "ZERO_ASSETS");
require(
(assets = _convertToAssets(S, shares, Math.Rounding.Down)) != 0,
"ZERO_ASSETS"
);

// We need to return the actual amount withdrawn in case of a loss.
return _withdraw(S, receiver, owner, assets, shares, maxLoss);
Expand Down Expand Up @@ -664,7 +674,7 @@ contract TokenizedStrategy {
* @return . Expected shares that `assets` represents.
*/
function convertToShares(uint256 assets) external view returns (uint256) {
return _convertToShares(_strategyStorage(), assets);
return _convertToShares(_strategyStorage(), assets, Math.Rounding.Down);
}

/**
Expand All @@ -676,7 +686,7 @@ contract TokenizedStrategy {
* @return . Expected amount of `asset` the shares represents.
*/
function convertToAssets(uint256 shares) external view returns (uint256) {
return _convertToAssets(_strategyStorage(), shares);
return _convertToAssets(_strategyStorage(), shares, Math.Rounding.Down);
}

/**
Expand All @@ -689,7 +699,7 @@ contract TokenizedStrategy {
* @return . Expected shares that would be issued.
*/
function previewDeposit(uint256 assets) external view returns (uint256) {
return _previewDeposit(_strategyStorage(), assets);
return _convertToShares(_strategyStorage(), assets, Math.Rounding.Down);
}

/**
Expand All @@ -703,7 +713,7 @@ contract TokenizedStrategy {
* @return . The needed amount of `asset` for the mint.
*/
function previewMint(uint256 shares) external view returns (uint256) {
return _previewMint(_strategyStorage(), shares);
return _convertToAssets(_strategyStorage(), shares, Math.Rounding.Up);
}

/**
Expand All @@ -717,7 +727,7 @@ contract TokenizedStrategy {
* @return . The amount of shares that would be burnt.
*/
function previewWithdraw(uint256 assets) external view returns (uint256) {
return _previewWithdraw(_strategyStorage(), assets);
return _convertToShares(_strategyStorage(), assets, Math.Rounding.Up);
}

/**
Expand All @@ -730,7 +740,7 @@ contract TokenizedStrategy {
* @return . The amount of `asset` that would be returned.
*/
function previewRedeem(uint256 shares) external view returns (uint256) {
return _previewRedeem(_strategyStorage(), shares);
return _convertToAssets(_strategyStorage(), shares, Math.Rounding.Down);
}

/**
Expand Down Expand Up @@ -802,7 +812,8 @@ contract TokenizedStrategy {
/// @dev Internal implementation of {convertToShares}.
function _convertToShares(
StrategyData storage S,
uint256 assets
uint256 assets,
Math.Rounding _rounding
) internal view returns (uint256) {
// Saves an extra SLOAD if totalAssets() is non-zero.
uint256 totalAssets_ = _totalAssets(S);
Expand All @@ -811,66 +822,22 @@ contract TokenizedStrategy {
// If assets are 0 but supply is not PPS = 0.
if (totalAssets_ == 0) return totalSupply_ == 0 ? assets : 0;

return assets.mulDiv(totalSupply_, totalAssets_, Math.Rounding.Down);
return assets.mulDiv(totalSupply_, totalAssets_, _rounding);
}

/// @dev Internal implementation of {convertToAssets}.
function _convertToAssets(
StrategyData storage S,
uint256 shares
) internal view returns (uint256) {
// Saves an extra SLOAD if totalSupply() is non-zero.
uint256 supply = _totalSupply(S);

return
supply == 0
? shares
: shares.mulDiv(_totalAssets(S), supply, Math.Rounding.Down);
}

/// @dev Internal implementation of {previewDeposit}.
function _previewDeposit(
StrategyData storage S,
uint256 assets
) internal view returns (uint256) {
return _convertToShares(S, assets);
}

/// @dev Internal implementation of {previewMint}.
function _previewMint(
StrategyData storage S,
uint256 shares
uint256 shares,
Math.Rounding _rounding
) internal view returns (uint256) {
// Saves an extra SLOAD if totalSupply() is non-zero.
uint256 supply = _totalSupply(S);

return
supply == 0
? shares
: shares.mulDiv(_totalAssets(S), supply, Math.Rounding.Up);
}

/// @dev Internal implementation of {previewWithdraw}.
function _previewWithdraw(
StrategyData storage S,
uint256 assets
) internal view returns (uint256) {
// Saves an extra SLOAD if totalAssets() is non-zero.
uint256 totalAssets_ = _totalAssets(S);
uint256 totalSupply_ = _totalSupply(S);

// If assets are 0 but supply is not, then PPS = 0.
if (totalAssets_ == 0) return totalSupply_ == 0 ? assets : 0;

return assets.mulDiv(totalSupply_, totalAssets_, Math.Rounding.Up);
}

/// @dev Internal implementation of {previewRedeem}.
function _previewRedeem(
StrategyData storage S,
uint256 shares
) internal view returns (uint256) {
return _convertToAssets(S, shares);
: shares.mulDiv(_totalAssets(S), supply, _rounding);
}

/// @dev Internal implementation of {maxDeposit}.
Expand All @@ -894,7 +861,7 @@ contract TokenizedStrategy {

maxMint_ = IBaseStrategy(address(this)).availableDepositLimit(owner);
if (maxMint_ != type(uint256).max) {
maxMint_ = _convertToShares(S, maxMint_);
maxMint_ = _convertToShares(S, maxMint_, Math.Rounding.Down);
}
}

Expand All @@ -911,10 +878,14 @@ contract TokenizedStrategy {
// If there is no limit enforced.
if (maxWithdraw_ == type(uint256).max) {
// Saves a min check if there is no withdrawal limit.
maxWithdraw_ = _convertToAssets(S, _balanceOf(S, owner));
maxWithdraw_ = _convertToAssets(
S,
_balanceOf(S, owner),
Math.Rounding.Down
);
} else {
maxWithdraw_ = Math.min(
_convertToAssets(S, _balanceOf(S, owner)),
_convertToAssets(S, _balanceOf(S, owner), Math.Rounding.Down),
maxWithdraw_
);
}
Expand All @@ -934,7 +905,7 @@ contract TokenizedStrategy {
} else {
maxRedeem_ = Math.min(
// Can't redeem more than the balance.
_convertToShares(S, maxRedeem_),
_convertToShares(S, maxRedeem_, Math.Rounding.Down),
_balanceOf(S, owner)
);
}
Expand Down Expand Up @@ -1143,19 +1114,28 @@ contract TokenizedStrategy {
unchecked {
performanceFeeShares = _convertToShares(
S,
totalFees - protocolFees
totalFees - protocolFees,
Math.Rounding.Down
);
}
if (protocolFees != 0) {
protocolFeeShares = _convertToShares(S, protocolFees);
protocolFeeShares = _convertToShares(
S,
protocolFees,
Math.Rounding.Down
);
}
}

// we have a net profit. Check if we are locking profit.
if (_profitMaxUnlockTime != 0) {
// lock (profit - fees)
unchecked {
sharesToLock = _convertToShares(S, profit - totalFees);
sharesToLock = _convertToShares(
S,
profit - totalFees,
Math.Rounding.Down
);
}
// Mint the shares to lock the strategy.
_mint(S, address(this), sharesToLock);
Expand All @@ -1181,7 +1161,7 @@ contract TokenizedStrategy {
// to offset the loss to prevent any PPS decline post report.
uint256 sharesToBurn = Math.min(
S.balances[address(this)],
_convertToShares(S, loss)
_convertToShares(S, loss, Math.Rounding.Down)
);

// Check if there is anything to burn.
Expand Down Expand Up @@ -1481,7 +1461,7 @@ contract TokenizedStrategy {
*/
function pricePerShare() external view returns (uint256) {
StrategyData storage S = _strategyStorage();
return _convertToAssets(S, 10 ** S.decimals);
return _convertToAssets(S, 10 ** S.decimals, Math.Rounding.Down);
}

/**
Expand Down Expand Up @@ -1557,13 +1537,11 @@ contract TokenizedStrategy {
* @dev Can only be called by the current `management`.
*
* Denominated in Basis Points. So 100% == 10_000.
* Cannot be set less than the MIN_FEE.
* Cannot set greater than to MAX_FEE.
*
* @param _performanceFee New performance fee.
*/
function setPerformanceFee(uint16 _performanceFee) external onlyManagement {
require(_performanceFee >= MIN_FEE, "MIN FEE");
require(_performanceFee <= MAX_FEE, "MAX FEE");
_strategyStorage().performanceFee = _performanceFee;

Expand Down Expand Up @@ -2058,8 +2036,10 @@ contract TokenizedStrategy {
/**
* @dev On contract creation we set `asset` for this contract to address(1).
* This prevents it from ever being initialized in the future.
* @param _factory Address of the factory of the same version for protocol fees.
*/
constructor() {
constructor(address _factory) {
FACTORY = _factory;
_strategyStorage().asset = ERC20(address(1));
}
}
2 changes: 0 additions & 2 deletions src/interfaces/ITokenizedStrategy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@ interface ITokenizedStrategy is IERC4626, IERC20Permit {
CONSTANTS
//////////////////////////////////////////////////////////////*/

function MIN_FEE() external view returns (uint16);

function MAX_FEE() external view returns (uint16);

function FACTORY() external view returns (address);
Expand Down
4 changes: 0 additions & 4 deletions src/test/AccessControl.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,6 @@ contract AccessControlTest is Setup {

assertEq(strategy.performanceFee(), _performanceFee);

vm.prank(management);
vm.expectRevert("MIN FEE");
strategy.setPerformanceFee(uint16(5));

vm.prank(management);
vm.expectRevert("MAX FEE");
strategy.setPerformanceFee(uint16(_amount + 5_001));
Expand Down
Loading
Loading