Clean up syntax (#1060)

* clean up cipher * update session variable * keyed constructor * setConnected * implements WalletLinkConnectionUpdateListener / `listener: this,` * handleWeb3ResponseMessage * remove deprecated code * tests
coinbase · Nov 14, 2023 · 23f4f9e · 23f4f9e
1 parent 0070e80
commit 23f4f9e
Show file tree

Hide file tree

Showing 11 changed files with 221 additions and 260 deletions.
diff --git a/packages/wallet-sdk/src/CoinbaseWalletSDK.ts b/packages/wallet-sdk/src/CoinbaseWalletSDK.ts
@@ -3,7 +3,6 @@
 
 import { LogoType, walletLogo } from './assets/wallet-logo';
 import { DiagnosticLogger } from './connection/DiagnosticLogger';
-import { EventListener } from './connection/EventListener';
 import { LINK_API_URL } from './constants';
 import { ScopedLocalStorage } from './lib/ScopedLocalStorage';
 import { CoinbaseWalletProvider } from './provider/CoinbaseWalletProvider';
@@ -28,9 +27,6 @@ export interface CoinbaseWalletSDKOptions {
   linkAPIUrl?: string;
   /** @optional an implementation of WalletUI; for most, leave it unspecified */
   uiConstructor?: (options: Readonly<WalletUIOptions>) => WalletUI;
-  /** @optional an implementation of EventListener for debugging; for most, leave it unspecified  */
-  /** @deprecated in favor of diagnosticLogger */
-  eventListener?: EventListener;
   /** @optional a diagnostic tool for debugging; for most, leave it unspecified  */
   diagnosticLogger?: DiagnosticLogger;
   /** @optional whether wallet link provider should override the isMetaMask property. */
@@ -76,20 +72,7 @@ export class CoinbaseWalletSDK {
     this._overrideIsCoinbaseWallet = options.overrideIsCoinbaseWallet ?? true;
     this._overrideIsCoinbaseBrowser = options.overrideIsCoinbaseBrowser ?? false;
 
-    if (options.diagnosticLogger && options.eventListener) {
-      throw new Error(
-        "Can't have both eventListener and diagnosticLogger options, use only diagnosticLogger"
-      );
-    }
-
-    if (options.eventListener) {
-      this._diagnosticLogger = {
-        // eslint-disable-next-line @typescript-eslint/unbound-method
-        log: options.eventListener.onEvent,
-      };
-    } else {
-      this._diagnosticLogger = options.diagnosticLogger;
-    }
+    this._diagnosticLogger = options.diagnosticLogger;
 
     this._reloadOnDisconnect = options.reloadOnDisconnect ?? true;
 

diff --git a/packages/wallet-sdk/src/connection/EventListener.ts b/packages/wallet-sdk/src/connection/EventListener.ts
diff --git a/packages/wallet-sdk/src/connection/WalletLinkConnection.test.ts b/packages/wallet-sdk/src/connection/WalletLinkConnection.test.ts
@@ -18,14 +18,18 @@ describe('WalletLinkConnection', () => {
   beforeEach(() => {
     jest.clearAllMocks();
 
-    connection = new WalletLinkConnection(session, 'http://link-api-url', {
-      linkedUpdated: jest.fn(),
-      connectedUpdated: jest.fn(),
-      handleResponseMessage: jest.fn(),
-      chainUpdated: jest.fn(),
-      accountUpdated: jest.fn(),
-      metadataUpdated: jest.fn(),
-      resetAndReload: jest.fn(),
+    connection = new WalletLinkConnection({
+      session,
+      linkAPIUrl: 'http://link-api-url',
+      listener: {
+        linkedUpdated: jest.fn(),
+        connectedUpdated: jest.fn(),
+        handleWeb3ResponseMessage: jest.fn(),
+        chainUpdated: jest.fn(),
+        accountUpdated: jest.fn(),
+        metadataUpdated: jest.fn(),
+        resetAndReload: jest.fn(),
+      },
     });
     listener = (connection as any).listener;
   });

diff --git a/packages/wallet-sdk/src/connection/WalletLinkConnection.ts b/packages/wallet-sdk/src/connection/WalletLinkConnection.ts
@@ -38,25 +38,36 @@ const REQUEST_TIMEOUT = 60000;
 export interface WalletLinkConnectionUpdateListener {
   linkedUpdated: (linked: boolean) => void;
   connectedUpdated: (connected: boolean) => void;
-  handleResponseMessage: (message: Web3ResponseMessage) => void;
+  handleWeb3ResponseMessage: (message: Web3ResponseMessage) => void;
   chainUpdated: (chainId: string, jsonRpcUrl: string) => void;
   accountUpdated: (selectedAddress: string) => void;
   metadataUpdated: (key: string, metadataValue: string) => void;
   resetAndReload: () => void;
 }
 
+interface WalletLinkConnectionParams {
+  session: Session;
+  linkAPIUrl: string;
+  listener: WalletLinkConnectionUpdateListener;
+  diagnostic?: DiagnosticLogger;
+  WebSocketClass?: typeof WebSocket;
+}
+
 /**
  * Coinbase Wallet Connection
  */
 export class WalletLinkConnection {
-  private ws: WalletLinkWebSocket;
-  private http: WalletLinkHTTP;
-  private cipher: WalletLinkConnectionCipher;
   private destroyed = false;
   private lastHeartbeatResponse = 0;
   private nextReqId = IntNumber(1);
 
+  private readonly session: Session;
+
   private listener?: WalletLinkConnectionUpdateListener;
+  private diagnostic?: DiagnosticLogger;
+  private cipher: WalletLinkConnectionCipher;
+  private ws: WalletLinkWebSocket;
+  private http: WalletLinkHTTP;
 
   /**
    * Constructor
@@ -65,27 +76,24 @@ export class WalletLinkConnection {
    * @param listener WalletLinkConnectionUpdateListener
    * @param [WebSocketClass] Custom WebSocket implementation
    */
-  private sessionId: string;
-  private sessionKey: string;
-  constructor(
-    session: Session,
-    linkAPIUrl: string,
-    listener: WalletLinkConnectionUpdateListener,
-    private diagnostic?: DiagnosticLogger,
-    WebSocketClass: typeof WebSocket = WebSocket
-  ) {
-    const sessionId = (this.sessionId = session.id);
-    const sessionKey = (this.sessionKey = session.key);
+  constructor({
+    session,
+    linkAPIUrl,
+    listener,
+    diagnostic,
+    WebSocketClass = WebSocket,
+  }: WalletLinkConnectionParams) {
+    this.session = session;
     this.cipher = new WalletLinkConnectionCipher(session.secret);
-
+    this.diagnostic = diagnostic;
     this.listener = listener;
 
     const ws = new WalletLinkWebSocket(`${linkAPIUrl}/rpc`, WebSocketClass);
     ws.setConnectionStateListener(async (state) => {
       // attempt to reconnect every 5 seconds when disconnected
       this.diagnostic?.log(EVENTS.CONNECTED_STATE_CHANGE, {
         state,
-        sessionIdHash: Session.hash(sessionId),
+        sessionIdHash: Session.hash(session.id),
       });
 
       let connected = false;
@@ -156,7 +164,7 @@ export class WalletLinkConnection {
         case 'Linked': {
           const msg = m as Omit<ServerMessageIsLinkedOK, 'type'> & ServerMessageLinked;
           this.diagnostic?.log(EVENTS.LINKED, {
-            sessionIdHash: Session.hash(sessionId),
+            sessionIdHash: Session.hash(session.id),
             linked: msg.linked,
             type: m.type,
             onlineGuests: msg.onlineGuests,
@@ -172,7 +180,7 @@ export class WalletLinkConnection {
           const msg = m as Omit<ServerMessageGetSessionConfigOK, 'type'> &
             ServerMessageSessionConfigUpdated;
           this.diagnostic?.log(EVENTS.SESSION_CONFIG_RECEIVED, {
-            sessionIdHash: Session.hash(sessionId),
+            sessionIdHash: Session.hash(session.id),
             metadata_keys: msg && msg.metadata ? Object.keys(msg.metadata) : undefined,
           });
           this.handleSessionMetadataUpdated(msg.metadata);
@@ -192,7 +200,7 @@ export class WalletLinkConnection {
     });
     this.ws = ws;
 
-    this.http = new WalletLinkHTTP(linkAPIUrl, sessionId, sessionKey);
+    this.http = new WalletLinkHTTP(linkAPIUrl, session.id, session.key);
   }
 
   /**
@@ -203,7 +211,7 @@ export class WalletLinkConnection {
       throw new Error('instance is destroyed');
     }
     this.diagnostic?.log(EVENTS.STARTED_CONNECTING, {
-      sessionIdHash: Session.hash(this.sessionId),
+      sessionIdHash: Session.hash(this.session.id),
     });
     this.ws.connect();
   }
@@ -217,7 +225,7 @@ export class WalletLinkConnection {
 
     this.ws.disconnect();
     this.diagnostic?.log(EVENTS.DISCONNECTED, {
-      sessionIdHash: Session.hash(this.sessionId),
+      sessionIdHash: Session.hash(this.session.id),
     });
 
     this.listener = undefined;
@@ -300,7 +308,7 @@ export class WalletLinkConnection {
 
       if (!isWeb3ResponseMessage(message)) return;
 
-      this.listener?.handleResponseMessage(message);
+      this.listener?.handleWeb3ResponseMessage(message);
     } catch {
       this.diagnostic?.log(EVENTS.GENERAL_ERROR, {
         message: 'Had error decrypting',
@@ -341,7 +349,7 @@ export class WalletLinkConnection {
   public async setSessionMetadata(key: string, value: string | null) {
     const message = ClientMessageSetSessionConfig({
       id: IntNumber(this.nextReqId++),
-      sessionId: this.sessionId,
+      sessionId: this.session.id,
       metadata: { [key]: value },
     });
 
@@ -371,7 +379,7 @@ export class WalletLinkConnection {
 
     const message = ClientMessagePublishEvent({
       id: IntNumber(this.nextReqId++),
-      sessionId: this.sessionId,
+      sessionId: this.session.id,
       event,
       data,
       callWebhook,
@@ -436,8 +444,8 @@ export class WalletLinkConnection {
   private async authenticate() {
     const msg = ClientMessageHostSession({
       id: IntNumber(this.nextReqId++),
-      sessionId: this.sessionId,
-      sessionKey: this.sessionKey,
+      sessionId: this.session.id,
+      sessionKey: this.session.key,
     });
     const res = await this.makeRequest<ServerMessageOK | ServerMessageFail>(msg);
     if (isServerMessageFail(res)) {
@@ -448,15 +456,15 @@ export class WalletLinkConnection {
   private sendIsLinked(): void {
     const msg = ClientMessageIsLinked({
       id: IntNumber(this.nextReqId++),
-      sessionId: this.sessionId,
+      sessionId: this.session.id,
     });
     this.sendData(msg);
   }
 
   private sendGetSessionConfig(): void {
     const msg = ClientMessageGetSessionConfig({
       id: IntNumber(this.nextReqId++),
-      sessionId: this.sessionId,
+      sessionId: this.session.id,
     });
     this.sendData(msg);
   }
@@ -490,7 +498,7 @@ export class WalletLinkConnection {
     this.listener?.resetAndReload();
     this.diagnostic?.log(EVENTS.METADATA_DESTROYED, {
       alreadyDestroyed: this.isDestroyed,
-      sessionIdHash: Session.hash(this.sessionId),
+      sessionIdHash: Session.hash(this.session.id),
     });
   };
 

diff --git a/packages/wallet-sdk/src/connection/WalletLinkConnectionCipher.test.ts b/packages/wallet-sdk/src/connection/WalletLinkConnectionCipher.test.ts
@@ -1,17 +1,19 @@
 import { randomBytesHex } from '../util';
-import { decrypt, encrypt } from './WalletLinkConnectionCipher';
+import { WalletLinkConnectionCipher } from './WalletLinkConnectionCipher';
 
 const secret = 'c356fe708ea7bbf7b1cc9ff9813c32772b6e0d16332da4c031ba9ea88be9b5ed';
 
 describe('aes256gcm', () => {
   test('encrypt/decrypt', async () => {
     const randSecret = randomBytesHex(32);
-    const encrypted = await encrypt('plain text string', randSecret);
+    const cipher = new WalletLinkConnectionCipher(randSecret);
+
+    const encrypted = await cipher.encrypt('plain text string');
 
     expect(encrypted.length).toEqual(90);
 
     // decrypted output matches original input
-    const decrypted = await decrypt(encrypted, randSecret);
+    const decrypted = await cipher.decrypt(encrypted);
 
     expect(decrypted).toBe('plain text string');
   });
@@ -28,18 +30,26 @@ describe('aes256gcm', () => {
       },
     };
 
-    const decrypted = await decrypt(cipherText, secret);
+    const cipher = new WalletLinkConnectionCipher(secret);
+
+    const decrypted = await cipher.decrypt(cipherText);
 
     expect(sampleDataResult).toEqual(JSON.parse(decrypted));
   });
 
   test('errors', async () => {
-    await expect(encrypt('plain text string', '123456')).rejects.toThrowError(
+    const cipher = new WalletLinkConnectionCipher('123456');
+
+    await expect(cipher.encrypt('plain text string')).rejects.toThrowError(
+      'secret must be 256 bits'
+    );
+
+    await expect(cipher.decrypt('plain stext string')).rejects.toThrowError(
       'secret must be 256 bits'
     );
 
-    expect(() => decrypt('plain stext string', '123456')).toThrowError('secret must be 256 bits');
+    const differentCipher = new WalletLinkConnectionCipher(secret);
 
-    await expect(decrypt('text', secret)).rejects.toThrow();
+    await expect(differentCipher.decrypt('text')).rejects.toThrow();
   });
 });