metamask-extension icon indicating copy to clipboard operation
metamask-extension copied to clipboard

Remove un-necessary word lists from bundle - replace ethers with submodules

Open jpuri opened this issue 2 years ago • 4 comments

We are including huge list of words of which only english is being used:

https://output.circle-artifacts.com/output/job/0b87725e-d9fa-4b1e-87c8-209ba5bb939a/artifacts/0/build-artifacts/source-map-explorer/common-0.html

https://output.circle-artifacts.com/output/job/0b87725e-d9fa-4b1e-87c8-209ba5bb939a/artifacts/0/build-artifacts/source-map-explorer/common-6.html

We can get rif of other languages to reduce bundle size.

This ticket is now about replacing ethers with submodules throughout our dependency graph.

ongoing related PRs:

  • https://github.com/MetaMask/metamask-extension/pull/15461
  • https://github.com/MetaMask/metamask-extension/pull/15818
  • https://github.com/ethereum-optimism/optimism/pull/3363
  • https://github.com/MetaMask/controllers/pull/907

jpuri avatar Jun 23 '22 09:06 jpuri

The wordlists that are added to the bundle from ethers (via ethersproject/wordlist) should be removed with this work currently in progress: Screen Shot 2022-08-09 at 2 23 47 PM

If they aren't removed in this work we may need to patch or create our own versions of @ethersproject/wallet and @ethersproject/json-wallet that don't depend on these wordlists to remove them from from our bundle.

There are other wordlists from bip39 that this ticket may also refer to.

adonesky1 avatar Aug 09 '22 19:08 adonesky1

This PR to our fork of BIP39 should remove another ~160-180kb from our common.js files: Screen Shot 2022-08-09 at 3 05 23 PM

After that we will still have the extra word-lists from @scure/bip39 another version of bip39 that we use in @metamask/key-tree (which is in the extension dependency tree). We may want to simply change that package to use our bip39 fork. Will investigate.

adonesky1 avatar Aug 09 '22 20:08 adonesky1

Actually I vaguely remember @rekmarks expressing that we should switch over to using @scure/bip39 rather than our fork of bitcoinjs/bip39? Maybe I'm imagining or misattributing that but... Erik and @kumavis do you think its worth swapping over to using a fork of @scure/bip39 with our same patches and then removing the unused wordlists as I am suggesting we do in this PR to our fork, and then consolidate around using that in @metamask/key-tree, @metamask/eth-hd-keyring and @metamask/keyringcontroller?

adonesky1 avatar Aug 09 '22 20:08 adonesky1

I think we should switch over to @scure, fork it, apply our patches, and then consolidate around that. Per its readme, the benefits of @scure/bip39 over bip39 are:

  • Is 491KB all-bundled instead of 1.3MB
  • Uses 2 dependencies instead of 15
  • Wordlists are 157KB instead of 315KB
  • Had an external security audit by Cure53 on Jan 5, 2022

rekmarks avatar Aug 10 '22 15:08 rekmarks