Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove endowment:long-running from Snap APIs #945

Closed
rekmarks opened this issue Nov 11, 2022 · 5 comments · Fixed by #1751 or MetaMask/metamask-extension#20951
Closed

Remove endowment:long-running from Snap APIs #945

rekmarks opened this issue Nov 11, 2022 · 5 comments · Fixed by #1751 or MetaMask/metamask-extension#20951
Assignees
Labels
area-capabilities Related to the kinds of things that snaps are able to do. area-execution-environments type-research A research task. type-security Related to enforcing our security model.

Comments

@rekmarks
Copy link
Member

rekmarks commented Nov 11, 2022

Each API available to snaps should be reviewed for consistency, cross-platform compatibility, maintainability, and usability ahead of our planned v1 release. This includes:

  • All endowments, permissioned or otherwise
  • All JSON-RPC methods, permissioned or otherwise, excluding those specific to the EIP-1193 provider and/or the Ethereum execution client interface

Known issues/open questions include:

  • endowment:long-running
    • This endowment permits snaps to run forever. It's something of a hack/crutch, and we should consider very carefully whether to continue supporting it.
@rekmarks rekmarks added @metamask/snap-controllers area-execution-environments area-capabilities Related to the kinds of things that snaps are able to do. type-research A research task. labels Nov 11, 2022
@MetaMask MetaMask deleted a comment from rekmarks Nov 18, 2022
@kenhkan
Copy link

kenhkan commented Dec 1, 2022

We need to come up with the list of features/aspects for v1 API. This should also be an epic that contains all the related features. We'll move this to the product backlog until we have that complete list.

@kenhkan kenhkan changed the title Review and finalize Snap APIs bound for v1 Remove endowment:long-running from Snap APIs Dec 20, 2022
@ritave ritave added type-security Related to enforcing our security model. and removed @metamask/rpc-methods labels Dec 21, 2022
@ritave
Copy link
Contributor

ritave commented Jan 5, 2023

Long running / background snaps are useful. Some of previous hackathon winners required that permission.

Examples uses:

  1. Longer cryptographic calculations (for example RSA)
  2. Polling for events from external servers
  3. Polling for logs from blockchain and do automations based on that (for example automatic NFT bidding)

Though this permission is too broad, we need to figure out how to allow those usages to exist while removing this permission.


A good starting point is how iOS manages background tasks.
They're time limited, but allow for many use-cases.

  1. Push messages
  2. Cronjobs
  3. URL downloads

Also see Background Modes

@GuillaumeRx
Copy link
Contributor

GuillaumeRx commented Feb 15, 2023

endowment:long-running will be disabled in stable until we find a better/less hacky way to handle long running snaps. For now it's still supported in Flask

@Montoya
Copy link
Collaborator

Montoya commented Jun 8, 2023

Still supported in Flask until we have a better alternative

@kenhkan
Copy link

kenhkan commented Sep 14, 2023

From standup:

  • Bringing this in and David will work on this. As a separate PR.

david0xd added a commit to MetaMask/metamask-extension that referenced this issue Sep 27, 2023
## **Description**
This PR removes `long-running` Snap endowment/permission.

Snaps team have decided to remove this endowment because of its
unpredictable security concerns around running Snaps indefinitely. This
endowment is removed in favor of the new endowments that will be focused
on solving certain use cases that require snap to be running for more
time than default.

## **Related issues**

_Fixes [#945](MetaMask/snaps#945

## **Related PR**
MetaMask/snaps#1751 (**required**)
MetaMask/metamask-docs#919

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've clearly explained:
  - [ ] What problem this PR is solving.
  - [ ] How this problem was solved.
  - [ ] How reviewers can test my changes.
- [ ] I’ve indicated what issue this PR is linked to: Fixes #???
- [ ] I’ve included tests if applicable.
- [ ] I’ve documented any added code.
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
- [ ] 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-capabilities Related to the kinds of things that snaps are able to do. area-execution-environments type-research A research task. type-security Related to enforcing our security model.
Projects
None yet
6 participants