Virtual Gas Marketplace

The increaseLiquidityPosition function calls the NonfungiblePositionManager with the Foil position ID instead of the Uniswap position ID. As there are liquidity and trade positions in Foil the IDs can differ.

Call Uniswap with the correct position ID.

Foil Team: The issue was resolved in PR#47.

The only way to decrease liquidity of a position is through the decreaseLiquidityPosition function. This has a check epoch.validateEpochNotSettled which will revert if the epoch has settled.

The settlePosition function is supposed to allow you to exit liquidity positions through a branch which calls _settleLiquidityPosition().

However, this calls the collect function in the NonFungiblePositionManager which will collect the tokens owed from fees and previous liquidity burns, but does not burn/decrease the liquidity still in the pool.

The user may still get their collateral back, but does not get the value of their liquidity if it exceeds the loan value.

It should be expected that the liquidity is worth than the loan value, since the LP positions are overcollateralized. When settling a liquidity position, consider first burning all the liquidity and the calling collect.

When settling a liquidity position, consider first burning all the liquidity and then calling collect.

Foil Team: The issue was resolved in PR#47.

In closePositionPosition, the vGasAmount should be set to collectedAmount0 - position.borrowVGas. However, position.borrowedVGas is set to 0 before the subtraction.

This means that the user's vGas amount is overestimated and allows them to drain the protocol.

Reset position.borrowedVGas after rather than before the subtraction.

Foil Team: The issue was resolved in PR#47.

Proof of concept: PoC

Uniswap pool creation is done with three variables (gasToken address, ethToken address, and feeRate). These variables are all predictable even before creation of specified tokens.

Hence it is possible to frontrun pool creations happening in Epoch.sol/createValid(), which will lead to revert in epoch creation.

Use CREATE2 to deploy the virtual tokens with a configurable salt so that pool creation cannot be permanently DoS, additionally be sure to use a private rpc to avoid being frontran to DoS individual epoch creations.

Foil Team: The issue was resolved in PR#85.

Proof of concept: PoC

The modifyTraderPosition function does not check if the given position is already settled. This enables an attack vector to steal funds by closing an already settled position.

As the collateral & the owned tokens of the position are not set to 0 during the settlement process.

Always use the validateNotSettled validation in the modifyTraderPosition function as no trader activity should occur after an epoch end besides settlement.

If a trader wishes to close their position, they should use the EpochSettlementModule to do so.

Thus the epoch.settled case handling can be removed from the swapTokensExactOut and swapTokensExactIn swap functions as they will no longer be callable after the epoch has been settled.

Foil Team: The issue was resolved in PR#87.

This Equation in the settle function of Position.sol:

self.depositedCollateralAmount += self.vEthAmount - self.borrowedVEth; is slightly different from:

self.depositedCollateralAmount = self.depositedCollateralAmount + self.vEthAmount - self.borrowedVEth;

Because in the first equation, if self.borrowedEth < self.vEthAmount, then the equation will underflow, even though this position is fully collateralized.

This underflow makes certain positions impossible to settle.

Replace self.depositedCollateralAmount += self.vEthAmount - self.borrowedVEth; With self.depositedCollateralAmount = self.depositedCollateralAmount + self.vEthAmount - self.borrowedVEth;

Foil Team: The issue was resolved in PR#47.

In this line, if loanAmount0 > maxAmount0, then the excess loanAmount0 is not considered in the collateralization check:

uint256 availableAmount0 = maxAmount0 > loanAmount0 ? maxAmount0 - loanAmount0 : 0;

The loanAmount0 entered into collateralRequirementAtMinTick is loanAmount0 - tokensOwed0.

Here is a sequence which leads to a state where loanAmount0 > maxAmount0: 1. Create a liquidity position when the current tick is below tickLower, so the loaned amount is 100% token0 2. Swap so the position is liquidity entirely token1 3. Remove most of the liquidity via decreaseLiquidityPosition.

Since the LP is now 100% token1, all the claimed tokens from decreaseLiquidityPosition would be token1 (except for a tiny amount of LP fees). DecreaseLiquidityPosition reduced the maxAmount0, while loanAmount0 is still the amount0 required to create the initial position, so maxAmount0 > loanAmount0.

