Skip to content

Commit 274b1c4

Browse files
benmabenma
committed
serialize calls
1 parent c6d0df8 commit 274b1c4

3 files changed

Lines changed: 215 additions & 84 deletions

File tree

‎src/index.ts‎

Lines changed: 59 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,7 @@ function makePairingBitBox(state: PairingState, close: () => void): PairingBitBo
480480
*/
481481
export class PairedBitBox {
482482
#state: PairedStateUnion = { kind: 'uninitialized' };
483+
#queue: Promise<void> = Promise.resolve();
483484

484485
/** @internal */
485486
constructor(init?: Omit<PairedOpen, 'kind'>) {
@@ -504,6 +505,43 @@ export class PairedBitBox {
504505
return state;
505506
}
506507

508+
/**
509+
* Serializes all device-touching public methods on this paired connection.
510+
*
511+
* The BitBox protocol and Noise channel are ordered streams, not multiplexed
512+
* request/response transports. Some public methods also perform multi-step
513+
* conversations (for example ETH streaming or anti-klepto signing), so the
514+
* lock must cover the whole public method, not only one encrypted query.
515+
*
516+
* Calls made after close fail before joining the queue, so they do not wait
517+
* behind a stuck transport read. The open state is checked again when the
518+
* queued operation starts, so calls queued before close still fail if they
519+
* did not already enter the device conversation.
520+
*
521+
* Each call chains onto the previous queue tail and then replaces the tail
522+
* with a settled `void` promise, so a rejected call does not poison later
523+
* queued operations.
524+
*/
525+
#runExclusive<T>(
526+
method: string,
527+
fn: (open: PairedOpen) => Promise<T>,
528+
): Promise<T> {
529+
this.#requireOpen(method);
530+
const run = this.#queue.catch(() => undefined).then(async () => {
531+
const open = this.#requireOpen(method);
532+
try {
533+
return await fn(open);
534+
} catch (err) {
535+
throw toPublicError(err);
536+
}
537+
});
538+
this.#queue = run.then(
539+
() => undefined,
540+
() => undefined,
541+
);
542+
return run;
543+
}
544+
507545
/** No-op; retained for ABI compatibility with the wasm-bindgen output. */
508546
free(): void {}
509547

