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

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

Merged
merged 6 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions background/services/provider-bridge/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,25 @@ export default class ProviderBridgeService extends BaseService<Events> {

this.addNetworkRequestId += 1

const [rawChainData, address, siteTitle, favicon] = params
const validatedData = validateAddEthereumChainParameter(
rawChainData as AddEthereumChainParameter
)

const supportedNetwork =
await this.internalEthereumProviderService.getTrackedNetworkByChainId(
validatedData.chainId
)

if (supportedNetwork) {
// If the network is already added - just switch to it.
return await this.internalEthereumProviderService.routeSafeRPCRequest(
method,
params,
origin
)
}

const window = await showExtensionPopup(
AllowedQueryParamPage.addNewChain,
{ requestId: id.toString() }
Expand All @@ -543,11 +562,6 @@ export default class ProviderBridgeService extends BaseService<Events> {
}
})

const [rawChainData, address, siteTitle, favicon] = params
const validatedData = validateAddEthereumChainParameter(
rawChainData as AddEthereumChainParameter
)

const userConfirmation = new Promise<void>((resolve, reject) => {
this.#pendingAddNetworkRequests[id] = {
resolve,
Expand Down
78 changes: 69 additions & 9 deletions background/services/provider-bridge/tests/index.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,17 @@ import {
} from "@tallyho/provider-bridge-shared"
import sinon from "sinon"
import browser from "webextension-polyfill"
// FIXME Pull the appropriate dependency to this package.json so we're not
// FIXME relying on weird cross-package dependencies.
// eslint-disable-next-line import/no-extraneous-dependencies
import { waitFor } from "@testing-library/dom"
import * as popupUtils from "../show-popup"
import * as featureFlags from "../../../features"
import { wait } from "../../../lib/utils"
import { createProviderBridgeService } from "../../../tests/factories"
import { AddEthereumChainParameter } from "../../internal-ethereum-provider"
import ProviderBridgeService from "../index"
import { validateAddEthereumChainParameter } from "../utils"
import { ETHEREUM } from "../../../constants"

const WINDOW = {
focused: true,
Expand Down Expand Up @@ -133,6 +136,7 @@ describe("ProviderBridgeService", () => {
const { enablingPermission } = BASE_DATA

jest.spyOn(featureFlags, "isEnabled").mockImplementation(() => true)
const showPopupSpy = jest.spyOn(popupUtils, "default")

const request = providerBridgeService.routeContentScriptRPCRequest(
{
Expand All @@ -143,21 +147,24 @@ describe("ProviderBridgeService", () => {
enablingPermission.origin
)

// eslint-disable-next-line @typescript-eslint/dot-notation
const IEP = providerBridgeService["internalEthereumProviderService"]
// @ts-expect-error private access to reference the service
const IEP = providerBridgeService.internalEthereumProviderService
const spy = jest.spyOn(IEP, "routeSafeRPCRequest")

await wait(0) // wait next tick to setup popup
// wait until popup is set up
await waitFor(() => expect(showPopupSpy).toHaveBeenCalled())

const validatedPayload = validateAddEthereumChainParameter(
params[0] as AddEthereumChainParameter
)

expect(providerBridgeService.getNewCustomRPCDetails("0")).toEqual({
...validatedPayload,
favicon: "favicon.png",
siteTitle: "some site",
})
await waitFor(() =>
expect(providerBridgeService.getNewCustomRPCDetails("0")).toEqual({
...validatedPayload,
favicon: "favicon.png",
siteTitle: "some site",
})
)

expect(spy).not.toHaveBeenCalled()
providerBridgeService.handleAddNetworkRequest("0", true)
Expand All @@ -172,5 +179,58 @@ describe("ProviderBridgeService", () => {

await expect(request).resolves.toEqual(null) // resolves without errors
})

it("should skip user confirmation if the network already exists", async () => {
const params = [
{
chainId: "1",
chainName: "Ethereum Mainnet",
nativeCurrency: { name: "Ether", symbol: "ETH", decimals: 18 },
iconUrl: undefined,
rpcUrls: ["booyan"],
blockExplorerUrls: ["https://etherscan.io"],
},
"0xd8da6bf26964af9d7eed9e03e53415d37aa96045",
"some site",
"favicon.png",
]

const { enablingPermission } = BASE_DATA

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

Choose a reason for hiding this comment

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

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

const showPopupSpy = jest.spyOn(popupUtils, "default")

const internalEthereumProvider =
// @ts-expect-error private access to reference the service
providerBridgeService.internalEthereumProviderService
jest
.spyOn(internalEthereumProvider, "getTrackedNetworkByChainId")
.mockImplementation(() => Promise.resolve(ETHEREUM))
const internalEthereumProviderSpy = jest.spyOn(
internalEthereumProvider,
"routeSafeRPCRequest"
)

const request = providerBridgeService.routeContentScriptRPCRequest(
{
...enablingPermission,
},
"wallet_addEthereumChain",
params,
enablingPermission.origin
)

await waitFor(() =>
expect(internalEthereumProviderSpy).toHaveBeenCalledWith(
"wallet_addEthereumChain",
params,
BASE_DATA.origin
)
)

// expect no popup
expect(showPopupSpy).not.toHaveBeenCalled()
await expect(request).resolves.toEqual(null) // resolves without errors
})
})
})