Open code423n4 opened 2 years ago
Using ++i for all the loops in the variable. it is known and common case that implementation by using ++i can cost less gas per iteration than i++
it can bee seen from here : https://github.com/ethereum/solidity/issues/10695
main/contracts/ClearingHouse.sol#L122; main/contracts/ClearingHouse.sol#L170; main/contracts/ClearingHouse.sol#L194; main/contracts/ClearingHouse.sol#L251; main/contracts/ClearingHouse.sol#L263; main/contracts/ClearingHouse.sol#L278; main/contracts/MarginAccount.sol#L331; main/contracts/MarginAccount.sol#L373; main/contracts/MarginAccount.sol#L521; main/contracts/MarginAccount.sol#L552;
i++ change to ++i
i++
++i
storage
memory
https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/AMM.sol#L588 https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/VUSD.sol#L58
instead of caching with memory it can be used by caching with storage, just read it directly for saving more gas
change to
Position storage position = positions[trader]; Withdrawal storage withdrawal = withdrawals[i];
Position storage position = positions[trader];
Withdrawal storage withdrawal = withdrawals[i];
&&
https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/MarginAccount.sol#L461 using mutiple require() is cheaper than use && if this was used many times. Because it can save gas execution cost
require()
require(idx > VUSD_IDX && idx < supportedCollateral.length, "collateral not seizable");
require idx > VUSD_IDX; require supportedCollateral.length, "collateral not seizable";
constructor()
initialize()
https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L35-L56 better to execute all the codes in the initialize() using constructor(). The idea is of using initialize is to run all the code inside it once in a lifetime. The current implementation of some of the contract is using both constructor() & initialize()
msg.sender
https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L69 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L65 by using msg.sender instead of _msgSender() can save gas
_msgSender()
6# The vars can set to immutable https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L22-L24 vusd, insuranceFund, & marginAccount set once at initialize(). Use immutable
vusd
insuranceFund
marginAccount
7# There is no int which using Safecast lib https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L13 In the ClearingHouse.sol there is no int which is using the lib. Remove the line 13
Safecast
ClearingHouse.sol
8# Better way to call SafeCastlib function https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L12-L13 By removing lines which declaring that this contract is using SafeCast lib and directly call it on each function, it can save execution gas cost
SafeCast
int256 marginCharge = realizedPnl - SafeCast.toInt256(fee);
The SafeCast.function is only called 3 times at ClearingHouse.sol so i recommend to change it thes way https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L332
SafeCast.function
9# Calling block.timestamp directly can save gas https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L86 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L201 instead of using _blockTimestamp() func to get block.timestamp value, using block.timestamp can save gas
block.timestamp
_blockTimestamp()
10# Caching position.size in isLongPosition bool is not gas efficient https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/AMM.sol#L614 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/AMM.sol#L141 By putting position.size inside the if() condition, and removing L141 and L614, can save a lot of gas
position.size
isLongPosition
if()
L141
L614
11# Using != instead > https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L211 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L253 since uint cant be < 0, by using != operator to check condition can save gas
12# Custom error can save gas Using string to throw an error to user is more gas consuming. From solidity 0.8.4 we can use custom error to save gas. Custom errors are defined using the error statement https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L51 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L75 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L84 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L101 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L120 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L154 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L164 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L189
error
Good report.
GAS OPTIMAZATION
1 Better increment for saving more gas
Using ++i for all the loops in the variable. it is known and common case that implementation by using ++i can cost less gas per iteration than i++
POC
it can bee seen from here : https://github.com/ethereum/solidity/issues/10695
Occurance
Mitigation
i++
change to++i
2 Using
storage
insteadmemory
for saving more gashttps://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/AMM.sol#L588 https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/VUSD.sol#L58
instead of caching with
memory
it can be used by caching withstorage
, just read it directly for saving more gaschange to
Position storage position = positions[trader];
Withdrawal storage withdrawal = withdrawals[i];
3 using
&&
is more expensive gashttps://github.com/code-423n4/2022-02-hubble/blob/main/contracts/MarginAccount.sol#L461 using mutiple
require()
is cheaper than use&&
if this was used many times. Because it can save gas execution costMitigation
change to
4 Using
constructor()
insteadinitialize()
or vice versahttps://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L35-L56 better to execute all the codes in the
initialize()
usingconstructor()
. The idea is of using initialize is to run all the code inside it once in a lifetime. The current implementation of some of the contract is using bothconstructor()
&initialize()
5 Directly call
msg.sender
https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L69 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L65 by using
msg.sender
instead of_msgSender()
can save gas6# The vars can set to immutable https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L22-L24
vusd
,insuranceFund
, &marginAccount
set once atinitialize()
. Use immutable7# There is no int which using
Safecast
lib https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L13 In theClearingHouse.sol
there is no int which is using the lib. Remove the line 138# Better way to call
SafeCast
lib function https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L12-L13 By removing lines which declaring that this contract is usingSafeCast
lib and directly call it on each function, it can save execution gas costThe
SafeCast.function
is only called 3 times atClearingHouse.sol
so i recommend to change it thes way https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L3329# Calling
block.timestamp
directly can save gas https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L86 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L201 instead of using_blockTimestamp()
func to getblock.timestamp
value, usingblock.timestamp
can save gas10# Caching
position.size
inisLongPosition
bool is not gas efficient https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/AMM.sol#L614 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/AMM.sol#L141 By puttingposition.size
inside theif()
condition, and removingL141
andL614
, can save a lot of gas11# Using != instead > https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L211 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L253 since uint cant be < 0, by using != operator to check condition can save gas
12# Custom error can save gas Using string to throw an error to user is more gas consuming. From solidity 0.8.4 we can use custom error to save gas. Custom errors are defined using the
error
statement https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L51 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L75 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L84 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L101 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L120 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L154 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L164 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L189