Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: TWAMM should use earningsFactorAtInterval[expiration] to calculate the tokens owed after expiration #35

Closed
paco0x opened this issue Jun 30, 2023 · 5 comments
Labels
bug Something isn't working revisit issues that were closed prior to cantina, but may be revisited

Comments

@paco0x
Copy link
Contributor

paco0x commented Jun 30, 2023

Describe the bug

After an order is expired, we should use earningsFactorAtInterval[expiration] as the latestEarningsFactor to calculate the tokensOwed instead of using the earningsFactorCurrent.

Expected Behavior

Fix to use earningsFactorAtInterval[expiration] in _updateOrder() function to calculate the tokensOwed.

To Reproduce

test function:

    function testTWAMM_updatedOrder_CalculateTokensOwedAfterExpiration() public {
        ITWAMM.OrderKey memory orderKey1;
        ITWAMM.OrderKey memory orderKey2;
        uint256 orderAmount;
        (orderKey1, orderKey2, orderAmount) = submitOrdersBothDirections();

        // submit two more orders so the `earningsFactorCurrent` will continue growing after expiration of order1 & order2
        uint256 extraOrderAmount = 2 ether;
        ITWAMM.OrderKey memory orderKey3 = ITWAMM.OrderKey(address(this), 60000, true);
        ITWAMM.OrderKey memory orderKey4 = ITWAMM.OrderKey(address(this), 60000, false);
        token0.approve(address(twamm), extraOrderAmount);
        token1.approve(address(twamm), extraOrderAmount);
        twamm.submitOrder(poolKey, orderKey3, extraOrderAmount);
        twamm.submitOrder(poolKey, orderKey4, extraOrderAmount);

        // set timestamp to after order1 & order2 expire
        vm.warp(40000);

        // update order1 & order2 after expiration, should use the earningsFactorAtInterval at expiration to settle
        twamm.updateOrder(poolKey, orderKey1, 0);
        twamm.updateOrder(poolKey, orderKey2, 0);

        uint256 token0Owed = twamm.tokensOwed(poolKey.currency0, orderKey2.owner);
        uint256 token1Owed = twamm.tokensOwed(poolKey.currency1, orderKey2.owner);

        assertEq(token0Owed, orderAmount);
        assertEq(token1Owed, orderAmount);
    }

result:

❯ forge test -vv --match-test 'testTWAMM_updatedOrder_CalculateTokensOwedAfterExpiration'
[⠒] Compiling...
[⠒] Compiling 1 files with 0.8.19
[⠆] Solc 0.8.19 finished in 3.08s
Compiler run successful!

Running 1 test for test/TWAMM.t.sol:TWAMMTest
[FAIL. Reason: Assertion failed.] testTWAMM_updatedOrder_CalculateTokensOwedAfterExpiration() (gas: 727152)
Logs:
  Error: a == b not satisfied [uint]
        Left: 1500000000000000000
       Right: 1000000000000000000
  Error: a == b not satisfied [uint]
        Left: 1500000000000000000
       Right: 1000000000000000000

Test result: FAILED. 0 passed; 1 failed; finished in 6.76ms

Failing tests:
Encountered 1 failing test in test/TWAMM.t.sol:TWAMMTest
[FAIL. Reason: Assertion failed.] testTWAMM_updatedOrder_CalculateTokensOwedAfterExpiration() (gas: 727152)

Encountered a total of 1 failing tests, 0 tests succeeded

Additional context

No response

@paco0x paco0x added the bug Something isn't working label Jun 30, 2023
@xben12
Copy link

xben12 commented Oct 14, 2023

Hey Bro, where do you get access to create new branch and open the PR? I found another issue in TWAMM liquidity part. and wanna contribute to it.

@paco0x
Copy link
Contributor Author

paco0x commented Oct 14, 2023

Hey Bro, where do you get access to create new branch and open the PR? I found another issue in TWAMM liquidity part. and wanna contribute to it.

You have to fork this repo and create a branch in your own repo.

@xben12
Copy link

xben12 commented Oct 14, 2023

Hey Bro, where do you get access to create new branch and open the PR? I found another issue in TWAMM liquidity part. and wanna contribute to it.

You have to fork this repo and create a branch in your own repo.

thanks! done it!

@saucepoint
Copy link
Collaborator

closing this issue for now, since example hooks do not live in the main branch anymore.

i've added a revisit tag as a reminder

@saucepoint saucepoint added the revisit issues that were closed prior to cantina, but may be revisited label Sep 4, 2024
@ewilz
Copy link
Member

ewilz commented Dec 5, 2024

fixed: #403

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working revisit issues that were closed prior to cantina, but may be revisited
Projects
None yet
Development

No branches or pull requests

4 participants