Skip to content

fix: upgrade transaction-controller to 32.0.0 to fix mmi#24947

Merged
zone-live merged 4 commits intodevelopfrom
dbrans/txc-upgrade
Jun 5, 2024
Merged

fix: upgrade transaction-controller to 32.0.0 to fix mmi#24947
zone-live merged 4 commits intodevelopfrom
dbrans/txc-upgrade

Conversation

@dbrans
Copy link
Copy Markdown
Contributor

@dbrans dbrans commented May 31, 2024 •
edited
Loading

Note

This PR is intended to be cherry-picked into RC 11.17.0 after being merged into develop

Description

Upgrade transaction-controller to get this fix: MetaMask/core#4343

Open in GitHub Codespaces

Related issues

Fixes:
A recent update to the transaction-controller has made the TransactionMeta object passed to the afterSign hook frozen. This change prevents adding new properties, leading to the error: “Cannot add property custodyId, object is not extensible.” This bug is breaking all transactions for MMI as the original txMeta cannot store required properties like custodyId.

Blocked by #24861
Blocked by #24913

Manual testing steps

I followed these steps to test the fix:

  1. yarn && yarn start:mmi
  2. create a new wallet
  3. visit https://saturn-custody-ui.dev.metamask-institutional.io/ and add two custodial addresses
  4. send 0ETH from one custodial address to the other

You should see a popup with an Approve button. Before this fix, the transaction would appear in the activity tab with an error.

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
Copy link
Copy Markdown
Contributor

github-actions bot commented May 31, 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.

@dbrans dbrans added the team-transactions Transactions team label May 31, 2024
@dbrans dbrans force-pushed the dbrans/txc-upgrade branch from 34c4d5d to 7bf5f4a Compare May 31, 2024 18:28
@dbrans
Copy link
Copy Markdown
Contributor Author

dbrans commented May 31, 2024

@metamaskbot update-policies

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented May 31, 2024

Policies updated

@socket-security
Copy link
Copy Markdown

socket-security bot commented May 31, 2024 •
edited
Loading

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

Package New capabilities Transitives Size Publisher
npm/@metamask/nonce-tracker@5.0.0 None 0 0 B
npm/@metamask/transaction-controller@30.0.0 None +2 81.4 kB

View full report↗︎

@dbrans dbrans marked this pull request as ready for review May 31, 2024 18:47
@dbrans dbrans requested review from a team as code owners May 31, 2024 18:47
@dbrans dbrans changed the title fix: upgrade transaction-controller to fix mmi fix: upgrade transaction-controller to 31.0.0 fix mmi May 31, 2024
@dbrans dbrans mentioned this pull request May 31, 2024
Merged
7 tasks
@dbrans dbrans force-pushed the dbrans/txc-upgrade branch from b62d3a3 to c5d5dea Compare May 31, 2024 20:08
@dbrans dbrans changed the title fix: upgrade transaction-controller to 31.0.0 fix mmi fix: upgrade transaction-controller to 32.0.0 to fix mmi May 31, 2024
@dbrans
Copy link
Copy Markdown
Contributor Author

dbrans commented May 31, 2024

@metamaskbot update-policies

@legobeat
Copy link
Copy Markdown
Contributor

legobeat commented May 31, 2024 •
edited
Loading

@dbrans Can we get this rebased on top of #24861 so we can update it 29 -> 30 -> 32? :)

Would be great to have the intermediary update to 30 merged independently so changed can be observed separately, I think?

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented May 31, 2024

Policies updated

zone-live
zone-live previously approved these changes Jun 3, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@zone-live zone-live left a comment

Choose a reason for hiding this comment

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

LGTM!

@legobeat
Copy link
Copy Markdown
Contributor

legobeat commented Jun 3, 2024

@zone-live #24861

@legobeat legobeat marked this pull request as draft June 3, 2024 09:40
@zone-live
Copy link
Copy Markdown
Contributor

zone-live commented Jun 4, 2024

@metamaskbot update-policies

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jun 4, 2024

Policies updated

@zone-live
Copy link
Copy Markdown
Contributor

zone-live commented Jun 4, 2024

Hi @dbrans, I noticed it had some conflicts with develop, so I went ahead and fixed them, think it looks good 👍🏼

@dbrans dbrans marked this pull request as ready for review June 4, 2024 12:59
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jun 4, 2024

Builds ready [83d3ffa]
Page Load Metrics (54 ± 5 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6611884115
domContentLoaded9391263
load437954105
domInteractive9391263

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 4, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.71%. Comparing base (d27a233) to head (83d3ffa).
Report is 242 commits behind head on develop.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #24947      +/-   ##
===========================================
- Coverage    67.37%   65.71%   -1.66%     
===========================================
  Files         1278     1369      +91     
  Lines        49881    54397    +4516     
  Branches     12944    14155    +1211     
===========================================
+ Hits         33605    35745    +2140     
- Misses       16276    18652    +2376

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

@zone-live zone-live merged commit b210b6f into develop Jun 5, 2024
@zone-live zone-live deleted the dbrans/txc-upgrade branch June 5, 2024 09:09
@github-actions github-actions bot locked and limited conversation to collaborators Jun 5, 2024
@metamaskbot metamaskbot added release-11.18.0 release-11.16.8 Issue or pull request that will be included in release 11.16.8 and removed release-11.18.0 labels Jun 5, 2024
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jun 6, 2024

Missing release label release-11.16.8 on PR. Adding release label release-11.16.8 on PR and removing other release labels(release-11.18.0), as PR was cherry-picked in branch 11.16.8.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@zone-live zone-live zone-live approved these changes

@DDDDDanica DDDDDanica DDDDDanica approved these changes

+1 more reviewer

@albertolive albertolive albertolive approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@legobeat legobeat

Labels

release-11.16.8 Issue or pull request that will be included in release 11.16.8 team-transactions Transactions team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@dbrans @metamaskbot @legobeat @zone-live @albertolive @DDDDDanica