Skip to content

Commit

Permalink
Merge pull request #8729 from LedgerHQ/bugfix/dsdk-620
Browse files Browse the repository at this point in the history
[DSDK-620] Fix LLD crash (race condition in transport registration)
  • Loading branch information
ofreyssinet-ledger authored Jan 6, 2025
2 parents 8cd1ab1 + 9baded6 commit b23530c
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 75 deletions.
5 changes: 5 additions & 0 deletions .changeset/calm-waves-battle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"ledger-live-desktop": patch
---

Fix transport registration crash
5 changes: 5 additions & 0 deletions .changeset/real-eagles-scream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@ledgerhq/live-common": patch
---

Firebase: fix typing of getFeature
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import isEqual from "lodash/isEqual";
import { useDispatch, useSelector } from "react-redux";
import { FeatureFlagsProvider, isFeature } from "@ledgerhq/live-common/featureFlags/index";
import type { FirebaseFeatureFlagsProviderProps as Props } from "@ledgerhq/live-common/featureFlags/index";
import { Feature, FeatureId, Features } from "@ledgerhq/types-live";
import { Feature, FeatureId } from "@ledgerhq/types-live";
import { useFirebaseRemoteConfig } from "./FirebaseRemoteConfig";
import { overriddenFeatureFlagsSelector } from "../reducers/settings";
import { setOverriddenFeatureFlag, setOverriddenFeatureFlags } from "../actions/settings";
Expand Down Expand Up @@ -41,7 +41,7 @@ export const FirebaseFeatureFlagsProvider = ({ children, getFeature }: Props): J
}, [dispatch]);

const wrappedGetFeature = useCallback(
<T extends FeatureId>(key: T): Features[T] => getFeature({ key, localOverrides }),
<T extends FeatureId>(key: T) => getFeature({ key, localOverrides }),
[getFeature, localOverrides],
);

Expand Down
142 changes: 85 additions & 57 deletions apps/ledger-live-desktop/src/renderer/live-common-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,93 +10,121 @@ import { getUserId } from "~/helpers/user";
import { setEnvOnAllThreads } from "./../helpers/env";
import { IPCTransport } from "./IPCTransport";
import logger from "./logger";
import { currentMode, setDeviceMode } from "@ledgerhq/live-common/hw/actions/app";
import { setDeviceMode } from "@ledgerhq/live-common/hw/actions/app";
import { getFeature } from "@ledgerhq/live-common/featureFlags/index";
import { FeatureId } from "@ledgerhq/types-live";
import { overriddenFeatureFlagsSelector } from "~/renderer/reducers/settings";
import { State } from "./reducers";
import { DeviceManagementKitTransport } from "@ledgerhq/live-dmk";
import { getEnv } from "@ledgerhq/live-env";

interface Store {
getState: () => State;
}

