fix(wc): check active sessions and extend existing pairing if present #4051
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4051 +/- ##
==========================================
- Coverage 83.23% 83.20% -0.03%
==========================================
Files 718 718
Lines 26735 26741 +6
Branches 3634 3634
==========================================
- Hits 22252 22250 -2
- Misses 4420 4428 +8
Partials 63 63
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
| @@ -15,6 +15,7 @@ import App from 'src/app/App' | |||
| import * as Sentry from '@sentry/react-native' | |||
| import 'react-native-gesture-handler' | |||
| import { Text, TextInput } from 'react-native' | |||
| import '@walletconnect/react-native-compat' | |||
There was a problem hiding this comment.
This is how several other wallets import @walletconnect/react-native-compat: GitHub Search Results.
// index.js
import './polyfills';
// polyfills.ts
import '@walletconnect/react-native-compat';
There was a problem hiding this comment.
is this directly related to the fix?
There was a problem hiding this comment.
When the import was in the src/walletconnect/saga, I was seeing more issues with pairings.
| @@ -15,6 +15,7 @@ import App from 'src/app/App' | |||
| import * as Sentry from '@sentry/react-native' | |||
| import 'react-native-gesture-handler' | |||
| import { Text, TextInput } from 'react-native' | |||
| import '@walletconnect/react-native-compat' | |||
There was a problem hiding this comment.
is this directly related to the fix?
| @@ -183,9 +182,15 @@ function* createWalletConnectChannel() { | |||
| } | |||
| }) as EventChannel<WalletConnectActions> | |||
| } | |||
|
|
|||
| /** | |||
| * It appears Actions.INITIALISE_PAIRING is emitting on both initial connections and on payload requests | |||
There was a problem hiding this comment.
I'm not sure I understand this comment. Can you rephrase it? Specifically what/who is emitting? Also, if what/who is emitting erroneously or unexpectedly, should we fix that instead?
There was a problem hiding this comment.
It looks like some old assumptions were made on our end here in src/walleconnect/walletconnect.ts about what what is a wallet connect action vs connection request. I agree a fix here is probably more appropriate.
|
|
||
| if (foundSession) { | ||
| Logger.debug(TAG + '@handleInitialisePairing', 'extending session start') | ||
| yield* call([client, 'extendSession'], { topic: foundSession }) |
There was a problem hiding this comment.
Q: isn't foundSession an instance of Session (vs string which is what I'd expect for topic)?
There was a problem hiding this comment.
In testing foundSession is just a string that is the pairingTopic, but I'll double check to be sure.
|
Closing in favor of #4062 |
Description
A fix for the error from wallet connect
{ context: 'core' }, 'Pairing already exists: <pairingTopic>'. This error was commonly occurring and could be contributing to session mismatches that result in the the following errors describe by @jeanregisser in https://github.com/orgs/WalletConnect/discussions/3291#discussioncomment-6608198.{ context: 'client' }, 'No matching key. session: 9a05b6099c6cc662ed39dd43795adec83ad64dfc6cfd7779ae1bc3d6470bb5c6' { context: 'client' }, [Error: No matching key. session: 9a05b6099c6cc662ed39dd43795adec83ad64dfc6cfd7779ae1bc3d6470bb5c6]Test plan
{ context: 'core' }, 'Pairing already exists: <pairingTopic>'error is no longer seen.Related issues
Fixes: RET-818
Backwards compatibility
Yes