wallet-selector icon indicating copy to clipboard operation
wallet-selector copied to clipboard

Ability to switch networks from a `NetworkId`

Open austinabell opened this issue 2 years ago • 6 comments

Is your feature request related to a problem? Please describe. Currently, the network can be overridden through selector.options.network = getNetworkPreset(network); (UT if that's functional or if any missing side effects) but getNetworkPreset is not exposed from the library.

Describe the solution you'd like Would be nice to have a helper function to update the network of the wallet selector using a NetworkId to avoid having to copy code (and assume no side effects of modifying options).

Might be worthwhile to consider #403 with this, as the changes are somewhat connected (although this affects the selector type where the other is relating to the modal).

Describe alternatives you've considered

  • This could possibly just be done by replacing the wallet selector instance, but unsure if there are unintended side effects from doing this. If this is the intended flow, feel free to close this issue

Acceptance criteria Network can be changed through a NetworkId once the wallet selector is already initialized

Additional context

austinabell avatar Sep 08 '22 17:09 austinabell

Currently in the Wallet Selector when we call setupWalletSelector the network can be set in two ways:

One by providing only a network id either testnet or mainnet by doing this we check internally for the presets for each of these 2 networks depending on which one has been provided.

So in this method here https://github.com/near/wallet-selector/blob/main/packages/core/src/lib/options.ts#L28 if the networkId has been provided as a string of testnet or mainnet we return the presets as far as we know these are the only official Networks we used to have betanet too but we were asked to remove it.

const _selector = await setupWalletSelector({
  network: "testnet"
  modules: [],
});

But a user can provide their own configurations as:

const _selector = await setupWalletSelector({
  network: {
    networkId: "customnet",
    nodeUrl: "https://rpc.custom-rpc.com",
    helperUrl: "https://helper.custom-helper.com",
    explorerUrl: "https://custom-explorer.com",
    indexerUrl: "https://api.custom-indexer.com",
  },
  modules: [],
});

This is documented here:

https://github.com/near/wallet-selector/tree/main/packages/core#options

At the moment it's not possible to change the network after Wallet Selector has been initialized. Network is not a "global state" in wallet selector we resolve it during setup based on what was passed to the network key and pass it to the modules(wallets) as with the options param.

What would happen when you are signed in with a wallet for example in testnet network with guest-book.testnet and suddenly change network to mainnet or a custom one?

kujtimprenkuSQA avatar Sep 13 '22 11:09 kujtimprenkuSQA

Hi @austinabell , can you share more info on the use case regarding this or possibly answer the question above, so we can leave this issue open? Thanks!

AmmarHumackicSQA avatar Oct 11 '22 13:10 AmmarHumackicSQA

Hey, @austinabell I will close this issue for now, but we can reopen it if necessary. Thanks!

AmmarHumackicSQA avatar Oct 20 '22 10:10 AmmarHumackicSQA

Sorry for the slow response!

What would happen when you are signed in with a wallet for example in testnet network with guest-book.testnet and suddenly change network to mainnet or a custom one?

Ideally, the in-memory data will be kept, would it be possible to keep a mapping of networkId to what data is currently kept for wallet sessions. Seems like something that could just be a method on wallet implementations and they can handle it however they want.

Basically, the ability to not be able to update the selector at runtime and there being a lot of hidden side-effects that are tough to reason about feels bad.

Probably best to loop in @calebjacob or @charleslavon as they have much more up to date context. It's been a while since I've had to interact with these

austinabell avatar Oct 20 '22 18:10 austinabell

+1 - the ability to swap between testnet/mainnet as needed is pretty key within our app.

It seems like rather than providing options to the wallet modules as a getter rather than caching the options provided in WalletBehaviorFactory is a promising approach - so that the options can be retrieved from the getter on an "as needed" basis (when signAndSend'ing).

jessupjn avatar Oct 27 '22 13:10 jessupjn

Then the options can be updated via selector.options.network = newNetwork and picked up by the wallets when needed

jessupjn avatar Oct 27 '22 13:10 jessupjn

+1 - the ability to swap between testnet/mainnet as needed is pretty key within our app.

@jessupjn I would like to know how did you handle switching networks in your app before using Wallet Selector?

Then the options can be updated via selector.options.network = newNetwork and picked up by the wallets when needed

As mentioned above at the moment there is no way to "properly" switch the network in Wallet Selector.

Just overriding the network selector.options.network is not enough because there's no event being emitted by doing so.

  • The provider service we pass to the modules would not be updated this way.
  • The wallet connection when you first signed in would not be updated.
  • The network on the wallet will not be updated to the one you're changing in the wallet selector.

kujtimprenkuSQA avatar Oct 31 '22 10:10 kujtimprenkuSQA

Amir Saran commented:

Currently this issue is blocked by the injected wallet implementation. There is no way to tell the injected wallets that the dApp has changed networks. This will cause a synching problem since the wallet and the dApp are using different active networks. In the case of Sender wallet if the user tries to connect on testnet network but the wallet network is set to mainnet it will not work.

This can be done with these two options:

  1. Provide a network parameter when signing in which will tell the wallet what network the user intends to use.
  2. Injected wallets create a function for changing the network from the dApp side.

These options will have checks because the user may not have a connected account with a particular network in the wallet.

These changes should also be done in the Injected Wallet NEP.

exalate-issue-sync[bot] avatar Dec 14 '22 12:12 exalate-issue-sync[bot]

Daryl Collins commented:

Is there any problem with Austin’s proposed solution to this?

  • This could possibly just be done by replacing the wallet selector instance, but unsure if there are unintended side effects from doing this. If this is the intended flow, feel free to close this issue

Some of the reasons that this is currently driven from the dapp scope are:

  1. Note that it is unlikely-to-impossible for the exact same account to exist on both testnet and mainnet as testnet seed phrases hash differently than mainnet ones do for deriving implicit account names
  2. There is a different top-level-account that is responsible for named account creation on testnet (.testnet) than on mainnet (.near).
  3. Also, most dapps are deployed in such a way that an instance of the dapp is always tied to a specific network ID, with an assumption that they are running in one specific network. Those apps will have no awareness of how to react to a network change, and would be broken if the wallet used accounts from a different chain than they are built to communicate with.

exalate-issue-sync[bot] avatar Jan 17 '23 18:01 exalate-issue-sync[bot]

Amir Saran commented:

Regarding Daryl Collins comment:

The initial implementation of network switching was using multiple wallet-selector instances. After talking with the Pagoda team we came to the conclusion that we want to have the network switching done inside the wallet-selector modal.

Having it inside the wallet-selector modal means that we need to add a networkChanged event so that the dApp can update its view after network switching. This is not an ideal and intuitive solution since the developer needs to add event listeners to all wallet-selector instances with same functionality.

exalate-issue-sync[bot] avatar Feb 02 '23 11:02 exalate-issue-sync[bot]

Switching the network dynamically has proven to be challenging for several reasons:

  • There would be no synchronization between the dApp's and the wallet's network.
  • The mentioned steps on the comment above https://github.com/near/wallet-selector/issues/448#issuecomment-1385830055
  • The related work has been stopped https://github.com/near/wallet-selector/pull/552 because it makes the use of wallet-selector way too complex by managing multiple instances.

We are closing this issue for now based on the above reasons.

kujtimprenkuSQA avatar Dec 11 '23 08:12 kujtimprenkuSQA