Now it is true that decreasing a liquidity position should make the collateral requirements lower, but this is already accounted for in tokensOwed0 tokensOwed1 being deducted from loan amounts. 1. Create a position that requires loaning token0 2. Swap so the position is entirely token1 3. decreaseLiquidityPosition.

All the claimed tokens from decreaseLiquidityPosition would be token1 (except for a tiny amount of LP fees).

Consider converting loanAmount0 to the corresponding ETH value and incorporating it into the collateral calculation rather than subtracting it from maxAmount0.

Foil Team: The issue was resolved in PR#91.

When a trader owns a long position that made a good amount of profit and the trader decreases the position so that the full loan is repaid the system will:

• calculate the excess vETH after fully repaying the borrowedVEth amount
• set the borrowedVEth to 0
• save the excess vETH as vEthAmount in the position by calling updateBalance

This is unusual as normally a long position has a vGasAmount amount > 0 and a borrowedVEth amount > 0, but the vEthAmount is usually 0.

This state is problematic when closing the long position as the flow of closing the position looks like the following:

• Swap the positions vGasAmount to vETH
• Increase the depositedCollateralAmount by the amount of vETH received from the swap
• Set the borrowedVEth to 0
• Call the resetBalance function to set the currentTokenAmount, vEthAmount & vGasAmount to 0

Therefore the trader's profit saved in the vEthAmount variable is deleted.

Increase the positions depositedCollateralAmount instead of the vEthAmount when decreasing a long position with profit > the borrowed vETH amount.

Foil Team: The issue was resolved in PR#75.

When a trader closes a position before settlement, there is currently no protection against slippage.

As long as the trader possesses sufficient vEth/vGas to settle their debts, the closure of the position will be successful.

This vulnerability could be exploited by an attacker to siphon off the trader's profits by manipulating the price of the pool, causing the trader to swap at a premium that would be covered by their profits.

By ensuring enough is returned to cover the trader's debt, the attacker can retain the profit minus fees.

Implement a parameter for closing a position that includes slippage protection to prevent potential exploitation by malicious parties.

Foil Team: The issue was resolved in PR#92.

In collateralRequirementAtMaxTick, this line can revert due to underflow: return totalLoanAmountInEth - maxAmount1;

It is possible totalLoanAmountInEth to be less than maxAmount1, as the loan amount of each token is loanAmount - tokensOwed. This means that the position is already overcollateralized by the tokensOwed + liquidity position.

Consider this scenario: 1. User opens liquidity position 2. They wash trade such that the fees paid for token0 and token1 exceed the loan amounts loanAmount0 and loanAmount1

The fees are stored in tokensOwed0 and tokensOwed1. Therefore the loanAmount - tokensOwed of both tokens are 0. This is logical because there’s actually no collateral required to back a position who's loans is entirely backed by collected fees. However, since maxAmount1 is greater than 0, then the equation return totalLoanAmountInEth - maxAmount1; will underflow.

An example where loanAmountInEth becomes 0 was chosen to make the underflow obvious, but just a slight reduction in loanAmountInEth could make the underflow revert happen. This makes it impossible to increase or partially decrease liquidity for some liquidity positions.

In the collateralRequirementAtMaxTick function, consider returning 0 if maxAmount1 > totalLoanAmountInEth.

Foil Team: The issue was resolved in PR#91.

In the updateValidLp function the loanAmount0 and loanAmount are computed by deducting the respective tokensOwed from the loaned amount. However the amount credited to pay down the loan cannot exceed the loaned amount.

Consider the following scenario:

• Trader A opens a position which is initially all vEth liquidity
• Price moves downwards, trader A’s position is now entirely vGas liquidity
• Trader A decreases their position and receives all vGas from reducing their liquidity
• Trader A’s tokensOwed0 are not reflected in a reduction of their loaned amount because they had

no loaned amount0 initially.

• Thus trader A does not receive any collateral back and instead must supply more collateral

because the collateralization validation measures their position as being worth less.

As a result partial decreases are prevented for positions in this scenario as the additionalCollateral is hardcoded to 0.

Consider passing the tokensOwed0 and tokensOwed1 through to the collateralRequirementAtMinTick function and adding them to the availableAmount0 and availableAmount1 values respectively so that this value is not truncated to zero.

Foil Team: The issue was resolved in PR#91.

