feat: link mode #5141

ignaciosantise · 2024-07-02T13:08:08Z

Description

Enable link-mode for mobile dapps/wallets
Reworked relayer to start wss connection only when required i.e. topics present in storage

~~TODO: cleanup logs before merging~~

Type of change

Chore (non-breaking change that addresses non-functional tasks, maintenance, or code quality improvements)
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Draft PR (breaking/non-breaking change which needs more work for having a proper functionality [Mark this PR as ready to review only when completely ready])
Breaking change (fix or feature that would cause existing functionality to not work as expected)

How has this been tested?

Tested in RN sample dapp and wallet

Checklist

I have performed a self-review of my own code
My changes generate no new warnings
Any dependent changes have been merged and published in downstream modules

Additional Information (Optional)

Please include any additional information that may be useful for the reviewer.

…ink if wallet supports linkmode

packages/core/src/core.ts

packages/sign-client/src/controllers/engine.ts

packages/utils/src/crypto.ts

ganchoradkov

Looking lit! writing down my initial thoughts, will dig more 💯

packages/utils/src/crypto.ts

packages/types/src/sign-client/engine.ts

packages/sign-client/src/controllers/engine.ts

…tion

…de apps

… param to getPayloadType

…it for some relayer subscriptions

packages/core/src/controllers/subscriber.ts

packages/core/src/controllers/relayer.ts

packages/sign-client/src/controllers/engine.ts

packages/types/src/sign-client/engine.ts

…st fails

# Conflicts: # packages/sign-client/src/controllers/engine.ts

Cali93

Left some minor comments but very good job 💪

packages/core/src/controllers/crypto.ts

packages/core/src/controllers/relayer.ts

Cali93 · 2024-09-05T12:12:33Z

packages/core/src/controllers/subscriber.ts

+      if (transportType === TRANSPORT_TYPES.link_mode) {
+        setTimeout(() => {
+          if (this.relayer.connected || this.relayer.connecting) {
+            this.relayer.request(request).catch((e) => this.logger.warn(e));
+          }
+        }, toMiliseconds(ONE_SECOND));
+        return subId;
+      }


Is this kind of delay really needed ? Wouldn't this add a 1 second delay in every case (even if airplane mode is off).

On another note we should clear the timeout.

Wouldn't this add a 1 second delay in every case (even if airplane mode is off).

with link mode, yes but it doesn't cause any issues as we need sometime for the react-native netinfo to register network changes when the app is back in foreground

On another note we should clear the timeout.

why? there isn't a case where the timeout has to be stopped mid execution

with link mode, yes but it doesn't cause any issues as we need sometime for the react-native netinfo to register network changes when the app is back in foreground

Ideally we should detect when this operation is needed instead of having the delay in every situation but I understand it's probably not trivial.

why? there isn't a case where the timeout has to be stopped mid execution

It's just a good practice I've been used to because of React development.

From Chat GPT

Using clearTimeout is not strictly necessary every time you use setTimeout, but there are specific situations where it is beneficial or required:

Preventing Unwanted Execution: If there's a possibility that the action scheduled with setTimeout should not occur in certain conditions (for example, if the user navigates away from a page or a component is unmounted), then you should clear the timeout to prevent the action from executing when it's no longer needed or relevant.

Memory Leaks: In React components or other dynamic UI environments, not clearing timeouts can lead to memory leaks. If a timeout is set on a component and the component is unmounted before the timeout fires, the callback function may still be executed, which can lead to errors or unintended behavior, especially if the callback modifies component state.

Resource Management: Timers hold onto resources until they execute. Clearing them when no longer needed frees up these resources, which is beneficial for performance, especially in environments with limited resources like mobile devices.

Control Over Asynchronous Actions: In complex applications where multiple timeouts might be set and their behavior could overlap or interfere with each other, managing timeouts with clearTimeout ensures that only the desired timers are running at any given time.

packages/utils/src/crypto.ts

Cali93

Nice one Nacho 💪

ignaciosantise added 4 commits June 24, 2024 15:01

chore: added new url param in authenticate functions, save universalL…

7077dcf

…ink if wallet supports linkmode

Merge branch 'v2.0' into feat/link-mode

f55eb51

chore: link_mode in auth request

7d846e6

chore: added transport type to sendRequest, sendResult, sendError

b3b4372

ignaciosantise requested a review from ganchoradkov July 2, 2024 13:08

ignaciosantise commented Jul 2, 2024

View reviewed changes

packages/core/src/core.ts Outdated Show resolved Hide resolved

ignaciosantise commented Jul 2, 2024

View reviewed changes

packages/sign-client/src/controllers/engine.ts Outdated Show resolved Hide resolved

ignaciosantise commented Jul 2, 2024

View reviewed changes

packages/sign-client/src/controllers/engine.ts Outdated Show resolved Hide resolved

ignaciosantise commented Jul 2, 2024

View reviewed changes

packages/utils/src/crypto.ts Show resolved Hide resolved

ganchoradkov reviewed Jul 2, 2024

View reviewed changes

packages/utils/src/crypto.ts Outdated Show resolved Hide resolved

packages/utils/src/crypto.ts Outdated Show resolved Hide resolved

packages/types/src/sign-client/engine.ts Outdated Show resolved Hide resolved

packages/sign-client/src/controllers/engine.ts Outdated Show resolved Hide resolved

ignaciosantise and others added 17 commits July 2, 2024 11:06

chore: renamed encode/decode type 2 functions

401dfd9

chore: moved functions to utils

9e621dc

chore: removed authenticateLinkMode fx, added misc test

2b71994

chore: code improvements

709f797

chore: added valid expiry

13f3e3c

Merge branch 'v2.0' into feat/link-mode

35801b8

chore: add url listeners after sign is initialized

4f68c1f

fix: prettier

d3257f4

chore: checking for linkmode supported list in isLinkModeEnabled func…

1764aaa

…tion

chore: added transportType to message events to save supported linkmo…

0f577c4

…de apps

chore: encode to base64url all link-mode requests/responses

ea3e33f

chore: added transportType to relayer test

ca8a734

chore: discard link-mode messages in relayer callback, added encoding…

7fb509d

… param to getPayloadType

chore: added transportType to session object

61ffd79

chore: check internet connection only for relay sessions, removed awa…

90a7013

…it for some relayer subscriptions

Merge branch 'v2.0' into feat/link-mode

2092441

chore: revert await change in relayer subscriptions

5383a2c

ignaciosantise marked this pull request as ready for review August 2, 2024 20:18

ignaciosantise marked this pull request as draft August 2, 2024 20:18

Merge branch 'v2.0' into feat/link-mode

7473901