From 120f9e6d92c6b994cc0e48b5d0700ff1afb33687 Mon Sep 17 00:00:00 2001 From: Mateusz Jasiuk Date: Wed, 28 Feb 2024 12:45:58 +0100 Subject: [PATCH] tests: approvals tests (#628) * test: approvals handler test * test: approvals messages and service tests * test: approvals service test cd --- apps/extension/package.json | 3 +- .../src/background/approvals/handler.test.ts | 98 ++++ .../src/background/approvals/messages.test.ts | 128 +++++ .../src/background/approvals/messages.ts | 10 +- .../src/background/approvals/service.test.ts | 481 ++++++++++++++++++ .../src/background/approvals/service.ts | 14 +- yarn.lock | 8 + 7 files changed, 735 insertions(+), 7 deletions(-) create mode 100644 apps/extension/src/background/approvals/handler.test.ts create mode 100644 apps/extension/src/background/approvals/messages.test.ts create mode 100644 apps/extension/src/background/approvals/service.test.ts diff --git a/apps/extension/package.json b/apps/extension/package.json index 815611a7a7..45e2357e8d 100644 --- a/apps/extension/package.json +++ b/apps/extension/package.json @@ -26,7 +26,7 @@ "start:chrome:proxy": "NAMADA_INTERFACE_PROXY=true yarn start:chrome", "start:firefox": "yarn clean:firefox && NODE_ENV=development TARGET=firefox yarn watch", "start:firefox:proxy": "NAMADA_INTERFACE_PROXY=true yarn start:firefox", - "test": "./scripts/build-node.sh && yarn jest", + "test": "./scripts/build-node.sh && yarn jest --coverage", "test:watch": "./scripts/build-node.sh && yarn jest --watchAll=true", "test:ci": "jest", "wasm:build": "node ./scripts/build.js --release", @@ -88,6 +88,7 @@ "fake-indexeddb": "^4.0.1", "file-loader": "^6.2.0", "jest": "^29.0.1", + "jest-create-mock-instance": "^2.0.0", "jest-environment-jsdom": "^29.3.1", "leb128": "^0.0.5", "merge-jsons-webpack-plugin": "^2.0.1", diff --git a/apps/extension/src/background/approvals/handler.test.ts b/apps/extension/src/background/approvals/handler.test.ts new file mode 100644 index 0000000000..328798356e --- /dev/null +++ b/apps/extension/src/background/approvals/handler.test.ts @@ -0,0 +1,98 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { TxType } from "@namada/shared"; +import { AccountType } from "@namada/types"; +import createMockInstance from "jest-create-mock-instance"; +import { + ApproveConnectInterfaceMsg, + ApproveSignArbitraryMsg, + ApproveTxMsg, +} from "provider"; +import { Message } from "router"; +import { getHandler } from "./handler"; +import { + ConnectInterfaceResponseMsg, + RejectSignatureMsg, + RejectTxMsg, + RevokeConnectionMsg, + SubmitApprovedSignatureMsg, + SubmitApprovedTxMsg, +} from "./messages"; +import { ApprovalsService } from "./service"; + +jest.mock("webextension-polyfill", () => ({})); + +class UnknownMsg extends Message { + validate(): void {} + route(): string { + return "unknown"; + } + type(): string { + return "unknown"; + } +} + +describe.only("approvals handler", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test("handlers switch", () => { + const service: jest.Mocked = createMockInstance( + ApprovalsService as any + ); + const handler = getHandler(service); + const env = { + isInternalMsg: true, + senderTabId: 1, + requestInteraction: () => {}, + }; + + const approveTxMsg = new ApproveTxMsg( + TxType.Bond, + "txMsg", + "specificMsg", + AccountType.Mnemonic + ); + handler(env, approveTxMsg); + expect(service.approveTx).toBeCalled(); + + const rejectTxMsg = new RejectTxMsg("msgId"); + handler(env, rejectTxMsg); + expect(service.rejectTx).toBeCalled(); + + const submitApprovedTxMsg = new SubmitApprovedTxMsg(TxType.Bond, "msgId"); + handler(env, submitApprovedTxMsg); + expect(service.submitTx).toBeCalled(); + + const approveConnectInterfaceMsg = new ApproveConnectInterfaceMsg(); + handler(env, approveConnectInterfaceMsg); + expect(service.approveConnection).toBeCalled(); + + const connectInterfaceResponseMsg = new ConnectInterfaceResponseMsg( + 0, + "", + true + ); + handler(env, connectInterfaceResponseMsg); + expect(service.approveConnectionResponse).toBeCalled(); + + const revokeConnectionMsg = new RevokeConnectionMsg(""); + handler(env, revokeConnectionMsg); + expect(service.revokeConnection).toBeCalled(); + + const approveSignArbitraryMsg = new ApproveSignArbitraryMsg("", ""); + handler(env, approveSignArbitraryMsg); + expect(service.approveSignature).toBeCalled(); + + const rejectSignatureMsg = new RejectSignatureMsg(""); + handler(env, rejectSignatureMsg); + expect(service.rejectSignature).toBeCalled(); + + const submitApprovedSignatureMsg = new SubmitApprovedSignatureMsg("", ""); + handler(env, submitApprovedSignatureMsg); + expect(service.submitSignature).toBeCalled(); + + const unknownMsg = new UnknownMsg(); + expect(() => handler(env, unknownMsg)).toThrow(); + }); +}); diff --git a/apps/extension/src/background/approvals/messages.test.ts b/apps/extension/src/background/approvals/messages.test.ts new file mode 100644 index 0000000000..6ad25a4efa --- /dev/null +++ b/apps/extension/src/background/approvals/messages.test.ts @@ -0,0 +1,128 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { TxType } from "@namada/shared"; +import { ROUTE } from "./constants"; +import { + ConnectInterfaceResponseMsg, + MessageType, + RejectSignatureMsg, + RejectTxMsg, + RevokeConnectionMsg, + SubmitApprovedSignatureMsg, + SubmitApprovedTxMsg, +} from "./messages"; + +jest.mock("webextension-polyfill", () => ({})); + +describe("approvals messages", () => { + test("valid RejectTxMsg", () => { + const msg = new RejectTxMsg("msgId"); + + expect(msg.type()).toBe(MessageType.RejectTx); + expect(msg.route()).toBe(ROUTE); + expect(msg.validate()).toBeUndefined(); + }); + + test("invalid RejectTxMsg", () => { + const msg = new RejectTxMsg("msgId"); + (msg as any).msgId = undefined; + + expect(() => msg.validate()).toThrow(); + }); + + test("valid SubmitApprovedTxMsg", () => { + const msg = new SubmitApprovedTxMsg(TxType.Bond, "msgId"); + + expect(msg.type()).toBe(MessageType.SubmitApprovedTx); + expect(msg.route()).toBe(ROUTE); + expect(msg.validate()).toBeUndefined(); + }); + + test("invalid SubmitApprovedTxMsg", () => { + const msg = new SubmitApprovedTxMsg(TxType.Bond, "msgId"); + (msg as any).msgId = undefined; + + expect(() => msg.validate()).toThrow(); + + const msg2 = new SubmitApprovedTxMsg(TxType.Bond, "msgId"); + (msg2 as any).txType = undefined; + + expect(() => msg2.validate()).toThrow(); + }); + + test("valid RejectSignatureMsg", () => { + const msg = new RejectSignatureMsg("msgId"); + + expect(msg.type()).toBe(MessageType.RejectSignature); + expect(msg.route()).toBe(ROUTE); + expect(msg.validate()).toBeUndefined(); + }); + + test("invalid RejectSignatureMsg", () => { + const msg = new RejectSignatureMsg("msgId"); + (msg as any).msgId = undefined; + + expect(() => msg.validate()).toThrow(); + }); + + test("valid SubmitApprovedSignatureMsg", () => { + const msg = new SubmitApprovedSignatureMsg("msgId", "signer"); + + expect(msg.type()).toBe(MessageType.SubmitApprovedSignature); + expect(msg.route()).toBe(ROUTE); + expect(msg.validate()).toBeUndefined(); + }); + + test("invalid SubmitApprovedSignatureMsg", () => { + const msg = new SubmitApprovedSignatureMsg("msgId", "signer"); + (msg as any).msgId = undefined; + + expect(() => msg.validate()).toThrow(); + + const msg2 = new SubmitApprovedSignatureMsg("msgId", "signer"); + (msg2 as any).signer = undefined; + + expect(() => msg2.validate()).toThrow(); + }); + + test("valid ConnectInterfaceResponseMsg", () => { + const msg = new ConnectInterfaceResponseMsg(0, "interface", true); + + expect(msg.type()).toBe(MessageType.ConnectInterfaceResponse); + expect(msg.route()).toBe(ROUTE); + expect(msg.validate()).toBeUndefined(); + }); + + test("invalid ConnectInterfaceResponseMsg", () => { + const msg = new ConnectInterfaceResponseMsg(0, "interface", true); + + (msg as any).interfaceTabId = undefined; + + expect(() => msg.validate()).toThrow(); + + const msg2 = new ConnectInterfaceResponseMsg(0, "interface", true); + (msg2 as any).interfaceOrigin = undefined; + + expect(() => msg2.validate()).toThrow(); + + const msg3 = new ConnectInterfaceResponseMsg(0, "interface", true); + (msg3 as any).allowConnection = undefined; + + expect(() => msg3.validate()).toThrow(); + }); + + test("valid RevokeConnectionMsg", () => { + const msg = new RevokeConnectionMsg(""); + + expect(msg.type()).toBe(MessageType.RevokeConnection); + expect(msg.route()).toBe(ROUTE); + expect(msg.validate()).toBeUndefined(); + }); + + test("invalid RevokeConnectionMsg", () => { + const msg = new RevokeConnectionMsg(""); + + (msg as any).originToRevoke = undefined; + + expect(() => msg.validate()).toThrow(); + }); +}); diff --git a/apps/extension/src/background/approvals/messages.ts b/apps/extension/src/background/approvals/messages.ts index 24d057410c..78fdf081a9 100644 --- a/apps/extension/src/background/approvals/messages.ts +++ b/apps/extension/src/background/approvals/messages.ts @@ -4,7 +4,7 @@ import { ROUTE } from "./constants"; import { validateProps } from "utils"; -enum MessageType { +export enum MessageType { SubmitApprovedTx = "submit-approved-tx", RejectTx = "reject-tx", SubmitApprovedSignature = "submit-approved-signature", @@ -127,13 +127,17 @@ export class ConnectInterfaceResponseMsg extends Message { } validate(): void { - if (!this.interfaceTabId) { + if (typeof this.interfaceTabId === "undefined") { throw new Error("interfaceTabId not set"); } if (!this.interfaceOrigin) { throw new Error("interfaceOrigin not set"); } + + if (typeof this.allowConnection === "undefined") { + throw new Error("allowConnection not set"); + } } route(): string { @@ -155,7 +159,7 @@ export class RevokeConnectionMsg extends Message { } validate(): void { - if (!this.originToRevoke) { + if (typeof this.originToRevoke === "undefined") { throw new Error("originToRevoke not set"); } } diff --git a/apps/extension/src/background/approvals/service.test.ts b/apps/extension/src/background/approvals/service.test.ts new file mode 100644 index 0000000000..7d3c2cff6b --- /dev/null +++ b/apps/extension/src/background/approvals/service.test.ts @@ -0,0 +1,481 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import * as borsh from "@dao-xyz/borsh"; +import { TxType } from "@namada/shared"; +import { + AccountType, + EthBridgeTransferMsgValue, + IbcTransferMsgValue, + SubmitBondMsgValue, + SubmitUnbondMsgValue, + SubmitVoteProposalMsgValue, + SubmitWithdrawMsgValue, + TokenInfo, + TransferMsgValue, +} from "@namada/types"; +import { KeyRingService, TabStore } from "background/keyring"; +import { LedgerService } from "background/ledger"; +import { VaultService } from "background/vault"; +import BigNumber from "bignumber.js"; +import createMockInstance from "jest-create-mock-instance"; +import { KVStoreMock } from "test/init"; +import { ApprovalsService } from "./service"; +import { ApprovedOriginsStore, TxStore } from "./types"; + +jest.mock("webextension-polyfill", () => ({ + runtime: { + getURL: () => "url", + }, + windows: { + create: jest.fn(), + }, +})); + +describe.only("approvals service", () => { + let service: ApprovalsService; + let keyRingService: jest.Mocked; + let dataStore: KVStoreMock; + let txStore: KVStoreMock; + + afterEach(() => { + jest.restoreAllMocks(); + }); + + beforeEach(() => { + jest.clearAllMocks(); + txStore = new KVStoreMock("TxStore"); + dataStore = new KVStoreMock("DataStore"); + const connectedTabsStore = new KVStoreMock("TabStore"); + const approvedOriginsStore = new KVStoreMock( + "ApprovedOriginsStore" + ); + keyRingService = createMockInstance(KeyRingService as any); + const ledgerService: jest.Mocked = createMockInstance( + LedgerService as any + ); + const vaultService: jest.Mocked = createMockInstance( + VaultService as any + ); + + service = new ApprovalsService( + txStore, + dataStore, + connectedTabsStore, + approvedOriginsStore, + keyRingService, + ledgerService, + vaultService + ); + }); + + describe("approveSignature", () => { + it("should add popupTabId to resolverMap", async () => { + const tabId = 1; + const sigResponse = { + hash: "hash", + signature: "sig", + }; + + jest.spyOn(service as any, "getPopupTabId").mockResolvedValue(tabId); + const signaturePromise = service.approveSignature("signer", "data"); + + await new Promise((resolve) => + setTimeout(() => { + resolve(true); + }) + ); + + (service as any).resolverMap[tabId].resolve(sigResponse); + const signature = await signaturePromise; + + expect(signature).toEqual(sigResponse); + }); + + it("should throw an error when popupTabId is not present", async () => { + jest.spyOn(service as any, "getPopupTabId").mockResolvedValue(undefined); + + expect(service.approveSignature("signer", "data")).rejects.toBeDefined(); + }); + + it("should throw an error when popupTabId is already in the map", async () => { + const tabId = 1; + const sigResponse = { + hash: "hash", + signature: "sig", + }; + jest.spyOn(service as any, "getPopupTabId").mockResolvedValue(tabId); + (service as any).resolverMap[tabId] = sigResponse; + + expect(service.approveSignature("signer", "data")).rejects.toBeDefined(); + }); + }); + + describe("submitSignature", () => { + it("should add popupTabId to resolverMap", async () => { + const tabId = 1; + const sigResponse = { + hash: "hash", + signature: "sig", + }; + + jest.spyOn(service as any, "getPopupTabId").mockResolvedValue(tabId); + jest.spyOn(dataStore, "get").mockResolvedValueOnce("data"); + jest + .spyOn(keyRingService, "signArbitrary") + .mockResolvedValue(sigResponse); + const signer = "signer"; + const signaturePromise = service.approveSignature(signer, "data"); + + await new Promise((resolve) => + setTimeout(() => { + resolve(true); + }) + ); + await service.submitSignature(tabId, "msgId", signer); + + expect(await signaturePromise).toEqual(sigResponse); + }); + + it("should throw an error when resolvers are not present", async () => { + const tabId = 1; + const signer = "signer"; + jest.spyOn(dataStore, "get").mockResolvedValueOnce("data"); + + expect( + service.submitSignature(tabId, "msgId", signer) + ).rejects.toBeDefined(); + }); + + it("should throw an error when data is not present", async () => { + const tabId = 1; + const signer = "signer"; + const sigResponse = { + hash: "hash", + signature: "sig", + }; + (service as any).resolverMap[tabId] = sigResponse; + jest.spyOn(dataStore, "get").mockResolvedValueOnce(undefined); + + expect( + service.submitSignature(tabId, "msgId", signer) + ).rejects.toBeDefined(); + }); + + it("should reject promise if can't sign", async () => { + const tabId = 1; + + jest.spyOn(service as any, "getPopupTabId").mockResolvedValue(tabId); + jest.spyOn(dataStore, "get").mockResolvedValueOnce("data"); + jest + .spyOn(keyRingService, "signArbitrary") + .mockRejectedValue("Can't sign"); + const signer = "signer"; + + await new Promise((resolve) => + setTimeout(() => { + resolve(true); + }) + ); + + expect( + service.submitSignature(tabId, "msgId", signer) + ).rejects.toBeDefined(); + }); + }); + + describe("submitSignature", () => { + it("should reject resolver", async () => { + const tabId = 1; + + jest.spyOn(service as any, "getPopupTabId").mockResolvedValue(tabId); + const signer = "signer"; + const signaturePromise = service.approveSignature(signer, "data"); + + await new Promise((resolve) => + setTimeout(() => { + resolve(true); + }) + ); + await service.rejectSignature(tabId, "msgId"); + + expect(signaturePromise).rejects.toEqual(undefined); + }); + + it("should throw an error if resolver is not found", async () => { + const tabId = 1; + + expect(service.rejectSignature(tabId, "msgId")).rejects.toBeDefined(); + }); + }); + + const txTypes = [ + [TxType.Bond, "getParamsBond"], + [TxType.Unbond, "getParamsUnbond"], + [TxType.Withdraw, "getParamsWithdraw"], + [TxType.Transfer, "getParamsTransfer"], + [TxType.IBCTransfer, "getParamsIbcTransfer"], + [TxType.EthBridgeTransfer, "getParamsEthBridgeTransfer"], + [TxType.VoteProposal, "getParamsVoteProposal"], + ] as const; + + describe("approveTx", () => { + it.each(txTypes)("should launch tx", async (type, paramsFn) => { + jest.spyOn(ApprovalsService, paramsFn).mockImplementation(() => ({})); + jest.spyOn(borsh, "deserialize").mockReturnValue({}); + jest.spyOn(service as any, "_launchApprovalWindow"); + + expect( + service.approveTx(type, "", "", AccountType.Mnemonic) + ).resolves.not.toBeDefined(); + }); + }); + + describe("getParamsTransfer", () => { + it("should return transfer params", () => { + const transferMsgValue = new TransferMsgValue({ + source: "source", + target: "target", + token: "token", + amount: BigNumber(100), + nativeToken: "nativeToken", + }); + + const txMsgValue = { + token: "token", + feeAmount: BigNumber(0.5), + gasLimit: BigNumber(0.5), + chainId: "chainId", + publicKey: "publicKey", + }; + + jest.spyOn(borsh, "deserialize").mockReturnValue(transferMsgValue); + + const params = ApprovalsService.getParamsTransfer( + new Uint8Array([]), + txMsgValue + ); + + expect(params).toEqual({ + source: transferMsgValue.source, + target: transferMsgValue.target, + tokenAddress: transferMsgValue.token, + amount: transferMsgValue.amount.toString(), + publicKey: txMsgValue.publicKey, + }); + }); + }); + + describe("getParamsIbcTransfer", () => { + it("should return transfer params", () => { + const token: TokenInfo = { + symbol: "symbol", + type: 0, + path: 0, + coin: "coin", + url: "url", + address: "address", + }; + + const transferMsgValue = new IbcTransferMsgValue({ + source: "source", + receiver: "target", + token: token, + amount: BigNumber(100), + portId: "portId", + channelId: "channelId", + }); + + const txMsgValue = { + token: "token", + feeAmount: BigNumber(0.5), + gasLimit: BigNumber(0.5), + chainId: "chainId", + publicKey: "publicKey", + }; + + jest.spyOn(borsh, "deserialize").mockReturnValue(transferMsgValue); + + const params = ApprovalsService.getParamsIbcTransfer( + new Uint8Array([]), + txMsgValue + ); + + expect(params).toEqual({ + source: transferMsgValue.source, + target: transferMsgValue.receiver, + tokenAddress: transferMsgValue.token, + amount: transferMsgValue.amount.toString(), + publicKey: txMsgValue.publicKey, + }); + }); + }); + + describe("getParamsEthBridgeTransfer", () => { + it("should return transfer params", () => { + const transferMsgValue = new EthBridgeTransferMsgValue({ + nut: false, + asset: "asset", + recipient: "recipient", + sender: "sender", + amount: BigNumber(100), + feeAmount: BigNumber(0.5), + feeToken: "feeToken", + }); + + const txMsgValue = { + token: "token", + feeAmount: BigNumber(0.5), + gasLimit: BigNumber(0.5), + chainId: "chainId", + publicKey: "publicKey", + }; + + jest.spyOn(borsh, "deserialize").mockReturnValue(transferMsgValue); + + const params = ApprovalsService.getParamsEthBridgeTransfer( + new Uint8Array([]), + txMsgValue + ); + + expect(params).toEqual({ + source: transferMsgValue.sender, + target: transferMsgValue.recipient, + tokenAddress: transferMsgValue.asset, + amount: transferMsgValue.amount.toString(), + publicKey: txMsgValue.publicKey, + }); + }); + }); + + describe("getParamsBond", () => { + it("should return bond params", () => { + const bondMsgValue = new SubmitBondMsgValue({ + source: "source", + validator: "validator", + amount: BigNumber(100), + nativeToken: "nativeToken", + }); + + const txMsgValue = { + token: "token", + feeAmount: BigNumber(0.5), + gasLimit: BigNumber(0.5), + chainId: "chainId", + publicKey: "publicKey", + }; + + jest.spyOn(borsh, "deserialize").mockReturnValue(bondMsgValue); + + const params = ApprovalsService.getParamsBond( + new Uint8Array([]), + txMsgValue + ); + + expect(params).toEqual({ + source: bondMsgValue.source, + tokenAddress: bondMsgValue.nativeToken, + amount: bondMsgValue.amount.toString(), + publicKey: txMsgValue.publicKey, + }); + }); + }); + + describe("getParamsUnbond", () => { + it("should return unbond params", () => { + const unbondMsgValue = new SubmitUnbondMsgValue({ + source: "source", + validator: "validator", + amount: BigNumber(100), + }); + + const txMsgValue = { + token: "token", + feeAmount: BigNumber(0.5), + gasLimit: BigNumber(0.5), + chainId: "chainId", + publicKey: "publicKey", + }; + + jest.spyOn(borsh, "deserialize").mockReturnValue(unbondMsgValue); + + const params = ApprovalsService.getParamsUnbond( + new Uint8Array([]), + txMsgValue + ); + + expect(params).toEqual({ + source: unbondMsgValue.source, + amount: unbondMsgValue.amount.toString(), + publicKey: txMsgValue.publicKey, + }); + }); + }); + + describe("getParamsWithdraw", () => { + it("should return withdraw params", () => { + const withdrawMsgValue = new SubmitWithdrawMsgValue({ + source: "source", + validator: "validator", + }); + + const txMsgValue = { + token: "token", + feeAmount: BigNumber(0.5), + gasLimit: BigNumber(0.5), + chainId: "chainId", + publicKey: "publicKey", + }; + + jest.spyOn(borsh, "deserialize").mockReturnValue(withdrawMsgValue); + + const params = ApprovalsService.getParamsWithdraw( + new Uint8Array([]), + txMsgValue + ); + + expect(params).toEqual({ + source: withdrawMsgValue.source, + validator: withdrawMsgValue.validator, + publicKey: txMsgValue.publicKey, + }); + }); + }); + + describe("getParamsVoteProposal", () => { + it("should return vote proposal params", () => { + const voteProposalMsgValue = new SubmitVoteProposalMsgValue({ + signer: "singer", + proposalId: BigInt(0), + vote: "yay", + }); + + const txMsgValue = { + token: "token", + feeAmount: BigNumber(0.5), + gasLimit: BigNumber(0.5), + chainId: "chainId", + publicKey: "publicKey", + }; + + jest.spyOn(borsh, "deserialize").mockReturnValue(voteProposalMsgValue); + + const params = ApprovalsService.getParamsVoteProposal( + new Uint8Array([]), + txMsgValue + ); + + expect(params).toEqual({ + source: voteProposalMsgValue.signer, + publicKey: txMsgValue.publicKey, + }); + }); + }); + + describe("rejectTx", () => { + it("should clear pending tx", async () => { + jest.spyOn(service as any, "_clearPendingTx"); + await service.rejectTx("msgId"); + + expect((service as any)._clearPendingTx).toHaveBeenCalledWith("msgId"); + }); + }); +}); diff --git a/apps/extension/src/background/approvals/service.ts b/apps/extension/src/background/approvals/service.ts index 07a1708a95..03eff62b7e 100644 --- a/apps/extension/src/background/approvals/service.ts +++ b/apps/extension/src/background/approvals/service.ts @@ -53,7 +53,7 @@ export class ApprovalsService { protected readonly keyRingService: KeyRingService, protected readonly ledgerService: LedgerService, protected readonly vaultService: VaultService - ) { } + ) {} async approveSignature( signer: string, @@ -69,9 +69,9 @@ export class ApprovalsService { const url = paramsToUrl(baseUrl, { msgId, }); - const approvalWindow = await this._launchApprovalWindow(url); - const popupTabId = approvalWindow.tabs?.[0]?.id; + const popupTabId = await this.getPopupTabId(url); + // TODO: can tabId be 0? if (!popupTabId) { throw new Error("no popup tab ID"); } @@ -100,6 +100,7 @@ export class ApprovalsService { if (!data) { throw new Error(`Signing data for ${msgId} not found!`); } + //TODO: Shouldn't we _clearPendingSignature when throwing? try { const signature = await this.keyRingService.signArbitrary(signer, data); @@ -421,4 +422,11 @@ export class ApprovalsService { type: "popup", }); }; + + private getPopupTabId = async (url: string): Promise => { + const window = await this._launchApprovalWindow(url); + const popupTabId = window.tabs?.[0]?.id; + + return popupTabId; + }; } diff --git a/yarn.lock b/yarn.lock index c8bcaca90e..17e8c18a3e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7612,6 +7612,7 @@ __metadata: file-loader: "npm:^6.2.0" framer-motion: "npm:6.2.4" jest: "npm:^29.0.1" + jest-create-mock-instance: "npm:^2.0.0" jest-environment-jsdom: "npm:^29.3.1" js-sha256: "npm:^0.10.1" leb128: "npm:^0.0.5" @@ -20718,6 +20719,13 @@ __metadata: languageName: node linkType: hard +"jest-create-mock-instance@npm:^2.0.0": + version: 2.0.0 + resolution: "jest-create-mock-instance@npm:2.0.0" + checksum: 418b06d0cbd2e02ad090d0be4e0d67c7c304997f9a819deeb65e75e538f48416ace5fbae440866c70d71ced3ea54930898526931b31903ba803e3ae25f0e7d87 + languageName: node + linkType: hard + "jest-dev-server@npm:^9.0.0": version: 9.0.0 resolution: "jest-dev-server@npm:9.0.0"