Skip to content

feat: Wallet connection improvements #2632

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

Merged
merged 3 commits into from
Apr 2, 2024
Merged

feat: Wallet connection improvements #2632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e0b0b5e
connection improvements
MananTank Apr 2, 2024
bc2ca74
add changeset
MananTank Apr 2, 2024
fb852c6
update account in wallet
MananTank Apr 2, 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
8 changes: 8 additions & 0 deletions .changeset/little-falcons-remain.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"thirdweb": patch
---

Various Improvements for wallet connection

- change `accountsChanged` event to `accountChanged` event and emit new `Account` object instead of creating it in the connection manager
- WalletConnect connection improvements
10 changes: 7 additions & 3 deletions packages/thirdweb/src/wallets/coinbase/coinbaseSDKWallet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,14 @@ function onConnect(
}

function onAccountsChanged(accounts: string[]) {
if (accounts.length === 0) {
onDisconnect();
if (accounts[0]) {
const newAccount = {
...account,
address: getAddress(accounts[0]),
};
emitter.emit("accountChanged", newAccount);
} else {
emitter.emit("accountsChanged", accounts);
onDisconnect();
}
}

Expand Down
8 changes: 8 additions & 0 deletions packages/thirdweb/src/wallets/create-wallet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ export function createWallet<const ID extends WalletId>(
unsubscribeDisconnect();
});

emitter.subscribe("accountChanged", (_account) => {
account = _account;
});

let handleSwitchChain: (chain: Chain) => Promise<void> = async () => {
throw new Error("Not implemented yet");
};
Expand Down Expand Up @@ -463,6 +467,10 @@ function coinbaseWalletSDK(): Wallet<"com.coinbase.wallet"> {
unsubscribeDisconnect();
});

emitter.subscribe("accountChanged", (_account) => {
account = _account;
});

