-
Notifications
You must be signed in to change notification settings - Fork 9
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
Feat/delta params #19
Changes from 70 commits
2c44aef
5229d6b
c353233
73a307f
20aaf26
a62ec59
ee29260
349ff7e
63ceb04
3378309
b96e20f
3fde14c
d6739bf
2431874
e160d83
c7225b0
fbed68c
af27a1b
f14a4d2
5114cc0
fbf5acd
16b9c1a
b8b034f
94206c0
8896e8a
f2e9205
a5d0b0b
fb6de61
2b715d1
ee8dc25
0d3c003
e15dec7
85c97b4
bfd4244
e34275e
f1b309c
78a4392
e63cea8
103208f
d0c6544
29b0aa8
e64e222
de67775
581403c
a9522ce
b74b536
3a74921
76a7e16
5d69908
6d6a431
7c07c2f
5013430
f69deac
8f5e4f3
ea99cb0
7349792
67a3cb0
52a02c0
641fa4d
fe27670
ad52810
3d8f5d0
9060465
42e2a04
fe2158b
634bd15
8004319
7c62c99
ef054f0
9672182
79141cf
886a62c
c27b6bc
9a98eb9
fb38c0f
49fa6be
2f3cdb5
6d4bc54
4480404
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,23 @@ | ||
# Contribution guidelines | ||
|
||
### General | ||
|
||
A few rules: | ||
1. Follow our [style guide](#style-guide). | ||
2. Format on save using Foundry formatter. | ||
|
||
## Style Guide | ||
|
||
|
||
### Imports | ||
|
||
1. Use named imports as much as possible. | ||
2. Import external dependencies first (libs, etc). | ||
|
||
### Tests | ||
|
||
1. Name the tests accordingly using the following format: | ||
`test_{name of the contract}_{name of the function}_{what should happen}` | ||
|
||
For example: | ||
`test_DFMM_init_IncrementsPoolId`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,7 @@ contract ConstantSum is IStrategy { | |
function init( | ||
address, | ||
uint256 poolId, | ||
IDFMM.Pool calldata pool, | ||
bytes calldata data | ||
) | ||
public | ||
|
@@ -73,6 +74,7 @@ contract ConstantSum is IStrategy { | |
function validateSwap( | ||
address, | ||
uint256 poolId, | ||
IDFMM.Pool calldata pool, | ||
bytes calldata data | ||
) | ||
external | ||
|
@@ -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)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are really There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like these are still returned to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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. Anyway, ff we want to use delta we can add a |
||
|
||
uint256 minLiquidityDelta; | ||
uint256 amountIn; | ||
uint256 fees; | ||
if (nextRx > startRy) { | ||
amountIn = nextRx - startRx; | ||
if (nextRx > pool.reserveX) { | ||
amountIn = nextRx - pool.reserveX; | ||
fees = amountIn.mulWadUp(params.swapFee); | ||
minLiquidityDelta += fees; | ||
} else if (nextRy > startRy) { | ||
amountIn = nextRy - startRy; | ||
} else if (nextRy > pool.reserveY) { | ||
amountIn = nextRy - pool.reserveY; | ||
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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait, if we're passing deltas into this method then why are we getting the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're still passing the reserves (see my reply above). |
||
assert(liquidityDelta >= int256(minLiquidityDelta)); | ||
|
||
invariant = | ||
|
@@ -133,28 +132,59 @@ contract ConstantSum is IStrategy { | |
} | ||
|
||
// This should literally always work lol | ||
function validateAllocateOrDeallocate( | ||
function validateAllocate( | ||
address, | ||
uint256 poolId, | ||
IDFMM.Pool calldata pool, | ||
bytes calldata data | ||
) | ||
external | ||
view | ||
returns ( | ||
bool valid, | ||
int256 invariant, | ||
uint256 reserveX, | ||
uint256 reserveY, | ||
uint256 totalLiquidity | ||
uint256 deltaX, | ||
uint256 deltaY, | ||
uint256 deltaLiquidity | ||
) | ||
{ | ||
(reserveX, reserveY, totalLiquidity) = | ||
(deltaX, deltaY, deltaLiquidity) = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I see. Only the allocations have the deltas. Perhaps we could also do this with swaps? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it would be better to have all the functions expecting the same kind of parameters. |
||
abi.decode(data, (uint256, uint256, uint256)); | ||
|
||
invariant = ConstantSumLib.tradingFunction( | ||
reserveX, | ||
reserveY, | ||
totalLiquidity, | ||
pool.reserveX + deltaX, | ||
pool.reserveX + deltaY, | ||
pool.totalLiquidity + deltaLiquidity, | ||
abi.decode(getPoolParams(poolId), (ConstantSumParams)).price | ||
); | ||
|
||
valid = -EPSILON < invariant && invariant < EPSILON; | ||
} | ||
|
||
// This should literally always work lol | ||
function validateDeallocate( | ||
address, | ||
uint256 poolId, | ||
IDFMM.Pool calldata pool, | ||
bytes calldata data | ||
) | ||
external | ||
view | ||
returns ( | ||
bool valid, | ||
int256 invariant, | ||
uint256 deltaX, | ||
uint256 deltaY, | ||
uint256 deltaLiquidity | ||
) | ||
{ | ||
(deltaX, deltaY, deltaLiquidity) = | ||
abi.decode(data, (uint256, uint256, uint256)); | ||
|
||
invariant = ConstantSumLib.tradingFunction( | ||
pool.reserveX - deltaX, | ||
pool.reserveY - deltaY, | ||
pool.totalLiquidity - deltaLiquidity, | ||
abi.decode(getPoolParams(poolId), (ConstantSumParams)).price | ||
); | ||
|
||
|
@@ -164,6 +194,7 @@ contract ConstantSum is IStrategy { | |
function update( | ||
address sender, | ||
uint256 poolId, | ||
IDFMM.Pool calldata pool, | ||
bytes calldata data | ||
) external onlyDFMM { | ||
if (sender != internalParams[poolId].controller) revert InvalidSender(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I explained this on Discord or during a standup, there are two main reasons:
IStrategy
are receiving thepool
parameter, so I added it for the sake of conformity