feat(): Adds confirmation screen for 'increaseAllowance' #23560

pedronfigueiredo · 2024-03-18T17:49:32Z

~~NOTE: This PR is blocked by changes to core repo as well as to the test dApp first.~~

Description

Reuses the token approve confirmation screen for 'increaseAllowance'. It also includes an e2e test for the complete flow.

Related issues

Fixes: #2224

Manual testing steps

Go to the test dApp
Create a token
Approve token for a small amount
Try to transfer a higher amount of tokens from another account. (it should fail)
Go back to the first account and increase the allowance.
Try to transfer again from the second account. (it should succeed)

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.

github-actions · 2024-03-18T17:49:47Z

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.

jpuri

great work 🚀

pedronfigueiredo · 2024-03-26T13:51:51Z

@metamaskbot update-policies

metamaskbot · 2024-03-26T14:00:01Z

Policies updated

app/scripts/lib/transaction/metrics.ts

test/e2e/tests/tokens/increase-token-allowance.spec.js

seaona

Explanation on gas changes required in the e2e: in this PR we are updating the test-dapp to the latest version. In the new version, we removed the suggested gas from the test dapp, to fix a non-related issue with network congestion, meaning that from now on, the gas for the approve and transfer is calculated differently

https://github.com/MetaMask/test-dapp/commit/a3a2ea834b8739005325901d48f19b551b02a609#diff-bfe9874d239014961b1ae4e8[…]34a410aaaa2ebe3cf89820556L1757

so we need to update both the transfer and approve occurrences where we check the gas, to the new gas values --> those will always be the same (bc ganache setup and mocks are the same) but they are not the same as the ones we currently have with the previous dapp version

extra note: not sure why FF is passing, but this might indicate that we are not properly validating in that browser - maybe in that browser something else is going on that misses it, but I think that could be out of scope of this PR and could be investigated separately

metamaskbot · 2024-03-28T09:13:12Z

