Platform: Code4rena
Start Date: 30/09/2021
Pot Size: $75,000 ETH
Total HM: 9
Participants: 15
Period: 7 days
Judge: 0xean
Total Solo HM: 2
Id: 39
League: ETH
Rank: 13/15
Findings: 2
Award: $1,421.02
π Selected for report: 2
π Solo Findings: 0
0.0893 ETH - $264.61
ye0lde
Open TODOs can point to programming or other errors that still need to be fixed.
The TODOs are here: https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/swivel/Hash.sol#L93 https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/swivel/Sig.sol#L41 https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/marketplace/MarketPlace.sol#L3 https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/marketplace/MarketPlace.sol#L62
VS Code
Resolve the TODOs.
#0 - JTraversa
2021-10-07T15:43:06Z
Personally wouldnt consider these issues Β―_(γ)_/Β―, and if issues, definitely non-critical
#1 - 0xean
2021-10-16T23:18:09Z
dupe of #67
π Selected for report: ye0lde
ye0lde
Operating on a copy of a state variable seems inefficient and confusing in this case.
From a "gas" standpoint it's less efficient. And future changes could render the addresses in the copied struct invalid if functions being called in redeemZcToken operate on the original state variable.
The copy occurs here: https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/marketplace/MarketPlace.sol#L123
The "mkt" variable is referenced here: https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/marketplace/MarketPlace.sol#L131
VS Code
Remove line #123: https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/marketplace/MarketPlace.sol#L123
Replace "mkt" with "markets[u][m]" in line #131 https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/marketplace/MarketPlace.sol#L131
#0 - JTraversa
2021-11-03T19:35:09Z
Superceded by removing matured and using mkt.maturityRate == 0 as an indicator
0.0171 ETH - $50.84
ye0lde
Public functions need to copy their arguments from calldata to memory resulting in more gas consumption. They can be declared external if they aren't called from within the contract and this results in gas savings.
https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/marketplace/MarketPlace.sol#L53 https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/marketplace/MarketPlace.sol#L234 https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/marketplace/MarketPlace.sol#L245
https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/vaulttracker/VaultTracker.sol#L36 https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/vaulttracker/VaultTracker.sol#L70 https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/vaulttracker/VaultTracker.sol#L243
VS Code
Change function visibility from public to externalβ¨
#0 - JTraversa
2021-10-10T05:41:22Z
Need to doublecheck for other duplicates of these exact functions.
#1 - 0xean
2021-10-16T22:36:41Z
dupe of #18
0.0423 ETH - $125.54
ye0lde
There is additional gas usage when an array's length value is used directly in a "for" loop.
The array's length value is used directly in a for loop here: https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/swivel/Swivel.sol#L57 https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/swivel/Swivel.sol#L211
Visual Studio Code, Remix
Change the loops above from: <code> for (uint256 i=0; i < o.length; i++) </code>
to <code> unit256 length = o.length; for (uint256 i=0; i < length; i++) </code>
When I tested these changes there was a small gas saving.
#0 - JTraversa
2021-10-24T20:52:09Z