Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ const baseConfig = {
// An object that configures minimum threshold enforcement for coverage results
coverageThreshold: {
global: {
branches: 70.2,
functions: 72.07,
lines: 71.03,
statements: 71.16,
branches: 70.68,
functions: 72.32,
lines: 71.27,
statements: 71.39,
},
},

Expand Down
57 changes: 46 additions & 11 deletions src/CAIP294.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ const getWalletData = (): CAIP294WalletData => ({
name: 'Example Wallet',
icon: 'data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg"/>',
rdns: 'com.example.wallet',
extensionId: 'abcdefghijklmnopqrstuvwxyz',
targets: [
{
type: 'caip-123',
value: 'abcdefghijklmnopqrstuvwxyz',
},
],
});

const walletDataValidationError = () =>
Expand Down Expand Up @@ -88,26 +93,56 @@ describe('CAIP-294', () => {
});
});

it('allows `extensionId` to be undefined or a string', () => {
it('allows `targets` to be undefined', () => {
const walletInfo = getWalletData();
expect(() => announceWallet(walletInfo)).not.toThrow();

delete walletInfo.extensionId;
delete walletInfo.targets;
expect(() => announceWallet(walletInfo)).not.toThrow();
});

it('allows `targets` to be empty array', () => {
const walletInfo = getWalletData();
expect(() => announceWallet(walletInfo)).not.toThrow();

walletInfo.extensionId = 'valid-string';
walletInfo.targets = [];
expect(() => announceWallet(walletInfo)).not.toThrow();
});
});

