4naly3er-2022-11-redactedcartel.md

Report

Gas Optimizations

	Issue	Instances
[GAS-1]	Use assembly to check for address(0)	68
[GAS-2]	State variables should be cached in stack variables rather than re-reading them from storage	9
[GAS-3]	Use calldata instead of memory for function arguments that do not get mutated	7
[GAS-4]	Use Custom Errors	3
[GAS-5]	Using private rather than public for constants, saves gas	16
[GAS-6]	Use != 0 instead of > 0 for unsigned integer comparison	1
[GAS-7]	internal functions not called by the contract should be removed	1

[GAS-1] Use assembly to check for address(0)

Saves 6 gas per instance

Instances (68):

File: src/PirexFees.sol

51:         if (_treasury == address(0)) revert ZeroAddress();

52:         if (_contributors == address(0)) revert ZeroAddress();

67:         if (recipient == address(0)) revert ZeroAddress();

Link to code

File: src/PirexGmx.sol

182:         if (_pxGmx == address(0)) revert ZeroAddress();

183:         if (_pxGlp == address(0)) revert ZeroAddress();

184:         if (_pirexFees == address(0)) revert ZeroAddress();

185:         if (_pirexRewards == address(0)) revert ZeroAddress();

186:         if (_delegateRegistry == address(0)) revert ZeroAddress();

187:         if (_gmxBaseReward == address(0)) revert ZeroAddress();

188:         if (_gmx == address(0)) revert ZeroAddress();

189:         if (_esGmx == address(0)) revert ZeroAddress();

190:         if (_gmxRewardRouterV2 == address(0)) revert ZeroAddress();

191:         if (_stakedGlp == address(0)) revert ZeroAddress();

317:         if (contractAddress == address(0)) revert ZeroAddress();

389:         if (receiver == address(0)) revert ZeroAddress();

433:         if (receiver == address(0)) revert ZeroAddress();

495:         if (receiver == address(0)) revert ZeroAddress();

