feat: decrypted notifications #3997

ganchoradkov · 2023-11-29T14:17:45Z

Description

This includes

new Echo controller in Core that allows devices to be registered with Echo server
new static property in Web3Wallet called notifications that allows for decrypting push payloads & getting the peer's metadata without initializing the whole client

Usage

Registering device
core.echo.registerDeviceToken(params) or shorthand web3wallet.registerDeviceToken(params)
Decrypting notification payload

 // handling firebase remote notificaiton
  messaging().setBackgroundMessageHandler(async notification => {
    const { topic, message } = notification.data;
    // decrypt the payload
    const decrypted = await Web3Wallet.notifications.decryptMessage({ topic, encryptedMessage: message })
    // get metadata (name, description, icons, etc) of the sender
    const metadata = await Web3Wallet.notifications.getMetadata({ topic })
    
    // emit local notification
    ...
  });

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?

tests
dogfooding

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)

specs WalletConnect/walletconnect-specs#147

devceline

LGTM - left questions and nits. All non blocking

packages/core/src/controllers/echo.ts

devceline · 2023-12-04T09:34:45Z

packages/core/src/controllers/echo.ts

+import { IEcho } from "@walletconnect/types";
+import { ECHO_CONTEXT, ECHO_URL } from "../constants";
+
+export class Echo extends IEcho {


nit: Why did we make an entire class for this? Couldn't we have just had a function defined somewhere to register the token? I feel like it's just extra complexity and more state to store needlessly

the idea is to have dedicated handling allowing functionality to be reused and expanded as needed in the future

devceline · 2023-12-04T09:47:59Z

packages/web3wallet/src/controllers/engine.ts

@@ -98,6 +98,11 @@ export class Engine extends IWeb3WalletEngine {
    return this.authClient.formatMessage(params, iss);
  };

+  // Push //


Suggested change

// Push //

// ---------- Push -------------------------------------------------- //

packages/web3wallet/src/utils/notifications.ts

devceline · 2023-12-04T09:50:44Z

packages/core/src/core.ts

@@ -77,6 +78,7 @@ export class Core extends ICore {
    });
    this.pairing = new Pairing(this, this.logger);
    this.verify = new Verify(this.projectId || "", this.logger);
+    this.echo = new Echo(this.projectId || "", this.logger);


Why are we putting this in core? Sign/Auth/Chat won't make a use of this, no?

devceline · 2023-12-04T09:53:09Z

packages/core/src/controllers/echo.ts

+
+    const echoUrl = `${ECHO_URL}/${this.projectId}/clients`;
+
+    await fetch(echoUrl, {


Don't we need to use isomorphic-unfetch or something here? Or is it already polyfilled

I'm actually kind of surprised this hasn't come up before here where there's already a fetch call: https://github.com/WalletConnect/walletconnect-monorepo/blob/v2.0/packages/core/src/controllers/verify.ts#L103

Is the Verify controller called only in browsers?

This reverts commit 7391142.

bkrem

🔥 🔥

bkrem · 2023-12-04T16:47:02Z

packages/core/src/controllers/echo.ts

+
+    const echoUrl = `${ECHO_URL}/${this.projectId}/clients`;
+
+    await fetch(echoUrl, {


I'm actually kind of surprised this hasn't come up before here where there's already a fetch call: https://github.com/WalletConnect/walletconnect-monorepo/blob/v2.0/packages/core/src/controllers/verify.ts#L103

Is the Verify controller called only in browsers?

packages/sign-client/src/index.ts

packages/web3wallet/src/types/client.ts

bkrem · 2023-12-04T17:01:39Z

packages/web3wallet/src/utils/notifications.ts

+
+export const Notifications: Web3WalletTypes.INotifications = {
+  decryptMessage: async (params) => {
+    const core = new Core({


Bit concerned regarding memory and creating a new Core reference on every decryptMessage call. Is there any way to avoid this and init the Core once? Guessing this may have to be stateless right.

I don't think we have a way around this because our modules are not stateless/pure - crypto depends on core and having a store AFAIK

Good point 💯 I will thinker a solution as we do need to reinitialize to update the data

Gancho Radkov added 4 commits November 28, 2023 11:20

feat: implements registerDeviceToken in Echo

19598a8

feat: export session store from sign

5b78ef9

feat: implements static decrypt notifications & getMetadata utils

2e6a0a2

feat: tests

3f9b900

arein added the accepted label Nov 29, 2023

Gancho Radkov added 4 commits November 29, 2023 16:26

chore: rm unused import

cab479a

chore: merge v2.0

b230e71

chore: remove logs

269c09c

refactor: types

9e086ab

ganchoradkov requested review from bkrem and devceline December 1, 2023 14:05

ganchoradkov marked this pull request as ready for review December 1, 2023 14:06

devceline approved these changes Dec 4, 2023

View reviewed changes

Gancho Radkov added 4 commits December 4, 2023 15:03

refactor: renames echo to echoClient

2b9483a

fix: polyfill fetch

7391142

Revert "fix: polyfill fetch"

1dcfece

This reverts commit 7391142.

fix: uses [email protected]

195a003

bkrem approved these changes Dec 4, 2023

View reviewed changes

Gancho Radkov added 2 commits December 5, 2023 09:58

refactor: restructure exports

74e8658

feat: clear class instances after work done

1f03e09

bkrem approved these changes Dec 13, 2023

View reviewed changes

ganchoradkov merged commit 51c3a97 into v2.0 Dec 19, 2023
9 checks passed

ganchoradkov deleted the feat/echo branch December 19, 2023 09:49

ganchoradkov mentioned this pull request Dec 21, 2023

chore: prep for 2.11.0 #4073

Merged

20 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: decrypted notifications #3997

feat: decrypted notifications #3997

ganchoradkov commented Nov 29, 2023 •

edited

Loading

devceline left a comment

devceline Dec 4, 2023

ganchoradkov Dec 4, 2023

devceline Dec 4, 2023

devceline Dec 4, 2023

devceline Dec 4, 2023

bkrem Dec 4, 2023

bkrem left a comment

bkrem Dec 4, 2023

bkrem Dec 4, 2023

devceline Dec 5, 2023

ganchoradkov Dec 5, 2023

	// Push //
	// ---------- Push -------------------------------------------------- //


		const echoUrl = `${ECHO_URL}/${this.projectId}/clients`;

		await fetch(echoUrl, {

feat: decrypted notifications #3997

feat: decrypted notifications #3997

Conversation

ganchoradkov commented Nov 29, 2023 • edited Loading

Description

Usage

Type of change

How has this been tested?

Checklist

Additional Information (Optional)

devceline left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkrem left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ganchoradkov commented Nov 29, 2023 •

edited

Loading