feat(vpool): sqrt of liquidity depth tracked on pool #1243
Conversation
|
I lack some context on why this change. Is there any doc? |
|
@jgimeno Missing fields from the AMM struct here: https://github.com/NibiruChain/think-tank/blob/794b28c3951a7605b8832595c03dbc82b46d2070/drift-v1/programs/clearing_house/src/state/market.rs#L90-L107 This is one of the pieces needed to close |
x/common/constants.go
Outdated
| @@ -3,5 +3,5 @@ package common | |||
| const ( | |||
| TreasuryPoolModuleAccount = "treasury_pool" | |||
| // Precision for int representation in sdk.Int objects | |||
| Precision = int64(1_000_000) | |||
| MICRO = int64(1_000_000) | |||
There was a problem hiding this comment.
MICRO or MEGA? Also what's wrong with Precision?
In my mind,
micro ==> multiply by 10^-6
mega ==> multiply by 10^6
There was a problem hiding this comment.
If you have 2 dollars, you have 2 million micro dollars. The semantics here would be 2$ * MICRO = 2_000_000, so I'm using MICRO to express a unit conversion.
I think an even clearer name would be TO_MICRO, just to emphasize how to use it. 5 * TO_MICRO = 5_000_000 is probably easier to follow.
Precision is vague, and someone wouldn't know what it means by looking at it. In our case, we're not really using this value as a precision. It's just a simple multiplier into micro units.
There was a problem hiding this comment.
Yes, TO_MICRO or AS_MICRO would be more clear.
|
|
||
| // Bias refers to the net long-short bias of the market. The bias of a pool | ||
| // is equivalent to the sum of all base reserve changes since its creation. | ||
| // string bias = 5 [ |
There was a problem hiding this comment.
Do we want to include these commented-out fields in this PR?
| BaseAssetReserve: sdk.NewDec(5 * common.Precision), | ||
| QuoteAssetReserve: sdk.NewDec(10 * common.TO_MICRO), | ||
| BaseAssetReserve: sdk.NewDec(5 * common.TO_MICRO), | ||
| SqrtDepth: common.MustSqrtDec(sdk.NewDec(5 * 10 * common.TO_MICRO * common.TO_MICRO)), |
There was a problem hiding this comment.
In this case, I would put in the literal number we expect rather than rely on a utility function to calculate it for us, because the utility function is also used in the class's code path to calculate this value. If something changes in the utility function, this test case would still pass even though the final value isn't what we expect for this vpool entity.
There was a problem hiding this comment.
I added a simplified constructor for this. I'll add a ticket to convert over most of the vpooltypes.Vpool initializations to use vpooltypes.NewVpool(types.ArgsNewVpool(...)) to accomplish this more directly.
- refactor(vpool): favor types.NewVpool over the direct Vpool constructor #1253
I started working on this in the vpool module, but the PR was becoming large.
Context: A Slack discussion
Summary
sqrt_depthon the vpool struct and use it for computations. #1241sdk.Decandbig.Intvalues and corresponding tests. The exact square root is computed using the underlying big integer for the decimal and preserves precision more than root solving.SqrtDepthas a field of theVpooltype. This is part of an iterative change toward using the base reserves, peg multiplier, and bias for pricing swaps.Vpool.Validate, and uses the new square root functions inEditSwapInvariantProposalNotes to self (ignore)