Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor/switch network and local storage controls #2888

Merged
merged 9 commits into from
Sep 20, 2024
Merged

refactor/switch network and local storage controls #2888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
bf297e3
refactor: update chainChanged handlers, refactor initializers on appk…
enesozturk Sep 20, 2024
91d9794
refactor: tests, linter and build issues
enesozturk Sep 20, 2024
587e96f
refactor: update tests for setting caip network calls
enesozturk Sep 20, 2024
9130337
chore: add new test cases and lab ts issues
enesozturk Sep 20, 2024
42eb1d8
chore: add changeset file
enesozturk Sep 20, 2024
528496f
chore: code review updates
enesozturk Sep 20, 2024
36527d9
Merge branch 'main' of https://github.com/reown-com/appkit into refac…
enesozturk Sep 20, 2024
dd1c571
fix: linter issues
enesozturk Sep 20, 2024
468ec8d
Merge branch 'main' into refactor/switch-network-and-local-storage-co…
enesozturk Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions .changeset/three-doors-love.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
'@reown/appkit-adapter-ethers5': patch
'@reown/appkit-adapter-ethers': patch
'@reown/appkit-adapter-solana': patch
'@reown/appkit-adapter-wagmi': patch
'@reown/appkit-scaffold-ui': patch
'@apps/laboratory': patch
'@reown/appkit': patch
'@reown/appkit-common': patch
'@reown/appkit-core': patch
'@apps/demo': patch
'@apps/gallery': patch
'@examples/html-ethers': patch
'@examples/html-ethers5': patch
'@examples/html-wagmi': patch
'@examples/next-ethers': patch
'@examples/next-wagmi': patch
'@examples/react-ethers': patch
'@examples/react-ethers5': patch
'@examples/react-solana': patch
'@examples/react-wagmi': patch
'@examples/vue-ethers5': patch
'@examples/vue-solana': patch
'@examples/vue-wagmi': patch
'@reown/appkit-adapter-polkadot': patch
'@reown/appkit-utils': patch
'@reown/appkit-cdn': patch
'@reown/appkit-ethers': patch
'@reown/appkit-ethers5': patch
'@reown/appkit-polyfills': patch
'@reown/appkit-siwe': patch
'@reown/appkit-solana': patch
'@reown/appkit-ui': patch
'@reown/appkit-wagmi': patch
'@reown/appkit-wallet': patch
---

