Add support for custom chains in changeNetwork function #425
Conversation
arjanjohan
requested review from
0xmaayan,
kaw2k,
hardsetting and
kent-white
as code owners
| @@ -39,7 +39,7 @@ export interface WalletContextState { | |||
| ): Promise<PendingTransactionResponse>; | |||
| signMessage(message: SignMessagePayload): Promise<SignMessageResponse>; | |||
| signMessageAndVerify(message: SignMessagePayload): Promise<boolean>; | |||
| changeNetwork(network: Network): Promise<AptosChangeNetworkOutput>; | |||
| changeNetwork(network: Network, chainId?: number): Promise<AptosChangeNetworkOutput>; | |||
There was a problem hiding this comment.
Lets let the user pass in the chainId as a string, and have the adapter do the parsing before sending it to the wallet
There was a problem hiding this comment.
I know in the example used in this repo, the chainId is provided in an input field and using string would make sense. However, I believe this to be an exceptional case. I imagina all dApps that will utilize this feature will handle the logic in the backend, and the chainId is not provided by the user but rather from some backend storage where it's stored as number.
Let me know what you think.
There was a problem hiding this comment.
I see. I'd argue that they will be retrieving it from an API query, where it will mostly be in a string format. Instead of requiring the dapp to convert it to a number, the adapter can handle that conversion for them.
There was a problem hiding this comment.
That makes sense, thanks for clarifying. I will modify it and push the changes.
| @@ -39,7 +39,7 @@ export interface WalletContextState { | |||
| ): Promise<PendingTransactionResponse>; | |||
| signMessage(message: SignMessagePayload): Promise<SignMessageResponse>; | |||
| signMessageAndVerify(message: SignMessagePayload): Promise<boolean>; | |||
| changeNetwork(network: Network): Promise<AptosChangeNetworkOutput>; | |||
| changeNetwork(network: Network, chainId?: number): Promise<AptosChangeNetworkOutput>; | |||
There was a problem hiding this comment.
I see. I'd argue that they will be retrieving it from an API query, where it will mostly be in a string format. Instead of requiring the dapp to convert it to a number, the adapter can handle that conversion for them.
I processed the comments and updated the example nextJS app. |
Current Situation:
The changeNetwork function does not support custom networks because the chainId cannot be inferred from the network name.
Fix:
This PR introduces an optional customChainId parameter to the changeNetwork function in
WalletCore.ts. The parameter is only used when switching to a custom chain.Additionally, I have updated both
wallet-adapter-reactandwallet-adapter-vueto pass this optional parameter when calling the changeNetwork function.Tests:
I updated
apps/nextjs-example, and it now supports switching between both custom and predefined networks. However, I did not updateapps/nuxt-exampleas I have limited experience with Nuxt, and network switching in that example already appears to be broken.