feat(proto)!: Condense redundant queries into market_summary #1188

toteki · 2022-08-01T02:21:01Z

Description

Removes individual queries which are already fields in market_summary.
Adds new fields to market_summary.
Updates tests.
Leaves TODOs for implementations of the token market limits leverage: implement MaxSupplyUtilization restriction #1162 and leverage: implement MinCollateralLiquidity restriction #1163

closes: #1164

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

codecov-commenter · 2022-08-01T02:25:38Z

Codecov Report

❗ No coverage uploaded for pull request base (main@bcd9aaa). Click here to learn what that means.
The diff coverage is 75.30%.

@@           Coverage Diff           @@
##             main    #1188   +/-   ##
=======================================
  Coverage        ?   47.95%           
=======================================
  Files           ?       65           
  Lines           ?     7551           
  Branches        ?        0           
=======================================
  Hits            ?     3621           
  Misses          ?     3684           
  Partials        ?      246

Impacted Files	Coverage Δ
x/leverage/client/cli/query.go	`0.00% <ø> (ø)`
x/leverage/types/query.pb.gw.go	`0.00% <0.00%> (ø)`
x/leverage/keeper/grpc_query.go	`22.07% <96.77%> (ø)`
x/leverage/client/tests/tests.go	`100.00% <100.00%> (ø)`

toteki · 2022-08-01T03:38:14Z

x/leverage/spec/03_queries.md

