chore(bitcoin): add bitcoin build feature + disable it temporarily

[ccharly]

Description

Temporarily disable Bitcoin support until we finalize the new Snap version with the latest updated architecture.

Important

Once this PR is merged, existing Bitcoin accounts might still be showing on your extension.
You might see some error messages on the console, but nothing should break. You can remove those
accounts, and the errors will be be gone.

Related issues

N/A

Manual testing steps

1. yarn start:flask
2. You should not be able to create a Bitcoin account (using the preinstalled Bitcoin Snap)

Screenshots/Recordings

Before

After

Pre-merge author checklist

• I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
• I've completed the PR template to the best of my ability
• I’ve included tests if applicable
• I’ve documented my code using JSDoc format if applicable
• I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

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.

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[metamaskbot]

Builds ready [7f1b273]

• builds: chrome, firefox
• builds (flask): chrome, firefox
• builds (MMI): chrome
• builds (test): chrome, firefox
• builds (test-flask): chrome, firefox
• build viz: Build System
• mv3: Bundle Size Stats
• mv2: E2e Actions Stats
• storybook: Storybook
• typescript migration: Dashboard
• all artifacts
• bundle viz:

  • background: 0, 1, 2, 3, 4, 5, 6, 7
  • common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
  • ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12
  • content-script: 0
  • offscreen: 0

Page Load Metrics (1671 ± 67 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1490	1963	1662	143	68
		domContentLoaded	1475	1909	1638	137	66
		load	1503	1970	1671	140	67
		domInteractive	24	203	43	39	19
		backgroundConnect	12	91	34	21	10
		firstReactRender	14	187	44	40	19
		getState	5	70	29	26	13
		initialActions	0	1	0	0	0
		loadScripts	1062	1497	1217	126	61
		setupStore	7	68	18	20	10
		uiStartup	1706	2634	1954	213	102

Bundle size diffs [🚀 Bundle size reduced!]

• background: -296 Bytes (-0.01%)
• ui: -268 Bytes (-0.00%)
• common: -247 Bytes (-0.00%)

[GuillaumeRx]

What about setting the E2E test to runnable only if bitcoin is enable ? Is it possible ?

[ccharly]

What about setting the E2E test to runnable only if bitcoin is enable ? Is it possible ?

@GuillaumeRx I wanted to do something like this yes, having like a conditional it.skip basically. Looks like there are solutions, but this might break eslint integration.

From what I've read, there's not builtin support for this, and you need to use trick like:

const itif = (condition) => condition ? it : it.skip;

describe('suite name', () => {
  itif(true)('test name', async () => {
    // Your test
  });
});

See: https://stackoverflow.com/a/60438234

But itif won't be recognized properly.

So I ended up, not doing anything for this.

Also, the plan is to fully re-enable Bitcoin once we have catch up with the new APIs, so it's only a temporary solution IMO.

---

Another option could be to tweak the test launcher (in test/e2e) and use the test/e2e/flask/btc folder if Bitcoin is enabled, but that might requires some more efforts.

---

EDIT: Forgot to add the stackoverflow references

[salimtb]

Why have we decided to leave this in place and comment on it rather than removing it ?

[ccharly]

My initial idea was to comment it out with a description so people can easily add it back (like in most configuration files).

I can remove it if you prefer.

[salimtb]

maybe i miss some context here , but can you help me to understand why we are not doing something like:

    // function to check if bitcoin is supported based on the state
    if (!getIsBitcoinSupportEnabled()) {
      return;
    }

rather than skip, on the e2e tests ?

[ccharly]

We cannot use those kind of selector in e2e tests. And most of the Bitcoin e2e test are actually mocking "enabling" this flag with some fixtures.

Like here: https://github.com/MetaMask/metamask-extension/blob/main/test/e2e/flask/btc/common-btc.ts#L223

Also, we cannot use code-fence in e2e tests (at least, to my knowledge), so we would need to find a trick (using a test build)

Unless there are some known tricks to do this?

[zone-live]

This needs to go 1st #30484