it('throws if the `extensionId` field is invalid', () => {
[null, '', 42, Symbol('bar')].forEach((invalidExtensionId) => {
const walletInfo = getWalletData();
walletInfo.extensionId = invalidExtensionId as any;
it('throws if the `targets` field is invalid', () => {
[null, '', 42, Symbol('bar'), {}].forEach((invalidTargets) => {
const walletInfo = getWalletData();
walletInfo.targets = invalidTargets as any;

expect(() => announceWallet(walletInfo)).toThrow(
walletDataValidationError(),
);
});
});

it('throws if the `targets` field contains invalid targets', () => {
[undefined, null, '', 42, Symbol('bar'), [], {}].forEach(
(invalidTarget) => {
const walletInfo = getWalletData();
walletInfo.targets = [invalidTarget as any];

expect(() => announceWallet(walletInfo)).toThrow(
walletDataValidationError(),
);
},
);
});

it('throws if the `targets` field contains invalid target types', () => {
[undefined, null, '', 42, Symbol('bar'), [], {}].forEach(
(invalidTargetType) => {
const walletInfo = getWalletData();
walletInfo.targets = [{ type: invalidTargetType as any }];

expect(() => announceWallet(walletInfo)).toThrow(
walletDataValidationError(),
expect(() => announceWallet(walletInfo)).toThrow(
walletDataValidationError(),
);
},
);
});
});
Expand Down
26 changes: 22 additions & 4 deletions src/CAIP294.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,25 @@ declare global {
}
}

/**
* Represents the protocol/transport supported by the wallet.
* @type CAIP294Target
* @property type - The type of the target. SHOULD reference a CAIP number in the `caip-x` format.
* @property value - The value specifying how to connect to the target as specified by the specification in the `type` property.
*/
export type CAIP294Target = { type: string; value?: unknown };
Copy link
Contributor

Choose a reason for hiding this comment

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

Also should we add some links to CAIP-294 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to the PR? it hasn't been merged yet so the link isn't going to be great

Copy link
Contributor

Choose a reason for hiding this comment

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

meh ok


/**
* Represents the assets needed to display and identify a wallet.
* @type CAIP294WalletData
* @property uuid - A locally unique identifier for the wallet. MUST be a v4 UUID.
* @property name - The name of the wallet.
* @property icon - The icon for the wallet. MUST be data URI.
* @property rdns - The reverse syntax domain name identifier for the wallet.
* @property extensionId - The canonical extension ID of the wallet provider for the active browser.
* @property targets - The target objects specifying the protocol/transport supported by the wallet.
*/
export type CAIP294WalletData = BaseProviderInfo & {
extensionId?: string | undefined;
targets?: CAIP294Target[];
};

/**
Expand Down Expand Up @@ -120,6 +128,16 @@ function isValidAnnounceWalletEvent(
);
}

/**
* Validates a {@link CAIP294Target} object.
*
* @param data - The {@link CAIP294Target} to validate.
* @returns Whether the {@link CAIP294Target} is valid.
*/
function isValidWalletTarget(data: unknown): data is CAIP294Target {
return isObject(data) && typeof data.type === 'string' && Boolean(data.type);
Copy link
Contributor

Choose a reason for hiding this comment

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

is Boolean(data.type) just to check that its not an empty string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah.... it's what Erik had before i think, so i just kept the pattern

}

/**
* Validates an {@link CAIP294WalletData} object.
*
Expand All @@ -137,8 +155,8 @@ function isValidWalletData(data: unknown): data is CAIP294WalletData {
data.icon.startsWith('data:image') &&
typeof data.rdns === 'string' &&
FQDN_REGEX.test(data.rdns) &&
(data.extensionId === undefined ||
(typeof data.extensionId === 'string' && data.extensionId.length > 0))
(data.targets === undefined ||
(Array.isArray(data.targets) && data.targets.every(isValidWalletTarget)))
);
}

Expand Down
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { RequestArguments } from './BaseProvider';
import type {
CAIP294AnnounceWalletEvent,
CAIP294WalletData,
CAIP294Target,
CAIP294RequestWalletEvent,
} from './CAIP294';
import {
Expand Down Expand Up @@ -41,6 +42,7 @@ export type {
EIP6963RequestProviderEvent,
CAIP294AnnounceWalletEvent,
CAIP294WalletData as CAIP294WalletInfo,
CAIP294Target,
CAIP294RequestWalletEvent,
};

Expand Down
16 changes: 12 additions & 4 deletions src/initializeInpageProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('announceCaip294WalletData', () => {
});

describe('build type is flask', () => {
it('should announce wallet with extensionId for non-firefox browsers', async () => {
it('should announce wallet with caip-348 target for chromium browsers', async () => {
const extensionId = 'test-extension-id';
(getBuildType as jest.Mock).mockReturnValue('flask');
(mockProvider.request as jest.Mock).mockReturnValue({ extensionId });
Expand All @@ -59,18 +59,26 @@ describe('announceCaip294WalletData', () => {
expect(getBuildType).toHaveBeenCalledWith(mockProviderInfo.rdns);
expect(announceWallet).toHaveBeenCalledWith({
...mockProviderInfo,
extensionId,
targets: [
{
type: 'caip-348',
value: extensionId,
},
],
});
});

it('should announce wallet without extensionId for firefox browser', async () => {
it('should announce wallet without caip-348 target for firefox browser', async () => {
(getBuildType as jest.Mock).mockReturnValue('flask');
(mockProvider.request as jest.Mock).mockReturnValue({});

await announceCaip294WalletData(mockProvider, mockProviderInfo);

expect(getBuildType).toHaveBeenCalledWith(mockProviderInfo.rdns);
expect(announceWallet).toHaveBeenCalledWith(mockProviderInfo);
expect(announceWallet).toHaveBeenCalledWith({
...mockProviderInfo,
targets: [],
});
});
});
});
11 changes: 10 additions & 1 deletion src/initializeInpageProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,20 @@ export async function announceCaip294WalletData(
const providerState = await provider.request<{ extensionId?: string }>({
method: 'metamask_getProviderState',
});

const targets = [];

const extensionId = providerState?.extensionId;
if (extensionId) {
targets.push({
type: 'caip-348',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i've gone back and forth on whether this belongs in a constant, and I don't think it's necessary because we don't do this for other standards numbers. The value itself is already effectively a symbol and moving it to a constant doesn't really buy us additional value

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a link to the CAIP PR?

value: extensionId,
});
}

const walletData = {
...providerInfo,
extensionId,
targets,
};

announceWallet(walletData);
Expand Down
Loading