Skip to content

Commit

Permalink
feat(btc): add BTC account creation menu entry (#25625)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.


-->

## **Description**

This adds a new menu entry "Create a new Bitcoin account" to create a
Bitcoin account using the Bitcoin Manager Snap.

> [!NOTE]
> This new feature is only available on Flask for now.

> [!WARNING]
> Some code logic of this PR might change depending if we are able to
reduce the e2e test flakiness from this PR:
> - #25191


[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25625?quickstart=1)

## **Related issues**

Fixes: MetaMask/accounts-planning#451

## **Manual testing steps**

### Main build

1. `yarn start`
2. Click on the account (in the app header)
3. Click "Add account or hardware wallet"
3. **You should not see anything related to Bitcoin account creation**

![Screenshot 2024-07-02 at 11 45
22](https://github.com/MetaMask/metamask-extension/assets/569258/477c9527-475f-4a30-a550-968c93c1ec8e)

### Flask build

1. `yarn start`
2. Click on the account (in the app header)
3. Click "Add account or hardware wallet"
4. Click "Create a new Bitcoin account"
5. Input your account name
6. You should now see your new Bitcoin account in the extension

![Screenshot 2024-07-02 at 11 53
17](https://github.com/MetaMask/metamask-extension/assets/569258/069c4466-fb18-43b1-b483-f72f6141ad65)
![Screenshot 2024-07-02 at 11 54
12](https://github.com/MetaMask/metamask-extension/assets/569258/1577981a-db62-4ee3-b6b6-2679028e23d3)
![Screenshot 2024-07-02 at 11 54
25](https://github.com/MetaMask/metamask-extension/assets/569258/18cec646-8c3c-495c-adef-f8983b48c828)
![Screenshot 2024-07-02 at 11 54
31](https://github.com/MetaMask/metamask-extension/assets/569258/2fbf99e5-b386-481f-905f-700612361eb2)

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

![Screenshot 2024-07-02 at 11 45
22](https://github.com/MetaMask/metamask-extension/assets/569258/477c9527-475f-4a30-a550-968c93c1ec8e)
![Screenshot 2024-07-02 at 11 53
17](https://github.com/MetaMask/metamask-extension/assets/569258/069c4466-fb18-43b1-b483-f72f6141ad65)
![Screenshot 2024-07-02 at 11 54
12](https://github.com/MetaMask/metamask-extension/assets/569258/1577981a-db62-4ee3-b6b6-2679028e23d3)
![Screenshot 2024-07-02 at 11 54
25](https://github.com/MetaMask/metamask-extension/assets/569258/18cec646-8c3c-495c-adef-f8983b48c828)
![Screenshot 2024-07-02 at 11 54
31](https://github.com/MetaMask/metamask-extension/assets/569258/2fbf99e5-b386-481f-905f-700612361eb2)

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] 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)).
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.

---------

Co-authored-by: Daniel Rocha <[email protected]>
  • Loading branch information
ccharly and danroc authored Jul 10, 2024
1 parent 6e1ed63 commit 059166b
Show file tree
Hide file tree
Showing 18 changed files with 350 additions and 29 deletions.
5 changes: 4 additions & 1 deletion app/_locales/en/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 25 additions & 0 deletions app/scripts/lib/snap-keyring/bitcoin-wallet-snap.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { SnapId } from '@metamask/snaps-sdk';
import { Sender } from '@metamask/keyring-api';
import { HandlerType } from '@metamask/snaps-utils';
import { Json, JsonRpcRequest } from '@metamask/utils';
// This dependency is still installed as part of the `package.json`, however
// the Snap is being pre-installed only for Flask build (for the moment).
import BitcoinWalletSnap from '@metamask/bitcoin-wallet-snap/dist/preinstalled-snap.json';
import { handleSnapRequest } from '../../../../ui/store/actions';

export const BITCOIN_WALLET_SNAP_ID: SnapId =
BitcoinWalletSnap.snapId as SnapId;

export class BitcoinWalletSnapSender implements Sender {
send = async (request: JsonRpcRequest): Promise<Json> => {
// We assume the caller of this module is aware of this. If we try to use this module
// without having the pre-installed Snap, this will likely throw an error in
// the `handleSnapRequest` action.
return (await handleSnapRequest({
origin: 'metamask',
snapId: BITCOIN_WALLET_SNAP_ID,
handler: HandlerType.OnKeyringRequest,
request,
})) as Json;
};
}
7 changes: 6 additions & 1 deletion test/e2e/accounts/snap-account-settings.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@ describe('Add snap account experimental settings', function (this: Suite) {
text: 'Add account Snap',
tag: 'button',
},
{ findElementGuard: { text: 'Add a new account', tag: 'button' } }, // wait for the modal to appear
{
findElementGuard: {
text: 'Add a new Ethereum account',
tag: 'button',
},
}, // wait for the modal to appear
);
await driver.clickElement('.mm-box button[aria-label="Close"]');

Expand Down
5 changes: 4 additions & 1 deletion test/e2e/tests/account/import-flow.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,10 @@ describe('Import flow @no-mmi', function () {
await driver.clickElement(
'[data-testid="multichain-account-menu-popover-action-button"]',
);
await driver.clickElement({ text: 'Add a new account', tag: 'button' });
await driver.clickElement({
text: 'Add a new Ethereum account',
tag: 'button',
});

// set account name
await driver.fill('[placeholder="Account 2"]', '2nd account');
Expand Down
5 changes: 4 additions & 1 deletion test/e2e/tests/tokens/nft/import-nft.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ describe('Import NFT', function () {
await driver.clickElement(
'[data-testid="multichain-account-menu-popover-action-button"]',
);
await driver.clickElement({ text: 'Add a new account', tag: 'button' });
await driver.clickElement({
text: 'Add a new Ethereum account',
tag: 'button',
});

// By clicking creating button without filling in the account name
// the default name would be set as Account 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ The selected network is Sepolia.",
7,Close the settings page.,,The overview page is displayed. ,
8,Open the Account menu.,,The account menu popover is shown.,
9,Click the Add account button.,,The Add account popover is shown.,
10,Click the Add a new account button.,,,
10,Click the Add a new Ethereum account button.,,,
11,Specify an account name and create the account.,e.g. Account 2.,The extension switches to Account 2.,
12,Open the Account menu.,,The account menu popover is shown.,
13,Switch back to the initial account.,e.g. Account 1.,The extension switches to Account 1.,
Expand Down
88 changes: 82 additions & 6 deletions ui/components/multichain/account-list-menu/account-list-menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import {
CreateEthAccount,
ImportAccount,
AccountListItemMenuTypes,
///: BEGIN:ONLY_INCLUDE_IF(build-flask)
CreateBtcAccount,
///: END:ONLY_INCLUDE_IF
} from '..';
import {
AlignItems,
Expand Down Expand Up @@ -59,6 +62,9 @@ import {
import { getEnvironmentType } from '../../../../app/scripts/lib/util';
import { ENVIRONMENT_TYPE_POPUP } from '../../../../shared/constants/app';
import { getAccountLabel } from '../../../helpers/utils/accounts';
///: BEGIN:ONLY_INCLUDE_IF(build-flask)
import { hasCreatedBtcMainnetAccount } from '../../../selectors/accounts';
///: END:ONLY_INCLUDE_IF
import { HiddenAccountList } from './hidden-account-list';

const ACTION_MODES = {
Expand All @@ -68,10 +74,38 @@ const ACTION_MODES = {
MENU: 'menu',
// Displays the add account form controls
ADD: 'add',
///: BEGIN:ONLY_INCLUDE_IF(build-flask)
// Displays the add account form controls (for bitcoin account)
ADD_BITCOIN: 'add-bitcoin',
///: END:ONLY_INCLUDE_IF
// Displays the import account form controls
IMPORT: 'import',
};

/**
* Gets the title for a given action mode.
*
* @param t - Function to translate text.
* @param actionMode - An action mode.
* @returns The title for this action mode.
*/
export const getActionTitle = (t, actionMode) => {
switch (actionMode) {
case ACTION_MODES.ADD:
return t('addAccount');
///: BEGIN:ONLY_INCLUDE_IF(build-flask)
case ACTION_MODES.ADD_BITCOIN:
return t('addAccount');
///: END:ONLY_INCLUDE_IF
case ACTION_MODES.MENU:
return t('addAccount');
case ACTION_MODES.IMPORT:
return t('importAccount');
default:
return t('selectAnAccount');
}
};

/**
* Merges ordered accounts with balances with each corresponding account data from internal accounts
*
Expand Down Expand Up @@ -117,6 +151,11 @@ export const AccountListMenu = ({
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
const addSnapAccountEnabled = useSelector(getIsAddSnapAccountEnabled);
///: END:ONLY_INCLUDE_IF
///: BEGIN:ONLY_INCLUDE_IF(build-flask)
const isBtcMainnetAccountAlreadyCreated = useSelector(
hasCreatedBtcMainnetAccount,
);
///: END:ONLY_INCLUDE_IF

const [searchQuery, setSearchQuery] = useState('');
const [actionMode, setActionMode] = useState(ACTION_MODES.LIST);
Expand All @@ -136,12 +175,7 @@ export const AccountListMenu = ({
}
searchResults = mergeAccounts(searchResults, accounts);

let title = t('selectAnAccount');
if (actionMode === ACTION_MODES.ADD || actionMode === ACTION_MODES.MENU) {
title = t('addAccount');
} else if (actionMode === ACTION_MODES.IMPORT) {
title = t('importAccount');
}
const title = getActionTitle(t, actionMode);

let onBack = null;
if (actionMode !== ACTION_MODES.LIST) {
Expand Down Expand Up @@ -180,6 +214,23 @@ export const AccountListMenu = ({
/>
</Box>
) : null}
{
///: BEGIN:ONLY_INCLUDE_IF(build-flask)
actionMode === ACTION_MODES.ADD_BITCOIN ? (
<Box paddingLeft={4} paddingRight={4} paddingBottom={4}>
<CreateBtcAccount
onActionComplete={(confirmed) => {
if (confirmed) {
onClose();
} else {
setActionMode(ACTION_MODES.LIST);
}
}}
/>
</Box>
) : null
///: END:ONLY_INCLUDE_IF
}
{actionMode === ACTION_MODES.IMPORT ? (
<Box
paddingLeft={4}
Expand Down Expand Up @@ -221,6 +272,31 @@ export const AccountListMenu = ({
{t('addNewAccount')}
</ButtonLink>
</Box>
{
///: BEGIN:ONLY_INCLUDE_IF(build-flask)
<Box marginTop={4}>
<ButtonLink
disabled={isBtcMainnetAccountAlreadyCreated}
size={Size.SM}
startIconName={IconName.Add}
onClick={() => {
trackEvent({
category: MetaMetricsEventCategory.Navigation,
event: MetaMetricsEventName.AccountAddSelected,
properties: {
account_type: MetaMetricsEventAccountType.Default,
location: 'Main Menu',
},
});
setActionMode(ACTION_MODES.ADD_BITCOIN);
}}
data-testid="multichain-account-menu-popover-add-account"
>
{t('addNewBitcoinAccount')}
</ButtonLink>
</Box>
///: END:ONLY_INCLUDE_IF
}
<Box marginTop={4}>
<ButtonLink
size={Size.SM}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ describe('AccountListMenu', () => {

// Click the button to ensure the options and close button display
button[0].click();
expect(getByText('Add a new account')).toBeInTheDocument();
expect(getByText('Add a new Ethereum account')).toBeInTheDocument();
expect(getByText('Import account')).toBeInTheDocument();
expect(getByText('Add hardware wallet')).toBeInTheDocument();
const header = document.querySelector('header');
Expand All @@ -243,7 +243,7 @@ describe('AccountListMenu', () => {
);
button.click();

fireEvent.click(getByText('Add a new account'));
fireEvent.click(getByText('Add a new Ethereum account'));
expect(getByText('Create')).toBeInTheDocument();
expect(getByText('Cancel')).toBeInTheDocument();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import React from 'react';
import { CreateBtcAccount } from '.';

export default {
title: 'Components/Multichain/CreateBtcAccount',
component: CreateBtcAccount,
};

export const DefaultStory = (args) => <CreateBtcAccount {...args} />;
DefaultStory.storyName = 'Default';
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/* eslint-disable jest/require-top-level-describe */
import React from 'react';
import { JsonRpcRequest } from '@metamask/utils';
import { BtcAccountType, BtcMethod } from '@metamask/keyring-api';
import { fireEvent, renderWithProvider, waitFor } from '../../../../test/jest';
import configureStore from '../../../store/store';
import mockState from '../../../../test/data/mock-state.json';
import { MultichainNetworks } from '../../../../shared/constants/multichain/networks';
import { CreateBtcAccount } from '.';

const render = (props = { onActionComplete: jest.fn() }) => {
const store = configureStore(mockState);
return renderWithProvider(<CreateBtcAccount {...props} />, store);
};

const ACCOUNT_NAME = 'Bitcoin Account';

const mockBtcAccount = {
type: BtcAccountType.P2wpkh,
id: '8a323a0b-9ff5-4ab6-95e0-d51ec7e09763',
address: 'bc1qwl8399fz829uqvqly9tcatgrgtwp3udnhxfq4k',
options: {
scope: MultichainNetworks.BITCOIN,
index: 0,
},
methods: [BtcMethod.SendMany],
};
const mockBitcoinWalletSnapSend = jest.fn().mockReturnValue(mockBtcAccount);
const mockSetAccountLabel = jest.fn().mockReturnValue({ type: 'TYPE' });

jest.mock('../../../store/actions', () => ({
forceUpdateMetamaskState: jest.fn(),
setAccountLabel: (address: string, label: string) =>
mockSetAccountLabel(address, label),
}));

jest.mock(
'../../../../app/scripts/lib/snap-keyring/bitcoin-wallet-snap',
() => ({
BitcoinWalletSnapSender: jest.fn().mockImplementation(() => {
return {
send: (_request: JsonRpcRequest) => {
return mockBitcoinWalletSnapSend();
},
};
}),
}),
);

describe('CreateBtcAccount', () => {
afterEach(() => {
jest.clearAllMocks();
});

it('displays account name input and suggests name', async () => {
const { getByPlaceholderText } = render();

await waitFor(() =>
expect(getByPlaceholderText(ACCOUNT_NAME)).toBeInTheDocument(),
);
});

it('fires onActionComplete when clicked', async () => {
const onActionComplete = jest.fn();
const { getByText, getByPlaceholderText } = render({ onActionComplete });

const input = await waitFor(() => getByPlaceholderText(ACCOUNT_NAME));
const newAccountName = 'New Account Name';

fireEvent.change(input, {
target: { value: newAccountName },
});
fireEvent.click(getByText('Create'));

await waitFor(() =>
expect(mockSetAccountLabel).toHaveBeenCalledWith(
mockBtcAccount.address,
newAccountName,
),
);
await waitFor(() => expect(onActionComplete).toHaveBeenCalled());
});

it(`doesn't allow duplicate account names`, async () => {
const { getByText, getByPlaceholderText } = render();

const input = await waitFor(() => getByPlaceholderText(ACCOUNT_NAME));
const usedAccountName =
mockState.metamask.internalAccounts.accounts[
'07c2cfec-36c9-46c4-8115-3836d3ac9047'
].metadata.name;

fireEvent.change(input, {
target: { value: usedAccountName },
});

const submitButton = getByText('Create');
expect(submitButton).toHaveAttribute('disabled');
});
});
Loading

0 comments on commit 059166b

Please sign in to comment.