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

chore: make calc_withdraw_one_coin simulation consistent with _calc_withdraw_one_coin #86

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

michaelhly
Copy link

@michaelhly michaelhly commented Nov 21, 2020

What I did

Change calc_withdraw_one_coin in simulation.py to represent the source of truth instead of approximation.

How I did it

Made calc_withdraw_one_coin in simulation.py to exhibit the same behavior as _calc_withdraw_one_coin

How to verify it

Added test

@michaelhly michaelhly marked this pull request as draft November 21, 2020 05:00
@michaelhly michaelhly changed the title Make calc_withdraw_one_coin consistent with _calc_withdraw_one_coin Make calc_withdraw_one_coin simulation consistent with _calc_withdraw_one_coin Nov 21, 2020
@michaelhly michaelhly changed the title Make calc_withdraw_one_coin simulation consistent with _calc_withdraw_one_coin chore: make calc_withdraw_one_coin simulation consistent with _calc_withdraw_one_coin Nov 21, 2020
@michwill
Copy link
Contributor

Ohh very nice

@michaelhly
Copy link
Author

michaelhly commented Nov 21, 2020

Hmm...not sure what to do about these timeouts. @iamdefinitelyahuman @michwill any tips?

Copy link
Contributor

@michwill michwill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to merge! Anyone has any more comments?

@michwill
Copy link
Contributor

Yeah, idk why timeouts

@iamdefinitelyahuman
Copy link
Member

iamdefinitelyahuman commented Nov 25, 2020

Seems the new test in tests/pools/common/unitary/test_remove_liquidity_one_coin.py is timing out when running against certain pools. Does anything weird happen when it runs locally against these pools?

@michaelhly
Copy link
Author

Seems the new test in tests/pools/common/unitary/test_remove_liquidity_one_coin.py is timing out when running against certain pools. Does anything weird happen when it runs locally against these pools?

I will give that a try and report back.

@@ -148,7 +148,7 @@ def remove_liquidity_imbalance(self, amounts):

def calc_withdraw_one_coin(self, token_amount, i):
xp = self.xp()
xp_reduced = xp
xp_reduced = list(xp)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you

@@ -166,6 +166,6 @@ def calc_withdraw_one_coin(self, token_amount, i):
self.x = [x // (p // 10 ** 18) for x, p in zip(xp_reduced, self.p)]
dy = xp_reduced[i] - self.y_D(i, D1)
self.x = [x // (p // 10 ** 18) for x, p in zip(xp, self.p)]
dy_0 = (xp[i] - new_y)
dy_0 = xp[i] - new_y

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you

curve_model = Curve(swap.A(), balances, n_coins, tokens=pool_token.totalSupply())
expected, _ = curve_model.calc_withdraw_one_coin(amount, idx)

assert swap.swap.calc_withdraw_one_coin(amount, idx) == expected

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you

curve_model = Curve(swap.A(), balances, n_coins, tokens=pool_token.totalSupply())
expected, _ = curve_model.calc_withdraw_one_coin(amount, idx)

assert swap.calc_withdraw_one_coin(amount, idx) == expected

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants