was reading the node.js process docs and theres an example that says not to log to stdout + process.exit right after

https://github.com/MetaMask/snaps-cli/blob/f7976cd6e23e61007bcc8065ff2f0f1fa98e0093/src/cmds/init/initUtils.ts#L42-L47

docs link: https://nodejs.org/api/process.html#process_process_exit_code

it recommends using

process.exitCode = 1;

#105 introduces a change where a new function is instantiated whenever an RPC request is received for a Snap. To avoid this, we should replace getRpcMessageHandler with a new method, handleRpcMessageFor, that accepts the RPC request object, gets the RPC message handler from the execution environment service (erroring if it's not found), performs any bookkeeping we want to do, and returns the result of the handler function.

Snaps should have a boolean state property called enabled. This indicates whether the snap is allowed to be started. We plan to allow users to toggle this state via snap settings in the UI, and potentially set snaps that crash repeatedly to disabled.

snaps-cli should be compatible with Windows. This is mostly of ensuring that it can handle both the Windows and *nix filesystems. Currently, paths are hardcoded to assume a Unix platform.

Per #56, we need to monitor snap busy and idle time. A snap is busy if it's processing a request, and idle if it's not. A snap is "processing a request" during the interval between receiving a request and responding to it.

The manifest command should warn or error if the web3Wallet field is missing from package.json.

We should standardize all common snap-related errors. This means that we should choose error codes and standard messages, and probably add them to our eth-rpc-errors library. If we can reuse existing error standards, we should do so.

Before we start an implementation, a proposal should be drafted (in written form, anywhere) of the different kinds of errors that exist and what error codes and messages they should have.

Includes the permission to:

• #107
  • MetaMask/key-tree#8
  • MetaMask/key-tree#23
• #108
• #109
  • i.e. get some key in localStorage
  • Unencrypted for MVP
  • Data stored as string, limited by length
• #110
  • i.e. use window.fetch
  • In the future: limit by origin
  • This requires changes to the execution environment packages so that we can programmatically include / exclude the window.fetch global.

These should be implemented as restricted methods in @mm-snap/rpc-methods.

The SnapsController (and execution environment services) has a series of maps that each point to the snaps that it manages.

This means when we want to do things like unregister event listeners, we end up going down each of these maps, and unsetting them, and in general it means that data that really is per-snap is sprawled across the code that applies to the entire snap controller.

I think there is a cleanup/refactor we could do where we create a Snap class that represents a single initialized snap. This would allow us to keep initialization and teardown steps very neatly isolated to the object being handled, and makes it easier to make further changes to our model (for example, if we decide to initialize a snap per persona).

This isn't urgent, but it's a refactor that I've consistently found myself wanting.

We should handle non-terminal errors from plugins, i.e. errors that bubble up to the parent realm but do not stop the plugin from executing, for example errors thrown in a setTimeout handler.

These errors should be logged such that they can be reported to plugin developers.

We should handle errors from the plugin that are re-thrown by Compartment.evaluate. These errors will always stop plugin execution, and single-plugin workers should be terminated when they occur.

It says metamask/snaps-cli but it should be metamask/snaps-examples
https://github.com/MetaMask/snaps-skunkworks/blob/1fcb27a3a77d184d5e2e6546a9953acb9853bf30/packages/snap-examples/package.json#L15

Error: Error: "root" manifest "version" is not a valid SemVer version: undefined

Source: https://github.com/MetaMask/snaps-skunkworks/runs/3293659137?check_suite_focus=true

Related: MetaMask/metamask-snaps-beta#135

Date and other platform APIs can provide subtle timers. We should make a decent effort to constrain them, and then process-separate as possible.

TODO

• #58
• #62
• #63
• #113

Future TODO

• #59
• #117

Specification

• Under normal circumstances, MetaMask will never terminate a snap while it's processing a request, before the processing time limit has expired.
  • The processing time in v1 of the MVP will be on the order of 30-60 seconds.
