Skip to content

Commit

Permalink
fix: expect disconnects during remote api method call
Browse files Browse the repository at this point in the history 
it happens due to unknown reason, possibly due to the
recently introduced BFCache feature in chromium)

expect it to re-connect within 10s, as normally it takes around 1s

also log runtime.lastError when port disconnects

BREAKING CHANGE: change return type of createWalletApi callbacks
  • Loading branch information
@mkazlauskas
mkazlauskas committed Sep 6, 2024
1 parent 85c38e2 commit 1171fed
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 51 deletions.
5 changes: 3 additions & 2 deletions packages/e2e/test/web-extension/extension/background/cip30.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { cip30 } from '@cardano-sdk/web-extension';
import { runtime } from 'webextension-polyfill';
import { cip30 as walletCip30 } from '@cardano-sdk/wallet';

import { NEVER } from 'rxjs';
import { authenticator } from './authenticator';
import { logger } from '../util';
import { wallet$ } from './walletManager';
Expand All @@ -12,12 +13,12 @@ const confirmationCallback: walletCip30.CallbackConfirmation = {
signData: async ({ sender }) => {
if (!sender) throw new Error('No sender context');
logger.info('signData request from', sender);
return true;
return { cancel$: NEVER };
},
signTx: async ({ sender }) => {
if (!sender) throw new Error('No sender context');
logger.info('signTx request', sender);
return true;
return { cancel$: NEVER };
},
submitTx: async () => true
};
Expand Down
62 changes: 38 additions & 24 deletions packages/wallet/src/cip30.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { HexBlob, ManagedFreeableScope } from '@cardano-sdk/util';
import { InputSelectionError, InputSelectionFailure } from '@cardano-sdk/input-selection';
import { Logger } from 'ts-log';
import { MessageSender } from '@cardano-sdk/key-management';
import { Observable, firstValueFrom, map } from 'rxjs';
import { Observable, firstValueFrom, from, map, mergeMap, race, throwError } from 'rxjs';
import { ObservableWallet, isKeyHashAddress, isScriptAddress } from './types';
import { requiresForeignSignatures } from './services';
import uniq from 'lodash/uniq.js';
Expand Down Expand Up @@ -70,14 +70,20 @@ export type GetCollateralCallbackParams = {

type GetCollateralCallback = (args: GetCollateralCallbackParams) => Promise<Cardano.Utxo[]>;

export type SignConfirmationOk = { cancel$: Observable<void> };
export type SignConfirmationResult = SignConfirmationOk | false;

const signOrCancel = <T>(result: Promise<T>, { cancel$ }: SignConfirmationOk, createError: () => Error) =>
firstValueFrom(race(from(result), cancel$.pipe(mergeMap(() => throwError(createError)))));

export type CallbackConfirmation = {
signData: (args: SignDataCallbackParams) => Promise<boolean>;
signTx: (args: SignTxCallbackParams) => Promise<boolean>;
signData: (args: SignDataCallbackParams) => Promise<SignConfirmationResult>;
signTx: (args: SignTxCallbackParams) => Promise<SignConfirmationResult>;
submitTx: (args: SubmitTxCallbackParams) => Promise<boolean>;
getCollateral?: GetCollateralCallback;
};

const mapCallbackFailure = (err: unknown, logger: Logger) => {
const mapCallbackFailure = (err: unknown, logger: Logger): false => {
logger.error(err);
return false;
};
Expand Down Expand Up @@ -444,7 +450,7 @@ const baseCip30WalletApi = (
const hexBlobPayload = HexBlob(payload);
const signWith = Cardano.DRepID.isValid(addr) ? Cardano.DRepID(addr) : Cardano.PaymentAddress(addr);

const shouldProceed = await confirmationCallback
const confirmationResult = await confirmationCallback
.signData({
data: {
addr: signWith,
Expand All @@ -455,13 +461,17 @@ const baseCip30WalletApi = (
})
.catch((error) => mapCallbackFailure(error, logger));

if (shouldProceed) {
if (confirmationResult) {
const wallet = await firstValueFrom(wallet$);
return wallet.signData({
payload: hexBlobPayload,
sender,
signWith
});
return signOrCancel(
wallet.signData({
payload: hexBlobPayload,
sender,
signWith
}),
confirmationResult,
() => new DataSignError(DataSignErrorCode.UserDeclined, 'user declined signing')
);
}
logger.debug('sign data declined');
throw new DataSignError(DataSignErrorCode.UserDeclined, 'user declined signing');
Expand All @@ -478,44 +488,48 @@ const baseCip30WalletApi = (

// If partialSign is false and the wallet could not sign the entire transaction
if (!partialSign && needsForeignSignature)
throw new DataSignError(
DataSignErrorCode.ProofGeneration,
throw new TxSignError(
TxSignErrorCode.ProofGeneration,
'The wallet does not have the secret key associated with some of the inputs or certificates.'
);

const shouldProceed = await confirmationCallback
const confirmationResult = await confirmationCallback
.signTx({
data: coreTx,
sender,
type: Cip30ConfirmationCallbackType.SignTx
})
.catch((error) => mapCallbackFailure(error, logger));
if (shouldProceed) {
if (confirmationResult) {
try {
const {
witness: { signatures }
} = await wallet.finalizeTx({
bodyCbor: txDecoded.body().toCbor(),
signingContext: { sender },
tx: { ...coreTx, hash }
});
} = await signOrCancel(
wallet.finalizeTx({
bodyCbor: txDecoded.body().toCbor(),
signingContext: { sender },
tx: { ...coreTx, hash }
}),
confirmationResult,
() => new TxSignError(TxSignErrorCode.UserDeclined, 'user declined signing tx')
);

// If partialSign is true, the wallet only tries to sign what it can. However, if
// signatures size is 0 then throw.
if (partialSign && signatures.size === 0) {
throw new DataSignError(
DataSignErrorCode.ProofGeneration,
throw new TxSignError(
TxSignErrorCode.ProofGeneration,
'The wallet does not have the secret key associated with any of the inputs and certificates.'
);
}

const cbor = Serialization.TransactionWitnessSet.fromCore({ signatures }).toCbor();
return Promise.resolve(cbor);
} catch (error) {
logger.error(error);
if (error instanceof DataSignError) {
if (error instanceof TxSignError) {
throw error;
} else {
logger.error(error);
const message = formatUnknownError(error);
throw new TxSignError(TxSignErrorCode.UserDeclined, message);
}
Expand Down
54 changes: 41 additions & 13 deletions packages/wallet/test/integration/cip30mapping.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
SenderContext,
TxSendError,
TxSignError,
TxSignErrorCode,
WalletApi,
WithSenderContext
} from '@cardano-sdk/dapp-connector';
Expand All @@ -29,10 +30,10 @@ import {
import { HexBlob, ManagedFreeableScope } from '@cardano-sdk/util';
import { InMemoryUnspendableUtxoStore, createInMemoryWalletStores } from '../../src/persistence';
import { InitializeTxProps, InitializeTxResult } from '@cardano-sdk/tx-construction';
import { NEVER, firstValueFrom, of } from 'rxjs';
import { Providers, createWallet } from './util';
import { address_0_0, address_1_0, rewardAccount_0, rewardAccount_1 } from '../services/ChangeAddress/testData';
import { buildDRepIDFromDRepKey, signTx, waitForWalletStateSettle } from '../util';
import { firstValueFrom, of } from 'rxjs';
import { dummyLogger as logger } from 'ts-log';
import { stakeKeyDerivationPath, testAsyncKeyAgent } from '../../../key-management/test/mocks';
import uniq from 'lodash/uniq.js';
Expand All @@ -50,7 +51,7 @@ const {

type TestProviders = Required<Pick<Providers, 'txSubmitProvider' | 'networkInfoProvider'>>;
const mockCollateralCallback = jest.fn().mockResolvedValue([mockUtxo[3]]);
const createMockGenericCallback = () => jest.fn().mockResolvedValue(true);
const createMockGenericCallback = <T>(result: T) => jest.fn().mockResolvedValue(result);
const foreignTx = Serialization.TxCBOR(
'84a70081825820dce442e983f3f5cd5b2644bc57f749075390f1fbae9ab55bf454342959c885db00018182583900d161d64eef0eeb59f9124f520f8c8f3b717ed04198d54c8b17e604aea63c153fb3ea8a4ea4f165574ea91173756de0bf30222ca0e95a649a1a0082607b021a0016360509a1581cb77934706fa311b6568d1070c2d23f092324b35ad623aa571a0e3726a14e4d6573685f476966745f43617264200b5820d8175f3b1276a48939a6ccee220a7f81b6422167317ba3ff6325cba1fb6ccbe70d818258208d68748457cd0f1a8596f41fd2125a415315897d2da4a4b94335829cee7198ae001281825820dce442e983f3f5cd5b2644bc57f749075390f1fbae9ab55bf454342959c885db00a2068259016b590168010000333232323232323223223222253330083232533300d3010002132533300b3370e6eb4c034009200113371e0020122940dd718058008b180700099299980499b8748008c028dd50008a5eb7bdb1804dd5980718059baa001323300100132330010013756601e602060206020602060186ea8c03cc030dd50019129998070008a5eb7bdb1804c8c8c8c94ccc03ccdc8a45000021533300f3371e91010000210031005133013337606ea4008dd3000998030030019bab3010003375c601c0046024004602000244a66601a002298103d87a8000132323232533300e337220140042a66601c66e3c0280084cdd2a4000660246e980052f5c02980103d87a80001330060060033756601e0066eb8c034008c044008c03c00452613656375c0026eb80055cd2ab9d5573caae7d5d02ba157449810f4e4d6573685f476966745f43617264004c011e581cb77934706fa311b6568d1070c2d23f092324b35ad623aa571a0e3726000159023c59023901000033323232323232322322232323225333009323232533300c3007300d3754002264646464a666026602c00426464a666024601a60266ea803854ccc048c034c04cdd5191980080080311299980b8008a60103d87a80001323253330163375e603660306ea800804c4cdd2a40006603400497ae0133004004001301b002301900115333012300c00113371e00402029405854ccc048cdc3800a4002266e3c0080405281bad3013002375c60220022c602800264a66601e601260206ea800452f5bded8c026eacc050c044dd500099191980080099198008009bab3016301730173017301700522533301500114bd6f7b630099191919299980b19b91488100002153330163371e9101000021003100513301a337606ea4008dd3000998030030019bab3017003375c602a0046032004602e00244a666028002298103d87a800013232323253330153372200e0042a66602a66e3c01c0084cdd2a4000660326e980052f5c02980103d87a80001330060060033756602c0066eb8c050008c060008c058004dd7180998081baa00337586024002601c6ea800858c040c044008c03c004c02cdd50008a4c26cac64a66601060060022a66601660146ea8010526161533300830020011533300b300a37540082930b0b18041baa003370e90011b8748000dd7000ab9a5573aaae7955cfaba05742ae8930010f4e4d6573685f476966745f43617264004c012bd8799fd8799f58203159a6f2ae24c5bfbed947fe0ecfe936f088c8d265484e6979cacb607d33c811ff05ff0001058284000040821a006acfc01ab2d05e00840100d87a80821a006acfc01ab2d05e00f5f6'
);
Expand All @@ -74,9 +75,9 @@ const createWalletAndApiWithStores = async (
wallet.utxo.available$ = of(availableUtxos);
}
const confirmationCallback = {
signData: createMockGenericCallback(),
signTx: createMockGenericCallback(),
submitTx: createMockGenericCallback(),
signData: createMockGenericCallback({ cancel$: NEVER }),
signTx: createMockGenericCallback({ cancel$: NEVER }),
submitTx: createMockGenericCallback(true),
...(!!getCollateralCallback && { getCollateral: getCollateralCallback })
};
wallet.governance.getPubDRepKey = jest.fn(wallet.governance.getPubDRepKey);
Expand Down Expand Up @@ -613,12 +614,25 @@ describe('cip30', () => {

it('doesnt invoke confirmationCallback.signTx if an error occurs', async () => {
const finalizeTxSpy = jest.spyOn(wallet, 'finalizeTx').mockClear();
confirmationCallback.signTx = jest.fn().mockResolvedValueOnce(true).mockClear();
confirmationCallback.signTx = jest.fn().mockResolvedValueOnce({ cancel$: NEVER }).mockClear();

await expect(api.signTx(context, foreignTx, false)).rejects.toThrowError();
expect(finalizeTxSpy).not.toHaveBeenCalled();
expect(confirmationCallback.signTx).not.toHaveBeenCalled();
});

it('rejects with UserDeclined error if cancel$ emits before finalizeTx resolves', async () => {
jest.spyOn(wallet, 'finalizeTx').mockResolvedValueOnce(
new Promise(() => {
// never resolves or rejects
})
);
confirmationCallback.signTx = jest.fn().mockResolvedValueOnce({ cancel$: of(void 0) });

await expect(api.signTx(context, hexTx)).rejects.toThrowError(
expect.objectContaining({ code: TxSignErrorCode.UserDeclined })
);
});
});

describe('api.signData', () => {
Expand Down Expand Up @@ -661,6 +675,20 @@ describe('cip30', () => {
);
expect(confirmationCallback.signData).toBeCalledWith(expect.objectContaining({ sender: context.sender }));
});

it('rejects with UserDeclined error if cancel$ emits before finalizeTx resolves', async () => {
const [{ address }] = await firstValueFrom(wallet.addresses$);
jest.spyOn(wallet, 'signData').mockResolvedValueOnce(
new Promise(() => {
// never resolves or rejects
})
);
confirmationCallback.signData = jest.fn().mockResolvedValueOnce({ cancel$: of(void 0) });

await expect(api.signData(context, address, HexBlob('abc123'))).rejects.toThrowError(
expect.objectContaining({ code: DataSignErrorCode.UserDeclined })
);
});
});

describe('api.submitTx', () => {
Expand Down Expand Up @@ -741,8 +769,8 @@ describe('cip30', () => {
describe('signData', () => {
const payload = 'abc123';

test('resolves true', async () => {
confirmationCallback.signData = jest.fn().mockResolvedValueOnce(true);
test('resolves ok', async () => {
confirmationCallback.signData = jest.fn().mockResolvedValueOnce({ cancel$: NEVER });
await expect(api.signData(context, address, payload)).resolves.not.toThrow();
});

Expand All @@ -757,7 +785,7 @@ describe('cip30', () => {
});

test('gets the Cardano.Address equivalent of the hex address', async () => {
confirmationCallback.signData = jest.fn().mockResolvedValueOnce(true);
confirmationCallback.signData = jest.fn().mockResolvedValueOnce({ cancel$: NEVER });

const hexAddr = Cardano.Address.fromBech32(address).toBytes();

Expand All @@ -776,8 +804,8 @@ describe('cip30', () => {
hexTx = Serialization.Transaction.fromCore(finalizedTx).toCbor();
});

test('resolves true', async () => {
confirmationCallback.signTx = jest.fn().mockResolvedValueOnce(true);
test('resolves ok', async () => {
confirmationCallback.signTx = jest.fn().mockResolvedValueOnce({ cancel$: NEVER });
await expect(api.signTx(context, hexTx)).resolves.not.toThrow();
});

Expand Down Expand Up @@ -869,8 +897,8 @@ describe('cip30', () => {
mockApi = cip30.createWalletApi(
of(mockWallet),
{
signData: jest.fn().mockResolvedValue(true),
signTx: jest.fn().mockResolvedValue(true),
signData: jest.fn().mockResolvedValue({ cancel$: NEVER }),
signTx: jest.fn().mockResolvedValue({ cancel$: NEVER }),
submitTx: jest.fn().mockResolvedValue(true)
},
{ logger }
Expand Down
3 changes: 3 additions & 0 deletions packages/web-extension/src/messaging/BackgroundMessenger.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ export const createBackgroundMessenger = ({ logger, runtime }: MessengerDependen
message$.next({ data, port });
};
const onPortDisconnected = (port: MessengerPort) => {
if (runtime.lastError) {
logger.warn(`[BackgroundMessenger(${port.name})] Last runtime error`, runtime.lastError);
}
port.onMessage.removeListener(onPortMessage);
port.onDisconnect.removeListener(onPortDisconnected);
const { ports$ } = channels.get(port.name)!;
Expand Down
3 changes: 3 additions & 0 deletions packages/web-extension/src/messaging/NonBackgroundMessenger.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ export const createNonBackgroundMessenger = (
message$.next({ data, port });
};
const onDisconnect = (port: MessengerPort) => {
if (runtime.lastError) {
logger.warn(`[NonBackgroundMessenger(${channel})] Last runtime error`, runtime.lastError);
}
disconnect$.next({
disconnected: port,
remaining: []
Expand Down
28 changes: 16 additions & 12 deletions packages/web-extension/src/messaging/remoteApi.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import {
import { CustomError } from 'ts-custom-error';
import {
EMPTY,
EmptyError,
NEVER,
Observable,
Subscription,
TeardownLogic,
concat,
defaultIfEmpty,
filter,
firstValueFrom,
from,
Expand All @@ -39,10 +39,10 @@ import {
switchMap,
takeUntil,
tap,
throwError
timeout
} from 'rxjs';
import { ErrorClass, Shutdown, fromSerializableObject, isPromise, toSerializableObject } from '@cardano-sdk/util';
import { NotImplementedError } from '@cardano-sdk/core';
import { Milliseconds, NotImplementedError } from '@cardano-sdk/core';
import { TrackerSubject } from '@cardano-sdk/util-rxjs';
import { WrongTargetError } from './errors';
import {
Expand All @@ -66,7 +66,7 @@ export class RemoteApiShutdownError extends CustomError {
const consumeMethod =
(
{ propName, errorTypes }: { propName: string; errorTypes?: ErrorClass[]; options?: MethodRequestOptions },
{ messenger: { message$, postMessage, channel, disconnect$ } }: MessengerApiDependencies
{ messenger: { message$, channel, postMessage, disconnect$, connect$ }, logger }: MessengerApiDependencies
) =>
async (...args: unknown[]) => {
const requestMessage: RequestMessage = {
Expand All @@ -87,17 +87,21 @@ const consumeMethod =
map(({ response }) => response),
filter((response) => !(response instanceof WrongTargetError))
),
// We might encounter unexpected disconnects between method call and response
disconnect$.pipe(
filter((dc) => dc.remaining.length === 0),
mergeMap(() => throwError(() => new EmptyError()))
tap(() => logger.warn(`API disconnected before "${propName}" resolved. Expecting reconnect.`)),
switchMap(() =>
connect$.pipe(
tap(() => logger.warn(`Reconnected. Waiting for "${propName}" response...`)),
// It usually reconnects in about 1 second. 10 should be more than enough to know that it won't re-connect.
timeout({ first: Milliseconds(10_000), with: () => of(new RemoteApiShutdownError(channel)) }),
mergeMap((value) => (value instanceof RemoteApiShutdownError ? of(value) : EMPTY))
)
)
)
)
).catch((error) => {
if (error instanceof EmptyError) {
throw new RemoteApiShutdownError(channel);
}
throw error;
});
).pipe(defaultIfEmpty(new RemoteApiShutdownError(channel)))
);

if (result instanceof Error) {
throw result;
Expand Down
1 change: 1 addition & 0 deletions packages/web-extension/src/messaging/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export interface MessengerPort {
export interface MinimalRuntime {
connect(connectInfo: Runtime.ConnectConnectInfoType): MessengerPort;
onConnect: MinimalEvent<(port: MessengerPort) => void>;
lastError?: unknown;
}

export interface MessengerDependencies {
Expand Down

0 comments on commit 1171fed

Please sign in to comment.