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

feat: ERC20 Revoke Allowance #26906

Merged
merged 67 commits into from
Sep 26, 2024
Merged

feat: ERC20 Revoke Allowance #26906

merged 67 commits into from
Sep 26, 2024

Conversation

pedronfigueiredo
Copy link
Contributor

@pedronfigueiredo pedronfigueiredo commented Sep 4, 2024

Description

Includes e2e test.

Open in GitHub Codespaces

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3004

Manual testing steps

  1. Go to https://etherscan.io/token/0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599#writeContract
  2. Connect your wallet
  3. Go to approve
  4. Input an address under spender
  5. Input 0 under value
  6. Click write
  7. Notice MM confirmation

Screenshots/Recordings

Before

After

Screenshot 2024-09-04 at 17 00 32

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 Sep 4, 2024
@pedronfigueiredo pedronfigueiredo self-assigned this Sep 4, 2024
Copy link
Contributor

github-actions bot commented Sep 4, 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.

Copy link
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.

Changes look good to me, but I am wondering if we can use existing ConfirmContext and add this piece of information, in pace of adding new context. We will have more of these kind of data which we need in different components and it will not be possible to add a context for each.

OGPoyraz
OGPoyraz previously approved these changes Sep 9, 2024
Comment on lines 1080 to 1085
console.log({
mockedRequests,
body: mockedRequests[0].body,
getJson: mockedRequests[0].getJson,
});

Copy link
Member

Choose a reason for hiding this comment

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

I guess this is forgotten

jpuri
jpuri previously approved these changes Sep 9, 2024
matthewwalsh0
matthewwalsh0 previously approved these changes Sep 10, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [26ca485]
Page Load Metrics (1591 ± 121 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint28526261526380183
domContentLoaded136826161577252121
load137526261591252121
domInteractive117028147
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 16.44 KiB (0.23%)
  • common: 440 Bytes (0.01%)

Copy link

sonarcloud bot commented Sep 25, 2024

@metamaskbot
Copy link
Collaborator

Builds ready [181af3d]
Page Load Metrics (1704 ± 81 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint25321791496520250
domContentLoaded15182051166513464
load15242193170416881
domInteractive258339178
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 7.06 KiB (0.10%)
  • common: 179 Bytes (0.00%)

@pedronfigueiredo pedronfigueiredo merged commit 0e82276 into develop Sep 26, 2024
77 checks passed
@pedronfigueiredo pedronfigueiredo deleted the pnf/3004 branch September 26, 2024 08:29
@github-actions github-actions bot locked and limited conversation to collaborators Sep 26, 2024
@metamaskbot metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Sep 26, 2024
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.

8 participants