-
Notifications
You must be signed in to change notification settings - Fork 388
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
Add a Custom Curve Pool Implementation Handling RAI #144
base: master
Are you sure you want to change the base?
Conversation
locking pragma for RAI pool
contracts/pools/rai/StableSwapRAI.vy
Outdated
N_COINS: constant(int128) = 2 | ||
MAX_COIN: constant(int128) = N_COINS - 1 | ||
REDEMPTION_COIN: constant(int128) = 0 # Index of asset with moving target redemption price | ||
REDMPTION_PRICE_SCALE: constant(int128) = 10 ** 9 |
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 think better to have it being uint256
contracts/pools/rai/StableSwapRAI.vy
Outdated
if i == REDEMPTION_COIN: | ||
x = xp[i] + (_dx * self._get_scaled_redemption_price() / PRECISION) | ||
else: | ||
x = xp[i] + _dx |
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 guess this line is never reached if we have 2-coin pools right? So it is just in case ever a 3-coin pool is needed?
contracts/pools/rai/StableSwapRAI.vy
Outdated
|
||
@pure | ||
@internal | ||
def _xp_mem(_redemption_price: uint256, _vp_rate: uint256, _balances: uint256[N_COINS]) -> uint256[N_COINS]: |
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.
Probably can pass a rates
array instead. This is hardcoded for 2-coin pools (which is fine) but also makes it a bit more verbose. Though doesn't do anything bad also
def rule_adjust_redemption_price(self, st_red_prices): | ||
self.redemption_price_snap.setRedemptionPriceSnap(st_red_prices) | ||
|
||
def invariant_check_virtual_price(self): |
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.
This checks only base virtual price. It will never go down, that's checked in other places. But the actual swap virtual price could!
And this makes it hard to test. Price going down inevitably leads to a loss, but in principle price going up should lead to virtual_price going up: maybe that's something which can be tested.
One more significant thing. Virtual price here is not an invariant and it involves price of RAI going up or down. It will show crazy high positive or crazy high negative apy which is no good. So describing what can be done... |
Can make another kind of
Needs to use sqrt (which can use either virtual_price_2 will be close to something between the price of LP token in USD and in RAI which makes sense. And that will behave really smoother than the traditional virtual price. And still, that stays somewhat an experiment |
Ok, more stable is:
who would have thought huh? |
We did some improvements to the RAI pool code here (as advised by Michael): These also need a final thumbs up so we can merge here as well. |
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.
thank you I am sorry for the delay
@@ -0,0 +1,376 @@ | |||
# @version 0.2.12 | |||
""" |
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.
good
@@ -0,0 +1,20 @@ | |||
import pytest |
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.
good
import pytest | ||
|
||
pytestmark = pytest.mark.usefixtures("add_initial_liquidity") | ||
|
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.
good
assert coin.balanceOf(swap) == amount / 2 | ||
|
||
assert pool_token.balanceOf(alice) == n_coins * 10 ** 18 * base_amount / 2 | ||
assert pool_token.totalSupply() == n_coins * 10 ** 18 * base_amount / 2 |
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.
good
|
||
# Ensure the max burn moves with the redemption price to reflect the proportion of liquidity value removed | ||
with brownie.reverts("Slippage screwed you"): | ||
swap.remove_liquidity_imbalance(amounts, max_burn * 0.999, {"from": alice}) |
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.
good
function snappedRedemptionPrice() public view returns (uint256) { | ||
return internalSnappedRedemptionPrice; | ||
} | ||
} |
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.
thank you
"underlying_address": "0x6c3F90f043a72FA612cbac8115EE7e52BDe6E490" | ||
} | ||
] | ||
} |
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.
thank you
|
||
* `DAI`: [0x6b175474e89094c44da98b954eedeac495271d0f](https://etherscan.io/address/0x6b175474e89094c44da98b954eedeac495271d0f) | ||
* `USDC`: [0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48](https://etherscan.io/address/0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48) | ||
* `USDT`: [0xdac17f958d2ee523a2206206994597c13d831ec7](https://etherscan.io/address/0xdac17f958d2ee523a2206206994597c13d831ec7) |
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.
thank you
|
||
base_tokens: uint256 = CurveBase(self.base_pool).calc_token_amount(base_amounts, _is_deposit) | ||
meta_amounts[MAX_COIN] = base_tokens | ||
|
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.
thank you
@@ -1,6 +1,7 @@ | |||
__pycache__ | |||
.history | |||
.hypothesis/ | |||
.idea/ |
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.
thank you
@@ -1,4 +1,4 @@ | |||
# @version ^0.2.12 | |||
# @version 0.2.12 |
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.
good
N_COINS: constant(uint256) = 2 | ||
MAX_COIN: constant(uint256) = N_COINS - 1 | ||
REDEMPTION_COIN: constant(uint256) = 0 # Index of asset with moving target redemption price | ||
REDMPTION_PRICE_SCALE: constant(uint256) = 10 ** 9 | ||
|
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.
Good
|
||
FEE_DENOMINATOR: constant(uint256) = 10 ** 10 | ||
PRECISION: constant(uint256) = 10 ** 18 # The precision to convert to | ||
BASE_N_COINS: constant(int128) = 3 | ||
BASE_N_COINS: constant(uint256) = 3 |
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.
Good
This pool provides documentation for the pull request to adapt curve v1 Contracts so a non-peggie (such as RAI) and a peggie (such as UST) can be swapped as described at [gitcoin](https://gitcoin.co/issue/reflexer-labs/curve-contract/6/100027296).
Raiust pool is covered by all preexisting common tests, and I have added some specific test for moving redemption price.
Raiust pool is covered by all preexisting common tests, and I have added some specific test for moving redemption price.
The detailed Readme document to describe implementation.
Two Coin Pool with peggie and non-peggie, Raiust Pool
Gitcoin submission review
What I did
Related issue: N/A
This PR adds a new type of Curve pool meant to handle stablecoins with a floating target price such as RAI.
How I did it
All changes and test descriptions can be found here.
How to verify it
You can verify the Quantstamp audit for this pool which can be seen here. Please note QSP#2 that mentions the pool does not update and fetch RAI's redemption price prior to every action because it would be expensive to do so. This means we have to mention this tradeoff to the community.