feat: added wallet connect integration via api #357
Conversation
…etConnect handling
…etConnect handling
…n void and enhance transaction validation
…n void and enhance transaction validation
| } | ||
|
|
||
| this.logger.debug( | ||
| `Starting a new browser context (temp context: ${browserContextPath})`, |
There was a problem hiding this comment.
Why do you print the path instead of the previous value?
| if (!background) | ||
| background = await this.browserContext.waitForEvent('serviceworker'); | ||
| this.extensionId = background.url().split('/')[2]; | ||
| if (this.walletExtensionStartPath) { |
There was a problem hiding this comment.
Maybe we can use the WalletConnectTypes here
There was a problem hiding this comment.
no, browser context doesn’t know anything about wallets
| private walletPage: WalletPage< | ||
| WalletConnectTypes.WC | WalletConnectTypes.EOA | WalletConnectTypes.IFRAME | ||
| | WalletConnectTypes.WC | ||
| | WalletConnectTypes.WC_SDK |
There was a problem hiding this comment.
Can we unite the WC and WC_SDK?
| await this.walletPage.setupNetwork(this.options.networkConfig); | ||
| await this.walletPage.changeNetwork(this.options.networkConfig.chainName); | ||
| await this.browserContextService.closePages(); | ||
| if (this.options.walletConfig.WALLET_TYPE !== WalletConnectTypes.WC_SDK) { |
There was a problem hiding this comment.
Let's move it to the private method?
| @@ -84,9 +87,13 @@ export class BrowserService { | |||
| await this.setupWithNode(); | |||
There was a problem hiding this comment.
Why don't we have any changes in the setupWithNode method?
| EXTENSION_WALLET_NAME: 'wcSDK', | ||
| CONNECTED_WALLET_NAME: 'WalletConnect', | ||
| CONNECT_BUTTON_NAME: 'WalletConnect', | ||
| STORE_EXTENSION_ID: 'nkbihfbeogaeaoehlefnkodbefgpgknn', |
There was a problem hiding this comment.
Why did you keep parameters are unnecessary for the WC_SDK here?
| } | ||
|
|
||
| async setup() { | ||
| if (this.options.walletConfig.WALLET_TYPE === WalletConnectTypes.WC_SDK) { |
There was a problem hiding this comment.
I think we can try not to add this condition here
| } | ||
|
|
||
| private setWalletPage() { | ||
| if (this.options.walletConfig.WALLET_TYPE === WalletConnectTypes.WC_SDK) { |
There was a problem hiding this comment.
Can we move it to the switch..case condition below?
| } from './pages/elements'; | ||
| import { getAddress } from 'viem'; | ||
| import { isPopularMainnetNetwork, isPopularTestnetNetwork } from './helper'; | ||
| import { WCSessionRequest } from '../../walletConnect/wc.service'; |
There was a problem hiding this comment.
Can you fill the index file and fix the import lines everywhere?
packages/wallets/src/okx/okx.page.ts
Outdated
|
|
||
| /** Confirm transaction */ | ||
| async confirmTx(page: Page) { | ||
| async confirmTx(page: Page | WCSessionRequest) { |
There was a problem hiding this comment.
Can we use the same format here?
page: T extends WalletConnectTypes.WC_SDK ? WCSessionRequest : Page
…tion cancellation
…tion cancellation
2 participants
No description provided.