Platform: Code4rena
Start Date: 10/03/2022
Pot Size: $75,000 USDT
Total HM: 25
Participants: 54
Period: 7 days
Judge: pauliax
Total Solo HM: 10
Id: 97
League: ETH
Rank: 32/54
Findings: 2
Award: $263.70
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: hickuphh3
Also found by: 0v3rf10w, 0x1f8b, 0xDjango, 0xNazgul, 0xngndev, 0xwags, Cantor_Dust, CertoraInc, Dravee, IllIllI, PPrieditis, Ruhum, TerrierLover, WatchPug, XDms, benk10, berndartmueller, bitbopper, catchup, cmichel, cryptphi, csanuragjain, danb, defsec, gzeon, hagrid, hubble, jayjonah8, kenta, kyliek, minhquanym, rfa, robee, saian, samruna, throttle, ye0lde, z3s
123.2529 USDT - $123.25
abicoder v2 is used by default in solidity version 0.8.0, so it can be removed
refer : https://blog.soliditylang.org/2020/12/16/solidity-v0.8.0-release-announcement/
Lack of zero address checks in
function changeFee( address tokenAddress, uint256 _equilibriumFee, uint256 _maxFee ) external override onlyOwner whenNotPaused { require(_equilibriumFee != 0, "Equilibrium Fee cannot be 0"); require(_maxFee != 0, "Max Fee cannot be 0"); tokensInfo[tokenAddress].equilibriumFee = _equilibriumFee; tokensInfo[tokenAddress].maxFee = _maxFee; emit FeeChanged(tokenAddress, tokensInfo[tokenAddress].equilibriumFee, tokensInfo[tokenAddress].maxFee); }
Remove unused import statements
Some functions in the contracts lack comments, or comments are incomplete, some use natspec comments and some use regular comments
Example :
function initialize( address _executorManagerAddress, address _pauser, address _trustedForwarder, address _tokenManager, address _liquidityProviders ) public initializer {
/** * @dev Function to allow LPs to remove their liquidity from an existing NFT * Also automatically redeems any earned fee */ function removeLiquidity(uint256 _nftId, uint256 _amount) external nonReentrant onlyValidLpToken(_nftId, _msgSender()) whenNotPaused {
/** * @dev Returns the maximum amount a single community LP has provided */ function getMaxCommunityLpPositon(address _token) external view returns (uint256) {
/** * @dev Function used to deposit native token into pool to initiate a cross chain token transfer. * @param receiver Address on toChainId where tokens needs to be transfered * @param toChainId Chain id where funds needs to be transfered */ function depositNative( address receiver, uint256 toChainId, string memory tag ) external payable whenNotPaused nonReentrant {
Add comments to all function
In event EthReceived address is not indexed, indexing parameters eases filtering for specific data
same event is declared in LiquidityProviders.sol
Some contracts does not follow solidity contract layout order described in
https://docs.soliditylang.org/en/develop/style-guide.html#order-of-layout
In contract TokenManager.sol modifier and event is placed between mappings, in Lptoken.sol modifier is placed after function
Reorder the contract
🌟 Selected for report: Dravee
Also found by: 0v3rf10w, 0x1f8b, 0xDjango, 0xNazgul, 0xngndev, 0xwags, Cantor_Dust, CertoraInc, IllIllI, Jujic, Kenshin, Kiep, PPrieditis, TerrierLover, Tomio, WatchPug, antonttc, benk10, berndartmueller, bitbopper, csanuragjain, defsec, gzeon, hagrid, hickuphh3, kenta, minhquanym, oyc_109, pedroais, peritoflores, rfa, robee, saian, samruna, sirhashalot, throttle, wuwe1, z3s
140.4459 USDT - $140.45
Storage variables that are read multiple times can be cached to save gas on SLOAD that costs ~100 gas (excluding first call)
tokenManager.getTokensInfo(tokenAddress).equilibriumFee can be cached and re-used
currentLiquidity[tokenAddress] can be cached and re-used
totalSharesMinted[_baseToken] can be replaced by variable supply
totalReserve[token] can be cached and re-used
perTokenTotalCap[_token] can re-used to save gas
svgHelpers[tokenAddress] can re-used
tokenMetadata[tokenId].suppliedLiquidity and ILiquidityProviders(liquidityProvidersAddress).totalReserve(tokenAddress) can re-used
The storage variable can be cached and reused
In function changeFee in TokenManager.sol function arguments can be used in the event instead of storage read to save gas.
emit FeeChanged(tokenAddress, tokensInfo[tokenAddress].equilibriumFee, tokensInfo[tokenAddress].maxFee);
can be changed to
emit FeeChanged(tokenAddress, _equilibriumFee, _maxFee);
getTokensInfo result of storage read can be returned directlyIn function getTokensInfo seperate storage reads is done for each value in the struct, the value can be returned directly
function getTokensInfo(address tokenAddress) public view override returns (TokenInfo memory) { TokenInfo memory tokenInfo = TokenInfo( tokensInfo[tokenAddress].transferOverhead, tokensInfo[tokenAddress].supportedToken, tokensInfo[tokenAddress].equilibriumFee, tokensInfo[tokenAddress].maxFee, transferConfig[tokenAddress] ); return tokenInfo; }
can be changed to
function getTokensInfo(address tokenAddress) public view override returns (TokenInfo memory) { return tokensInfo[tokenAddress]; }
_msgSender() return value can be cached to save gasreturn value of _msgSender can be cached and re-used to save gas
Using unchecked keyword, arithmetic underflow/overflow check can be avoided where underflow/overflow cannot happen which will save gas
In function getRewardAmount in LiquidityPool liquidityDifference calculation can be placed inside unchecked
In function depositErc20 reward amount subtraction since reward <= incentives
In for loops increment statement can be placed inside a unchecked block since i cannot overflow
Add unchecked block to the statements
Statements can be ordered in the order of gas consumption so that it can reduce gas consumption in the case of revert
_Pausable_init(_pauser) can be placed after the require statements to save gas if _pauser is address(0)
require( tokenManager.getDepositConfig(toChainId, tokenAddress).min <= amount && tokenManager.getDepositConfig(toChainId, tokenAddress).max >= amount, "Deposit amount not in Cap limit" ); require(receiver != address(0), "Receiver address cannot be 0"); require(amount != 0, "Amount cannot be 0");
can be changed to
require(amount != 0, "Amount cannot be 0"); require(receiver != address(0), "Receiver address cannot be 0"); require( tokenManager.getDepositConfig(toChainId, tokenAddress).min <= amount && tokenManager.getDepositConfig(toChainId, tokenAddress).max >= amount, "Deposit amount not in Cap limit" );
require( tokenManager.getDepositConfig(toChainId, NATIVE).min <= msg.value && tokenManager.getDepositConfig(toChainId, NATIVE).max >= msg.value, "Deposit amount not in Cap limit" ); require(receiver != address(0), "Receiver address cannot be 0"); require(msg.value != 0, "Amount cannot be 0");
can be changed to
require(msg.value != 0, "Amount cannot be 0"); require(receiver != address(0), "Receiver address cannot be 0"); require( tokenManager.getDepositConfig(toChainId, NATIVE).min <= msg.value && tokenManager.getDepositConfig(toChainId, NATIVE).max >= msg.value, "Deposit amount not in Cap limit" );
require( tokenManager.getTransferConfig(tokenAddress).min <= amount && tokenManager.getTransferConfig(tokenAddress).max >= amount, "Withdraw amnt not in Cap limits" ); require(receiver != address(0), "Bad receiver address");
can be changed to
require(receiver != address(0), "Bad receiver address"); require( tokenManager.getTransferConfig(tokenAddress).min <= amount && tokenManager.getTransferConfig(tokenAddress).max >= amount, "Withdraw amnt not in Cap limits" );
In initializeRewardPool _baseToken and _rewardToken checks can be done before state variable read to save gas on revert
revert strings can be <= 32 bytes so that it can be placed in a single slot of memory and require statement will cost less gas
Shorten the revert string or use error codes and explanation in docs
library function SafeERC20Upgradeable.safeTransferFrom is used directly in contract LiquidityProvider.sol
In LiquidityFarming.sol library function safeTransferFrom can be called directly
remove statement using for
amount and msg.value can be validated to save gasIn function increaseTokenLiquidity and increaseNativeLiquidity amount and msg.value can be validated for 0 value in
the primary function to prevent gas consumption if call fails in _increaseLiquidty
place require(amount > 0) in the functions
!= costs less compared to >!= costs less gas when compared to > for unsigned int in require statements
replace > with !=