Skip to content

Commit

Permalink
Fixed getChainIdDefault() race condition (#1648)
Browse files Browse the repository at this point in the history
  • Loading branch information
jribbink authored May 17, 2023
1 parent 434c2f4 commit 06655ae
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/polite-moons-study.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@onflow/fcl": patch
---

Fixed `setChainIdDefault()` race condition, instead run function on accessNode.api value changes
7 changes: 3 additions & 4 deletions packages/fcl/src/default-config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {config} from "@onflow/config"
import {setChainIdDefault} from "./utils/getChainId"
import {setChainIdDefault, watchForChainIdChanges} from "./utils"

const isServerSide = () => typeof window === "undefined"

Expand All @@ -14,9 +14,8 @@ config({
"fcl.storage.default": SESSION_STORAGE,
})

// this is an async function but we can't await bc it's run at top level.
// NOT guaranteed that flow.network.default is set after this call (or at startup)
setChainIdDefault()
// Set chain id default on access node change
watchForChainIdChanges()

export async function configLens(regex) {
return Object.fromEntries(
Expand Down
21 changes: 21 additions & 0 deletions packages/fcl/src/utils/chain-id-watcher.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import {config} from "@onflow/config"
import {setChainIdDefault} from "./get-chain-id"

/**
* @description
* Watches the config for changes to access node and updates the chain id accordingly
*
* @returns {Function} A function that unsubscribes the listener
*
*/
export function watchForChainIdChanges() {
return config.subscribe(
function configSubscriber(config) {
const nextAccessNode = config?.["accessNode.api"]
if (this.prevAccessNode !== nextAccessNode) {
setChainIdDefault()
}
this.prevAccessNode = nextAccessNode
}.bind({})
)
}
54 changes: 54 additions & 0 deletions packages/fcl/src/utils/chain-id-watcher.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import {watchForChainIdChanges} from "./chain-id-watcher"
import {config} from "@onflow/config"
import * as chainIdUtils from "./get-chain-id"

describe("chain-id-watcher", () => {
let unsubscribe

afterEach(() => {
jest.restoreAllMocks()
unsubscribe && unsubscribe()
})

test("flow.network.default is correctly set on first call", async () => {
await config.overload(
{"accessNode.api": "https://example.com"},
async () => {
// Mock the setChainIdDefault function
const spy = jest.spyOn(chainIdUtils, "setChainIdDefault")
spy.mockImplementation(() => {})

// Start watching for changes
unsubscribe = watchForChainIdChanges()

// Wait for microtask queue to flush
await new Promise(resolve => setTimeout(resolve, 0))

// Expect only one call at initial setup
expect(chainIdUtils.setChainIdDefault).toHaveBeenCalledTimes(1)
}
)
})

test("flow.network.default is correctly set when changed later", async () => {
await config.overload({}, async () => {
// Mock the setChainIdDefault function
const spy = jest.spyOn(chainIdUtils, "setChainIdDefault")
spy.mockImplementation(() => {})

// Start watching for changes
unsubscribe = watchForChainIdChanges()

// Wait for microtask queue to flush
await new Promise(resolve => setTimeout(resolve, 0))

config.put("accessNode.api", "https://example.com")

// Wait for microtask queue to flush
await new Promise(resolve => setTimeout(resolve, 0))

// Expect two calls since we changed the access node and there is an initial call
expect(chainIdUtils.setChainIdDefault).toHaveBeenCalledTimes(1)
})
})
})
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as sdk from "@onflow/sdk"
import {config} from "@onflow/config"
import {log} from "@onflow/util-logger"
import { invariant } from "@onflow/util-invariant"
import {invariant} from "@onflow/util-invariant"

async function getChainIdFromAccessNode() {
const response = await sdk.send([sdk.getNetworkParameters()]).then(sdk.decode)
Expand All @@ -10,9 +10,9 @@ async function getChainIdFromAccessNode() {

/**
* Sets the default chain id to the chain id of the access node
*
*
* @returns {string} The chain id of the access node
*
*
* @example
* // returns "testnet"
* setChainIdDefault()
Expand All @@ -26,10 +26,10 @@ export async function setChainIdDefault() {
/**
* @description
* Gets the chain ID if its set, otherwise gets the chain ID from the access node
*
*
* @returns {string} The chain ID of the access node
* @throws {Error} If the chain ID is not found
*
*
* @example
* // returns "testnet"
* getChainId()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {config} from "@onflow/config"
import assert from "assert"
import {getChainId} from "./getChainId"
import {getChainId} from "./get-chain-id"

describe("getChainId", () => {
it("getChainId assuming it's already in config", async () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/fcl/src/utils/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export {getChainId} from "./getChainId"
export {getChainId} from "./get-chain-id"
export {watchForChainIdChanges} from "./chain-id-watcher"

export function isAndroid() {
return (
Expand Down

0 comments on commit 06655ae

Please sign in to comment.