MetaMask / snaps

Extend the functionality of MetaMask using Snaps
https://metamask.io/snaps/
Other
721 stars 554 forks source link

Establish snap source code size limit #947

Closed rekmarks closed 1 year ago

rekmarks commented 1 year ago

Installing a snap should not cause the extension to blow up. Snaps of a certain size (>=100mb) are known to be able to cause the extension or entire browser to crash. We should empirically establish some upper bounds and set a reasonable limit.

Once the limit is established, we should:

rekmarks commented 1 year ago

Hey team! Please add your planning poker estimate with Zenhub @FrederikBolding @david0xd

david0xd commented 1 year ago

After some extensive stress testing and research I got some good results on this.

So, here is the table representing different scenarios of testing and outcome: Bundle size Installs? Snap is running? Can do long processing of data? endowment:long-running enabled?
50 MB Yes Yes Yes No
75 MB Yes Yes Yes No
100 MB Yes Yes No (timed-out and terminated) No
100 MB Yes Yes No (no response, snap crashed) Yes
225 MB Yes Yes No (no response, snap crashed) Yes

Note about the way of running tests:

Further explanation of some stress testing scenarios:

Details about environment on which tests have been performed:

david0xd commented 1 year ago

After the analysis of the research result, we made a conclusion that the results and measurements are good enough and are not a major concern. It is proven that the extension and web browser are stable and performing as expected under the circumstances of a "very large" Snap bundles. The conclusion is as well that this is not particularly memory problem as it was firstly identified to be, but rather problem of a long processing of large amount of data (>=100MB).

The long processing of large amount of data issue is forwarded to this ticket: https://github.com/MetaMask/snaps-monorepo/issues/981

I'm closing this one as we agreed when discussed on a standup.