Platform: Code4rena
Start Date: 21/10/2021
Pot Size: $80,000 ETH
Total HM: 28
Participants: 15
Period: 7 days
Judge: ghoulsol
Total Solo HM: 19
Id: 42
League: ETH
Rank: 9/15
Findings: 2
Award: $1,188.76
🌟 Selected for report: 4
🚀 Solo Findings: 0
0.0345 ETH - $143.80
ye0lde
Redundant arithmetic underflow/overflow checks can be avoided when an underflow/overflow cannot happen.
The "unchecked" keyword can be applied here since there is an "if" statement before to ensure the arithmetic operations, would not cause an integer underflow or overflow. https://github.com/code-423n4/2021-10-mochi/blob/8458209a52565875d8b2cefcb611c477cefb9253/projects/mochi-core/contracts/vault/MochiVault.sol#L267
Change the code at 267 to: <code> unchecked { debts -= _amount; } </code>
A similar change can be made here: https://github.com/code-423n4/2021-10-mochi/blob/8458209a52565875d8b2cefcb611c477cefb9253/projects/mochi-core/contracts/vault/MochiVault.sol#L269
Visual Studio Code, Remix
Add the "unchecked" keyword as shown above.
🌟 Selected for report: ye0lde
0.0768 ETH - $319.56
ye0lde
Caching the "vesting" state variable instead of repeatedly reading and writing it will decrease deployment and runtime gas. This is especially true for the modifier "checkClaimable" which is used on every function in the contract.
The checkClaimable function is here: https://github.com/code-423n4/2021-10-mochi/blob/8458209a52565875d8b2cefcb611c477cefb9253/projects/mochi-core/contracts/emission/VestedRewardPool.sol#L22-L29
An example of its use is here along with many other accesses to the "vesting" state variable. https://github.com/code-423n4/2021-10-mochi/blob/8458209a52565875d8b2cefcb611c477cefb9253/projects/mochi-core/contracts/emission/VestedRewardPool.sol#L36-L46
Visual Studio Code, Remix
I suggest modifying "checkClaimable as follows:
<code> function checkClaimable(Vesting memory v) internal pure returns(Vesting memory) { if (v.ends < block.timestamp) { v.claimable += v.vested; v.vested = 0; v.ends = 0; } return v; } </code>and I suggest these changes to function "vest"
<code> function vest(address _recipient) external { Vesting memory v = checkClaimable(vesting[_recipient]); uint256 amount = mochi.balanceOf(address(this)) - mochiUnderManagement; uint256 weightedEnd = (v.vested * v.ends + amount * (block.timestamp + 90 days)) / (v.vested + amount); v.vested += amount; v.ends = weightedEnd; vesting[_recipient] = v; mochiUnderManagement += amount; } </code>These functions are also candidates for similar changes: https://github.com/code-423n4/2021-10-mochi/blob/8458209a52565875d8b2cefcb611c477cefb9253/projects/mochi-core/contracts/emission/VestedRewardPool.sol#L48-L71
#0 - r2moon
2021-10-27T13:48:29Z
Could you simulate how much gas can be saved for this update?
🌟 Selected for report: ye0lde
0.0768 ETH - $319.56
ye0lde
Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition has been met.
Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.
Revert strings > 32 bytes are here: https://github.com/code-423n4/2021-10-mochi/blob/8458209a52565875d8b2cefcb611c477cefb9253/projects/mochi-library/contracts/SushiswapV2Library.sol#L12 https://github.com/code-423n4/2021-10-mochi/blob/8458209a52565875d8b2cefcb611c477cefb9253/projects/mochi-library/contracts/SushiswapV2Library.sol#L37-L38 https://github.com/code-423n4/2021-10-mochi/blob/8458209a52565875d8b2cefcb611c477cefb9253/projects/mochi-library/contracts/SushiswapV2Library.sol#L44-L45 https://github.com/code-423n4/2021-10-mochi/blob/8458209a52565875d8b2cefcb611c477cefb9253/projects/mochi-library/contracts/SushiswapV2Library.sol#L54-L55
Visual Studio Code
Shorten the revert strings to fit in 32 bytes.
Or consider using Custom Errors (solc >=0.8.4).
#0 - r2moon
2021-10-27T13:45:51Z
we don't want to fix that.
🌟 Selected for report: ye0lde
0.0768 ETH - $319.56
ye0lde
Cache the result of engine.usdm().balanceOf to simplify code and save gas.
engine.usdm().balanceOf is called twice in function updateReserve here: https://github.com/code-423n4/2021-10-mochi/blob/8458209a52565875d8b2cefcb611c477cefb9253/projects/mochi-core/contracts/feePool/FeePoolV0.sol#L32-L38
I suggest modifying the code as follows: <code> function updateReserve() external override { uint256 balanceOf = engine.usdm().balanceOf(address(this)); treasuryShare += ((balanceOf - mochiShare - treasuryShare) * treasuryRatio) / 1e18; mochiShare = balanceOf - treasuryShare; } </code>
Visual Studio Code, Remix
See POC
ye0lde
Caching the reference to "details[_id]" will decrease deployment and runtime gas.
The "details[_id]" variable is used 11 times in function "borrow" here https://github.com/code-423n4/2021-10-mochi/blob/8458209a52565875d8b2cefcb611c477cefb9253/projects/mochi-core/contracts/vault/MochiVault.sol#L213-L250
Other functions that could benefit from caching "detail[_id]" are here: https://github.com/code-423n4/2021-10-mochi/blob/8458209a52565875d8b2cefcb611c477cefb9253/projects/mochi-core/contracts/vault/MochiVault.sol#L85 https://github.com/code-423n4/2021-10-mochi/blob/8458209a52565875d8b2cefcb611c477cefb9253/projects/mochi-core/contracts/vault/MochiVault.sol#L158 https://github.com/code-423n4/2021-10-mochi/blob/8458209a52565875d8b2cefcb611c477cefb9253/projects/mochi-core/contracts/vault/MochiVault.sol#L181
Visual Studio Code, Remix
For example I suggest adding the following to function "borrow" at line 223 as follows:
<code> Detail storage d = details[_id]; </code>and then changing all references to "details[_id]" to "d".
#0 - r2moon
2021-10-27T15:24:49Z
duplicated with https://github.com/code-423n4/2021-10-mochi-findings/issues/28