Skip to content

fix issue where conversionRates aren't shown for tokens stored in non-checksum format#12206

Merged
adonesky1 merged 1 commit intodevelopfrom
fix-missing-conversion-rates
Sep 27, 2021
Merged

fix issue where conversionRates aren't shown for tokens stored in non-checksum format#12206
adonesky1 merged 1 commit intodevelopfrom
fix-missing-conversion-rates

Conversation

@adonesky1
Copy link
Contributor

Fixes: #2 in 10.2.0 RC issues doc

  • This is a bandaid for an issue to do with the lack of standardization around hex address casing throughout both the UI and background. Per this conversation and this long standing ticket we will make a push to standardize around checksummed addresses everywhere. But for the sake of getting this RC ready to go I propose this patch.

@adonesky1 adonesky1 requested a review from a team as a code owner September 24, 2021 18:26
@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@adonesky1 adonesky1 force-pushed the fix-missing-conversion-rates branch from f1584de to ba4d549 Compare September 24, 2021 18:30
tmashuang
tmashuang previously approved these changes Sep 24, 2021
@metamaskbot
Copy link
Collaborator

Builds ready [ba4d549]
Page Load Metrics (296 ± 23 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint5039524110249
domContentLoaded2323822794622
load2484152964923
domInteractive2323822794622

highlights:

storybook

mcmire
mcmire previously approved these changes Sep 24, 2021
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

One little comment, but otherwise looks good.

Comment on lines 37 to 42
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of representing "not found" with contractExchangeTokenKey[''], would it be more accurate to say this? Also would it be more clear to represent the key as key rather than i?

Suggested change
const contractExchangeTokenKey =
Object.keys(contractExchangeRates).find((i) =>
isEqualCaseInsensitive(i, tokenAddress),
) || '';
const tokenExchangeRate =
overrides.exchangeRate ?? contractExchangeRates[tokenAddress];
overrides.exchangeRate ?? contractExchangeRates[contractExchangeTokenKey];
const contractExchangeTokenKey =
Object.keys(contractExchangeRates).find((key) =>
isEqualCaseInsensitive(key, tokenAddress),
);
const tokenExchangeRate =
overrides.exchangeRate ?? (contractExchangeTokenKey &&
contractExchangeRates[contractExchangeTokenKey]);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that makes sense!

@adonesky1 adonesky1 dismissed stale reviews from mcmire and tmashuang via 88eb810 September 24, 2021 20:21
@adonesky1 adonesky1 force-pushed the fix-missing-conversion-rates branch from ba4d549 to 88eb810 Compare September 24, 2021 20:21
@metamaskbot
Copy link
Collaborator

Builds ready [88eb810]
Page Load Metrics (306 ± 11 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint593402429847
domContentLoaded2483432902311
load2563513062311
domInteractive2483432902311

highlights:

storybook

@adonesky1 adonesky1 merged commit 14f0d82 into develop Sep 27, 2021
@adonesky1 adonesky1 deleted the fix-missing-conversion-rates branch September 27, 2021 14:06
@github-actions github-actions bot locked and limited conversation to collaborators Sep 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WEBGL dies on ubuntu

5 participants