chore: Relay QR generation flow improvements #2652
Conversation
…e after 15m. Remove duplicated display_uri handlers and subscribe in client constructors
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
tomiir
requested review from
glitch-txs,
ganchoradkov,
svenvoskamp and
chris13524
| @@ -699,6 +695,10 @@ export class Web3Modal extends Web3ModalScaffold { | |||
|
|
|||
| this.walletConnectProvider = await EthereumProvider.init(walletConnectProviderOptions) | |||
|
|
|||
| this.walletConnectProvider.on('display_uri', (uri: string) => { | |||
There was a problem hiding this comment.
minor but you could also
this.walletConnectProvider.on('display_uri', this.setQRCodeURI)
| @@ -327,6 +327,10 @@ export class Web3ModalScaffold { | |||
| BlockchainApiController.setClientId(clientId) | |||
| } | |||
|
|
|||
| protected setQRCodeURI: (typeof ConnectionController)['setQRCodeURI'] = uri => { | |||
There was a problem hiding this comment.
the uri is not only for the QR code but for all connections using WalletConnect protocol. maybe setURI or setWalletConnectURI works better?
| @@ -7,7 +7,7 @@ import { UniversalProviderFactory } from './universalProvider.js' | |||
| import { BaseConnector } from './baseConnector.js' | |||
|
|
|||
| import type { Signer } from '@solana/web3.js' | |||
| import type UniversalProvider from '@walletconnect/universal-provider' | |||
There was a problem hiding this comment.
This actually breaks in some bundlers (default import of UP), it should be a name import.
There was a problem hiding this comment.
@glitch-txs can we make a lint or tsconfig role to disallow this?
| * For 15minutes, the relay handles retries and reconnections internally. | ||
| * We re-initialize the connection after that | ||
| */ | ||
| this.interval = setInterval(this.initializeConnection.bind(this), 15 * 60 * 1000) |
There was a problem hiding this comment.
This is the Sign SDK that handles this, not the relay. And I would be concerned that depending on this behavior is dangerous. E.g. if the SDK is updated to a version w/o this
Description
=> Only subscribe to
display_urievent once per client=> Only re-initialize the connection flow after 15 minutes, given the sdk internally handles retries for that time
=> Always regenerate the URI on an error.
WIP:
Type of change
Associated Issues
For Linear issues: Closes APKT-144
Checklist