-
-
Notifications
You must be signed in to change notification settings - Fork 187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fetch token rates in batches of 100 #3650
Conversation
In cases where users have an enormous amount of tokens, we don't want to slow down our Price API with large requests. To combat this, when fetching token rates, send requests in batches of 100.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
[tokenContractAddress]: | ||
tokenPrice.value * fallbackCurrencyToNativeCurrencyConversionRate, | ||
[tokenContractAddress]: tokenValue | ||
? tokenValue * fallbackCurrencyToNativeCurrencyConversionRate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I don't totally follow why this additional branch was needed here. Is it now possible for tokenValue
to be falsey?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Realistically, no, but because I've changed #fetchAndMapExchangeRatesForUnsupportedNativeCurrency
to use #fetchAndMapExchangeRatesForSupportedNativeCurrency
, contractExchangeRates
is now a type of ContractExchangeRates, and that is defined as:
interface ContractExchangeRates {
[address: string]: number | undefined;
}
So that means that tokenValue
here is number | undefined
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting.... that | undefined
seems like a mistake, I don't see a case where it is undefined.
Pre-existing issue though, and not a blocker, this doesn't really hurt
Explanation
In cases where users have an enormous amount of tokens, we don't want to slow down our Price API with large requests. To combat this, when fetching token rates, send requests in batches of 100.
References
Fixes #3573.
Changelog
(Updated in PR)
Checklist