497:         if (token == address(0)) {

599:         if (token == address(0)) revert ZeroAddress();

630:         if (receiver == address(0)) revert ZeroAddress();

651:         redeemed = token == address(0)

727:         if (token == address(0)) revert ZeroAddress();

829:         if (token == address(0)) revert ZeroAddress();

831:         if (receiver == address(0)) revert ZeroAddress();

885:         if (voteDelegate == address(0)) revert ZeroAddress();

926:         if (newContract == address(0)) revert ZeroAddress();

942:         if (gmxRewardRouterV2.pendingReceivers(address(this)) != address(0))

961:         if (oldContract == address(0)) revert ZeroAddress();

Link to code

File: src/PirexRewards.sol

94:         if (_producer == address(0)) revert ZeroAddress();

112:         if (address(producerToken) == address(0)) revert ZeroAddress();

113:         if (address(rewardToken) == address(0)) revert ZeroAddress();

114:         if (recipient == address(0)) revert ZeroAddress();

136:         if (address(producerToken) == address(0)) revert ZeroAddress();

137:         if (address(rewardToken) == address(0)) revert ZeroAddress();

155:         if (address(producerToken) == address(0)) revert ZeroAddress();

156:         if (address(rewardToken) == address(0)) revert ZeroAddress();

183:         if (address(producerToken) == address(0)) revert ZeroAddress();

271:         if (address(producerToken) == address(0)) revert ZeroAddress();

282:         if (address(producerToken) == address(0)) revert ZeroAddress();

283:         if (user == address(0)) revert ZeroAddress();

374:         if (address(producerToken) == address(0)) revert ZeroAddress();

375:         if (user == address(0)) revert ZeroAddress();

399:                 address recipient = rewardRecipient != address(0)

439:         if (address(producerToken) == address(0)) revert ZeroAddress();

440:         if (address(rewardToken) == address(0)) revert ZeroAddress();

441:         if (recipient == address(0)) revert ZeroAddress();

467:         if (address(producerToken) == address(0)) revert ZeroAddress();

468:         if (address(rewardToken) == address(0)) revert ZeroAddress();

Link to code

File: src/PxERC20.sol

30:         if (_pirexRewards == address(0)) revert ZeroAddress();

Link to code

File: src/vaults/AutoPxGlp.sol

75:         if (_gmxBaseReward == address(0)) revert ZeroAddress();

76:         if (_asset == address(0)) revert ZeroAddress();

79:         if (_platform == address(0)) revert ZeroAddress();

80:         if (_rewardsModule == address(0)) revert ZeroAddress();

131:         if (_platform == address(0)) revert ZeroAddress();

336:         if (receiver == address(0)) revert ZeroAddress();

374:         if (token == address(0)) revert ZeroAddress();

378:         if (receiver == address(0)) revert ZeroAddress();

421:         if (receiver == address(0)) revert ZeroAddress();

Link to code

File: src/vaults/AutoPxGmx.sol

82:         if (_gmxBaseReward == address(0)) revert ZeroAddress();

83:         if (_gmx == address(0)) revert ZeroAddress();

84:         if (_asset == address(0)) revert ZeroAddress();

87:         if (_platform == address(0)) revert ZeroAddress();

88:         if (_rewardsModule == address(0)) revert ZeroAddress();

153:         if (_platform == address(0)) revert ZeroAddress();

376:         if (receiver == address(0)) revert ZeroAddress();

Link to code

File: src/vaults/PxGmxReward.sol

41:         if (_pxGmx == address(0)) revert ZeroAddress();

69:         if (user == address(0)) revert ZeroAddress();

106:         if (receiver == address(0)) revert ZeroAddress();

Link to code

[GAS-2] State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second+ access of a state variable within a function. Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.

Saves 100 gas per instance

Instances (9):

File: src/PirexGmx.sol

754:         producerTokens[2] = pxGmx;

755:         producerTokens[3] = pxGlp;

757:         rewardTokens[1] = gmxBaseReward;

789:             address(esGmx)

847:                 gmxBaseReward.safeTransfer(address(pirexFees), feeAmount);

Link to code

File: src/vaults/AutoPxGlp.sol

233:         PirexRewards(rewardsModule).claim(pxGmx, address(this));

268:             totalPxGmxFee = (pxGmxAmountOut * platformFee) / FEE_DENOMINATOR;

347:         stakedGlp.safeApprove(platform, amount);

394:         (, uint256 assets, ) = PirexGmx(platform).depositGlp(

Link to code

[GAS-3] Use calldata instead of memory for function arguments that do not get mutated

Mark data types as calldata instead of memory where possible. This makes it so that the data is not automatically loaded into memory. If the data passed into the function does not need to be changed (like updating values in an array), it can be passed in as calldata. The one exception to this is if the argument must later be passed into another function that takes an argument that specifies memory storage.

Instances (7):

File: src/PirexGmx.sol

863:         string memory _delegationSpace,

Link to code

File: src/PxERC20.sol

26:         string memory _name,

27:         string memory _symbol,

Link to code

File: src/vaults/AutoPxGlp.sol

70:         string memory _name,

71:         string memory _symbol,

Link to code

File: src/vaults/AutoPxGmx.sol

77:         string memory _name,

78:         string memory _symbol,

Link to code

[GAS-4] Use Custom Errors

Source Instead of using error strings, to reduce deployment and runtime cost, you should use Custom Errors. This would save both deployment and runtime cost.

Instances (3):

File: src/vaults/AutoPxGmx.sol

355:         require((assets = previewRedeem(shares)) != 0, "ZERO_ASSETS");

Link to code

File: src/vaults/PirexERC4626.sol

68:         require((shares = previewDeposit(assets)) != 0, "ZERO_SHARES");

137:         require((assets = previewRedeem(shares)) != 0, "ZERO_ASSETS");

Link to code

[GAS-5] Using private rather than public for constants, saves gas

If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table

Instances (16):

File: src/PirexFees.sol

20:     uint8 public constant FEE_PERCENT_DENOMINATOR = 100;

23:     uint8 public constant MAX_TREASURY_FEE_PERCENT = 75;

Link to code

File: src/PirexGmx.sol

44:     uint256 public constant FEE_DENOMINATOR = 1_000_000;

47:     uint256 public constant FEE_MAX = 200_000;

Link to code

File: src/PxERC20.sol

9:     bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");

10:     bytes32 public constant BURNER_ROLE = keccak256("BURNER_ROLE");

Link to code

File: src/vaults/AutoPxGlp.sol

18:     uint256 public constant MAX_WITHDRAWAL_PENALTY = 500;

19:     uint256 public constant MAX_PLATFORM_FEE = 2000;

20:     uint256 public constant FEE_DENOMINATOR = 10000;

21:     uint256 public constant MAX_COMPOUND_INCENTIVE = 5000;

22:     uint256 public constant EXPANDED_DECIMALS = 1e30;

Link to code

File: src/vaults/AutoPxGmx.sol

18:     IV3SwapRouter public constant SWAP_ROUTER =

20:     uint256 public constant MAX_WITHDRAWAL_PENALTY = 500;

21:     uint256 public constant MAX_PLATFORM_FEE = 2000;

22:     uint256 public constant FEE_DENOMINATOR = 10000;

23:     uint256 public constant MAX_COMPOUND_INCENTIVE = 5000;

Link to code

[GAS-6] Use != 0 instead of > 0 for unsigned integer comparison

Instances (1):

File: src/vaults/PirexERC4626.sol

2: pragma solidity >=0.8.0;

Link to code

[GAS-7] internal functions not called by the contract should be removed

If the functions are required by an interface, the contract should inherit from that interface and use the override keyword

Instances (1):

File: src/vaults/PxGmxReward.sol

90:     function _harvest(uint256 rewardAmount) internal {

Link to code

Non Critical Issues

	Issue	Instances
[NC-1]	Functions not used internally could be marked external	11

[NC-1] Functions not used internally could be marked external

Instances (11):

File: src/PirexRewards.sol

85:     function initialize() public initializer {

Link to code

File: src/PxERC20.sol

80:     function transfer(address to, uint256 amount)

109:     function transferFrom(

Link to code

File: src/vaults/AutoPxGlp.sol

151:     function previewRedeem(uint256 shares)

177:     function previewWithdraw(uint256 assets)

436:     function withdraw(

449:     function redeem(

Link to code

File: src/vaults/AutoPxGmx.sol

315:     function withdraw(

339:     function redeem(

Link to code

File: src/vaults/PirexERC4626.sol

238:     function transfer(address to, uint256 amount)

256:     function transferFrom(

Link to code

Low Issues

	Issue	Instances
[L-1]	Do not use deprecated library functions	10
[L-2]	Unsafe ERC20 operation(s)	3
[L-3]	Unspecific compiler version pragma	1

[L-1] Do not use deprecated library functions

Instances (10):

File: src/PirexGmx.sol

292:         gmx.safeApprove(address(stakedGmx), type(uint256).max);

348:             gmx.safeApprove(address(stakedGmx), 0);

353:             gmx.safeApprove(contractAddress, type(uint256).max);

507:             t.safeApprove(glpManager, tokenAmount);

Link to code

File: src/PxERC20.sol

37:         _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);

Link to code

File: src/vaults/AutoPxGlp.sol

87:         gmxBaseReward.safeApprove(address(_platform), type(uint256).max);

347:         stakedGlp.safeApprove(platform, amount);

391:             erc20Token.safeApprove(platform, tokenAmount);

Link to code

File: src/vaults/AutoPxGmx.sol

96:         gmxBaseReward.safeApprove(address(SWAP_ROUTER), type(uint256).max);

97:         gmx.safeApprove(_platform, type(uint256).max);

Link to code

[L-2] Unsafe ERC20 operation(s)

Instances (3):

File: src/PirexGmx.sol

436:         stakedGlp.transferFrom(msg.sender, address(this), amount);

Link to code

File: src/vaults/PirexERC4626.sol

243:         bool status = ERC20.transfer(to, amount);

261:         bool status = ERC20.transferFrom(from, to, amount);

Link to code

[L-3] Unspecific compiler version pragma

Instances (1):

File: src/vaults/PirexERC4626.sol

2: pragma solidity >=0.8.0;

Link to code