fix(gamm): SpotPrice keeper function #3715
Conversation
github-actions
bot
added
C:app-wiring
| quoteAssetDenom string, | ||
| baseAssetDenom string, |
There was a problem hiding this comment.
Note: the order is changed to avoid twap migrations
| @@ -40,7 +40,7 @@ func (k Keeper) CalculateSpotPrice( | |||
| } | |||
| }() | |||
|
|
|||
| spotPrice, err = pool.SpotPrice(ctx, baseAssetDenom, quoteAssetDenom) | |||
| spotPrice, err = pool.SpotPrice(ctx, quoteAssetDenom, baseAssetDenom) | |||
There was a problem hiding this comment.
note: technically, pools could keep the existing parameter order and just get fixed on the implementation level. However, I think it is useful to keep the order consistent with the keeper. As a result, I changed the denom order for pools as well
p0mvn
requested review from
czarcas7ic,
nicolaslara,
ValarDragon and
mattverse
| // | ||
| // Case 1: base = uosmo, quote = uatom -> for one uosmo, get 2 uatom = 4 / 2 = 2 | ||
| // | ||
| // Case 2: base = uatom, quote = uosmo -> for one uatom, get 0.5 uosmo = 2 / 4 = 0.5 |
There was a problem hiding this comment.
This is the same thing, but I find it easier to read "it costs 0.5 uosmo to get one uatom" and "it costs "2 uatom to get 1 osmo".
I think this is because the word "quote" implies to me that it is the price/cost of the base asset denominated in quote asset. So, what you're "getting" is the base asset.
There was a problem hiding this comment.
Gotcha, I added your interpretation in comments as well
| sp0, err0 := k.CalculateSpotPrice(ctx, poolId, denom0, denom1) | ||
| // sp1 = denom0 base, denom1 quote. |
There was a problem hiding this comment.
useful to have these comments here!
|
Merging once CI passes since have 2 approvals |
Closes: #XXX
What is the purpose of the change
This PR fixes the SpotPrice keeper function.
It fixes the underlying balancer pool bug and stableswap pool hack. Then, it switches parameter order on the keeper layer so that TWAP does not require migrations.
The parameter order is switched for the following functions:
CalculateSpotPricebalancer.SpotPricestableswap.SpotPriceIt refactors tests that utilize these functions. As an outcome of this change,
v1.SpotPricequerier switches the arguments in the wrong order to make the result correct.Testing and Verifying
Documentation and Release Note
Unreleasedsection inCHANGELOG.md? yes