TLX

In the _submitLeverageUpdate function the acceptablePrice value is determined by applying a standard slippage amount to the result of the fillPrice function.

However the result of the fillPrice function itself can be manipulated such that it returns a higher fillPrice and thus allows for significant extractable value by sandwiching the TLX vault’s order.

Consider the following scenario:

• A malicious actor observes that a significant amount of PnL has built up for the TLX vault and that a rebalance will be triggered by even a small deposit.
• The malicious actor creates a large long order to push the skew of the market higher.
• The malicious actor triggers a small deposit with the mintFor function, triggering a rebalance.
• The rebalance order is assigned a high acceptablePrice which can be significantly more than the fair market value of the index asset due to the inflated price impact.
• The malicious actor subsequently closes their position directly after the rebalance order is executed, receiving positive impact at the expense of the leveraged token vault holders.

Consider basing the acceptable price off of an independent oracle which cannot be manipulated on-chain.

In the redeemFor function the baseAmountReceived which is sent to the user is reduced by the slippage amount, however the full baseWithdrawn amount which is removed from the margin in Perps V3 includes this slippage amount.

Therefore the slippage amount of sUSD will sit in the LeveragedToken contract and will now not be tracked as value in the system since the totalValue result is entirely dependent on the result of perpsMarket.getAvailableMargin.

Explicitly, the accounting which occurs on redemption is as follows.

Removed from SNX and credited to the LeveragedToken contract:

• baseWithdrawn

Debited from the LeveragedToken contract and sent elsewhere:

• baseAmountReceived = baseWithdrawn - fee - slippage
• fee

Thus it is clear to see that baseWithdrawn is received and baseAmountReceived + fee = baseWithdrawn - fee - slippage + fee = baseWithdrawn - slippage is sent out. This results in slippage amount of sUSD being left in the contract.

Deduct slippage from baseWithdrawn after computing the slippage. Notice that this will charge the feePercent on the amount which will be withdrawn and sent to the user instead of the amount to be withdrawn plus the slippage. We believe this behavior to be correct, but the protocol should independently verify this.

In the _validateMintAmount function the mint amount is validated to be within the maximum open interest for the relevant side and market. However the Perps market as a whole is not validated to be within the minimumCredit constraint.

The deposited funds may put the Perps market over the allowed open interest based on the current credit capacity delegated to it by the V3 core system. This results in a DoS of all deposit/withdrawal actions as they attempt to commit orders which will revert due to the minimum credit validation here: https://github.com/Synthetixio/synthetix-v3/blob/306c928de0da52d6e839f0620f8f32d544217648/markets/perps-market/contracts/storage/AsyncOrder.sol#L371

Include the minimum credit validation with an appropriate buffer in the _validateMintAmount function.

The hasPendingLeverageUpdate function relies upon the sizeDelta of the stored order in the Perps V3 system, however stale orders which will be automatically overwritten upon a new order commitment still exist in the order storage.

Therefore stale orders are considered a pending leverage update even though they are inactive. In the case that a stale order is left for the system this will prevent rebalances during this period of time.

Consider checking if the current timestamp is within the settlementStrategy.settlementDelay + settlementStrategy.settlementWindowDuration of the order commitment, if it is not then the order is stale and should not be considered pending.

In the mintFor function there is no validation that deposits cannot occur while the leveraged token’s Perps V3 position is in a liquidatable state. If a rebalance is triggered by the deposit then the validation in the SNX validateRequest function will prevent such an action. However if the deposit is not large enough, or if the deposit corrects for a high leverage caused by losses and thus puts the leverage back in range and does not trigger a rebalance then the deposit will be allowed while the position is liquidatable.

Users who deposit in this case will experience an immediate loss of funds as there is no way to withdraw their assets due to the liquidation checks and the position will be subsequently liquidated.

Consider validating that the position is not in a liquidatable state in the _validateMintAmount function. Notice that being in a liquidatable state is not the same as checking if the position is flagged for liquidation, the LiquidationModule.canLiquidate function should be used for a complete validation.

In Synthetix V3 pending profits cannot be immediately withdrawn from the system until an order has been made to settle those profits to the account margin.

This behavior means that the TLX system may be illiquid for users to withdraw from until a rebalance occurs. In the past TLX has recommended a recursive approach for withdrawals to trigger a rebalance so that the pending PnL can be settled to margin in order to withdraw.

However there are certain edge cases that this approach would miss. The clearest example is if the Perps V3 position is in a state where there is 0 margin and all pending profit. In this case no withdrawal can be triggered to induce a rebalance and unlock all of the pending PnL as liquidity for withdrawers.

