Platform: Code4rena
Start Date: 14/06/2022
Pot Size: $50,000 USDC
Total HM: 19
Participants: 99
Period: 5 days
Judge: HardlyDifficult
Total Solo HM: 4
Id: 136
League: ETH
Rank: 18/99
Findings: 6
Award: $637.55
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: hyh
Also found by: 0x29A, 0xNineDec, 0xf15ers, 0xkowloon, GreyArt, IllIllI, KIntern, Kenshin, Lambda, WatchPug, Wayne, berndartmueller, byterocket, cccz, codexploder, horsefacts, kenzo, obront, obtarian, oyc_109, peritoflores, rajatbeladiya, rfa, saian, unforgiven, zer0dot
11.084 USDC - $11.08
https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L119-L121 https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1228-L1232
The rescueETH
function does not work as expected and the ETH
balance of the contract, received from takeMultipleOneOrders
and takeOrders
(rare but also from fallback
and receive
), gets stuck on contract
With the takeMultipleOneOrders
and takeOrders
(also by mistake the fallback
and receive
) functions the contract receive ETH to pay the exchange fees, the owner withdraw these fees with the rescueETH
but this function send the msg.value
, what was sent in the transaction, to the destination
and this it's not the balance of the contract
The fallback
and receive
functions worst this scenario because available the contract to receive ETH
Manual revision
I recommend:
fallback
, receive
the contract should not receive ETHrescueETH
function:event RescueETH(address destination, uint256 amount); // @audit optional, you can add the msg.sender but always will be the owner /// @dev Used for rescuing exchange fees paid to the contract in ETH function rescueETH(address payable destination) external onlyOwner { require(destination != address(0), "The destination can't be the address 0"); // @audit optional uint256 balance = address(this).balance; (bool sent, ) = destination.call{value: balance}(''); require(sent, 'Failed to send Ether'); emit RescueETH(destination, balance); // @audit optional }
Note: A contract can use selfdestruct to send ETH to the contract, but why would someone do this?
#0 - nneverlander
2022-06-20T14:39:37Z
#1 - nneverlander
2022-07-05T11:41:56Z
#2 - HardlyDifficult
2022-07-09T16:53:16Z
276.9248 USDC - $276.92
If a user sells a NFT that doesn't respect the EIP165 (example Cryptokitties contract), after the sell occurs seller will receive the funds, but buyer will not recive the NFT.
External functions affects:
0x80ac58cd
(or interface 0xd9b67a26
), _transferNFTs
is not rolled back and the transaction continues(see L1069 goes to the else path)Manual revision
The best aproach is add a new parameter in the TokenInfo
struct(library OrderTypes) to indentifie the type of token
In OrderTypes
library:
... + enum CollectionType { + ERC721, + ERC1155 + } /** * @title OrderTypes * @author nneverlander. Twitter @nneverlander * @notice This library contains the order types used by the main exchange and complications */ library OrderTypes { /// @dev the tokenId and numTokens (=1 for ERC721 and >=1 for ERC1155) struct TokenInfo { uint256 tokenId; uint256 numTokens; + CollectionType nftType; } ...
In InfinityExchange.sol
:
... - import {OrderTypes} from '../libs/OrderTypes.sol'; + import {CollectionType, OrderTypes} from '../libs/OrderTypes.sol'; ... /** * @notice Transfer NFTs * @param from address of the sender * @param to address of the recipient * @param item item to transfer */ function _transferNFTs( address from, address to, OrderTypes.OrderItem calldata item ) internal { - if (IERC165(item.collection).supportsInterface(0x80ac58cd)) { + if (item.nftType == CollectionType.ERC721) { _transferERC721s(from, to, item); - } else if (IERC165(item.collection).supportsInterface(0xd9b67a26)) { + } else { _transferERC1155s(from, to, item); } } ...
#0 - nneverlander
2022-06-22T10:08:14Z
#1 - nneverlander
2022-07-05T11:28:26Z
#2 - HardlyDifficult
2022-07-09T19:44:24Z
🌟 Selected for report: horsefacts
Also found by: 0x29A, GimelSec, GreyArt, Lambda, Ruhum, antonttc, berndartmueller, byterocket, cccz, codexploder, dipp, oyc_109, unforgiven
https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L326 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L362
Ether send by the user cang gets locks when taking order/s
If Bob use function takeMultipleOneOrders
or takeOrders
to buy and sends more ETH that it supposes to remaing ETH will be lost, also if the seller is selling for other token and sends ETH by mistake ETH will be lost.
Manual revision
There are two things that you could do, make a exact match require for the ether send or give user remaing ETH back.
diff --git a/contracts/core/InfinityExchange.sol b/contracts/core/InfinityExchange.sol index 3639554..59c5f0f 100644 --- a/contracts/core/InfinityExchange.sol +++ b/contracts/core/InfinityExchange.sol @@ -323,7 +323,9 @@ contract InfinityExchange is ReentrancyGuard, Ownable { // check to ensure that for ETH orders, enough ETH is sent // for non ETH orders, IERC20 safeTransferFrom will throw error if insufficient amount is sent if (isMakerSeller && currency == address(0)) { - require(msg.value >= totalPrice, 'invalid total price'); + require(msg.value == totalPrice, 'invalid total price'); + } else { + require(msg.value == 0, 'invalid total price'); } } @@ -359,7 +361,9 @@ contract InfinityExchange is ReentrancyGuard, Ownable { // check to ensure that for ETH orders, enough ETH is sent // for non ETH orders, IERC20 safeTransferFrom will throw error if insufficient amount is sent if (isMakerSeller && currency == address(0)) { - require(msg.value >= totalPrice, 'invalid total price'); + require(msg.value == totalPrice, 'invalid total price'); + } else { + require(msg.value == 0, 'invalid total price'); } }
Another alternative could be sending back to the user the reimaing ETH
diff --git a/contracts/core/InfinityExchange.sol b/contracts/core/InfinityExchange.sol index 3639554..94d5a30 100644 --- a/contracts/core/InfinityExchange.sol +++ b/contracts/core/InfinityExchange.sol @@ -324,6 +324,15 @@ contract InfinityExchange is ReentrancyGuard, Ownable { // for non ETH orders, IERC20 safeTransferFrom will throw error if insufficient amount is sent if (isMakerSeller && currency == address(0)) { require(msg.value >= totalPrice, 'invalid total price'); + if (msg.value > totalPrice) { + // transfer remaining amount to buyer + (bool sent, ) = msg.sender.call{value: msg.value - totalPrice}(''); + require(sent, 'failed to send ether to seller'); + } + } else if (msg.value > 0 ) { + // transfer remaining amount to buyer + (bool sent, ) = msg.sender.call{value: msg.value}(''); + require(sent, 'failed to send ether to seller'); } } @@ -360,6 +369,15 @@ contract InfinityExchange is ReentrancyGuard, Ownable { // for non ETH orders, IERC20 safeTransferFrom will throw error if insufficient amount is sent if (isMakerSeller && currency == address(0)) { require(msg.value >= totalPrice, 'invalid total price'); + if (msg.value > totalPrice) { + // transfer remaining amount to buyer + (bool sent, ) = msg.sender.call{value: msg.value - totalPrice}(''); + require(sent, 'failed to send ether to seller'); + } + } else if (msg.value > 0 ) { + // transfer remaining amount to buyer + (bool sent, ) = msg.sender.call{value: msg.value}(''); + require(sent, 'failed to send ether to seller'); } }
#0 - KenzoAgada
2022-06-21T12:19:45Z
Duplicate of #244
#1 - HardlyDifficult
2022-07-10T12:41:01Z
🌟 Selected for report: shenwilly
Also found by: 0x29A, BowTiedWardens, VAD37, berndartmueller, peritoflores
136.753 USDC - $136.75
The protocol can steal WETH
founds with the refunds gas cost mechanism in the functions matchOneToOneOrders
, matchOneToManyOrders
and matchOrders
This functions can call only by the MATCH_EXECUTOR
but we don't know what is this contract/address according the sponsor in discord this code is not public
MATCH_EXECUTOR
send matchOneToManyOrders
, with the Alice signature, it send the transaction with a high amount of gasgasCost
will be highgasCost
from Alice to the exchangeThe best approach its remove the refunds gas cost mechanism, and use other mechanism for charge fees in a easy way, like a fixed amount of X token
Other option it's add another mechanism that gives to the signer of the order the max amount of fee in WETH who is willing to pay
In OrderTypes
library:
... struct MakerOrder { ///@dev is order sell or buy bool isSellOrder; ///@dev signer of the order (maker address) address signer; + ///@dev Maximum WETH to pay gas cost mechanism + uint256 maxWETHGasCost; ///@dev Constraints array contains the order constraints. Total constraints: 6. In order: // numItems - min (for buy orders) / max (for sell orders) number of items in the order // start price in wei // end price in wei // start time in block.timestamp // end time in block.timestamp // nonce of the order uint256[] constraints; ///@dev nfts array contains order items where each item is a collection and its tokenIds OrderItem[] nfts; ///@dev address of complication for trade execution (e.g. InfinityOrderBookComplication), address of the currency (e.g., WETH) address[] execParams; ///@dev additional parameters like traits for trait orders, private sale buyer for OTC ordes etc bytes extraParams; ///@dev the order signature uint8 v: parameter (27 or 28), bytes32 r, bytes32 s bytes sig; } ...
In InfinityExchange.sol
add this lines:
In L231: uint256 gasCost = (startGas - gasleft() + WETH_TRANSFER_GAS_UNITS) * tx.gasprice; + require(gasCost <= makerOrder.maxWETHGasCost, "The gasCost it's to high"); In L739: uint256 gasCost = (startGasPerOrder - gasleft() + wethTransferGasUnits) * tx.gasprice; + require(gasCost <= buy.maxWETHGasCost, "The gasCost it's to high"); In L787: uint256 gasCost = (startGasPerOrder - gasleft() + wethTransferGasUnits) * tx.gasprice; + require(gasCost <= buy.maxWETHGasCost, "The gasCost it's to high"); In L893: uint256 gasCost = (startGasPerOrder - gasleft() + wethTransferGasUnits) * tx.gasprice; + require(gasCost <= buy.maxWETHGasCost, "The gasCost it's to high");
#0 - KenzoAgada
2022-06-21T12:07:23Z
uint256 gasCost = (startGas - gasleft() + WETH_TRANSFER_GAS_UNITS) * tx.gasprice;
As far as I see, this will only transfer from Alice the actual gas cost of the transaction. Not an arbitrary amount. So the issue seems invalid.
#1 - nneverlander
2022-06-22T16:34:54Z
Duplicate
#2 - HardlyDifficult
2022-07-11T22:49:35Z
🌟 Selected for report: joestakey
Also found by: 0x1f8b, 0x29A, 0x52, 0xDjango, 0xNazgul, 0xNineDec, 0xf15ers, 0xkowloon, 0xmint, 8olidity, BowTiedWardens, Chom, Cityscape, Czar102, ElKu, FSchmoede, Funen, GimelSec, GreyArt, IllIllI, KIntern, Kaiziron, Kenshin, Lambda, MadWookie, MiloTruck, PPrieditis, Picodes, Ruhum, Sm4rty, StErMi, TerrierLover, TomJ, Treasure-Seeker, VAD37, WatchPug, Wayne, _Adam, a12jmx, abhinavmir, antonttc, apostle0x01, asutorufos, berndartmueller, cccz, cloudjunky, codexploder, cryptphi, csanuragjain, defsec, delfin454000, fatherOfBlocks, georgypetrov, hake, hansfriese, horsefacts, hyh, k, kenta, nxrblsrpr, oyc_109, peritoflores, rajatbeladiya, reassor, rfa, robee, sach1r0, saian, samruna, shenwilly, simon135, sorrynotsorry, sseefried, throttle, unforgiven, wagmi, zzzitron
96.761 USDC - $96.76
In some code blocks you use a lot of uncheckeds scopes, a cleanner pattern would be to use a wider unchecked scope.
InfinityOrderBookComplication.sol
diff --git a/contracts/core/InfinityOrderBookComplication.sol b/contracts/core/InfinityOrderBookComplication.sol index 17b2b45..dbf4997 100644 --- a/contracts/core/InfinityOrderBookComplication.sol +++ b/contracts/core/InfinityOrderBookComplication.sol @@ -242,25 +242,21 @@ contract InfinityOrderBookComplication is IComplication, Ownable { } uint256 numCollsMatched = 0; - // check if taker has all items in maker - for (uint256 i = 0; i < order2NftsLength; ) { - for (uint256 j = 0; j < order1NftsLength; ) { - if (order1Nfts[j].collection == order2Nfts[i].collection) { - // increment numCollsMatched - unchecked { + unchecked { + // check if taker has all items in maker + for (uint256 i = 0; i < order2NftsLength; ) { + for (uint256 j = 0; j < order1NftsLength; ) { + if (order1Nfts[j].collection == order2Nfts[i].collection) { + // increment numCollsMatched ++numCollsMatched; + // check if tokenIds intersect + bool tokenIdsIntersect = doTokenIdsIntersect(order1Nfts[j], order2Nfts[i]); + require(tokenIdsIntersect, 'tokenIds dont intersect'); + // short circuit + break; } - // check if tokenIds intersect - bool tokenIdsIntersect = doTokenIdsIntersect(order1Nfts[j], order2Nfts[i]); - require(tokenIdsIntersect, 'tokenIds dont intersect'); - // short circuit - break; - } - unchecked { ++j; } - } - unchecked { ++i; } } @@ -287,23 +283,19 @@ contract InfinityOrderBookComplication is IComplication, Ownable { return true; } uint256 numTokenIdsPerCollMatched = 0; - for (uint256 k = 0; k < item2TokensLength; ) { - for (uint256 l = 0; l < item1TokensLength; ) { - if ( - item1.tokens[l].tokenId == item2.tokens[k].tokenId && item1.tokens[l].numTokens == item2.tokens[k].numTokens - ) { - // increment numTokenIdsPerCollMatched - unchecked { + unchecked { + for (uint256 k = 0; k < item2TokensLength; ) { + for (uint256 l = 0; l < item1TokensLength; ) { + if ( + item1.tokens[l].tokenId == item2.tokens[k].tokenId && item1.tokens[l].numTokens == item2.tokens[k].numTokens + ) { + // increment numTokenIdsPerCollMatched ++numTokenIdsPerCollMatched; + // short circuit + break; } - // short circuit - break; - } - unchecked { ++l; } - } - unchecked { ++k; } }
On InfinityExchange.sol#L1128
the signature of the function is function _transferFees(address seller, address buyer, uint256 amount, address currency)
I recommend you to change it to a more common pattern;
function _transferFees(address seller, address buyer, address currency, uint256 amount)
On OrderTypes.sol#L46-L48 the struct TakerOrder
is never used
on InfinityExchange.sol#L859
the comment is;
* @param weth weth address
And should be
* @param weth WETH address
In IStaker.sol, the import {OrderTypes} from '../libs/OrderTypes.sol';
is unused
Out of scope but InfinityToken.sol inherits from IStaker.sol
Import Ownable on InfinityOrderBookComplication.sol
InfinityOrderBookComplication
contract is importing Ownable
but no one is using it. My recommendation is to remove it.
diff --git a/contracts/core/InfinityOrderBookComplication.sol b/contracts/core/InfinityOrderBookComplication.sol index 17b2b45..6e4cf77 100644 --- a/contracts/core/InfinityOrderBookComplication.sol +++ b/contracts/core/InfinityOrderBookComplication.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.14; -import {Ownable} from '@openzeppelin/contracts/access/Ownable.sol'; import {OrderTypes} from '../libs/OrderTypes.sol'; import {IComplication} from '../interfaces/IComplication.sol'; @@ -11,7 +10,7 @@ import {IComplication} from '../interfaces/IComplication.sol'; * @author nneverlander. Twitter @nneverlander * @notice Complication to execute orderbook orders */ -contract InfinityOrderBookComplication is IComplication, Ownable { +contract InfinityOrderBookComplication is IComplication { // ======================================================= EXTERNAL FUNCTIONS ================================================== /**
IERC20
In InfinityStaker.sol:L25, declare INFINITY_TOKEN
as IERC20
would avoid it cast to IERC20
when use it and improve code clarity:
- address public INFINITY_TOKEN; + IERC20 public INFINITY_TOKEN;
Cast in the constructor, or declare _tokenAddress
as IERC20
constructor(address _tokenAddress, address _infinityTreasury) { - INFINITY_TOKEN = _tokenAddress; + INFINITY_TOKEN = IERC20(_tokenAddress); INFINITY_TREASURY = _infinityTreasury; }
Remove cast on:
L69: - require(IERC20(INFINITY_TOKEN).balanceOf(msg.sender) >= amount, 'insufficient balance to stake'); + require(INFINITY_TOKEN.balanceOf(msg.sender) >= amount, 'insufficient balance to stake'); L74: - IERC20(INFINITY_TOKEN).safeTransferFrom(msg.sender, address(this), amount); + INFINITY_TOKEN.safeTransferFrom(msg.sender, address(this), amount); L128: - IERC20(INFINITY_TOKEN).safeTransfer(msg.sender, amount); + INFINITY_TOKEN.safeTransfer(msg.sender, amount); L141: - IERC20(INFINITY_TOKEN).safeTransfer(msg.sender, totalToUser); + INFINITY_TOKEN.safeTransfer(msg.sender, totalToUser); L142: - IERC20(INFINITY_TOKEN).safeTransfer(INFINITY_TREASURY, penalty); + INFINITY_TOKEN.safeTransfer(INFINITY_TREASURY, penalty);
In TimelockConfig.sol, L28, L29, L31, L32: By default, the visibility of variables in solidity is internal
, but it is good practice to declare it.
Out of scope but InfinityToken.sol inherits from TimelockConfig.sol
In InfinityExchange.sol:
* @param weth weth address
to * @param weth WETH address
updateWethTranferGas
to updateWethTransferGas
The emitted events are not indexed, making off-chain scripts such as front-ends of dApps difficult to filter the events efficiently.
Recommended Mitigation Steps: Add the indexed
keyword in filter parameters of the events
Instances include:
InfinityExchange.sol:L80-L102: - event CancelAllOrders(address user, uint256 newMinNonce); + event CancelAllOrders(address indexed user, uint256 newMinNonce); - event CancelMultipleOrders(address user, uint256[] orderNonces); + event CancelMultipleOrders(address indexed user, uint256[] orderNonces); event NewWethTransferGasUnits(uint32 wethTransferGasUnits); event NewProtocolFee(uint16 protocolFee); event MatchOrderFulfilled( bytes32 sellOrderHash, bytes32 buyOrderHash, - address seller, + address indexed seller, - address buyer, + address indexed buyer, address complication, // address of the complication that defines the execution - address currency, // token address of the transacting currency + address indexed currency, // token address of the transacting currency uint256 amount // amount spent on the order ); event TakeOrderFulfilled( bytes32 orderHash, - address seller, + address indexed seller, - address buyer, + address indexed buyer, address complication, // address of the complication that defines the execution - address currency, // token address of the transacting currency + address indexed currency, // token address of the transacting currency uint256 amount // amount spent on the order );
InfinityToken.sol:L35: - event EpochAdvanced(uint256 currentEpoch, uint256 supplyMinted); + event EpochAdvanced(uint256 indexed currentEpoch, uint256 supplyMinted);
TimelockConfig.sol:L34-L36: - event ChangeRequested(bytes32 configId, uint256 value); + event ChangeRequested(bytes32 indexed configId, uint256 value); - event ChangeConfirmed(bytes32 configId, uint256 value); + event ChangeConfirmed(bytes32 indexed configId, uint256 value); - event ChangeCanceled(bytes32 configId, uint256 value); + event ChangeCanceled(bytes32 indexed configId, uint256 value);
Out of scope but InfinityToken.sol inherits from TimelockConfig.sol
require
NEVER REVERTIn InfinityStaker.sol, L193: This require
never reverts the transaction, any number of uint256
data type it's greater or equal to 0, by definition
Remove the equal in therequire
comparation
- require(totalStaked >= 0, 'nothing staked to rage quit'); + require(totalStaked > 0, 'nothing staked to rage quit');
This also save gas, if the user dont have staked balance
Unnecesary multiplication by one and unnecesary exponetiation.
InfinityStaker.sol
diff --git a/contracts/staking/InfinityStaker.sol b/contracts/staking/InfinityStaker.sol index 9b36cb5..872a254 100644 --- a/contracts/staking/InfinityStaker.sol +++ b/contracts/staking/InfinityStaker.sol @@ -231,10 +231,10 @@ contract InfinityStaker is IStaker, Ownable, Pausable, ReentrancyGuard { */ function getUserStakePower(address user) public view override returns (uint256) { return - ((userstakedAmounts[user][Duration.NONE].amount * 1) + + ((userstakedAmounts[user][Duration.NONE].amount) + (userstakedAmounts[user][Duration.THREE_MONTHS].amount * 2) + (userstakedAmounts[user][Duration.SIX_MONTHS].amount * 3) + - (userstakedAmounts[user][Duration.TWELVE_MONTHS].amount * 4)) / (10**18); + (userstakedAmounts[user][Duration.TWELVE_MONTHS].amount * 4)) / 1e18; } /**
diff --git a/contracts/core/InfinityExchange.sol b/contracts/core/InfinityExchange.sol index 3639554..d58b965 100644 --- a/contracts/core/InfinityExchange.sol +++ b/contracts/core/InfinityExchange.sol @@ -1158,7 +1158,7 @@ contract InfinityExchange is ReentrancyGuard, Ownable { return startPrice; } uint256 elapsedTime = block.timestamp - order.constraints[3]; - uint256 PRECISION = 10**4; // precision for division; similar to bps + uint256 PRECISION = 1e4; // precision for division; similar to bps uint256 portionBps = elapsedTime > duration ? PRECISION : ((elapsedTime * PRECISION) / duration); priceDiff = (priceDiff * portionBps) / PRECISION; return startPrice > endPrice ? startPrice - priceDiff : startPrice + priceDiff;
InfinityOrderBookComplication.sol
diff --git a/contracts/core/InfinityOrderBookComplication.sol b/contracts/core/InfinityOrderBookComplication.sol index 17b2b45..f1975bf 100644 --- a/contracts/core/InfinityOrderBookComplication.sol +++ b/contracts/core/InfinityOrderBookComplication.sol @@ -335,7 +335,7 @@ contract InfinityOrderBookComplication is IComplication, Ownable { return startPrice; } uint256 elapsedTime = block.timestamp - order.constraints[3]; - uint256 PRECISION = 10**4; // precision for division; similar to bps + uint256 PRECISION = 1e4; // precision for division; similar to bps uint256 portionBps = elapsedTime > duration ? PRECISION : ((elapsedTime * PRECISION) / duration); priceDiff = (priceDiff * portionBps) / PRECISION; return startPrice > endPrice ? startPrice - priceDiff : startPrice + priceDiff;
InfinityStaker.sol
InfinityStaker.sol#L351
function updateStakeLevelThreshold
should emit event
InfinityStaker.sol#L364
function updatePenalties
should emit event
InfinityStaker.sol#L375
function updateInfinityTreasury
should emit event
InfinityExchange.sol
Critical admin functions rescueTokens
, rescueETH
, addCurrency
, addComplication
, removeCurrency
, removeComplication
and updateMatchExecutor
should emit events
You are currently using solidity 0.8.14, best recommendation is to switch to 0.8.15 two major bugs introduced on previus versions were fixes, plus its more gas efficient.
#0 - nneverlander
2022-06-22T16:55:50Z
Thanks
#1 - HardlyDifficult
2022-07-11T21:27:33Z
🌟 Selected for report: IllIllI
Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xAsm0d3us, 0xDjango, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, 0xkowloon, BowTiedWardens, Chom, ElKu, FSchmoede, Funen, GimelSec, Kaiziron, Kenshin, Lambda, MadWookie, MiloTruck, PPrieditis, Picodes, PwnedNoMore, StErMi, Tadashi, TerrierLover, TomJ, Tomio, Wayne, Waze, _Adam, antonttc, apostle0x01, asutorufos, c3phas, codexploder, defsec, delfin454000, fatherOfBlocks, hake, hansfriese, hyh, joestakey, k, kenta, oyc_109, peritoflores, reassor, rfa, robee, sach1r0, simon135, slywaters, zer0dot
31.9443 USDC - $31.94
immutable
In InfinityStaker.sol:L25, marking INFINITY_TOKEN
as immutable (as it never change outside the constructor) would avoid it taking space in the storage:
- address public INFINITY_TOKEN; + address public immutable INFINITY_TOKEN;
Since the conditions are order from lower to high you dont have to double check the lower bounds.
diff --git a/contracts/staking/InfinityStaker.sol b/contracts/staking/InfinityStaker.sol index 9b36cb5..1533ddb 100644 --- a/contracts/staking/InfinityStaker.sol +++ b/contracts/staking/InfinityStaker.sol @@ -212,11 +212,11 @@ contract InfinityStaker is IStaker, Ownable, Pausable, ReentrancyGuard { if (totalPower <= BRONZE_STAKE_THRESHOLD) { return StakeLevel.NONE; - } else if (totalPower > BRONZE_STAKE_THRESHOLD && totalPower <= SILVER_STAKE_THRESHOLD) { + } else if (totalPower <= SILVER_STAKE_THRESHOLD) { return StakeLevel.BRONZE; - } else if (totalPower > SILVER_STAKE_THRESHOLD && totalPower <= GOLD_STAKE_THRESHOLD) { + } else if (totalPower <= GOLD_STAKE_THRESHOLD) { return StakeLevel.SILVER; - } else if (totalPower > GOLD_STAKE_THRESHOLD && totalPower <= PLATINUM_STAKE_THRESHOLD) { + } else if (totalPower <= PLATINUM_STAKE_THRESHOLD) { return StakeLevel.GOLD; } else { return StakeLevel.PLATINUM;
You could save 5 gas if you avoid caching array length of calldata variables on:
InfinityExchange.sol
InfinityExchange.sol#L137 InfinityExchange.sol#L191 InfinityExchange.sol#L262 InfinityExchange.sol#L301 InfinityExchange.sol#L341 InfinityExchange.sol#L391 InfinityExchange.sol#L1047 InfinityExchange.sol#L1085 InfinityExchange.sol#L1106 InfinityExchange.sol#L1188 InfinityExchange.sol#L1204
InfinityOrderBookComplication.sol
InfinityOrderBookComplication.sol#L75 InfinityOrderBookComplication.sol#L81 InfinityOrderBookComplication.sol#L198 InfinityOrderBookComplication.sol#L215 InfinityOrderBookComplication.sol#L237-L238 InfinityOrderBookComplication.sol#L283-L284 InfinityOrderBookComplication.sol#L319
on InfinityStaker.sol#L69 there is no need to check if contract has enough balance, because safeTransferFrom
will fail if contract doesnt have enough balance.