fix: use transactionMeta for TransactionController events by ccharly · Pull Request #24003 · MetaMask/metamask-extension

ccharly · 2024-04-12T15:34:05Z

Description

Use of a wrong field name which ends up as an error once published.

Related issues

Fixes: https://github.com/MetaMask/accounts-planning/issues/359

Manual testing steps

Create a 4337 account using: https://metamask.github.io/snap-account-abstraction-keyring/latest/

You might need to change the EIP_4337_ENTRYPOINT in your .metamaskrc or builds.yml (or he might already be up-to-date)

Send some ETH to your newly created account
Try to send back some ETH to any account

The transaction should now succeed (it was failing before this fix)

Screenshots/Recordings

Before

After

Pre-merge author checklist

I’ve followed MetaMask 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-04-12T15:34:18Z

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.

mikesposito

Is there a way to test that the _onUserOperationTransactionUpdated function does what it is expected to do?

The _ suggests that it's intended as private function and we don't usually add tests for private functions directly, but perhaps we could test that the transaction-updated event is handled properly?

ccharly · 2024-04-15T12:16:37Z

I must say that I don't really know 😅

We did a small investigation with @montelaidev and we found out what was this error. We were doing manual tests though...

I don't know if the e2e tests should cover this one (implicitly)... Any thought regarding the best approach to test this @matthewwalsh0?

mcmire

Good catch. I must have missed this in the TransactionController upgrade in #23252.

montelaidev

Tested with @ccharly

…ntroller events

metamaskbot · 2024-04-24T16:10:13Z

Builds ready [d69b91c]

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

Page Load Metrics (818 ± 551 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	57	173	101	29	14
		domContentLoaded	9	67	24	14	7
		load	46	2717	818	1146	551
		domInteractive	9	67	24	14	7

Bundle size diffs [🚀 Bundle size reduced!]

background: -7 Bytes (-0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

codecov · 2024-04-25T02:47:58Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.28%. Comparing base (9cd8c79) to head (f06f660).
Report is 1 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #24003   +/-   ##
========================================
  Coverage    67.28%   67.28%           
========================================
  Files         1275     1275           
  Lines        49707    49707           
  Branches     12916    12916           
========================================
  Hits         33444    33444           
  Misses       16263    16263

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

metamaskbot · 2024-04-30T07:41:59Z

Builds ready [f06f660]

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

Page Load Metrics (1675 ± 830 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	84	583	155	102	49
		domContentLoaded	11	301	43	61	29
		load	67	4117	1675	1729	830
		domInteractive	11	301	43	61	29

Bundle size diffs [🚀 Bundle size reduced!]

background: -7 Bytes (-0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

k-g-j · 2024-05-21T17:28:46Z

Testing on Chrome Version 125.0.6422.61 with the Flask build linked here

❌ The extension shows a forever loading screen when I go to try and send funds from my 4337 Account back to my main EOA account.

Screen.Recording.2024-05-21.at.1.24.40.PM.mov

k-g-j · 2024-06-12T17:14:14Z

Using Flask build 12.0.0 046766c1

Testing on Chrome Version 126.0.6478.57 ✅

pr-24003-chrome-validation.mov

ccharly added type-bug Something isn't working area-transactions labels Apr 12, 2024

ccharly requested a review from a team as a code owner April 12, 2024 15:34

ccharly added the team-shared-libraries label Apr 12, 2024

mikesposito reviewed Apr 15, 2024

View reviewed changes

mcmire reviewed Apr 19, 2024

View reviewed changes

montelaidev approved these changes Apr 23, 2024

View reviewed changes

fix(user-operation-controller): use transactionMeta for TransactionCo…

d69b91c

…ntroller events

ccharly force-pushed the fix/user-ops-transaction-meta-events branch from d2c84b7 to d69b91c Compare April 24, 2024 15:38

matthewwalsh0 approved these changes Apr 29, 2024

View reviewed changes

ccharly and others added 3 commits April 29, 2024 17:14

Merge branch 'develop' into fix/user-ops-transaction-meta-events

4a0d50f

Merge branch 'develop' into fix/user-ops-transaction-meta-events

917c82b

Merge branch 'develop' into fix/user-ops-transaction-meta-events

f06f660

ccharly merged commit 8a0e455 into develop Apr 30, 2024

ccharly deleted the fix/user-ops-transaction-meta-events branch April 30, 2024 07:48

github-actions bot locked and limited conversation to collaborators Apr 30, 2024

metamaskbot added the release-11.17.0 label Apr 30, 2024

gauthierpetetin added the release-12.0.0 Issue or pull request that will be included in release 12.0.0 label Jun 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: use transactionMeta for TransactionController events#24003

fix: use transactionMeta for TransactionController events#24003
ccharly merged 4 commits intodevelopfrom
fix/user-ops-transaction-meta-events

ccharly commented Apr 12, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Apr 12, 2024

Uh oh!

mikesposito left a comment •

edited

Loading

Uh oh!

ccharly commented Apr 15, 2024

Uh oh!

mcmire left a comment

Uh oh!

montelaidev left a comment

Uh oh!

metamaskbot commented Apr 24, 2024

Uh oh!

codecov bot commented Apr 25, 2024 •

edited

Loading

Uh oh!

metamaskbot commented Apr 30, 2024

Uh oh!

k-g-j commented May 21, 2024 •

edited

Loading

Uh oh!

k-g-j commented Jun 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Conversation

ccharly commented Apr 12, 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 Apr 12, 2024

Uh oh!

mikesposito left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ccharly commented Apr 15, 2024

Uh oh!

mcmire left a comment

Choose a reason for hiding this comment

Uh oh!

montelaidev left a comment

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Apr 24, 2024

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

metamaskbot commented Apr 30, 2024

Uh oh!

k-g-j commented May 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k-g-j commented Jun 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

ccharly commented Apr 12, 2024 •

edited

Loading

mikesposito left a comment •

edited

Loading

codecov bot commented Apr 25, 2024 •

edited

Loading

k-g-j commented May 21, 2024 •

edited

Loading