This scenario can be achieved if smaller withdrawals are made which stay within the deviation range and these withdrawals are subsequently corrected for in the PnL changes.

This is a rare edge case and may never present itself in production. Nevertheless it may be helpful to include a trusted function to settle outstanding PnL by submitting a dummy order to the Perps V3 system so that users can get liquidity to withdraw.

In the leveraged token system a decaying redemption fee is applied to users who have recently deposited. Throughout the codebase the decaying redemption fee is assigned to start off at a 25% fee and decay to 0% over the course of 25 minutes.

The minimum deposit for a user which will reset the timer for the decaying redemption fee is set as 1e18 or 1 USD in the Config contract as the DECAYING_REDEMPTION_MIN_BASE_AMOUNT value.

With these configured parameters it can be significantly profitable for one vault depositor to do a small deposit on behalf of another depositor who is about to withdraw and cause them to experience a significant decay fee. The malicious vault depositor in this case would gain from the significant fee paid by the victim depositor in this case.

On networks without a public mempool specifically frontrunning a user’s withdrawal transaction is not reliably possible so this attack may operate based upon key indicators that a user is about to withdraw such as Discord messages or market volatility.

Furthermore, if the DECAYING_REDEMPTION_MIN_BASE_AMOUNT is configured too high, then a depositor could simply deposit DECAYING_REDEMPTION_MIN_BASE_AMOUNT - 1 wei multiple times to avoid the decaying redemption fee while still depositing a large amount. This could occur in a single transaction with a multicall or for-loop contract call around the mintFor function.

As a first option, consider requiring that a user has approved another user to mintFor on their behalf.

Alternatively, configure the DECAYING_REDEMPTION_MIN_BASE_AMOUNT, decayingRedemptionFeeStart, and decayingRedemptionFeeDuration with these behaviors in mind. Ensuring that the DECAYING_REDEMPTION_MIN_BASE_AMOUNT is neither too low to incentivize bad faith mints on behalf of other users and that the DECAYING_REDEMPTION_MIN_BASE_AMOUNT value is not too high to incentivize split deposits to avoid the decay fee measure.

