Snap publishing v0.1 by rekmarks · Pull Request #140 · MetaMask/snaps

rekmarks · 2021-11-24T00:12:21Z

This PR updates all skunkworks packages to match the new Snaps publishing specification. Changes are predominantly made in /controllers and /snaps-cli.

ATTN: Reviewers should read the publishing specification before continuing.

After reading that and this description, I recommend starting with the changes in controllers/snaps, and moving on to snaps-cli afterwards.

In broad strokes, we had to make the following changes to become compatible with the new publishing specification:

Migrate from snap "names" to "ids" (Rename Snap "name" property to "id" #147)
Handle the new Snap manifest file format
Fetch Snaps from npm or locally, via localhost

All fetching and manifest validation logic is implemented in controllers/snaps/, and relevant utilities are imported therefrom in snaps-cli. There are a number of utilities that should probably be migrated to their own package, but we will do that later.

Changes in detail:

The SnapController changes are mostly isolated to the Snap installation functionality, namely installSnaps, processRequestedSnap, and add.
controllers/src/snaps/json-schemas includes the JSON Schemas for Snap package.json and snap.manifest.json files.
- We use ajv ("another json schema validator") to precompile validation functions that we use in production. This is done in a build step. Thanks to @shanejonas for suggesting this approach, which saved a ton of effort.
The snaps-cli changes are mostly isolated to src/cmds/init and src/cmds/manifest. Utilities are imported from /controllers whenever possible.

Closes: https://github.com/MetaMask/MetaMask-planning/issues/54

shanejonas · 2021-12-01T20:21:15Z

@@ -0,0 +1,79 @@
+/**


should we check this in? or have it as a postinstall

I'm not completely sure how to think about it. It seems harmless to check in? CI will blow up if it changes unexpectedly. I think there is something nice about having it recorded in the git history as well.

Yea. I'm not partial to either or.

I know golang culture is totally fine with checked in generated code. In JS culture I've found people like to keep the repo tree clean of generated files.

In Golang the convention is to put a comment at the top to warn people not to edit checked-in generated code:

// Code generated by stringer -type Pill pill.go; DO NOT EDIT.

Done in: e9acf13

In preparation for #140, renames the snap name property to id, and updates all variable names and comments to match. Also tweaks some docstrings.

* Add shasum validation, compile ajv validator at build time * Update manifest schema * Update snaps-cli readme * Generate SnapManifest type from the JSON Schema * Add npm Snap package.json schema and types * Add example-snap package * Implement Snap installation from npm * Update package.json schema * Remove unused dependencies * Update controllers tsconfig and build process * Fix example-snap lint script * Fix test fixture generation * Woe to the vanquished (TypeScript interfaces)

Removes the "inline" Snap functionality from the `SnapController`, also in preparation for launch / #140.

Co-authored-by: Shane <jonas.shane@gmail.com>

Removes the "inline" Snap functionality from the `SnapController`, also in preparation for launch / #140.

@shanejonas

This PR updates all skunkworks packages to match the [new Snaps publishing specification](https://github.com/MetaMask/specifications/blob/87f5e2a29235b174b897e002ca202a66fded9ddc/snaps/publishing.md). Changes are predominantly made in `/controllers` and `/snaps-cli`. In broad strokes, we had to make the following changes to become compatible with the new publishing specification: - Migrate from snap "names" to "ids" (#147) - Handle the new Snap manifest file format - Fetch Snaps from npm _or_ locally, via `localhost` All fetching and manifest validation logic is implemented in `controllers/snaps/`, and relevant utilities are imported therefrom in `snaps-cli`. There are a number of utilities that should probably be migrated to their own package, but we will do that later. Changes in detail: - The `SnapController` changes are mostly isolated to the Snap installation functionality, namely `installSnaps`, `processRequestedSnap`, and `add`. - `controllers/src/snaps/json-schemas` includes the JSON Schemas for Snap `package.json` and `snap.manifest.json` files. - We use `ajv` ("another json schema validator") to precompile validation functions that we use in production. This is done in a build step. Thanks to @shanejonas for suggesting this approach, which saved a ton of effort. - The `snaps-cli` changes are mostly isolated to `src/cmds/init` and `src/cmds/manifest`. Utilities are imported from `/controllers` whenever possible. Closes: https://github.com/MetaMask/MetaMask-planning/issues/54

This PR adds installation-related `ControllerMessenger` events to the `SnapController`, fixes a bug in `installSnaps`, makes some methods private, and deletes `runExistingSnaps`. The following events are added: - `snapAdded`, emitted when a Snap has been added to state and its status is `installing` - We will leverage this event to record metadata about the Snap in the extension. - `snapInstalled`, emitted when a Snap has been fully installed - `snapRemoved`, emitted when a Snap is removed A bug was introduced in #140, where a successfully installed Snap would be overwritten with an unrelated error in the `installSnaps` result object. This PR prevents this overwrite from happening. The `authorize` and `processRequestedSnap` methods are made private, since they should never be called by third parties. In truth, `add` should also be private, but we currently rely on it for the majority of our tests, and will attend to it later. Finally, the `runExistingSnaps` method is removed because it's a hangover from the old Snaps lifecycle.

rekmarks self-assigned this Nov 24, 2021

rekmarks force-pushed the snap-publishing-v0.1 branch from 5e58f7f to b78bf59 Compare November 24, 2021 01:48

shanejonas reviewed Nov 24, 2021

View reviewed changes

Comment thread packages/types/index.d.ts Resolved

rekmarks force-pushed the snap-publishing-v0.1 branch 11 times, most recently from e8f277c to 77b13ee Compare November 29, 2021 08:08

rekmarks commented Nov 29, 2021

View reviewed changes

Comment thread packages/controllers/package.json Resolved

shanejonas reviewed Dec 1, 2021

View reviewed changes

Comment thread packages/snaps-cli/README.md Resolved

shanejonas reviewed Dec 1, 2021

View reviewed changes

Comment thread packages/controllers/src/snaps/json-schemas/SnapManifest.ts Resolved

rekmarks force-pushed the snap-publishing-v0.1 branch 10 times, most recently from a4be02d to 1e68d57 Compare December 8, 2021 04:31

rekmarks mentioned this pull request Dec 8, 2021

Rename Snap "name" property to "id" #147

Merged

rekmarks force-pushed the snap-publishing-v0.1 branch from 5620836 to 6b72d4f Compare December 8, 2021 05:28

rekmarks changed the base branch from main to snapName-to-snapId December 8, 2021 05:28

rekmarks force-pushed the snap-publishing-v0.1 branch from 6b72d4f to d3a24da Compare December 8, 2021 05:37

rekmarks requested a review from shanejonas December 8, 2021 06:05

Base automatically changed from snapName-to-snapId to main December 8, 2021 06:27

rekmarks added a commit that referenced this pull request Dec 8, 2021

Rename Snap "name" property to "id" (#147)

4631e97

In preparation for #140, renames the snap name property to id, and updates all variable names and comments to match. Also tweaks some docstrings.

rekmarks added 2 commits December 7, 2021 22:28

Implement snaps-cli updates, various fixes (squashed)

1bb02ff

rekmarks force-pushed the snap-publishing-v0.1 branch from d3a24da to 1bb02ff Compare December 8, 2021 06:28

rekmarks marked this pull request as ready for review December 8, 2021 06:28

rekmarks mentioned this pull request Dec 8, 2021

Remove inline Snap functionality #148

Merged

Use pump instead of Node internals

97aae60

rekmarks added a commit that referenced this pull request Dec 8, 2021

Remove inline snap functionality (#148)

dc3832a

Removes the "inline" Snap functionality from the `SnapController`, also in preparation for launch / #140.

rekmarks added 2 commits December 8, 2021 10:35

Merge branch 'main' into snap-publishing-v0.1

3ff5a1c

Merge branch 'main' into snap-publishing-v0.1

3adea86

shanejonas reviewed Dec 8, 2021

View reviewed changes

Comment thread packages/controllers/src/snaps/SnapController.ts Resolved

shanejonas reviewed Dec 8, 2021

View reviewed changes

Comment thread packages/example-snap/.eslintrc.js Resolved

shanejonas reviewed Dec 8, 2021

View reviewed changes

Comment thread packages/controllers/src/snaps/json-schemas/snap-manifest.schema.json Resolved

rekmarks mentioned this pull request Dec 8, 2021

Fix restricted Snap method implementations #150

Merged

rekmarks and others added 4 commits December 9, 2021 12:04

Update packages/controllers/src/snaps/SnapController.ts

e808a95

Co-authored-by: Shane <jonas.shane@gmail.com>

fixup! Update packages/controllers/src/snaps/SnapController.ts

26d69eb

Merge branch 'main' into snap-publishing-v0.1

4ea4cc7

Fixup

e9acf13

shanejonas approved these changes Dec 9, 2021

View reviewed changes

rekmarks merged commit 97f83a0 into main Dec 9, 2021

rekmarks deleted the snap-publishing-v0.1 branch December 9, 2021 21:07

rekmarks added a commit that referenced this pull request Dec 10, 2021

Remove inline snap functionality (#148)

a8c587d

Removes the "inline" Snap functionality from the `SnapController`, also in preparation for launch / #140.

rekmarks mentioned this pull request Dec 15, 2021

Add Snap installation events #162

Merged

Navigation Menu

Repository navigation

Snap publishing v0.1 - #140

Snap publishing v0.1#140
rekmarks merged 9 commits into
mainfrom
snap-publishing-v0.1

rekmarks commented Nov 24, 2021 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

shanejonas Dec 1, 2021

Uh oh!

rekmarks Dec 8, 2021

Uh oh!

shanejonas Dec 8, 2021

Uh oh!

rekmarks Dec 9, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

rekmarks commented Nov 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shanejonas Dec 1, 2021

Choose a reason for hiding this comment

Uh oh!

rekmarks Dec 8, 2021

Choose a reason for hiding this comment

Uh oh!

shanejonas Dec 8, 2021

Choose a reason for hiding this comment

Uh oh!

rekmarks Dec 9, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rekmarks commented Nov 24, 2021 •

edited

Loading