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

Fix error that occurs when attempting to display transaction value for a transaction with no value argument in the transaction data (in this case approve()) #15398

Merged
merged 1 commit into from Aug 1, 2022

Conversation

adonesky1
Copy link
Contributor

@adonesky1 adonesky1 commented Aug 1, 2022

Fixes: Issue reported by multiple users to support over the weekend

The issue: When users approve an asset and then go to the home screen for that asset (or the activity tab on the home screen without clicking into the asset) they get BigNumber error:
https://user-images.githubusercontent.com/34557516/182171511-8f252132-b826-42a5-a982-dc3a1e8ef5ca.mp4

This issue appears to have been introduced here, since after this change a truthy value was passed to useTokenDisplay here, and so shouldCalculateTokenValue gets set to true here, and then we attempt to calculate the tokenAmount even though the transaction data does not contain a _value arg.

The Fix:
modify the shouldCalculateTokenValue conditional to only move on to calculating the amount if we are able to extract the _value arg required to successful/safely calculate the amount.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2022

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.

…r an approval transaction with no value argument in the transaction data
@adonesky1 adonesky1 force-pushed the fix-transaction-display-error branch from 3bb49d1 to 0d9b0e0 Compare August 1, 2022 14:53
@adonesky1 adonesky1 marked this pull request as ready for review August 1, 2022 15:12
@adonesky1 adonesky1 requested a review from a team as a code owner August 1, 2022 15:12
@metamaskbot
Copy link
Collaborator

Builds ready [0d9b0e0]
Page Load Metrics (1719 ± 41 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint84143108168
domContentLoaded1565185117006933
load1570195017198541
domInteractive1565185117006933

highlights:

storybook

@adonesky1 adonesky1 merged commit fc30468 into develop Aug 1, 2022
@adonesky1 adonesky1 deleted the fix-transaction-display-error branch August 1, 2022 16:42
@github-actions github-actions bot locked and limited conversation to collaborators Aug 1, 2022
@danjm danjm added this to the v10.18.2 milestone Aug 1, 2022
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.

None yet

4 participants