Skip to content

Commit

Permalink
Merge pull request #12 from euler-xyz/tony-review
Browse files Browse the repository at this point in the history 
feat: various improvements
  • Loading branch information
@haythemsellami
haythemsellami authored May 28, 2024
2 parents b86efc9 + 2e650db commit 142e57b
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 25 deletions.
5 changes: 4 additions & 1 deletion .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,7 @@ docs/

# coverage
lcov.info
coverage
coverage

# gas
.gas-snapshot
47 changes: 23 additions & 24 deletions src/FourSixTwoSixAgg.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ contract FourSixTwoSixAgg is BalanceForwarder, EVCUtil, ERC4626, AccessControlEn

error Reentrancy();
error ArrayLengthMismatch();
error AddressesOutOfOrder();
error DuplicateInitialStrategy();
error InitialAllocationPointsZero();
error NotEnoughAssets();
error NegativeYield();
Expand Down Expand Up @@ -120,14 +118,16 @@ contract FourSixTwoSixAgg is BalanceForwarder, EVCUtil, ERC4626, AccessControlEn

strategies[address(0)] =
Strategy({allocated: 0, allocationPoints: uint120(_initialCashAllocationPoints), active: true});
totalAllocationPoints += _initialCashAllocationPoints;

uint256 _totalAllocationPoints = _initialCashAllocationPoints;

for (uint256 i; i < _initialStrategies.length; ++i) {
strategies[_initialStrategies[i]] =
Strategy({allocated: 0, allocationPoints: uint120(_initialStrategiesAllocationPoints[i]), active: true});

totalAllocationPoints += _initialStrategiesAllocationPoints[i];
_totalAllocationPoints += _initialStrategiesAllocationPoints[i];
}
totalAllocationPoints = _totalAllocationPoints;

// Setup DEFAULT_ADMIN
_grantRole(DEFAULT_ADMIN_ROLE, _msgSender());
Expand Down Expand Up @@ -179,9 +179,8 @@ contract FourSixTwoSixAgg is BalanceForwarder, EVCUtil, ERC4626, AccessControlEn
function harvestMultipleStrategies(address[] calldata _strategies) external nonReentrant {
for (uint256 i; i < _strategies.length; ++i) {
_harvest(_strategies[i]);

_gulp();
}
_gulp();
}

/// @notice Adjust a certain strategy's allocation points.
Expand All @@ -194,17 +193,13 @@ contract FourSixTwoSixAgg is BalanceForwarder, EVCUtil, ERC4626, AccessControlEn
onlyRole(ALLOCATION_ADJUSTER_ROLE)
{
Strategy memory strategyDataCache = strategies[strategy];
uint256 totalAllocationPointsCache = totalAllocationPoints;

if (!strategyDataCache.active) {
revert InactiveStrategy();
}

strategies[strategy].allocationPoints = uint120(newPoints);

totalAllocationPoints = (newPoints > strategyDataCache.allocationPoints)
? totalAllocationPointsCache + (newPoints - strategyDataCache.allocationPoints)
: totalAllocationPointsCache - (strategyDataCache.allocationPoints - newPoints);
totalAllocationPoints = totalAllocationPoints + newPoints - strategyDataCache.allocationPoints;
}

