Mochi contest - pauliax's results

Handle

pauliax

Vulnerability details

Impact

Using the unchecked keyword to avoid redundant arithmetic underflow/overflow checks to save gas when an underflow/overflow cannot happen. We can apply the unchecked keyword in the following lines of code since there are checks before that ensure the arithmetic operations would not cause an integer underflow or overflow.

numerator: auction.startedAt + DURATION > block.number ? auction.startedAt + DURATION - block.number : 0

or

return type(uint256).max - totalSupply();

Recommended Mitigation Steps

Consider using 'unchecked' where it is safe to do so.

Handle

pauliax

Vulnerability details

Impact

Cache duplicate access of storage variables when they do not change between these interactions. For example, here: usdm.approve(address(uniswapRouter), reward[msg.sender]); uniswapRouter.swapExactTokensForTokens( reward[msg.sender], ... );

this duplicate storage read can be eliminated: uint userReward = reward[msg.sender]; usdm.approve(address(uniswapRouter), userReward); uniswapRouter.swapExactTokensForTokens( userReward, ... );

There are places where these duplicate accesses are all over the place so I expect huge gas costs there, e.g. modifier checkClaimable and function vest access vesting[recipient] 5 times, function accrueDebt accesses details mapping 8 times, and so on.

Recommended Mitigation Steps

Please revisit such places and consider caching values that do not change to improve gas usage.

Handle

pauliax

Vulnerability details

Impact

The check could be inclusive <= here to save some gas when debts = _amount (also 'unchecked' keyword can be used):

if (debts < _amount) { // safe gaurd to some underflows debts = 0; } else { debts -= _amount; }

Handle

pauliax

Vulnerability details

Impact

This not only loses some precision (cuz of multiplication and division) but also consumes more gas: // send Mochi to vMochi Vault mochi.transfer( address(engine.vMochi()), (mochiBalance * vMochiRatio) / 1e18 ); // send Mochi to veCRV Holders mochi.transfer( crvVoterRewardPool, (mochiBalance * (1e18 - vMochiRatio)) / 1e18 );

Recommended Mitigation Steps

Proposed improvement: // send Mochi to vMochi Vault uint toVault = (mochiBalance * vMochiRatio) / 1e18; mochi.transfer( address(engine.vMochi()), toVault ); // send Mochi to veCRV Holders mochi.transfer( crvVoterRewardPool, mochiBalance - toVault; );

This way you the whole mochiBalance will be transferred and it will cost less to do that as fewer math operations are performed.

Handle

pauliax

Vulnerability details

Impact

Cache results of duplicate external calls when the results between them do not change. E.g. here engine.mochi() is called 2 times: path[2] = address(engine.mochi()); ... engine.mochi().transfer( msg.sender, engine.mochi().balanceOf(address(this)) );

Recommended Mitigation Steps

can be refactored to: IMochi mochi = engine.mochi(); path[2] = address(mochi); ... mochi.transfer( msg.sender, mochi.balanceOf(address(this)) );

Handle

pauliax

Vulnerability details

Impact

contract USDM does not need to import IERC3156FlashLender again as it was already imported in IUSDM. import "../interfaces/IERC3156FlashLender.sol";

contract DutchAuctionLiquidator makes no use of these imports: import "@openzeppelin/contracts/token/ERC721/IERC721.sol"; import "@uniswap/v2-periphery/contracts/interfaces/IUniswapV2Router02.sol"; import "@mochifi/library/contracts/BeaconProxyDeployer.sol";

Recommended Mitigation Steps

Consider reviewing all the unused imports and removing them to reduce the size of the contract and thus save some deployment gas.

Handle

pauliax

Vulnerability details

Impact

There are some validations that could be enforced by smart contracts, e.g.:

function vest in VestedRewardPool should require that _recipient is not empty to prevent accidentally locked funds.

functions changevMochiRatio and changeTreasuryRatio in FeePoolV0 should require that _ratio is <= 1e18 (100%).

function addReward in NoMochiReferralFeePool and ReferralFeePoolV0 should require that _recipient is not empty to prevent accidentally locked funds.

Recommended Mitigation Steps

Consider enforcing proposed validations. Also, review other functions that may contain such missing validations.

Handle

pauliax

Vulnerability details

Impact

Duplicate math operations can be cached to reduce gas usage, e.g. no need to calculate updatedFee / 2 twice here: operationShare += updatedFee / 2; veCRVShare += updatedFee / 2;

or here: mochi.transfer(msg.sender, _amount / 2); mochi.transfer(address(vMochi), _amount / 2);

Recommended Mitigation Steps

Eliminate useless duplicate calculations.

Handle

pauliax

Vulnerability details

Impact

function flashLoan does not necesseraly need this check: require(_token == address(this), "!supported"); as it will be later checked again in function flashFee.

Recommended Mitigation Steps

Consider removing it to reduce gas usage.

Handle

pauliax

Vulnerability details

Impact

Gas efficiency can be achieved by tightly packing the struct. Struct variables are stored in 32 bytes each so you can group smaller types to occupy less storage. For example, startedAt or boughtAt in Auction struct hold block.number so realistically this does not need uint256 and you can consider storing it in lower type. You can read more here: https://fravoll.github.io/solidity-patterns/tight_variable_packing.html or in the official documentation: https://docs.soliditylang.org/en/v0.4.21/miscellaneous.html

Recommended Mitigation Steps

Search for an optimal size and order of structs to reduce gas usage.