How much amount0 and amount1 an LP provides initially and how much it receives while closing the position are subject to change based upon the pool price.

Considering LP's can provide liquidity amounts larger than their collateral to the pool, the following scenario is applicable in many situations:

LP's token 1 swapped to token 0 such that LP's borrowedVEth - collectedVEth amount will be bigger than LP's collateral which will result with underflow while closing the position.

This occurs when attempting to deduct the collateral in _closeLiquidityPosition: position.depositedCollateralAmount = position.borrowedVEth - collectedAmount1; In this case the LP is prevented from closing their position.

Allow excess debt that can’t be covered to live on in the position.borrowedVEth.

Note that the case where position.borrowedVEth is left can only occur when the price of the pool has moved downwards through the LP relative to where the LP was first created.

Thus the LP will take on a long position after closing and it is expected and correctly handled when a nonzero position.borrowedVEth exists.

Foil Team: The issue was resolved in PR#91.

In the SwapRouter.exactInputSingle function in Uniswap V3, the exact amountIn is not guaranteed to always be used. If the sqrtPriceLimitX96 is hit during the swap then the swap will complete and the exactInputSingle function call will pass.

https://github.com/Uniswap/v3-periphery/blob/0682387198a24c7cd63566a2c58398533860a5d1/c

ontracts/SwapRouter.sol#L87

A sqrtPriceLimitX96 of 0 is used in the EpochTradeModule.swapTokensExactIn function, therefore the sqrtPriceLimitX96 is assigned to roughly the min or max tick upon performing the actual swap.This means if the swap are to go outside of the range of valid prices for the epoch the exact input amount will not be entirely used up.

In the context of a short, this can mean an overestimation of the amount borrowed which was not entirely used for the swap and causes immediate loss for the user. This is not an issue for the exactOutputSingle function as the amountOutReceived is validated to be exactly the amountOut:

https://github.com/Uniswap/v3-periphery/blob/0682387198a24c7cd63566a2c58398533860a5d1/c

ontracts/SwapRouter.sol#L199.

There are a number of ways this edge case can be validated against:

• Consider reverting if the price of the Uniswap pool is outside of the valid range after a swap, or as

an invariant check after all functions which interact with Uniswap.

• Consider validating whether the swap would put price outside of the valid range, and either

reverting or using a partial fill if this is the case.

• Consider reverting if the balance used up by the swap is not exactly the amount specified,

measured by the balance of address(this).

Foil Team: The issue was resolved in PR#90.

The baseAssetMaxPriceTick is the input parameter to createValid to set the maximum trading tick of the epoch.

However, this tick is not the same tick that is used in epoch.sqrtPriceMaxX96 or epoch.maxPriceD18 as the Uniswap tickSpacing is added to the tick.It is important to distinguish between ticks and tickSpacing.

Each tick is 0.01% price difference apart. A tickSpacing contains multiple ticks depending on the fee tier, and for a 1% fee pool this is 200 ticks which corresponds to a 2.02% price difference.

Therefore, epoch.maxPriceD18 and baseAssetMaxPriceTick correspond to different ticks which are 2.02% price difference apart. The maxPriceD18 is used to bound the settlement price, and is also the highestPrice during trades.

baseAssetMaxPriceTick is used in the validateLp function, which limits the range which liquidity is added.

Since the baseAssetMaxPriceTick is lower than maxPriceD18, it is impossible for liquidity to be added up to the maximum price, and consequently for traders to swap to that price.

Consider consistently using the baseAssetMaxPriceTick to derive the max tick and max price without adding a tick spacing.

If tick adjustment is necessary, consider adding a single tick rather than an entire tickSpacing.

Foil Team: Acknowledged. 26

The owner of a market is set with the initializeMarket function, which is called after deploying the system and can be called by anyone.

This allows an attacker to take over the market by calling the function before the protocol calls it. A malicious owner would be able to configure malicious Uniswap contracts to steal user funds.

This also acts as a griefing attack as the protocol needs to pay gas for re-deploying the system.

Set the owner of the system at deployment.

Foil Team: The issue was resolved in PR#74.

When closing a short position in the _closePosition function, position.vEthAmount is swapped to vGas. Next, if position.borrowedVGas > tokenAmountVGas, then vEth is swapped back to vGas.

