feat(proto)!: aggregate queries on account address to account_balances and account_summary queries #1199
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
toteki
commented
Aug 1, 2022
There was a problem hiding this comment.
Summary of queries affected
Queries with addr but no denom are simply moved:
Supplied(addr)->AccountBalances(addr).SuppliedCollateral(addr)->AccountBalances(addr).Collateral * MarketSummary(denom).UTokenExchangeRateBorrowed(addr)->AccountBalances(addr).Borrowed
Note that collateral is a uToken amount, so it needs an exchange rate multiplier to be interpreted properly (we were doing this already, right?)
Queries with both addr and denom can be extracted from the combined coins:
Supplied(addr, denom)->AccountBalances(addr).Supplied.AmountOf(denom)Collateral(addr, denom)->AccountBalances(addr).Collateral.AmountOf(denom) * MarketSummary(denom).UTokenExchangeRateBorrowed(addr, denom)->AccountBalances(addr).Borrowed.AmountOf(denom)
Queries for USD value with addr but no denom are simply moved:
SuppliedValue(addr)->AccountSummary(addr).SuppliedValueCollateralValue(addr)->AccountSummary(addr).CollateralValueBorrowedValue(addr)->AccountSummary(addr).BorrowedValueBorrowLimit(addr)->AccountSummary(addr).BorrowLimitLiquidationThreshold(addr)->AccountSummary(addr).LiquidationThreshold
Queries for USD values with both addr and denom should now be computed with help from market_summary:
SuppliedValue(addr, denom)->AccountBalances(addr).Supplied.AmountOf(denom) * MarketSummary(denom).OraclePriceCollateralValue(addr, denom)->AccountBalances(addr).Collateral.AmountOf(denom) * MarketSummary(denom).OraclePrice * MarketSummary(denom).UTokenExchangeRateBorrowedValue(addr,denom)->AccountBalances(addr).Borrowed.AmountOf(denom) * MarketSummary(denom).OraclePrice
Final State of the Proto
The remaining queries (which I have moved into a consistent order in the files affected) are:
Params()RegisteredTokens()MarketSummary(denom)AccountBalances(addr)AccountSummary(addr)LiquidationTargets()
Codecov Report
@@ Coverage Diff @@
## main #1199 +/- ##
==========================================
+ Coverage 47.95% 51.10% +3.15%
==========================================
Files 65 65
Lines 7551 6647 -904
==========================================
- Hits 3621 3397 -224
+ Misses 3684 2983 -701
- Partials 246 267 +21
|
proto/umee/leverage/v1/query.proto
Outdated
|
|
||
| // QueryAccountHealthResponse defines the response structure for the AccountHealth gRPC service handler. | ||
| message QueryAccountHealthResponse { |
There was a problem hiding this comment.
With Health I would expect some kind of metric, maybe 0-100%. Here we are returning aggregated balances.
Let's change Health in the query, request and response names to something else. Maybe:
- we can rename
QueryAccountSummarytoQueryAccountBalancesand use the former name here (renamingQueryAccountHealthtoQueryAccountSummary)?
There was a problem hiding this comment.
Good idea - that solves the naming ambiguity
toteki
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Similarly to #1188, this PR combines individual queries into aggregate ones to reduce API surface.
Since some of the fields depend heavily on oracle prices and some do not, they were separate into token-only (
account_balances) and price-sensitive (account_summary) queries. The latter will error when oracle is down.This aggregation also clears the way for future performance optimizations - in particular the
account_summaryfields fetch the same token prices, values, and parameters multiple times during computation. This was unavoidable when they were separate queries, but can be improved soon.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...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers 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...