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.

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

Set the BSC_CONTRACT_ADDRESS to lowercase #10800

Merged
merged 1 commit into from
Apr 1, 2021

Conversation

danjm
Copy link
Contributor

@danjm danjm commented Apr 1, 2021

Set the BSC_CONTRACT_ADDRESS to lowercase. This makes it consistent with other address constants and the way all addresses are handled in our front-end. This fixes a bug found while I was QAing where swaps transactions don't appear in token specific views while on binance.

@danjm danjm requested a review from a team as a code owner April 1, 2021 15:37
@danjm danjm requested a review from Gudahtt April 1, 2021 15:37
@github-actions
Copy link
Contributor

github-actions bot commented Apr 1, 2021

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.

@Gudahtt
Copy link
Member

Gudahtt commented Apr 1, 2021

swaps transactions don't appear in token specific views while on binance.

How did the case of the address cause this to happen? 🤔

@tmashuang
Copy link
Contributor

tmashuang commented Apr 1, 2021

Is this the case where txParams.to is not checksummed?

(txParams?.to === SWAPS_CHAINID_CONTRACT_ADDRESS_MAP[chainId] &&

@danjm
Copy link
Contributor Author

danjm commented Apr 1, 2021

How did the case of the address cause this to happen? 🤔

@Gudahtt See Thomas' comment above

@danjm
Copy link
Contributor Author

danjm commented Apr 1, 2021

But maybe then there is a question of why the to params are not checksummed... perhaps because we get it from the api?

@danjm
Copy link
Contributor Author

danjm commented Apr 1, 2021

oh, we normalize it to lowercase in addUnapprovedTransaction

@metamaskbot
Copy link
Collaborator

Builds ready [e39a604]
Page Load Metrics (531 ± 49 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint46715574
domContentLoaded38572353010249
load38772453110249
domInteractive38572352910249

Copy link
Contributor

@tmashuang tmashuang left a comment

Choose a reason for hiding this comment

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

LGTM

@danjm danjm merged commit f12f60a into develop Apr 1, 2021
@danjm danjm deleted the bsc-contract-address-lowercase branch April 1, 2021 17:31
@github-actions github-actions bot locked and limited conversation to collaborators Apr 1, 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.

4 participants