From 1243a02b4038c35e89839c578edaa2e16dd6e0fb Mon Sep 17 00:00:00 2001 From: Gergo Nagy Date: Wed, 23 Mar 2022 18:21:20 +0100 Subject: [PATCH] make default wallet injection more explicit The initial flow `load window provider > message background script for default config > if set to default overwrite politely the window.ethereum` caused problems because window.ethereum was not present or was occupied by others libs when dApps started looking for it. Changed the strategy to overwrite window.ethereum by default and reset if tally is set not to be the default wallet explicitly by the user. Another solution would be for the background script to start the communication on connect and act accordingly, but that also has the chance for the race condition to happen. I chose the slightly more "selfish" but more robust approach here. --- provider-bridge/index.ts | 13 ++----------- src/window-provider.ts | 12 +++++++++--- window-provider/index.ts | 22 ++++++++-------------- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/provider-bridge/index.ts b/provider-bridge/index.ts index 926e673752..825fa838ec 100644 --- a/provider-bridge/index.ts +++ b/provider-bridge/index.ts @@ -11,17 +11,13 @@ const INJECTED_WINDOW_PROVIDER_SOURCE = "@@@WINDOW_PROVIDER@@@" export function connectProviderBridge(): void { const port = browser.runtime.connect({ name: EXTERNAL_PORT_NAME }) - // TODO: internal provider state - - // TODO: grab initial provider state on load - window.addEventListener("message", (event) => { if ( event.origin === windowOriginAtLoadTime && // we want to recieve msgs only from the in-page script event.source === window && // we want to recieve msgs only from the in-page script event.data.target === PROVIDER_BRIDGE_TARGET ) { - // to demonstrate how it works it was necessary. Will remove later + // TODO: replace with better logging before v1. Now it's invaluable in debugging. // eslint-disable-next-line no-console console.log( `%c content: inpage > background: ${JSON.stringify(event.data)}`, @@ -33,7 +29,7 @@ export function connectProviderBridge(): void { }) port.onMessage.addListener((data) => { - // to demonstrate how it works it was necessary. Will remove later + // TODO: replace with better logging before v1. Now it's invaluable in debugging. // eslint-disable-next-line no-console console.log( `%c content: background > inpage: ${JSON.stringify(data)}`, @@ -59,13 +55,8 @@ export function injectTallyWindowProvider(): void { // this makes the script loading blocking which is good for us // bc we want to load before anybody has a chance to temper w/ the window obj scriptTag.setAttribute("async", "false") - - // need to decode the encodes string so it can be used as a source code - // in non optimised builds the source is a multi line string > `` was used - // but ${ needs to be escaped separatly otherwise it breaks the `` scriptTag.textContent = INJECTED_WINDOW_PROVIDER_SOURCE - // TODO: fill in the provider state for window-provider container.insertBefore(scriptTag, container.children[0]) } catch (e) { throw new Error( diff --git a/src/window-provider.ts b/src/window-provider.ts index 930780c152..5bdef867df 100644 --- a/src/window-provider.ts +++ b/src/window-provider.ts @@ -16,6 +16,12 @@ window.tally = new TallyWindowProvider({ window.removeEventListener("message", fn, false), origin: window.location.origin, }) -// If there are no other wallets around, we set the rules! -// This is needed for websites that immediately check the presence of `window.ethereum` -window.ethereum ??= window.tally + +// let's be a bit more pushy but stable: if window.ethereum is occupied let's make a backup +// so we can reset to the original if tally is NOT set to be the default wallet +if (window.ethereum) { + window.oldEthereum = window.ethereum +} + +// and set window.ethereum by default, so it's available from the very beginning +window.ethereum = window.tally diff --git a/window-provider/index.ts b/window-provider/index.ts index 80a8e8f0e1..753f9247a8 100644 --- a/window-provider/index.ts +++ b/window-provider/index.ts @@ -57,22 +57,16 @@ export default class TallyWindowProvider extends EventEmitter { } if (isTallyConfigPayload(result)) { - if (result.defaultWallet) { - // let's set Tally as a default wallet - // and bkp any object that maybe using window.ethereum - if (window.ethereum) { - window.oldEthereum = window.ethereum + if (!result.defaultWallet) { + // if tally is NOT set to be default wallet + // AND window.ethereum was taken + if (window.oldEthereum) { + // then let's reset window.ethereum to the original value + window.ethereum = window.oldEthereum } - window.ethereum = window.tally - } else if (window.oldEthereum) { - // let's remove tally as a default wallet - // and put back whatever it was there before us - window.ethereum = window.oldEthereum - } else if (window.ethereum?.isTally) { - // we were told not to be a default wallet anymore - // so in case if we have `window.ethereum` just remove ourselves - window.ethereum = undefined + // NOTE: we do not remove the TallyWindowProvider from window.ethereum + // if there is nothing else that want's to use it. } } else if (isTallyAccountPayload(result)) { this.handleAddressChange.bind(this)(result.address)