xiaoming90 - `CrossCurrencyfCashVault` Cannot Settle Its Assets In Pieces

[sherlock-admin]

xiaoming90

high

CrossCurrencyfCashVault Cannot Settle Its Assets In Pieces

Summary

The CrossCurrencyfCashVault vault cannot settle its assets in pieces. Thus, it might cause the vault to incur unnecessary slippage.

Vulnerability Detail

The settle vault function is designed in a manner where its assets can be settled in pieces. Therefore, the settleVault function accepts a strategyTokens or strategyTokensToRedeem parameter to allow the caller to specify the number of strategy tokens to be settled.

The reason as mentioned in Notional's walkthrough video (Refer to the explanation at 15.50min mark) is that in some cases the caller might want to break down into multiple transactions due to massive slippage.

For instance, the vault might utilize a 2 day settlement period to allow the vault to settle its assets in pieces so that it can avoid unnecessary transaction costs associated with converting all its assets back to USDC in a single transaction.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/CrossCurrencyfCashVault.sol#L121

File: CrossCurrencyfCashVault.sol
113:     /**
114:      * @notice During settlement all of the fCash balance in the lend currency will be redeemed to the
115:      * underlying token and traded back to the borrow currency. All of the borrow currency will be deposited
116:      * into the Notional contract as asset tokens and held for accounts to withdraw. Settlement can only
117:      * be called after maturity.
118:      * @param maturity the maturity to settle
119:      * @param settlementTrade details for the settlement trade
120:      */
121:     function settleVault(uint256 maturity, uint256 strategyTokens, bytes calldata settlementTrade) external {
122:         require(maturity <= block.timestamp, "Cannot Settle");
123:         VaultState memory vaultState = NOTIONAL.getVaultState(address(this), maturity);
124:         require(vaultState.isSettled == false);
125:         require(vaultState.totalStrategyTokens >= strategyTokens);
126: 
127:         RedeemParams memory params = abi.decode(settlementTrade, (RedeemParams));
128:     
129:         // The only way for underlying value to be negative would be if the vault has somehow ended up with a borrowing
130:         // position in the lend underlying currency. This is explicitly prevented during redemption.
131:         uint256 underlyingValue = convertStrategyToUnderlying(
132:             address(0), vaultState.totalStrategyTokens, maturity
133:         ).toUint();
134: 
135:         // Authenticate the minimum purchase amount, all tokens will be sold given this slippage limit.
136:         uint256 minAllowedPurchaseAmount = (underlyingValue * settlementSlippageLimit) / SETTLEMENT_SLIPPAGE_PRECISION;
137:         require(params.minPurchaseAmount >= minAllowedPurchaseAmount, "Purchase Limit");
138: 
139:         NOTIONAL.redeemStrategyTokensToCash(maturity, strategyTokens, settlementTrade);
140: 
141:         // If there are no more strategy tokens left, then mark the vault as settled
142:         vaultState = NOTIONAL.getVaultState(address(this), maturity);
143:         if (vaultState.totalStrategyTokens == 0) {
144:             NOTIONAL.settleVault(address(this), maturity);
145:         }
146:     }

During vault settlement, the CrossCurrencyfCashVault._redeemFromNotional function will be called, and the code in lines 252-262 will be executed. However, it was observed that the strategyTokens parameter is ignored, and the vault will forcefully settle all the strategy tokens in one go. As such, there is no way for the caller to break down the settle vault transaction into multiple transactions.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/CrossCurrencyfCashVault.sol#L243

File: CrossCurrencyfCashVault.sol
243:     function _redeemFromNotional(
244:         address account,
245:         uint256 strategyTokens,
246:         uint256 maturity,
247:         bytes calldata data
248:     ) internal override returns (uint256 borrowedCurrencyAmount) {
249:         uint256 balanceBefore = LEND_UNDERLYING_TOKEN.balanceOf(address(this));
250:         RedeemParams memory params = abi.decode(data, (RedeemParams));
251: 
252:         if (maturity <= block.timestamp) {
253:             // Only allow the vault to redeem past maturity to settle all positions
254:             require(account == address(this));
255:             NOTIONAL.settleAccount(address(this));
256:             (int256 cashBalance, /* */, /* */) = NOTIONAL.getAccountBalance(LEND_CURRENCY_ID, address(this));
257: 
258:             // It should never be possible that this contract has a negative cash balance
259:             require(0 <= cashBalance && cashBalance <= int256(uint256(type(uint88).max)));
260: 
261:             // Withdraws all cash to underlying
262:             NOTIONAL.withdraw(LEND_CURRENCY_ID, uint88(uint256(cashBalance)), true);
263:         } else {
264:             // Sells fCash on Notional AMM (via borrowing)
265:             BalanceActionWithTrades[] memory action = _encodeBorrowTrade(
266:                 maturity,
267:                 strategyTokens,
268:                 params.maxBorrowRate
269:             );
270:             NOTIONAL.batchBalanceAndTradeAction(address(this), action);
271: 
272:             // Check that we have not somehow borrowed into a negative fCash position, vault borrows
273:             // are not included in account context
274:             AccountContext memory accountContext = NOTIONAL.getAccountContext(address(this));
275:             require(accountContext.hasDebt == 0x00);
276:         }
277: 
278:         uint256 balanceAfter = LEND_UNDERLYING_TOKEN.balanceOf(address(this));
279:         
280:         // Trade back to borrow currency for repayment
281:         Trade memory trade = Trade({
282:             tradeType: TradeType.EXACT_IN_SINGLE,
283:             sellToken: address(LEND_UNDERLYING_TOKEN),
284:             buyToken: address(_underlyingToken()),
285:             amount: balanceAfter - balanceBefore,
286:             limit: params.minPurchaseAmount,
287:             deadline: block.timestamp,
288:             exchangeData: params.exchangeData
289:         });
290: 
291:         (/* */, borrowedCurrencyAmount) = _executeTrade(params.dexId, trade);
292:     }

Impact

The vault might incur unnecessary slippage during settlement as the settlement cannot be broken into multiple transactions.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/CrossCurrencyfCashVault.sol#L121

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/CrossCurrencyfCashVault.sol#L243

Tool used

Manual Review

Recommendation

It is recommended to update the CrossCurrencyfCashVault._redeemFromNotional function to allow the vault to be settled in multiple transactions.

[jeffywu]

Valid suggestion.