Updates active network state management and local storage controls
2 changes: 1 addition & 1 deletion apps/laboratory/src/components/AppKitNetworkInfo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export function AppKitNetworkInfo() {
<Heading size="xs" textTransform="uppercase" pb="2">
Address
</Heading>
<Text data-testid="w3m-address">{address}</Text>
<Text data-testid="w3m-address">{address || '-'}</Text>
</Box>

<Box>
Expand Down
4 changes: 2 additions & 2 deletions apps/laboratory/src/components/Wagmi/WagmiModalInfo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ export function WagmiModalInfo() {
async function getClientId() {
if (connector?.type === 'walletConnect') {
const provider = await connector?.getProvider?.()
const ethereumProvider = provider as UniversalProvider
const universalProvider = provider as UniversalProvider

return ethereumProvider.client?.core?.crypto?.getClientId()
return universalProvider?.client?.core?.crypto?.getClientId()
}

return null
Expand Down
17 changes: 16 additions & 1 deletion apps/laboratory/tests/wallet.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,23 @@ sampleWalletTest('it should switch networks and sign', async ({ library }) => {
await processChain(0)
})

sampleWalletTest('it should show last connected network after refreshing', async ({ library }) => {
const chainName = library === 'solana' ? 'Solana Testnet' : 'Polygon'

await modalPage.switchNetwork(chainName)
await modalValidator.expectSwitchedNetwork(chainName)
await modalPage.closeModal()

await modalPage.page.reload()

await modalPage.openModal()
await modalPage.openNetworks()
await modalValidator.expectSwitchedNetwork(chainName)
await modalPage.closeModal()
})

sampleWalletTest('it should reject sign', async ({ library }) => {
const chainName = library === 'solana' ? 'Solana' : DEFAULT_CHAIN_NAME
const chainName = library === 'solana' ? 'Solana Testnet' : 'Polygon'
await modalPage.sign()
await walletValidator.expectReceivedSign({ chainName })
await walletPage.handleRequest({ accept: false })
Expand Down
26 changes: 9 additions & 17 deletions packages/adapters/ethers/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,12 +613,10 @@ export class EthersAdapter {
if (provider) {
const { addresses, chainId } = await EthersHelpersUtil.getUserInfo(provider)
const firstAddress = addresses?.[0]
const caipNetwork = this.caipNetworks.find(c => c.chainId === chainId)
const caipAddress = `${this.chainNamespace}:${chainId}:${firstAddress}` as CaipAddress

if (firstAddress && chainId && caipNetwork) {
if (firstAddress && chainId) {
this.appKit?.setCaipAddress(caipAddress, this.chainNamespace)
this.appKit?.setCaipNetwork(caipNetwork)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should do all the caipNetwork setters from a single place and do it only when it's necessary.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++++++

ProviderUtil.setProviderId('eip155', providerId)
ProviderUtil.setProvider<Provider>('eip155', provider)
this.appKit?.setStatus('connected', this.chainNamespace)
Expand Down Expand Up @@ -660,8 +658,6 @@ export class EthersAdapter {
: [{ address, type: preferredAccountType as 'eoa' | 'smartAccount' }],
this.chainNamespace
)
const caipNetwork = this.caipNetworks.find(c => c.chainId === chainId)
this.appKit?.setCaipNetwork(caipNetwork)
this.appKit?.setStatus('connected', this.chainNamespace)
this.appKit?.setCaipAddress(
`${this.chainNamespace}:${chainId}:${address}`,
Expand Down Expand Up @@ -717,13 +713,13 @@ export class EthersAdapter {
}
}

const chainChangedHandler = (networkId: string) => {
if (networkId) {
const networkIdNumber =
typeof networkId === 'string'
? EthersHelpersUtil.hexStringToNumber(networkId)
: Number(networkId)
const caipNetwork = this.caipNetworks.find(c => c.chainId === networkIdNumber)
const chainChangedHandler = (chainId: string) => {
const chainIdNumber =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should control if the received chainId is already active, otherwise - still couldn't figure it out why - this listener is getting triggered multiple times in a single network switch and breaking the state

Same logic applied to all clients

typeof chainId === 'string' ? EthersHelpersUtil.hexStringToNumber(chainId) : Number(chainId)
const caipNetwork = this.caipNetworks.find(c => c.chainId === chainIdNumber)
const currentCaipNetwork = this.appKit?.getCaipNetwork()

if (!currentCaipNetwork || currentCaipNetwork?.id !== caipNetwork?.id) {
this.appKit?.setCaipNetwork(caipNetwork)
}
}
Expand Down Expand Up @@ -836,10 +832,7 @@ export class EthersAdapter {

this.appKit?.setLoading(true)
const chainId = NetworkUtil.caipNetworkIdToNumber(this.appKit?.getCaipNetwork()?.id)
const caipNetwork = this.caipNetworks.find(c => c.chainId === chainId)

this.appKit?.setCaipAddress(`eip155:${chainId}:${address}`, this.chainNamespace)
this.appKit?.setCaipNetwork(caipNetwork)
this.appKit?.setStatus('connected', this.chainNamespace)
this.appKit?.setPreferredAccountType(type as W3mFrameTypes.AccountType, this.chainNamespace)

Expand Down Expand Up @@ -1014,13 +1007,12 @@ export class EthersAdapter {
}

public async switchNetwork(caipNetwork: CaipNetwork) {
const requestSwitchNetwork = async (provider: Provider) => {
async function requestSwitchNetwork(provider: Provider) {
try {
await provider.request({
method: 'wallet_switchEthereumChain',
params: [{ chainId: EthersHelpersUtil.numberToHexString(caipNetwork.chainId) }]
})
this.appKit?.setCaipNetwork(caipNetwork)
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} catch (switchError: any) {
if (
Expand Down
6 changes: 1 addition & 5 deletions packages/adapters/ethers/src/tests/client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ describe('EthersAdapter', () => {
])
})
)
expect(mockAppKit.setCaipNetwork).toHaveBeenCalledWith(newNetwork)
})

it('should add network if not recognized by wallet', async () => {
Expand Down Expand Up @@ -344,7 +343,6 @@ describe('EthersAdapter', () => {

expect(mockAppKit.setLoading).toHaveBeenCalledWith(true)
expect(mockAppKit.setCaipAddress).toHaveBeenCalledWith(`eip155:${1}:${address}`, 'eip155')
expect(mockAppKit.setCaipNetwork).toHaveBeenCalled()
expect(mockAppKit.setStatus).toHaveBeenCalledWith('connected', 'eip155')
expect(mockAppKit.setPreferredAccountType).toHaveBeenCalledWith(type, 'eip155')

Expand Down Expand Up @@ -388,7 +386,6 @@ describe('EthersAdapter', () => {
[{ address: mockAddress, type: mockPreferredAccountType }],
'eip155'
)
expect(mockAppKit.setCaipNetwork).toHaveBeenCalled()
expect(mockAppKit.setStatus).toHaveBeenCalledWith('connected', 'eip155')
expect(mockAppKit.setCaipAddress).toHaveBeenCalledWith(
`eip155:${mockChainId}:${mockAddress}`,
Expand Down Expand Up @@ -527,7 +524,6 @@ describe('EthersAdapter', () => {
SafeLocalStorageKeys.WALLET_NAME,
'MetaMask'
)
expect(mockAppKit.setCaipNetwork).toHaveBeenCalled()
expect(mockAppKit.setCaipAddress).toHaveBeenCalled()
expect(ProviderUtil.setProviderId).toHaveBeenCalledWith('eip155', 'injected')
expect(ProviderUtil.setProvider).toHaveBeenCalledWith('eip155', mockProvider)
Expand Down Expand Up @@ -584,7 +580,7 @@ describe('EthersAdapter', () => {
const chainChangedHandler = mockProvider.on.mock.calls.find(
(call: string[]) => call[0] === 'chainChanged'
)[1]
await chainChangedHandler('0x1')
await chainChangedHandler('0x137')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be trigger the setCaipNetwork only if the network has changed. So passing Polygon id now


expect(mockAppKit.setCaipNetwork).toHaveBeenCalled()
})
Expand Down
14 changes: 7 additions & 7 deletions packages/adapters/ethers5/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -722,13 +722,13 @@ export class Ethers5Adapter {
}
}

const chainChangedHandler = (networkId: string) => {
if (networkId) {
const networkIdNumber =
typeof networkId === 'string'
? EthersHelpersUtil.hexStringToNumber(networkId)
: Number(networkId)
const caipNetwork = this.caipNetworks.find(c => c.chainId === networkIdNumber)
const chainChangedHandler = (chainId: string) => {
const chainIdNumber =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

typeof chainId === 'string' ? EthersHelpersUtil.hexStringToNumber(chainId) : Number(chainId)
const caipNetwork = this.caipNetworks.find(c => c.chainId === chainIdNumber)
const currentCaipNetwork = this.appKit?.getCaipNetwork()

if (!currentCaipNetwork || currentCaipNetwork?.id !== caipNetwork?.id) {
this.appKit?.setCaipNetwork(caipNetwork)
}
}
Expand Down
4 changes: 1 addition & 3 deletions packages/adapters/ethers5/src/tests/client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,6 @@ describe('EthersAdapter', () => {

expect(mockAppKit.setLoading).toHaveBeenCalledWith(true)
expect(mockAppKit.setCaipAddress).toHaveBeenCalledWith(`eip155:${1}:${address}`, 'eip155')
expect(mockAppKit.setCaipNetwork).toHaveBeenCalled()
expect(mockAppKit.setStatus).toHaveBeenCalledWith('connected', 'eip155')
expect(mockAppKit.setPreferredAccountType).toHaveBeenCalledWith(type, 'eip155')

Expand Down Expand Up @@ -532,7 +531,6 @@ describe('EthersAdapter', () => {
SafeLocalStorageKeys.WALLET_NAME,
'MetaMask'
)
expect(mockAppKit.setCaipNetwork).toHaveBeenCalled()
expect(ProviderUtil.setProviderId).toHaveBeenCalledWith('eip155', 'injected')
expect(ProviderUtil.setProvider).toHaveBeenCalledWith('eip155', mockProvider)
expect(mockAppKit.setStatus).toHaveBeenCalledWith('connected', 'eip155')
Expand Down Expand Up @@ -588,7 +586,7 @@ describe('EthersAdapter', () => {
const chainChangedHandler = mockProvider.on.mock.calls.find(
(call: string[]) => call[0] === 'chainChanged'
)[1]
await chainChangedHandler('0x1')
await chainChangedHandler('0x2')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same - updating the value to make sure it's triggering setCaipNetwork


expect(mockAppKit.setCaipNetwork).toHaveBeenCalled()
})
Expand Down
5 changes: 0 additions & 5 deletions packages/adapters/solana/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { Connection } from '@solana/web3.js'
import {
AccountController,
ApiController,
AssetController,
ChainController,
CoreHelperUtil,
EventsController
Expand Down Expand Up @@ -311,10 +310,6 @@ export class SolanaAdapter implements ChainAdapter {

this.syncRequestedNetworks(this.caipNetworks)

AssetController.subscribeNetworkImages(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary. I'll handle the network images fetching in another PR

this.syncNetwork()
})

ChainController.subscribeKey('activeCaipNetwork', (newCaipNetwork: CaipNetwork | undefined) => {
const newChain = this.caipNetworks.find(
_chain => _chain.chainId === newCaipNetwork?.id.split(':')[1]
Expand Down
3 changes: 0 additions & 3 deletions packages/adapters/wagmi/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,6 @@ export class WagmiAdapter implements ChainAdapter {
await SIWEController.signOut()
}
SafeLocalStorage.removeItem(SafeLocalStorageKeys.WALLET_ID)
SafeLocalStorage.removeItem(SafeLocalStorageKeys.ACTIVE_CAIP_NETWORK)
SafeLocalStorage.removeItem(SafeLocalStorageKeys.CONNECTED_CONNECTOR)
SafeLocalStorage.removeItem(SafeLocalStorageKeys.WALLET_NAME)
this.appKit?.setClientId(null)
Expand Down Expand Up @@ -609,7 +608,6 @@ export class WagmiAdapter implements ChainAdapter {
this.appKit?.resetNetwork()
this.appKit?.setAllAccounts([], this.chainNamespace)
SafeLocalStorage.removeItem(SafeLocalStorageKeys.WALLET_ID)
SafeLocalStorage.removeItem(SafeLocalStorageKeys.ACTIVE_CAIP_NETWORK)

return
}
Expand Down Expand Up @@ -647,7 +645,6 @@ export class WagmiAdapter implements ChainAdapter {
}
} else if (status === 'connected' && address && chainId) {
const caipAddress = `eip155:${chainId}:${address}` as CaipAddress
this.appKit?.resetAccount(this.chainNamespace)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we've connected, we don't have to resetAccount. This should have been removed long ago - I remember we removed these

this.syncNetwork(address, chainId, true)
this.appKit?.setCaipAddress(caipAddress, this.chainNamespace)
await Promise.all([
Expand Down
5 changes: 3 additions & 2 deletions packages/adapters/wagmi/src/connectors/UniversalConnector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ import {
numberToHex
} from 'viem'
import { WcHelpersUtil } from '@reown/appkit'
import { StorageUtil } from '@reown/appkit-core'
import type { AppKitOptions } from '@reown/appkit'
import type { AppKit } from '@reown/appkit'
import { convertToAppKitChains } from '../utils/helpers.js'
import { SafeLocalStorage, SafeLocalStorageKeys } from '@reown/appkit-common'

type UniversalConnector = Connector & {
onDisplayUri(uri: string): void
Expand Down Expand Up @@ -231,7 +231,8 @@ export function walletConnect(parameters: AppKitOptionsParams, appKit: AppKit) {
const currentChainId = appKit.getCaipNetwork()?.chainId

if (chainId && currentChainId !== chainId) {
const storedCaipNetwork = SafeLocalStorage.getItem(SafeLocalStorageKeys.ACTIVE_CAIP_NETWORK)
const storedCaipNetwork = StorageUtil.getStoredActiveCaipNetwork()

if (storedCaipNetwork && storedCaipNetwork.chainNamespace === 'eip155') {
await this.switchChain?.({ chainId: Number(storedCaipNetwork.chainId) })
} else {
Expand Down
Loading
Loading