Since tokens were swapped from vEth to vGas back to vEth the position closer had to pay extra fees for unnecessary swaps when they could just swapped a lower amount of vETH initially and skipped the second swap.

Consider first calculating the amount of vETH that needs to be swapped to vGas to close the position and then executing only a single swap.

Foil Team: The issue was resolved in PR#75.

The highestPrice for a swap is the exchange rate at the maximum tick. However, this exchange rate does not account for fees.

Therefore, the highestPrice could prevent swaps which occur near the edge of the tick range, as the price after fees are included exceeds the highestPrice.

Account for fees when calculating the highest price.

Foil Team: Acknowledged.

Any user can grief the protocol by frontrunning the submitSettlementPrice function call and asserting a price directly to UMA.

UMA determines the assertion ID by taking in the following parameters: assertionId = _getId(claim, bond, time, liveness, currency, callbackRecipient, escalationManager, identifier);

All of which an attacker can copy what Foil was going to use. When the attacker's transaction gets executed first, Foil's will revert shortly after with the following check:

require(assertions[assertionId].asserter == address(0), "Assertion already exists");

Since time is one of the parameters to create an assertion, submitting the transaction through a private mem-pool will be sufficient to prevent this attack.

Foil Team: Resolved.

The UmaSettlementModule is a contract which ensures owner submits a valid settlement price.

However, the owner can change the address of the oracle at any time, and the address does not have to be a legitimate oracle.

The owner can call updateMarket, change the optimisticOracleV3 address to themselves, and then call assertionResolvedCallback() to accept a malicious price.

Consider only allowing oracle updates when the epoch has not ended.

Foil Team: The issue was resolved in PR#76.

In the getCollateralRequirementForAdditionalTokens the loanAmount is not capped at a minimum of 0 as it is done in the updateValidLp function.

This can lead to underflows if the borrowed amount plus the given increase amount is smaller than the tokensOwed.

Calculate the loanAmount as it is done in the updateValidLp function to prevent underflows.

Foil Team: Acknowledged.

When calling modifyTraderPosition to close a position the system will not automatically withdraw all collateral of the position and instead withdraw based on the given collateralAmount.

This means when a trader makes the mistake of providing a positive collateralAmount to the modifyTraderPosition when closing the position, the closed position will still own collateral.

The user can't call the function again with a 0 tokenAmount and 0 collateralAmount to withdraw the remaining collateral, as this will call _closePosition again and perform a 0 token swap which will revert.

Therefore there are only two ways to withdraw the remaining collateral:

• Wait till the epoch is settled (which could take up to a month)
• Call modifyTraderPosition to open a new position and close it again (which costs trading fees)

Automatically withdraw all remaining collateral when closing a position.

Foil Team: The issue was resolved in PR#75.

There are instances where the protocol rounds in favor of the user. Even though these rounding errors are small, a user withdrawing even a slight amount more than they are entitled to can lead to insufficient funds to pay out the last withdrawer.

In the settle function of Position.sol: self.borrowedVEth rounds down: self.borrowedVEth = (self.borrowedVGas * settlementPriceD18) / 1e18;

Then in the next line self.borrowedVEth is subtracted as part of the user's collateral calculation: self.depositedCollateralAmount = self.vEthAmount - self.borrowedVEth;

Since borrowedVEth is lower than the exact value, then this makes the user's collateral slightly higher than it should be.

In _afterSettlementSwapExactOut, this equation calculates the amountIn a user needs to get a certain amount out.

Since it rounds down, the user can put in less than their required amount: requiredAmountInVGas = amountOutVEth.divDecimal(epoch.settlementPriceD18);

Consider substituting a division function which rounds in the instances where rounding down would be in favor of the user.

Foil Team: The issue was resolved in PR#90.

updateValid() allows the owner to change the Uniswap v3 NonFungiblePositionManager. However, changing this will invalidate the tokenID's of all previous positions, among other problems.

Additionally, an update to uniswapSwapRouter will freeze previous positions as the tokens are approved to the old and not the new router.

Consider storing the variables such as the uniswapPositionManager, uniswapSwapRouter and optimisticOracle as part of the epoch parameters so that changes to market parameters only apply to future epochs.

Foil Team: The issue was resolved in PR#76.