const getFeatureWithOverrides = (key: FeatureId, store: Store) => {
const isDeviceManagementKitEnabled = (store: Store) => {
const state = store.getState();
const localOverrides = overriddenFeatureFlagsSelector(state);
return getFeature({ key, localOverrides });
return getFeature({ key: "ldmkTransport", localOverrides })?.enabled;
};

enum RendererTransportModule {
DeviceManagementKit,
IPC,
Vault,
}

/**
* Register transport modules for the renderer process.
*
* NB: the order of the transport modules is important.
* Whenever calling `withDevice` in the renderer process, the first registered transport
* module that will return a truthy value from `open()` will be used.
*
* This logic allows all transports to be registered at initialization time,
* and then depending on a set of conditions, the right transport will be used.
*/
export function registerTransportModules(store: Store) {
setEnvOnAllThreads("USER_ID", getUserId());
const vaultTransportPrefixID = "vault-transport:";
const ldmkFeatureFlag = getFeatureWithOverrides("ldmkTransport", store);

listenLogs(({ id, date, ...log }) => {
if (log.type === "hid-frame") return;
logger.debug(log);
});

if (ldmkFeatureFlag.enabled) {
registerTransportModule({
id: "sdk",
open: (_id: string, timeoutMs?: number, context?: TraceContext) => {
trace({
type: "renderer-setup",
message: "Open called on registered module",
data: {
transport: "SDKTransport",
timeoutMs,
},
context: {
openContext: context,
},
});
return DeviceManagementKitTransport.open();
},
function whichTransportModuleToUse(deviceId: string): RendererTransportModule {
if (deviceId.startsWith(vaultTransportPrefixID)) return RendererTransportModule.Vault;
if (getEnv("SPECULOS_API_PORT")) return RendererTransportModule.IPC;
if (getEnv("DEVICE_PROXY_URL")) return RendererTransportModule.IPC;
if (isDeviceManagementKitEnabled(store)) return RendererTransportModule.DeviceManagementKit;
return RendererTransportModule.IPC;
}

disconnect: () => Promise.resolve(),
});
} else {
// Register IPC Transport Module
registerTransportModule({
id: "ipc",
open: (id: string, timeoutMs?: number, context?: TraceContext) => {
const originalDeviceMode = currentMode;
// id could be another type of transport such as vault-transport
if (id.startsWith(vaultTransportPrefixID)) return;
/**
* DeviceManagementKit Transport Module.
* It only supports regular USB devices.
*/
registerTransportModule({
id: "deviceManagementKitTransport",
open: (id: string, timeoutMs?: number, context?: TraceContext) => {
if (whichTransportModuleToUse(id) !== RendererTransportModule.DeviceManagementKit) return;

if (originalDeviceMode !== currentMode) {
setDeviceMode(originalDeviceMode);
}
trace({
type: "renderer-setup",
message: "Open called on registered module",
data: {
transport: "DeviceManagementKitTransport",
timeoutMs,
},
context: {
openContext: context,
},
});

trace({
type: "renderer-setup",
message: "Open called on registered module",
data: {
transport: "IPCTransport",
timeoutMs,
},
context: {
openContext: context,
},
});
return DeviceManagementKitTransport.open();
},

// Retries in the `renderer` process if the open failed. No retry is done in the `internal` process to avoid multiplying retries.
return retry(() => IPCTransport.open(id, timeoutMs, context), {
interval: 500,
maxRetry: 4,
});
},
disconnect: () => Promise.resolve(),
});
}
disconnect: () => Promise.resolve(),
});

/**
* IPC Transport Module.
* It acts as a bridge with transports registered in the internal process:
* Node HID as well as HTTP transports (Speculos & Proxy).
*/
registerTransportModule({
id: "ipc",
open: (id: string, timeoutMs?: number, context?: TraceContext) => {
if (whichTransportModuleToUse(id) !== RendererTransportModule.IPC) return;

trace({
type: "renderer-setup",
message: "Open called on registered module",
data: {
transport: "IPCTransport",
timeoutMs,
},
context: {
openContext: context,
},
});

// Retries in the `renderer` process if the open failed. No retry is done in the `internal` process to avoid multiplying retries.
return retry(() => IPCTransport.open(id, timeoutMs, context), {
interval: 500,
maxRetry: 4,
});
},
disconnect: () => Promise.resolve(),
});

// Register Vault Transport Module
/**
* Vault Transport Module.
*/
registerTransportModule({
id: "vault-transport",
open: (id: string) => {
if (!id.startsWith(vaultTransportPrefixID)) return;
if (whichTransportModuleToUse(id) !== RendererTransportModule.Vault) return;
setDeviceMode("polling");
const params = new URLSearchParams(id.split(vaultTransportPrefixID)[1]);
return retry(() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
getFeature as getFeatureFlag,
} from "@ledgerhq/live-common/featureFlags/index";
import type { FirebaseFeatureFlagsProviderProps } from "@ledgerhq/live-common/featureFlags/index";
import { FeatureId, Feature, Features } from "@ledgerhq/types-live";
import { FeatureId, Feature } from "@ledgerhq/types-live";
import { overriddenFeatureFlagsSelector } from "~/reducers/settings";
import { setOverriddenFeatureFlag, setOverriddenFeatureFlags } from "~/actions/settings";
import { setAnalyticsFeatureFlagMethod } from "~/analytics/segment";
Expand Down Expand Up @@ -70,8 +70,7 @@ export const FirebaseFeatureFlagsProvider = ({
}, [dispatch]);

const wrappedGetFeature = useCallback(
<T extends FeatureId>(key: T): Features[T] =>
getFeature({ key, appLanguage: language, localOverrides }),
<T extends FeatureId>(key: T) => getFeature({ key, appLanguage: language, localOverrides }),
[localOverrides, language, getFeature],
);

Expand Down
3 changes: 2 additions & 1 deletion libs/ledger-live-common/src/e2e/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ export const getAllFeatureFlags = (
const res: Partial<{ [key in FeatureId]: Feature }> = {};
Object.keys(DEFAULT_FEATURES).forEach(k => {
const key = k as keyof typeof DEFAULT_FEATURES;
res[key] = getFeature({ key, appLanguage });
const value = getFeature({ key, appLanguage });
if (value !== null) res[key] = value;
});
return res;
};
Expand Down
21 changes: 9 additions & 12 deletions libs/ledger-live-common/src/featureFlags/firebaseFeatureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import { Feature, FeatureId, Features } from "@ledgerhq/types-live";
import { getEnv } from "@ledgerhq/live-env";
import { LiveConfig } from "@ledgerhq/live-config/LiveConfig";

type GetFeature = <T extends FeatureId>(param: {
key: T;
appLanguage?: string;
allowOverride?: boolean;
localOverrides?: { [key in FeatureId]?: Feature | undefined };
}) => Features[T] | null;

export interface FirebaseFeatureFlagsProviderProps {
getFeature: <T extends FeatureId>(param: {
key: T;
appLanguage?: string;
allowOverride?: boolean;
localOverrides?: { [key in FeatureId]?: Feature | undefined };
}) => Features[T];
getFeature: GetFeature;
children: React.ReactNode;
}

Expand Down Expand Up @@ -63,12 +65,7 @@ export const isFeature = (key: string): boolean => {
}
};

export const getFeature = (args: {
key: FeatureId;
appLanguage?: string;
localOverrides?: { [key in FeatureId]?: Feature };
allowOverride?: boolean;
}) => {
export const getFeature: GetFeature = args => {
if (!LiveConfig.instance?.provider?.getValueByKey) {
return null;
}
Expand Down

0 comments on commit b23530c

Please sign in to comment.