Skip to content

fix: PermitSingle, PermitBatch, PermitTransferFrom, PermitBatchTransferFrom simulation "Spending cap"#26684

Merged
digiwand merged 19 commits intodevelopfrom
fix-permit-single-and-batch-simulation
Sep 4, 2024
Merged

fix: PermitSingle, PermitBatch, PermitTransferFrom, PermitBatchTransferFrom simulation "Spending cap"#26684
digiwand merged 19 commits intodevelopfrom
fix-permit-single-and-batch-simulation

Conversation

@digiwand
Copy link
Copy Markdown
Contributor

@digiwand digiwand commented Aug 27, 2024 •
edited
Loading

Description

Fixes PermitSingle, PermitBatch, PermitTransferFrom, PermitBatchTransferFrom simulation to use their respective provided token and amount in token details.

Additionally:

  • adjusts styles to have an 8px gap between each "Spending Cap" entry and right align content
  • create PermitSimulationValueDisplay component to separate individual token contract value logic
  • adds tests
  • adds sentry error to capture exception in case token isn't provided for the relevant primaryTypes

dev note: majority of the newlines come from updated and new test snapshots

Open in GitHub Codespaces

Related issues

Fixes: #26591 (PermitSingle)
Fixes: #26592 (PermitBatch)

Manual testing steps

Test PermitSingle & PermitBatch with Improved Signature setting enabled

See related issues for repro steps

Screenshots/Recordings

Before

See Issue screenshots

After PermitBatch

image

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 27, 2024

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.

@github-actions github-actions bot added the team-confirmations Push issues to confirmations team label Aug 27, 2024
@digiwand digiwand changed the title Fix permit single and batch simulation fix: PermitSingle and PermitBatch simulation "Spending cap" Aug 28, 2024
@digiwand digiwand marked this pull request as ready for review August 28, 2024 07:50
@digiwand digiwand requested review from a team as code owners August 28, 2024 07:50
jpuri
jpuri previously approved these changes Aug 28, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@jpuri jpuri left a comment

Choose a reason for hiding this comment

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

LGTM

matthewwalsh0
matthewwalsh0 previously approved these changes Aug 28, 2024
View reviewed changes
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Aug 28, 2024

Builds ready [6e33351]
Page Load Metrics (107 ± 12 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1041891392110
domContentLoaded621631042512
load691641072512
domInteractive237741157
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 2.2 KiB (0.03%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Aug 28, 2024

Builds ready [e4b8744]
Page Load Metrics (81 ± 13 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint712721124120
domContentLoaded43183752914
load49184812813
domInteractive96928136
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 2.2 KiB (0.03%)
  • common: 0 Bytes (0.00%)

@digiwand
Copy link
Copy Markdown
Contributor Author

digiwand commented Aug 29, 2024

currently puzzled as to why the PermitSingle and PermitBatch snapshot tests I added are working locally but not here
https://github.com/MetaMask/metamask-extension/actions/runs/10602080265/job/29383342715?pr=26684

vinistevam
vinistevam previously approved these changes Aug 29, 2024
View reviewed changes
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Aug 29, 2024

Builds ready [a60b159]
Page Load Metrics (1996 ± 60 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint25422271735627301
domContentLoaded17252207196812158
load17352230199612560
domInteractive23583695
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 2.2 KiB (0.03%)
  • common: 0 Bytes (0.00%)

@digiwand digiwand dismissed stale reviews from matthewwalsh0 and jpuri via 462e0eb September 2, 2024 13:46
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Sep 2, 2024

Builds ready [462e0eb]
Page Load Metrics (1700 ± 43 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint28517881554424203
domContentLoaded1480181216808842
load1489181817008943
domInteractive136838168
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 2.67 KiB (0.04%)
  • common: 0 Bytes (0.00%)

@digiwand
Copy link
Copy Markdown
Contributor Author

digiwand commented Sep 2, 2024

resolved a snapshot merge conflict. may I have rereviews please? 🙏🏼 @jpuri @matthewwalsh0

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Sep 3, 2024

Builds ready [25cc62d]
Page Load Metrics (1709 ± 46 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint20619001493538258
domContentLoaded1461188616849747
load1469190217099646
domInteractive23563595
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 2.67 KiB (0.04%)
  • common: 0 Bytes (0.00%)

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Sep 3, 2024

Quality Gate Passed Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Sep 3, 2024

Builds ready [d8a3f55]
Page Load Metrics (1914 ± 90 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint16862325191818488
domContentLoaded16702312188717785
load16902333191418890
domInteractive248241157
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 2.67 KiB (0.04%)
  • common: 0 Bytes (0.00%)

@digiwand digiwand merged commit 1801174 into develop Sep 4, 2024
@digiwand digiwand deleted the fix-permit-single-and-batch-simulation branch September 4, 2024 00:17
@github-actions github-actions bot locked and limited conversation to collaborators Sep 4, 2024
@metamaskbot metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Sep 4, 2024
@digiwand digiwand added release-12.4.0 Issue or pull request that will be included in release 12.4.0 type-bug Something isn't working release-blocker This bug is blocking the next release regression-RC-12.4.0 and removed release-12.6.0 Issue or pull request that will be included in release 12.6.0 release-blocker This bug is blocking the next release regression-RC-12.4.0 labels Sep 5, 2024
@gauthierpetetin gauthierpetetin added release-12.3.0 Issue or pull request that will be included in release 12.3.0 and removed release-12.4.0 Issue or pull request that will be included in release 12.4.0 labels Sep 11, 2024
@digiwand digiwand changed the title fix: PermitSingle, PermitBatch, PermitTransferFrom, PermitBatchTransferFrom simulation "Spending cap" fix: [cherrypick][V12.3.0] PermitSingle, PermitBatch, PermitTransferFrom, PermitBatchTransferFrom simulation "Spending cap" (1/3 permit simulation fix) Sep 17, 2024
@digiwand digiwand changed the title fix: [cherrypick][V12.3.0] PermitSingle, PermitBatch, PermitTransferFrom, PermitBatchTransferFrom simulation "Spending cap" (1/3 permit simulation fix) fix: PermitSingle, PermitBatch, PermitTransferFrom, PermitBatchTransferFrom simulation "Spending cap" Sep 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jpuri jpuri jpuri approved these changes

@matthewwalsh0 matthewwalsh0 matthewwalsh0 approved these changes

@pedronfigueiredo pedronfigueiredo Awaiting requested review from pedronfigueiredo

@vinistevam vinistevam Awaiting requested review from vinistevam

Assignees

No one assigned

Labels

regression-RC-12.4.0 release-12.3.0 Issue or pull request that will be included in release 12.3.0 release-blocker This bug is blocking the next release team-confirmations Push issues to confirmations team type-bug Something isn't working

Projects

None yet

Milestone

No milestone

7 participants

@digiwand @metamaskbot @jpuri @pedronfigueiredo @matthewwalsh0 @vinistevam @gauthierpetetin