Feat/delta params #19

clemlak · 2024-02-23T14:24:30Z

This PR brings a major change in the DFMM contract: when allocating or deallocating liquidity, strategies are no longer expecting the adjusted reserves as parameters but a deltaLiquidity amount along with maximum or minimum deltaX and deltaY amounts that the liquidity provider is willing to pay or expecting to receive.

This change allows multiple liquidity providers to interact with the protocol at the "same time", without having their transactions being invalidated. Also a swap happening before a liquidity change will no longer revert it.

Other changes:

Forge was added as a dependency (and is not anymore a dependency of Solstat). This change allows us to benefit from the latests updates of the Forge library.

Closes:

…dity

…ategy

Autoparallel

Looks good to me!

Autoparallel · 2024-02-27T14:28:51Z

src/ConstantSum/ConstantSum.sol

@@ -89,27 +91,24 @@ contract ConstantSum is IStrategy {
        ConstantSumParams memory params =
            abi.decode(getPoolParams(poolId), (ConstantSumParams));

-        (uint256 startRx, uint256 startRy, uint256 startL) =
-            IDFMM(dfmm).getReservesAndLiquidity(poolId);
-
        (nextRx, nextRy, nextL) = abi.decode(data, (uint256, uint256, uint256));


These are really deltaRx, deltaRy and deltaL right?

It seems like these are still returned to DFMM as next reserves and liquidity

@kinrezC is right, in this case the parameters are still the reserves, I think it's easier because we don't have to think about if the swap is x for y or vice versa.
@Autoparallel I think this is why you were talking about using int256 yesterday? I understand how they could be useful here but I still think they are annoying to deal with.

Anyway, ff we want to use delta we can add a isXForY parameter.

Autoparallel · 2024-02-27T14:30:55Z

src/ConstantSum/ConstantSum.sol

            fees = amountIn.mulWadUp(params.swapFee);
            minLiquidityDelta += fees.divWadUp(params.price);
        } else {
            revert("invalid swap: inputs x and y have the same sign!");
        }

-        liquidityDelta = int256(nextL) - int256(startL);
+        liquidityDelta = int256(nextL) - int256(pool.totalLiquidity);


Wait, if we're passing deltas into this method then why are we getting the liquidityDelta here?

We're still passing the reserves (see my reply above).

Autoparallel · 2024-02-27T14:32:49Z

