Skip to content

Commit

Permalink
make default wallet injection more explicit
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Gergo Nagy committed Mar 23, 2022
1 parent 938ed8c commit 1243a02
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 28 deletions.
13 changes: 2 additions & 11 deletions provider-bridge/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)}`,
Expand All @@ -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)}`,
Expand All @@ -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(
Expand Down
12 changes: 9 additions & 3 deletions src/window-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
22 changes: 8 additions & 14 deletions window-provider/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 1243a02

Please sign in to comment.