From 5bf99b1444ff6c16c400b47935b46f6d0a7ad210 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Mon, 3 Feb 2025 14:01:08 -0800 Subject: [PATCH 1/9] Fix race condition in AccountManager --- package-lock.json | 21 ---- package.json | 3 - packages/fcl-ethereum-provider/package.json | 2 - .../src/accounts/account-manager.test.ts | 73 +++++++---- .../src/accounts/account-manager.ts | 54 +++++++-- .../src/create-provider.ts | 2 +- .../src/events/event-dispatcher.test.ts | 57 +++++---- .../src/events/event-dispatcher.ts | 39 ++---- .../src/hash-utils.test.ts | 113 ------------------ .../fcl-ethereum-provider/src/hash-utils.ts | 41 ------- .../src/network/network-manager.test.ts | 12 +- .../src/network/network-manager.ts | 16 ++- .../src/rpc/handlers/eth-signtypeddata.ts | 30 ----- .../src/rpc/rpc-processor.ts | 29 +---- .../fcl-ethereum-provider/src/types/eth.ts | 25 ---- .../src/util/observable.ts | 69 ++++++----- 16 files changed, 199 insertions(+), 387 deletions(-) delete mode 100644 packages/fcl-ethereum-provider/src/hash-utils.test.ts delete mode 100644 packages/fcl-ethereum-provider/src/hash-utils.ts delete mode 100644 packages/fcl-ethereum-provider/src/rpc/handlers/eth-signtypeddata.ts diff --git a/package-lock.json b/package-lock.json index 30b629384..c638df32a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,9 +8,6 @@ "workspaces": [ "./packages/*" ], - "dependencies": { - "@noble/hashes": "^1.7.1" - }, "devDependencies": { "@babel/preset-typescript": "^7.25.7", "@changesets/changelog-github": "^0.4.8", @@ -2637,8 +2634,6 @@ }, "node_modules/@ethersproject/bytes": { "version": "5.7.0", - "resolved": "https://registry.npmjs.org/@ethersproject/bytes/-/bytes-5.7.0.tgz", - "integrity": "sha512-nsbxwgFXWh9NyYWo+U8atvmMsSdKJprTcICAkvbBffT75qDocbuggBU0SJiVK2MuTrp0q+xvLkTnGMPK1+uA9A==", "funding": [ { "type": "individual", @@ -2673,8 +2668,6 @@ }, "node_modules/@ethersproject/hash": { "version": "5.7.0", - "resolved": "https://registry.npmjs.org/@ethersproject/hash/-/hash-5.7.0.tgz", - "integrity": "sha512-qX5WrQfnah1EFnO5zJv1v46a8HW0+E5xuBBDTwMFZLuVTx0tbU2kkx15NqdjxecrLGatQN9FGQKpb1FKdHCt+g==", "funding": [ { "type": "individual", @@ -5775,18 +5768,6 @@ "@tybys/wasm-util": "^0.9.0" } }, - "node_modules/@noble/hashes": { - "version": "1.7.1", - "resolved": "https://registry.npmjs.org/@noble/hashes/-/hashes-1.7.1.tgz", - "integrity": "sha512-B8XBPsn4vT/KJAGqDzbwztd+6Yte3P4V7iafm24bxgDe/mlRuK6xmWPuCNrKt2vDafZ8MfJLlchDG/vYafQEjQ==", - "license": "MIT", - "engines": { - "node": "^14.21.3 || >=16" - }, - "funding": { - "url": "https://paulmillr.com/funding/" - } - }, "node_modules/@nodelib/fs.scandir": { "version": "2.1.5", "license": "MIT", @@ -29220,8 +29201,6 @@ "license": "Apache-2.0", "dependencies": { "@babel/runtime": "^7.25.7", - "@ethersproject/bytes": "^5.7.0", - "@ethersproject/hash": "^5.7.0", "@onflow/fcl": "1.13.4", "@onflow/rlp": "^1.2.3", "@walletconnect/jsonrpc-http-connection": "^1.0.8", diff --git a/package.json b/package.json index c3e04d794..4699e1f07 100644 --- a/package.json +++ b/package.json @@ -36,8 +36,5 @@ "@nx/nx-darwin-x64": "^17.3.2", "@nx/nx-linux-x64-gnu": "^17.3.2", "@nx/nx-win32-x64-msvc": "^17.3.2" - }, - "dependencies": { - "@noble/hashes": "^1.7.1" } } diff --git a/packages/fcl-ethereum-provider/package.json b/packages/fcl-ethereum-provider/package.json index 9907ac63d..7986990a3 100644 --- a/packages/fcl-ethereum-provider/package.json +++ b/packages/fcl-ethereum-provider/package.json @@ -37,8 +37,6 @@ }, "dependencies": { "@babel/runtime": "^7.25.7", - "@ethersproject/bytes": "^5.7.0", - "@ethersproject/hash": "^5.7.0", "@onflow/fcl": "1.13.4", "@onflow/rlp": "^1.2.3", "@walletconnect/jsonrpc-http-connection": "^1.0.8", diff --git a/packages/fcl-ethereum-provider/src/accounts/account-manager.test.ts b/packages/fcl-ethereum-provider/src/accounts/account-manager.test.ts index 42b87a035..d816f43fd 100644 --- a/packages/fcl-ethereum-provider/src/accounts/account-manager.test.ts +++ b/packages/fcl-ethereum-provider/src/accounts/account-manager.test.ts @@ -3,6 +3,8 @@ import {mockUser} from "../__mocks__/fcl" import * as fcl from "@onflow/fcl" import * as rlp from "@onflow/rlp" import {CurrentUser} from "@onflow/typedefs" +import {ChainIdStore, NetworkManager} from "../network/network-manager" +import {BehaviorSubject, Subject} from "../util/observable" jest.mock("@onflow/fcl", () => { const fcl = jest.requireActual("@onflow/fcl") @@ -24,35 +26,46 @@ const mockFcl = jest.mocked(fcl) const mockQuery = jest.mocked(fcl.query) describe("AccountManager", () => { + let networkManager: jest.Mocked + let $mockChainId: Subject + let userMock: ReturnType + beforeEach(() => { jest.clearAllMocks() + + $mockChainId = new BehaviorSubject({ + chainId: 747, + error: null, + isLoading: false, + }) + networkManager = { + $chainId: $mockChainId, + getChainId: jest.fn(), + } as any as jest.Mocked + userMock = mockUser() }) it("should initialize with null COA address", async () => { - const user = mockUser() - const accountManager = new AccountManager(user.mock) + const accountManager = new AccountManager(userMock.mock, networkManager) expect(await accountManager.getCOAAddress()).toBeNull() expect(await accountManager.getAccounts()).toEqual([]) }) it("should reset state when the user is not logged in", async () => { - const user = mockUser() - - const accountManager = new AccountManager(user.mock) + const accountManager = new AccountManager(userMock.mock, networkManager) expect(await accountManager.getCOAAddress()).toBeNull() expect(await accountManager.getAccounts()).toEqual([]) }) it("should fetch and update COA address when user logs in", async () => { - const user = mockUser() mockQuery.mockResolvedValue("0x123") - const accountManager = new AccountManager(user.mock) + const accountManager = new AccountManager(userMock.mock, networkManager) expect(await accountManager.getCOAAddress()).toBe(null) - user.set!({addr: "0x1"} as CurrentUser) + userMock.set!({addr: "0x1"} as CurrentUser) expect(await accountManager.getCOAAddress()).toBe("0x123") expect(await accountManager.getAccounts()).toEqual(["0x123"]) @@ -66,7 +79,7 @@ describe("AccountManager", () => { const user = mockUser() mockQuery.mockResolvedValue("0x123") - const accountManager = new AccountManager(user.mock) + const accountManager = new AccountManager(userMock.mock, networkManager) user.set!({addr: "0x1"} as CurrentUser) @@ -93,7 +106,7 @@ describe("AccountManager", () => { // 2nd fetch: immediate .mockResolvedValueOnce("0x456") - const accountManager = new AccountManager(user.mock) + const accountManager = new AccountManager(userMock.mock, networkManager) await user.set!({addr: "0x1"} as CurrentUser) await user.set!({addr: "0x2"} as CurrentUser) @@ -106,7 +119,7 @@ describe("AccountManager", () => { const user = mockUser() mockQuery.mockRejectedValueOnce(new Error("Fetch failed")) - const accountManager = new AccountManager(user.mock) + const accountManager = new AccountManager(userMock.mock, networkManager) await user.set!({addr: "0x1"} as CurrentUser) @@ -120,7 +133,7 @@ describe("AccountManager", () => { .mockResolvedValueOnce("0x123") // for user 0x1 .mockResolvedValueOnce("0x456") // for user 0x2 - const accountManager = new AccountManager(user.mock) + const accountManager = new AccountManager(userMock.mock, networkManager) await user.set({addr: "0x1"} as CurrentUser) expect(await accountManager.getCOAAddress()).toBe("0x123") @@ -136,7 +149,7 @@ describe("AccountManager", () => { const user = mockUser() - const accountManager = new AccountManager(user.mock) + const accountManager = new AccountManager(user.mock, networkManager) const callback = jest.fn() accountManager.subscribe(callback) @@ -154,7 +167,7 @@ describe("AccountManager", () => { const callback = jest.fn() - const accountManager = new AccountManager(user.mock) + const accountManager = new AccountManager(user.mock, networkManager) accountManager.subscribe(callback) @@ -170,7 +183,7 @@ describe("AccountManager", () => { mockQuery.mockResolvedValueOnce("0x123") - const accountManager = new AccountManager(user.mock) + const accountManager = new AccountManager(user.mock, networkManager) accountManager.subscribe(callback) @@ -181,7 +194,7 @@ describe("AccountManager", () => { it("should return an empty array when COA address is null", async () => { const {mock: user} = mockUser() - const accountManager = new AccountManager(user) + const accountManager = new AccountManager(user, networkManager) expect(await accountManager.getAccounts()).toEqual([]) }) @@ -189,20 +202,32 @@ describe("AccountManager", () => { mockQuery.mockResolvedValueOnce("0x123") const {mock: user} = mockUser({addr: "0x1"} as CurrentUser) - const accountManager = new AccountManager(user) + const accountManager = new AccountManager(user, networkManager) expect(await accountManager.getAccounts()).toEqual(["0x123"]) }) }) describe("send transaction", () => { + let networkManager: jest.Mocked + let $mockChainId: Subject + beforeEach(() => { + $mockChainId = new BehaviorSubject({ + chainId: 747, + error: null, + isLoading: false, + }) + networkManager = { + $chainId: $mockChainId, + } as any as jest.Mocked + jest.clearAllMocks() }) test("send transaction mainnet", async () => { const user = mockUser().mock - const accountManager = new AccountManager(user) + const accountManager = new AccountManager(user, networkManager) const mockTxResult = { onceExecuted: jest.fn().mockResolvedValue({ @@ -248,7 +273,7 @@ describe("send transaction", () => { test("send transaction testnet", async () => { const user = mockUser() - const accountManager = new AccountManager(user.mock) + const accountManager = new AccountManager(user.mock, networkManager) const mockTxResult = { onceExecuted: jest.fn().mockResolvedValue({ @@ -294,7 +319,7 @@ describe("send transaction", () => { test("throws error if no executed event not found", async () => { const user = mockUser() - const accountManager = new AccountManager(user.mock) + const accountManager = new AccountManager(user.mock, networkManager) const mockTxResult = { onceExecuted: jest.fn().mockResolvedValue({ @@ -322,6 +347,8 @@ describe("send transaction", () => { }) describe("signMessage", () => { + let $mockChainId: Subject + let networkManager: jest.Mocked let accountManager: AccountManager let user: ReturnType["mock"] let updateUser: ReturnType["set"] @@ -330,7 +357,11 @@ describe("signMessage", () => { jest.clearAllMocks() ;({mock: user, set: updateUser} = mockUser({addr: "0x123"} as CurrentUser)) jest.mocked(fcl.query).mockResolvedValue("0xCOA1") - accountManager = new AccountManager(user) + $mockChainId = new Subject() + networkManager = { + $chainId: $mockChainId, + } as any as jest.Mocked + accountManager = new AccountManager(user, networkManager) }) it("should throw an error if the COA address is not available", async () => { diff --git a/packages/fcl-ethereum-provider/src/accounts/account-manager.ts b/packages/fcl-ethereum-provider/src/accounts/account-manager.ts index 154e1dea1..630bffe74 100644 --- a/packages/fcl-ethereum-provider/src/accounts/account-manager.ts +++ b/packages/fcl-ethereum-provider/src/accounts/account-manager.ts @@ -12,6 +12,7 @@ import { import {TransactionExecutedEvent} from "../types/events" import { BehaviorSubject, + combineLatest, concat, distinctUntilChanged, filter, @@ -24,6 +25,7 @@ import { switchMap, } from "../util/observable" import {EthSignatureResponse} from "../types/eth" +import {NetworkManager} from "../network/network-manager" export class AccountManager { private $addressStore = new BehaviorSubject<{ @@ -36,9 +38,12 @@ export class AccountManager { error: null, }) - constructor(private user: typeof fcl.currentUser) { - // Create an observable from the user - const $user = new Observable(subscriber => { + constructor( + private user: typeof fcl.currentUser, + private networkManager: NetworkManager + ) { + // Create an observable from the Flow address + const $flowAddress = new Observable(subscriber => { return this.user.subscribe((currentUser: CurrentUser, error?: Error) => { if (error) { subscriber.error?.(error) @@ -46,14 +51,15 @@ export class AccountManager { subscriber.next(currentUser) } }) as Subscription - }) + }).pipe( + map(user => user.addr), + distinctUntilChanged() + ) // Bind the address store to the user observable - $user + combineLatest([$flowAddress, this.networkManager.$chainId]) .pipe( - map(snapshot => snapshot.addr || null), - distinctUntilChanged(), - switchMap(addr => + switchMap(([addr, chainId]) => concat( of({isLoading: true} as { isLoading: boolean @@ -63,12 +69,22 @@ export class AccountManager { from( (async () => { try { + if (chainId.isLoading) { + return {isLoading: true, address: null, error: null} + } + if (chainId.error) { + throw chainId.error + } + if (!addr) { return {isLoading: false, address: null, error: null} } return { isLoading: false, - address: await this.fetchCOAFromFlowAddress(addr), + address: await this.fetchCOAFromFlowAddress( + addr, + chainId.chainId! + ), error: null, } } catch (error: any) { @@ -82,9 +98,25 @@ export class AccountManager { .subscribe(this.$addressStore) } - private async fetchCOAFromFlowAddress(flowAddr: string): Promise { + private async fetchCOAFromFlowAddress( + flowAddr: string, + chainId: number + ): Promise { + // Find the Flow network based on the chain ID + const flowNetwork = Object.entries(FLOW_CHAINS).find( + ([, chain]) => chain.eip155ChainId === chainId + )?.[0] as FlowNetwork | undefined + + if (!flowNetwork) { + throw new Error("Flow network not found for chain ID") + } + + const evmContractAddress = fcl.withPrefix( + FLOW_CONTRACTS[ContractType.EVM][flowNetwork] + ) + const cadenceScript = ` - import EVM + import EVM from ${evmContractAddress} access(all) fun main(address: Address): String? { diff --git a/packages/fcl-ethereum-provider/src/create-provider.ts b/packages/fcl-ethereum-provider/src/create-provider.ts index 73a7f9e34..b656b7344 100644 --- a/packages/fcl-ethereum-provider/src/create-provider.ts +++ b/packages/fcl-ethereum-provider/src/create-provider.ts @@ -43,7 +43,7 @@ export function createProvider(config: { ) const networkManager = new NetworkManager(config.config) - const accountManager = new AccountManager(config.user) + const accountManager = new AccountManager(config.user, networkManager) const gateway = new Gateway({ ...defaultRpcUrls, ...(config.rpcUrls || {}), diff --git a/packages/fcl-ethereum-provider/src/events/event-dispatcher.test.ts b/packages/fcl-ethereum-provider/src/events/event-dispatcher.test.ts index 7f338e749..2bdf35b44 100644 --- a/packages/fcl-ethereum-provider/src/events/event-dispatcher.test.ts +++ b/packages/fcl-ethereum-provider/src/events/event-dispatcher.test.ts @@ -1,5 +1,5 @@ import {AccountManager} from "../accounts/account-manager" -import {ChainIdStore, NetworkManager} from "../network/network-manager" +import {NetworkManager} from "../network/network-manager" import {BehaviorSubject, Subject} from "../util/observable" import {EventDispatcher} from "./event-dispatcher" @@ -7,20 +7,13 @@ jest.mock("../accounts/account-manager") jest.mock("../network/network-manager") describe("event dispatcher", () => { - let networkManager: jest.Mocked - let accountManager: jest.Mocked - let $mockChainId: Subject - - beforeEach(() => { - jest.clearAllMocks() - $mockChainId = new Subject() - networkManager = { - $chainId: $mockChainId, - getChainId: jest.fn(), - } as any - accountManager = new (AccountManager as any)() - }) test("unsubscribe should remove listener", () => { + const accountManager: jest.Mocked = + new (AccountManager as any)() + + const networkManager: jest.Mocked = + new (NetworkManager as any)() + let subs: ((accounts: string[]) => void)[] = [] accountManager.subscribe.mockImplementation(cb => { subs.push(cb) @@ -53,6 +46,12 @@ describe("event dispatcher", () => { }) test("should emit accountsChanged", () => { + const accountManager: jest.Mocked = + new (AccountManager as any)() + + const networkManager: jest.Mocked = + new (NetworkManager as any)() + let mockMgrSubCb: (accounts: string[]) => void accountManager.subscribe.mockImplementation(cb => { mockMgrSubCb = cb @@ -76,6 +75,12 @@ describe("event dispatcher", () => { }) test("should emit accountsChanged multiple times", () => { + const accountManager: jest.Mocked = + new (AccountManager as any)() + + const networkManager: jest.Mocked = + new (NetworkManager as any)() + let mockMgrSubCb: (accounts: string[]) => void accountManager.subscribe.mockImplementation(cb => { mockMgrSubCb = cb @@ -100,20 +105,30 @@ describe("event dispatcher", () => { expect(listener).toHaveBeenNthCalledWith(2, ["0x5678"]) }) - test("should emit chainChanged", async () => { + test("should emit chainChanged", () => { + const accountManager: jest.Mocked = + new (AccountManager as any)() + + const networkManager: jest.Mocked = + new (NetworkManager as any)() + + let mockSubject = new Subject() + networkManager.subscribe.mockImplementation(cb => { + return mockSubject.subscribe(cb) + }) const listener = jest.fn() const eventDispatcher = new EventDispatcher(accountManager, networkManager) eventDispatcher.on("chainChanged", listener) - // Initial chain id, should not emit as a change - $mockChainId.next({isLoading: false, error: null, chainId: 0x286}) - - // Change chain id - $mockChainId.next({isLoading: false, error: null, chainId: 0x2eb}) + expect(networkManager.subscribe).toHaveBeenCalled() + expect(networkManager.subscribe).toHaveBeenCalledTimes(1) + expect(networkManager.subscribe).toHaveBeenCalledWith(expect.any(Function)) - await new Promise(resolve => setTimeout(resolve, 100)) + // Simulate network change from network manager + mockSubject.next(0x2eb) + expect(listener).toHaveBeenCalled() expect(listener).toHaveBeenCalledTimes(1) expect(listener).toHaveBeenCalledWith("0x2eb") }) diff --git a/packages/fcl-ethereum-provider/src/events/event-dispatcher.ts b/packages/fcl-ethereum-provider/src/events/event-dispatcher.ts index e94cc7a47..f7ccbf85e 100644 --- a/packages/fcl-ethereum-provider/src/events/event-dispatcher.ts +++ b/packages/fcl-ethereum-provider/src/events/event-dispatcher.ts @@ -1,14 +1,7 @@ import {EventCallback, ProviderEvents} from "../types/provider" import {AccountManager} from "../accounts/account-manager" import {NetworkManager} from "../network/network-manager" -import { - filter, - map, - Observable, - skip, - Subscription, - takeFirst, -} from "../util/observable" +import {Observable, Subscription} from "../util/observable" export class EventDispatcher { private $emitters: { @@ -23,29 +16,23 @@ export class EventDispatcher { constructor(accountManager: AccountManager, networkManager: NetworkManager) { this.$emitters = { - // Emit changes to the accounts as an accountsChanged event accountsChanged: new Observable(subscriber => { return accountManager.subscribe(accounts => { subscriber.next(accounts) }) }), - // Emit changes to the chainId as a chainChanged event - chainChanged: networkManager.$chainId.pipe( - filter(({isLoading, error}) => !isLoading && !error), - map(({chainId}) => { - return `0x${chainId!.toString(16)}` - }), - skip(1) - ) as Observable, - // Emit the first chainId as a connect event - connect: networkManager.$chainId.pipe( - filter(({isLoading, error}) => !isLoading && !error), - map(({chainId}) => { - return {chainId: `0x${chainId!.toString(16)}`} - }), - takeFirst() - ), - disconnect: new Observable<{reason: string}>(() => { + chainChanged: new Observable(subscriber => { + return networkManager.subscribe(chainId => { + if (!chainId) return + subscriber.next(`0x${chainId.toString(16)}`) + }) + }), + connect: new Observable(subscriber => { + subscriber.complete?.() + return () => {} + }), + disconnect: new Observable(subscriber => { + subscriber.complete?.() return () => {} }), } diff --git a/packages/fcl-ethereum-provider/src/hash-utils.test.ts b/packages/fcl-ethereum-provider/src/hash-utils.test.ts deleted file mode 100644 index e57afbca1..000000000 --- a/packages/fcl-ethereum-provider/src/hash-utils.test.ts +++ /dev/null @@ -1,113 +0,0 @@ -import {keccak_256} from "@noble/hashes/sha3" -import {bytesToHex} from "@noble/hashes/utils" -import {concat, arrayify} from "@ethersproject/bytes" -import {_TypedDataEncoder as TypedDataEncoder} from "@ethersproject/hash" -import {TypedData} from "./types/eth" -import { - hashTypedDataLegacy, - hashTypedDataV3, - hashTypedDataV4, -} from "./hash-utils" - -jest.mock("@noble/hashes/sha3", () => ({ - keccak_256: jest.fn(() => - Uint8Array.from([0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x90]) - ), -})) - -jest.mock("@ethersproject/hash", () => { - const original = jest.requireActual("@ethersproject/hash") - return { - ...original, - _TypedDataEncoder: { - hashDomain: jest.fn( - domain => - // Return a valid 32-byte hex string (64 hex characters after "0x") - "0x1111111111111111111111111111111111111111111111111111111111111111" - ), - hash: jest.fn( - (domain, types, message) => - "0x2222222222222222222222222222222222222222222222222222222222222222" - ), - }, - } -}) - -describe("Hash Utils", () => { - const mockTypedData: TypedData = { - domain: {name: "Ether Mail", chainId: 1}, - message: {from: "Alice", to: "Bob", contents: "Hello"}, - types: { - EIP712Domain: [ - {name: "name", type: "string"}, - {name: "chainId", type: "uint256"}, - ], - Mail: [ - {name: "from", type: "string"}, - {name: "to", type: "string"}, - {name: "contents", type: "string"}, - ], - }, - primaryType: "Mail", - } - - afterEach(() => { - jest.clearAllMocks() - }) - - describe("hashTypedDataLegacy", () => { - it("should throw an error for legacy (legacy support is not provided)", () => { - expect(() => hashTypedDataLegacy(mockTypedData)).toThrowError( - "Legacy eth_signTypedData is not supported. Please use eth_signTypedData_v3 or eth_signTypedData_v4 instead." - ) - }) - }) - - describe("hashTypedDataV3", () => { - it("should call the TypedDataEncoder functions and then keccak_256 correctly", () => { - const result = hashTypedDataV3(mockTypedData) - - expect(TypedDataEncoder.hashDomain).toHaveBeenCalledWith( - mockTypedData.domain - ) - - expect(TypedDataEncoder.hash).toHaveBeenCalledWith( - mockTypedData.domain, - mockTypedData.types, - mockTypedData.message - ) - - // The implementation concatenates: - // prefix (0x1901), domainSeparator, and messageHash. - const prefix = "0x1901" - const expectedConcat = concat([ - arrayify(prefix), - arrayify( - "0x1111111111111111111111111111111111111111111111111111111111111111" - ), - arrayify( - "0x2222222222222222222222222222222222222222222222222222222222222222" - ), - ]) - - expect(keccak_256).toHaveBeenCalledWith(expectedConcat) - - // The keccak_256 mock always returns our fixed Uint8Array, - // so the expected hash is: - const expectedV3Hash = - "0x" + - bytesToHex( - Uint8Array.from([0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x90]) - ) - expect(result).toBe(expectedV3Hash) - }) - }) - - describe("hashTypedDataV4", () => { - it("should produce the same result as v3 (for non-nested cases)", () => { - const v3Result = hashTypedDataV3(mockTypedData) - const v4Result = hashTypedDataV4(mockTypedData) - expect(v4Result).toBe(v3Result) - }) - }) -}) diff --git a/packages/fcl-ethereum-provider/src/hash-utils.ts b/packages/fcl-ethereum-provider/src/hash-utils.ts deleted file mode 100644 index a986dc6f1..000000000 --- a/packages/fcl-ethereum-provider/src/hash-utils.ts +++ /dev/null @@ -1,41 +0,0 @@ -import {keccak_256} from "@noble/hashes/sha3" -import {bytesToHex} from "@noble/hashes/utils" -import {arrayify, concat} from "@ethersproject/bytes" -import {_TypedDataEncoder as TypedDataEncoder} from "@ethersproject/hash" -import {TypedData} from "./types/eth" - -export function hashTypedDataLegacy(data: TypedData): string { - throw new Error( - "Legacy eth_signTypedData is not supported. Please use eth_signTypedData_v3 or eth_signTypedData_v4 instead." - ) -} - -/** - * Hash for `eth_signTypedData_v3` - * - * Uses EIP‑712 encoding: - * digest = keccak_256( "\x19\x01" || domainSeparator || messageHash ) - */ -export function hashTypedDataV3(data: TypedData): string { - const domainSeparator = TypedDataEncoder.hashDomain(data.domain) - const messageHash = TypedDataEncoder.hash( - data.domain, - data.types, - data.message - ) - // The EIP‑191 prefix is "0x1901". - const prefix = "0x1901" - const digest = keccak_256( - concat([arrayify(prefix), arrayify(domainSeparator), arrayify(messageHash)]) - ) - return "0x" + bytesToHex(digest) -} - -/** - * Hash for `eth_signTypedData_v4` - * - * For many cases, v3 and v4 yield the same result (if you’re not using arrays or nested dynamic types). - */ -export function hashTypedDataV4(data: TypedData): string { - return hashTypedDataV3(data) -} diff --git a/packages/fcl-ethereum-provider/src/network/network-manager.test.ts b/packages/fcl-ethereum-provider/src/network/network-manager.test.ts index 081abc8b2..e0dbcf0a9 100644 --- a/packages/fcl-ethereum-provider/src/network/network-manager.test.ts +++ b/packages/fcl-ethereum-provider/src/network/network-manager.test.ts @@ -73,9 +73,9 @@ describe("network manager", () => { const manager = new NetworkManager(config.mock) const chainId = await new Promise(resolve => { - const unsub = manager.$chainId.subscribe(({chainId}) => { - if (chainId) { - resolve(chainId) + const unsub = manager.subscribe(id => { + if (id) { + resolve(id) unsub() } }) @@ -96,9 +96,9 @@ describe("network manager", () => { const chainIds: number[] = [] - const unsub = manager.$chainId.subscribe(({chainId}) => { - if (chainId) { - chainIds.push(chainId) + const unsub = manager.subscribe(id => { + if (id) { + chainIds.push(id) } }) diff --git a/packages/fcl-ethereum-provider/src/network/network-manager.ts b/packages/fcl-ethereum-provider/src/network/network-manager.ts index bb7d630e4..dd2e420f0 100644 --- a/packages/fcl-ethereum-provider/src/network/network-manager.ts +++ b/packages/fcl-ethereum-provider/src/network/network-manager.ts @@ -7,7 +7,9 @@ import { firstValueFrom, from, map, + Observable, of, + Subscription, switchMap, } from "../util/observable" import * as fcl from "@onflow/fcl" @@ -17,7 +19,6 @@ export type ChainIdStore = { chainId: number | null error: unknown | null } - export class NetworkManager { private $chainIdStore = new BehaviorSubject({ isLoading: true, @@ -66,8 +67,17 @@ export class NetworkManager { .subscribe(this.$chainIdStore) } - get $chainId() { - return this.$chainIdStore.asObservable() + public get $chainId() { + return this.$chainIdStore as Observable + } + + public subscribe(callback: (chainId: number | null) => void): Subscription { + return this.$chainIdStore + .pipe( + filter(x => !x.isLoading && !x.error), + map(x => x.chainId) + ) + .subscribe(callback) } public async getChainId(): Promise { diff --git a/packages/fcl-ethereum-provider/src/rpc/handlers/eth-signtypeddata.ts b/packages/fcl-ethereum-provider/src/rpc/handlers/eth-signtypeddata.ts deleted file mode 100644 index 4a3350cfa..000000000 --- a/packages/fcl-ethereum-provider/src/rpc/handlers/eth-signtypeddata.ts +++ /dev/null @@ -1,30 +0,0 @@ -import {AccountManager} from "../../accounts/account-manager" -import {SignTypedDataParams} from "../../types/eth" -import { - hashTypedDataLegacy, - hashTypedDataV3, - hashTypedDataV4, -} from "../../hash-utils" - -export async function signTypedData( - accountManager: AccountManager, - params: SignTypedDataParams, - version: "eth_signTypedData" | "eth_signTypedData_v3" | "eth_signTypedData_v4" -) { - const {address, data} = params - - if (!address || !data) { - throw new Error("Missing signer address or typed data") - } - - let hashedMessage: string - if (version === "eth_signTypedData_v3") { - hashedMessage = hashTypedDataV3(data) - } else if (version === "eth_signTypedData_v4") { - hashedMessage = hashTypedDataV4(data) - } else { - hashedMessage = hashTypedDataLegacy(data) - } - - return await accountManager.signMessage(hashedMessage, address) -} diff --git a/packages/fcl-ethereum-provider/src/rpc/rpc-processor.ts b/packages/fcl-ethereum-provider/src/rpc/rpc-processor.ts index efd82c269..f86bf7212 100644 --- a/packages/fcl-ethereum-provider/src/rpc/rpc-processor.ts +++ b/packages/fcl-ethereum-provider/src/rpc/rpc-processor.ts @@ -5,8 +5,7 @@ import {AccountManager} from "../accounts/account-manager" import {ethSendTransaction} from "./handlers/eth-send-transaction" import {NetworkManager} from "../network/network-manager" import {personalSign} from "./handlers/personal-sign" -import {PersonalSignParams, SignTypedDataParams, TypedData} from "../types/eth" -import {signTypedData} from "./handlers/eth-signtypeddata" +import {PersonalSignParams} from "../types/eth" export class RpcProcessor { constructor( @@ -28,32 +27,6 @@ export class RpcProcessor { return ethRequestAccounts(this.accountManager) case "eth_sendTransaction": return await ethSendTransaction(this.accountManager, params) - case "eth_signTypedData": - case "eth_signTypedData_v3": - case "eth_signTypedData_v4": { - if (!params || typeof params !== "object") { - throw new Error(`${method} requires valid parameters.`) - } - - const {address, data} = params as {address?: unknown; data?: unknown} - - if ( - typeof address !== "string" || - typeof data !== "object" || - data === null - ) { - throw new Error( - `${method} requires 'address' (string) and a valid 'data' object.` - ) - } - - const validParams: SignTypedDataParams = { - address, - data: data as TypedData, - } - - return await signTypedData(this.accountManager, validParams, method) - } case "personal_sign": return await personalSign( this.accountManager, diff --git a/packages/fcl-ethereum-provider/src/types/eth.ts b/packages/fcl-ethereum-provider/src/types/eth.ts index 92652d89f..4cd8076db 100644 --- a/packages/fcl-ethereum-provider/src/types/eth.ts +++ b/packages/fcl-ethereum-provider/src/types/eth.ts @@ -1,28 +1,3 @@ export type EthSignatureResponse = string export type PersonalSignParams = [string, string] - -export interface SignTypedDataParams { - address: string - data: TypedData // This represents the EIP-712 structured data -} - -export interface TypedDataField { - name: string - type: string -} - -export interface TypedDataDomain { - name?: string - version?: string - chainId?: number - verifyingContract?: string - salt?: string -} - -export interface TypedData { - types: Record - domain: TypedDataDomain - primaryType: string - message: Record -} diff --git a/packages/fcl-ethereum-provider/src/util/observable.ts b/packages/fcl-ethereum-provider/src/util/observable.ts index ca975b040..70eeb82f9 100644 --- a/packages/fcl-ethereum-provider/src/util/observable.ts +++ b/packages/fcl-ethereum-provider/src/util/observable.ts @@ -57,12 +57,6 @@ export class Observable { this as Observable ) } - - asObservable(): Observable { - return new Observable(subscriber => { - return this.subscribe(subscriber) - }) - } } export type Subscription = () => void @@ -306,42 +300,47 @@ export function of(value: T): Observable { }) } -/** skip */ -export function skip( - count: number -): (source: Observable) => Observable { - return source => { - return new Observable(subscriber => { - let skipped = 0 - return source.subscribe({ +/** combineLatest */ +export function combineLatest( + sources: [Observable, Observable] +): Observable<[T1, T2]> +export function combineLatest( + sources: [Observable, Observable, Observable] +): Observable<[T1, T2, T3]> +export function combineLatest( + sources: [Observable, Observable, Observable, Observable] +): Observable<[T1, T2, T3, T4]> +export function combineLatest(sources: Observable[]): Observable { + return new Observable(subscriber => { + const latestValues = new Array(sources.length) + const hasValue = new Array(sources.length).fill(false) + const completed = new Array(sources.length).fill(false) + + const subscriptions = sources.map((source, index) => + source.subscribe({ next: value => { - if (skipped >= count) { - subscriber.next(value) - } else { - skipped++ + latestValues[index] = value + hasValue[index] = true + + if (hasValue.every(Boolean)) { + subscriber.next(latestValues.slice()) } }, error: subscriber.error?.bind(subscriber), - complete: subscriber.complete?.bind(subscriber), - }) - }) - } -} + complete: () => { + completed[index] = true -/** takeFirst */ -export function takeFirst(): (source: Observable) => Observable { - return source => { - return new Observable(subscriber => { - return source.subscribe({ - next: value => { - subscriber.next(value) - subscriber.complete?.() + if (completed.every(Boolean)) { + subscriber.complete?.() + } }, - error: subscriber.error?.bind(subscriber), - complete: subscriber.complete?.bind(subscriber), }) - }) - } + ) + + return () => { + subscriptions.forEach(unsub => unsub()) + } + }) } /******************************* From 82eacb0712a3605ff61ee7a0be613814707895f9 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Wed, 5 Feb 2025 12:20:03 -0800 Subject: [PATCH 2/9] fix tests --- .../src/accounts/account-manager.test.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/fcl-ethereum-provider/src/accounts/account-manager.test.ts b/packages/fcl-ethereum-provider/src/accounts/account-manager.test.ts index 9f1eda87c..b6f3bb95a 100644 --- a/packages/fcl-ethereum-provider/src/accounts/account-manager.test.ts +++ b/packages/fcl-ethereum-provider/src/accounts/account-manager.test.ts @@ -364,7 +364,6 @@ describe("send transaction", () => { }) describe("signMessage", () => { - let $mockChainId: Subject let networkManager: jest.Mocked let accountManager: AccountManager let user: ReturnType["mock"] @@ -374,9 +373,14 @@ describe("signMessage", () => { jest.clearAllMocks() ;({mock: user, set: updateUser} = mockUser({addr: "0x123"} as CurrentUser)) jest.mocked(fcl.query).mockResolvedValue("0xCOA1") - $mockChainId = new Subject() + const $mockChainId = new BehaviorSubject({ + chainId: 747, + error: null, + isLoading: false, + }) networkManager = { $chainId: $mockChainId, + getChainId: $mockChainId.getValue(), } as any as jest.Mocked accountManager = new AccountManager(user, networkManager) }) @@ -423,10 +427,7 @@ describe("signMessage", () => { }) it("should throw an error if signUserMessage returns an empty array", async () => { - accountManager["coaAddress"] = "0xCOA1" - - user.signUserMessage = jest.fn().mockResolvedValue([]) - + user.signUserMessage.mockResolvedValue([]) await expect( accountManager.signMessage("Test message", "0xCOA1") ).rejects.toThrow("Failed to sign message") From ec86613bf21cc6f7c7e66e890a1ea1053bab582c Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Wed, 5 Feb 2025 12:27:19 -0800 Subject: [PATCH 3/9] fix merge --- .../src/events/event-dispatcher.ts | 39 ++++-- .../src/hash-utils.test.ts | 113 ++++++++++++++++++ .../fcl-ethereum-provider/src/hash-utils.ts | 41 +++++++ .../src/rpc/handlers/eth-signtypeddata.ts | 30 +++++ .../src/util/observable.ts | 6 + 5 files changed, 216 insertions(+), 13 deletions(-) create mode 100644 packages/fcl-ethereum-provider/src/hash-utils.test.ts create mode 100644 packages/fcl-ethereum-provider/src/hash-utils.ts create mode 100644 packages/fcl-ethereum-provider/src/rpc/handlers/eth-signtypeddata.ts diff --git a/packages/fcl-ethereum-provider/src/events/event-dispatcher.ts b/packages/fcl-ethereum-provider/src/events/event-dispatcher.ts index f7ccbf85e..e94cc7a47 100644 --- a/packages/fcl-ethereum-provider/src/events/event-dispatcher.ts +++ b/packages/fcl-ethereum-provider/src/events/event-dispatcher.ts @@ -1,7 +1,14 @@ import {EventCallback, ProviderEvents} from "../types/provider" import {AccountManager} from "../accounts/account-manager" import {NetworkManager} from "../network/network-manager" -import {Observable, Subscription} from "../util/observable" +import { + filter, + map, + Observable, + skip, + Subscription, + takeFirst, +} from "../util/observable" export class EventDispatcher { private $emitters: { @@ -16,23 +23,29 @@ export class EventDispatcher { constructor(accountManager: AccountManager, networkManager: NetworkManager) { this.$emitters = { + // Emit changes to the accounts as an accountsChanged event accountsChanged: new Observable(subscriber => { return accountManager.subscribe(accounts => { subscriber.next(accounts) }) }), - chainChanged: new Observable(subscriber => { - return networkManager.subscribe(chainId => { - if (!chainId) return - subscriber.next(`0x${chainId.toString(16)}`) - }) - }), - connect: new Observable(subscriber => { - subscriber.complete?.() - return () => {} - }), - disconnect: new Observable(subscriber => { - subscriber.complete?.() + // Emit changes to the chainId as a chainChanged event + chainChanged: networkManager.$chainId.pipe( + filter(({isLoading, error}) => !isLoading && !error), + map(({chainId}) => { + return `0x${chainId!.toString(16)}` + }), + skip(1) + ) as Observable, + // Emit the first chainId as a connect event + connect: networkManager.$chainId.pipe( + filter(({isLoading, error}) => !isLoading && !error), + map(({chainId}) => { + return {chainId: `0x${chainId!.toString(16)}`} + }), + takeFirst() + ), + disconnect: new Observable<{reason: string}>(() => { return () => {} }), } diff --git a/packages/fcl-ethereum-provider/src/hash-utils.test.ts b/packages/fcl-ethereum-provider/src/hash-utils.test.ts new file mode 100644 index 000000000..e57afbca1 --- /dev/null +++ b/packages/fcl-ethereum-provider/src/hash-utils.test.ts @@ -0,0 +1,113 @@ +import {keccak_256} from "@noble/hashes/sha3" +import {bytesToHex} from "@noble/hashes/utils" +import {concat, arrayify} from "@ethersproject/bytes" +import {_TypedDataEncoder as TypedDataEncoder} from "@ethersproject/hash" +import {TypedData} from "./types/eth" +import { + hashTypedDataLegacy, + hashTypedDataV3, + hashTypedDataV4, +} from "./hash-utils" + +jest.mock("@noble/hashes/sha3", () => ({ + keccak_256: jest.fn(() => + Uint8Array.from([0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x90]) + ), +})) + +jest.mock("@ethersproject/hash", () => { + const original = jest.requireActual("@ethersproject/hash") + return { + ...original, + _TypedDataEncoder: { + hashDomain: jest.fn( + domain => + // Return a valid 32-byte hex string (64 hex characters after "0x") + "0x1111111111111111111111111111111111111111111111111111111111111111" + ), + hash: jest.fn( + (domain, types, message) => + "0x2222222222222222222222222222222222222222222222222222222222222222" + ), + }, + } +}) + +describe("Hash Utils", () => { + const mockTypedData: TypedData = { + domain: {name: "Ether Mail", chainId: 1}, + message: {from: "Alice", to: "Bob", contents: "Hello"}, + types: { + EIP712Domain: [ + {name: "name", type: "string"}, + {name: "chainId", type: "uint256"}, + ], + Mail: [ + {name: "from", type: "string"}, + {name: "to", type: "string"}, + {name: "contents", type: "string"}, + ], + }, + primaryType: "Mail", + } + + afterEach(() => { + jest.clearAllMocks() + }) + + describe("hashTypedDataLegacy", () => { + it("should throw an error for legacy (legacy support is not provided)", () => { + expect(() => hashTypedDataLegacy(mockTypedData)).toThrowError( + "Legacy eth_signTypedData is not supported. Please use eth_signTypedData_v3 or eth_signTypedData_v4 instead." + ) + }) + }) + + describe("hashTypedDataV3", () => { + it("should call the TypedDataEncoder functions and then keccak_256 correctly", () => { + const result = hashTypedDataV3(mockTypedData) + + expect(TypedDataEncoder.hashDomain).toHaveBeenCalledWith( + mockTypedData.domain + ) + + expect(TypedDataEncoder.hash).toHaveBeenCalledWith( + mockTypedData.domain, + mockTypedData.types, + mockTypedData.message + ) + + // The implementation concatenates: + // prefix (0x1901), domainSeparator, and messageHash. + const prefix = "0x1901" + const expectedConcat = concat([ + arrayify(prefix), + arrayify( + "0x1111111111111111111111111111111111111111111111111111111111111111" + ), + arrayify( + "0x2222222222222222222222222222222222222222222222222222222222222222" + ), + ]) + + expect(keccak_256).toHaveBeenCalledWith(expectedConcat) + + // The keccak_256 mock always returns our fixed Uint8Array, + // so the expected hash is: + const expectedV3Hash = + "0x" + + bytesToHex( + Uint8Array.from([0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x90]) + ) + expect(result).toBe(expectedV3Hash) + }) + }) + + describe("hashTypedDataV4", () => { + it("should produce the same result as v3 (for non-nested cases)", () => { + const v3Result = hashTypedDataV3(mockTypedData) + const v4Result = hashTypedDataV4(mockTypedData) + expect(v4Result).toBe(v3Result) + }) + }) +}) diff --git a/packages/fcl-ethereum-provider/src/hash-utils.ts b/packages/fcl-ethereum-provider/src/hash-utils.ts new file mode 100644 index 000000000..a986dc6f1 --- /dev/null +++ b/packages/fcl-ethereum-provider/src/hash-utils.ts @@ -0,0 +1,41 @@ +import {keccak_256} from "@noble/hashes/sha3" +import {bytesToHex} from "@noble/hashes/utils" +import {arrayify, concat} from "@ethersproject/bytes" +import {_TypedDataEncoder as TypedDataEncoder} from "@ethersproject/hash" +import {TypedData} from "./types/eth" + +export function hashTypedDataLegacy(data: TypedData): string { + throw new Error( + "Legacy eth_signTypedData is not supported. Please use eth_signTypedData_v3 or eth_signTypedData_v4 instead." + ) +} + +/** + * Hash for `eth_signTypedData_v3` + * + * Uses EIP‑712 encoding: + * digest = keccak_256( "\x19\x01" || domainSeparator || messageHash ) + */ +export function hashTypedDataV3(data: TypedData): string { + const domainSeparator = TypedDataEncoder.hashDomain(data.domain) + const messageHash = TypedDataEncoder.hash( + data.domain, + data.types, + data.message + ) + // The EIP‑191 prefix is "0x1901". + const prefix = "0x1901" + const digest = keccak_256( + concat([arrayify(prefix), arrayify(domainSeparator), arrayify(messageHash)]) + ) + return "0x" + bytesToHex(digest) +} + +/** + * Hash for `eth_signTypedData_v4` + * + * For many cases, v3 and v4 yield the same result (if you’re not using arrays or nested dynamic types). + */ +export function hashTypedDataV4(data: TypedData): string { + return hashTypedDataV3(data) +} diff --git a/packages/fcl-ethereum-provider/src/rpc/handlers/eth-signtypeddata.ts b/packages/fcl-ethereum-provider/src/rpc/handlers/eth-signtypeddata.ts new file mode 100644 index 000000000..4a3350cfa --- /dev/null +++ b/packages/fcl-ethereum-provider/src/rpc/handlers/eth-signtypeddata.ts @@ -0,0 +1,30 @@ +import {AccountManager} from "../../accounts/account-manager" +import {SignTypedDataParams} from "../../types/eth" +import { + hashTypedDataLegacy, + hashTypedDataV3, + hashTypedDataV4, +} from "../../hash-utils" + +export async function signTypedData( + accountManager: AccountManager, + params: SignTypedDataParams, + version: "eth_signTypedData" | "eth_signTypedData_v3" | "eth_signTypedData_v4" +) { + const {address, data} = params + + if (!address || !data) { + throw new Error("Missing signer address or typed data") + } + + let hashedMessage: string + if (version === "eth_signTypedData_v3") { + hashedMessage = hashTypedDataV3(data) + } else if (version === "eth_signTypedData_v4") { + hashedMessage = hashTypedDataV4(data) + } else { + hashedMessage = hashTypedDataLegacy(data) + } + + return await accountManager.signMessage(hashedMessage, address) +} diff --git a/packages/fcl-ethereum-provider/src/util/observable.ts b/packages/fcl-ethereum-provider/src/util/observable.ts index 74bf2f4b5..675db24de 100644 --- a/packages/fcl-ethereum-provider/src/util/observable.ts +++ b/packages/fcl-ethereum-provider/src/util/observable.ts @@ -57,6 +57,12 @@ export class Observable { this as Observable ) } + + asObservable(): Observable { + return new Observable(subscriber => { + return this.subscribe(subscriber) + }) + } } export type Subscription = () => void From 6e518cb263a1c9c105e1a91c3a410d104ff098a5 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Wed, 5 Feb 2025 12:28:45 -0800 Subject: [PATCH 4/9] fix merge --- package-lock.json | 21 +++++++++++++++++++++ package.json | 3 +++ packages/fcl-ethereum-provider/package.json | 2 ++ 3 files changed, 26 insertions(+) diff --git a/package-lock.json b/package-lock.json index c638df32a..30b629384 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,6 +8,9 @@ "workspaces": [ "./packages/*" ], + "dependencies": { + "@noble/hashes": "^1.7.1" + }, "devDependencies": { "@babel/preset-typescript": "^7.25.7", "@changesets/changelog-github": "^0.4.8", @@ -2634,6 +2637,8 @@ }, "node_modules/@ethersproject/bytes": { "version": "5.7.0", + "resolved": "https://registry.npmjs.org/@ethersproject/bytes/-/bytes-5.7.0.tgz", + "integrity": "sha512-nsbxwgFXWh9NyYWo+U8atvmMsSdKJprTcICAkvbBffT75qDocbuggBU0SJiVK2MuTrp0q+xvLkTnGMPK1+uA9A==", "funding": [ { "type": "individual", @@ -2668,6 +2673,8 @@ }, "node_modules/@ethersproject/hash": { "version": "5.7.0", + "resolved": "https://registry.npmjs.org/@ethersproject/hash/-/hash-5.7.0.tgz", + "integrity": "sha512-qX5WrQfnah1EFnO5zJv1v46a8HW0+E5xuBBDTwMFZLuVTx0tbU2kkx15NqdjxecrLGatQN9FGQKpb1FKdHCt+g==", "funding": [ { "type": "individual", @@ -5768,6 +5775,18 @@ "@tybys/wasm-util": "^0.9.0" } }, + "node_modules/@noble/hashes": { + "version": "1.7.1", + "resolved": "https://registry.npmjs.org/@noble/hashes/-/hashes-1.7.1.tgz", + "integrity": "sha512-B8XBPsn4vT/KJAGqDzbwztd+6Yte3P4V7iafm24bxgDe/mlRuK6xmWPuCNrKt2vDafZ8MfJLlchDG/vYafQEjQ==", + "license": "MIT", + "engines": { + "node": "^14.21.3 || >=16" + }, + "funding": { + "url": "https://paulmillr.com/funding/" + } + }, "node_modules/@nodelib/fs.scandir": { "version": "2.1.5", "license": "MIT", @@ -29201,6 +29220,8 @@ "license": "Apache-2.0", "dependencies": { "@babel/runtime": "^7.25.7", + "@ethersproject/bytes": "^5.7.0", + "@ethersproject/hash": "^5.7.0", "@onflow/fcl": "1.13.4", "@onflow/rlp": "^1.2.3", "@walletconnect/jsonrpc-http-connection": "^1.0.8", diff --git a/package.json b/package.json index 4699e1f07..c3e04d794 100644 --- a/package.json +++ b/package.json @@ -36,5 +36,8 @@ "@nx/nx-darwin-x64": "^17.3.2", "@nx/nx-linux-x64-gnu": "^17.3.2", "@nx/nx-win32-x64-msvc": "^17.3.2" + }, + "dependencies": { + "@noble/hashes": "^1.7.1" } } diff --git a/packages/fcl-ethereum-provider/package.json b/packages/fcl-ethereum-provider/package.json index 7986990a3..9907ac63d 100644 --- a/packages/fcl-ethereum-provider/package.json +++ b/packages/fcl-ethereum-provider/package.json @@ -37,6 +37,8 @@ }, "dependencies": { "@babel/runtime": "^7.25.7", + "@ethersproject/bytes": "^5.7.0", + "@ethersproject/hash": "^5.7.0", "@onflow/fcl": "1.13.4", "@onflow/rlp": "^1.2.3", "@walletconnect/jsonrpc-http-connection": "^1.0.8", From b944c377232ca29900532c38e5093c543fe5d066 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Wed, 5 Feb 2025 12:29:14 -0800 Subject: [PATCH 5/9] fix merge --- .../src/rpc/rpc-processor.ts | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/fcl-ethereum-provider/src/rpc/rpc-processor.ts b/packages/fcl-ethereum-provider/src/rpc/rpc-processor.ts index eb038a761..17b72959a 100644 --- a/packages/fcl-ethereum-provider/src/rpc/rpc-processor.ts +++ b/packages/fcl-ethereum-provider/src/rpc/rpc-processor.ts @@ -8,8 +8,11 @@ import {personalSign} from "./handlers/personal-sign" import { AddEthereumChainParams, PersonalSignParams, + SignTypedDataParams, SwitchEthereumChainParams, + TypedData, } from "../types/eth" +import {signTypedData} from "./handlers/eth-signtypeddata" export class RpcProcessor { constructor( @@ -31,6 +34,32 @@ export class RpcProcessor { return ethRequestAccounts(this.accountManager) case "eth_sendTransaction": return await ethSendTransaction(this.accountManager, params) + case "eth_signTypedData": + case "eth_signTypedData_v3": + case "eth_signTypedData_v4": { + if (!params || typeof params !== "object") { + throw new Error(`${method} requires valid parameters.`) + } + + const {address, data} = params as {address?: unknown; data?: unknown} + + if ( + typeof address !== "string" || + typeof data !== "object" || + data === null + ) { + throw new Error( + `${method} requires 'address' (string) and a valid 'data' object.` + ) + } + + const validParams: SignTypedDataParams = { + address, + data: data as TypedData, + } + + return await signTypedData(this.accountManager, validParams, method) + } case "personal_sign": return await personalSign( this.accountManager, From 090f6822d42cf400bc8b0b76c09aba608e114f70 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Wed, 5 Feb 2025 13:13:44 -0800 Subject: [PATCH 6/9] fix tests --- .../src/events/event-dispatcher.test.ts | 106 ++++++++---------- .../src/events/event-dispatcher.ts | 4 +- .../src/util/observable.ts | 40 +++++++ 3 files changed, 91 insertions(+), 59 deletions(-) diff --git a/packages/fcl-ethereum-provider/src/events/event-dispatcher.test.ts b/packages/fcl-ethereum-provider/src/events/event-dispatcher.test.ts index 2bdf35b44..55a964835 100644 --- a/packages/fcl-ethereum-provider/src/events/event-dispatcher.test.ts +++ b/packages/fcl-ethereum-provider/src/events/event-dispatcher.test.ts @@ -1,26 +1,39 @@ import {AccountManager} from "../accounts/account-manager" -import {NetworkManager} from "../network/network-manager" -import {BehaviorSubject, Subject} from "../util/observable" +import {ChainIdStore, NetworkManager} from "../network/network-manager" +import {BehaviorSubject} from "../util/observable" import {EventDispatcher} from "./event-dispatcher" jest.mock("../accounts/account-manager") jest.mock("../network/network-manager") describe("event dispatcher", () => { - test("unsubscribe should remove listener", () => { - const accountManager: jest.Mocked = - new (AccountManager as any)() - - const networkManager: jest.Mocked = - new (NetworkManager as any)() - - let subs: ((accounts: string[]) => void)[] = [] - accountManager.subscribe.mockImplementation(cb => { - subs.push(cb) - return () => { - subs = subs.filter(sub => sub !== cb) - } + let $chainIdMock: BehaviorSubject + let networkManager: jest.Mocked + let $accountsMock: BehaviorSubject + let accountManager: jest.Mocked + + beforeEach(() => { + jest.clearAllMocks() + + $chainIdMock = new BehaviorSubject({ + isLoading: false, + chainId: 0x1, + error: null, }) + networkManager = { + subscribe: jest.fn(), + $chainId: $chainIdMock, + } as unknown as jest.Mocked + + $accountsMock = new BehaviorSubject([]) + accountManager = { + subscribe: jest.fn().mockImplementation(cb => { + return $accountsMock.subscribe(cb) + }), + } as unknown as jest.Mocked + }) + + test("unsubscribe should remove listener", () => { const listener = jest.fn() const eventDispatcher = new EventDispatcher(accountManager, networkManager) @@ -31,7 +44,7 @@ describe("event dispatcher", () => { expect(accountManager.subscribe).toHaveBeenCalledWith(expect.any(Function)) // Simulate account change from account manager - subs.forEach(sub => sub(["0x1234"])) + $accountsMock.next(["0x1234"]) expect(listener).toHaveBeenCalled() expect(listener).toHaveBeenCalledTimes(1) @@ -40,23 +53,12 @@ describe("event dispatcher", () => { eventDispatcher.off("accountsChanged", listener) // Simulate account change from account manager - subs.forEach(sub => sub(["0x5678"])) + $accountsMock.next(["0x5678"]) expect(listener).toHaveBeenCalledTimes(1) }) test("should emit accountsChanged", () => { - const accountManager: jest.Mocked = - new (AccountManager as any)() - - const networkManager: jest.Mocked = - new (NetworkManager as any)() - - let mockMgrSubCb: (accounts: string[]) => void - accountManager.subscribe.mockImplementation(cb => { - mockMgrSubCb = cb - return () => {} - }) const listener = jest.fn() const eventDispatcher = new EventDispatcher(accountManager, networkManager) @@ -67,7 +69,7 @@ describe("event dispatcher", () => { expect(accountManager.subscribe).toHaveBeenCalledWith(expect.any(Function)) // Simulate account change from account manager - mockMgrSubCb!(["0x1234"]) + $accountsMock.next(["0x1234"]) expect(listener).toHaveBeenCalled() expect(listener).toHaveBeenCalledTimes(1) @@ -75,17 +77,6 @@ describe("event dispatcher", () => { }) test("should emit accountsChanged multiple times", () => { - const accountManager: jest.Mocked = - new (AccountManager as any)() - - const networkManager: jest.Mocked = - new (NetworkManager as any)() - - let mockMgrSubCb: (accounts: string[]) => void - accountManager.subscribe.mockImplementation(cb => { - mockMgrSubCb = cb - return () => {} - }) const listener = jest.fn() const eventDispatcher = new EventDispatcher(accountManager, networkManager) @@ -96,8 +87,8 @@ describe("event dispatcher", () => { expect(accountManager.subscribe).toHaveBeenCalledWith(expect.any(Function)) // Simulate account change from account manager - mockMgrSubCb!(["0x1234"]) - mockMgrSubCb!(["0x5678"]) + $accountsMock.next(["0x1234"]) + $accountsMock.next(["0x5678"]) expect(listener).toHaveBeenCalled() expect(listener).toHaveBeenCalledTimes(2) @@ -106,30 +97,31 @@ describe("event dispatcher", () => { }) test("should emit chainChanged", () => { - const accountManager: jest.Mocked = - new (AccountManager as any)() - - const networkManager: jest.Mocked = - new (NetworkManager as any)() - - let mockSubject = new Subject() - networkManager.subscribe.mockImplementation(cb => { - return mockSubject.subscribe(cb) - }) const listener = jest.fn() const eventDispatcher = new EventDispatcher(accountManager, networkManager) eventDispatcher.on("chainChanged", listener) - expect(networkManager.subscribe).toHaveBeenCalled() - expect(networkManager.subscribe).toHaveBeenCalledTimes(1) - expect(networkManager.subscribe).toHaveBeenCalledWith(expect.any(Function)) - // Simulate network change from network manager - mockSubject.next(0x2eb) + $chainIdMock.next({isLoading: false, chainId: 0x2eb, error: null}) expect(listener).toHaveBeenCalled() expect(listener).toHaveBeenCalledTimes(1) expect(listener).toHaveBeenCalledWith("0x2eb") }) + + test("should emit connect", () => { + const listener = jest.fn() + + const eventDispatcher = new EventDispatcher(accountManager, networkManager) + eventDispatcher.on("connect", listener) + + // Simulate network change from network manager + // Should be ignored because connect only emits the first chainId + $chainIdMock.next({isLoading: false, chainId: 0x2eb, error: null}) + + expect(listener).toHaveBeenCalled() + expect(listener).toHaveBeenCalledTimes(1) + expect(listener).toHaveBeenCalledWith({chainId: "0x1"}) + }) }) diff --git a/packages/fcl-ethereum-provider/src/events/event-dispatcher.ts b/packages/fcl-ethereum-provider/src/events/event-dispatcher.ts index e94cc7a47..4e9d57ed4 100644 --- a/packages/fcl-ethereum-provider/src/events/event-dispatcher.ts +++ b/packages/fcl-ethereum-provider/src/events/event-dispatcher.ts @@ -24,11 +24,11 @@ export class EventDispatcher { constructor(accountManager: AccountManager, networkManager: NetworkManager) { this.$emitters = { // Emit changes to the accounts as an accountsChanged event - accountsChanged: new Observable(subscriber => { + accountsChanged: new Observable(subscriber => { return accountManager.subscribe(accounts => { subscriber.next(accounts) }) - }), + }).pipe(skip(1)), // Emit changes to the chainId as a chainChanged event chainChanged: networkManager.$chainId.pipe( filter(({isLoading, error}) => !isLoading && !error), diff --git a/packages/fcl-ethereum-provider/src/util/observable.ts b/packages/fcl-ethereum-provider/src/util/observable.ts index 675db24de..b7f8f16b4 100644 --- a/packages/fcl-ethereum-provider/src/util/observable.ts +++ b/packages/fcl-ethereum-provider/src/util/observable.ts @@ -351,6 +351,46 @@ export function combineLatest(sources: Observable[]): Observable { }) } +export function skip( + count: number +): (source: Observable) => Observable { + return source => { + return new Observable(subscriber => { + let skipped = 0 + return source.subscribe({ + next: value => { + if (skipped >= count) { + subscriber.next(value) + } else { + skipped++ + } + }, + error: subscriber.error?.bind(subscriber), + complete: subscriber.complete?.bind(subscriber), + }) + }) + } +} + +export function takeFirst(): (source: Observable) => Observable { + return source => { + return new Observable(subscriber => { + let hasEmitted = false + return source.subscribe({ + next: value => { + if (!hasEmitted) { + hasEmitted = true + subscriber.next(value) + subscriber.complete?.() + } + }, + error: subscriber.error?.bind(subscriber), + complete: subscriber.complete?.bind(subscriber), + }) + }) + } +} + /******************************* * Internal utility *******************************/ From a2a821d80705fa06c7c1d4086ca87244c89a2b3f Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Wed, 5 Feb 2025 14:20:57 -0800 Subject: [PATCH 7/9] remove subscribe function --- .../src/network/network-manager.test.ts | 23 +++++++++++-------- .../src/network/network-manager.ts | 15 ++---------- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/packages/fcl-ethereum-provider/src/network/network-manager.test.ts b/packages/fcl-ethereum-provider/src/network/network-manager.test.ts index e0dbcf0a9..36733d29c 100644 --- a/packages/fcl-ethereum-provider/src/network/network-manager.test.ts +++ b/packages/fcl-ethereum-provider/src/network/network-manager.test.ts @@ -63,7 +63,7 @@ describe("network manager", () => { await expect(manager.getChainId()).rejects.toThrow("error") }) - test("subscribe should return correct chain id", async () => { + test("observable should return correct chain id", async () => { jest.mocked(fcl.getChainId).mockResolvedValue("mainnet") const config = mockConfig() @@ -73,12 +73,15 @@ describe("network manager", () => { const manager = new NetworkManager(config.mock) const chainId = await new Promise(resolve => { - const unsub = manager.subscribe(id => { - if (id) { - resolve(id) - unsub() + const unsub = manager.$chainId.subscribe( + ({isLoading, chainId, error}) => { + expect(error).toBeUndefined() + if (!isLoading && chainId) { + resolve(chainId) + unsub() + } } - }) + ) }) expect(chainId).toBe(747) @@ -96,9 +99,11 @@ describe("network manager", () => { const chainIds: number[] = [] - const unsub = manager.subscribe(id => { - if (id) { - chainIds.push(id) + const unsub = manager.$chainId.subscribe(({isLoading, chainId, error}) => { + expect(error).toBeUndefined() + if (!isLoading && chainId) { + chainIds.push(chainId) + unsub() } }) diff --git a/packages/fcl-ethereum-provider/src/network/network-manager.ts b/packages/fcl-ethereum-provider/src/network/network-manager.ts index 1c924c885..a16c4e658 100644 --- a/packages/fcl-ethereum-provider/src/network/network-manager.ts +++ b/packages/fcl-ethereum-provider/src/network/network-manager.ts @@ -27,6 +27,8 @@ export class NetworkManager { error: null, }) + public readonly $chainId = this.$chainIdStore.asObservable() + constructor(config: typeof fcl.config) { // Map FCL config to behavior subject const $config = new BehaviorSubject | null>(null) @@ -68,19 +70,6 @@ export class NetworkManager { .subscribe(this.$chainIdStore) } - public get $chainId() { - return this.$chainIdStore as Observable - } - - public subscribe(callback: (chainId: number | null) => void): Subscription { - return this.$chainIdStore - .pipe( - filter(x => !x.isLoading && !x.error), - map(x => x.chainId) - ) - .subscribe(callback) - } - public async getChainId(): Promise { const {chainId, error} = await firstValueFrom( this.$chainIdStore.pipe(filter(x => !x.isLoading)) From 6a2bf226b9cee0388f65df3faf37b160818678a3 Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Wed, 5 Feb 2025 14:53:34 -0800 Subject: [PATCH 8/9] fix tests --- .../src/network/network-manager.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/fcl-ethereum-provider/src/network/network-manager.test.ts b/packages/fcl-ethereum-provider/src/network/network-manager.test.ts index 36733d29c..cc76d9841 100644 --- a/packages/fcl-ethereum-provider/src/network/network-manager.test.ts +++ b/packages/fcl-ethereum-provider/src/network/network-manager.test.ts @@ -72,10 +72,12 @@ describe("network manager", () => { }) const manager = new NetworkManager(config.mock) - const chainId = await new Promise(resolve => { + const chainId = await new Promise((resolve, reject) => { const unsub = manager.$chainId.subscribe( ({isLoading, chainId, error}) => { - expect(error).toBeUndefined() + if (error) { + reject(error) + } if (!isLoading && chainId) { resolve(chainId) unsub() @@ -100,7 +102,7 @@ describe("network manager", () => { const chainIds: number[] = [] const unsub = manager.$chainId.subscribe(({isLoading, chainId, error}) => { - expect(error).toBeUndefined() + expect(error).toBeFalsy() if (!isLoading && chainId) { chainIds.push(chainId) unsub() From dcbb13f6e5d949fb1a967f9002dd49f55d4cf58c Mon Sep 17 00:00:00 2001 From: Jordan Ribbink Date: Wed, 5 Feb 2025 15:11:53 -0800 Subject: [PATCH 9/9] Fix tests --- .../src/network/network-manager.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/fcl-ethereum-provider/src/network/network-manager.test.ts b/packages/fcl-ethereum-provider/src/network/network-manager.test.ts index cc76d9841..e432c9b97 100644 --- a/packages/fcl-ethereum-provider/src/network/network-manager.test.ts +++ b/packages/fcl-ethereum-provider/src/network/network-manager.test.ts @@ -105,21 +105,24 @@ describe("network manager", () => { expect(error).toBeFalsy() if (!isLoading && chainId) { chainIds.push(chainId) - unsub() } }) + jest.mocked(fcl.getChainId).mockResolvedValue("testnet") await config.set({ "accessNode.api": "https://example2.com", }) + jest.mocked(fcl.getChainId).mockResolvedValue("mainnet") await config.set({ "accessNode.api": "https://example3.com", }) + await new Promise(resolve => setTimeout(resolve, 100)) unsub() - expect(chainIds).toEqual([747, 747, 747]) + expect(fcl.getChainId).toHaveBeenCalledTimes(3) + expect(chainIds).toEqual([747, 646, 747]) expect(fcl.getChainId).toHaveBeenCalledTimes(3) })