*We regularly publish short recaps on a decentralized audit in which we participated. This time, we cover the audit of [GFX Labs](https://gfx.xyz).*


*brainbot is a web3 service provider, offering consulting and development services as well as [smart contract audits](https://brainbot.com/smart-contract-audits/). To gain more experience in auditing, our security researchers regularly participate in [decentralized audits](https://medium.com/@brainbot/decentralised-audits-are-here-to-stay-bcee6d1118a8). In this series, we will publish recaps of audits in which we participated in order to provide some insight into the functioning, the smart contract architecture and our findings for the respective protocols.*
## How it works

GFX Labs’ Uniswap V3 product presents a decentralized limit order service that capitalizes on Uniswap V3’s concentrated liquidity mechanism. This unique feature of Uniswap V3 can be leveraged to function as a limit order trading platform against the liquidity pool, providing that concentrated liquidity is committed to a future, densely populated tick.

Once this tick is reached, the pool instantly swaps one asset for another, allowing users to claim directly from the pool. However, there’s a caveat: if users do not claim their position in time, it will revert back to a single asset.

To counter this issue, GFX Labs utilizes the Chainlink Keepers feature. These Keepers monitor the ticks, and when any trade tick is satisfied in the V3 pool, they execute transactions ensuring all trades are fulfilled at the correct times. Consequently, users can confidently claim their tokens from the GFX Labs contract.

Consequently, users can confidently claim their tokens from the GFX Labs contract. GFX Labs charges a fee from the users’ claimed orders to compensate for the Chainlink Keeper service they are running. This fee is collected at the point when users claim their tokens from the contract.

## Noteworthy findings in the codebase

### Fee is wrongly calculated when the token to be claimed/traded is WETH

The fee collected by the contract owner can be either in ETH or WETH form. The owner can claim this WETH from the contract at any time by invoking the following function:

function withdrawNative() external onlyOwner {
        uint256 wrappedNativeBalance = WRAPPED_NATIVE.balanceOf(address(this));
        uint256 nativeBalance = address(this).balance;
        // Make sure there is something to withdraw.
        if (wrappedNativeBalance == 0 && nativeBalance == 0) revert LimitOrderRegistry__ZeroNativeBalance();

        // transfer wrappedNativeBalance if it exists
        if (wrappedNativeBalance > 0) WRAPPED_NATIVE.safeTransfer(owner, wrappedNativeBalance);
        // transfer nativeBalance if it exists
        if (nativeBalance > 0) owner.safeTransferETH(nativeBalance);
    }

This function transfers the entire WETH balance of the contract. However, this might lead to an issue where users' claimable WETH balances, idle in the contract, could inadvertently be deducted.

For instance, suppose a user created an order with $2000 in USDC at a tick, and upon reaching that tick, they are to receive 1 WETH. When the tick is reached and keepers execute the trade, the contract now holds 1 WETH, ready for the user to claim. However, due to the current configuration, the owner could inadvertently withdraw this WETH while collecting swap fees, as the fee calculation considers the contract's total balance.

### Rounding issue in division operation
When calculating the ratio by dividing two token balances, it's crucial to account for the differing decimals between tokens. Failure to do so can lead to significant discrepancies in the computed values.

The existing code determines a user's claimable token balance as follows:

function claimOrder(uint128 batchId, address user) external payable returns (ERC20, uint256) {
        Claim storage userClaim = claim[batchId];
        if (!userClaim.isReadyForClaim) revert LimitOrderRegistry__OrderNotReadyToClaim(batchId);
        uint256 depositAmount = batchIdToUserDepositAmount[batchId][user];
        if (depositAmount == 0) revert LimitOrderRegistry__UserNotFound(user, batchId);

        // Zero out user balance.
        delete batchIdToUserDepositAmount[batchId][user];

        // Calculate owed amount.
        uint256 totalTokenDeposited;
        uint256 totalTokenOut;
        ERC20 tokenOut;

        // again, remembering that direction == true means that the input token is token0.
        if (userClaim.direction) {
            totalTokenDeposited = userClaim.token0Amount;
            totalTokenOut = userClaim.token1Amount;
            tokenOut = poolToData[userClaim.pool].token1;
        } else {
            totalTokenDeposited = userClaim.token1Amount;
            totalTokenOut = userClaim.token0Amount;
            tokenOut = poolToData[userClaim.pool].token0;
        }

        // @audit user deposited WETH, tokenOut is USDC
        // @audit 10**2 * 10**18 / 100**18
        uint256 owed = (totalTokenOut * depositAmount) / totalTokenDeposited; // @audit decimal issue?

        // Transfer tokens owed to user.
        tokenOut.safeTransfer(user, owed);

For clarity, consider that `totalTokenOut` is tokenA and the ratio `depositAmount, totalTokenDeposited` represents tokenB.

The code comments highlight a potential issue when there's a difference in decimals between the tokens. This can lead to the computed value for owed being incorrect or even zero in certain cases.

Let's illustrate this with an example: Suppose Alice deposits 10 SHIB tokens, and Bob deposits 100,000,000 SHIB tokens. After the batch order is fulfilled, the total tokens received amount to 9 USDC (represented by totalTokenOut).

When Alice wants to claim her share `owed = (9 * 10**6 * 10 * 10**18) / (100000000 * 10**18)`, the result is `owed = 0`.

This computation effectively gives Alice a zero USDC share due to the discrepancy between the decimal representations of the tokens.

It's essential to maintain best practices when performing calculations between tokens with different decimals. The recommended approach is to scale both values to a common decimal representation and then round down if necessary.

## What's next?
We'll continue to regularly publish audit recaps for different protocols. Meanwhile, you can also have a look at our other publications on [Medium](https://medium.com/@brainbot).

You can also find our previous audit recaps on our [overview page](https://hackmd.io/@brainbot-services).

## Hard Facts **Decentralized Audit Platform:**
**Platform:** [Sherlock](https://www.sherlock.xyz)

**Audited Protocol:** [GFX Labs](https://app.sherlock.xyz/audits/contests/97)

**Security Researcher:** [mstpr](https://twitter.com/mstprbb) (ranked 2/106 in this contest)