In Uniswap V3 the getAmount0Delta function rounds in the favor of the Uniswap protocol and against the user. Specifically, when supplying liquidity the amount in is rounded up and when burning liquidity the amountOut is rounded down. This behavior results in potentially insolvent positions as the collateralization requirement may not have the same rounding against the user.

For example, only one amount can be rounded by 1 wei since the position is assumed to be entirely in one asset. Additionally, the position may not be subject to precision loss at the max tick, but could be at the current tick.

Consider requiring an additional minimal amount of collateral to address any potential rounding from Uniswap that may occur. This amount could be as small as 2 wei.

Foil Team: The issue was resolved in PR#91.

When calculating the required collateral for a trade position the uniswap fee to close the position is not included.

Therefore the trader might not be able to close the position with the deposited collateral as the fee was not accounted for.

The same could happen for liquidity positions which are converted to trade positions when they are closed.

Include the uniswap fees in the required collateral calculations.

Foil Team: The issue was resolved in PR#47.

Guardian Team: As recommended in M-12 the fee is now added to the collateral requirement calculation, but this is only done if the epoch is settled. The fee is needed in this calculation before it is settled as before settlement trades happen in Uniswap. After settlement, the required collateral calculation is no longer needed in general. We recommend to always add the fee to the collateral requirement calculation.

Functions createLiquidityPosition and createTraderPosition are minting position NFT's to msg.sender.

If the msg.sender is a contract and is not capable of handling NFT related actions and/or is not capable of calling other functions in the system, this position NFT's will stuck at the contract.

Considering all actions related to both LP's and Trader's have NFT ownership check, this can lead to locked funds for users.

Check if the caller of these functions can safely receive ERC721's via checkOnErc721Received.

Foil Team: The issue was resolved in PR#62.

Traders may encounter difficulties in closing positions and, to a lesser extent, modifying positions.

But of utmost significance, traders may find themselves unable to close their positions before settlement if Liquidity Providers close their positions first.

As a result, traders may not be able to realize profit based on the current pool price and may have to wait until settlement, leading to temporarily locked funds and potential loss of yield for traders who are unable to close a profitable position promptly.

It is advised to document to users that the option to close trades before settlement is not guaranteed and is dependent on the availability of liquidity.

Foil Team: Users in any market understand this is a possibility and are always able to trade out of any position with no slippage at expiration, so there is no additional risk compared to any other instrument/market.

In the updateValidLp function the additionalCollateral amount is provided by the user but not used as the amount of collateral tokens transferred in.

Instead the minimum required amount of collateral tokens are transferred in. Thus users will unexpectedly transfer in a lower amount of tokens than their provided additionalCollateral value.

Consider removing the use of additionalCollateral and the associated validation and always transfer in the required collateral, while making it clear to the user how much collateral is to be transferred.

Otherwise consider transferring in the entire additionalCollateral amount and using this full amount for the position’s collateral.

Additionally, consider standardizing on a consistent behavior across LP positions and trader positions.

Such that either both LP positions and trader positions transfer in the minimum required collateral or both transfer in a specified amount of collateral from the user.

Foil Team: Because we are unable to use desired collateral changes as inputs, both trade and LP functions specify a desired position size and slippage protection is implemented by setting limits on additional collateral requirements.

The description above the tokenByIndex function describes the behavior of the totalSupply function.

Update the description to match the behavior of the tokenByIndex function and move the description to the totalSupply function.

Foil Team: The issue was resolved in PR#69.

Some ERC-20 tokens return a boolean instead of reverting therefore using transferFrom will not revert when the transfer fails.

This enables attack vectors in the system when such a token would be used as collateral.

Use safeTransferFrom instead of transferFrom or be aware not to add such tokens to the system.

Foil Team: The issue was resolved in PR#73.

The submitSettlementPrice function checks if the epoch is already settled and returns a "Market already settled" error in that case.

This error is misleading because the epoch is settled, not the market.

Change the error message to "Epoch already settled".

Foil Team: The issue was resolved in PR#61.

The _closeLiquidityPosition function updates the depositedCollateralAmount to 0 after withdrawing it. This pattern is vulnerable to a reentrancy attack if an ERC-777 token is used as collateral.

Also, there is a ReentrancyGuard inherited in the EpochLiquidityModule but never used.

