794 improve pricing method #916
Conversation
| @@ -6,6 +6,14 @@ export const localAssets: Metadata[] = LocalAssetRegistry.map(a => | |||
| Metadata.fromJson(a as JsonValue), | |||
| ); | |||
|
|
|||
| export const NUMERAIRE_DENOMS: string[] = ['test_usd', 'usdc']; | |||
| export const NUMERAIRES: Metadata[] = localAssets.filter(m => NUMERAIRE_DENOMS.includes(m.display)); | |||
There was a problem hiding this comment.
I assume we will have two different local assets registy for testnet and mainnet. And we will have to revise the definition of NUMERAIRES
There was a problem hiding this comment.
Yes, I'm working on related registry stuff now
|
|
||
| const aggregator = new BalancesAggregator(ctx, indexedDb); | ||
| const latestBlockHeight = await querier.tendermint.latestBlockHeight(); |
There was a problem hiding this comment.
I think it's better to use latestBlockHeight rather than fullSyncHeight to avoid showing old prices during the sync process
There was a problem hiding this comment.
Ah, I see. Can you add a code comment above about this?
| return results | ||
| .map(price => EstimatedPrice.fromJson(price)) | ||
| .filter(price => price.asOfHeight >= minHeight); | ||
| } |
There was a problem hiding this comment.
I decided to do this in-memory filtering because we won't have a lot of price records for each asset. And using a cursor or double index would make the code and its support much more complicated
Valentine1898
force-pushed
the
794-improve-pricing-method
branch
from
d932701 to
a69f97a
Compare
Valentine1898
requested review from
jessepinho and
grod220
and removed request for
jessepinho
There was a problem hiding this comment.
Nice job! One more thing I noticed. It looks like a good number of prices in the db have 1 for the numerairePerUnit field. The remaining all appear to be between 1 and ~1.2.
Feel free to merge and follow up with another PR if it requires a good deal more of investigation.
| @@ -6,6 +6,14 @@ export const localAssets: Metadata[] = LocalAssetRegistry.map(a => | |||
| Metadata.fromJson(a as JsonValue), | |||
| ); | |||
|
|
|||
| export const NUMERAIRE_DENOMS: string[] = ['test_usd', 'usdc']; | |||
| export const NUMERAIRES: Metadata[] = localAssets.filter(m => NUMERAIRE_DENOMS.includes(m.display)); | |||
There was a problem hiding this comment.
Yes, I'm working on related registry stuff now
|
|
||
| const aggregator = new BalancesAggregator(ctx, indexedDb); | ||
| const latestBlockHeight = await querier.tendermint.latestBlockHeight(); |
There was a problem hiding this comment.
Ah, I see. Can you add a code comment above about this?
| const assetId = getAssetId.optional()(new Metadata(denomMetadata)); | ||
| if (assetId?.inner && !this.estimatedPriceByPricedAsset[uint8ArrayToBase64(assetId.inner)]) { | ||
| const prices = await this.indexedDb.getPricesForAsset(new AssetId(assetId)); | ||
| const assetMetadata = new Metadata(denomMetadata); |
There was a problem hiding this comment.
I wonder why assetMetadataById() returns a partial 🤔 . You can also do:
const prices = await this.indexedDb.getPricesForAsset(
denomMetadata as Metadata,
this.latestBlockHeight,
);
These are all prices for delegation tokens, which are calculated based on the validator's exchange rate. |
This PR contains some improvements on asset pricing
Close #794