Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,50 @@ describe('HardwareWalletsSwaps', () => {
await waitFor(() => {
expect(mockSubmitBridgeTx).toHaveBeenCalledWith(MOCK_SUBMISSION_PARAMS);
});
expect(mockEnsureDeviceReady).not.toHaveBeenCalled();
// Bridge flow now wires the DMK session before submitting, mirroring
// submitSendFlow. ensureDeviceReady must run AND must happen before
// submitBridgeTx so the keyring has a live session when it signs.
expect(mockEnsureDeviceReady).toHaveBeenCalledTimes(1);
const [ensureCallOrder] = mockEnsureDeviceReady.mock.invocationCallOrder;
const [submitCallOrder] = mockSubmitBridgeTx.mock.invocationCallOrder;
expect(submitCallOrder).toBeGreaterThan(ensureCallOrder);
});

it('awaits ensureDeviceReady before calling submitBridgeTx', async () => {
// Block ensureDeviceReady until we release it, so we can prove
// submitBridgeTx is gated on its resolution rather than racing.
let releaseDeviceReady: () => void = () => undefined;
mockEnsureDeviceReady.mockImplementationOnce(
() =>
new Promise<boolean>((resolve) => {
releaseDeviceReady = () => resolve(true);
}),
);

renderScreen({});

// ensureDeviceReady has been invoked but is still pending; submitBridgeTx
// must not have fired yet.
await waitFor(() => {
expect(mockEnsureDeviceReady).toHaveBeenCalledTimes(1);
});
expect(mockSubmitBridgeTx).not.toHaveBeenCalled();

releaseDeviceReady();
await waitFor(() => {
expect(mockSubmitBridgeTx).toHaveBeenCalledWith(MOCK_SUBMISSION_PARAMS);
});
});

it('dispatches TRANSACTION_FAILED and skips submitBridgeTx when ensureDeviceReady returns false', async () => {
mockEnsureDeviceReady.mockResolvedValueOnce(false);

const { store } = renderScreen({});

await waitFor(() => {
expect(getBridgeStatus(store)).toBe(HardwareWalletsSwapsStatus.Failed);
});
expect(mockSubmitBridgeTx).not.toHaveBeenCalled();
});

