feat: verify v2 #5236

ganchoradkov · 2024-08-01T13:30:01Z

Description

Implemented verify v2 according to spec https://www.notion.so/walletconnect/Verify-Server-rewrite-tech-doc-Verify-API-V2-30e207c77f8447d8af37a186799b7c9e?pvs=4#fbc0ca6d67c444118599ba49d8b358f9

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?

reown-com/web-examples#662

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.

packages/core/src/constants/verify.ts

packages/core/src/controllers/verify.ts

…ct-monorepo into feat/verify-v2 # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.

…d to be used and to fetch new public key if validation fails with the current stored key

…ct-monorepo into feat/verify-v2

packages/core/src/controllers/verify.ts

…ct-monorepo into feat/verify-v2

packages/core/src/controllers/verify.ts

chris13524 · 2024-08-12T15:39:43Z

There is a race condition that I'd like to see resolved, if not before merging then let's create an issue for it.

The logic is if there isn't a saved public key in the verification step then it will request the public key in a blocking manner. In the happy case the public key will already be requested and stored during SDK initialization, but if the network request isn't complete when the verification step comes, then it will result in a second request and additional latency.

The race condition can be avoided by awaiting the original request in the verification step.

[ key fetch in SDK init      ]
  [receive paring req ][ key fetch again ]

Cali93

Looks solid 💪
A few more tests and it should be good to go

packages/core/src/constants/verify.ts

packages/core/src/controllers/verify.ts

Cali93 · 2024-08-12T15:48:37Z

packages/core/src/controllers/verify.ts

+    } catch (e) {
+      this.logger.warn(e);
+    }
+    return "";


Why are we retruning an empty string ?

packages/core/src/controllers/verify.ts

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

packages/utils/test/crypto.spec.ts

packages/core/src/controllers/verify.ts