Use a reentrancy guard in all state changing functions, or be aware not to add such tokens to the system.

Foil Team: The issue was resolved in PR#66.

The assertionResolvedCallback function checks if the given assertion exists & that the epoch is not settled yet, but the assertionDisputedCallback function does not.

Add these checks to the assertionDisputedCallback function to prevent unexpected state changes.

Foil Team: The issue was resolved in PR#64.

The system assumes that the collateral token has the same precision as the vETH token (18 decimals). If a collateral token with a different precision is added, calculations will be incorrect.

Consider adding a check in the initializeMarket flow to ensure that the collateral token has 18 decimals.

Foil Team: The issue was resolved in PR#77.

The updateCollateral could transfer 0 tokens if the given amt equals the current depositedCollateralAmount.

Some tokens revert on 0 transfers, which can lead to DoS if such a token is added as collateral in the future.

Consider adding a if statement to check if the amount to transfer is greater than 0 before calling IERC20.transfer.

Foil Team: The issue was resolved in PR#72.

The swapTokensExactOut function checks at the end of the available amount in vETH or vGAS is bigger than the amountIn and reverts otherwise.

This means that it will revert if the available amount equals the amountIn.

Consider changing the condition to availableAmount >= amountIn to prevent unnecessary reverts.

Foil Team: The issue was resolved in PR#75.

As the system calculates within absolute amounts, rebasing tokens are not supported.

When a rebasing token would be used as collateral excess tokens would be stuck in the system if the supply increased, or it would not be possible to withdraw collateral in some cases if the supply decreased.

Handle the collateral amounts with a share price calculation instead, or be aware to not use rebasing tokens as collateral.

Foil Team: The issue was resolved in PR#65.

There are no deadline checks when performing swaps through Uniswap.

This can result in swaps occurring long after the transaction was initially submitted which means the user could have an unexpected price and their slippage parameters would be outdated.

Consider adding a deadline check to swaps and liquidity modification actions.

Foil Team: The issue was resolved in PR#86.

The function validateEpochNotSettled() reverts if the the epoch has ended, regardless of whether it is settled, which contradicts the name

Consider changing the name of the function to reflect it's behaviour

Foil Team: The issue was resolved in PR#60.

Whenever a token is minted in createLiquidityPosition(), it's index is set to totalSupply() + 1.

That means that the first token minted has an index of 1, not 0 and the highest indexed token has an index equal to the total supply.

The tokenByIndex() function reverts when index >= totalSupply. However it is incorrect to revert when index == totalSupply as that is the index of a token that has already been minted.

Consider changing >= to > in tokenByIndex.

Foil Team: The issue was resolved in PR#63.

In modifyTraderPosition, it says that "closing can happen at any time". However, positions positions cannot be closed between when the epoch has ended and when it is settled.

Consider removing the misleading comment, and correctly specifying when the position can be closed.

Foil Team: The issue was resolved in commit 2d83d89.

Throughout the codebase there are outstanding TODO comments, some of which would address findings raised in this report.

Be sure to resolve all TODO comments.

Foil Team: The issue was resolved in commit 4f864ab.

In the submitSettlementPrice function the assertionId is overwritten when the owner calls the function a second time before the first assertion has reached a terminal state.

As a result the previous submitted assertion will not be able to settle as the callback functions in the module will revert.

Additionally, a previously submitted assertion could resolve as disputed after a new valid assertion is submitted by the owner.

This would mark the settlement.disputed value as true and disallow the settlement of the valid true assertion.

Do not allow new assertions to be submitted by the owner until the existing assertion reaches a terminal state.

Foil Team: For this iteration of the protocol, we intend to have only the owner address capable of asserting a settlement price. In the event that a false assertion is mistakenly submitted, the intent is that the owner address can immediately resubmit and overwrite the current assertion (resolving the bond sent to UMA separately).

When creating a new epoch an arbitrary baseAssetMinPriceTick and baseAssetMaxPriceTick are accepted and used for the respective epoch minimum and maximum prices.

Additionally, an arbitrary epochParams.feeRate is provided to determine the fee tier used in the Uniswap pool.

However there is no validation that the baseAssetMinPriceTick and baseAssetMaxPriceTick are even multiples that adhere to the associated tick spacing of the chosen fee tier.

This may lead to unexpected behavior and accounting issues.