@@ -524,12 +562,7 @@ export class PairedBitBox {
524562

525563
/** Query device metadata. */
526564
async deviceInfo(): Promise<DeviceInfo> {
527-
const open = this.#requireOpen('deviceInfo');
528-
try {
529-
return await deviceInfoImpl(open.channel);
530-
} catch (err) {
531-
throw toPublicError(err);
532-
}
565+
return this.#runExclusive('deviceInfo', open => deviceInfoImpl(open.channel));
533566
}
534567

535568
/** Returns which product we are connected to. */
@@ -544,12 +577,7 @@ export class PairedBitBox {
544577

545578
/** Returns the hex-encoded 4-byte root fingerprint. */
546579
async rootFingerprint(): Promise<string> {
547-
const open = this.#requireOpen('rootFingerprint');
548-
try {
549-
return await rootFingerprintImpl(open.channel);
550-
} catch (err) {
551-
throw toPublicError(err);
552-
}
580+
return this.#runExclusive('rootFingerprint', open => rootFingerprintImpl(open.channel));
553581
}
554582

555583
/** Not implemented in this TypeScript iteration. */
@@ -647,12 +675,7 @@ export class PairedBitBox {
647675

648676
/** Query the device for an Ethereum account xpub. */
649677
async ethXpub(keypath: Keypath): Promise<string> {
650-
const open = this.#requireOpen('ethXpub');
651-
try {
652-
return await ethXpubImpl(open.channel, keypath);
653-
} catch (err) {
654-
throw toPublicError(err);
655-
}
678+
return this.#runExclusive('ethXpub', open => ethXpubImpl(open.channel, keypath));
656679
}
657680

658681
/**
@@ -661,12 +684,9 @@ export class PairedBitBox {
661684
* Set `display` to `true` to require on-device confirmation.
662685
*/
663686
async ethAddress(chain_id: bigint, keypath: Keypath, display: boolean): Promise<string> {
664-
const open = this.#requireOpen('ethAddress');
665-
try {
666-
return await ethAddressImpl(open.channel, chain_id, keypath, display);
667-
} catch (err) {
668-
throw toPublicError(err);
669-
}
687+
return this.#runExclusive('ethAddress', open =>
688+
ethAddressImpl(open.channel, chain_id, keypath, display),
689+
);
670690
}
671691

672692
/**
@@ -681,19 +701,16 @@ export class PairedBitBox {
681701
tx: EthTransaction,
682702
address_case?: EthAddressCase,
683703
): Promise<EthSignature> {
684-
const open = this.#requireOpen('ethSignTransaction');
685-
try {
686-
return await ethSignTransactionImpl(
704+
return this.#runExclusive('ethSignTransaction', open =>
705+
ethSignTransactionImpl(
687706
open.channel,
688707
open.info,
689708
chain_id,
690709
keypath,
691710
tx,
692711
address_case,
693-
);
694-
} catch (err) {
695-
throw toPublicError(err);
696-
}
712+
),
713+
);
697714
}
698715

699716
/**
@@ -706,18 +723,15 @@ export class PairedBitBox {
706723
tx: Eth1559Transaction,
707724
address_case?: EthAddressCase,
708725
): Promise<EthSignature> {
709-
const open = this.#requireOpen('ethSign1559Transaction');
710-
try {
711-
return await ethSign1559TransactionImpl(
726+
return this.#runExclusive('ethSign1559Transaction', open =>
727+
ethSign1559TransactionImpl(
712728
open.channel,
713729
open.info,
714730
keypath,
715731
tx,
716732
address_case,
717-
);
718-
} catch (err) {
719-
throw toPublicError(err);
720-
}
733+
),
734+
);
721735
}
722736

723737
/**
@@ -731,12 +745,9 @@ export class PairedBitBox {
731745
keypath: Keypath,
732746
msg: Uint8Array,
733747
): Promise<EthSignature> {
734-
const open = this.#requireOpen('ethSignMessage');
735-
try {
736-
return await ethSignMessageImpl(open.channel, open.info, chain_id, keypath, msg);
737-
} catch (err) {
738-
throw toPublicError(err);
739-
}
748+
return this.#runExclusive('ethSignMessage', open =>
749+
ethSignMessageImpl(open.channel, open.info, chain_id, keypath, msg),
750+
);
740751
}
741752

742753
/**
@@ -750,19 +761,16 @@ export class PairedBitBox {
750761
msg: any,
751762
use_antiklepto?: boolean,
752763
): Promise<EthSignature> {
753-
const open = this.#requireOpen('ethSignTypedMessage');
754-
try {
755-
return await ethSignTypedMessageImpl(
764+
return this.#runExclusive('ethSignTypedMessage', open =>
765+
ethSignTypedMessageImpl(
756766
open.channel,
757767
open.info,
758768
chain_id,
759769
keypath,
760770
msg,
761771
use_antiklepto,
762-
);
763-
} catch (err) {
764-
throw toPublicError(err);
765-
}
772+
),
773+
);
766774
}
767775

768776
/** Cardano support is not implemented in this TypeScript iteration. */

‎test/device-methods.test.ts‎

Lines changed: 118 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -21,44 +21,56 @@ const INFO: Info = {
2121
initialized: true,
2222
};
2323

24+
function responseFor(request: Request): Uint8Array {
25+
let response: Response;
26+
switch (request.request.case) {
27+
case 'deviceInfo':
28+
response = create(ResponseSchema, {
29+
response: {
30+
case: 'deviceInfo',
31+
value: create(DeviceInfoResponseSchema, {
32+
name: 'My BitBox',
33+
initialized: true,
34+
version: '9.26.1',
35+
mnemonicPassphraseEnabled: false,
36+
securechipModel: 'ATECC608B',
37+
monotonicIncrementsRemaining: 42,
38+
passwordStretchingAlgo: 'pwhash',
39+
}),
40+
},
41+
});
42+
break;
43+
case 'fingerprint':
44+
response = create(ResponseSchema, {
45+
response: {
46+
case: 'fingerprint',
47+
value: create(RootFingerprintResponseSchema, {
48+
fingerprint: new Uint8Array([0x4c, 0x00, 0x73, 0x9d]),
49+
}),
50+
},
51+
});
52+
break;
53+
default:
54+
throw new Error(`unexpected request: ${request.request.case}`);
55+
}
56+
return toBinary(ResponseSchema, response);
57+
}
58+
59+
function deferred(): { promise: Promise<void>; resolve: () => void } {
60+
let resolve!: () => void;
61+
const promise = new Promise<void>((r) => {
62+
resolve = r;
63+
});
64+
return { promise, resolve };
65+
}
66+
2467
class FakeDeviceChannel implements EncryptedChannel {
2568
readonly requests: Request['request']['case'][] = [];
2669

2770
async query(plaintext: Uint8Array): Promise<Uint8Array> {
2871
const request = fromBinary(RequestSchema, plaintext);
2972
this.requests.push(request.request.case);
30-
let response: Response;
31-
switch (request.request.case) {
32-
case 'deviceInfo':
33-
response = create(ResponseSchema, {
34-
response: {
35-
case: 'deviceInfo',
36-
value: create(DeviceInfoResponseSchema, {
37-
name: 'My BitBox',
38-
initialized: true,
39-
version: '9.26.1',
40-
mnemonicPassphraseEnabled: false,
41-
securechipModel: 'ATECC608B',
42-
monotonicIncrementsRemaining: 42,
43-
passwordStretchingAlgo: 'pwhash',
44-
}),
45-
},
46-
});
47-
break;
48-
case 'fingerprint':
49-
response = create(ResponseSchema, {
50-
response: {
51-
case: 'fingerprint',
52-
value: create(RootFingerprintResponseSchema, {
53-
fingerprint: new Uint8Array([0x4c, 0x00, 0x73, 0x9d]),
54-
}),
55-
},
56-
});
57-
break;
58-
default:
59-
throw new Error(`unexpected request: ${request.request.case}`);
60-
}
61-
return toBinary(ResponseSchema, response);
73+
return responseFor(request);
6274
}
6375
}
6476

@@ -68,6 +80,41 @@ class EmptyResponseChannel implements EncryptedChannel {
6880
}
6981
}
7082

83+
class BlockingFirstQueryChannel implements EncryptedChannel {
84+
readonly requests: Request['request']['case'][] = [];
85+
activeQueries = 0;
86+
maxActiveQueries = 0;
87+
readonly firstQueryStarted = deferred();
88+
readonly releaseFirstQuery = deferred();
89+
90+
async query(plaintext: Uint8Array): Promise<Uint8Array> {
91+
const request = fromBinary(RequestSchema, plaintext);
92+
this.requests.push(request.request.case);
93+
this.activeQueries += 1;
94+
this.maxActiveQueries = Math.max(this.maxActiveQueries, this.activeQueries);
95+
try {
96+
if (this.requests.length === 1) {
97+
this.firstQueryStarted.resolve();
98+
await this.releaseFirstQuery.promise;
99+
}
100+
return responseFor(request);
101+
} finally {
102+
this.activeQueries -= 1;
103+
}
104+
}
105+
}
106+
107+
class FailsFirstChannel extends FakeDeviceChannel {
108+
override async query(plaintext: Uint8Array): Promise<Uint8Array> {
109+
const request = fromBinary(RequestSchema, plaintext);
110+
this.requests.push(request.request.case);
111+
if (this.requests.length === 1) {
112+
return toBinary(ResponseSchema, create(ResponseSchema, {}));
113+
}
114+
return responseFor(request);
115+
}
116+
}
117+
71118
describe('device methods', () => {
72119
it('deviceInfo returns the wasm package DeviceInfo shape', async () => {
73120
const channel = new FakeDeviceChannel();
@@ -104,4 +151,43 @@ describe('device methods', () => {
104151
message: 'protobuf message could not be decoded',
105152
});
106153
});
154+
155+
it('serializes concurrent device queries', async () => {
156+
const channel = new BlockingFirstQueryChannel();
157+
const paired = new PairedBitBox({ channel, info: INFO, close(): void {} });
158+
159+
const deviceInfo = paired.deviceInfo();
160+
await channel.firstQueryStarted.promise;
161+
162+
const rootFingerprint = paired.rootFingerprint();
163+
await Promise.resolve();
164+
await Promise.resolve();
165+
166+
expect(channel.requests).toEqual(['deviceInfo']);
167+
expect(channel.maxActiveQueries).toBe(1);
168+
169+
channel.releaseFirstQuery.resolve();
170+
await expect(Promise.all([deviceInfo, rootFingerprint])).resolves.toEqual([
171+
{
172+
name: 'My BitBox',
173+
initialized: true,
174+
version: '9.26.1',
175+
mnemonicPassphraseEnabled: false,
176+
securechipModel: 'ATECC608B',
177+
monotonicIncrementsRemaining: 42,
178+
},
179+
'4c00739d',
180+
]);
181+
expect(channel.requests).toEqual(['deviceInfo', 'fingerprint']);
182+
expect(channel.maxActiveQueries).toBe(1);
183+
});
184+
185+
it('continues serializing after a rejected query', async () => {
186+
const channel = new FailsFirstChannel();
187+
const paired = new PairedBitBox({ channel, info: INFO, close(): void {} });
188+
189+
await expect(paired.deviceInfo()).rejects.toMatchObject({ code: 'protobuf-decode' });
190+
await expect(paired.rootFingerprint()).resolves.toBe('4c00739d');
191+
expect(channel.requests).toEqual(['deviceInfo', 'fingerprint']);
192+
});
107193
});

0 commit comments

Comments
 (0)