feat: ERC20 Revoke Allowance by pedronfigueiredo · Pull Request #26906 · MetaMask/metamask-extension

pedronfigueiredo · 2024-09-04T16:03:52Z

Description

Includes e2e test.

Related issues

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

Manual testing steps

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

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

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 · 2024-09-04T16:04:06Z

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.

...ations/components/confirm/info/approve/revoke-static-simulation/revoke-static-simulation.tsx

ui/pages/confirmations/components/confirm/info/approve/spending-cap-context.tsx

ui/pages/confirmations/components/confirm/title/title.tsx

ui/pages/confirmations/components/confirm/info/approve/hooks/use-custom-spending-cap.ts

jpuri

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 · 2024-09-09T09:30:52Z

test/e2e/helpers.js

I guess this is forgotten

metamaskbot · 2024-09-10T09:58:43Z

Builds ready [26ca485]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1591 ± 121 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	285	2626	1526	380	183
		domContentLoaded	1368	2616	1577	252	121
		load	1375	2626	1591	252	121
		domInteractive	11	70	28	14	7

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 16.44 KiB (0.23%)
common: 440 Bytes (0.01%)

sonarqubecloud · 2024-09-25T13:27:31Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

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

See analysis details on SonarCloud

metamaskbot · 2024-09-25T14:12:23Z

Builds ready [181af3d]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1704 ± 81 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	253	2179	1496	520	250
		domContentLoaded	1518	2051	1665	134	64
		load	1524	2193	1704	168	81
		domInteractive	25	83	39	17	8

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 added the team-confirmations Push issues to confirmations team label Sep 4, 2024

pedronfigueiredo self-assigned this Sep 4, 2024

pedronfigueiredo requested review from a team as code owners September 4, 2024 16:03

pedronfigueiredo force-pushed the pnf/3078 branch from aad6584 to d75006d Compare September 4, 2024 16:34

pedronfigueiredo force-pushed the pnf/3004 branch from 68aa72b to 2c95417 Compare September 5, 2024 09:03

matthewwalsh0 requested changes Sep 5, 2024

View reviewed changes

jpuri reviewed Sep 5, 2024

View reviewed changes

pedronfigueiredo force-pushed the pnf/3078 branch 3 times, most recently from 913ae0e to 7fe33dd Compare September 6, 2024 12:41

pedronfigueiredo mentioned this pull request Sep 6, 2024

feat: Edit spending cap for ERC20 approve and increaseAllowance #26845

Merged

7 tasks

pedronfigueiredo force-pushed the pnf/3004 branch from 912ccef to dba74ef Compare September 6, 2024 13:30

pedronfigueiredo force-pushed the pnf/3078 branch from 7fe33dd to 49afb16 Compare September 6, 2024 13:32

pedronfigueiredo requested review from jpuri and matthewwalsh0 September 6, 2024 13:33

pedronfigueiredo force-pushed the pnf/3004 branch 3 times, most recently from 230d222 to 098c276 Compare September 6, 2024 15:33

OGPoyraz previously approved these changes Sep 9, 2024

View reviewed changes

jpuri previously approved these changes Sep 9, 2024

View reviewed changes

pedronfigueiredo force-pushed the pnf/3078 branch from 201ef19 to aed1ca4 Compare September 9, 2024 16:57

pedronfigueiredo force-pushed the pnf/3004 branch from b01ef5f to d0fdd5f Compare September 10, 2024 09:03

matthewwalsh0 previously approved these changes Sep 10, 2024

View reviewed changes

pedronfigueiredo force-pushed the pnf/3078 branch 3 times, most recently from 04db149 to 98111ba Compare September 11, 2024 12:46

pedronfigueiredo force-pushed the pnf/3004 branch from 26ca485 to fbc5e84 Compare September 11, 2024 13:00

pedronfigueiredo added 20 commits September 25, 2024 14:06

feat: ERC20 Revoke Allowance

2e34369

wip

56dec9c

Address feedback

d041600

Update revoke flow logic

5233b45

fix integration test

957cb65

fix unit tests

1952658

Fix spending cap decoding

1932ac5

fix linting error

59827eb

Fix linting errors

60a149a

Fix hook

7931f3d

tweak heading text logic

1b8bb2b

Extract title.tsx spending cap logic into custom hook

74d7ee3

Fix test

50b74b9

Revert unneeded changes

6386051

generalize value param lookup in hook

b85d7bb

Fix lint and test errors after rebase

7d08652

wip

73bc748

wip

dfeede7

wip

c4be88b

wip

bf919af

pedronfigueiredo force-pushed the pnf/3004 branch from fef7bfa to bf919af Compare September 25, 2024 13:07

wip

181af3d

jpuri approved these changes Sep 25, 2024

View reviewed changes

vinistevam approved these changes Sep 26, 2024

View reviewed changes

pedronfigueiredo merged commit 0e82276 into develop Sep 26, 2024

pedronfigueiredo deleted the pnf/3004 branch September 26, 2024 08:29

github-actions bot locked and limited conversation to collaborators Sep 26, 2024

metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Sep 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: ERC20 Revoke Allowance#26906

feat: ERC20 Revoke Allowance#26906
pedronfigueiredo merged 67 commits intodevelopfrom
pnf/3004

pedronfigueiredo commented Sep 4, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Sep 4, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jpuri left a comment

Uh oh!

OGPoyraz Sep 9, 2024

Uh oh!

metamaskbot commented Sep 10, 2024

Uh oh!

sonarqubecloud bot commented Sep 25, 2024

Uh oh!

metamaskbot commented Sep 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

pedronfigueiredo commented Sep 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Sep 4, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jpuri left a comment

Choose a reason for hiding this comment

Uh oh!

OGPoyraz Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Sep 10, 2024

Uh oh!

sonarqubecloud bot commented Sep 25, 2024

Quality Gate passed

Uh oh!

metamaskbot commented Sep 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

pedronfigueiredo commented Sep 4, 2024 •

edited

Loading