it('does not resubmit after initial submission', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ interface UseHardwareWalletSubmitOptions {
approvalRequestId?: string;
submissionParams?: SubmissionParams;
ensureDeviceReady?: (deviceId?: string | null) => Promise<boolean>;
setPendingOperationAddress?: (address: string | null) => void;
setPendingOperationAddress: (address: string | null) => void;
}

/**
Expand Down Expand Up @@ -207,7 +207,7 @@ export function useHardwareWalletSubmit({
}

await runSubmit(async () => {
setPendingOperationAddress?.(walletAddress);
setPendingOperationAddress(walletAddress);
try {
const deviceId = await getDeviceIdForAddress(walletAddress);
const isReady = await ensureDeviceReady?.(deviceId);
Expand Down Expand Up @@ -242,7 +242,7 @@ export function useHardwareWalletSubmit({
await retrySendTransaction(currentPreparedTxMeta);
}
} finally {
setPendingOperationAddress?.(null);
setPendingOperationAddress(null);
}
});
}, [
Expand All @@ -253,7 +253,14 @@ export function useHardwareWalletSubmit({
setPendingOperationAddress,
]);

// ── Bridge flow (UNCHANGED) ─────────────────────────────────────────
// ── Bridge flow ────────────────────────────────────────────────────
// Wire the DMK session before submitting. BridgeStatusController signs
// the EIP-712 intent directly via KeyringController (intent-strategy.mjs:
// signTypedMessage → KeyringController:signTypedMessage), which bypasses
// the confirmation pipeline. Without this readiness gate the keyring signs
// cold and throws "Session ID not set". Mirrors submitSendFlow. For
// approval-tx quotes the controller's own requireApproval flow still runs,
// hitting ensureDeviceReady's fast path (already connected) — no conflict.
const submitBridgeFlow = useCallback(async () => {
const cachedParams = cachedSubmissionParams.current;
if (!cachedParams || !walletAddress) {
Expand All @@ -265,8 +272,31 @@ export function useHardwareWalletSubmit({
return;
}

await runSubmit(() => submitBridgeTxRef.current(cachedParams));
}, [dispatch, walletAddress, runSubmit]);
await runSubmit(async () => {
setPendingOperationAddress(walletAddress);
try {
const deviceId = await getDeviceIdForAddress(walletAddress);
const isReady = await ensureDeviceReady?.(deviceId);
if (!isReady) {
dispatch(
updateHardwareWalletsSwaps({
type: HardwareWalletsSwapsEventType.TransactionFailed,
}),
);
return;
}
await submitBridgeTxRef.current(cachedParams);
} finally {
setPendingOperationAddress(null);
}
});
}, [
dispatch,
walletAddress,
runSubmit,
ensureDeviceReady,
setPendingOperationAddress,
]);

const submit = useCallback(async () => {
if (isSendFlow) {
Expand Down
18 changes: 17 additions & 1 deletion app/core/Engine/wallet-init/initialization.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Wallet, type WalletOptions } from '@metamask/wallet';
import { RootMessenger } from '../types';
import { isDmkEnabled } from '../../Ledger/dmk';
import { getApprovalControllerInstanceOptions } from './instance-options/approval-controller';
import { getKeyringControllerInstanceOptions } from './instance-options/keyring-controller';
import { getRemoteFeatureFlagControllerInstanceOptions } from './instance-options/remote-feature-flag-controller';
Expand All @@ -21,13 +22,28 @@ export function initializeWallet({
messenger: RootMessenger;
state: NonNullable<WalletOptions['state']>;
}) {
// DMK stack selection. Read the ledgerDmk flag fresh from the persisted
// RemoteFeatureFlagController state (LEDGER_FORCE_DMK env var overrides).
// No caching — the adapter factory reads the same flag from live state.
const remoteFeatureFlagState = (state as Record<string, unknown>)
?.RemoteFeatureFlagController as
| {
remoteFeatureFlags?: Record<string, unknown>;
localOverrides?: Record<string, unknown>;
}
| undefined;
const useDmk = isDmkEnabled({
...(remoteFeatureFlagState?.remoteFeatureFlags ?? {}),
...(remoteFeatureFlagState?.localOverrides ?? {}),
});

const wallet = new Wallet({
messenger,
state,
instanceOptions: {
approvalController: getApprovalControllerInstanceOptions(),
connectivityController: getConnectivityControllerInstanceOptions(),
keyringController: getKeyringControllerInstanceOptions(messenger),
keyringController: getKeyringControllerInstanceOptions(messenger, useDmk),
networkController: getNetworkControllerInstanceOptions(),
remoteFeatureFlagController:
getRemoteFeatureFlagControllerInstanceOptions({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ describe('getKeyringControllerInstanceOptions', () => {
});

it('builds options with the encryptor and V1/V2 keyring builders', () => {
const options = getKeyringControllerInstanceOptions(messenger);
const options = getKeyringControllerInstanceOptions(messenger, false);

expect(Encryptor).toHaveBeenCalledWith({
keyDerivationOptions: { algorithm: 'legacy' },
});
expect(getKeyringBuilders).toHaveBeenCalledWith(messenger);
expect(getKeyringBuilders).toHaveBeenCalledWith(messenger, false);
expect(getKeyringV2Builders).toHaveBeenCalled();
expect(options).toEqual({
encryptor: { name: 'mock-encryptor' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,22 @@ type KeyringControllerInstanceOptions = NonNullable<
/**
* @param messenger - Needed to build the V1 keyring builders, some of which
* interact with the shared bus.
* TODO: Remove this parameter when we remove the DMK feature flag.
* @param useDmk - Resolved DMK flag, threaded into the keyring builders so the
* Ledger bridge choice agrees with the adapter choice.
*/
export function getKeyringControllerInstanceOptions(
messenger: RootMessenger,
// TODO: Remove this parameter when we remove the DMK feature flag.
useDmk: boolean,
): KeyringControllerInstanceOptions {
const encryptor = new Encryptor({
keyDerivationOptions: LEGACY_DERIVATION_OPTIONS,
});

return {
encryptor,
keyringBuilders: getKeyringBuilders(messenger),
keyringBuilders: getKeyringBuilders(messenger, useDmk),
keyringV2Builders: getKeyringV2Builders(),
};
}
8 changes: 4 additions & 4 deletions app/core/Engine/wallet-init/keyrings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('wallet-init/keyrings', () => {

describe('getKeyringBuilders', () => {
it('registers QR, Ledger, HD, Money, and Snap builders by type', () => {
const builders = getKeyringBuilders(getRootMessenger()) ?? [];
const builders = getKeyringBuilders(getRootMessenger(), false) ?? [];
const types = builders.map((b) => b.type);

expect(types).toEqual(
Expand All @@ -68,7 +68,7 @@ describe('wallet-init/keyrings', () => {
});

it('constructs the right keyring instance for each type', () => {
const builders = getKeyringBuilders(getRootMessenger()) ?? [];
const builders = getKeyringBuilders(getRootMessenger(), false) ?? [];
const byType = Object.fromEntries(builders.map((b) => [b.type, b]));

expect(byType[LegacyQrKeyring.type]()).toBeInstanceOf(LegacyQrKeyring);
Expand Down Expand Up @@ -112,7 +112,7 @@ describe('wallet-init/keyrings', () => {
handler,
);

const builders = getKeyringBuilders(messenger) ?? [];
const builders = getKeyringBuilders(messenger, false) ?? [];
const moneyBuilder = builders.find(
(b) => b.type === LegacyMoneyKeyring.type,
);
Expand Down Expand Up @@ -149,7 +149,7 @@ describe('wallet-init/keyrings', () => {
}),
);

const builders = getKeyringBuilders(messenger) ?? [];
const builders = getKeyringBuilders(messenger, false) ?? [];
const moneyBuilder = builders.find(
(b) => b.type === LegacyMoneyKeyring.type,
);
Expand Down
17 changes: 16 additions & 1 deletion app/core/Engine/wallet-init/keyrings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
} from '@metamask/eth-qr-keyring';
import { QrKeyring as QrKeyringV2 } from '@metamask/eth-qr-keyring/v2';
import {
LedgerDmkBridge,
LedgerKeyring,
LedgerMobileBridge,
LedgerTransportMiddleware,
Expand All @@ -24,13 +25,15 @@ import { MoneyKeyring } from '@metamask/eth-money-keyring';
import { MoneyKeyring as MoneyKeyringV2 } from '@metamask/eth-money-keyring/v2';
import { hmacSha512 } from '@metamask/native-utils';
import { pbkdf2 } from '../../Encryptor';
import Logger from '../../../util/Logger';
import { getLegacySnapKeyringBuilderMessenger } from '../messengers/accounts/snap-keyring-builder-messenger';
import { getSnapKeyringV2BuilderMessenger } from '../messengers/accounts/snap-keyring-v2-builder-messenger';
import { store } from '../../../store';
import {
scanCompleted,
scanRequested,
} from '../../redux/slices/qrKeyringScanner';
import { RNBleTransportFactory } from '@ledgerhq/device-transport-kit-react-native-ble';
import {
snapKeyringV2AdaptedAsV1Builder,
snapKeyringV2Builder,
Expand All @@ -53,10 +56,17 @@ export const qrKeyringBridge = new QrKeyringDeferredPromiseBridge({
/**
* Build the list of keyring builders.
*
* @param messenger - Needed by some builders that interact with the shared bus.
* TODO: Remove this parameter when we remove the DMK feature flag.
* @param useDmk - Whether to use the DMK Ledger bridge for Ledger keyrings.
* Read fresh from feature-flag state via `isDmkEnabled(flags)` (the `ledgerDmk`
* flag) at engine init; the adapter factory reads the same flag in
* `useAdapterLifecycle`.
* @returns The keyring builders to register with the `KeyringController`.
*/
export function getKeyringBuilders(
messenger: RootMessenger,
useDmk: boolean,
): KeyringControllerOptions['keyringBuilders'] {
// Required by the HD keyring and money keyring to use native crypto functions.
const cryptographicFunctions: CryptographicFunctions = {
Expand All @@ -78,7 +88,12 @@ export function getKeyringBuilders(

keyrings.push(qrKeyringBuilder);

const bridge = new LedgerMobileBridge(new LedgerTransportMiddleware());
const bridge = useDmk
? new LedgerDmkBridge({ transportFactory: RNBleTransportFactory })
: new LedgerMobileBridge(new LedgerTransportMiddleware());
Logger.log(
`[Ledger] Using ${useDmk ? 'LedgerDmkBridge' : 'LedgerMobileBridge'}`,
);
const ledgerKeyringBuilder = () => new LedgerKeyring({ bridge });
ledgerKeyringBuilder.type = LedgerKeyring.type;

Expand Down
Loading
Loading