Skip to content

feat: Snaps execution refactor#8369

Closed
Jonathansoufer wants to merge 23 commits into
mainfrom
fb/snaps-execution-refactor
Closed

feat: Snaps execution refactor#8369
Jonathansoufer wants to merge 23 commits into
mainfrom
fb/snaps-execution-refactor

Conversation

@Jonathansoufer

@Jonathansoufer Jonathansoufer commented Jan 23, 2024

Copy link
Copy Markdown
Contributor

Description

This PR adds a refactored Snaps Webview approach and embedded execution environment to allow mobile app consume the new mobile snaps architecture.

Related issues

Fixes: N/A

Manual testing steps

First --> Copy snaps_controllers files to metamask-mobile repo

  1. Clone snaps monorepo and run yarn on root dir.
  2. navigate to packages/snaps-controller and run: yarn build:source && yarn build
  3. open the dist directory and copy the service folder from cjs, esm, and types and paste inside node_modules/@metamask/snaps-controller (cjs, esm, and types) on metamask-mobile repo.

Second --> Start the execution environment locally

  1. Inside the snaps monorepo cloned above, navigate to: packages/snaps-execution-environments.
  2. run: yarn build:lavamoat:policy && yarn build:lavamoat && yarn build:source && yarn build
  3. Modify scripts/start.js from const PUBLIC = path.join(ROOT, '/dist/browserify/iframe'); to const PUBLIC = path.join(ROOT, '/dist/browserify/webview');
  4. run: yarn start

Third:

  1. Modify app/lib/snaps/SnapsExecutionWebView.tsx webview source to point to http://localhost:6363.
  2. Open the mobile app
  3. Navigate to explore session and access snaps example page
  4. add any snap
  5. go to snaps item on settings
  6. check if the installed snap is listed.
Screen.Recording.2024-01-31.at.10.05.48.mov

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • 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.
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

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.

CI Related Section: DO NOT EDIT

Flag used by Bitrise to either run or skip E2E smoke tests

  • Bitrise Flag:[skip ci]

@Jonathansoufer Jonathansoufer self-assigned this Jan 23, 2024
@github-actions

Copy link
Copy Markdown
Contributor

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.

@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Jan 23, 2024
@Jonathansoufer Jonathansoufer marked this pull request as ready for review January 31, 2024 10:13
@Jonathansoufer Jonathansoufer requested a review from a team as a code owner January 31, 2024 10:13
@github-actions

Copy link
Copy Markdown
Contributor

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/19a1c25f-217e-4dc7-8ad7-0cf279f77ea6
You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

@Jonathansoufer Jonathansoufer changed the title Fb/snaps execution refactor feat: Snaps execution refactor Jan 31, 2024
@Jonathansoufer Jonathansoufer added team-mobile-client and removed INVALID-PR-TEMPLATE PR's body doesn't match template labels Jan 31, 2024
@owencraston

Copy link
Copy Markdown
Contributor

I tested https://metamask.github.io/snaps/test-snaps/latest/ and I kept getting this error on iOS. This occurs on first load and when you try to install a snap. This also occurred with version 1.0.1 of the test snap.
Simulator Screenshot - iPhone 13 Pro - 2024-01-31 at 11 57 26
Simulator Screenshot - iPhone 13 Pro - 2024-01-31 at 12 01 47

@owencraston owencraston left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How were these web.bundle files generated? is there a way for us to create a script so that it automatically loads the bundled execution env code into the app so that we do not need to manually update it?

@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Jan 31, 2024
@gauthierpetetin gauthierpetetin added team-snaps-deprecated DEPRECATED: please use "team-core-platform" instead team-mobile-platform Mobile Platform team and removed team-mobile-client labels Feb 1, 2024
@sethkfman sethkfman removed the team-mobile-platform Mobile Platform team label Feb 6, 2024
Jonathansoufer added a commit that referenced this pull request Feb 27, 2024
## **Description**

This PR bumps snaps packages alongside others in order to allow snaps
new architecture and refactored code from snaps codebase be used into
mobile as an external dependency.

The current PR builds the bases to the
[8369](#8369). **Any
failing tests related to snaps can be safely ignored** since the snaps
"consumer" code is/will be handled/refactored on the PR above.

## **Related issues**

Fixes: N/A

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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.

---------

Co-authored-by: Owen Craston <owengc12@gmail.com>
@Jonathansoufer

Copy link
Copy Markdown
Contributor Author

Closing in favor of #8700 already merged.

@github-actions github-actions Bot locked and limited conversation to collaborators Mar 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

INVALID-PR-TEMPLATE PR's body doesn't match template mobile-snaps team-snaps-deprecated DEPRECATED: please use "team-core-platform" instead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants