Uniswap / v3-core

🦄 🦄 🦄 Core smart contracts of Uniswap v3
https://uniswap.org
Other
4.28k stars 2.62k forks source link

Implementation of createLimitOrder and Auto-Closure of Positions Once Filled #688

Closed duralsh closed 4 months ago

duralsh commented 8 months ago

Enhanced Uniswap Core Pool Functionality

This implementation introduces two additional functions to the core Uniswap pool functionality:

1. createLimitOrder

function createLimitOrder(address recipient, int24 tickLower, uint128 amount)

2. collectLimitOrder

function collectLimitOrder(address recipient, int24 tickLower)

Motivation:

  1. Limit Order Creation: A new function for limit order creation has been introduced, which simplifies the process for users. While it adds no new logic, it provides an easier interface for those who want to create limit orders at a specific tick rather than providing liquidity over a range.

  2. Automatic Position Closure: A new function has been added to automatically close positions associated with the current tick once the tick has changed. After a swap operation, if the order size is significant, it might force the current tick to shift. When this happens, the system can iterate over the list of active positions associated with the current tick and clear the array of position owners, offering a gas refund.

A mapping, mapping(int24 => address[]), keeps track of active positions. Each time a limit order is created, this mapping is updated. This list of orders is maintained so that they can be iterated over and canceled later on when the current tick changes, and the order is either fully or partially fulfilled.

Since native burn function doesn't take address argument for the owner of the position as argument for security reasons, a new burnAutomatic function is introduced, since the new function has internal scope and not a part of the public interface, it is safe to use.

Additional Files

Callbacks

The swap and mint functions are designed to be invoked from other contracts rather than directly by users. The calling contract must implement the IUniswapV3MintCallback and IUniswapV3SwapCallback interfaces to ensure proper interaction.

For a reference implementation of these callbacks, a sample can be found in the contracts/UniswapV3PoolSampleCallbacks.sol file. This sample provides a clear example of how to structure and implement the necessary callbacks for interacting with the Uniswap V3 pool's swap and mint functions.

Testing

An incomplete environment with tests can be found in https://github.com/partychad/v3-core/tree/hacky-repo

Possible Issues

Since we are iterating over an array of dynamic and possibly long list, the user who does the swap which shifts the tick might end up paying a high gas or worse the transaction may run out of gas/ exceed block gas limit. To solve this we can close positions in batches if the position array is above a certain length.

However this solution is not ideal as portion of the positions may end up getting unfilled. This approach also requires introduction of new state variables to keep track of the current indexes within the array and additional storage write/ reads.

    mapping(int24 => uint24) public activePositionsCloseIndex;

    ...

    address[] memory memActivePositions = activePositions[(slot0Start.tick)]; 
    uint numberOfPositionsToCLose = memActivePositions.length;

    uint startIndex = activePositionsCloseIndex[slot0Start.tick];
    uint endIndex = (startIndex + BATCH_SIZE) > numberOfPositionsToCLose ? numberOfPositionsToCLose : (startIndex + BATCH_SIZE);

    if (numberOfPositionsToCLose != 0) {
        for (uint i = startIndex; i < endIndex; i++) {
            collectLimitOrder(memActivePositions[i], slot0Start.tick);
        }
        emit FilledPositions(memActivePositions, slot0Start.tick);

        // Update the index or delete if all positions are processed
        if (endIndex == numberOfPositionsToCLose) {
            delete activePositions[slot0Start.tick];
            delete activePositionsCloseIndex[slot0Start.tick];
        } else {
            activePositionsCloseIndex[slot0Start.tick] = uint24(endIndex);
        }
    }

Alternative Approaches

Forwarding gas to the address who closes positions

Although distributing the costs sounds better, this approach makes creating smaller positions not quite feasible. Plus the Out of gas is still and issue and distributing the rewards between batch processors brings another complexity.

    mapping(int24 => uint256) public gasPool;

    function createLimitOrder(address recipient, int24 tickLower, uint128 amount) external payable {
        require(msg.value > 0, "Must send a gas fee");

        // Accumulate the gas fee
        gasPool[tickLower] += msg.value;

        // ... rest of the createLimitOrder logic ...
    }

    function swap(...) external {
        // ... swap logic ...

        // Check if the swap shifts the currentTick
        if (/* logic to check if currentTick shifted */) {
            // Reward the swapper with the accumulated gas fees
            uint256 reward = gasPool[tickLower];
            gasPool = 0; 
            payable(msg.sender).transfer(reward);
        }
    }

Rendering positions inactive after the tick is shifted

If the assignment had different specifications, I would have favored a "pull" mechanism over the current "push" strategy. This would involve allowing position owners to initiate and claim their positions directly from the contract, which is generally a more robust design choice.

library Position {

    // info stored for each user's position
    struct Info {
        bool active; 
        // the amount of liquidity owned by this position
        uint128 liquidity;
        // fee growth per unit of liquidity as of the last update to liquidity or fees owed
        uint256 feeGrowthInside0LastX128;
        uint256 feeGrowthInside1LastX128;
        // the fees owed to the position owner in token0/token1
        uint128 tokensOwed0;
        uint128 tokensOwed1;
    }
}

    function swap(...) external {
        // ... swap logic ...

        // Check if the swap shifts the currentTick
        if (/* logic to check if currentTick shifted */) {
            Position.Info storage userPosition = positions[userAddress];

        // Set the position as inactive
        userPosition.active = false;
        }
    }

stale[bot] commented 4 months ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.