Switch to a given network if adding a network that is already added. #3154

0xDaedalus · 2023-03-14T13:08:54Z

Since a given dapp does not necessarily have a sense of what networks are supported within the wallet (and since some dapps rely exclusively on wallet_addEthereumChain) we should support the functionality of switching to a network when receiving a wallet_addEthereumChain message with a chainID that we already support.

To Test

Navigate to chainlist.org
Add the Ethereum Network to wallet
You should not see a popup
Add the Avalanche Network to wallet
You should not see a popup
Open the wallet popover from the Taho icon—Avalanche Network should be selected.
In Application/Storage/IndexedDB/internal-ethereum-provider/currentNetwork - you should see the network of https://chainlist.org listed as Avalanche

Latest build: extension-builds-3154 (as of Mon, 10 Jul 2023 21:23:08 GMT).

Since a given dapp does not necessarily have a sense of what networks are supported within the wallet (and since some dapps rely exclusively on wallet_addEthereumChain) we should support the functionality of switching to a network when receiving a wallet_addEthereumChain message with a chainID that we already support.

0xDaedalus · 2023-03-14T15:55:52Z

Converting back to draft pending some product discussion

hyphenized · 2023-03-15T04:43:51Z

I know this is a draft PR but this should fix the issue with the flaky test, the wait for next tick assertion broke because it relied too much on the structure of the code (and the # of macro tasks).
https://gist.github.com/hyphenized/4ce1e44767c9f320d9470e008242b0cd

The new check for whether a new custom network already exists changes timing that was expected to be static. The relevant assertions are now wrapped in retries that will wait a certain amount of time for the given assertion to pass.

This fix is the result of adding a test to confirm a popup is not shown. Notably, the behavior of switching the network silently requires extra vetting; however, this commit implements it as that was the branch's intent.

hyphenized

Works well, can we remove the "Network already added screen"?

hyphenized · 2023-07-05T03:18:59Z

background/services/provider-bridge/tests/index.unit.test.ts

+
+      const { enablingPermission } = BASE_DATA
+
+      jest.spyOn(featureFlags, "isEnabled").mockImplementation(() => true)


I don't think this is causing any issues but we don't need to stub feature flags here anymore. n/b

Shadowfiend · 2023-07-08T04:58:52Z

That screen will only appear once #3481 lands if the user adds an existing network, right? In this flow it never appears because we skip displaying the popup altogether.

With that said, are there any remaining blockers here or are you good to merge?

hyphenized · 2023-07-10T20:31:56Z

That screen will only appear once #3481 lands if the user adds an existing network, right? In this flow it never appears because we skip displaying the popup altogether.

With that said, are there any remaining blockers here or are you good to merge?

This screen is appearing as of the latest release in main 👀

Shadowfiend · 2023-07-10T21:01:12Z

Yes, this PR changes that :) The return for this case is moved above the showExtensionPopup call in https://github.com/tahowallet/extension/pull/3154/files#diff-cebbcecfbd654e43999eb66e8653ca07d814ec9972cffb29a2791bb1b3730740 .

EDIT to say: what I meant in my original comment is that, after this PR, that screen will not show because the popup is not displayed. It will show again when the work from #3481 lands, because at that point there will be a flow for adding a network that will happen in an existing popup. We can fix the language it displays at that time.

hyphenized · 2023-07-10T21:11:21Z

Ah I see, I thought we were unintentionally discarding a feature 😅

hyphenized

👑 👑 👑

@jagodarybacka

