Skip to content

fix: Prevent coercing small spending caps to zero#28179

Merged
pedronfigueiredo merged 1 commit intodevelopfrom
pnf/28117
Oct 30, 2024
Merged

fix: Prevent coercing small spending caps to zero#28179
pedronfigueiredo merged 1 commit intodevelopfrom
pnf/28117

Conversation

@pedronfigueiredo
Copy link
Copy Markdown
Contributor

@pedronfigueiredo pedronfigueiredo commented Oct 30, 2024

Description

Previously there was a bug that affected the approve screen. When users had a small spending cap (between 0.001 and 0.0001 or smaller), it was coerced to 0.

This was caused by the method new Intl.NumberFormat(locale).format(spendingCap) that applied the 1,000 large number formatting, so the fix is to bypass it entirely for values smaller than 1.

Additionally, these unformatted small numbers are presented in scientific notation, so we leverage toNonScientificString(spendingCap) to prevent that.

Open in GitHub Codespaces

Related issues

Fixes: #28117

Manual testing steps

See original bug report.

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@pedronfigueiredo pedronfigueiredo added the team-confirmations Push issues to confirmations team label Oct 30, 2024
@pedronfigueiredo pedronfigueiredo self-assigned this Oct 30, 2024
@pedronfigueiredo pedronfigueiredo requested a review from a team as a code owner October 30, 2024 10:36
@github-actions
Copy link
Copy Markdown
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.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [abd4de7]
Page Load Metrics (1913 ± 52 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint17792156192910450
domContentLoaded1708208218799747
load17222159191310952
domInteractive21106482210
backgroundConnect1183362211
firstReactRender50127832713
getState561252411
initialActions00000
loadScripts1240156213889043
setupStore1182292512
uiStartup19312370211810852
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 199 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

DDDDDanica
DDDDDanica previously approved these changes Oct 30, 2024
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, Can we append a simple test?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I added one test.

@DDDDDanica
Copy link
Copy Markdown
Contributor

LGTM !

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [aa689a9]
Page Load Metrics (1955 ± 52 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint27123311801513246
domContentLoaded17802192192210349
load17852229195510852
domInteractive198050189
backgroundConnect797332713
firstReactRender691661062713
getState576242211
initialActions01000
loadScripts1256156013808842
setupStore127025178
uiStartup20012495217612058
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 199 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@pedronfigueiredo pedronfigueiredo added this pull request to the merge queue Oct 30, 2024
@pedronfigueiredo pedronfigueiredo removed this pull request from the merge queue due to a manual request Oct 30, 2024
@pedronfigueiredo pedronfigueiredo added this pull request to the merge queue Oct 30, 2024
Merged via the queue into develop with commit 3d4bab3 Oct 30, 2024
@pedronfigueiredo pedronfigueiredo deleted the pnf/28117 branch October 30, 2024 14:39
@github-actions github-actions bot locked and limited conversation to collaborators Oct 30, 2024
@metamaskbot metamaskbot added release-12.7.0 Issue or pull request that will be included in release 12.7.0 release-12.6.0 Issue or pull request that will be included in release 12.6.0 and removed release-12.7.0 Issue or pull request that will be included in release 12.7.0 labels Oct 30, 2024
@metamaskbot
Copy link
Copy Markdown
Collaborator

Missing release label release-12.6.0 on PR. Adding release label release-12.6.0 on PR and removing other release labels(release-12.7.0), as PR was cherry-picked in branch 12.6.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-12.6.0 Issue or pull request that will be included in release 12.6.0 team-confirmations Push issues to confirmations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: dapp initiated Approval is presented as Remove Permission for some tokens

4 participants