Cancel transaction signing from activity list by matthewwalsh0 · Pull Request #22676 · MetaMask/metamask-extension

matthewwalsh0 · 2024-01-26T17:42:22Z

Description

Currently, the TransactionController allows infinite time for a transaction to be signed.

In the case of a hardware wallet, where this is reliant on the user, this process can take a long time or never resolve if the user disconnects their device for example.

This situation results in a transaction that can never be submitted, and subsequent transactions are also blocked due to the TransactionController using locks to safely sign and submit transactions one at a time.

This can be avoided with additional and alternate UX flows however this provides a quick solution via the following:

Transactions that are being signed are displayed with the Signing status in the activity list for additional clarity.
The standard Cancel button in the activity list will now cancel the signing process for a transaction and mark it as failed.
While a transaction is being signed and submitted, subsequent transactions will display an error in the confirmation and the Confirm button will be disabled.

Temporarily using a patch for the TransactionController pending a new release.

Related issues

Fixes: #1969

Manual testing steps

Use Ledger hardware wallet.
Disconnect device while signing.
Attempt to create a new transaction.
Note error message and disabled button.
Click Cancel in activity list.
Ensure transaction is now failed with suitable error.

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.

Show signing status in activity list. Prevent confirmation if previous transactions still signing or submitting.

socket-security · 2024-01-26T17:44:27Z

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

app/_locales/en/messages.json

plasmacorral · 2024-01-27T07:15:41Z

Conducted some preliminary tests against commit 76bc1cf using both hardware and account snaps, and LGTM pending further unit tests being added.

Note that previous transaction message can be observed appearing 2x in Flask builds, but could also be related to using Ganache with ChainId=1.

Rename container property. Rename error key.

socket-security · 2024-01-29T17:35:31Z

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/jsonschema@1.4.1	None	`0`	81.8 kB	acubed

🚮 Removed packages: npm/jsonschema@1.2.4

View full report↗︎

Fix unit test. Remove Yarn lock duplication.

sleepytanya · 2024-01-30T17:55:50Z

Tested with the commit 112db7c
Works as expected on Chrome 121.0.6167.85:
Transaction displayed as signing in the Activity list:

Confirm button is disabled and appropriate error message is shown:

Failed transaction notification:

codecov · 2024-01-31T09:28:46Z

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (862617d) 68.11% compared to head (950dcc8) 68.14%.
Report is 3 commits behind head on develop.

Files	Patch %	Lines
...ction-list-item/transaction-list-item.component.js	69.23%	4 Missing ⚠️
...saction-base/confirm-transaction-base.component.js	75.00%	1 Missing ⚠️
ui/store/actions.ts	80.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #22676      +/-   ##
===========================================
+ Coverage    68.11%   68.14%   +0.02%     
===========================================
  Files         1087     1087              
  Lines        42794    42827      +33     
  Branches     11381    11398      +17     
===========================================
+ Hits         29148    29181      +33     
  Misses       13646    13646

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

metamaskbot · 2024-01-31T09:48:43Z

Builds ready [51412ae]

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
- common: 0, 1, 2, 3, 4, 5, 6, 7
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (855 ± 29 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	101	155	122	14	7
		domContentLoaded	9	48	20	11	5
		load	755	937	855	61	29
		domInteractive	9	48	20	11	5

Bundle size diffs [🚀 Bundle size reduced!]

background: -24.55 KiB (-0.73%)
ui: 1.33 KiB (0.02%)
common: 589 Bytes (0.01%)

montelaidev · 2024-01-31T12:22:27Z

Tested as well with the SSK and Safeheron snap

Screen.Recording.2024-01-31.at.20.19.50.mov

Screen.Recording.2024-01-31.at.20.20.53.mov

app/_locales/en/messages.json

ui/components/app/cancel-button/cancel-button.js

...rm-page-container/confirm-page-container-content/confirm-page-container-content.component.js

ui/helpers/constants/error-keys.js

ui/pages/confirm-transaction-base/confirm-transaction-base.component.js

ui/selectors/transactions.js

ui/store/actions.ts

metamaskbot · 2024-01-31T17:59:33Z

Builds ready [950dcc8]

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
- common: 0, 1, 2, 3, 4, 5, 6, 7
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (935 ± 182 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	92	255	132	49	23
		domContentLoaded	9	78	22	17	8
		load	688	2056	935	378	182
		domInteractive	9	78	22	17	8

Bundle size diffs [🚀 Bundle size reduced!]

background: -24.55 KiB (-0.73%)
ui: 1.33 KiB (0.02%)
common: 589 Bytes (0.01%)

Support cancellation of transaction signing from UI

76bc1cf

Show signing status in activity list. Prevent confirmation if previous transactions still signing or submitting.

matthewwalsh0 added the team-confirmations-system-deprecated DEPRECATED: please use "team-confirmations" instead label Jan 26, 2024

sallykhomikh reviewed Jan 26, 2024

View reviewed changes

app/_locales/en/messages.json Outdated Show resolved Hide resolved

Hide speed up button if signing or submitting

ced2a81

Add unit tests

6d0d261

Rename container property. Rename error key.

matthewwalsh0 added 3 commits January 30, 2024 10:00

Update patch

0715204

Update yarn lock

526b884

Update Lavamoat policies

112db7c

Fix unit test. Remove Yarn lock duplication.

matthewwalsh0 marked this pull request as ready for review January 30, 2024 10:24

matthewwalsh0 requested review from a team as code owners January 30, 2024 10:24

AlexJupiter added team-accounts-framework Accounts team Priority - High labels Jan 31, 2024

Merge branch 'develop' into fix/1969-cancel-transaction-signing

51412ae

matthewwalsh0 added 2 commits January 31, 2024 17:25

Update transaction controller

5835578

Remove patch

950dcc8

vinistevam approved these changes Jan 31, 2024

View reviewed changes

OGPoyraz approved these changes Jan 31, 2024

View reviewed changes

dbrans reviewed Jan 31, 2024

View reviewed changes

sleepytanya added the QA Passed label Jan 31, 2024

matthewwalsh0 merged commit 7a3d9e2 into develop Jan 31, 2024

matthewwalsh0 deleted the fix/1969-cancel-transaction-signing branch January 31, 2024 18:31

github-actions bot locked and limited conversation to collaborators Jan 31, 2024

metamaskbot added the release-11.11.0 Issue or pull request that will be included in release 11.11.0 label Jan 31, 2024

Conversation

matthewwalsh0 commented Jan 26, 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!

socket-security bot commented Jan 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

plasmacorral commented Jan 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

socket-security bot commented Jan 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sleepytanya commented Jan 30, 2024

Uh oh!

codecov bot commented Jan 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

metamaskbot commented Jan 31, 2024

Uh oh!

montelaidev commented Jan 31, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

metamaskbot commented Jan 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

matthewwalsh0 commented Jan 26, 2024 •

edited

Loading

socket-security bot commented Jan 26, 2024 •

edited

Loading

plasmacorral commented Jan 27, 2024 •

edited

Loading

socket-security bot commented Jan 29, 2024 •

edited

Loading

codecov bot commented Jan 31, 2024 •

edited

Loading