return {
id: "com.coinbase.wallet",
subscribe: emitter.subscribe,
Expand Down
11 changes: 8 additions & 3 deletions packages/thirdweb/src/wallets/injected/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,15 @@ async function onConnect(
}

function onAccountsChanged(accounts: string[]) {
if (accounts.length === 0) {
onDisconnect();
if (accounts[0]) {
const newAccount = {
...account,
address: getAddress(accounts[0]),
Copy link
Member

Choose a reason for hiding this comment

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

is there an assumption that the 'new' account will always be accounts[0] ?

also kind of weird to spread the old account functions and just override the address, but i guess in this case it'll work

Copy link
Member Author

@MananTank MananTank Apr 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes - the first one is the "active" account if the wallet has multiple accounts (ex: metamask )

Re: the spread, We can also simply mutate the address property - but if we do that - the account object will not trigger a re-render in react - so we clone the object and change the address since that's the only change we need to make

};

emitter.emit("accountChanged", newAccount);
} else {
emitter.emit("accountsChanged", accounts);
onDisconnect();
}
}

Expand Down
23 changes: 4 additions & 19 deletions packages/thirdweb/src/wallets/manager/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,27 +102,12 @@ export function createConnectionManager(storage: AsyncStorage) {

// setup listeners

const onAccountsChanged = (addresses: string[]) => {
const newAddress = addresses[0];
if (!newAddress) {
onWalletDisconnect(wallet);
return;
} else {
// TODO: get this new object as argument from onAccountsChanged
// this requires emitting events from the wallet
const newAccount: Account = {
...account,
address: newAddress,
};

activeAccountStore.setValue(newAccount);
}
const onAccountsChanged = (newAccount: Account) => {
activeAccountStore.setValue(newAccount);
};

const unsubAccounts = wallet.subscribe(
"accountsChanged",
onAccountsChanged,
);
const unsubAccounts = wallet.subscribe("accountChanged", onAccountsChanged);

const unsubChainChanged = wallet.subscribe("chainChanged", (chain) =>
activeWalletChainStore.setValue(chain),
);
Expand Down
122 changes: 71 additions & 51 deletions packages/thirdweb/src/wallets/wallet-connect/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
getRpcUrlForChain,
} from "../../chains/utils.js";
import type { Chain } from "../../chains/types.js";
import { ethereum } from "../../chains/chain-definitions/ethereum.js";
import { isHex, numberToHex, type Hex } from "../../utils/encoding/hex.js";
import { getDefaultAppMetadata } from "../utils/defaultDappMetadata.js";
import type { WCSupportedWalletIds } from "../__generated__/wallet-ids.js";
Expand All @@ -34,6 +33,8 @@ import {
getSavedConnectParamsFromStorage,
saveConnectParamsToStorage,
} from "../storage/walletStorage.js";
import type { ThirdwebClient } from "../../client/client.js";
import { getAddress } from "../../utils/address.js";

const asyncLocalStorage =
typeof window !== "undefined" ? _asyncLocalStorage : undefined;
Expand All @@ -42,7 +43,7 @@ type WCProvider = InstanceType<typeof EthereumProvider>;

type SavedConnectParams = {
optionalChains?: Chain[];
chain: Chain;
chain?: Chain;
pairingTopic?: string;
};

Expand Down Expand Up @@ -76,14 +77,6 @@ export async function connectWC(

const wcOptions = options.walletConnect;

const targetChain = options?.chain || ethereum;
const targetChainId = targetChain.id;

const rpc = getRpcUrlForChain({
chain: targetChain,
client: options.client,
});

const { onDisplayUri } = wcOptions || {};

if (onDisplayUri) {
Expand All @@ -92,17 +85,23 @@ export async function connectWC(
}
}

const { rpcMap, chainsToRequest } = getChainsToRequest({
client: options.client,
chain: options.chain,
optionalChains: options.walletConnect?.optionalChains,
});

// If there no active session, or the chain is stale, force connect.
if (!provider.session || _isChainsState) {
await provider.connect({
pairingTopic: wcOptions?.pairingTopic,
chains: [Number(targetChainId)],
rpcMap: {
[targetChainId.toString()]: rpc,
},
...(wcOptions?.pairingTopic
? { pairingTopic: wcOptions?.pairingTopic }
: {}),
optionalChains: chainsToRequest,
rpcMap: rpcMap,
});

setRequestedChainsIds([targetChainId]);
setRequestedChainsIds(chainsToRequest);
}

// If session exists and chains are authorized, enable provider for required chain
Expand All @@ -117,7 +116,7 @@ export async function connectWC(
if (options) {
const savedParams: SavedConnectParams = {
optionalChains: options.walletConnect?.optionalChains,
chain: chain,
chain: options.chain,
pairingTopic: options.walletConnect?.pairingTopic,
};

Expand Down Expand Up @@ -205,11 +204,10 @@ async function initProvider(
"@walletconnect/ethereum-provider"
);

const targetChain = options.chain || ethereum;

const rpc = getRpcUrlForChain({
chain: targetChain,
const { rpcMap, chainsToRequest } = getChainsToRequest({
client: options.client,
chain: options.chain,
optionalChains: options.walletConnect?.optionalChains,
});

const provider = await EthereumProvider.init({
Expand All @@ -220,7 +218,7 @@ async function initProvider(
projectId: wcOptions?.projectId || defaultWCProjectId,
optionalMethods: OPTIONAL_METHODS,
optionalEvents: OPTIONAL_EVENTS,
optionalChains: [targetChain.id],
optionalChains: chainsToRequest,
metadata: {
name: wcOptions?.appMetadata?.name || getDefaultAppMetadata().name,
description:
Expand All @@ -231,9 +229,7 @@ async function initProvider(
wcOptions?.appMetadata?.logoUrl || getDefaultAppMetadata().logoUrl,
],
},
rpcMap: {
[targetChain.id]: rpc,
},
rpcMap: rpcMap,
qrModalOptions: wcOptions?.qrModalOptions,
disableProviderPing: true,
});
Expand All @@ -242,15 +238,7 @@ async function initProvider(

// disconnect the provider if chains are stale when (if not auto connecting)
if (!isAutoConnect) {
const chains = [
targetChain,
...(options?.walletConnect?.optionalChains || []),
];

const isStale = await isChainsStale(
provider,
chains.map((c) => c.id),
);
const isStale = await isChainsStale(provider, chainsToRequest);

if (isStale && provider.session) {
await provider.disconnect();
Expand Down Expand Up @@ -293,20 +281,6 @@ async function initProvider(
});
}

// try switching to correct chain
if (options?.chain && provider.chainId !== options?.chain.id) {
try {
await switchChainWC(provider, options.chain);
} catch (e) {
console.error("Failed to Switch chain to target chain");
console.error(e);
// throw only if not auto connecting
if (!isAutoConnect) {
throw e;
}
}
}

return provider;
}

Expand Down Expand Up @@ -385,10 +359,14 @@ function onConnect(
}

function onAccountsChanged(accounts: string[]) {
if (accounts.length === 0) {
onDisconnect();
if (accounts[0]) {
const newAccount: Account = {
...account,
address: getAddress(accounts[0]),
};
emitter.emit("accountChanged", newAccount);
} else {
emitter.emit("accountsChanged", accounts);
onDisconnect();
}
}

Expand Down Expand Up @@ -521,3 +499,45 @@ async function getRequestedChainsIds(): Promise<number[]> {
const data = localStorage.getItem(storageKeys.requestedChains);
return data ? JSON.parse(data) : [];
}

type ArrayOneOrMore<T> = {
0: T;
} & Array<T>;

function getChainsToRequest(options: {
chain?: Chain;
optionalChains?: Chain[];
client: ThirdwebClient;
}) {
const rpcMap: Record<number, string> = {};

if (options.chain) {
rpcMap[options.chain.id] = getRpcUrlForChain({
chain: options.chain,
client: options.client,
});
}

// limit optional chains to 10
const optionalChains = (options?.optionalChains || []).slice(0, 10);
Copy link
Member

Choose a reason for hiding this comment

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

was this a limitation from WC? or our choice?

Copy link
Member Author

@MananTank MananTank Apr 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Wallet connection does not work if send too many optional chains ( this happens in our dashboard - we also limit the optional chains in v4 for this reason )


optionalChains.forEach((chain) => {
rpcMap[chain.id] = getRpcUrlForChain({
chain: chain,
client: options.client,
});
});

const optionalChainIds = optionalChains.map((c) => c.id) || [];

const chainsToRequest: ArrayOneOrMore<number> = options.chain
? [options.chain.id, ...optionalChainIds]
: optionalChainIds.length > 0
? (optionalChainIds as ArrayOneOrMore<number>)
: [1];

return {
rpcMap,
chainsToRequest,
};
}
3 changes: 2 additions & 1 deletion packages/thirdweb/src/wallets/wallet-emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import type { Chain } from "../chains/types.js";
import { createEmitter, type Emitter } from "../utils/tiny-emitter.js";
import type { Account } from "./interfaces/wallet.js";
import type { WalletAutoConnectionOption, WalletId } from "./wallet-types.js";

export type WalletEmitterEvents<TWalletId extends WalletId> = {
accountsChanged: string[];
accountChanged: Account;
disconnect?: never;
chainChanged: Chain;
onConnect: WalletAutoConnectionOption<TWalletId>;
Expand Down