Builds ready [da4c4e8]

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, 2, 3, 4, 5, 6
- content-script: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (799 ± 466 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	70	257	115	41	20
		domContentLoaded	10	66	26	14	7
		load	54	2314	799	970	466
		domInteractive	10	66	26	14	7

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

background: 47 Bytes (0.00%)
ui: 1.2 KiB (0.02%)
common: 26.64 KiB (0.53%)

codecov · 2024-03-28T09:30:52Z

Codecov Report

Attention: Patch coverage is 52.00000% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 69.11%. Comparing base (b2d843f) to head (704b115).

Files	Patch %	Lines
ui/hooks/useTransactionDisplayData.js	16.67%	5 Missing ⚠️
...ion-switch/confirm-transaction-switch.component.js	0.00%	3 Missing ⚠️
ui/helpers/utils/transactions.util.js	0.00%	2 Missing ⚠️
...firmations/confirm-approve/confirm-approve.util.js	66.67%	1 Missing ⚠️
...rm-transaction/confirm-token-transaction-switch.js	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #23560      +/-   ##
===========================================
- Coverage    69.13%   69.11%   -0.01%     
===========================================
  Files         1160     1160              
  Lines        44275    44296      +21     
  Branches     11844    11850       +6     
===========================================
+ Hits         30606    30615       +9     
- Misses       13669    13681      +12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

metamaskbot · 2024-03-28T09:34:23Z

Builds ready [ef4caab]

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, 2, 3, 4, 5, 6
- content-script: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1272 ± 461 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	79	377	155	71	34
		domContentLoaded	11	62	32	16	8
		load	65	2221	1272	960	461
		domInteractive	11	62	32	16	8

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

background: 47 Bytes (0.00%)
ui: 1.2 KiB (0.02%)
common: 26.64 KiB (0.53%)

metamaskbot · 2024-03-28T10:20:05Z

Builds ready [1b917dd]

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, 2, 3, 4, 5, 6
- content-script: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1147 ± 530 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	71	623	176	137	66
		domContentLoaded	10	264	43	54	26
		load	59	3129	1147	1104	530
		domInteractive	10	192	39	40	19

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

background: 47 Bytes (0.00%)
ui: 1.2 KiB (0.02%)
common: 26.64 KiB (0.53%)

metamaskbot · 2024-03-28T13:02:44Z

Builds ready [704b115]

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, 2, 3, 4, 5, 6
- content-script: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1230 ± 501 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	71	282	127	47	23
		domContentLoaded	9	83	30	21	10
		load	58	2492	1230	1044	501
		domInteractive	9	83	30	21	10

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

background: 47 Bytes (0.00%)
ui: 1.2 KiB (0.02%)
common: 497 Bytes (0.01%)

pedronfigueiredo added team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead team-confirmations-secure-ux-PR PRs from the confirmations team labels Mar 18, 2024

pedronfigueiredo self-assigned this Mar 18, 2024

pedronfigueiredo force-pushed the pnf/2224 branch 2 times, most recently from 77e338b to 4335463 Compare March 25, 2024 16:13

pedronfigueiredo force-pushed the pnf/2224 branch from 4e56dd3 to b7798b7 Compare March 26, 2024 10:24

pedronfigueiredo changed the title ~~Adds confirmation screen for 'increaseAllowance'~~ feat(): Adds confirmation screen for 'increaseAllowance' Mar 26, 2024

pedronfigueiredo marked this pull request as ready for review March 26, 2024 10:26

pedronfigueiredo requested a review from a team as a code owner March 26, 2024 10:26

jpuri previously approved these changes Mar 26, 2024

View reviewed changes

pedronfigueiredo dismissed jpuri’s stale review via fe57015 March 26, 2024 13:25

pedronfigueiredo force-pushed the pnf/2224 branch from fe57015 to b3cd7a1 Compare March 26, 2024 13:27

pedronfigueiredo requested review from a team as code owners March 26, 2024 14:00

digiwand previously approved these changes Mar 26, 2024

View reviewed changes

app/scripts/lib/transaction/metrics.ts Outdated Show resolved Hide resolved

test/e2e/tests/tokens/increase-token-allowance.spec.js Show resolved Hide resolved

pedronfigueiredo dismissed digiwand’s stale review via 0dce0f9 March 27, 2024 12:02

pedronfigueiredo requested review from a team as code owners March 27, 2024 12:02

pedronfigueiredo force-pushed the pnf/2224 branch from 0dce0f9 to 9919aca Compare March 27, 2024 12:02

pedronfigueiredo requested review from digiwand and jpuri March 27, 2024 12:53

digiwand previously approved these changes Mar 27, 2024

View reviewed changes

jpuri previously approved these changes Mar 27, 2024

View reviewed changes

segun previously approved these changes Mar 27, 2024

View reviewed changes

seaona reviewed Mar 27, 2024

View reviewed changes

pedronfigueiredo dismissed stale reviews from segun and jpuri via 17c1222 March 27, 2024 18:55

pedronfigueiredo added the release-blocker This bug is blocking the next release label Mar 27, 2024

pedronfigueiredo force-pushed the pnf/2224 branch from 7b35dba to da4c4e8 Compare March 28, 2024 08:51

pedronfigueiredo force-pushed the pnf/2224 branch from da4c4e8 to ef4caab Compare March 28, 2024 09:13

pedronfigueiredo mentioned this pull request Mar 28, 2024

cherry-pick: Adds opensea to blockaid migration BannerAlert (#23743) #23761

Merged

jpuri previously approved these changes Mar 28, 2024

View reviewed changes

pedronfigueiredo force-pushed the pnf/2224 branch from ef4caab to 1b917dd Compare March 28, 2024 09:56

pedronfigueiredo added 10 commits March 28, 2024 12:19

Adds confirmation screen for 'increaseAllowance'

7d4e9ff

update dependencies

71193c6

update e2e test

8e5ee07

fix linting

6094268

fix tests

36d0afc

clean up

128891d

fix e2e test

4e4bac3

fix e2e tests

182d537

fix e2e tests

eaa5bd1

fix e2e tests

f5eb89e

pedronfigueiredo dismissed jpuri’s stale review via f5eb89e March 28, 2024 12:28

pedronfigueiredo force-pushed the pnf/2224 branch from 1b917dd to f5eb89e Compare March 28, 2024 12:28

fix lockfile after rebase

704b115

jpuri approved these changes Mar 28, 2024

View reviewed changes

cryptotavares approved these changes Mar 28, 2024

View reviewed changes

pedronfigueiredo merged commit 860c03f into develop Mar 28, 2024
66 checks passed

pedronfigueiredo deleted the pnf/2224 branch March 28, 2024 13:06

github-actions bot locked and limited conversation to collaborators Mar 28, 2024

metamaskbot added the release-11.15.0 Issue or pull request that will be included in release 11.15.0 label Mar 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(): Adds confirmation screen for 'increaseAllowance' #23560

feat(): Adds confirmation screen for 'increaseAllowance' #23560

pedronfigueiredo commented Mar 18, 2024 •

edited

Loading

github-actions bot commented Mar 18, 2024

jpuri left a comment

pedronfigueiredo commented Mar 26, 2024

metamaskbot commented Mar 26, 2024

seaona left a comment •

edited

Loading

metamaskbot commented Mar 28, 2024

codecov bot commented Mar 28, 2024 •

edited

Loading

metamaskbot commented Mar 28, 2024

metamaskbot commented Mar 28, 2024

metamaskbot commented Mar 28, 2024

feat(): Adds confirmation screen for 'increaseAllowance' #23560

feat(): Adds confirmation screen for 'increaseAllowance' #23560

Conversation

pedronfigueiredo commented Mar 18, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Mar 18, 2024

jpuri left a comment

Choose a reason for hiding this comment

pedronfigueiredo commented Mar 26, 2024

metamaskbot commented Mar 26, 2024

seaona left a comment • edited Loading

Choose a reason for hiding this comment

metamaskbot commented Mar 28, 2024

codecov bot commented Mar 28, 2024 • edited Loading

Codecov Report

metamaskbot commented Mar 28, 2024

metamaskbot commented Mar 28, 2024

metamaskbot commented Mar 28, 2024

pedronfigueiredo commented Mar 18, 2024 •

edited

Loading

seaona left a comment •

edited

Loading

codecov bot commented Mar 28, 2024 •

edited

Loading