From cde6117a46d8ae2bc30c5b3d44dedb739bc8a5b4 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Thu, 8 Jun 2023 16:41:09 -0400 Subject: [PATCH 1/8] Don't notify pages unless necessary on permission revoke This largely doesn't matter, but we were pushing a notification to all pages when a single permission was revoked, even if that permission had not been granted in the first place. This is a tiny step in improving the permission revocation process. --- background/services/provider-bridge/index.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/background/services/provider-bridge/index.ts b/background/services/provider-bridge/index.ts index b2881729e2..61a0e467cc 100644 --- a/background/services/provider-bridge/index.ts +++ b/background/services/provider-bridge/index.ts @@ -400,7 +400,7 @@ export default class ProviderBridgeService extends BaseService { const { address } = await this.preferenceService.getSelectedAccount() // TODO make this multi-network friendly - await this.db.deletePermission( + const deleted = await this.db.deletePermission( permission.origin, address, permission.chainID @@ -411,7 +411,9 @@ export default class ProviderBridgeService extends BaseService { delete this.#pendingPermissionsRequests[permission.origin] } - this.notifyContentScriptsAboutAddressChange() + if (deleted > 0) { + this.notifyContentScriptsAboutAddressChange() + } } async revokePermissionsForAddress(revokeAddress: string): Promise { From f55574ba5432485fe3f4c54500bdb435465d2707 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Thu, 8 Jun 2023 16:43:12 -0400 Subject: [PATCH 2/8] Add `routeToNewDefault` to the wallet router When called, this pushes an RPC request to the current provider, which has ostensibly been previously updated. It is meant to be used to re-forward to MetaMask or another wallet when Taho disables its default status during an interaction. --- env.d.ts | 1 + src/window-provider.ts | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/env.d.ts b/env.d.ts index 43184b1d9a..81310c9b2c 100644 --- a/env.d.ts +++ b/env.d.ts @@ -55,6 +55,7 @@ interface Window { shouldSetTally: boolean, shouldReload?: boolean ) => void + routeToNewDefault: (request: unknown) => Promise getProviderInfo: ( provider: WalletProvider ) => WalletProvider["providerInfo"] diff --git a/src/window-provider.ts b/src/window-provider.ts index 106032404d..f671ad1459 100644 --- a/src/window-provider.ts +++ b/src/window-provider.ts @@ -1,4 +1,5 @@ import { + RequestArgument, WindowListener, WindowRequestEvent, } from "@tallyho/provider-bridge-shared" @@ -65,6 +66,9 @@ if (!window.walletRouter) { }, 1000) } }, + routeToNewDefault(request: Required): Promise { + return this.currentProvider.request(request) + }, getProviderInfo(provider: WalletProvider) { return ( provider.providerInfo || { From 70372dd8b75eeebd76330869af2567230fe909c9 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Thu, 8 Jun 2023 16:47:49 -0400 Subject: [PATCH 3/8] Reroute to new default when default wallet toggle moves off of Taho In addition to re-routing all in-flight requests to the new wallet when the Taho-as-default toggle is flipped, we also no longer reload the page for any dApps. --- window-provider/index.ts | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/window-provider/index.ts b/window-provider/index.ts index 6e6bbe65ac..6b0b34f160 100644 --- a/window-provider/index.ts +++ b/window-provider/index.ts @@ -141,9 +141,12 @@ export default class TallyWindowProvider extends EventEmitter { } if (isTallyConfigPayload(result)) { + const wasTallySetAsDefault = this.tallySetAsDefault + window.walletRouter?.shouldSetTallyForCurrentProvider( result.defaultWallet, - result.shouldReload + result.shouldReload && + process.env.ENABLE_UPDATED_DAPP_CONNECTIONS !== "true" ) const currentHost = window.location.host if ( @@ -165,6 +168,34 @@ export default class TallyWindowProvider extends EventEmitter { this.tallySetAsDefault = result.defaultWallet } + + // When the default state flips, reroute any unresolved requests to the + // new default provider. + if ( + process.env.ENABLE_UPDATED_DAPP_CONNECTIONS === "true" && + wasTallySetAsDefault && + !this.tallySetAsDefault + ) { + const existingRequests = [...this.requestResolvers.entries()] + this.requestResolvers.clear() + + existingRequests + // Make sure to re-route the requests in the order they were + // received. + .sort(([id], [id2]) => Number(BigInt(id2) - BigInt(id))) + .forEach(([id, { sendData, reject, resolve }]) => { + window.walletRouter + ?.routeToNewDefault(sendData.request) + // On success or error, call the original reject/resolve + // functions to notify the requestor of the new wallet's + // response. + .then((...a) => resolve(...a)) + .catch((...b) => reject(...b)) + + this.requestResolvers.delete(id) + }) + } + if (result.chainId && result.chainId !== this.chainId) { this.handleChainIdChange(result.chainId) } From 2df835609f2763f18113224d67c003f07bda730c Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Thu, 8 Jun 2023 16:52:47 -0400 Subject: [PATCH 4/8] Always route new requests to the current provider The way that the provider proxy was set up, changing the current provider would not route new requests to that provider, instead requiring the dApp to reconnect. To allow transparent forwarding to the new default when the connection switches, the proxy now always forwards to the current provider (when available). --- src/window-provider.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/window-provider.ts b/src/window-provider.ts index f671ad1459..a8ebdca629 100644 --- a/src/window-provider.ts +++ b/src/window-provider.ts @@ -142,7 +142,17 @@ Object.defineProperty(window, "ethereum", { return window.walletRouter[prop] } - return Reflect.get(target, prop, receiver) + return Reflect.get( + // Always proxy to the current provider, even if it has changed. This + // allows changes in the current provider, particularly when the user + // changes their default wallet, to take effect immediately. Combined + // with walletRouter.routeToNewDefault, this allows Taho to effect a + // change in provider without a page reload or even a second attempt + // at connecting. + window.walletRouter?.currentProvider ?? target, + prop, + receiver + ) }, }) cachedCurrentProvider = window.walletRouter.currentProvider From b3889c6c4011b26b18f6988bfc793f091b6849e3 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Thu, 8 Jun 2023 16:53:33 -0400 Subject: [PATCH 5/8] Fix window.ethereum when currentProvider is unset When currentProvider is unset on the router, return `undefined` for `window.ethereum` as well. --- src/window-provider.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/window-provider.ts b/src/window-provider.ts index a8ebdca629..1eb7e80a37 100644 --- a/src/window-provider.ts +++ b/src/window-provider.ts @@ -116,6 +116,10 @@ Object.defineProperty(window, "ethereum", { return cachedWindowEthereumProxy } + if (window.walletRouter.currentProvider === undefined) { + return undefined + } + cachedWindowEthereumProxy = new Proxy(window.walletRouter.currentProvider, { get(target, prop, receiver) { if ( From 89fc1ad8113c5af3c44d8daaff1eae1edb93701f Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Thu, 8 Jun 2023 16:56:17 -0400 Subject: [PATCH 6/8] Close dApp connection page when default wallet changes Only when updated dApp connections are enabled, the dApp connection popup auto-closes (after a 300ms delay to let the user see the toggle animate). The expectation is that the new default wallet will pop up immediately as the window provider replays the connection request from Taho to the new default. --- ui/pages/DAppConnect/DAppConnectPage.tsx | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/ui/pages/DAppConnect/DAppConnectPage.tsx b/ui/pages/DAppConnect/DAppConnectPage.tsx index 8da6155f8a..fdf0efde3e 100644 --- a/ui/pages/DAppConnect/DAppConnectPage.tsx +++ b/ui/pages/DAppConnect/DAppConnectPage.tsx @@ -1,4 +1,4 @@ -import React, { ReactElement } from "react" +import React, { ReactElement, useEffect } from "react" import { AccountTotal } from "@tallyho/tally-background/redux-slices/selectors" import { PermissionRequest } from "@tallyho/provider-bridge-shared" import { useTranslation } from "react-i18next" @@ -7,10 +7,12 @@ import { isDisabled, isEnabled, } from "@tallyho/tally-background/features" +import { selectDefaultWallet } from "@tallyho/tally-background/redux-slices/ui" import SharedButton from "../../components/Shared/SharedButton" import SharedAccountItemSummary from "../../components/Shared/SharedAccountItemSummary" import RequestingDAppBlock from "./RequestingDApp" import SwitchWallet from "./SwitchWallet" +import { useBackgroundSelector } from "../../hooks" type DAppConnectPageProps = { permission: PermissionRequest @@ -30,6 +32,19 @@ export default function DAppConnectPage({ const { t } = useTranslation("translation", { keyPrefix: "dappConnection" }) const { title, origin, faviconUrl, accountAddress } = permission + const isDefaultWallet = useBackgroundSelector(selectDefaultWallet) + + useEffect(() => { + if ( + isEnabled(FeatureFlags.ENABLE_UPDATED_DAPP_CONNECTIONS) && + !isDefaultWallet + ) { + // When default wallet is toggled off, wait for the toggle animation to + // complete and close the window out. + setTimeout(() => window.close(), 300) + } + }, [isDefaultWallet]) + return ( <>
From 0da8205356a4f0f4813c305d327bfcc3643b65c6 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Thu, 8 Jun 2023 16:57:24 -0400 Subject: [PATCH 7/8] Impersonate MetaMask on all sites when set as default Now that the MetaMask fallback is more seamless, make it so that Taho behaves as MetaMask on all sites when it is set as default. Only when updated dApp connections are enabled. --- window-provider/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/window-provider/index.ts b/window-provider/index.ts index 6b0b34f160..2a30503ce9 100644 --- a/window-provider/index.ts +++ b/window-provider/index.ts @@ -150,6 +150,7 @@ export default class TallyWindowProvider extends EventEmitter { ) const currentHost = window.location.host if ( + process.env.ENABLE_UPDATED_DAPP_CONNECTIONS === "true" || impersonateMetamaskWhitelist.some((host) => currentHost.includes(host) ) From 117fed883489277e9a6ad93584475e3b99515aa1 Mon Sep 17 00:00:00 2001 From: Antonio Salazar Cardozo Date: Wed, 14 Jun 2023 22:05:43 -0400 Subject: [PATCH 8/8] Avoid double-cleaning requestResolvers in window provider reroute The resolvers were being cleared immediately, then being cleaned up again one by one during the rerouting. Let the initial clearing do the work. --- window-provider/index.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/window-provider/index.ts b/window-provider/index.ts index 2a30503ce9..76e4096e60 100644 --- a/window-provider/index.ts +++ b/window-provider/index.ts @@ -184,16 +184,14 @@ export default class TallyWindowProvider extends EventEmitter { // Make sure to re-route the requests in the order they were // received. .sort(([id], [id2]) => Number(BigInt(id2) - BigInt(id))) - .forEach(([id, { sendData, reject, resolve }]) => { + .forEach(([, { sendData, reject, resolve }]) => { window.walletRouter ?.routeToNewDefault(sendData.request) // On success or error, call the original reject/resolve // functions to notify the requestor of the new wallet's // response. - .then((...a) => resolve(...a)) - .catch((...b) => reject(...b)) - - this.requestResolvers.delete(id) + .then(resolve) + .catch(reject) }) }