Mochi contest - hyh's results

Handle

hyh

Vulnerability details

MochiCSSRv0.getLiquidity isn't called by other contract's functions and can be external

Make getLiquidity external instead of public to avoid placing its input to memory https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-cssr/contracts/MochiCSSRv0.sol#L126

Vault asset variable is set on initialization, doesn't change afterwards and should be immutable

IERC20 public immutable override asset; in: https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-core/contracts/vault/MochiVault.sol#L21

Save repetitive function call result to memory

address memory id_owner = engine.nft().ownerOf(_id) Use id_owner in https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-core/contracts/vault/MochiVault.sol#L186 https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-core/contracts/vault/MochiVault.sol#L210

Save the result of credit cap access, increase readability

uint256 memory credit_cap = engine.mochiProfile().creditCap(address(asset)) - debts if (_amount > credit_cap) { _amount = credit_cap } In: https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-core/contracts/vault/MochiVault.sol#L231

Add auctionID variable to DutchAuctionLiquidator.triggerLiquidation

auctionId(_asset, _nftId) is called twice, so saving its result to memory saves gas. Also, first line, 'IMochiVault vault = engine.vaultFactory().getVault(_asset)' should be placed after require (l. 75) as it's needed only afterwards. https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-core/contracts/liquidator/DutchAuctionLiquidator.sol#L74

Unify Vault.repay logic

https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-core/contracts/vault/MochiVault.sol#L254 Lines 259-261 and 269-272 can be unified (as amount_ variable doesn't change elsewhere), removing l.270 check and increasing readability. Now (l.259-272): if (_amount > details[_id].debt) { _amount = details[_id].debt; } require(_amount > 0, "zero"); if (debts < _amount) { // safe gaurd to some underflows debts = 0; } else { debts -= _amount; } details[_id].debt -= _amount; if (details[_id].debt == 0) { details[_id].status = Status.Collaterized; } To be: require(_amount > 0 && details[_id].debt > 0, "zero amount or debt"); if (_amount >= details[_id].debt) { _amount = details[_id].debt; details[_id].debt = 0; details[_id].status = Status.Collaterized; } else { details[_id].debt -= _amount; } if (debts < _amount) { // underflow safe guard debts = 0; } else { debts -= _amount; } Also, the comment https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-core/contracts/vault/MochiVault.sol#L252 should be updated

currentDebt call is redundant in Vault.liquidate

As liquidate function has updateDebt(_id) modifier details[_id].debt is already current and can be used instead of currentDebt(_id) call https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-core/contracts/vault/MochiVault.sol#L286 Now: require( _liquidatable(details[_id].collateral, price, currentDebt(_id)), "healthy" ); To be: require( _liquidatable(details[_id].collateral, price, details[_id].debt), "healthy" );

Vault.liquidate can take in position id only and return position's collateral and debt

As Vault.liquidate should be called with current debt and collateral only (i.e. there is no other scenarios) it can set _collateral, _usdm to details[_id].collateral and details[_id].debt directly, while its signature can be simplified to be function liquidate(uint256 _id). This way liquidator code will be simplified as follows: Now: uint256 debt = vault.currentDebt(_nftId); (, uint256 collateral, , , ) = vault.details(_nftId); vault.liquidate(_nftId, collateral, debt); To be: (uint256 debt, uint256 collateral) = vault.liquidate(_nftId);

In order to save unnecessary calculations Vault.liquidate can check position collateral and debt to be positive at the start

Adding require(details[_id].collateral > 0 && details[_id].debt > 0, "!positive collateral or debt"); Will save gas on running numbers for non-valid positions. https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-core/contracts/vault/MochiVault.sol#L283

UniswapV2CSSR.saveReserve's stateRoot variable isn't needed

Remove the variable and replace the only stateRoot usage (l. 92 in both contracts) with 'blockState[blockNumber].stateRoot' https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-cssr/contracts/cssr/SushiswapV2CSSR.sol#L86 https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-cssr/contracts/cssr/UniswapV2CSSR.sol#L86

UniswapV2CSSR.getLiquidity optimization

historicData is redundant, direct access to storage saves memory copying gas Remove the variable, l. 169 return statement now: return denominationTokenIs0 ? historicData.reserve0 : historicData.reserve1; to be: return denominationTokenIs0 ? observedData[lastObserved][address(pair)].reserve0 : observedData[lastObserved][address(pair)].reserve1; https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-cssr/contracts/cssr/SushiswapV2CSSR.sol#L166 https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-cssr/contracts/cssr/UniswapV2CSSR.sol#L166