Redacted Cartel contest - 0xAgro's results

QA Report

Finding Summary

[NC-01] Copied Solmate Code Missing NatSpec Function Headers

Although the modifications of the solmate ERC4626.sol contract are noted at the top of PirexERC4626.sol there are no NatSpec comments per function. Function documentation helps readability and understanding as a developer reads the code top to bottom.

Findings:

/src/vaults/PirexERC4626.sol

All PirexERC4626.sol functions excluding transfer and transferFrom lack NatSpec headers.

[NC-02] Events Not Indexed

It is best practice to have 3 indexed fields per event. Indexed fields help off-chain tools analyze on-chain activity through filtering.

Findings:

/src/vaults/AutoPxGmx.sol Links: 39, 40, 41, 42, 43.

39:	event PoolFeeUpdated(uint24 _poolFee);
40:	event WithdrawalPenaltyUpdated(uint256 penalty);
41:	event PlatformFeeUpdated(uint256 fee);
42:	event CompoundIncentiveUpdated(uint256 incentive);
43:	event PlatformUpdated(address _platform);

/src/PirexFees.sol Links: 34, 35.

34:	event SetFeeRecipient(FeeRecipient f, address recipient);
35:	event SetTreasuryFeePercent(uint8 _treasuryFeePercent);

/src/vaults/PxGmxReward.sol Links: 21, 28.

21:	event GlobalAccrue(uint256 lastUpdate, uint256 lastSupply, uint256 rewards);
28:	event Harvest(uint256 rewardAmount);

/src/vaults/AutoPxGlp.sol Links: 35, 36, 37, 38.

35:	event WithdrawalPenaltyUpdated(uint256 penalty);
36:	event PlatformFeeUpdated(uint256 fee);
37:	event CompoundIncentiveUpdated(uint256 incentive);
38:	event PlatformUpdated(address _platform);

/src/PirexGmx.sol Links: 125, 141, 142, 143, 144.

125:	event ClaimRewards(
141:	event InitiateMigration(address newContract);
142:	event CompleteMigration(address oldContract);
143:	event SetDelegationSpace(string delegationSpace, bool shouldClear);
144:	event SetVoteDelegate(address voteDelegate);

/src/PirexRewards.sol Links: 33, 63.

33:	event SetProducer(address producer);
63:	event Harvest(

[NC-03] Underscore Notation Not Used / Not Used Consistently

Consider using underscore notation to help with contract readability (Ex. 23453 -> 23_453).

Findings:

/src/vaults/AutoPxGlp.sol Links: 19, 21, 25, 26.

19:	uint256 public constant MAX_PLATFORM_FEE = 2000;
21:	uint256 public constant MAX_COMPOUND_INCENTIVE = 5000;
25:	uint256 public platformFee = 1000;
26:	uint256 public compoundIncentive = 1000;

/src/vaults/AutoPxGmx.sol Links: 21, 22, 23, 26, 29, 30.

21:	uint256 public constant MAX_PLATFORM_FEE = 2000;
22:	uint256 public constant FEE_DENOMINATOR = 10000;
23:	uint256 public constant MAX_COMPOUND_INCENTIVE = 5000;
26:	uint24 public poolFee = 3000;
29:	uint256 public platformFee = 1000;
30:	uint256 public compoundIncentive = 1000;

[NC-04] Order of Functions Not Compliant With Solidity Docs

The Solidity documentation suggests the following function ordering: constructor, receive function (if exists), fallback function (if exists), external, public, internal, private.

The following contracts are not compliant: PxGmxReward.sol: Internal functions are positioned before external functions. AutoPxGmx.sol: External functions are positioned after public functions. AutoPxGlp.sol: External functions are positioned after public functions. PirexRewards.sol: Public functions are positioned after internal functions. PirexGmx.sol: External functions are positioned after internal functions.

[NC-05] No Documentation For Interfaces

It is good practice to have documentation for interfaces, even if there is documentation for the functions that inherit the interface.

Findings:

IPirexRewards.col, IProducer.sol, IAutoPxGlp.sol, Common.sol.

[NC-06] Power of Ten Literal > 10e3 Not In Scientific Notation

Power of ten literals > 10e3 are easier to read when expressed in scientific notation. Consider expressing large powers of ten in scientific notation (Ex. 10e5).

Findings:

/src/vaults/AutoPxGlp.sol Links: 20.

20:	uint256 public constant FEE_DENOMINATOR = 10000;

[NC-07] Magic Number Used

The uint 18 is used without indication of it's use. For code clarity it is best to not use magic numbers and replace them with well-named constants.

Findings:

/src/PirexFees.sol Links: 11.

11:	PxERC20(_pirexRewards, "Pirex GMX", "pxGMX", 18)