feat: smart-tx-small-logic

[infiniteflower]

Description

This PR only covers a smaller subset of the files from https://github.com/MetaMask/metamask-mobile/pull/9315/ for ease of reviewing. Please refer to that issue for videos. This PR is mostly for the core logical parts of Smart Transactions (STX). This PR can be merged as it won't change any existing behavior in the application without a follow up PR to actually use these utility functions.

It covers the following areas

1. /app/util
   • except app/util/test/initial-background-state.json since it causes a test to fail
2. /app/images
3. /e2e/selectors
4. app/core/RPCMethods/RPCMethodMiddleware.ts (to fix tests)
5. package.json (Added smart-transactions-controller to this PR to fix failing tests)
6. patches/@metamask+smart-transactions-controller+8.1.0.patch (https://github.com/MetaMask/smart-transactions-controller/compare/main...patch/mobile-stx-v8.1.0-tx-v8)
7. patches/@metamask+transaction-controller+8.0.1.patch (https://github.com/MetaMask/core/pull/4134)
8. yarn.lock
9. jest.config.js

Related issues

• Broken out from https://github.com/MetaMask/metamask-mobile/pull/9565
• Functionality in this PR enabled in follow-up https://github.com/MetaMask/metamask-mobile/pull/9448

Manual testing steps

Refer to https://github.com/MetaMask/metamask-mobile/pull/9315/

Screenshots/Recordings

Refer to https://github.com/MetaMask/metamask-mobile/pull/9315/

Before

After

Pre-merge author checklist

• [x] I’ve followed MetaMask Coding Standards.
• [x] I've completed the PR template to the best of my ability
• [x] I’ve included tests if applicable
• [x] I’ve documented my code using JSDoc format if applicable
• [x] 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[bot]]

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.

[socket-security[bot]]

👍 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.

Ignoring: npm/@ethereumjs/[email protected], npm/@metamask/[email protected], npm/@metamask/[email protected], npm/@metamask/[email protected], npm/@spruceid/[email protected]

View full report↗︎

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

[infiniteflower]

@SocketSecurity ignore npm/@metamask/[email protected] @SocketSecurity ignore npm/@ethereumjs/[email protected] @SocketSecurity ignore npm/@metamask/[email protected] @SocketSecurity ignore npm/@spruceid/[email protected] @SocketSecurity ignore npm/@metamask/[email protected]

[legobeat]

7. patches/@metamask+transaction-controller+8.0.1.patch (https://github.com/MetaMask/core/pull/4134)

That PR is still in draft and marked as DO-NO-NOT-MERGE. What is the outstanding items remaining for that to be considered ready?

cf 4a64a20593b289b6a94e632ac83fcaf14ca77c44

[infiniteflower]

@legobeat That's a patch for an old version of the TransactionController (v8). It back ports methods from the most recent version of the TransactionController to support Smart Transactions. So all the work that it contains is technically already "in" the TransactionController library.

Perhaps @vinistevam can provide some more context as to what happens to patch PRs for the TransactionController.

[legobeat]

Yes, I was thinking perhaps the intention was to actually backport the changes in that PR for an 8.1.0 release.. But perhaps that is not the case?

I'm noting some divergence between the functions added here and their correspondents in upstream @metamask/transaction-controller, e.g. approveTransactionsWithSameNonce requires chainId in upstream but does not recognize the parameter here.

[legobeat]

An alternative approach to this would be to first upgrade @metamask/transaction-controller to a later version that supports this feature, and then implement this feature once metamask-mobile as actually using a version of the library that supports it.

Just curious if this was considered and if there is some reason why the implementing towards the legacy version is preferred?

That is to say, why not let this feature come after merge of #9088 and base the work on that branch? That would mean using a base of @metamask/transaction-controller v13 instead of v8, and would make part of the introduced patching redundant.

[vinistevam]

hey @legobeat , I don't have much context on the decision of the approach but I think this was considered, and the work on upgrading the TransactionController started in v5 or v6. Still, there were several dependencies from upgrading the rest of the controllers, so to unblock smart transactions those methods were added in a separate branch(patch). We have been upgrading the patch as necessary when we move to a new version of the TransactionController. I think the diversion that you mentioned was introduced in v23 and it was not updated in the patch. The PR is a reference to see what is in the patch and once the development is ready, reviewed and tested we can merge it into the patch/mobile-transaction-controller-8-0-1 but in the case we already have the upgrade of v13 in review/tests which we will need to update with the necessary changes the patch (patch/mobile-transaction-controller-13-0-0) depending on what PR goes in first.

I hope it clarified a bit more around the patch, let me know if you have any questions. :pray:

[infiniteflower]

@legobeat Initially we didn't want to be dependent on a specific version of the TransactionController so the Confirmations team would just support STX work by patching whatever the current version of TransactionController Mobile is at. This would allow the STX feature development team to move faster rather than rely on the timelines of the version upgrades.

[legobeat]

@legobeat Initially we didn't want to be dependent on a specific version of the TransactionController so the Confirmations team would just support STX work by patching whatever the current version of TransactionController Mobile is at. This would allow the STX feature development team to move faster rather than rely on the timelines of the version upgrades.

Got it.

I guess it seemed more like it would make things smoother and more predictable to build on top of that upgrade from the get-go and share efforts and progress.

In any case, #9088 has now been merged so this PR will need to be rebased on main.

[infiniteflower]

Have to get the newest changes from https://github.com/MetaMask/metamask-mobile/pull/9565

[infiniteflower]

@SocketSecurity ignore npm/@metamask/[email protected] @SocketSecurity ignore npm/@metamask/[email protected]

[infiniteflower]

This has the latest changes from #9565, should be ready for reviews again

[github-actions[bot]]

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: b2877a61cead5e8e4d9f1818528c6d5d24a046da Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/692b4f3c-9469-469f-98a4-ce5351f565cc

[!NOTE]

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[github-actions[bot]]

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 8526be716de99823b33582b9d24a0c2d16198607 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/89e6b00c-4153-46f0-91dc-0a56d018c2d9

[!NOTE]

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[github-actions[bot]]

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: c74c28118edcff2814a0442ed6d1902c3b9a7109 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/b434282f-e27f-4868-ac11-fec1669ecd34

[!NOTE]

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[github-actions[bot]]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: e029bad4f8e36571d6eb4cdf9341e3263b73720c Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/548a4548-48d3-4a77-8975-a0486103e1b8

[!NOTE]

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[socket-security[bot]]

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

Package	New capabilities	Transitives	Size	Publisher
npm/@metamask/[email protected]	None	0	44.6 kB	mcmire
npm/@metamask/[email protected]	network	+6	884 kB	metamaskbot
npm/@metamask/[email protected]	network Transitive: environment, filesystem	+41	10.6 MB	mcmire
npm/@metamask/[email protected]	None	0	36.9 kB	metamaskbot

🚮 Removed packages: npm/@metamask/[email protected]

View full report↗︎

[github-actions[bot]]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 0529bf32a2252cc3ffd6c24677f7964906e95374 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/7fb063a3-79a2-4fca-bf83-ed99bec0db7e

[!NOTE]

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[sonarqubecloud[bot]]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

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

See analysis details on SonarCloud