Consider implementing validation in the Market.createValid and Market.updateValid functions to assert that the baseAssetMinPriceTick and baseAssetMaxPriceTick are indeed even multiples of the relevant tick spacing for the fee tier provided on the epochParams.

Foil Team: The issue was resolved in PR#70.

In the assertionResolvedCallback the settlement is prevented if the epoch.settled value has been assigned to true.However this value will be assigned to true as soon as any dispute, valid or not, is submitted.

This is because the assertionDisputedCallback is called inside of the disputeAssertion function in the OptimisticOracleV3 contract.

If the dispute is resolved to false, and the original claim is decided to be true, then the settleAssertion function will then call the assertionResolvedCallback with a value of true for assertedTruthfully.

However this call will not settle the epoch or set the settlement price in the Foil system as the epoch.settlement.disputed is true.

Use the assertedTruthfully value to decide whether the settlement should occur in the assertionResolvedCallback function rather than epoch.settlement.disputed.

Foil Team: Acknowledged.

In the swapTokensExactOut function the refundAmountVGas is redundantly assigned twice when doing a gas for eth swap.

Remove the second assignment of the refundAmountVGas value.

Foil Team: The issue was resolved in PR#75.

Only owner of the market can create epochs, update market variables, and submit settlement price.

One step ownership change that is happening with updateMarket call is error-prone and can lead to catastrophic results such as not being able to submit settlement price and locked funds.

Implement a two step ownership transfer mechanism, Ownable2Step libraries can also be used instead of custom ownership mechanism.

Foil Team: The issue was resolved in PR#71.

Dispute time for settlement prices configured as one hour. It is possible that users can not react to settlements within configured time frame.

Consider giving more time to disputers such that wrong prices can be prevented by anyone around the world in any settlement hour. We recommend configuring it to at least six hours.

Foil Team: The issue was resolved in PR#68.

It is possible to create epoch with variable lengths. Which is prone to mistake and can be guaranteed by code.

Make sure epoch times are 30 days by checking difference between endTime and startTime.

It is recommended to create epochs with just startTime as a variable and creatingendTimevia adding 30 days to thestartTime.

Foil Team: We’d like to retain the ability to have different epoch durations (to fit to the calendar year, for example).

It is possible to create epochs for the past. Which is prone to mistake.

Be sure to check startTime is at least block.timeStamp

Foil Team: The issue was resolved in PR#67.

Market can be created and updated with variables that are not putted as parameter which can lead to catastrophic effects.

Be sure to check parameters are not "0" in createValid and updateValid functions of Market.sol

Foil Team: The issue was resolved in PR#77.

A compromised admin has the ability to change the bond currency to an arbitrary token that only they have control over.

This action would enable them to submit any price without any opportunity for the price to be disputed.

It is advised to make the bond currency constant to ensure that the price can always be disputed.

Foil Team: The bond currency is set at epoch creation (in EpochParams). A compromised admin would only be able to change it for an upcoming epoch, at which point market participants would have the option to leave the market.

In order to ensure accurate validation of a user's collateral for their LP position, it is essential to calculate the appropriate amount of gas for the position's liquidity.

This is achieved by utilizing the getAmount0ForLiquidity function, which has been customized based on UniswapV3's original function.

However, the adjustment made to this function in Foil causes an underestimation of the maxAmount0 for a position.

This results in the perceived value of the position being lower, leading to a greater collateral requirement to support the loaned amount.

As a consequence, users will be required to deposit slightly more collateral than necessary in some scenarios.

This rounding is fine from the protocol’s perspective because it ensures users are more collateralized instead of less collateralized.

Additionally, the rounding amount is trivial from the user’s perspective.

Be aware of this rounding and document it for users if deemed necessary.

Foil Team: Pending.

If an LP provide a liquidity to a ranger in order to short/long and their liquidity is used, hence they became a counter-party to the trader, their position closing will likely require two step:

• Closing the liquidity position which will turn the position to a trader.
• Closing the trader position.

Closing the trader position means swapping the same amount back (if epoch is not settled) which will encounter a price impact in the opposite direction this time.

Uninformed users about this might get surprised with the end result of their position closing before epoch is settled.

Be sure to inform users about all intricacies of being an LP and what their actions will result with exactly.

Foil Team: Pending.