Skip to content

feat: add support for setSelectedAccounts #543

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat: add support for setSelectedAccounts #543

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bf99101
feat: introduce `setSelectedAccounts`
orestiseth Oct 14, 2025
dd19e37
feat: add jsdoc for new methods
orestiseth Oct 14, 2025
576d098
feat: throw validation error if selected accounts dont overlap with e…
orestiseth Oct 14, 2025
5a6d17f
feat: simplify logic and add unit tests
orestiseth Oct 14, 2025
8fd8a3f
feat: fix linter
orestiseth Oct 14, 2025
d0f5244
feat: fix params passing
orestiseth Oct 16, 2025
741cddf
feat: address PR comments
orestiseth Oct 16, 2025
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
4 changes: 2 additions & 2 deletions packages/snap/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
"@jest/globals": "^30.0.5",
"@metamask/bitcoindevkit": "^0.1.13",
"@metamask/key-tree": "^10.1.1",
"@metamask/keyring-api": "^20.1.1",
"@metamask/keyring-snap-sdk": "^6.0.0",
"@metamask/keyring-api": "^21.1.0",
"@metamask/keyring-snap-sdk": "^7.1.0",
"@metamask/slip44": "^4.2.0",
"@metamask/snaps-cli": "^8.3.0",
"@metamask/snaps-jest": "^9.4.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/snap/snap.manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
{
"version": "1.2.0",
"version": "1.3.0",
"description": "Manage Bitcoin using MetaMask",
"proposedName": "Bitcoin",
"repository": {
"type": "git",
"url": "https://github.com/MetaMask/snap-bitcoin-wallet.git"
},
"source": {
"shasum": "6Tzrjzyccj6Ane17A/FBIUEVNaB6pgoUEWRTQUbpB4o=",
"shasum": "4uaBMQNFfI4kB+HQyWxv6LzbX8ov+KYBwH1yDX8uQAE=",
"location": {
"npm": {
"filePath": "dist/bundle.js",
Expand Down
3 changes: 3 additions & 0 deletions packages/snap/src/entities/account.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,9 @@ export type BitcoinAccountRepository = {
* @returns the frozen UTXO outpoints.
*/
getFrozenUTXOs(id: string): Promise<string[]>;

setSelectedAccounts(accountIds: string[]): Promise<BitcoinAccount[]>;
getSelectedAccounts(): Promise<BitcoinAccount[]>;
};

export enum Purpose {
Expand Down
2 changes: 2 additions & 0 deletions packages/snap/src/entities/snap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export type AccountState = {
wallet: string;
// Wallet inscriptions for meta protocols (ordinals, etc.)
inscriptions: Inscription[];
// if the account is selected
isSelected: boolean;
};

export enum TrackingSnapEvent {
Expand Down
2 changes: 1 addition & 1 deletion packages/snap/src/handlers/CronHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export class CronHandler {
}

async synchronizeAccounts(): Promise<void> {
const accounts = await this.#accountsUseCases.list();
const accounts = await this.#accountsUseCases.getSelectedAccounts();
const results = await Promise.allSettled(
accounts.map(async (account) => {
return this.#accountsUseCases.synchronize(account, 'cron');
Expand Down
29 changes: 24 additions & 5 deletions packages/snap/src/handlers/KeyringHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
ListAccountsRequestStruct,
ListAccountTransactionsRequestStruct,
MetaMaskOptionsStruct,
SetSelectedAccountsRequestStruct,
SubmitRequestRequestStruct,
} from '@metamask/keyring-api';
import type {
Expand Down Expand Up @@ -143,6 +144,11 @@ export class KeyringHandler implements Keyring {
assert(request, SubmitRequestRequestStruct);
return this.submitRequest(request.params);
}
case `${KeyringRpcMethod.SetSelectedAccounts}`: {
assert(request, SetSelectedAccountsRequestStruct);
await this.setSelectedAccounts((request as any).params.accountIds);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the as any is needed here.

On line 149, assert narrows the type of request, so that on line 150 request is of type inferred from the struct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it should be await this.setSelectedAccounts(request.params.accounts);. I have the change locally but haven't pushed it yet. Will do in a bit.

return null;
}

default: {
throw new InexistentMethodError('Keyring method not supported', {
Expand Down Expand Up @@ -174,7 +180,7 @@ export class KeyringHandler implements Keyring {
index,
derivationPath,
addressType,
synchronize = true,
synchronize = false,
accountNameSuggestion,
} = options;

Expand Down Expand Up @@ -257,10 +263,7 @@ export class KeyringHandler implements Keyring {
),
);

// Return only accounts with history.
return accounts
.filter((account) => account.listTransactions().length > 0)
.map(mapToDiscoveredAccount);
return accounts.map(mapToDiscoveredAccount);
}

async getAccountBalances(
Expand Down Expand Up @@ -330,6 +333,22 @@ export class KeyringHandler implements Keyring {
return this.#keyringRequest.route(request);
}

async setSelectedAccounts(accounts: string[]): Promise<void> {
const isSubset = (first: Set<string>, second: Set<string>): boolean => {
return Array.from(first).every((element) => second.has(element));
};

const currentAccounts = new Set(
(await this.#accountsUseCases.list()).map((account) => account.id),
);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTC snap has 2 methods for updating accounts. One is a full scan which previously was used when an account was created/discovered and the other one is a lighter synchronize which identified updates.

Since now we don't do any scanning on discovery we don't really know when it is the first time we see an account (without storing anything on snaps state).

Thus, on this PR we are going to run a full scan everytime the keyring method gets called and a synchronize on the cron job running every 30 seconds.

Feel free to add your thoughts. We can always store "seen" accounts on the snap state but I am not sure how efficient is going to be if a user creates loads of them.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes a lot of sense to me 👍

if (!isSubset(new Set(accounts), currentAccounts)) {
throw new Error(`Accounts ids were not part of existing accounts.`);
}

await this.#accountsUseCases.setSelectedAccounts(accounts);
}

#extractAddressType(path: string): AddressType {
const segments = path.split('/');
if (segments.length < 4) {
Expand Down
35 changes: 35 additions & 0 deletions packages/snap/src/store/BdkAccountRepository.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,4 +227,39 @@ export class BdkAccountRepository implements BitcoinAccountRepository {
return `${txid}:${vout}`;
});
}

async setSelectedAccounts(accountIds: string[]): Promise<BitcoinAccount[]> {
const allAccounts = await this.getAll();
const accountIdSet = new Set(accountIds);

const updatePromises = allAccounts.map(async (account) => {
return this.#snapClient.setState(
`accounts.${account.id}.isSelected`,
accountIdSet.has(account.id), // this should switch to inactive previously active accounts
);
});

await Promise.all(updatePromises);
return allAccounts.filter((account) => accountIdSet.has(account.id));
}

async getSelectedAccounts(): Promise<BitcoinAccount[]> {
const accounts = (await this.#snapClient.getState('accounts')) as
| SnapState['accounts']
| null;

if (!accounts) {
return [];
}

return Object.entries(accounts)
.filter(([_id, accState]) => accState.isSelected)
.map(([id, account]) =>
BdkAccountAdapter.load(
id,
account.derivationPath,
ChangeSet.from_json(account.wallet),
),
);
}
}
24 changes: 18 additions & 6 deletions packages/snap/src/use-cases/AccountUseCases.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,6 @@ export class AccountUseCases {
addressType,
);

await this.#chain.fullScan(newAccount);

this.#logger.info(
'Bitcoin account discovered successfully. Request: %o',
req,
);
return newAccount;
}

Expand Down Expand Up @@ -545,6 +539,24 @@ export class AccountUseCases {
);
}

async setSelectedAccounts(accountIds: string[]): Promise<void> {
const selectedAccounts =
await this.#repository.setSelectedAccounts(accountIds);

const scanPromises = selectedAccounts.map(async (account) => {
await this.fullScan(account);
this.#logger.info(
`Bitcoin account discovered successfully. Account id: ${account.id}`,
);
});

await Promise.all(scanPromises);
}

async getSelectedAccounts(): Promise<BitcoinAccount[]> {
return await this.#repository.getSelectedAccounts();
}

async #fillPsbt(
account: BitcoinAccount,
templatePsbt: Psbt,
Expand Down
26 changes: 19 additions & 7 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2879,8 +2879,8 @@ __metadata:
"@jest/globals": "npm:^30.0.5"
"@metamask/bitcoindevkit": "npm:^0.1.13"
"@metamask/key-tree": "npm:^10.1.1"
"@metamask/keyring-api": "npm:^20.1.1"
"@metamask/keyring-snap-sdk": "npm:^6.0.0"
"@metamask/keyring-api": "npm:^21.1.0"
"@metamask/keyring-snap-sdk": "npm:^7.1.0"
"@metamask/slip44": "npm:^4.2.0"
"@metamask/snaps-cli": "npm:^8.3.0"
"@metamask/snaps-jest": "npm:^9.4.0"
Expand Down Expand Up @@ -3159,19 +3159,31 @@ __metadata:
languageName: node
linkType: hard

"@metamask/keyring-snap-sdk@npm:^6.0.0":
version: 6.0.0
resolution: "@metamask/keyring-snap-sdk@npm:6.0.0"
"@metamask/keyring-api@npm:^21.1.0":
version: 21.1.0
resolution: "@metamask/keyring-api@npm:21.1.0"
dependencies:
"@metamask/keyring-utils": "npm:^3.1.0"
"@metamask/superstruct": "npm:^3.1.0"
"@metamask/utils": "npm:^11.1.0"
bitcoin-address-validation: "npm:^2.2.3"
checksum: 10/3371a5ab0e9ba0e9b23b30b03a7d83d029e223def5485ab1aa2ec793ba18ff3738422dbe3c47f9cf82411d2ca6ca918928bf998741f0977055071b7bf3042314
languageName: node
linkType: hard

"@metamask/keyring-snap-sdk@npm:^7.1.0":
version: 7.1.0
resolution: "@metamask/keyring-snap-sdk@npm:7.1.0"
dependencies:
"@metamask/keyring-utils": "npm:^3.1.0"
"@metamask/snaps-sdk": "npm:^9.0.0"
"@metamask/superstruct": "npm:^3.1.0"
"@metamask/utils": "npm:^11.1.0"
webextension-polyfill: "npm:^0.12.0"
peerDependencies:
"@metamask/keyring-api": ^20.0.0
"@metamask/keyring-api": ^21.1.0
"@metamask/providers": ^19.0.0
checksum: 10/c306615cee12f5669a1e5e5a22a7e0d1a8aa8eb981578ff4cd125067d8aa9fbf788d02ffc17e123ef47458133b6ec8b5a01966599800ea443b4b07f0cdd0ee7e
checksum: 10/1a1809733c1f21af87f3491d292c499c5441afa0780e848718ec2b6aff50d76bb03ea44ee93ecaa80d79453a98926d84cd13ff406256ab6a2136d9e31250faa8
languageName: node
linkType: hard

Expand Down
Loading