Skip to content

Commit

Permalink
fix: approve sign promise should resolve when rejected (#886)
Browse files Browse the repository at this point in the history
  • Loading branch information
jurevans authored Jun 21, 2024
1 parent 72149ab commit ff29f8e
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 42 deletions.
2 changes: 1 addition & 1 deletion apps/extension/src/background/approvals/handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ describe("approvals handler", () => {

const rejectSignArbitraryMsg = new RejectSignArbitraryMsg("");
handler(env, rejectSignArbitraryMsg);
expect(service.rejectSignature).toBeCalled();
expect(service.rejectSignArbitrary).toBeCalled();

const submitApprovedSignArbitrartyMsg = new SubmitApprovedSignArbitraryMsg(
"",
Expand Down
6 changes: 3 additions & 3 deletions apps/extension/src/background/approvals/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ const handleApproveSignTxMsg: (
const handleRejectSignTxMsg: (
service: ApprovalsService
) => InternalHandler<RejectSignTxMsg> = (service) => {
return async (_, { msgId }) => {
return await service.rejectSignTx(msgId);
return async ({ senderTabId: popupTabId }, { msgId }) => {
return await service.rejectSignTx(popupTabId, msgId);
};
};

Expand All @@ -145,7 +145,7 @@ const handleRejectSignArbitraryMsg: (
service: ApprovalsService
) => InternalHandler<RejectSignArbitraryMsg> = (service) => {
return async ({ senderTabId: popupTabId }, { msgId }) => {
return await service.rejectSignature(popupTabId, msgId);
return await service.rejectSignArbitrary(popupTabId, msgId);
};
};

Expand Down
54 changes: 21 additions & 33 deletions apps/extension/src/background/approvals/service.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { AccountType } from "@namada/types";
import { paramsToUrl } from "@namada/utils";
import { KeyRingService } from "background/keyring";
import { VaultService } from "background/vault";
Expand Down Expand Up @@ -69,7 +70,7 @@ describe("approvals service", () => {
);
});

describe("approveSignature", () => {
describe("approveSignArbitrary", () => {
it("should add popupTabId to resolverMap", async () => {
const tabId = 1;
const sigResponse = {
Expand Down Expand Up @@ -118,7 +119,7 @@ describe("approvals service", () => {
});
});

describe("submitSignature", () => {
describe("approveSignArbitrary", () => {
it("should add popupTabId to resolverMap", async () => {
const tabId = 1;
const sigResponse = {
Expand Down Expand Up @@ -200,11 +201,19 @@ describe("approvals service", () => {
});
});

describe("submitSignature", () => {
describe("approveSignTx", () => {
it("should reject resolver", async () => {
const tabId = 1;
const signer = "signer";
const signaturePromise = service.approveSignArbitrary(signer, "data");
// data expected to be base64-encoded
const txData = "dHhEYXRh"; // "txData"
const signingData = "c2lnbmluZ0RhdGE="; // "signingData"
const signaturePromise = service.approveSignTx(
AccountType.PrivateKey,
signer,
[[txData, signingData]]
);
jest.spyOn(service as any, "_clearPendingTx");

(webextensionPolyfill.windows.create as any).mockResolvedValue({
tabs: [{ id: tabId }],
Expand All @@ -215,40 +224,19 @@ describe("approvals service", () => {
resolve(true);
})
);
await service.rejectSignature(tabId, "msgId");

await expect(signaturePromise).rejects.toBeUndefined();
// Reject the pending Tx
await service.rejectSignTx(tabId, "msgId");

// rejectSignTx should clear promise resolver for that msgId
expect((service as any)._clearPendingTx).toHaveBeenCalledWith("msgId");

await expect(signaturePromise).rejects.toBeDefined();
});

it("should throw an error if resolver is not found", async () => {
const tabId = 1;
await expect(
service.rejectSignature(tabId, "msgId")
).rejects.toBeDefined();
});
});

// describe("approveTx", () => {
// it.each(txTypes)("%i txType fn: %s", async (type, paramsFn) => {
// jest.spyOn(service as any, "_launchApprovalWindow");
//
// try {
// const res = await service.approveTx(
// type,
// tx,
// AccountType.Mnemonic
// );
// expect(res).toBeUndefined();
// } catch (e) {}
// });
// });

describe("rejectSignTx", () => {
it("should clear pending tx", async () => {
jest.spyOn(service as any, "_clearPendingTx");
await service.rejectSignTx("msgId");

expect((service as any)._clearPendingTx).toHaveBeenCalledWith("msgId");
await expect(service.rejectSignTx(tabId, "msgId")).rejects.toBeDefined();
});
});

Expand Down
14 changes: 9 additions & 5 deletions apps/extension/src/background/approvals/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ export class ApprovalsService {
this.resolverMap[popupTabId] = { resolve, reject };
});
}

async approveSignArbitrary(
signer: string,
data: string
Expand Down Expand Up @@ -104,7 +103,6 @@ export class ApprovalsService {
signer: string
): Promise<void> {
const pendingTx = await this.txStore.get(msgId);
console.log("encodedTx", pendingTx);
const resolvers = this.resolverMap[popupTabId];

if (!resolvers) {
Expand Down Expand Up @@ -156,20 +154,26 @@ export class ApprovalsService {
await this._clearPendingSignature(msgId);
}

async rejectSignature(popupTabId: number, msgId: string): Promise<void> {
async rejectSignArbitrary(popupTabId: number, msgId: string): Promise<void> {
const resolvers = this.resolverMap[popupTabId];

if (!resolvers) {
throw new Error(`no resolvers found for tab ID ${popupTabId}`);
}

await this._clearPendingSignature(msgId);
resolvers.reject();
resolvers.reject(new Error("Sign arbitrary rejected"));
}

// Remove pending transaction from storage
async rejectSignTx(msgId: string): Promise<void> {
async rejectSignTx(popupTabId: number, msgId: string): Promise<void> {
const resolvers = this.resolverMap[popupTabId];
if (!resolvers) {
throw new Error(`no resolvers found for tab ID ${popupTabId}`);
}

await this._clearPendingTx(msgId);
resolvers.reject(new Error("Sign Tx rejected"));
}

async isConnectionApproved(interfaceOrigin: string): Promise<boolean> {
Expand Down

0 comments on commit ff29f8e

Please sign in to comment.