Platform: Code4rena
Start Date: 16/11/2021
Pot Size: $30,000 USDC
Total HM: 3
Participants: 18
Period: 3 days
Judge: leastwood
Total Solo HM: 2
Id: 56
League: ETH
Rank: 7/18
Findings: 2
Award: $903.24
🌟 Selected for report: 3
🚀 Solo Findings: 0
ye0lde
There are issues with the comment describing this state variable and the event that is emitted when it is set.
The comment for borrowFee is exactly the same as harvestFee.
The borrowFee is charged during minting while the harvestFee is charged during harvest.
https://github.com/code-423n4/2021-11-nested/blob/f646002b692ca5fa3631acfff87dda897541cf41/contracts/NestedFactory.sol#L553-L554
In the setBorrowFee function an HarvestFeeUpdated emit is done. There is no setBorrowFee event.
https://github.com/code-423n4/2021-11-yaxis/blob/146febcb61ae7fe20b0920849c4f4bbe111c6ba7/contracts/v3/alchemix/Alchemist.sol#L299
Visual Studio Code, Remix
Clarify the comments about the purpose of each state variable and add a unique event/emit for BorrowFeeUpdated.
#0 - Xuefeng-Zhu
2021-12-09T06:09:55Z
🌟 Selected for report: ye0lde
751.5154 USDC - $751.52
ye0lde
Distribution could be off slightly if the code should match the comment.
Visual Studio Code, Remix
Correct the code to
if(deltaTime > TRANSMUTATION_PERIOD)
or clarify the comment to match the code.
#0 - 0xleastwood
2021-12-21T04:24:42Z
According to the judging documentation, issues related to comments incorrect to spec are marked as low.
Will keep this issue as is.
1 — Low (L): vulns that have a risk of 1 are considered “Low” severity when assets are not at risk. Includes state handling, function incorrect as to spec, and issues with comments.
🌟 Selected for report: ye0lde
47.3259 USDC - $47.33
ye0lde
Removing unused named return variables can reduce gas usage and improve code clarity.
Unused named returns https://github.com/code-423n4/2021-11-yaxis/blob/146febcb61ae7fe20b0920849c4f4bbe111c6ba7/contracts/v3/alchemix/Transmuter.sol#L394-L397 https://github.com/code-423n4/2021-11-yaxis/blob/146febcb61ae7fe20b0920849c4f4bbe111c6ba7/contracts/v3/alchemix/Transmuter.sol#L424
Visual Studio Code, Remix
Either remove the named variables or Delete the similar local variables that were created and used instead of the named variables. The return statement can also be deleted.
For example, refactor the userInfo function https://github.com/code-423n4/2021-11-yaxis/blob/146febcb61ae7fe20b0920849c4f4bbe111c6ba7/contracts/v3/alchemix/Transmuter.sol#L394-L397 like this:
function userInfo(address user) public view returns ( uint256 depositedAl, uint256 pendingdivs, uint256 inbucket, uint256 realised ) { depositedAl = depositedAlTokens[user]; uint256 _toDistribute = buffer.mul(block.number.sub(lastDepositBlock)).div(TRANSMUTATION_PERIOD); if(block.number.sub(lastDepositBlock) > TRANSMUTATION_PERIOD){ _toDistribute = buffer; } pendingdivs = _toDistribute.mul(depositedAlTokens[user]).div(totalSupplyAltokens); inbucket = tokensInBucket[user].add(dividendsOwing(user)); realised = realisedTokens[user]; }
getMultipleUserInfo can be refactored in the same way.
#0 - Xuefeng-Zhu
2021-11-23T07:09:21Z
I think this is for readability
#1 - 0xleastwood
2021-12-21T05:33:53Z
This is a valid finding. As far as I'm aware, declaring named returns uses memory to store the created state variables. As this is unnecessary, there are a number of memory operations which can be removed, slightly improving gas costs for affected function calls.
🌟 Selected for report: ye0lde
47.3259 USDC - $47.33
ye0lde
Using existing local variables instead of reading state variables will save gas by converting SLOADs to MLOADs.
newTransmutationPeriod can be used here instead of TRANSMUTATION_PERIOD : https://github.com/code-423n4/2021-11-yaxis/blob/146febcb61ae7fe20b0920849c4f4bbe111c6ba7/contracts/v3/alchemix/Transmuter.sol#L194
TRANSMUTATION_PERIOD is named like a constant when it is actually an updatable state variable.
Visual Studio Code, Remix
Use the local variable instead of the state variable. Rename TRANSMUTATION_PERIOD appropriately.