Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: fallback conversion rate for token market data #4615

Merged
merged 4 commits into from
Sep 4, 2024
Merged

fix: fallback conversion rate for token market data #4615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
20c3393
fix fallback conversion rate
bergeron Aug 16, 2024
9faba8f
fix unit test
bergeron Aug 16, 2024
209fc3c
Merge branch 'main' into brian/fix-fallback-conversion-rate
bergeron Aug 16, 2024
6d7a495
Merge branch 'main' into brian/fix-fallback-conversion-rate
bergeron Sep 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 36 additions & 21 deletions packages/assets-controllers/src/TokenRatesController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1326,10 +1326,11 @@ describe('TokenRatesController', () => {
});

describe('when the native currency is not supported', () => {
const fallbackRate = 0.5;
it('returns the exchange rates using ETH as a fallback currency', async () => {
nock('https://min-api.cryptocompare.com')
.get('/data/price?fsym=ETH&tsyms=LOL')
.reply(200, { LOL: 0.5 });
.reply(200, { LOL: fallbackRate });
const tokenPricesService = buildMockTokenPricesService({
fetchTokenPrices: fetchTokenPricesWithIncreasingPriceForEachToken,
validateCurrencySupported(currency: unknown): currency is string {
Expand Down Expand Up @@ -1381,47 +1382,47 @@ describe('TokenRatesController', () => {
// token price in LOL = (token price in ETH) * (ETH value in LOL)
'0x02': {
tokenAddress: '0x02',
currency: 'ETH',
currency: 'LOL',
pricePercentChange1d: 0,
priceChange1d: 0,
allTimeHigh: 4000,
allTimeLow: 900,
allTimeHigh: 4000 * fallbackRate,
allTimeLow: 900 * fallbackRate,
circulatingSupply: 2000,
dilutedMarketCap: 100,
high1d: 200,
low1d: 100,
marketCap: 1000,
dilutedMarketCap: 100 * fallbackRate,
high1d: 200 * fallbackRate,
low1d: 100 * fallbackRate,
marketCap: 1000 * fallbackRate,
marketCapPercentChange1d: 100,
price: 0.0005,
price: (1 / 1000) * fallbackRate,
pricePercentChange14d: 100,
pricePercentChange1h: 1,
pricePercentChange1y: 200,
pricePercentChange200d: 300,
pricePercentChange30d: 200,
pricePercentChange7d: 100,
totalVolume: 100,
totalVolume: 100 * fallbackRate,
},
'0x03': {
tokenAddress: '0x03',
currency: 'ETH',
currency: 'LOL',
pricePercentChange1d: 0,
priceChange1d: 0,
allTimeHigh: 4000,
allTimeLow: 900,
allTimeHigh: 4000 * fallbackRate,
allTimeLow: 900 * fallbackRate,
circulatingSupply: 2000,
dilutedMarketCap: 100,
high1d: 200,
low1d: 100,
marketCap: 1000,
dilutedMarketCap: 100 * fallbackRate,
high1d: 200 * fallbackRate,
low1d: 100 * fallbackRate,
marketCap: 1000 * fallbackRate,
marketCapPercentChange1d: 100,
price: 0.001,
price: (2 / 1000) * fallbackRate,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/MetaMask/core/pull/4615/files#r1719238989

pricePercentChange14d: 100,
pricePercentChange1h: 1,
pricePercentChange1y: 200,
pricePercentChange200d: 300,
pricePercentChange30d: 200,
pricePercentChange7d: 100,
totalVolume: 100,
totalVolume: 100 * fallbackRate,
},
},
});
Expand Down Expand Up @@ -1981,14 +1982,28 @@ describe('TokenRatesController', () => {
"marketData": Object {
"0x89": Object {
"0x0000000000000000000000000000000000000001": Object {
"currency": "ETH",
"allTimeHigh": undefined,
"allTimeLow": undefined,
"currency": "UNSUPPORTED",
"dilutedMarketCap": undefined,
"high1d": undefined,
"low1d": undefined,
"marketCap": undefined,
"price": 0.0005,
"tokenAddress": "0x0000000000000000000000000000000000000001",
"totalVolume": undefined,
},
"0x0000000000000000000000000000000000000002": Object {
"currency": "ETH",
"allTimeHigh": undefined,
"allTimeLow": undefined,
"currency": "UNSUPPORTED",
"dilutedMarketCap": undefined,
"high1d": undefined,
"low1d": undefined,
"marketCap": undefined,
"price": 0.001,
"tokenAddress": "0x0000000000000000000000000000000000000002",
"totalVolume": undefined,
},
},
},
Expand Down
18 changes: 15 additions & 3 deletions packages/assets-controllers/src/TokenRatesController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -745,16 +745,28 @@ export class TokenRatesController extends StaticIntervalPollingController<
return {};
}

// Converts the price in the fallback currency to the native currency
const convertFallbackToNative = (value: number | undefined) =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these calculations be done with a non-destructive decimal type like BigNumber, instead of lossy floats (JS Number)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a case you're worried about that can produce a problem, like very small/big numbers? Is this different than other multiplications ?

And how would you do it in code? I think it should eventually serialize as a number, so does that look like converting to BigNumber, multiplying, then back to number?

value !== undefined && value !== null
? value * fallbackCurrencyToNativeCurrencyConversionRate
: undefined;

const updatedContractExchangeRates = Object.entries(
contractExchangeInformations,
).reduce((acc, [tokenAddress, token]) => {
acc = {
...acc,
[tokenAddress]: {
...token,
price: token.price
? token.price * fallbackCurrencyToNativeCurrencyConversionRate
: undefined,
currency: nativeCurrency,
price: convertFallbackToNative(token.price),
marketCap: convertFallbackToNative(token.marketCap),
allTimeHigh: convertFallbackToNative(token.allTimeHigh),
allTimeLow: convertFallbackToNative(token.allTimeLow),
totalVolume: convertFallbackToNative(token.totalVolume),
high1d: convertFallbackToNative(token.high1d),
low1d: convertFallbackToNative(token.low1d),
dilutedMarketCap: convertFallbackToNative(token.dilutedMarketCap),
},
};
return acc;
Expand Down
Loading