Function redeployInactiveToken calls delete _tokens[marketId][targetLeverage][isLong] which deletes the newly deployed token rather than the old, inactive token. Consequently tokenExists will produce an incorrect result and leveraged tokens can be created even for active markets with createLeveragedTokens since if (tokenExists(marketId, targetLeverage, true) passes.

Do not clear the mapping, since it is set to the new token upon redeploy.

The rebalance function does not include any validation against the oracle price staleness with the _ensureOraclePriceLiveliness function, nor does it offer a way for the upkeep caller to update the pyth price.

Therefore stale prices may be used on accident or on purpose which will incorrectly compute the sizeDelta for the leverage update of the system. This will ultimately perturb the leverage of the vault system after a rebalance because the price used to determine the sizeDelta can be in some cases on the order of days old.

Include a _ensureOraclePriceLiveliness validation in the rebalance function as well as a pathway to update the oracle price with the _updateOraclePrice function.

In the _chargeRedemptionFee function if a referralCode is provided the baseAsset is transferred to the referrals address, however there is no logic to accredit the referral fee amount to the referrer who owns the referralCode.

Implement the necessary logic to accredit the referrer who owns the referralCode.

The mintFor function accepts a referralCode parameter, however no referral fee is charged. Currently the referralCode is only emitted in the Minted event.

Implement the referral fee logic for the mintFor function, otherwise remove the referralCode parameter.

In the LeveragedToken contract the pyth feed used to determine the value of the system position is required to be updated with a price in the last 60 seconds of the action being performed.

In times of volatility this can present an arbitrage opportunity whereby a price from 60 seconds ago is abused to make a net positive deposit/withdrawal pair from the vault.

Consider the following scenario:

• The leveraged token vault is long
• Price increases by 1% for the index asset of the vault in the last 60 seconds, from $100 to $101
• User A has an existing deposit of 10 tokens in the vault
• User A makes a deposit for 10 tokens with a new address and provides pyth update data for 60 seconds ago, when the price is $100
• Directly after this User A makes a withdrawal for their original 10 token address and provides the latest Pyth data of $101

User A has now arbitraged the difference between the $100 price and $101 price. Having withdrawn at $101 and deposited the same amount at $100.

Consider reducing the staleness period allowed for the Pyth feed. Furthermore, consider adding an additional fee that is charged to the user upon deposit/withdrawal to reduce the feasibility of this attack.

In the _orderFee function the order fee computed does not include the settlement costs which are levied based on the settlementRewardCost in the Perps V3 system.

This is a one time hard cost that will be applied to every rebalance that occurs and is not specifically remunerated by the depositors/withdrawers who are triggering the rebalance.

Consider if this is acceptable. If it is not, consider requiring that the actor who triggers the rebalance covers the fee for the settlement cost.

The Perps market validatePositionSize validation validates both the max market size and max market value, however the TLX _validateMintAmount validation only validates the max market value.

It is possible that the max value validation is not reached but the max size validation is reached, thus allowing a mint to occur in the TLX vault when the corresponding order cannot go through causing a DoS for actions that would trigger a rebalance.

Consider adding validation for the market size in addition to the existing value validation.

The TlxOwnableUpgradeable contract is intended to be compatible with upgradeable contracts, but it does not use namespaced storage.

This can create issues with future upgrades if the storage layout were to change as documented here: https://eips.ethereum.org/EIPS/eip-7201.

Consider using namespaced storage in the TlxOwnableUpgradeable contract.

In the redeemFor function the redemption fee is multiplied by the target leverage to determine the fee amount levied on the redeemer. The redemption fee percentage is currently configured as 0.5% in the Config contract, therefore if the leveraged token vault is using a target leverage of 10x the redemption fee can be as high as 5%.

This value seems quite high for a redemption fee and may be unexpected by the protocol.

Confirm whether the redemption fee ought to be multiplied by the target leverage, effectively making it a proxy for a notional value fee. If the redemption fee is not meant to reach 5% of the redeemers deposited funds then consider removing the leverage multiplication.

In the computePriceImpact function the price impact for each deposit/withdrawal is computed regardless of if it triggers a rebalance. Furthermore the price impact charged to the user is not concerning the individual contribution the user made to the order size, but instead the entire price impact experienced by the whole rebalanceSizeDelta.

Consider the following scenario in which the rebalanceSizeDelta which will correspond to a rebalance is 100 index tokens.

• rebalanceSizeDelta is initially 0
• Leverage is 2x for example
• 1 sUSD == 1 index token
• User A deposits 20 sUSD and is charged impact for a rebalanceSizeDelta of 40
• User A deposits 20 more sUSD and is charged impact for a rebalanceSizeDelta of 80
• User A deposits 20 more sUSD and is charged impact for a rebalanceSizeDelta of 120
• The rebalanceSizeDelta is now greater than 100 and thus an order is triggered. The vault pays impact for an order size of 120. However user A has paid impact fees for each of 40, 80, and 120 sizeDelta orders.

Consider if this behavior is acceptable. If it is not then consider one of the following solutions.

Firstly, the price impact on the rebalanceSizeDelta can only be levied as a fee to the user if canRebalance is true and the rebalance order will be submitted.

Alternatively, the price impact charged to the user can be the difference in price impact experienced between the pre-existing rebalanceSizeDelta which results from the marginBalance without their deposit/withdrawal and the new rebalanceSizeDelta which includes their deposit/withdrawal. In other words, the price impact that is charged to the user is exactly the price impact difference that their action induces. Notice that these two solutions should not be combined.

The computePriceImpact function returns a boolean as it’s second return value, however this value is always assigned as true.

Consider removing the boolean from the return values of the computePriceImpact function or implement it’s use-case.

When performing a deposit or withdrawal action the price impact that would be experienced by the entire rebalance order is levied against the user. In some cases where there is a large pending rebalance and the deposit or withdrawal is relatively small this may result in an underflow revert when deducting the slippage from the amount to be deposited or withdrawn.

Consider if this is acceptable behavior. If it is then be sure to document it for users and consider introducing a more descriptive custom error and validation for this case.

The _maxLeverage function computes the maximum allowable leverage as 1/minimumInitialMarginRatioD18_ however in the Perps V3 system the maximum allowed leverage for the initial margin validation is based on minimumInitialMarginRatioD18 + initialMarginRatioD18 * impactOnSkew.

By this logic the max leverage should be 1/(minimumInitialMarginRatioD18 + initialMarginRatioD18 * impactOnSkew) where the impactOnSkew is based on the upper bound of leveraged token vault size.

In the following case we can examine how much the maximum leverage calculation would be off for the ETH market:

• skewScale = 1_000_000e18
• initialMarginRatioD18 = 1160000000000000000 (1.16)
• minimumInitialMarginRatioD18 = 20000000000000000 (0.02)
• Max expected vault size: 2,000 ETH

maximum leverage = 1 / (minimumInitialMarginRatioD18 + initialMarginRatioD18 * impactOnSkew) = 1 / (0.02e18 + 1.16e18 * 2_000e18/1_000_000e18) = 44.8028673835x max leverage

However the existing calculation computes the maximum leverage as 1 / minimumInitialMarginRatioD18 = 1e18 / 0.02e18 = 50x max leverage

The max leverage which is used to validate the initial leverage is simply divided by 2 to give a rough validation so that the target leverage allows some buffer between the maximum allowable initial margin, so some inaccuracy may be acceptable here.

If the level of inaccuracy demonstrated is not acceptable, then consider adopting the more exact max leverage calculation which incorporates the initialMarginRatioD18 and impactOnSkew.

In the ZapSwap mint function transferFrom is used on an arbitrary zapAsset without checking the return value.

To be compatible with all tokens use safeTransferFrom in case any token which does not revert on failure is supported.

In the PythPriceHandler contract there is no validation to ensure that the pythPrice.expo is indeed a negative number.

Currently there are no feeds which have this characteristic, however out an abundance of caution if any feeds were to be supported with a positive expo or if any feeds were to be misconfigured the application should revert instead of providing a significantly incorrect price result.

Consider adding a validation against a positive pythPrice.expo value in the getLatestPrice function.

The FundUpkeep contract does not include a resetFailedCounter function which is seen in the RebalanceUpkeep contract. This function may be useful if any unexpected issues were to occur and the failed counter needed to be reset.

Consider adding a resetFailedCounter to the FundUpkeep contract in case of any unexpected scenarios where the failed counter should be reset.

The setupRebalanceUpkeep function configures a new linkTopupUpkeep for every new marketId which is configured. The checkData for this upkeep is the marketId, however the checkUpkeep function for the LinkTopupUpkeep contract does not make use of this upkeep check data.

Every instance of the LinkTopupUpkeep upkeep will check all upkeep contracts registered in the upkeep registry for upkeeps, therefore there is no need to configure a LinkTopupUpkeep for every market id.

Only configure the LinkTopupUpkeep once as this single instance can cover all upkeep contracts.

In the linkTopupUpkeep function the returned value uses the IFundUpkeep interface, however this is not the interface of the linkTopupUpkeep contract.

Correct the return value type to the ILinkTopupUpkeep interface and consider if there should be a function in the AddressProvider to retrieve the FundUpkeep contract instance.

In the FundUpkeep contract the upkeep action attempts to balance the remaining margin value of the leveraged token at the targetAmountPerToken when the amount goes out of the minimum or maximum ranges.

However in the case where the deposits in a LeveragedToken vault become too large it may be impossible for the FundUpkeep contract to redeem to meet the targetAmountPerToken as it does not have enough leveraged tokens to do so. This will result in consistent failures for the upkeep action as the redeem action will continue to revert with an ERC20 balance underflow.

The same could occur for a mint upkeep action though is less likely as there is a lower bound of value which can be in the vault.

Be aware of this behavior and consider limiting the minted/redeemed amount to the balance of the FundUpkeep contract in these cases.

Because ZapSwap simply uses the entire balance of baseAsset within the contract to mint leveraged tokens, a malicious user can donate baseAsset tokens directly to the contract, and potentially trigger MaxMarketValueExceeded in the mint which would prevent users from utilizing the zap functionality.

Either use the amount of bases assets obtained from the swap or clearly document this behavior.

The zapAssetAddress is an arbitrary address passed by the user which may lead to an unsafe, external contract taking control of execution.

It is generally best practice to avoid arbitrary addresses. Consider validation on the zapAssetAddress or document this behavior.

_validateOdosSwapAllData uses bytes 28 and 8 depending on whether the address is encoded or not to ensure the entire balance is being swapped. This works for swapCompact but not swapMultiCompact which loops through multiple inputs.

Clearly document this behavior.

Users may observe that a significant streaming fee has accrued over time and choose to withdraw their deposits to avoid the streaming fee. So long as a user does not withdraw enough funds to trigger a rebalance, they will successfully avoid the streaming fee.

Be aware of this minor gaming of the streaming fee. Know that the amount that can be avoided is limited to what the deviation factor will allow to go without rebalance. However if a solution is desired then the streaming fee can be levied on all deposits and withdrawals.

In the _rebalance function the streaming fee and rebalance fee is withdrawn from the margin of the system’s account. However in cases where the streaming fee has grown to be significant and more than the available margin for the account this can result in a DoS of rebalances.

This is an unlikely scenario however a permissioned function which can settle profits to the accounts margin can resolve this scenario if it were to ever arise.

In the LeveragedToken contract the rebalance function can still be called by the keeper when the contract is in a paused state. This may lead to unexpected issues when the contract is paused.

Be aware of this behavior and consider reverting in the rebalance function if the contract is paused.

The order fee charged in the computePriceImpact function is based on the sizeDelta of the individual deposit or withdrawal being made. As a result it is possible for the net amount of order fees collected from depositors/withdrawers to not cover the order fees levied by the Perps V3 system upon rebalancing.

This is because the calculateOrderFee function in Perps V3 charges a taker fee for the size delta which flips the skew. The taker fee is larger than the maker fee and so therefore many individual order fee computations will estimate that more volume will be charged at the maker fee, when in reality when the entire rebalance sizeDelta order is made more volume will be charged at the higher taker fee.

Similar to L-03, order fees can be charged based on the incremental amount that the user contributes to the overall order fee that will be levied.

Given complications with the skew changing between the order fee being charged to withdrawers and depositors, the safest approach would be to charge users the entire order fee of the rebalance sizeDelta for maximum security against fee gaming. However there is a tradeoff with excessively charging users so these options should be weighed carefully.

The LeveragedToken contract uses _getLeverageUpdateSizeDelta to calculate the size delta of the next order to maintain the intended leverage. Based on the current margin of the position, it calculates what should be the target size of the position: uint256 targetNotional = marginAmount.mul(targetLeverage).div(assetPrice_);

The issue is that it divided by (assetPrice_); rather than the exact price used by the PerpsV3 system, which may potentially differ if Perps uses a Chainlink feed for the market.

Consider the following example:

Margin: $200 Intended Leverage: 2x assetPrice_: $100 SNX Price: $110

targetNotional = $200 * 2 / $100 = 4 tokens

This would return a leverage() of 4 * $110 / $200 = 2.2x which is higher than intended for the position.

Ensure the price feed used in PerpsV3 is also Pyth rather than Chainlink for each market a position is made, otherwise document this behavior.

In the SNX perps market when a cancellation is performed the user who initiates the order cancellation is rewarded for invoking the transaction.

Orders can be submitted to the Perps V3 system that are immediately cancellable because the acceptablePrice is unfulfillable by the current market fill price. This is an acknowledged behavior from a previous Perps V3 audit referenced below.

The TLX vault can be placed into a state where every order committed is thereby immediately cancellable due to the skew exceeding the configured price impact tolerance.

While in this state, a malicious actor can drain the vault by intentionally minting small amounts such that an order is committed by the TLX system and then cancelling the order after the settlement delay has passed. For each cancellation the TLX vault will lose the keeper cancellation reward from it’s margin and the cancelling keeper will gain it.

Reference finding from the SNX Perps V3 audit:

Compare the current market fill price + a small buffer to the acceptable price. If the acceptable price is not met by the current market fill price + a small buffer then do not commit an order in the _submitLeverageUpdate function.

In the _validateMintAmount function only the current amount being minted, represented as the baseAmountIn is validated against the max market size and value validations.

However several smaller mints could take place where each of the individual mint amounts remain below the max market validations, while the summation of the mints are above the max market validations. All of these mints may occur before a rebalance is triggered due to the somewhat significant deviation threshold of 25% of the margin value as a difference between OI and target OI.

Consider validating the current outstanding rebalance sizeDelta against the market maximums as opposed to the immediate baseAmountIn that is currently being minted.

In the _validateMintAmount function there is no buffer applied to the maxMarketSize like there is with the maxSideValue validation. As a result the vault’s mint amount is more likely to drift over the maxMarketSize depending on the market price of the asset and the other interactions with the Perps V3 market outside of the TLX system.

Consider using a buffer to reduce the maxMarketSize to give some wiggle room for changing Perps market conditions.

In the recoverAsset function in the LinkTopupUpkeep contract transfer is used on the arbitrary asset token without validating the return value. This can lead to false positive successful execution of the recoverAsset function even when a token transfer fails for tokens that return false upon failure.

Consider using safeTransfer for the recoverAsset function.

In the Addresses and ProxyOwner contracts there are outstanding TODOs to update the missing addresses and to transfer ownership to the ProxyOwner.

Be sure to address these outstanding TODO comments before deploying the system.