## What's Changed * Add private key onboarding flow by @jagodarybacka in #3119 * Private key JSON import by @jagodarybacka in #3177 * Allow export of private keys and mnemonics by @jagodarybacka in #3248 * Export private key form by @jagodarybacka in #3255 * Unlock screen for the account backup by @kkosiorowska in #3257 * Show mnemonic menu by @jagodarybacka in #3259 * Fix background blur issue by @jagodarybacka in #3265 * Account backup UI fixes by @jagodarybacka in #3270 * Fix unhiding removed accounts by @jagodarybacka in #3282 * New error for incorrectly decrypted JSON file by @jagodarybacka in #3293 * Export private keys from HD wallet addresses by @jagodarybacka in #3253 * Refactor keyring redux slice to remove `importing` field by @jagodarybacka in #3309 * 📚 Accounts backup by @kkosiorowska in #3252 * Catch Enter keypress on Unlock screen by @jagodarybacka in #3355 * Rename `keyring` to `internal signer` and other improvements by @jagodarybacka in #3331 * 🗝 QA - Accounts backup and private key import by @jagodarybacka in #3266 * Remove private key signers if they are replaced by accounts from HD wallet by @jagodarybacka in #3377 * RFB 4: One-Off Keyring Design by @Shadowfiend in #3372 * Copy to clipboard warning by @kkosiorowska in #3488 * Allow setting custom auto-lock timer by @hyphenized in #3477 * Use Argon2 for encrypted vaults by @jagodarybacka in #3502 * 👑 Private keys import and accounts backup by @jagodarybacka in #3089 * Untrusted assets should not block the addition of custom tokens by @kkosiorowska in #3491 * Flip updated dApp connections flag by @Shadowfiend in #3492 * v0.41.0 by @Shadowfiend in #3531 * Switch to a given network if adding a network that is already added. by @0xDaedalus in #3154 * Remove waiting for Loading Doggo component in E2E tests by @jagodarybacka in #3541 * Squeeze content to better fit on Swaps page by @jagodarybacka in #3542 * Refactor of terms for verified/unverified assets by @kkosiorowska in #3528 * Fix ChainList styling by @fulldecent in #3547 * Update release checklist by @jagodarybacka in #3548 * Fix custom asset price fetching by @hyphenized in #3508 * Sticky Defaults: Make Taho-as-default replace MetaMask in almost all cases by @Shadowfiend in #3546 ## New Contributors * @fulldecent made their first contribution in #3547 **Full Changelog**: v0.41.0...v0.42.0 Latest build: [extension-builds-3549](https://github.com/tahowallet/extension/suites/14268975651/artifacts/801826435) (as of Thu, 13 Jul 2023 09:51:56 GMT).

0xDaedalus marked this pull request as draft March 14, 2023 15:55

0xDaedalus added this to the Support bare RPC providers milestone Mar 15, 2023

Merge branch 'main' into switch-to-added-network

6c2acec

Shadowfiend marked this pull request as ready for review June 16, 2023 18:35

Shadowfiend added 3 commits July 3, 2023 21:20

Merge remote-tracking branch 'origin/main' into switch-to-added-network

5469aec

Fix concurrency error in provider bridge test

5e8bc26

The new check for whether a new custom network already exists changes timing that was expected to be static. The relevant assertions are now wrapped in retries that will wait a certain amount of time for the given assertion to pass.

Fix switching networks when adding an existing network

af041d6

This fix is the result of adding a test to confirm a popup is not shown. Notably, the behavior of switching the network silently requires extra vetting; however, this commit implements it as that was the branch's intent.

Shadowfiend requested a review from a team July 5, 2023 01:02

hyphenized reviewed Jul 5, 2023

View reviewed changes

hyphenized approved these changes Jul 10, 2023

View reviewed changes

hyphenized enabled auto-merge July 10, 2023 21:12

Merge branch 'main' into switch-to-added-network

0ebefb5

hyphenized merged commit 7eaabfb into main Jul 10, 2023
4 of 5 checks passed

hyphenized deleted the switch-to-added-network branch July 10, 2023 21:15

kkosiorowska mentioned this pull request Jul 13, 2023

v0.42.0 #3549

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to a given network if adding a network that is already added. #3154

Switch to a given network if adding a network that is already added. #3154

0xDaedalus commented Mar 14, 2023 •

edited by github-actions bot

0xDaedalus commented Mar 14, 2023

hyphenized commented Mar 15, 2023

hyphenized left a comment

hyphenized Jul 5, 2023

Shadowfiend commented Jul 8, 2023

hyphenized commented Jul 10, 2023

Shadowfiend commented Jul 10, 2023 •

edited

hyphenized commented Jul 10, 2023

hyphenized left a comment


		const { enablingPermission } = BASE_DATA

		jest.spyOn(featureFlags, "isEnabled").mockImplementation(() => true)

Switch to a given network if adding a network that is already added. #3154

Switch to a given network if adding a network that is already added. #3154

Conversation

0xDaedalus commented Mar 14, 2023 • edited by github-actions bot

To Test

0xDaedalus commented Mar 14, 2023

hyphenized commented Mar 15, 2023

hyphenized left a comment

Choose a reason for hiding this comment

hyphenized Jul 5, 2023

Choose a reason for hiding this comment

Shadowfiend commented Jul 8, 2023

hyphenized commented Jul 10, 2023

Shadowfiend commented Jul 10, 2023 • edited

hyphenized commented Jul 10, 2023

hyphenized left a comment

Choose a reason for hiding this comment

0xDaedalus commented Mar 14, 2023 •

edited by github-actions bot

Shadowfiend commented Jul 10, 2023 •

edited