- **Total Supplied** queries the [Total Supplied](01_concepts.md#Total-Supplied) of a specified denomination.
- **Total Supplied Value** queries the equivalent USD value of [Total Supplied](01_concepts.md#Total-Supplied) of a specified denomination.
- **Market Summary** combines several asset-specifying queries for more efficient frontend access.
+- **Market Summary** collects data on a given `Token` denomination. A description of each response field can be found in the [QueryMarketSummaryResponse proto definition](../../../proto/umee/leverage/v1/query.proto)


Since the line numbers in the destination file are unstable whether I use the proto or the pb.go, the best I can do right now is link to the smaller one and put the relevant type name in the displayed text for the link.

toteki · 2022-08-01T04:16:40Z

Heads up analytics (@khoerling) frontend (@EbrahimUmee) - query changes here, most importantly eliminating solo queries for anything that is a field in market summary.

For frontend, note that just like available_borrow, there are also available_withdraw and available_collateralize fields that could place upper limits on whale-sized transactions (or any transactions if market conditions were extreme).

In addition, a lot of the new market summary fields could make good graphs if we're really getting into market details (e.g. graphing liquidity vs collateral * uToken exchange rate and minimum_liqudity.

Note: This only takes effect when v3 binaries are released, assuming this gets merged.

RafilxTenfen

Right now, if this was merged, the user cannot retrieve what we have in the TODO's
neither with the specific queries

Maybe we could let the specific ones, until we finish the TODO's or even let both

RafilxTenfen · 2022-08-01T12:30:19Z

x/leverage/client/cli/query.go

@@ -224,33 +216,6 @@ func GetCmdQuerySuppliedValue() *cobra.Command {
 	return cmd
 }

-// GetCmdQueryReserveAmount creates a Cobra command to query for the
-// reserved amount of a specific token.
-func GetCmdQueryReserveAmount() *cobra.Command {


I like the idea of condensing queries, but why remove the specific ones?
Sometimes the user can need only part of that full information '-'

but why remove the specific ones?

The idea is to reduce the API surface so there's less to maintain - that said, it's possible that this measure is a bit aggressive. We could keep the specific ones as well as the improved market summary.

edit: Also eliminates the possibility of the queries and the market summary ever being out of sync, and ensures that downstream every use case started with the same data.

The cons are of course doing extra computation when only one thing is needed (might be a gas problem in wasm?) and having to get the specific thing out of a struct field.

It could increase the gas consumption if someone doesn't need all those fields, but I don't know if that is a big problem

The part of less to maintain is definitely significant on all sides...

toteki · 2022-08-01T13:15:29Z

x/leverage/keeper/grpc_query.go

+	availableBorrow := q.Keeper.GetAvailableToBorrow(ctx, req.Denom) // TODO #1162 #1163 - update implementation
+
+	resp := types.QueryMarketSummaryResponse{
+		SymbolDenom:            token.SymbolDenom,
+		Exponent:               token.Exponent,
+		UTokenExchangeRate:     rate,
+		Supply_APY:             supplyAPY,
+		Borrow_APY:             borrowAPY,
+		Supplied:               supplied.Amount,
+		Reserved:               reserved,
+		Collateral:             uCollateral,
+		Borrowed:               borrowed.Amount,
+		Liquidity:              balance.Sub(reserved),
+		MaximumBorrow:          supplied.Amount, // TODO #1162 #1163 - implement limits
+		MaximumCollateral:      uSupply.Amount,  // TODO #1163 - implement limits
+		MinimumLiquidity:       sdk.ZeroInt(),   // TODO #1163 - implement limits
+		UTokenSupply:           uSupply.Amount,
+		AvailableBorrow:        availableBorrow,
+		AvailableWithdraw:      uSupply.Amount,                  // TODO #1163 - implement limits
+		AvailableCollateralize: uSupply.Amount.Sub(uCollateral), // TODO #1163 - implement limits
+	}


Note that the current fields in market summary are all working - only the new ones having to do with safety limits have TODOs.

I would prefer to do them in a separate PR, since they'll require some math and module behavior changes. This PR would meanwhile merge everything proto-breaking, reducing the impact of the implementation PR.

toteki added 2 commits July 31, 2022 19:04

market summary query

d77a4c6

update market summary url

e2fa8ae

undo url change

78dbb77

toteki added C:x/leverage C:API labels Aug 1, 2022

toteki added 5 commits July 31, 2022 19:50

add minimum_liquidity

1d7a15b

spec query page

e9cfef9

add availablility queries

68e1062

switch collateral and withdraw queries from base token to utoken amounts

ce6158a

cl++

45d50d3

toteki commented Aug 1, 2022

View reviewed changes

comment whitespace

2f22f9d

toteki marked this pull request as ready for review August 1, 2022 03:49

toteki requested review from a team as code owners August 1, 2022 03:49

RafilxTenfen reviewed Aug 1, 2022

View reviewed changes

toteki commented Aug 1, 2022

View reviewed changes

RafilxTenfen approved these changes Aug 1, 2022

View reviewed changes

toteki merged commit 68f0c5e into main Aug 1, 2022

toteki deleted the adam/summary branch August 1, 2022 16:20

This was referenced Aug 1, 2022

leverage: Consolidate token queries to market summary #1174

Closed

feat(proto)!: aggregate queries on account address to account_balances and account_summary queries #1199

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(proto)!: Condense redundant queries into market_summary #1188

feat(proto)!: Condense redundant queries into market_summary #1188

toteki commented Aug 1, 2022 •

edited

Loading

codecov-commenter commented Aug 1, 2022 •

edited

Loading

toteki Aug 1, 2022

toteki commented Aug 1, 2022 •

edited

Loading

RafilxTenfen left a comment

RafilxTenfen Aug 1, 2022

toteki Aug 1, 2022 •

edited

Loading

RafilxTenfen Aug 1, 2022

RafilxTenfen Aug 1, 2022

toteki Aug 1, 2022

feat(proto)!: Condense redundant queries into market_summary #1188

feat(proto)!: Condense redundant queries into market_summary #1188

Conversation

toteki commented Aug 1, 2022 • edited Loading

Description

Author Checklist

Reviewers Checklist

codecov-commenter commented Aug 1, 2022 • edited Loading

Codecov Report

toteki Aug 1, 2022

Choose a reason for hiding this comment

toteki commented Aug 1, 2022 • edited Loading

RafilxTenfen left a comment

Choose a reason for hiding this comment

RafilxTenfen Aug 1, 2022

Choose a reason for hiding this comment

toteki Aug 1, 2022 • edited Loading

Choose a reason for hiding this comment

RafilxTenfen Aug 1, 2022

Choose a reason for hiding this comment

RafilxTenfen Aug 1, 2022

Choose a reason for hiding this comment

toteki Aug 1, 2022

Choose a reason for hiding this comment

toteki commented Aug 1, 2022 •

edited

Loading

codecov-commenter commented Aug 1, 2022 •

edited

Loading

toteki commented Aug 1, 2022 •

edited

Loading

toteki Aug 1, 2022 •

edited

Loading