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

feat(btc): add BTC account creation menu entry #25625

Merged
merged 31 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
54db504
refactor: better typing for handleSnapRequest
ccharly Jun 4, 2024
448b28e
feat(btc): add CreateBtcAccount component
ccharly Jun 4, 2024
ab68a06
feat(btc): add [email protected] as a preinstalled snap
ccharly Jul 2, 2024
11edb03
refactor(btc): only add btc account menu entry for flask
ccharly Jul 2, 2024
9216114
chore: ignore prettier for app/scripts/snaps/*.json
ccharly Jul 2, 2024
2ab28e7
chore: lint
ccharly Jul 2, 2024
f73c467
fix(storybook): fix title for Create{Btc,Eth}Account
ccharly Jul 2, 2024
afc2d35
refactor: use new addAccount value everywhere
ccharly Jul 2, 2024
c8ca2a7
fix(btc): call onActionComplete after account has been created + renamed
ccharly Jul 2, 2024
ac85ac7
test(btc): fix CreateBtcAccount tests
ccharly Jul 2, 2024
f2ffffe
chore: lint
ccharly Jul 2, 2024
af7b143
Merge branch 'develop' into feature/create-btc-account-without-snap-flow
ccharly Jul 2, 2024
7b0b10b
fix(btc): fix initial account BTC creation
ccharly Jul 2, 2024
93d5ee5
test(btc): add missing mock for forceUpdateMetamaskState
ccharly Jul 2, 2024
8c0a432
Merge branch 'develop' into feature/create-btc-account-without-snap-flow
ccharly Jul 2, 2024
34a1e15
chore(btc): use bitcoin-manager-snap as a dependency
ccharly Jul 2, 2024
a970e3d
Merge branch 'develop' into feature/create-btc-account-without-snap-flow
ccharly Jul 2, 2024
28c9474
Revert "chore(btc): use bitcoin-manager-snap as a dependency"
ccharly Jul 2, 2024
f471e36
refactor(btc): remove shared/constants/bitcoin-manager-snap.ts
ccharly Jul 3, 2024
8189a42
Merge branch 'develop' into feature/create-btc-account-without-snap-flow
ccharly Jul 3, 2024
4d034c1
chore(deps): use [email protected] as a dependency
ccharly Jul 4, 2024
f05e23b
Merge branch 'develop' into feature/create-btc-account-without-snap-flow
ccharly Jul 9, 2024
fa37320
refactor(btc): use new preinstalled @metamask/bitcoin-wallet-snap Snap
ccharly Jul 9, 2024
e63b0dd
fix(btc): fix code fences
ccharly Jul 10, 2024
f929666
fix(btc): fix BitcoinWalletSnap code fences
ccharly Jul 10, 2024
ae79ece
chore: revert changes to .prettierignore
ccharly Jul 10, 2024
ed06ecc
Merge branch 'develop' into feature/create-btc-account-without-snap-flow
ccharly Jul 10, 2024
5a196a0
fix(btc): disable menu entry if account has been already created
ccharly Jul 10, 2024
11f450e
chore: add " (Beta)" suffix
ccharly Jul 10, 2024
eb4ea27
chore: lint
ccharly Jul 10, 2024
716861e
fix(btc): fix missing code fences
ccharly Jul 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -211,7 +211,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 @@ -235,7 +235,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