feat: smart-tx-small-logic

[infiniteflower]

Description

This PR only covers a smaller subset of the files from #9565 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, app/util/transactions/index.js, app/util/transactions/index.test.ts since it causes tests 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. app/components/Nav/Main/RootRPCMethodsUI.js to fix failing tests
7. @metamask+transaction-controller+13.0.0.patch (https://github.com/MetaMask/core/compare/patch/mobile-transaction-controller-13-0-0-smart-transactions)
8. yarn.lock
9. jest.config.js

Related issues

• Broken out from Feat/smart tx txcontroller v13 – DO NOT REVIEW #9565
• Functionality in this PR enabled in follow-up feat: smart tx small views #9448

Manual testing steps

Refer to #9565

Screenshots/Recordings

Refer to #9565

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]

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]

👍 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/util@9.0.3, npm/@metamask/eth-json-rpc-infura@9.1.0, npm/@metamask/smart-transactions-controller@10.0.0, npm/@metamask/transaction-controller@28.1.1, npm/@spruceid/siwe-parser@2.1.0

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/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

[infiniteflower]

@SocketSecurity ignore npm/@metamask/eth-json-rpc-infura@9.1.0
@SocketSecurity ignore npm/@ethereumjs/util@9.0.3
@SocketSecurity ignore npm/@metamask/transaction-controller@25.3.0
@SocketSecurity ignore npm/@spruceid/siwe-parser@2.1.0
@SocketSecurity ignore npm/@metamask/smart-transactions-controller@8.1.0

[legobeat]

7. patches/@MetaMask+transaction-controller+8.0.1.patch (draft: add required method to support smart transactions core#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 4a64a20

[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. 🙏

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

[infiniteflower]

Have to get the newest changes from #9565

[github-actions]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: e029bad
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

[tommasini]

Apart from the nit picks on previous comments and once the smart controller version is stored via redux!
LGTM

[socket-security]

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

Package	New capabilities	Transitives	Size	Publisher
npm/@metamask/eth-json-rpc-infura@9.1.0	None	0	44.6 kB	mcmire
npm/@metamask/eth-json-rpc-middleware@12.1.0	network	+6	884 kB	metamaskbot
npm/@metamask/smart-transactions-controller@10.0.0	network Transitive: environment, filesystem	+41	10.6 MB	mcmire
npm/@metamask/swappable-obj-proxy@2.2.0	None	0	36.9 kB	metamaskbot

🚮 Removed packages: npm/@metamask/swappable-obj-proxy@2.1.0

View full report↗︎

[github-actions]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 0529bf3
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

[sonarcloud]

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