…e:crypto` or `crypto.subtle`

packages/core/src/controllers/verify.ts

Cali93

LFG 🚀

…^2.15.0 (#5795) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@walletconnect/react-native-compat](https://togithub.com/walletconnect/walletconnect-monorepo) | [`^2.14.0` -> `^2.15.0`](https://renovatebot.com/diffs/npm/@walletconnect%2freact-native-compat/2.14.0/2.15.0) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@walletconnect%2freact-native-compat/2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@walletconnect%2freact-native-compat/2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@walletconnect%2freact-native-compat/2.14.0/2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@walletconnect%2freact-native-compat/2.14.0/2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>walletconnect/walletconnect-monorepo (@walletconnect/react-native-compat)</summary> ### [`v2.15.0`](https://togithub.com/WalletConnect/walletconnect-monorepo/releases/tag/2.15.0) [Compare Source](https://togithub.com/walletconnect/walletconnect-monorepo/compare/2.14.0...2.15.0) #### What's Changed - chore: prep for `2.14.0` release by [@ganchoradkov](https://togithub.com/ganchoradkov) in [WalletConnect/walletconnect-monorepo#5195 - refactor: removes session approval peer acknowledgment delay by [@ganchoradkov](https://togithub.com/ganchoradkov) in [WalletConnect/walletconnect-monorepo#5197 - chore(deps): update docker/build-push-action action to v6 by [@renovate](https://togithub.com/renovate) in [WalletConnect/walletconnect-monorepo#4641 - chore(deps): update actions/add-to-project action to v1.0.2 by [@renovate](https://togithub.com/renovate) in [WalletConnect/walletconnect-monorepo#5101 - chore: fix some comments by [@renshuncui](https://togithub.com/renshuncui) in [WalletConnect/walletconnect-monorepo#4640 - AlgorandProvider [#877](https://togithub.com/walletconnect/walletconnect-monorepo/issues/877) [#2760](https://togithub.com/walletconnect/walletconnect-monorepo/issues/2760) by [@scholtz](https://togithub.com/scholtz) in [WalletConnect/walletconnect-monorepo#5201 - chore: fix typo by [@riyueguang](https://togithub.com/riyueguang) in [WalletConnect/walletconnect-monorepo#5216 - chore: change handshake canary to only measure 1 client initialization by [@chris13524](https://togithub.com/chris13524) in [WalletConnect/walletconnect-monorepo#5250 - refactor: session settle response by [@ganchoradkov](https://togithub.com/ganchoradkov) in [WalletConnect/walletconnect-monorepo#5259 - refactor: publishes session settle request before the session propose… by [@ganchoradkov](https://togithub.com/ganchoradkov) in [WalletConnect/walletconnect-monorepo#5246 - feat: verify v2 by [@ganchoradkov](https://togithub.com/ganchoradkov) in [WalletConnect/walletconnect-monorepo#5236 - chore: partially dedupe fetch implementations and use native fetch instead by [@talentlessguy](https://togithub.com/talentlessguy) in [WalletConnect/walletconnect-monorepo#5112 #### New Contributors - [@renshuncui](https://togithub.com/renshuncui) made their first contribution in [WalletConnect/walletconnect-monorepo#4640 - [@scholtz](https://togithub.com/scholtz) made their first contribution in [WalletConnect/walletconnect-monorepo#5201 - [@riyueguang](https://togithub.com/riyueguang) made their first contribution in [WalletConnect/walletconnect-monorepo#5216 - [@talentlessguy](https://togithub.com/talentlessguy) made their first contribution in [WalletConnect/walletconnect-monorepo#5112 **Full Changelog**: WalletConnect/walletconnect-monorepo@2.14.0...2.15.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 5pm,every weekend" in timezone America/Los_Angeles, Automerge - "after 5pm,every weekend" in timezone America/Los_Angeles. 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View the [repository job log](https://developer.mend.io/github/valora-inc/wallet).  --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: valora-bot <[email protected]>

ganchoradkov added 3 commits July 30, 2024 14:57

wip: verify v2 - submit attestation

951f993

chore: sents attestation with publish payload

0596131

feat: verify v2

d1966d4

arein added the accepted label Aug 1, 2024

chris13524 reviewed Aug 1, 2024

View reviewed changes

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

chris13524 reviewed Aug 1, 2024

View reviewed changes

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

chris13524 reviewed Aug 1, 2024

View reviewed changes

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

chris13524 reviewed Aug 1, 2024

View reviewed changes

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

chris13524 reviewed Aug 1, 2024

View reviewed changes

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

chris13524 reviewed Aug 5, 2024

View reviewed changes

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

chris13524 reviewed Aug 5, 2024

View reviewed changes

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

ganchoradkov and others added 11 commits August 6, 2024 10:31

refactor: uses getDocument()

f755ef6

Merge branch 'v2.0' into feat/verify-v2

07823a5

refactor: makes verify v2 public key being fetched only when attempte…

318a0e2

…d to be used and to fetch new public key if validation fails with the current stored key

chore: canary relay api

d11901e

chore: prettier

18d5f6b

fix: adds new attestation field to publisher tests

6b5a70a

fix: relayer test with new attestation param

e0d979e

chore: updates relay-api to stable

730bca3

feat: adds test

5210e5a

chore: clean up logs

3ddbfc7

ganchoradkov marked this pull request as ready for review August 9, 2024 10:39

ganchoradkov requested review from chris13524 and Cali93 August 9, 2024 10:39

ganchoradkov mentioned this pull request Aug 9, 2024

fix(deps): update dependency @walletconnect/relay-api to v1.0.11 #5260

Closed

1 task

ganchoradkov changed the title ~~wip: feat: verify v2~~ feat: verify v2 Aug 9, 2024

ganchoradkov added 3 commits August 9, 2024 13:44

chore: bump node to 20

372a42e

fix: logging & rm event listener

9659018

fix: subtle crypto in node

faeed6d

Merge branch 'feat/verify-v2' of github.com:WalletConnect/walletconne…

9535ca9

…ct-monorepo into feat/verify-v2

ganchoradkov requested a review from chris13524 August 12, 2024 12:22

chore: handle invalid keys

0ef5ee2

ganchoradkov requested review from glitch-txs, jakubuid, bkrem and ignaciosantise August 12, 2024 12:48

Merge branch 'v2.0' into feat/verify-v2

92d77a9

ignaciosantise reviewed Aug 12, 2024

View reviewed changes

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

ganchoradkov added 2 commits August 12, 2024 16:38

chore: fixes typo

24614bd

Merge branch 'feat/verify-v2' of github.com:WalletConnect/walletconne…

9e9bf2e

…ct-monorepo into feat/verify-v2

ganchoradkov requested a review from ignaciosantise August 12, 2024 13:39

ignaciosantise reviewed Aug 12, 2024

View reviewed changes

packages/core/src/controllers/verify.ts Show resolved Hide resolved

ganchoradkov requested a review from ignaciosantise August 12, 2024 14:03

chris13524 approved these changes Aug 12, 2024

View reviewed changes

Cali93 reviewed Aug 12, 2024

View reviewed changes

chris13524 reviewed Aug 12, 2024

View reviewed changes

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

feat: implements ecdsa signature validation without relying on `nod…

fb3dceb

…e:crypto` or `crypto.subtle`

chris13524 reviewed Aug 14, 2024

View reviewed changes

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

ganchoradkov added 3 commits August 15, 2024 11:46

chore: cleanup

0c4df5c

feat: adds unhappy path tests

29f8df8

chore: uses const

1c229bd

ganchoradkov requested a review from Cali93 August 15, 2024 08:57

Cali93 approved these changes Aug 15, 2024

View reviewed changes

ganchoradkov merged commit ab408aa into v2.0 Aug 15, 2024
9 checks passed

ganchoradkov deleted the feat/verify-v2 branch August 15, 2024 10:18

ganchoradkov mentioned this pull request Aug 15, 2024

chore: prep for 2.15.0 release #5273

Merged

20 tasks

ganchoradkov restored the feat/verify-v2 branch August 20, 2024 07:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: verify v2 #5236

feat: verify v2 #5236

ganchoradkov commented Aug 1, 2024 •

edited

Loading

chris13524 commented Aug 12, 2024 •

edited

Loading

Cali93 left a comment

Cali93 Aug 12, 2024

Cali93 left a comment

feat: verify v2 #5236

feat: verify v2 #5236

Conversation

ganchoradkov commented Aug 1, 2024 • edited Loading

Description

Type of change

How has this been tested?

Checklist

Additional Information (Optional)

chris13524 commented Aug 12, 2024 • edited Loading

Cali93 left a comment

Choose a reason for hiding this comment

Cali93 Aug 12, 2024

Choose a reason for hiding this comment

Cali93 left a comment

Choose a reason for hiding this comment

ganchoradkov commented Aug 1, 2024 •

edited

Loading

chris13524 commented Aug 12, 2024 •

edited

Loading