feat: auth aka sign 2.5 #4331

ganchoradkov · 2024-03-14T12:29:36Z

Description

Full Sign 2.5 implementation according to specs WalletConnect/walletconnect-specs#171

added new authenticate method where dapps can request One Client Authentication as per https://github.com/ChainAgnostic/CAIPs/blob/main/CAIPs/caip-222.md
added the corresponding wallet side approve/reject methods & handling
tons of utils for recap handling & validations

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

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.

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

… 15m & general cleanup

# Conflicts: # package-lock.json # packages/sign-client/src/constants/client.ts # packages/types/src/sign-client/client.ts

# Conflicts: # package-lock.json

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

# Conflicts: # package-lock.json

Cali93

Very impressive stuff going on here 💪
I'm almost half way through the PR, I'll try to come back at it later this week.

packages/sign-client/src/client.ts

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

…ential actions with parallel

bkrem

Posting initial batch of notes, will come back to this after interview calls/throughout the day

packages/web3wallet/src/controllers/engine.ts

bkrem · 2024-03-26T09:33:45Z

packages/web3wallet/src/client.ts

  };

  public removeListener: IWeb3Wallet["removeListener"] = (name, listener) => {
-    return this.events.removeListener(name, listener);
+    return this.engine.removeListener(name, listener);


Context? Event listeners are being merged into the core to avoid web3wallet client having its own set?

This is done in order to make sure we're handling session_authenticate requests only when wallet is subscribed for it, otherwise fallback to session_propose

packages/sign-client/src/client.ts

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

bkrem · 2024-03-26T09:48:31Z

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

+    );
+
+    // handle fallback session proposal response
+    const onSessionConnect = async ({ error, session }: any) => {


bkrem · 2024-03-26T09:48:35Z

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

+      }
+    };
+    // handle session authenticate response
+    const onAuthenticate = async (payload: any) => {


packages/sign-client/src/client.ts

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

devceline · 2024-03-27T10:09:05Z

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

+
+    const { chains, statement = "", uri, domain, nonce, type, exp, nbf, methods = [] } = params;
+    // reassign resources to remove reference as the array is modified and might cause side effects
+    const resources = [...(params.resources || [])];


Potentially Important: This does not create a deep copy and we might face unexepected side effects since the array is modified in place.

+1, spread operator works for primitive types like strings and numbers at the root level of the object-to-be-cloned, but any nested object types (array, object etc) are shallow reference copies.

Current implementation should be fine as resources by spec are an array of text values aka strings

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

bkrem

Still going 🏃

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

bkrem · 2024-03-27T11:15:51Z

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

+
+    const { chains, statement = "", uri, domain, nonce, type, exp, nbf, methods = [] } = params;
+    // reassign resources to remove reference as the array is modified and might cause side effects
+    const resources = [...(params.resources || [])];


+1, spread operator works for primitive types like strings and numbers at the root level of the object-to-be-cloned, but any nested object types (array, object etc) are shallow reference copies.

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

bkrem · 2024-03-27T11:33:08Z

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

+            [...new Set(approvedMethods)],
+            [...new Set(approvedAccounts)],


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

bkrem

💯 💯

Huge seeing this reach the finish line. Thank you for your continued efforts on this Gancho 🙏

packages/utils/src/signatures.ts

packages/web3wallet/src/controllers/engine.ts

devceline

LGTM

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

Gancho Radkov added 30 commits December 14, 2023 11:42

feat: WIP: authenticated sessions draft

Loading
Loading status checks…

bfbc9d5

feat: WIP: tests

Loading
Loading status checks…

66f832d

feat: wip: dynamic switcher

c1535f9

feat: wip: reject multi namespace requests

3c8398f

feat: adds signed cacaos to session struct

95bf4ce

Merge branch 'v2.0' into feat/auth

66f7bf3

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

feat: sorts recap methods alphabetically, increases default expiry to…

1461fea

… 15m & general cleanup

feat: adds expiryTimestamp & fixes types

992ef2d

refactor: sets URI method in pairing instead of sign client

716a8ad

chore: removes unused dev package

d6f0162

refactor: renames method to methdos

d3eddf5

Merge branch 'v2.0' into feat/auth

f9e5c6c

# Conflicts: # package-lock.json # packages/sign-client/src/constants/client.ts # packages/types/src/sign-client/client.ts

feat: updates wallet authentication to latest spec

09985df

refactor uses base64 encoding

1ef7c22

chore: always include resources in siwe message

9a28460

chore: tests

ade85c1

feat: adds siwe test

68977c0

refactor: updates auth type to cacao array

4650bb9

Merge branch 'v2.0' into feat/auth

800d04e

fix: adds resources to siwe message only if resources array exist

a56ad13

feat: adds caip1271 signature validation

1c0d750

fix: doesn't append new line after Resources: if array is empty

61a9117

Merge branch 'v2.0' into feat/auth

67ffc0d

# Conflicts: # package-lock.json

fix: adds params for eip1271 sig validation

b80e305

fix: removes extra empty line

a2d1519

feat: adds param to createRecap util

62d35f1

Merge branch 'v2.0' into feat/auth

5ff5cd3

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

fix: uses only required props by spec

ddea467

refactor: renames sessionAuthenticate to authenticate

d249858

feat: reject when auth signature validation fails

6a3d303

Merge branch 'v2.0' into feat/auth

edfc92e

# Conflicts: # package-lock.json

ganchoradkov requested a review from Cali93 March 21, 2024 09:11

Merge branch 'v2.0' into feat/auth

cddd246

Cali93 reviewed Mar 25, 2024

View reviewed changes

refactor: adds await, uses utils for namespace parsing, replaces sequ…

8a4f860

…ential actions with parallel

ganchoradkov requested a review from Cali93 March 26, 2024 08:41

bkrem changed the title ~~feat: auth~~ feat: auth aka sign 2.5 Mar 26, 2024

bkrem reviewed Mar 26, 2024

View reviewed changes

refactor: moves auth to own stores

34bba1a

ganchoradkov requested a review from bkrem March 27, 2024 08:53

devceline reviewed Mar 27, 2024

View reviewed changes

bkrem reviewed Mar 27, 2024

View reviewed changes

refactor: inits authStore and updates types

45c8c7a

ganchoradkov requested review from bkrem and devceline March 27, 2024 14:00

bkrem approved these changes Mar 27, 2024

View reviewed changes

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

packages/web3wallet/src/controllers/engine.ts Outdated Show resolved Hide resolved

packages/web3wallet/src/controllers/engine.ts Outdated Show resolved Hide resolved

Gancho Radkov and others added 2 commits March 27, 2024 16:58

chore: cleanup logs & removes export

0ccebbd

Merge branch 'v2.0' into feat/auth

906b71a

devceline approved these changes Apr 2, 2024

View reviewed changes

Gancho Radkov and others added 2 commits April 2, 2024 12:47

Merge branch 'v2.0' into feat/auth

eb7d144

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

Merge branch 'v2.0' into feat/auth

ed537a7

ganchoradkov merged commit 73f76ff into v2.0 Apr 2, 2024
9 checks passed

ganchoradkov deleted the feat/auth branch April 2, 2024 11:13

ganchoradkov mentioned this pull request Apr 3, 2024

chore: prep for 2.12.0 release #4392

Merged

20 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: auth aka sign 2.5 #4331

feat: auth aka sign 2.5 #4331

ganchoradkov commented Mar 14, 2024 •

edited

Loading

Cali93 left a comment

bkrem left a comment

bkrem Mar 26, 2024

ganchoradkov Mar 27, 2024

bkrem Mar 26, 2024

bkrem Mar 26, 2024

devceline Mar 27, 2024

bkrem Mar 27, 2024

ganchoradkov Mar 27, 2024

bkrem left a comment

bkrem Mar 27, 2024

bkrem Mar 27, 2024

bkrem left a comment

devceline left a comment

		[...new Set(approvedMethods)],
		[...new Set(approvedAccounts)],

feat: auth aka sign 2.5 #4331

feat: auth aka sign 2.5 #4331

Conversation

ganchoradkov commented Mar 14, 2024 • edited Loading

Description

Type of change

How has this been tested?

Checklist

Additional Information (Optional)

Cali93 left a 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

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

bkrem left a comment

Choose a reason for hiding this comment

devceline left a comment

Choose a reason for hiding this comment

ganchoradkov commented Mar 14, 2024 •

edited

Loading