Enhance abstraction: Reassign roles of Connection and WebSocket classes #1061

bangtoven · 2023-11-14T01:01:28Z

Summary

Apply the "listener" pattern introduced in Refactor session config handling and cryptography logic for improved abstraction #1059

How did you test your changes?

# Conflicts: # packages/wallet-sdk/src/connection/WalletLinkConnection.ts

oops

bangtoven · 2023-11-14T01:47:47Z

packages/wallet-sdk/src/connection/WalletLinkConnection.ts

-        case ConnectionState.CONNECTING:
-          break;


bangtoven · 2023-11-14T01:48:11Z

packages/wallet-sdk/src/connection/WalletLinkConnection.ts


-    this.http = new WalletLinkHTTP(linkAPIUrl, session.id, session.key);
-  }


bangtoven · 2023-11-14T01:58:16Z

packages/wallet-sdk/src/connection/DiagnosticLogger.ts

@@ -17,7 +16,7 @@ export type LogProperties = {
  onlineGuests?: number; // number of online guests (should be 0 or 1)
  sessionIdHash?: string; // anonymous session id for debugging specific sessions
  sessionMetadataChange?: string; // change in session metadata
-  state?: ConnectionState;
+  state?: boolean;


btw, in case you're wondering,
the change maintains functional integrity for logging purposes, because
ConnectionState was an enum with DISCONNECTED as its first value => having a raw value of 0

bangtoven · 2023-11-14T02:17:20Z

packages/wallet-sdk/src/connection/DiagnosticLogger.ts

@@ -17,7 +16,7 @@ export type LogProperties = {
  onlineGuests?: number; // number of online guests (should be 0 or 1)
  sessionIdHash?: string; // anonymous session id for debugging specific sessions
  sessionMetadataChange?: string; // change in session metadata
-  state?: ConnectionState;


Logger should not depend on ConnectionState

fix tests fix tests

bangtoven · 2023-11-14T07:35:53Z

packages/wallet-sdk/src/connection/WalletLinkWebSocket.ts

-export enum ConnectionState {
-  DISCONNECTED,
-  CONNECTING,
-  CONNECTED,


it was arbitrary client-side-only concept
we can manage connection state without this enum

bangtoven · 2023-11-14T18:53:13Z

packages/wallet-sdk/src/connection/WalletLinkConnection.ts

+  /**
+   * This section of code implements a reconnect behavior that was ported from a legacy system.
+   * Preserving original comments to maintain the rationale and context provided by the original author.
+   * https://github.com/coinbase/coinbase-wallet-sdk/commit/2087ee4a7d40936cd965011bfacdb76ce3462894#diff-dd71e86752e2c20c0620eb0ba4c4b21674e55ae8afeb005b82906a3821e5023cR84
+   * TOOD: revisit this logic to assess its validity in the current system context.
+   */
+


it's ~4 year old logic. will revisit later to see if we can improve this

2087ee4#diff-dd71e86752e2c20c0620eb0ba4c4b21674e55ae8afeb005b82906a3821e5023cR84

bangtoven · 2023-11-14T19:00:25Z

packages/wallet-sdk/src/connection/WalletLinkWebSocket.ts

-  setIncomingDataListener(listener: (_: ServerMessage) => void): void {
-    this.incomingDataListener = listener;
-  }
+  private pendingData: ClientMessage[] = [];


pendingData now stores ClientMessages instead of raw request value

…Socket

bangtoven · 2023-11-14T20:15:04Z

packages/wallet-sdk/src/connection/WalletLinkConnection.ts

-          // perform authentication upon connection
-          try {
-            // if CONNECTED, authenticate, and then check link status
-            await this.authenticate();
-            this.sendIsLinked();
-            this.sendGetSessionConfig();
-            connected = true;
-          } catch {
-            /* empty */
-          }
-
-          // send heartbeat every n seconds while connected
-          // if CONNECTED, start the heartbeat timer
-          // first timer event updates lastHeartbeat timestamp
-          // subsequent calls send heartbeat message
-          this.updateLastHeartbeat();
-          setInterval(() => {
-            this.heartbeat();
-          }, HEARTBEAT_INTERVAL);


auth, heartbeat management stuff has been migrated to WalletLinkWebSocket class

bangtoven · 2023-11-14T22:33:46Z

packages/wallet-sdk/src/connection/WalletLinkConnection.ts

+      session,
+      WebSocketClass,
+      listener: this,
+    });


linkAPIUrl => ws url conversion has been migrated to WalletLinkWebSocket class

commit 467fafe Author: Jungho Bang <[email protected]> Date: Wed Nov 15 12:40:57 2023 -0800 apply to HTTP API commit 883715a Author: Jungho Bang <[email protected]> Date: Wed Nov 15 12:40:47 2023 -0800 cleanup commit d01912a Author: Jungho Bang <[email protected]> Date: Wed Nov 15 12:25:30 2023 -0800 fi xWalletUI commit 0228706 Author: Jungho Bang <[email protected]> Date: Wed Nov 15 12:22:27 2023 -0800 cleanup commit 620216f Author: Jungho Bang <[email protected]> Date: Wed Nov 15 12:16:15 2023 -0800 WalletLinkEventType commit ea54a0c Author: Jungho Bang <[email protected]> Date: Wed Nov 15 12:07:40 2023 -0800 apply change to selectProvider commit 577e596 Author: Jungho Bang <[email protected]> Date: Wed Nov 15 12:00:47 2023 -0800 clarify param commit 66cbeb8 Author: Jungho Bang <[email protected]> Date: Wed Nov 15 11:45:37 2023 -0800 revert rename commit 6f3b4bd Author: Jungho Bang <[email protected]> Date: Wed Nov 15 11:36:45 2023 -0800 fix tests commit b709f69 Author: Jungho Bang <[email protected]> Date: Wed Nov 15 10:26:08 2023 -0800 rename relay message to be `WalletLinkEventData` commit cf0ad57 Author: Jungho Bang <[email protected]> Date: Wed Nov 15 00:25:21 2023 -0800 cleaner commit bfafff9 Author: Jungho Bang <[email protected]> Date: Wed Nov 15 00:14:57 2023 -0800 fix provider commit 90feb0e Author: Jungho Bang <[email protected]> Date: Tue Nov 14 23:48:07 2023 -0800 no error on relay commit cb90fd1 Author: Jungho Bang <[email protected]> Date: Tue Nov 14 23:17:51 2023 -0800 wtf commit 17c67b5 Author: Jungho Bang <[email protected]> Date: Tue Nov 14 23:06:32 2023 -0800 consolidate RelayMessage commit f636d5f Author: Jungho Bang <[email protected]> Date: Tue Nov 14 22:12:25 2023 -0800 support case of request and response having different method commit a95e96e Author: Jungho Bang <[email protected]> Date: Tue Nov 14 22:04:19 2023 -0800 comment commit 8f07056 Author: Jungho Bang <[email protected]> Date: Tue Nov 14 22:00:18 2023 -0800 sync commit c478f1e Author: Jungho Bang <[email protected]> Date: Tue Nov 14 21:57:26 2023 -0800 no need to be exported RelayMessageType commit 7c9cb7b Author: Jungho Bang <[email protected]> Date: Tue Nov 14 21:57:08 2023 -0800 apply Web3Response changes commit e2232a8 Author: Jungho Bang <[email protected]> Date: Tue Nov 14 21:49:44 2023 -0800 cleanup RelayMessage commit c5a93d6 Author: Jungho Bang <[email protected]> Date: Tue Nov 14 19:54:54 2023 -0800 introducing SupportedWeb3Methods commit 99176c8 Author: Jungho Bang <[email protected]> Date: Tue Nov 14 19:26:24 2023 -0800 improvements commit 9ceab2a Author: Jungho Bang <[email protected]> Date: Tue Nov 14 19:10:11 2023 -0800 help git commit 6c86f5a Author: Jungho Bang <[email protected]> Date: Tue Nov 14 19:04:53 2023 -0800 Web3Response commit 93edb63 Author: Jungho Bang <[email protected]> Date: Tue Nov 14 18:16:33 2023 -0800 refactoring Web3Request commit 154ba57 Author: Jungho Bang <[email protected]> Date: Tue Nov 14 17:51:49 2023 -0800 lfg commit 2bcc7a7 Author: Jungho Bang <[email protected]> Date: Tue Nov 14 17:12:40 2023 -0800 apply changes commit 911c669 Author: Jungho Bang <[email protected]> Date: Tue Nov 14 16:51:58 2023 -0800 apply it to ClientMessage commit 20f7612 Author: Jungho Bang <[email protected]> Date: Tue Nov 14 15:48:09 2023 -0800 refactor ServerMessage type

# Conflicts: # packages/wallet-sdk/src/connection/DiagnosticLogger.ts # packages/wallet-sdk/src/connection/WalletLinkConnection.ts

bangtoven added 4 commits November 13, 2023 14:53

remove no-op

d85809a

WalletLinkWebSocketUpdateListener

494c417

Merge branch 'master' into jungho/connection-state

da6e711

# Conflicts: # packages/wallet-sdk/src/connection/WalletLinkConnection.ts

fix tests

7f3aa02

bangtoven changed the title ~~connection state~~ WalletLinkWebSocketUpdateListener Nov 14, 2023

replace switch with if

fda6484

oops

bangtoven force-pushed the jungho/connection-state branch from 45c9214 to fda6484 Compare November 14, 2023 01:40

bangtoven commented Nov 14, 2023

View reviewed changes

ConnectionState

523472e

bangtoven commented Nov 14, 2023

View reviewed changes

fix test

ad5d71c

fix tests fix tests

bangtoven force-pushed the jungho/connection-state branch from 2dd3ebc to ad5d71c Compare November 14, 2023 02:42

bangtoven added 14 commits November 13, 2023 19:25

WebSocket construction

97abf8d

migrate websocket connection state update

44462fc

cleanup

34a2218

revert walletlinkrelay changes

bca565a

bring back handleConnectionStateChange

7af21b0

cleanup

093283b

deprecate ConnectionState

101a0ce

minimize diff

cacdd91

min diff

9f1af4f

this.isDestroyed

a7e7951

wip

9a43dc7

websocketConnected, websocketMessageReceived

40b635e

remove diagnostic from websocket

d1458c3

dm

6b83cac

bangtoven closed this Nov 14, 2023

bangtoven reopened this Nov 14, 2023

bangtoven commented Nov 14, 2023

View reviewed changes

sendPendingMessages

56180be

cleanup

b397ced

bangtoven force-pushed the jungho/connection-state branch from 582c917 to b5987fd Compare November 14, 2023 18:36

pull out reconnect logic

a6bd0a3

bangtoven force-pushed the jungho/connection-state branch from b5987fd to a6bd0a3 Compare November 14, 2023 18:37

reconnect disclaimer

484a3c5

bangtoven commented Nov 14, 2023

View reviewed changes

cleanup

74f368f

bangtoven force-pushed the jungho/connection-state branch from 0eafafd to 74f368f Compare November 14, 2023 19:05

bangtoven added 2 commits November 14, 2023 12:08

linked, revert CONNECTED_STATE_CHANGE change, DiagnosticLogger to Web…

dca9b05

…Socket

fix tests

1033e78

bangtoven commented Nov 14, 2023

View reviewed changes

bangtoven added 8 commits November 14, 2023 13:50

remove diagnostic from WebSocket, preserve ConnectionState enum

c0c3a93

remove eslint override

721129d

keep SessionConfig

8d05986

keep logging

5774eaf

cleanup

b4292bc

comment

264a662

comment

d92fe7f

comments

4e01333

bangtoven commented Nov 14, 2023

View reviewed changes

bangtoven force-pushed the jungho/connection-state branch from b037004 to 4c02a75 Compare November 14, 2023 22:39

help github

ed795fe

bangtoven force-pushed the jungho/connection-state branch from 4c02a75 to ed795fe Compare November 14, 2023 22:41

bangtoven added 3 commits November 14, 2023 15:06

ugh

4abaf7d

Merge branch 'master' into jungho/connection-state

b04eac6

# Conflicts: # packages/wallet-sdk/src/connection/DiagnosticLogger.ts # packages/wallet-sdk/src/connection/WalletLinkConnection.ts

bangtoven changed the title ~~WalletLinkWebSocketUpdateListener~~ Enhance abstraction: Reassign roles of Connection and WebSocket classes Nov 16, 2023

bangtoven closed this Feb 29, 2024

bangtoven deleted the jungho/connection-state branch February 29, 2024 05:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance abstraction: Reassign roles of Connection and WebSocket classes #1061

Enhance abstraction: Reassign roles of Connection and WebSocket classes #1061

bangtoven commented Nov 14, 2023 •

edited

Loading

bangtoven Nov 14, 2023

bangtoven Nov 14, 2023

bangtoven Nov 14, 2023

bangtoven Nov 14, 2023

bangtoven Nov 14, 2023 •

edited

Loading

bangtoven Nov 14, 2023 •

edited

Loading

bangtoven Nov 14, 2023

bangtoven Nov 14, 2023

bangtoven Nov 14, 2023


		this.http = new WalletLinkHTTP(linkAPIUrl, session.id, session.key);
		}

Enhance abstraction: Reassign roles of Connection and WebSocket classes #1061

Enhance abstraction: Reassign roles of Connection and WebSocket classes #1061

Conversation

bangtoven commented Nov 14, 2023 • edited Loading

Summary

How did you test your changes?

bangtoven Nov 14, 2023

Choose a reason for hiding this comment

bangtoven Nov 14, 2023

Choose a reason for hiding this comment

bangtoven Nov 14, 2023

Choose a reason for hiding this comment

bangtoven Nov 14, 2023

Choose a reason for hiding this comment

bangtoven Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

bangtoven Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

bangtoven Nov 14, 2023

Choose a reason for hiding this comment

bangtoven Nov 14, 2023

Choose a reason for hiding this comment

bangtoven Nov 14, 2023

Choose a reason for hiding this comment

bangtoven commented Nov 14, 2023 •

edited

Loading

bangtoven Nov 14, 2023 •

edited

Loading

bangtoven Nov 14, 2023 •

edited

Loading