/// @notice Swap two strategies indexes in the withdrawal queue.
Expand All @@ -216,7 +211,8 @@ contract FourSixTwoSixAgg is BalanceForwarder, EVCUtil, ERC4626, AccessControlEn
nonReentrant
onlyRole(WITHDRAW_QUEUE_REORDERER_ROLE)
{
if (index1 >= withdrawalQueue.length || index2 >= withdrawalQueue.length) {
uint256 length = withdrawalQueue.length;
if (index1 >= length || index2 >= length) {
revert OutOfBounds();
}

Expand Down Expand Up @@ -255,21 +251,23 @@ contract FourSixTwoSixAgg is BalanceForwarder, EVCUtil, ERC4626, AccessControlEn
/// @dev Can only be called by an address that have the STRATEGY_REMOVER_ROLE
/// @param strategy Address of the strategy
function removeStrategy(address strategy) external nonReentrant onlyRole(STRATEGY_REMOVER_ROLE) {
if (!strategies[strategy].active) {
Strategy storage strategyStorage = strategies[strategy];

if (!strategyStorage.active) {
revert AlreadyRemoved();
}

strategies[strategy].active = false;
totalAllocationPoints -= strategies[strategy].allocationPoints;
strategies[strategy].allocationPoints = 0;
totalAllocationPoints -= strategyStorage.allocationPoints;
strategyStorage.active = false;
strategyStorage.allocationPoints = 0;

// remove from withdrawalQueue
uint256 lastStrategyIndex = withdrawalQueue.length - 1;

for (uint256 i = 0; i <= lastStrategyIndex; i++) {
if ((withdrawalQueue[i] == strategy) && (i != lastStrategyIndex)) {
(withdrawalQueue[i], withdrawalQueue[lastStrategyIndex]) =
(withdrawalQueue[lastStrategyIndex], withdrawalQueue[i]);
for (uint256 i = 0; i < lastStrategyIndex; ++i) {
if (withdrawalQueue[i] == strategy) {
withdrawalQueue[i] = withdrawalQueue[lastStrategyIndex];
withdrawalQueue[lastStrategyIndex] = strategy;
}
}

Expand Down Expand Up @@ -419,7 +417,8 @@ contract FourSixTwoSixAgg is BalanceForwarder, EVCUtil, ERC4626, AccessControlEn
totalAssetsDeposited -= assets;

uint256 assetsRetrieved = IERC20(asset()).balanceOf(address(this));
for (uint256 i; i < withdrawalQueue.length; i++) {
uint256 numStrategies = withdrawalQueue.length;
for (uint256 i; i < numStrategies; ++i) {
if (assetsRetrieved >= assets) {
break;
}
Expand All @@ -429,16 +428,16 @@ contract FourSixTwoSixAgg is BalanceForwarder, EVCUtil, ERC4626, AccessControlEn
_harvest(address(strategy));
_gulp();

Strategy memory strategyData = strategies[withdrawalQueue[i]];
Strategy storage strategyStorage = strategies[address(strategy)];

uint256 sharesBalance = strategy.balanceOf(address(this));
uint256 underlyingBalance = strategy.convertToAssets(sharesBalance);

uint256 desiredAssets = assets - assetsRetrieved;
uint256 withdrawAmount = (underlyingBalance >= desiredAssets) ? desiredAssets : underlyingBalance;
uint256 withdrawAmount = (underlyingBalance > desiredAssets) ? desiredAssets : underlyingBalance;

// Update allocated assets
strategies[withdrawalQueue[i]].allocated = strategyData.allocated - uint120(withdrawAmount);
strategyStorage.allocated -= uint120(withdrawAmount);
totalAllocated -= withdrawAmount;

// update assetsRetrieved
Expand Down
10 changes: 10 additions & 0 deletions test/common/FourSixTwoSixAggBase.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,14 @@ contract FourSixTwoSixAggBase is EVaultTestBase {
vm.prank(from);
fourSixTwoSixAgg.addStrategy(strategy, allocationPoints);
}

function _getWithdrawalQueue() internal view returns (address[] memory) {
uint256 length = fourSixTwoSixAgg.withdrawalQueueLength();

address[] memory queue = new address[](length);
for (uint256 i = 0; i < length; ++i) {
queue[i] = fourSixTwoSixAgg.withdrawalQueue(i);
}
return queue;
}
}
50 changes: 50 additions & 0 deletions test/unit/RemoveStrategy.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,56 @@ contract RemoveStrategyTest is FourSixTwoSixAggBase {
assertEq(fourSixTwoSixAgg.withdrawalQueueLength(), withdrawalQueueLengthBefore - 1);
}

function testRemoveStrategy_WithdrawalQueueOrdering() public {
address strategy1 =
factory.createProxy(address(0), true, abi.encodePacked(address(assetTST), address(oracle), unitOfAccount));
address strategy2 =
factory.createProxy(address(0), true, abi.encodePacked(address(assetTST), address(oracle), unitOfAccount));
address strategy3 =
factory.createProxy(address(0), true, abi.encodePacked(address(assetTST), address(oracle), unitOfAccount));

_addStrategy(manager, strategy1, strategyAllocationPoints);
_addStrategy(manager, strategy2, strategyAllocationPoints);
_addStrategy(manager, strategy3, strategyAllocationPoints);

address[] memory withdrawalQueue = _getWithdrawalQueue();
assertEq(withdrawalQueue.length, 4);
assertEq(withdrawalQueue[0], address(eTST));
assertEq(withdrawalQueue[1], strategy1);
assertEq(withdrawalQueue[2], strategy2);
assertEq(withdrawalQueue[3], strategy3);

vm.prank(manager);
fourSixTwoSixAgg.removeStrategy(strategy2);

withdrawalQueue = _getWithdrawalQueue();
assertEq(withdrawalQueue.length, 3);
assertEq(withdrawalQueue[0], address(eTST));
assertEq(withdrawalQueue[1], strategy1);
assertEq(withdrawalQueue[2], strategy3);

vm.prank(manager);
fourSixTwoSixAgg.removeStrategy(strategy3);

withdrawalQueue = _getWithdrawalQueue();
assertEq(withdrawalQueue.length, 2);
assertEq(withdrawalQueue[0], address(eTST));
assertEq(withdrawalQueue[1], strategy1);

vm.prank(manager);
fourSixTwoSixAgg.removeStrategy(address(eTST));

withdrawalQueue = _getWithdrawalQueue();
assertEq(withdrawalQueue.length, 1);
assertEq(withdrawalQueue[0], strategy1);

vm.prank(manager);
fourSixTwoSixAgg.removeStrategy(strategy1);

withdrawalQueue = _getWithdrawalQueue();
assertEq(withdrawalQueue.length, 0);
}

function testRemoveStrategy_fromUnauthorized() public {
vm.prank(deployer);
vm.expectRevert();
Expand Down

0 comments on commit 142e57b

Please sign in to comment.