Amm#574
Conversation
balances should include all the pools
| const marketPrice = parseEther("1"); | ||
| const estimatedPrice = parseEther("1.2"); | ||
| const res = getPriceImpact(marketPrice, estimatedPrice); | ||
| console.log(res.toString()); |
There was a problem hiding this comment.
I think we should actually ensure desired behavior here for above three methods, no? If this is just a TODO item, I'll take care of it
|
|
||
| expect(res.isError).to.be.true; | ||
| expect(res.getError()!.message).to.be.eq(ConfigServiceError.reasons.UnableToGetSwapRate); | ||
| }); |
There was a problem hiding this comment.
Think we need one more test here to check for desired behavior in a valid use case. I will add shortly
| return uniquePairs; | ||
| }; | ||
|
|
||
| export const getPriceImpact = (marketPrice: BigNumber, estimatedPrice: BigNumber): BigNumber => { |
There was a problem hiding this comment.
Do we return price impact as a percentage integer, as opposed to a decimal? Seems a bit dangerous - we are assuming method callers/users will know it's returning a perc integer.
I'm adding a method header just to clarify for now. We should be doing better docstrings soon, so this might not be worth concern rn
There was a problem hiding this comment.
Also - isn't price impact the percentage difference between the value of a token before and after a swap?
If so, math should be:
// Assuming user is swapping token B for (-->) token A in router:
// Get token's current prices.
priceTokenA = ..
priceTokenB = ..
// Get amounts for each token after executing the desired swap.
amountTokenA = <amount of token A in liquidity pool> - <amount being withdrawn>
amountTokenB = <amount of token B in liquidity pool> + <amount being deposited>
// Calculate new projected price of token after execution.
projectedPriceTokenB = amountTokenA * priceTokenA / amountTokenB
// Convert to percentage difference from previous value.
1 - projectedPriceTokenB / priceTokenB
There was a problem hiding this comment.
Alternatively, I could be misunderstanding this method's purpose, and it could just be a case of a misnomer here
There was a problem hiding this comment.
If you wanted to account for fees, assuming we take those in tokenA, we would change this one line:
// Assuming a 3% fee:
projectedPriceTokenB = 0.97 * amountTokenA * priceTokenA / amountTokenB
3 participants
The Problem
The Solution