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

feat: v3.0.3 #98

Merged
merged 13 commits into from
Sep 24, 2024
Binary file added audits/Yearn V3 report Statemind.pdf
Binary file not shown.
2 changes: 0 additions & 2 deletions foundry.toml
Original file line number Diff line number Diff line change
@@ -17,5 +17,3 @@ max_test_rejects = 1_000_000
[invariant]
runs = 100
depth = 100

# See more config options https://github.com/gakonst/foundry/tree/master/config
73 changes: 41 additions & 32 deletions src/TokenizedStrategy.sol
Original file line number Diff line number Diff line change
@@ -212,10 +212,8 @@ contract TokenizedStrategy {
// These are the corresponding ERC20 variables needed for the
// strategies token that is issued and burned on each deposit or withdraw.
uint8 decimals; // The amount of decimals that `asset` and strategy use.
uint88 INITIAL_CHAIN_ID; // The initial chain id when the strategy was created.
string name; // The name of the token for the strategy.
uint256 totalSupply; // The total amount of shares currently issued.
bytes32 INITIAL_DOMAIN_SEPARATOR; // The domain separator used for permits on the initial chain.
mapping(address => uint256) nonces; // Mapping of nonces used for permit functions.
mapping(address => uint256) balances; // Mapping to track current balances for each account that holds shares.
mapping(address => mapping(address => uint256)) allowances; // Mapping to track the allowances for the strategies shares.
@@ -345,7 +343,7 @@ contract TokenizedStrategy {
//////////////////////////////////////////////////////////////*/

/// @notice API version this TokenizedStrategy implements.
string internal constant API_VERSION = "3.0.2";
string internal constant API_VERSION = "3.0.3";

/// @notice Value to set the `entered` flag to during a call.
uint8 internal constant ENTERED = 2;
@@ -451,11 +449,6 @@ contract TokenizedStrategy {
S.name = _name;
// Set decimals based off the `asset`.
S.decimals = ERC20(_asset).decimals();
// Set initial chain id for permit replay protection.
require(block.chainid < type(uint88).max, "invalid chain id");
S.INITIAL_CHAIN_ID = uint88(block.chainid);
// Set the initial domain separator for permit functions.
S.INITIAL_DOMAIN_SEPARATOR = _computeDomainSeparator(S);

// Default to a 10 day profit unlock period.
S.profitMaxUnlockTime = 10 days;
@@ -497,6 +490,12 @@ contract TokenizedStrategy {
) external nonReentrant returns (uint256 shares) {
// Get the storage slot for all following calls.
StrategyData storage S = _strategyStorage();

// Deposit full balance if using max uint.
if (assets == type(uint256).max) {
assets = S.asset.balanceOf(msg.sender);
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the use case for using uint256.max? I like this flow when repaying the debt but for deposit, I haven't seen it yet.

Will it break the ERC4626 standard: “MUST revert if all of assets cannot be deposited”? https://eips.ethereum.org/EIPS/eip-4626#deposit

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The most common case we have run into is when doing multiple txns in a row, especially in a SAFE script, when the value of the underlying the wallet will have is not know.

Ex:

  1. Redeem 100% of shares from DAI-1
  2. Deposit full DAI balance into DAI-2

Since the txn executes at a different block than created the amount cannot be known exactly since DAI-1 PPS is always changing and so dust will get left behind.

It does not match 4626 exactly and would be a special case user would need to know about similar to our masLoss additional parameter.

}

// Checking max deposit will also check if shutdown.
require(
assets <= _maxDeposit(S, receiver),
@@ -524,6 +523,7 @@ contract TokenizedStrategy {
) external nonReentrant returns (uint256 assets) {
// Get the storage slot for all following calls.
StrategyData storage S = _strategyStorage();

// Checking max mint will also check if shutdown.
require(shares <= _maxMint(S, receiver), "ERC4626: mint more than max");
// Check for rounding error.
@@ -780,6 +780,18 @@ contract TokenizedStrategy {
return _maxWithdraw(_strategyStorage(), owner);
}

/**
* @notice Variable `maxLoss` is ignored.
* @dev Accepts a `maxLoss` variable in order to match the multi
* strategy vaults ABI.
*/
function maxWithdraw(
address owner,
uint256 /*maxLoss*/
) external view returns (uint256) {
return _maxWithdraw(_strategyStorage(), owner);
}

/**
* @notice Total number of strategy shares that can be
* redeemed from the strategy by `owner`, where `owner`
@@ -792,6 +804,18 @@ contract TokenizedStrategy {
return _maxRedeem(_strategyStorage(), owner);
}

/**
* @notice Variable `maxLoss` is ignored.
* @dev Accepts a `maxLoss` variable in order to match the multi
* strategy vaults ABI.
*/
function maxRedeem(
address owner,
uint256 /*maxLoss*/
) external view returns (uint256) {
return _maxRedeem(_strategyStorage(), owner);
}

/*//////////////////////////////////////////////////////////////
INTERNAL 4626 VIEW METHODS
//////////////////////////////////////////////////////////////*/
@@ -1594,6 +1618,14 @@ contract TokenizedStrategy {
emit UpdateProfitMaxUnlockTime(_profitMaxUnlockTime);
}

/**
* @notice Updates the name for the strategy.
* @param _name The new name for the strategy.
*/
function setName(string calldata _name) external onlyManagement {
_strategyStorage().name = _name;
}

/*//////////////////////////////////////////////////////////////
ERC20 METHODS
//////////////////////////////////////////////////////////////*/
@@ -1982,39 +2014,16 @@ contract TokenizedStrategy {
* @notice Returns the domain separator used in the encoding of the signature
* for {permit}, as defined by {EIP712}.
*
* @dev This checks that the current chain id is the same as when the contract
* was deployed to prevent replay attacks. If false it will calculate a new
* domain separator based on the new chain id.
*
* @return . The domain separator that will be used for any {permit} calls.
*/
function DOMAIN_SEPARATOR() public view returns (bytes32) {
StrategyData storage S = _strategyStorage();
return
block.chainid == S.INITIAL_CHAIN_ID
? S.INITIAL_DOMAIN_SEPARATOR
: _computeDomainSeparator(S);
}

/**
* @dev Calculates and returns the domain separator to be used in any
* permit functions for the strategies {permit} calls.
*
* This will be used at the initialization of each new strategies storage.
* It would then be used in the future in the case of any forks in which
* the current chain id is not the same as the original.
*
*/
function _computeDomainSeparator(
StrategyData storage S
) internal view returns (bytes32) {
return
keccak256(
abi.encode(
keccak256(
"EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
),
keccak256(bytes(S.name)),
keccak256("Yearn Vault"),
keccak256(bytes(API_VERSION)),
block.chainid,
address(this)
12 changes: 12 additions & 0 deletions src/interfaces/ITokenizedStrategy.sol
Original file line number Diff line number Diff line change
@@ -72,6 +72,16 @@ interface ITokenizedStrategy is IERC4626, IERC20Permit {
uint256 maxLoss
) external returns (uint256);

function maxWithdraw(
address owner,
uint256 /*maxLoss*/
) external view returns (uint256);

function maxRedeem(
address owner,
uint256 /*maxLoss*/
) external view returns (uint256);

/*//////////////////////////////////////////////////////////////
MODIFIER HELPERS
//////////////////////////////////////////////////////////////*/
@@ -150,6 +160,8 @@ interface ITokenizedStrategy is IERC4626, IERC20Permit {

function setProfitMaxUnlockTime(uint256 _profitMaxUnlockTime) external;

function setName(string calldata _newName) external;

function shutdownStrategy() external;

function emergencyWithdraw(uint256 _amount) external;
15 changes: 15 additions & 0 deletions src/test/AccessControl.t.sol
Original file line number Diff line number Diff line change
@@ -369,4 +369,19 @@ contract AccessControlTest is Setup {
vm.prank(keeper);
strategy.tend();
}

function test_setName(address _address) public {
vm.assume(_address != address(strategy) && _address != management);

string memory newName = "New Strategy Name";

vm.prank(_address);
vm.expectRevert("!management");
strategy.setName(newName);

vm.prank(management);
strategy.setName(newName);

assertEq(strategy.name(), newName);
}
}
31 changes: 31 additions & 0 deletions src/test/Accounting.t.sol
Original file line number Diff line number Diff line change
@@ -582,4 +582,35 @@ contract AccountingTest is Setup {
assertEq(strategy.pricePerShare(), wad);
checkStrategyTotals(strategy, 0, 0, 0, 0);
}

function test_maxUintDeposit_depositsBalance(
address _address,
uint256 _amount
) public {
_amount = bound(_amount, minFuzzAmount, maxFuzzAmount);
vm.assume(
_address != address(0) &&
_address != address(strategy) &&
_address != address(yieldSource)
);

asset.mint(_address, _amount);

vm.prank(_address);
asset.approve(address(strategy), _amount);

assertEq(asset.balanceOf(_address), _amount);

vm.prank(_address);
strategy.deposit(type(uint256).max, _address);

// Should just deposit the available amount.
checkStrategyTotals(strategy, _amount, _amount, 0, _amount);

assertEq(asset.balanceOf(_address), 0);
assertEq(strategy.balanceOf(_address), _amount);
assertEq(asset.balanceOf(address(strategy)), 0);

assertEq(asset.balanceOf(address(yieldSource)), _amount);
}
}
97 changes: 97 additions & 0 deletions src/test/CustomImplementation.t.sol
Original file line number Diff line number Diff line change
@@ -100,6 +100,103 @@ contract CustomImplementationsTest is Setup {
);
}

function test_customWithdrawLimit_maxLossVariable(
address _address,
uint256 _amount,
uint16 _profitFactor
) public {
_amount = bound(_amount, minFuzzAmount, maxFuzzAmount);
_profitFactor = uint16(bound(uint256(_profitFactor), 10, MAX_BPS));

uint256 profit = (_amount * _profitFactor) / MAX_BPS;

strategy = IMockStrategy(setUpIlliquidStrategy());

vm.assume(
_address != address(0) &&
_address != address(strategy) &&
_address != address(yieldSource)
);

setFees(0, 0);

mintAndDepositIntoStrategy(strategy, _address, _amount);

uint256 idle = asset.balanceOf(address(strategy));
assertGt(idle, 0);

// Assure we have a withdraw limit
assertEq(strategy.availableWithdrawLimit(_address), idle);
assertGt(strategy.totalAssets(), idle);

// Make sure max withdraw and redeem return the correct amounts
assertEq(strategy.maxWithdraw(_address, 9), idle);
assertEq(
strategy.maxRedeem(_address, 0),
strategy.convertToShares(idle)
);
assertLe(
strategy.convertToAssets(strategy.maxRedeem(_address)),
strategy.availableWithdrawLimit(_address)
);

vm.expectRevert("ERC4626: redeem more than max");
vm.prank(_address);
strategy.redeem(_amount, _address, _address);

vm.expectRevert("ERC4626: withdraw more than max");
vm.prank(_address);
strategy.withdraw(_amount, _address, _address);

createAndCheckProfit(strategy, profit, 0, 0);

increaseTimeAndCheckBuffer(strategy, 5 days, profit / 2);

idle = asset.balanceOf(address(strategy));
assertGt(idle, 0);

// Assure we have a withdraw limit
assertEq(strategy.availableWithdrawLimit(_address), idle);
assertGt(strategy.totalAssets(), idle);

// Make sure max withdraw and redeem return the correct amounts
assertEq(strategy.maxWithdraw(_address, 69), idle);
assertEq(
strategy.maxRedeem(_address, 2 ** 256 - 1),
strategy.convertToShares(idle)
);
assertLe(
strategy.convertToAssets(strategy.maxRedeem(_address)),
strategy.availableWithdrawLimit(_address)
);

vm.expectRevert("ERC4626: redeem more than max");
vm.prank(_address);
strategy.redeem(_amount, _address, _address);

vm.expectRevert("ERC4626: withdraw more than max");
vm.prank(_address);
strategy.withdraw(_amount, _address, _address);

uint256 before = asset.balanceOf(_address);
uint256 redeem = strategy.maxRedeem(_address, _amount);
uint256 conversion = strategy.convertToAssets(_amount);

vm.prank(_address);
strategy.redeem(redeem, _address, _address, 0);

// We need to give 2 wei rounding buffer
assertApproxEq(strategy.convertToAssets(_amount), conversion, 2);
assertApproxEq(asset.balanceOf(_address) - before, idle, 2);
assertApproxEq(strategy.availableWithdrawLimit(_address), 0, 2);
assertApproxEq(strategy.maxWithdraw(_address, 99), 0, 2);
assertApproxEq(strategy.maxRedeem(_address, 0), 0, 2);
assertLe(
strategy.convertToAssets(strategy.maxRedeem(_address, _amount)),
strategy.availableWithdrawLimit(_address)
);
}

function test_customDepositLimit(
address _allowed,
address _notAllowed,
2 changes: 1 addition & 1 deletion src/test/ERC20Std.t.sol
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@ contract ERC20BaseTest is Setup {
string(abi.encodePacked("ys", asset.symbol()))
);
assertEq(strategy.decimals(), 18);
assertEq(strategy.apiVersion(), "3.0.2");
assertEq(strategy.apiVersion(), "3.0.3");
}

function testFuzz_mint(address account_, uint256 amount_) public {
18 changes: 18 additions & 0 deletions src/test/ERC4626Std.t.sol
Original file line number Diff line number Diff line change
@@ -15,4 +15,22 @@ contract ERC4626StdTest is ERC4626Test, Setup {
_vaultMayBeEmpty = true;
_unlimitedAmount = true;
}

//Avoid special case for deposits of uint256 max
function test_previewDeposit(
Init memory init,
uint256 assets
) public override {
if (assets == type(uint256).max) assets -= 1;
super.test_previewDeposit(init, assets);
}

function test_deposit(
Init memory init,
uint256 assets,
uint256 allowance
) public override {
if (assets == type(uint256).max) assets -= 1;
super.test_deposit(init, assets, allowance);
}
}
4 changes: 2 additions & 2 deletions src/test/handlers/StrategyHandler.sol
Original file line number Diff line number Diff line change
@@ -123,7 +123,7 @@ contract StrategyHandler is ExtendedTest {
}

function reportProfit(uint256 _amount) public countCall("reportProfit") {
_amount = bound(_amount, 1, strategy.totalAssets() / 2);
_amount = bound(_amount, 1_000, strategy.totalAssets() / 2);

// Simulate earning interest
asset.mint(address(strategy), _amount);
@@ -152,7 +152,7 @@ contract StrategyHandler is ExtendedTest {
}

function tend(uint256 _amount) public countCall("tend") {
_amount = bound(_amount, 1, strategy.totalAssets() / 2);
_amount = bound(_amount, 1_000, strategy.totalAssets() / 2);
asset.mint(address(strategy), _amount);

vm.prank(setup.keeper());
Loading
Loading