[L-1] Commend mismatch
on strategy mainnet
Comment mismatch in StrategyMainnet.sol
StrategyMainnet.sol file has the following note:
But onlyEmergencyAuthorized is never used. Consider removing it from the file to avoid future confusion.
[L-2] Unused line in _harvestAndReport function
_harvestAndReport function in StrategyMainnet and StrategyArb contracts have an unused line. StrategyOp function does not have the same issue.
Details
In StrategyMainnet
function _harvestAndReport() internal override returns (uint256 _totalAssets) {
uint256 claimable = transmuter.getClaimableBalance(address(this));
if (claimable > 0) {
}
uint256 unexchanged = transmuter.getUnexchangedBalance(address(this));
uint256 underlyingBalance = underlying.balanceOf(address(this));
_totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance;
}
In StrategyArb
function _harvestAndReport()
internal
override
returns (uint256 _totalAssets)
{
uint256 claimable = transmuter.getClaimableBalance(address(this));
if (claimable > 0) {
}
uint256 unexchanged = transmuter.getUnexchangedBalance(address(this));
uint256 underlyingBalance = underlying.balanceOf(address(this));
_totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance;
}
Recommendation
Remove the unused line, it doesn’t do anything but take place. Or consider actually implementing the logic required.
[L-3] Code compatibility issue on claimAndSwap function
All three strategies, StrategyArb, StrategyMainnet,StrategyOp make use of the function claimAndSwap, but, while they’re doing, or supposed to do the exactly same thing, they are written differently.
Details
In StrategyMainnet::claimAndSwap we see the following:
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, uint256 _routeNumber) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
require(_minOut > _amountClaim, "minOut too low");
router.exchange(
routes[_routeNumber], swapParams[_routeNumber], _amountClaim, _minOut, pools[_routeNumber], address(this)
);
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
While in StrategyArb.sol and StrategyOp.sol the same function is written differently in the sense that, for the swapping, in these contracts, we have a novel _swapUnderlyingToAsset function. This is the flow:
StrategyArb ⇒
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IRamsesRouter.route[] calldata _path) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
_swapUnderlyingToAsset(_amountClaim, _minOut, _path);
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IRamsesRouter.route[] calldata _path) internal {
require(minOut > _amount, "minOut too low");
uint256 underlyingBalance = underlying.balanceOf(address(this));
require(underlyingBalance >= _amount, "not enough underlying balance");
IRamsesRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), block.timestamp);
}
StrategyOp ⇒
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path ) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
_swapUnderlyingToAsset(_amountClaim, _minOut, _path);
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
@dev internal function for swapping WETH to alETH via Velo Router
*/
function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IVeloRouter.route[] calldata _path) internal {
require(minOut > _amount, "minOut too low");
uint256 underlyingBalance = underlying.balanceOf(address(this));
require(underlyingBalance >= _amount, "not enough underlying balance");
IVeloRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), block.timestamp);
}
Impact
Affects the code readability.
Recommendation
It is recommended to choose one style and stick to it.
[L-4] No SPDX license identifier in IAlchemist.sol, ITransmuter.sol, IVelo.sol
Recommendation
Add SPDX license identifier
