From 30a4e90679382e1d1489723c30083361d6a03a56 Mon Sep 17 00:00:00 2001 From: Arjun Dureja Date: Sun, 19 May 2024 18:11:40 -0400 Subject: [PATCH 1/4] Fix popup blocked on Safari --- .../src/core/communicator/Communicator.ts | 30 +++++++++---------- packages/wallet-sdk/src/sign/scw/SCWSigner.ts | 7 +++++ packages/wallet-sdk/src/sign/util.ts | 1 + 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/packages/wallet-sdk/src/core/communicator/Communicator.ts b/packages/wallet-sdk/src/core/communicator/Communicator.ts index b68625419f..b795569f63 100644 --- a/packages/wallet-sdk/src/core/communicator/Communicator.ts +++ b/packages/wallet-sdk/src/core/communicator/Communicator.ts @@ -62,24 +62,10 @@ export class Communicator { }); }; - /** - * Closes the popup, rejects all requests and clears the listeners - */ - private disconnect = () => { - closePopup(this.popup); - this.popup = null; - - this.listeners.forEach(({ reject }, listener) => { - reject(standardErrors.provider.userRejectedRequest('Request rejected')); - window.removeEventListener('message', listener); - }); - this.listeners.clear(); - }; - /** * Waits for the popup window to fully load and then sends a version message. */ - private waitForPopupLoaded = async (): Promise => { + waitForPopupLoaded = async (): Promise => { if (this.popup) return this.popup; this.popup = openPopup(this.url); @@ -100,4 +86,18 @@ export class Communicator { return this.popup; }); }; + + /** + * Closes the popup, rejects all requests and clears the listeners + */ + private disconnect = () => { + closePopup(this.popup); + this.popup = null; + + this.listeners.forEach(({ reject }, listener) => { + reject(standardErrors.provider.userRejectedRequest('Request rejected')); + window.removeEventListener('message', listener); + }); + this.listeners.clear(); + }; } diff --git a/packages/wallet-sdk/src/sign/scw/SCWSigner.ts b/packages/wallet-sdk/src/sign/scw/SCWSigner.ts index 3505add4fc..6d0f8fed61 100644 --- a/packages/wallet-sdk/src/sign/scw/SCWSigner.ts +++ b/packages/wallet-sdk/src/sign/scw/SCWSigner.ts @@ -22,16 +22,19 @@ type SwitchEthereumChainParam = [ export class SCWSigner implements Signer { private readonly metadata: AppMetadata; + private readonly waitForPopupLoaded: () => Promise; private readonly postMessageToPopup: (_: RPCRequestMessage) => Promise; private readonly keyManager: SCWKeyManager; private readonly stateManager: SCWStateManager; constructor(params: { metadata: AppMetadata; + waitForPopupLoaded: () => Promise; postMessageToPopup: (_: RPCRequestMessage) => Promise; updateListener: StateUpdateListener; }) { this.metadata = params.metadata; + this.waitForPopupLoaded = params.waitForPopupLoaded; this.postMessageToPopup = params.postMessageToPopup; this.keyManager = new SCWKeyManager(); this.stateManager = new SCWStateManager({ @@ -75,6 +78,10 @@ export class SCWSigner implements Signer { return localResult; } + // Open the popup before constructing the request message. + // This is to ensure that the popup is not blocked by some browsers (i.e. Safari) + await this.waitForPopupLoaded(); + const response = await this.sendEncryptedRequest(request); const decrypted = await this.decryptResponseMessage(response); this.updateInternalState(request, decrypted); diff --git a/packages/wallet-sdk/src/sign/util.ts b/packages/wallet-sdk/src/sign/util.ts index a5ec5060fa..bc7347576d 100644 --- a/packages/wallet-sdk/src/sign/util.ts +++ b/packages/wallet-sdk/src/sign/util.ts @@ -48,6 +48,7 @@ export function createSigner(params: { return new SCWSigner({ metadata, updateListener, + waitForPopupLoaded: communicator.waitForPopupLoaded, postMessageToPopup: communicator.postRequestAndWaitForResponse, }); case 'walletlink': From 076ca9899ef61503f0aa1d093a7acd096e7f783f Mon Sep 17 00:00:00 2001 From: Arjun Dureja Date: Sun, 19 May 2024 18:19:46 -0400 Subject: [PATCH 2/4] fix test --- packages/wallet-sdk/src/core/communicator/Communicator.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/wallet-sdk/src/core/communicator/Communicator.test.ts b/packages/wallet-sdk/src/core/communicator/Communicator.test.ts index 89f30de81e..6518450610 100644 --- a/packages/wallet-sdk/src/core/communicator/Communicator.test.ts +++ b/packages/wallet-sdk/src/core/communicator/Communicator.test.ts @@ -60,7 +60,6 @@ describe('Communicator', () => { it('should open a popup window', async () => { queueMessageEvent(popupLoadedMessage); - // @ts-expect-error accessing private method await communicator.waitForPopupLoaded(); expect(openPopup).toHaveBeenCalledWith(new URL(CB_KEYS_URL)); From 42daed0d3c8819053efa87d360898da4ad0c69b4 Mon Sep 17 00:00:00 2001 From: Arjun Dureja Date: Mon, 20 May 2024 17:26:35 -0400 Subject: [PATCH 3/4] Pass communicator to SCWSigner --- .../src/core/communicator/Communicator.ts | 22 +++++++++---------- packages/wallet-sdk/src/sign/scw/SCWSigner.ts | 18 +++++++-------- packages/wallet-sdk/src/sign/util.ts | 3 +-- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/packages/wallet-sdk/src/core/communicator/Communicator.ts b/packages/wallet-sdk/src/core/communicator/Communicator.ts index b795569f63..dd3929bd0c 100644 --- a/packages/wallet-sdk/src/core/communicator/Communicator.ts +++ b/packages/wallet-sdk/src/core/communicator/Communicator.ts @@ -87,17 +87,17 @@ export class Communicator { }); }; - /** + /** * Closes the popup, rejects all requests and clears the listeners */ - private disconnect = () => { - closePopup(this.popup); - this.popup = null; - - this.listeners.forEach(({ reject }, listener) => { - reject(standardErrors.provider.userRejectedRequest('Request rejected')); - window.removeEventListener('message', listener); - }); - this.listeners.clear(); - }; + private disconnect = () => { + closePopup(this.popup); + this.popup = null; + + this.listeners.forEach(({ reject }, listener) => { + reject(standardErrors.provider.userRejectedRequest('Request rejected')); + window.removeEventListener('message', listener); + }); + this.listeners.clear(); + }; } diff --git a/packages/wallet-sdk/src/sign/scw/SCWSigner.ts b/packages/wallet-sdk/src/sign/scw/SCWSigner.ts index 6d0f8fed61..253a756273 100644 --- a/packages/wallet-sdk/src/sign/scw/SCWSigner.ts +++ b/packages/wallet-sdk/src/sign/scw/SCWSigner.ts @@ -1,6 +1,7 @@ import { StateUpdateListener } from '../interface'; import { SCWKeyManager } from './SCWKeyManager'; import { SCWStateManager } from './SCWStateManager'; +import { Communicator } from ':core/communicator/Communicator'; import { standardErrors } from ':core/error'; import { RPCRequestMessage, RPCResponse, RPCResponseMessage } from ':core/message'; import { AppMetadata, RequestArguments, Signer } from ':core/provider/interface'; @@ -22,20 +23,17 @@ type SwitchEthereumChainParam = [ export class SCWSigner implements Signer { private readonly metadata: AppMetadata; - private readonly waitForPopupLoaded: () => Promise; - private readonly postMessageToPopup: (_: RPCRequestMessage) => Promise; + private readonly communicator: Communicator; private readonly keyManager: SCWKeyManager; private readonly stateManager: SCWStateManager; constructor(params: { metadata: AppMetadata; - waitForPopupLoaded: () => Promise; - postMessageToPopup: (_: RPCRequestMessage) => Promise; + communicator: Communicator; updateListener: StateUpdateListener; }) { this.metadata = params.metadata; - this.waitForPopupLoaded = params.waitForPopupLoaded; - this.postMessageToPopup = params.postMessageToPopup; + this.communicator = params.communicator; this.keyManager = new SCWKeyManager(); this.stateManager = new SCWStateManager({ appChainIds: this.metadata.appChainIds, @@ -55,7 +53,9 @@ export class SCWSigner implements Signer { params: this.metadata, }, }); - const response: RPCResponseMessage = await this.postMessageToPopup(handshakeMessage); + const response: RPCResponseMessage = await this.communicator.postRequestAndWaitForResponse( + handshakeMessage + ); // store peer's public key if ('failure' in response.content) throw response.content.failure; @@ -80,7 +80,7 @@ export class SCWSigner implements Signer { // Open the popup before constructing the request message. // This is to ensure that the popup is not blocked by some browsers (i.e. Safari) - await this.waitForPopupLoaded(); + await this.communicator.waitForPopupLoaded(); const response = await this.sendEncryptedRequest(request); const decrypted = await this.decryptResponseMessage(response); @@ -142,7 +142,7 @@ export class SCWSigner implements Signer { ); const message = await this.createRequestMessage({ encrypted }); - return this.postMessageToPopup(message); + return this.communicator.postRequestAndWaitForResponse(message); } private async createRequestMessage( diff --git a/packages/wallet-sdk/src/sign/util.ts b/packages/wallet-sdk/src/sign/util.ts index bc7347576d..3eff22f048 100644 --- a/packages/wallet-sdk/src/sign/util.ts +++ b/packages/wallet-sdk/src/sign/util.ts @@ -48,8 +48,7 @@ export function createSigner(params: { return new SCWSigner({ metadata, updateListener, - waitForPopupLoaded: communicator.waitForPopupLoaded, - postMessageToPopup: communicator.postRequestAndWaitForResponse, + communicator, }); case 'walletlink': return new WalletLinkSigner({ From 5714919c777e0efbac56f230c75c39a5d311b28e Mon Sep 17 00:00:00 2001 From: Arjun Dureja Date: Mon, 20 May 2024 17:33:54 -0400 Subject: [PATCH 4/4] move disconnect method back --- .../src/core/communicator/Communicator.ts | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/wallet-sdk/src/core/communicator/Communicator.ts b/packages/wallet-sdk/src/core/communicator/Communicator.ts index dd3929bd0c..356735ef01 100644 --- a/packages/wallet-sdk/src/core/communicator/Communicator.ts +++ b/packages/wallet-sdk/src/core/communicator/Communicator.ts @@ -62,6 +62,20 @@ export class Communicator { }); }; + /** + * Closes the popup, rejects all requests and clears the listeners + */ + private disconnect = () => { + closePopup(this.popup); + this.popup = null; + + this.listeners.forEach(({ reject }, listener) => { + reject(standardErrors.provider.userRejectedRequest('Request rejected')); + window.removeEventListener('message', listener); + }); + this.listeners.clear(); + }; + /** * Waits for the popup window to fully load and then sends a version message. */ @@ -86,18 +100,4 @@ export class Communicator { return this.popup; }); }; - - /** - * Closes the popup, rejects all requests and clears the listeners - */ - private disconnect = () => { - closePopup(this.popup); - this.popup = null; - - this.listeners.forEach(({ reject }, listener) => { - reject(standardErrors.provider.userRejectedRequest('Request rejected')); - window.removeEventListener('message', listener); - }); - this.listeners.clear(); - }; }