From ba0fa4904b4218b8217a2c649e47241011c1cd00 Mon Sep 17 00:00:00 2001 From: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com> Date: Thu, 19 Oct 2023 14:52:35 -0700 Subject: [PATCH] Add logging for deleted prekeys and other records Co-authored-by: Scott Nonnenberg --- ts/SignalProtocolStore.ts | 42 +++++++++++++++---- ts/sql/Interface.ts | 28 ++++++------- ts/sql/Server.ts | 30 +++++++------- ts/sql/util.ts | 32 +++++++------- ts/test-node/util/groupBy_test.ts | 63 ++++++++++++++++++++++++++++ ts/util/groupWhile.ts | 69 +++++++++++++++++++++++++++++++ 6 files changed, 213 insertions(+), 51 deletions(-) create mode 100644 ts/test-node/util/groupBy_test.ts create mode 100644 ts/util/groupWhile.ts diff --git a/ts/SignalProtocolStore.ts b/ts/SignalProtocolStore.ts index 6cf433b456a..8a4b65ed45a 100644 --- a/ts/SignalProtocolStore.ts +++ b/ts/SignalProtocolStore.ts @@ -64,6 +64,7 @@ import { KYBER_KEY_ID_KEY, SIGNED_PRE_KEY_ID_KEY, } from './textsecure/AccountManager'; +import { formatGroups, groupWhile } from './util/groupWhile'; const TIMESTAMP_THRESHOLD = 5 * 1000; // 5 seconds const LOW_KEYS_THRESHOLD = 25; @@ -526,7 +527,8 @@ export class SignalProtocolStore extends EventEmitter { const ids = keyIds.map(keyId => this._getKeyId(ourServiceId, keyId)); - await window.Signal.Data.removeKyberPreKeyById(ids); + const changes = await window.Signal.Data.removeKyberPreKeyById(ids); + log.info(`removeKyberPreKeys: Removed ${changes} kyber prekeys`); ids.forEach(id => { kyberPreKeyCache.delete(id); }); @@ -543,7 +545,8 @@ export class SignalProtocolStore extends EventEmitter { if (this.kyberPreKeys) { this.kyberPreKeys.clear(); } - await window.Signal.Data.removeAllKyberPreKeys(); + const changes = await window.Signal.Data.removeAllKyberPreKeys(); + log.info(`clearKyberPreKeyStore: Removed ${changes} kyber prekeys`); } // PreKeys @@ -623,6 +626,7 @@ export class SignalProtocolStore extends EventEmitter { toSave.push(preKey); }); + log.info(`storePreKeys: Saving ${toSave.length} prekeys`); await window.Signal.Data.bulkAddPreKeys(toSave); toSave.forEach(preKey => { preKeyCache.set(preKey.id, { @@ -643,7 +647,22 @@ export class SignalProtocolStore extends EventEmitter { const ids = keyIds.map(keyId => this._getKeyId(ourServiceId, keyId)); - await window.Signal.Data.removePreKeyById(ids); + log.info( + 'removePreKeys: Removing prekeys:', + // Potentially hundreds of items, so we'll group together sequences, + // take the first 10 of the sequences, format them as ranges, + // and log that once. + // => '1-10, 12, 14-20' + formatGroups( + groupWhile(keyIds.sort(), (a, b) => a + 1 === b).slice(0, 10), + '-', + ', ', + String + ) + ); + + const changes = await window.Signal.Data.removePreKeyById(ids); + log.info(`removePreKeys: Removed ${changes} prekeys`); ids.forEach(id => { preKeyCache.delete(id); }); @@ -657,7 +676,8 @@ export class SignalProtocolStore extends EventEmitter { if (this.preKeys) { this.preKeys.clear(); } - await window.Signal.Data.removeAllPreKeys(); + const changes = await window.Signal.Data.removeAllPreKeys(); + log.info(`clearPreKeyStore: Removed ${changes} prekeys`); } // Signed PreKeys @@ -797,7 +817,8 @@ export class SignalProtocolStore extends EventEmitter { if (this.signedPreKeys) { this.signedPreKeys.clear(); } - await window.Signal.Data.removeAllSignedPreKeys(); + const changes = await window.Signal.Data.removeAllSignedPreKeys(); + log.info(`clearSignedPreKeysStore: Removed ${changes} signed prekeys`); } // Sender Key @@ -1728,7 +1749,8 @@ export class SignalProtocolStore extends EventEmitter { this.sessions.clear(); } this.pendingSessions.clear(); - await window.Signal.Data.removeAllSessions(); + const changes = await window.Signal.Data.removeAllSessions(); + log.info(`clearSessionStore: Removed ${changes} sessions`); }); } @@ -1848,7 +1870,13 @@ export class SignalProtocolStore extends EventEmitter { await this._saveIdentityKey(newRecord); this.identityKeys.delete(record.fromDB.id); - await window.Signal.Data.removeIdentityKeyById(record.fromDB.id); + const changes = await window.Signal.Data.removeIdentityKeyById( + record.fromDB.id + ); + + log.info( + `getOrMigrateIdentityRecord: Removed ${changes} old identity keys for ${record.fromDB.id}` + ); return newRecord; } diff --git a/ts/sql/Interface.ts b/ts/sql/Interface.ts index 48c1497fb38..62da91a362b 100644 --- a/ts/sql/Interface.ts +++ b/ts/sql/Interface.ts @@ -428,27 +428,27 @@ export type DataInterface = { removeDB: () => Promise; removeIndexedDBFiles: () => Promise; - removeIdentityKeyById: (id: IdentityKeyIdType) => Promise; - removeAllIdentityKeys: () => Promise; + removeIdentityKeyById: (id: IdentityKeyIdType) => Promise; + removeAllIdentityKeys: () => Promise; removeKyberPreKeyById: ( id: PreKeyIdType | Array - ) => Promise; + ) => Promise; removeKyberPreKeysByServiceId: (serviceId: ServiceIdString) => Promise; - removeAllKyberPreKeys: () => Promise; + removeAllKyberPreKeys: () => Promise; - removePreKeyById: (id: PreKeyIdType | Array) => Promise; + removePreKeyById: (id: PreKeyIdType | Array) => Promise; removePreKeysByServiceId: (serviceId: ServiceIdString) => Promise; - removeAllPreKeys: () => Promise; + removeAllPreKeys: () => Promise; removeSignedPreKeyById: ( id: SignedPreKeyIdType | Array - ) => Promise; + ) => Promise; removeSignedPreKeysByServiceId: (serviceId: ServiceIdString) => Promise; - removeAllSignedPreKeys: () => Promise; + removeAllSignedPreKeys: () => Promise; - removeAllItems: () => Promise; - removeItemById: (id: ItemKeyType | Array) => Promise; + removeAllItems: () => Promise; + removeItemById: (id: ItemKeyType | Array) => Promise; createOrUpdateSenderKey: (key: SenderKeyType) => Promise; getSenderKeyById: (id: SenderKeyIdType) => Promise; @@ -494,10 +494,10 @@ export type DataInterface = { unprocessed: Array; }): Promise; bulkAddSessions: (array: Array) => Promise; - removeSessionById: (id: SessionIdType) => Promise; + removeSessionById: (id: SessionIdType) => Promise; removeSessionsByConversation: (conversationId: string) => Promise; removeSessionsByServiceId: (serviceId: ServiceIdString) => Promise; - removeAllSessions: () => Promise; + removeAllSessions: () => Promise; getAllSessions: () => Promise>; getConversationCount: () => Promise; @@ -704,8 +704,8 @@ export type DataInterface = { id: string, pending: boolean ) => Promise; - removeAttachmentDownloadJob: (id: string) => Promise; - removeAllAttachmentDownloadJobs: () => Promise; + removeAttachmentDownloadJob: (id: string) => Promise; + removeAllAttachmentDownloadJobs: () => Promise; createOrUpdateStickerPack: (pack: StickerPackType) => Promise; updateStickerPackStatus: ( diff --git a/ts/sql/Server.ts b/ts/sql/Server.ts index e422794e602..3f941d16e7e 100644 --- a/ts/sql/Server.ts +++ b/ts/sql/Server.ts @@ -742,10 +742,10 @@ async function bulkAddIdentityKeys( ): Promise { return bulkAdd(await getWritableInstance(), IDENTITY_KEYS_TABLE, array); } -async function removeIdentityKeyById(id: IdentityKeyIdType): Promise { +async function removeIdentityKeyById(id: IdentityKeyIdType): Promise { return removeById(await getWritableInstance(), IDENTITY_KEYS_TABLE, id); } -async function removeAllIdentityKeys(): Promise { +async function removeAllIdentityKeys(): Promise { return removeAllFromTable(await getWritableInstance(), IDENTITY_KEYS_TABLE); } async function getAllIdentityKeys(): Promise> { @@ -774,7 +774,7 @@ async function bulkAddKyberPreKeys( } async function removeKyberPreKeyById( id: PreKeyIdType | Array -): Promise { +): Promise { return removeById(await getWritableInstance(), KYBER_PRE_KEYS_TABLE, id); } async function removeKyberPreKeysByServiceId( @@ -787,7 +787,7 @@ async function removeKyberPreKeysByServiceId( serviceId, }); } -async function removeAllKyberPreKeys(): Promise { +async function removeAllKyberPreKeys(): Promise { return removeAllFromTable(await getWritableInstance(), KYBER_PRE_KEYS_TABLE); } async function getAllKyberPreKeys(): Promise> { @@ -808,7 +808,7 @@ async function bulkAddPreKeys(array: Array): Promise { } async function removePreKeyById( id: PreKeyIdType | Array -): Promise { +): Promise { return removeById(await getWritableInstance(), PRE_KEYS_TABLE, id); } async function removePreKeysByServiceId( @@ -821,7 +821,7 @@ async function removePreKeysByServiceId( serviceId, }); } -async function removeAllPreKeys(): Promise { +async function removeAllPreKeys(): Promise { return removeAllFromTable(await getWritableInstance(), PRE_KEYS_TABLE); } async function getAllPreKeys(): Promise> { @@ -850,7 +850,7 @@ async function bulkAddSignedPreKeys( } async function removeSignedPreKeyById( id: SignedPreKeyIdType | Array -): Promise { +): Promise { return removeById(await getWritableInstance(), SIGNED_PRE_KEYS_TABLE, id); } async function removeSignedPreKeysByServiceId( @@ -863,7 +863,7 @@ async function removeSignedPreKeysByServiceId( serviceId, }); } -async function removeAllSignedPreKeys(): Promise { +async function removeAllSignedPreKeys(): Promise { return removeAllFromTable(await getWritableInstance(), SIGNED_PRE_KEYS_TABLE); } async function getAllSignedPreKeys(): Promise> { @@ -912,10 +912,10 @@ async function getAllItems(): Promise { } async function removeItemById( id: ItemKeyType | Array -): Promise { +): Promise { return removeById(await getWritableInstance(), ITEMS_TABLE, id); } -async function removeAllItems(): Promise { +async function removeAllItems(): Promise { return removeAllFromTable(await getWritableInstance(), ITEMS_TABLE); } @@ -1421,7 +1421,7 @@ async function commitDecryptResult({ async function bulkAddSessions(array: Array): Promise { return bulkAdd(await getWritableInstance(), SESSIONS_TABLE, array); } -async function removeSessionById(id: SessionIdType): Promise { +async function removeSessionById(id: SessionIdType): Promise { return removeById(await getWritableInstance(), SESSIONS_TABLE, id); } async function removeSessionsByConversation( @@ -1450,7 +1450,7 @@ async function removeSessionsByServiceId( serviceId, }); } -async function removeAllSessions(): Promise { +async function removeAllSessions(): Promise { return removeAllFromTable(await getWritableInstance(), SESSIONS_TABLE); } async function getAllSessions(): Promise> { @@ -4331,14 +4331,14 @@ async function resetAttachmentDownloadPending(): Promise { ` ).run(); } -function removeAttachmentDownloadJobSync(db: Database, id: string): void { +function removeAttachmentDownloadJobSync(db: Database, id: string): number { return removeById(db, ATTACHMENT_DOWNLOADS_TABLE, id); } -async function removeAttachmentDownloadJob(id: string): Promise { +async function removeAttachmentDownloadJob(id: string): Promise { const db = await getWritableInstance(); return removeAttachmentDownloadJobSync(db, id); } -async function removeAllAttachmentDownloadJobs(): Promise { +async function removeAllAttachmentDownloadJobs(): Promise { return removeAllFromTable( await getWritableInstance(), ATTACHMENT_DOWNLOADS_TABLE diff --git a/ts/sql/util.ts b/ts/sql/util.ts index 76636aa2fd6..bb3157b8284 100644 --- a/ts/sql/util.ts +++ b/ts/sql/util.ts @@ -323,37 +323,39 @@ export function getById( export function removeById( db: Database, - table: TableType, + tableName: TableType, id: Key | Array -): void { +): number { + const table = sqlConstant(tableName); if (!Array.isArray(id)) { - db.prepare( - ` + const [query, params] = sql` DELETE FROM ${table} - WHERE id = $id; - ` - ).run({ id }); - return; + WHERE id = ${id}; + `; + return db.prepare(query).run(params).changes; } if (!id.length) { throw new Error('removeById: No ids to delete!'); } + let totalChanges = 0; + const removeByIdsSync = (ids: ReadonlyArray): void => { - db.prepare( - ` + const [query, params] = sql` DELETE FROM ${table} - WHERE id IN ( ${id.map(() => '?').join(', ')} ); - ` - ).run(ids); + WHERE id IN (${sqlJoin(ids, ', ')}); + `; + totalChanges += db.prepare(query).run(params).changes; }; batchMultiVarQuery(db, id, removeByIdsSync); + + return totalChanges; } -export function removeAllFromTable(db: Database, table: TableType): void { - db.prepare(`DELETE FROM ${table};`).run(); +export function removeAllFromTable(db: Database, table: TableType): number { + return db.prepare(`DELETE FROM ${table};`).run().changes; } export function getAllFromTable(db: Database, table: TableType): Array { diff --git a/ts/test-node/util/groupBy_test.ts b/ts/test-node/util/groupBy_test.ts new file mode 100644 index 00000000000..3539cf087df --- /dev/null +++ b/ts/test-node/util/groupBy_test.ts @@ -0,0 +1,63 @@ +// Copyright 2023 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; +import { groupWhile, formatGroups } from '../../util/groupWhile'; + +describe('groupWhile/formatGroups', () => { + function check( + input: Array, + expected: Array>, + expectedFormatted: string + ) { + const result = groupWhile(input, (item, prev) => { + return prev + 1 === item; + }); + assert.deepEqual(result, expected); + const formatted = formatGroups(result, '-', ', ', String); + assert.equal(formatted, expectedFormatted); + } + + it('empty', () => { + check([], [], ''); + }); + + it('one', () => { + check([1], [[1]], '1'); + }); + + it('sequential', () => { + check([1, 2, 3, 4, 5, 6], [[1, 2, 3, 4, 5, 6]], '1-6'); + }); + + it('non-sequential', () => { + check( + [1, 2, 4, 5], + [ + [1, 2], + [4, 5], + ], + '1-2, 4-5' + ); + }); + + it('multiple non-sequential', () => { + check( + [1, 2, 4, 5, 7, 9, 10], + [[1, 2], [4, 5], [7], [9, 10]], + '1-2, 4-5, 7, 9-10' + ); + }); + + function range(start: number, end: number) { + return Array.from({ length: end - start + 1 }, (_, index) => start + index); + } + + it('huge', () => { + check( + [...range(1, 100), ...range(102, 200), ...range(202, 300)], + [range(1, 100), range(102, 200), range(202, 300)], + '1-100, 102-200, 202-300' + ); + }); +}); diff --git a/ts/util/groupWhile.ts b/ts/util/groupWhile.ts new file mode 100644 index 00000000000..15a3487c728 --- /dev/null +++ b/ts/util/groupWhile.ts @@ -0,0 +1,69 @@ +// Copyright 2023 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +/** + * Accepts an array and a predicate function. Returns an array of arrays, where + * each time the predicate returns false, a new sub-array is started. + * + * Useful for grouping sequential items in an array. + * + * @example + * ```ts + * groupWhile([1, 2, 3, 4, 5, 6], (item, prev) => { + * return prev + 1 === item + * }) + * // => [[1, 2, 3], [4, 5, 6]] + * ``` + */ +export function groupWhile( + array: ReadonlyArray, + iteratee: (item: T, prev: T) => boolean +): Array> { + const groups: Array> = []; + let cursor = 0; + while (cursor < array.length) { + const group: Array = []; + do { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + group.push(array[cursor]!); + cursor += 1; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + if (!iteratee(array[cursor]!, array[cursor - 1]!)) { + break; + } + } while (cursor < array.length); + groups.push(group); + } + return groups; +} + +/** + * @example + * ```ts + * let result = [[1, 2], [4, 5], [7], [9, 10]] + * let formatted = formatGroups(result, "-", ", ", String) + * // => "1-2, 4-5, 7, 9-10" + * ``` + */ +export function formatGroups( + groups: Array>, + rangeSeparator: string, + groupSeparator: string, + formatItem: (value: T) => string +): string { + return groups + .map(group => { + if (group.length === 0) { + return ''; + } + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const start = formatItem(group.at(0)!); + if (group.length === 1) { + return start; + } + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const end = formatItem(group.at(-1)!); + return `${start}${rangeSeparator}${end}`; + }) + .join(groupSeparator); +}