) public nonReentrant returns (uint256) {
    tokenBalance = token.balanceOf(address(this));
    uint256 shares = maxShare;
}

if (value > token.balanceOf(address(this))) {
      value = token.balanceOf(address(this));
      shares = _sharesForAmount(value);
    }

tokenBalance = tokenBalance.sub(value);

require(totalLoss <= maxLoss.mul(value.add(totalLoss)).div(MAX_BPS), 
"revert if totalLoss is more than permitted");
token.safeTransfer(recipient, value);

require(totalLoss <= maxLoss.mul(value.add(totalLoss)).div(MAX_BPS), 
"revert if totalLoss is more than permitted");
token.safeTransfer(recipient, value);

Deposited values 1000 and 2000, but pool value is only 2998.

Recommendation 2:

Add check if pool is bigger than totalDepositedAmount.

uint256 fee = pool > totalDepositedAmount
    ? 
    pool.sub(totalDepositedAmount).mul(feeAmount).div(feePrecision)
    : 0;

‍

Possibility to exceed maximum strategies number at xVault.sol

Medium  |  Resolved

Function setWithdrawalQueue() and addStrategy() allows to exceed a MAXIMUM_STRATEGIES number.

Recommendation:

Add check for withdrawalQueue length for those functions.

‍

Ignore the return value of transfer at xVault.sol and BaseStrategy.sol

Medium  |  Resolved

In function report() at xVault.sol tokenBalance is updating but it ignores return value by token.transfer() and token.transferFrom().

In function distributeRewards() at BaseStrategy no check provided for return value of transfer.

Recommendation:

Use SafeERC20, or ensure that the transfer/transferFrom return value is checked.

Wrong approve tokens in xAuto contracts

Medium  |  Resolved

In xAuto contracts there are different lenders. xAuto approves tokens for active lenders on initialize function. When the owner activates a not active initial lender contract calls rebalance function to deposit on a new lender but since this lender tokens are not approved it can’t transfer tokens from xAuto.

Recommendation:

One of few:

1) Approve all lenders on initialization;

2) Approve a new lender before rebalancing in activateLender function.

Re-Audit:

Fixed.

‍

Optimization at XAAVE.sol and XUSDT.sol

Low |  Resolved

Functions balanceFortubeInToken() at XUSDT.sol, balanceFulcrumInToken() XAAVE.sol and XUSDT.sol are fully duplicates of internal functions  _balanceFortubeInToken(), _balanceFulcrumInToken().

Function _rebalance() at XAAVE.sol and XUSDT.sol is internal and never used.

Recommendation:

In public functions balanceFortubeInToken() and balanceFulcrumInToken() you can call their represented internal functions and return it’s value. If _rebalance() is unnecessary then just remove it.

‍

Variables can be declared as constant at xVault.sol

Low |  Resolved

MAX_BPS and SECS_PER_YEAR can be declared as constant.

Recommendation:

Change those variables to constant.

‍

Optimization at xVault.sol

Low |  Resolved

Multiple usage of require statements

msg.sender == governance, msg.sender == management,
msg.sender == guardian.

Recommendation:

Сreate a modifier for those checks or add AccessControl from OpenZeppelin where you can set all needed roles for access to specific functions.

‍

Lacks a zero-check in the constructor of xVault.sol and functions

Low |  Resolved

Constructor parameter _governance and functions setTreasury(), setGuardian(), setGovernance() have no check for zero address.

Recommendation:

Check that the address is not zero.

‍

Illogical use of deposit limit at xVault

Low |  Resolved

Contract xVault in deposit function has a check for input amount of zero to deposit all possible allowed funds. It is calculated by depositLimit - totalAssets which can lead to subtraction underflow. This check is not included if the amount is greater than zero, which means that users can’t deposit more than the limit when they deposit zero but can when more.

Recommendation:

If the logic of checking only when the amount is zero is correct then need to add check for underflow or even leave it as it is. Otherwise need to add check the global scope of function.

‍

Wrong predefined apr address as XAAVE

Low |  Resolved

Apr address at the XAAVE contract in a polygon is not a contract.

Recommendation:

Put correct address.

‍

Missed check for the token in EarnAPYWithPool

Low |  Resolved

Contract EarnAPYWithPool has 2 mappings of tokens in fulcrum and fortube to check if the token exists on this platform. For aave this check is missed which means that some tokens cannot be processed in getAPROptionsInc.

Recommendation:

Add check for existing token on aave platform.

‍

Unnecessary event and import statement at xVault.sol

Informational  |  Unresolved

All related functionality to event UpdateGuestList Is commented and not used farther in the code.

IERC20 is not used.

Recommendation:

Delete this event and all related commented functionality, delete unused import statement.

‍

Commented functions at xVault.sol

Informational  |  Resolved

Functions setName() and setSymbol() are commented.

Recommendation:

Remove commented functionality.

‍

Not proper use of assert at xVault.sol

Informational  |  Resolved

The assert function should only be used to test for internal errors and to check invariants.

Recommendation:

Require function should be used to ensure valid conditions. Replace assert with require with corresponding revert message.

‍

Not proper use of assert at XAAVE.sol and XUSDT.sol

Informational  |  Resolved

In function set_new_feePrecision() assert is used to check parameters of _newFeePrecision.

The assert function should only be used to test for internal errors and to check invariants.

Recommendation:

Require function should be used to ensure valid conditions. Replace assert with require with corresponding revert message.