Skip to content
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.

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

Repeater Connection: Forward requests to new default when default is switched off during dApp connection flow #3462

Merged
merged 9 commits into from
Jun 15, 2023
6 changes: 4 additions & 2 deletions background/services/provider-bridge/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ export default class ProviderBridgeService extends BaseService<Events> {
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
Expand All @@ -411,7 +411,9 @@ export default class ProviderBridgeService extends BaseService<Events> {
delete this.#pendingPermissionsRequests[permission.origin]
}

this.notifyContentScriptsAboutAddressChange()
if (deleted > 0) {
this.notifyContentScriptsAboutAddressChange()
}
}

async revokePermissionsForAddress(revokeAddress: string): Promise<void> {
Expand Down
1 change: 1 addition & 0 deletions env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ interface Window {
shouldSetTally: boolean,
shouldReload?: boolean
) => void
routeToNewDefault: (request: unknown) => Promise<unknown>
getProviderInfo: (
provider: WalletProvider
) => WalletProvider["providerInfo"]
Expand Down
20 changes: 19 additions & 1 deletion src/window-provider.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
RequestArgument,
WindowListener,
WindowRequestEvent,
} from "@tallyho/provider-bridge-shared"
Expand Down Expand Up @@ -65,6 +66,9 @@ if (!window.walletRouter) {
}, 1000)
}
},
routeToNewDefault(request: Required<RequestArgument>): Promise<unknown> {
return this.currentProvider.request(request)
},
getProviderInfo(provider: WalletProvider) {
return (
provider.providerInfo || {
Expand Down Expand Up @@ -112,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 (
Expand All @@ -138,7 +146,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
Expand Down
17 changes: 16 additions & 1 deletion ui/pages/DAppConnect/DAppConnectPage.tsx
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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"
import DAppConnectionInfoBar from "../../components/DAppConnection/DAppConnectionInfoBar"

type DAppConnectPageProps = {
Expand All @@ -31,6 +33,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)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is causing an issue when Taho is the only wallet available. On https://pancakeswap.finance/ when Taho is not the default, connecting with the injected option causes the popup to close immediately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Incidentally this shouldn't trigger in that scenario—it should really only trigger when the user actively toggles it off, i.e. on transition. Let me noodle on this a little.

If this isn't causing that behavior with the feature flag off, let's not let it block the merge since there's a follow-up in the mix.

}
}, [isDefaultWallet])

return (
<>
{isEnabled(FeatureFlags.ENABLE_UPDATED_DAPP_CONNECTIONS) && (
Expand Down
32 changes: 31 additions & 1 deletion window-provider/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,16 @@ 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 (
process.env.ENABLE_UPDATED_DAPP_CONNECTIONS === "true" ||
impersonateMetamaskWhitelist.some((host) =>
currentHost.includes(host)
)
Expand All @@ -165,6 +169,32 @@ 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(([, { 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(resolve)
.catch(reject)
})
}

if (result.chainId && result.chainId !== this.chainId) {
this.handleChainIdChange(result.chainId)
}
Expand Down