src/ConstantSum/ConstantSum.sol

        )
    {
-        (reserveX, reserveY, totalLiquidity) =
+        (deltaX, deltaY, deltaLiquidity) =


Oh I see. Only the allocations have the deltas. Perhaps we could also do this with swaps?

Yes, it would be better to have all the functions expecting the same kind of parameters.

0xJepsen · 2024-02-27T15:17:08Z

src/ConstantSum/ConstantSum.sol

@@ -41,6 +41,7 @@ contract ConstantSum is IStrategy {
    function init(
        address,
        uint256 poolId,
+        IDFMM.Pool calldata pool,


What is this for?

yeah, it's not entirely clear to me why we're passing the pool in init, I checked the implementations in all of the strategies and none of them seem to use this argument.

I explained this on Discord or during a standup, there are two main reasons:

Most of the other functions in IStrategy are receiving the pool parameter, so I added it for the sake of conformity

This extends the possibilities of the strategies, for example restricting the pool creation to stable tokens only, etc...

kinrezC

LGTM, only nit is that it doesn't seem like we need the pool arg in IDFMM.init

kinrezC · 2024-02-27T15:55:36Z

src/ConstantSum/ConstantSum.sol

@@ -41,6 +41,7 @@ contract ConstantSum is IStrategy {
    function init(
        address,
        uint256 poolId,
+        IDFMM.Pool calldata pool,


yeah, it's not entirely clear to me why we're passing the pool in init, I checked the implementations in all of the strategies and none of them seem to use this argument.

kinrezC · 2024-02-27T15:57:03Z

src/ConstantSum/ConstantSum.sol

@@ -89,27 +91,24 @@ contract ConstantSum is IStrategy {
        ConstantSumParams memory params =
            abi.decode(getPoolParams(poolId), (ConstantSumParams));

-        (uint256 startRx, uint256 startRy, uint256 startL) =
-            IDFMM(dfmm).getReservesAndLiquidity(poolId);
-
        (nextRx, nextRy, nextL) = abi.decode(data, (uint256, uint256, uint256));


It seems like these are still returned to DFMM as next reserves and liquidity

0xJepsen · 2024-02-27T17:47:34Z

Only comment is that keeping solver function signatures similar across strategies would be nice dev ex for the kit.

clemlak added 30 commits February 22, 2024 16:05

fix: stack too deep issue in GeometricMean strategy

2c44aef

feat: revamp G3MExtendedLib to solve stack too deep issues

5229d6b

feat: remove deltaL from allocate and deallocate

c353233

chore: add terminology to README

73a307f

feat: allocate and deallocate are using delta params

20aaf26

feat: add extra functions to compute deltas

a62ec59

feat: allocate / deallocate validation funcs are now using deltaLiqui…

ee29260

…dity

test: update MockStrategy to new IStrategy interface

349ff7e

feat: IStrategy now passes pool struct, splits liquidity validation

63ceb04

feat: updates ConstantSum to new IStrategy pattern

3378309

feat: update ConstantSumSolver

b96e20f

feat: updates LogNormal to new IStrategy pattern

3fde14c

feat: solve stack too deep issues

d6739bf

feat: update LogNormalSolver

2431874

test: update ConstantSumTest

e160d83

test: fix G3M allocate test

c7225b0

test: fix G3M init test

fbed68c

feat: fix deltaLiquidity in G3M

af27a1b

feat: add allocateGivenDeltaX in G3MSolver

f14a4d2

feat: rename returned variables in IStrategy

5114cc0

feat: add allocateGivenDeltaY to G3MSolver

fbf5acd

test: fix G3M allocate tests

16b9c1a

feat: fix deallocate in G3M strategy

b8b034f

build: remove strategies deployment for now

94206c0

test: fix G3M deallocate tests

8896e8a

feat: update validateAllocate and validateDeallocate in LogNormal str…

f2e9205

…ategy

feat: add allocateGivenDeltaX and DeltaY to LogNormalSolver

a5d0b0b

test: update LogNormal allocate tests

fb6de61

test: check multiple allocations in a row in a G3M pool

2b715d1

test: add revert check when maxDeltaX is reached in G3M strategy

ee8dc25

test: skip broken test in ConstantSum tests

fe2158b

clemlak requested review from kinrezC, Autoparallel and Alexangelj February 27, 2024 11:28

This was linked to issues Feb 27, 2024

Change the allocate & deallocate parameters to deltas instead of pool reserves #13

Closed

Fee on transfer tokens break initialization of pools and other payments #22

Closed

clemlak marked this pull request as ready for review February 27, 2024 11:33

clemlak and others added 5 commits February 27, 2024 15:51

Merge branch 'main' into feat/delta-params

634bd15

chore: fix conflicts merge error

8004319

test: add WETH, fix wrong constructor param for DFMM

7c62c99

test: add WETH address check in DFMM

ef054f0

test: add skipped test for DFMM allocate

9672182

Autoparallel previously approved these changes Feb 27, 2024

View reviewed changes

0xJepsen reviewed Feb 27, 2024

View reviewed changes

kinrezC previously approved these changes Feb 27, 2024

View reviewed changes

fix: cleanup arb logic

79141cf

kinrezC dismissed stale reviews from Autoparallel and themself via 79141cf February 27, 2024 21:31

kinrezC and others added 8 commits February 27, 2024 16:32

fix optimalArbRaise signature

886a62c

fmt

c27b6bc

test: add DFMM receive revert check

9a98eb9

test: add DFMM receive ETH from WETH check

fb38c0f

chore: add missing NatSpec for weth function

49fa6be

test: change test tokens metadata

2f3cdb5

test: add LPToken metadata check

6d4bc54

test: add DFMM lp tokens mint in init

4480404

clemlak added the 📃 contracts Anything related to the DFMM contracts (or strategies) label Feb 28, 2024

0xJepsen merged commit 5a9295a into main Feb 28, 2024
5 checks passed

0xJepsen deleted the feat/delta-params branch February 28, 2024 17:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/delta params #19

Feat/delta params #19

clemlak commented Feb 23, 2024 •

edited

Loading

Autoparallel left a comment

Autoparallel Feb 27, 2024

kinrezC Feb 27, 2024

clemlak Feb 28, 2024

Autoparallel Feb 27, 2024

clemlak Feb 28, 2024

Autoparallel Feb 27, 2024

clemlak Feb 28, 2024

0xJepsen Feb 27, 2024

kinrezC Feb 27, 2024

clemlak Feb 28, 2024

kinrezC left a comment

kinrezC Feb 27, 2024

kinrezC Feb 27, 2024

0xJepsen commented Feb 27, 2024

Feat/delta params #19

Feat/delta params #19

Conversation

clemlak commented Feb 23, 2024 • edited Loading

Autoparallel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kinrezC left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xJepsen commented Feb 27, 2024

clemlak commented Feb 23, 2024 •

edited

Loading