• MetaMask may terminate a snap immediately after the processing time limit has expired, or once processing has completed, which we define as when MetaMask observes the snap sending a response to a particular request.
• Under abnormal circumstances, MetaMask may be forced to terminate a snap while it's processing a request, but will only do so if absolutely necessary.
  • Such circumstances include:
    • If the snap is consuming too many resources, where the threshold for "too many" may vary with the number of snaps running concurrently, and other factors.
    • If the snap "hangs" and its worker becomes unresponsive.
    • If the user force-quits the snap.
• Regardless of when and how a snap is terminated, it should gracefully start back up.
  • If a snap's state becomes so corrupted that it cannot be restarted, the user may have limited options for resolving the matter, short of reinstalling the snap.

We should implement the "SES shim in iframe/WebView execution environment", as described here.

• Implement execution environment (https://github.com/MetaMask/iframe-execution-environment)
• Implement Iframe Execution Environment Service in Skunkworks
• Integrate execution environment with extension

Per #56 and #62, we should continually monitor running snaps, and terminate them when:

• they've been idle for a certain period of time (pulling a number out of a hat, on the order of 2 < 10 minutes)
• they and/or their workers aren't responding to health checks for a certain period of time (on the order of 5 - 10 seconds)

After the MVP, we should either migrate the packages in this repo to the MetaMask npm org (as their own packages or as part of existing ones), or delete them:

• @mm-snaps/controllers: #12
• @mm-snaps/post-message-stream
  • Merge into the regular post-message-stream.
• @mm-snaps/workers
  • Make compatible with above WebWorkerController changes.
• @mm-snaps/rpc-methods
  • We can't do anything with this until after the controllers have been migrated. It may serve as a useful point of reference for the methods that the controllers will need to provide.
  • It should probably continue to be its own package.
• @mm-snaps/types
  • Determine whether this package makes sense after everything else is migrated or if we should just get rid of it.

To prevent weird JSON mis-serialization issues, as well as other unexpected behavior.

There is a lot of duplication in code between the WebWorkerExecutionEnvironmentService and IframeExecutionEnvironmentService classes. We should create a single, abstract base class and move all duplicated code into it. In addition, we may want to consider changing ExecutionEnvironmentService to ExecutionService for brevity.

• @mm-snaps/controllers
  • All controllers should be migrated to the v2 BaseController and moved to @metamask/controllers. (This is a longer-term item.)
  • PluginController
    • Define a generic, asynchronous interface for communicating with snaps/plugins.
    • Make completely agnostic of the snaps execution environment.
      • It currently assumes that plugins are executed in dedicated web workers.
  • WebWorkerController
    • Support the generic snaps communication interface mentioned above.
    • Replace internal CommandEngine module with json-rpc-engine.

This ticket is the backend work (Snaps API additions) for the custom confirmation UI ticket https://app.zenhub.com/workspaces/snaps--core-615b3a7c08d2b20015eb6c4e/issues/metamask/metamask-extension/11686

Browserify has a second argument, an opts argument, which can take a debug:true flag that generates source maps in the main file:

https://github.com/browserify/browserify/blob/master/example/source_maps/build.js#L9

There is a set of JSON-RPC methods that we expect every conceivable execution environment to implement. This includes:

• executePlugin, formerly known as installPlugin, for executing plugin source code
• handshake, formerly known as ping, for confirming that a connection has been established with the worker
• pluginRpc, for routing RPC messages to a plugin running in the execution environment

The iframe execution environment enables us to create an outstanding debugging environment for snaps by integrating said environment with the OpenRPC toolchain. In its most basic form, it enables us to test if a snap will execute in SES, and whether its JSON-RPC API is working. In this form, the snap won't have access to the API MetaMask exposes to snaps in production, and we may have to stub those methods. However, with some additional work, we can actually add snaps to MetaMask and execute them inside an iframe created by the OpenRPC inspector playground.

• Implement iframe execution environment OpenRPC inspector transport
  • Done here: https://github.com/MetaMask/iframe-ee-openrpc-inspector-transport
• Do one or none of the following
  • Create a mock MetaMask snaps provider to pass to plugins executing in the OpenRPC inspector
  • Wire up the OpenRPC inspector as an actual execution environment that can be used for snaps added to MetaMask

Work on the execution environment itself is tracked in #24.

Once we have established a configurable Snap concurrency limit (#59), we should implement a queue for JSON-RPC requests for snaps that are stopped. As running snaps are stopped and concurrency "slots" (we have yet to pick a word for this) become available, snaps with queued requests should be started in a FIFO manner, and receive all of their requests once booted.

It is missing "build:prep": "echo "@metamask/snap-cli has no build:prep command."" inside scripts
https://github.com/MetaMask/snaps-skunkworks/blob/1fcb27a3a77d184d5e2e6546a9953acb9853bf30/packages/snaps-cli/package.json#L16

We should also support doing this from the CLI. We could perhaps base it on meeseeks.

Builds on #78:

Once we can generate a list of required globals by closing #78, we have two additional ways we can act:

• Warn the developer about globals that are strictly prohibited, suggesting strategies of refining their dependency graph around that API.
• Offer to automatically add this API to the requested permissions object (if it isn't already added to this object).

These can be interactive prompts from the build command, and if done right, would result in a perfectly working, consent-requesting plugin right out of the build, regardless of the authority that the dependency tree required.

Once #132 is merged, We should add a new permission type, tentatively named the endowment type, which will be used to implement e.g. #110.

Snaps are snaps, and the names of all classes, functions, variables, enums, constants etc. should reflect this.

The WebWorkerController uses an internal module, CommandEngine, which is an almost-but-not-quite JSON-RPC-compatible RPC-message-manager-thing. It should just use json-rpc-engine instead.

Ref: #9

The same way we see is done in lava-moat examples, the lavamoat tool uses tofu, which can generate a file that emits every global variable that a module obviously requires to operate.

Many snap developers have found themselves hitting points where a module would not fully operate under SES because of some deeply-hidden setTimeout or Uint8Array in one of their dependencies.

We could add a tofu pre-process to the build step, and proactively warn developers when a module had some global requirement, and link to a blog post demonstrating how to do it.

For example:

• Adding globals to plugin endowments
• Adding a whole module to plugin endowments

Ref: MetaMask/snaps-cli#46

Dapps should be rate-limited in terms of the number of requests they can send to a snap. We may need to arrive at this number experimentally. Our RPC stack should support rate-limiting.

As part of endo, agoric is developing bundle-source and import-bundle, which is intended to do a lot of what our Snaps bundle format is doing, but preserves much better debugging (like preserving line numbers).

We are unsure if these tools will work (and expect they have some issues in them!), and so Agoric requests that we try them out, and let them know what issues (if any) we hit.

They recommend we:

• Copy these packages to the endo org from the agoric-sdk monorepo
• Try to generalize them for our purposes
• Finalize their interfaces

This permission dictates whether the fetch API is available to the Snap. This is shown both during the installation flow and on the Snaps settings page. This ticket is to track the addition to the Snaps API to enable this capability. Changes are required to the SnapController and the iframe execution environment to accomplish this.

Per #56, we need to be able to monitor snap request processing times and terminate non-compliant snaps.

Per #56, we need some kind of limit on the concurrent number of snaps. We need to arrive at this number experimentally.

Blocked by: #1

Per #56, we need to be able to monitor snap request processing times and terminate non-compliant snaps. This should be implemented as a configurable request processing time limit in the PluginController. The implementation should rely on setTimeout and Promise.race for each incoming JSON-RPC request.

Following MetaMask/snaps-cli#74, there are still some unit test improvements to make:

• Achieve 100% branch coverage for all files
• Test the built evalWorker file
• Test the yargs-generated argv object for all commands
  • See TODO in cli.test.ts

The SnapController should use ControllerMessenger events and actions to their full effect. Currently, it doesn't define any actions or events in addition to stateChange.

This is a refactoring ticket that we should undertake after the permissions controller changes are landed.