Redacted Cartel contest - oyc_109's results

[L-01] Unspecific Compiler Version Pragma

Avoid floating pragmas for non-library contracts.

While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations.

A known vulnerable compiler version may accidentally be selected or security tools might fall-back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.

It is recommended to pin to a concrete compiler version.

2022-11-redactedcartel/src/vaults/PirexERC4626.sol::2 => pragma solidity >=0.8.0;

[L-02] Use of Block.timestamp

Block timestamps have historically been used for a variety of applications, such as entropy for random numbers (see the Entropy Illusion for further details), locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.

2022-11-redactedcartel/src/PirexRewards.sol::291 => (block.timestamp - u.lastUpdate);
2022-11-redactedcartel/src/PirexRewards.sol::293 => u.lastUpdate = block.timestamp.safeCastTo32();
2022-11-redactedcartel/src/PirexRewards.sol::297 => emit UserAccrue(producerToken, user, block.timestamp, balance, rewards);
2022-11-redactedcartel/src/PirexRewards.sol::314 => if (block.timestamp != lastUpdate || totalSupply != lastSupply) {
2022-11-redactedcartel/src/PirexRewards.sol::316 => (block.timestamp - lastUpdate) *
2022-11-redactedcartel/src/PirexRewards.sol::319 => globalState.lastUpdate = block.timestamp.safeCastTo32();
2022-11-redactedcartel/src/PirexRewards.sol::325 => block.timestamp,
2022-11-redactedcartel/src/vaults/PxGmxReward.sol::54 => (block.timestamp - globalState.lastUpdate) *
2022-11-redactedcartel/src/vaults/PxGmxReward.sol::57 => globalState.lastUpdate = block.timestamp.safeCastTo32();
2022-11-redactedcartel/src/vaults/PxGmxReward.sol::61 => emit GlobalAccrue(block.timestamp, totalSupply, rewards);
2022-11-redactedcartel/src/vaults/PxGmxReward.sol::77 => (block.timestamp - u.lastUpdate);
2022-11-redactedcartel/src/vaults/PxGmxReward.sol::79 => u.lastUpdate = block.timestamp.safeCastTo32();
2022-11-redactedcartel/src/vaults/PxGmxReward.sol::83 => emit UserAccrue(user, block.timestamp, balance, rewards);

[L-03] safeApprove() is deprecated

safeApprove() is deprecated in favor of safeIncreaseAllowance() and safeDecreaseAllowance(). If only setting the initial allowance to the value that means infinite, safeIncreaseAllowance() can be used instead

2022-11-redactedcartel/src/PirexGmx.sol::292 => gmx.safeApprove(address(stakedGmx), type(uint256).max);
2022-11-redactedcartel/src/PirexGmx.sol::348 => gmx.safeApprove(address(stakedGmx), 0);
2022-11-redactedcartel/src/PirexGmx.sol::353 => gmx.safeApprove(contractAddress, type(uint256).max);
2022-11-redactedcartel/src/PirexGmx.sol::507 => t.safeApprove(glpManager, tokenAmount);
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::87 => gmxBaseReward.safeApprove(address(_platform), type(uint256).max);
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::347 => stakedGlp.safeApprove(platform, amount);
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::391 => erc20Token.safeApprove(platform, tokenAmount);
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::96 => gmxBaseReward.safeApprove(address(SWAP_ROUTER), type(uint256).max);
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::97 => gmx.safeApprove(_platform, type(uint256).max);

[L-04] decimals() not part of ERC20 standard

decimals() is not part of the official ERC20 standard and might fail for tokens that do not implement it. While in practice it is very unlikely, as usually most of the tokens implement it, this should still be considered as a potential issue.

2022-11-redactedcartel/src/vaults/PirexERC4626.sol::52 => ) ERC20(_name, _symbol, _asset.decimals()) {

[L-05] require() should be used instead of assert()

require() should be used for checking error conditions on inputs and return values while assert() should be used for invariant checking

2022-11-redactedcartel/src/PirexGmx.sol::225 => assert(feeAmount + postFeeAmount == assets);

[L-06] Unsafe use of transfer()/transferFrom() with IERC20

Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether (USDT)'s transfer() and transferFrom() functions do not return booleans as the specification requires, and instead have no return value. When these sorts of tokens are cast to IERC20, their function signatures do not match and therefore the calls made, revert. Use OpenZeppelin’s SafeERC20's safeTransfer()/safeTransferFrom() instead

2022-11-redactedcartel/src/PirexGmx.sol::436 => stakedGlp.transferFrom(msg.sender, address(this), amount);
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::243 => bool status = ERC20.transfer(to, amount);
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::261 => bool status = ERC20.transferFrom(from, to, amount);

[L-07] Events not emitted for important state changes

When changing state variables events are not emitted. Emitting events allows monitoring activities with off-chain monitoring tools.

2022-11-redactedcartel/src/PirexGmx.sol::909 => function setPauseState(bool state) external onlyOwner {
2022-11-redactedcartel/src/interfaces/IVault.sol::92 => function setVaultUtils(IVaultUtils _vaultUtils) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::94 => function setError(uint256 _errorCode, string calldata _error) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::143 => function setMaxLeverage(uint256 _maxLeverage) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::145 => function setInManagerMode(bool _inManagerMode) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::147 => function setManager(address _manager, bool _isManager) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::149 => function setIsSwapEnabled(bool _isSwapEnabled) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::151 => function setIsLeverageEnabled(bool _isLeverageEnabled) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::153 => function setMaxGasPrice(uint256 _maxGasPrice) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::155 => function setUsdgAmount(address _token, uint256 _amount) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::157 => function setBufferAmount(address _token, uint256 _amount) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::159 => function setMaxGlobalShortSize(address _token, uint256 _amount) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::161 => function setInPrivateLiquidationMode(bool _inPrivateLiquidationMode)
2022-11-redactedcartel/src/interfaces/IVault.sol::164 => function setLiquidator(address _liquidator, bool _isActive) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::166 => function setFundingRate(
2022-11-redactedcartel/src/interfaces/IVault.sol::172 => function setFees(
2022-11-redactedcartel/src/interfaces/IVault.sol::184 => function setTokenConfig(
2022-11-redactedcartel/src/interfaces/IVault.sol::194 => function setPriceFeed(address _priceFeed) external;
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::12 => function setAdjustment(
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::20 => function setIsAmmEnabled(bool _isEnabled) external;
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::22 => function setIsSecondaryPriceEnabled(bool _isEnabled) external;
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::24 => function setSpreadBasisPoints(address _token, uint256 _spreadBasisPoints)
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::27 => function setSpreadThresholdBasisPoints(uint256 _spreadThresholdBasisPoints)
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::30 => function setFavorPrimaryPrice(bool _favorPrimaryPrice) external;
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::32 => function setPriceSampleSpace(uint256 _priceSampleSpace) external;
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::34 => function setMaxStrictPriceDeviation(uint256 _maxStrictPriceDeviation)
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::51 => function setTokenConfig(

[L-08] _safeMint() should be used rather than _mint() wherever possible

Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether (USDT)'s transfer() and transferFrom() functions do not return booleans as the specification requires, and instead have no return value. When these sorts of tokens are cast to IERC20, their function signatures do not match and therefore the calls made, revert. Use OpenZeppelin’s SafeERC20's safeTransfer()/safeTransferFrom() instead

2022-11-redactedcartel/src/PxERC20.sol::50 => _mint(to, amount);
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::317 => _mint(receiver, shares);
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::400 => _mint(receiver, shares);
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::73 => _mint(receiver, shares);
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::92 => _mint(receiver, shares);

[L-09] Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions

__gap is empty reserved space in storage that is recommended to be put in place in upgradeable contracts. It allows new state variables to be added in the future without compromising the storage compatibility with existing deployments

2022-11-redactedcartel/src/PirexRewards.sol::4 => import {OwnableUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol";

[L-10] Implementation contract may not be initialized

Implementation contract does not have a constructor with the initializer modifier therefore may be uninitialized. Implementation contracts should be initialized to avoid potential griefs or exploits.

2022-11-redactedcartel/src/PirexRewards.sol::4 => import {OwnableUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol";

[N-1] Use a more recent version of solidity

Use a solidity version of at least 0.8.4 to get bytes.concat() instead of abi.encodePacked(<bytes>,<bytes>) Use a solidity version of at least 0.8.12 to get string.concat() instead of abi.encodePacked(<str>,<str>) Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions

2022-11-redactedcartel/src/vaults/PirexERC4626.sol::2 => pragma solidity >=0.8.0;

[N-2] Large multiples of ten should use scientific notation

Use (e.g. 1e6) rather than decimal literals (e.g. 1000000), for better code readability

2022-11-redactedcartel/src/PirexGmx.sol::83 => // Fees (e.g. 5000 / 1000000 = 0.5%)
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::20 => uint256 public constant FEE_DENOMINATOR = 10000;
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::22 => uint256 public constant FEE_DENOMINATOR = 10000;

[N-3] Event is missing indexed fields

Each event should use three indexed fields if there are three or more fields

2022-11-redactedcartel/src/PirexFees.sol::34 => event SetFeeRecipient(FeeRecipient f, address recipient);
2022-11-redactedcartel/src/PirexFees.sol::35 => event SetTreasuryFeePercent(uint8 _treasuryFeePercent);
2022-11-redactedcartel/src/PirexGmx.sol::140 => event InitiateMigration(address newContract);
2022-11-redactedcartel/src/PirexGmx.sol::141 => event CompleteMigration(address oldContract);
2022-11-redactedcartel/src/PirexGmx.sol::142 => event SetDelegationSpace(string delegationSpace, bool shouldClear);
2022-11-redactedcartel/src/PirexGmx.sol::143 => event SetVoteDelegate(address voteDelegate);
2022-11-redactedcartel/src/PirexGmx.sol::144 => event ClearVoteDelegate();
2022-11-redactedcartel/src/PirexRewards.sol::33 => event SetProducer(address producer);
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::35 => event WithdrawalPenaltyUpdated(uint256 penalty);
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::36 => event PlatformFeeUpdated(uint256 fee);
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::37 => event CompoundIncentiveUpdated(uint256 incentive);
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::38 => event PlatformUpdated(address _platform);
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::39 => event PoolFeeUpdated(uint24 _poolFee);
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::40 => event WithdrawalPenaltyUpdated(uint256 penalty);
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::41 => event PlatformFeeUpdated(uint256 fee);
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::42 => event CompoundIncentiveUpdated(uint256 incentive);
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::43 => event PlatformUpdated(address _platform);
2022-11-redactedcartel/src/vaults/PxGmxReward.sol::21 => event GlobalAccrue(uint256 lastUpdate, uint256 lastSupply, uint256 rewards);
2022-11-redactedcartel/src/vaults/PxGmxReward.sol::28 => event Harvest(uint256 rewardAmount);

[N-4] Missing NatSpec

Code should include NatSpec

2022-11-redactedcartel/src/Common.sol::1 => // SPDX-License-Identifier: MIT
2022-11-redactedcartel/src/interfaces/IAutoPxGlp.sol::1 => // SPDX-License-Identifier: MIT
2022-11-redactedcartel/src/interfaces/IBasePositionManager.sol::1 => // SPDX-License-Identifier: MIT
2022-11-redactedcartel/src/interfaces/IGMX.sol::1 => // SPDX-License-Identifier: MIT
2022-11-redactedcartel/src/interfaces/IGlpManager.sol::1 => // SPDX-License-Identifier: MIT
2022-11-redactedcartel/src/interfaces/IPirexRewards.sol::1 => // SPDX-License-Identifier: MIT
2022-11-redactedcartel/src/interfaces/IProducer.sol::1 => // SPDX-License-Identifier: MIT
2022-11-redactedcartel/src/interfaces/IRewardDistributor.sol::1 => // SPDX-License-Identifier: MIT
2022-11-redactedcartel/src/interfaces/IRewardRouterV2.sol::1 => // SPDX-License-Identifier: MIT
2022-11-redactedcartel/src/interfaces/IStakedGlp.sol::1 => // SPDX-License-Identifier: MIT
2022-11-redactedcartel/src/interfaces/ITimelock.sol::1 => // SPDX-License-Identifier: MIT
2022-11-redactedcartel/src/interfaces/IVault.sol::1 => // SPDX-License-Identifier: MIT
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::1 => // SPDX-License-Identifier: MIT
2022-11-redactedcartel/src/interfaces/IWETH.sol::1 => // SPDX-License-Identifier: MIT

[N-5] Public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public.

2022-11-redactedcartel/src/PirexRewards.sol::85 => function initialize() public initializer {
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::217 => function maxDeposit(address) public view virtual returns (uint256) {
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::221 => function maxMint(address) public view virtual returns (uint256) {
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::225 => function maxWithdraw(address owner) public view virtual returns (uint256) {
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::229 => function maxRedeem(address owner) public view virtual returns (uint256) {

[N-6] Missing event for critical parameter change

Emitting events after sensitive changes take place, to facilitate tracking and notify off-chain clients following changes to the contract.

2022-11-redactedcartel/src/PirexGmx.sol::909 => function setPauseState(bool state) external onlyOwner {
2022-11-redactedcartel/src/interfaces/IVault.sol::92 => function setVaultUtils(IVaultUtils _vaultUtils) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::94 => function setError(uint256 _errorCode, string calldata _error) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::143 => function setMaxLeverage(uint256 _maxLeverage) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::145 => function setInManagerMode(bool _inManagerMode) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::147 => function setManager(address _manager, bool _isManager) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::149 => function setIsSwapEnabled(bool _isSwapEnabled) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::151 => function setIsLeverageEnabled(bool _isLeverageEnabled) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::153 => function setMaxGasPrice(uint256 _maxGasPrice) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::155 => function setUsdgAmount(address _token, uint256 _amount) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::157 => function setBufferAmount(address _token, uint256 _amount) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::159 => function setMaxGlobalShortSize(address _token, uint256 _amount) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::161 => function setInPrivateLiquidationMode(bool _inPrivateLiquidationMode)
2022-11-redactedcartel/src/interfaces/IVault.sol::164 => function setLiquidator(address _liquidator, bool _isActive) external;
2022-11-redactedcartel/src/interfaces/IVault.sol::166 => function setFundingRate(
2022-11-redactedcartel/src/interfaces/IVault.sol::172 => function setFees(
2022-11-redactedcartel/src/interfaces/IVault.sol::184 => function setTokenConfig(
2022-11-redactedcartel/src/interfaces/IVault.sol::194 => function setPriceFeed(address _priceFeed) external;
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::12 => function setAdjustment(
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::20 => function setIsAmmEnabled(bool _isEnabled) external;
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::22 => function setIsSecondaryPriceEnabled(bool _isEnabled) external;
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::24 => function setSpreadBasisPoints(address _token, uint256 _spreadBasisPoints)
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::27 => function setSpreadThresholdBasisPoints(uint256 _spreadThresholdBasisPoints)
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::30 => function setFavorPrimaryPrice(bool _favorPrimaryPrice) external;
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::32 => function setPriceSampleSpace(uint256 _priceSampleSpace) external;
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::34 => function setMaxStrictPriceDeviation(uint256 _maxStrictPriceDeviation)
2022-11-redactedcartel/src/interfaces/IVaultPriceFeed.sol::51 => function setTokenConfig(

[N-7] Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

instances:

2022-11-redactedcartel/src/PxERC20.sol::9 => bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
2022-11-redactedcartel/src/PxERC20.sol::10 => bytes32 public constant BURNER_ROLE = keccak256("BURNER_ROLE");

[G-01] Using private rather than public for constants, saves gas

If needed, the value can be read from the verified contract source code. Savings are due to the compiler not having to create non-payable getter functions for deployment calldata, and not adding another entry to the method ID table

2022-11-redactedcartel/src/PirexGmx.sol::50 => ERC20 public immutable gmxBaseReward; // e.g. WETH (Ethereum)
2022-11-redactedcartel/src/PirexGmx.sol::51 => ERC20 public immutable gmx;
2022-11-redactedcartel/src/PirexGmx.sol::52 => ERC20 public immutable esGmx;
2022-11-redactedcartel/src/PirexGmx.sol::55 => PxERC20 public immutable pxGmx;
2022-11-redactedcartel/src/PirexGmx.sol::56 => PxERC20 public immutable pxGlp;
2022-11-redactedcartel/src/PirexGmx.sol::62 => address public immutable pirexRewards;
2022-11-redactedcartel/src/PirexGmx.sol::65 => DelegateRegistry public immutable delegateRegistry;
2022-11-redactedcartel/src/PxERC20.sol::12 => PirexRewards public immutable pirexRewards;
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::30 => address public immutable rewardsModule;
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::33 => ERC20 public immutable gmxBaseReward;
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::34 => address public immutable rewardsModule;
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::36 => ERC20 public immutable gmxBaseReward;
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::37 => ERC20 public immutable gmx;
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::46 => ERC20 public immutable asset;

[G-02] Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost

2022-11-redactedcartel/src/PirexGmx.sol::272 => function configureGmxState() external onlyOwner whenPaused {
2022-11-redactedcartel/src/PirexGmx.sol::300 => function setFee(Fees f, uint256 fee) external onlyOwner {
2022-11-redactedcartel/src/PirexGmx.sol::884 => function setVoteDelegate(address voteDelegate) external onlyOwner {
2022-11-redactedcartel/src/PirexGmx.sol::895 => function clearVoteDelegate() public onlyOwner {
2022-11-redactedcartel/src/PirexGmx.sol::909 => function setPauseState(bool state) external onlyOwner {
2022-11-redactedcartel/src/PirexRewards.sol::93 => function setProducer(address _producer) external onlyOwner {
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::94 => function setWithdrawalPenalty(uint256 penalty) external onlyOwner {
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::106 => function setPlatformFee(uint256 fee) external onlyOwner {
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::118 => function setCompoundIncentive(uint256 incentive) external onlyOwner {
2022-11-redactedcartel/src/vaults/AutoPxGlp.sol::130 => function setPlatform(address _platform) external onlyOwner {
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::104 => function setPoolFee(uint24 _poolFee) external onlyOwner {
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::116 => function setWithdrawalPenalty(uint256 penalty) external onlyOwner {
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::128 => function setPlatformFee(uint256 fee) external onlyOwner {
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::140 => function setCompoundIncentive(uint256 incentive) external onlyOwner {
2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::152 => function setPlatform(address _platform) external onlyOwner {

[G-03] Empty blocks should be removed or emit something

The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting.

2022-11-redactedcartel/src/PxGmx.sol::12 => {}
2022-11-redactedcartel/src/PxGmx.sol::23 => {}
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::276 => ) internal virtual {}
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::282 => ) internal virtual {}
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::288 => ) internal virtual {}
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::294 => ) internal virtual {}
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::300 => ) internal virtual {}

[G-04] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

2022-11-redactedcartel/src/Common.sol::5 => uint32 lastUpdate;
2022-11-redactedcartel/src/Common.sol::11 => uint32 lastUpdate;
2022-11-redactedcartel/src/PirexFees.sol::20 => uint8 public constant FEE_PERCENT_DENOMINATOR = 100;
2022-11-redactedcartel/src/PirexFees.sol::23 => uint8 public constant MAX_TREASURY_FEE_PERCENT = 75;
2022-11-redactedcartel/src/PirexFees.sol::28 => uint8 public treasuryFeePercent = MAX_TREASURY_FEE_PERCENT;
2022-11-redactedcartel/src/interfaces/IV3SwapRouter.sol::16 => uint160 sqrtPriceLimitX96;
2022-11-redactedcartel/src/interfaces/IV3SwapRouter.sol::53 => uint160 sqrtPriceLimitX96;

[G-05] ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, for example when used in for- and while-loops

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop

2022-11-redactedcartel/src/PirexRewards.sol::163 => for (uint256 i; i < len; ++i) {
2022-11-redactedcartel/src/PirexRewards.sol::351 => for (uint256 i; i < pLen; ++i) {
2022-11-redactedcartel/src/PirexRewards.sol::396 => for (uint256 i; i < rLen; ++i) {

[G-06] Use custom errors rather than revert()/require() strings to save gas

Custom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they're hitby avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas

2022-11-redactedcartel/src/vaults/AutoPxGmx.sol::355 => require((assets = previewRedeem(shares)) != 0, "ZERO_ASSETS");
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::68 => require((shares = previewDeposit(assets)) != 0, "ZERO_SHARES");
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::137 => require((assets = previewRedeem(shares)) != 0, "ZERO_ASSETS");

[G-07] Use a more recent version of solidity

Use a solidity version of at least 0.8.0 to get overflow protection without SafeMath Use a solidity version of at least 0.8.2 to get compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value

2022-11-redactedcartel/src/vaults/PirexERC4626.sol::2 => pragma solidity >=0.8.0;

[G-08] Usage of assert() instead of require()

Between solc 0.4.10 and 0.8.0, require() used REVERT (0xfd) opcode which refunded remaining gas on failure while assert() used INVALID (0xfe) opcode which consumed all the supplied gas. require() should be used for checking error conditions on inputs and return values while assert() should be used for invariant checking (properly functioning code should never reach a failing assert statement, unless there is a bug in your contract you should fix).

2022-11-redactedcartel/src/PirexGmx.sol::225 => assert(feeAmount + postFeeAmount == assets);

[G-09] Public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public and can save gas by doing so.

2022-11-redactedcartel/src/vaults/PirexERC4626.sol::217 => function maxDeposit(address) public view virtual returns (uint256) {
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::221 => function maxMint(address) public view virtual returns (uint256) {
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::225 => function maxWithdraw(address owner) public view virtual returns (uint256) {
2022-11-redactedcartel/src/vaults/PirexERC4626.sol::229 => function maxRedeem(address owner) public view virtual returns (uint256) {

[G-10] Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.

2022-11-redactedcartel/src/PirexRewards.sol::22 => mapping(address => UserState) userStates;
2022-11-redactedcartel/src/PirexRewards.sol::24 => mapping(address => mapping(ERC20 => address)) rewardRecipients;

[G-11] Use assembly to check for address(0)

Saves 6 gas per instance if using assembly to check for address(0)

e.g.

assembly {
 if iszero(_addr) {
  mstore(0x00, "zero address")
  revert(0x00, 0x20)
 }
}

instances:

2022-11-redactedcartel/src/PirexGmx.sol::942 => if (gmxRewardRouterV2.pendingReceivers(address(this)) != address(0))
2022-11-redactedcartel/src/PirexRewards.sol::399 => address recipient = rewardRecipient != address(0)

[G-12] Using storage instead of memory for structs/arrays saves gas

When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read.

Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct

2022-11-redactedcartel/src/PirexRewards.sol::216 => UserState memory userState = producerTokens[producerToken].userStates[

[G-13] internal functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

2022-11-redactedcartel/src/vaults/PxGmxReward.sol::49 => function _globalAccrue() internal {
2022-11-redactedcartel/src/vaults/PxGmxReward.sol::68 => function _userAccrue(address user) internal {
2022-11-redactedcartel/src/vaults/PxGmxReward.sol::90 => function _harvest(uint256 rewardAmount) internal {

[G-14] internal functions not called by the contract should be removed to save deployment gas

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

2022-11-redactedcartel/src/vaults/PxGmxReward.sol::90 => function _harvest(uint256 rewardAmount) internal {