Skip to content

Commit c1b55b0

Browse files
MrtenzMrtenz
authored
Debounce state updates (#3258)
This changes the persisting logic in the Snap Controller to debounce state updates for each Snap individually by 500 milliseconds. This means that if the Snap updates state multiple times within 500 milliseconds, the state is only persisted once, 500 milliseconds after the last update.
1 parent fdc83e1 commit c1b55b0

File tree

6 files changed

+219
-80
lines changed

6 files changed

+219
-80
lines changed

‎packages/snaps-controllers/coverage.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
2-
"branches": 93.34,
3-
"functions": 97.37,
4-
"lines": 98.33,
5-
"statements": 98.06
2+
"branches": 93.36,
3+
"functions": 97.38,
4+
"lines": 98.34,
5+
"statements": 98.07
66
}

‎packages/snaps-controllers/src/snaps/SnapController.test.tsx‎

Lines changed: 69 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,10 @@ import { pipeline } from 'readable-stream';
7171
import type { Duplex } from 'readable-stream';
7272
import { inc } from 'semver';
7373

74-
import { LEGACY_ENCRYPTION_KEY_DERIVATION_OPTIONS } from './constants';
74+
import {
75+
LEGACY_ENCRYPTION_KEY_DERIVATION_OPTIONS,
76+
STATE_DEBOUNCE_TIMEOUT,
77+
} from './constants';
7578
import { SnapsRegistryStatus } from './registry';
7679
import type { SnapControllerState } from './SnapController';
7780
import {
@@ -9228,6 +9231,14 @@ describe('SnapController', () => {
92289231
});
92299232

92309233
describe('SnapController:getSnapState', () => {
9234+
beforeAll(() => {
9235+
jest.useFakeTimers();
9236+
});
9237+
9238+
afterAll(() => {
9239+
jest.useRealTimers();
9240+
});
9241+
92319242
it(`gets the snap's state`, async () => {
92329243
const messenger = getSnapControllerMessenger();
92339244

@@ -9316,6 +9327,7 @@ describe('SnapController', () => {
93169327
DEFAULT_ENCRYPTION_KEY_DERIVATION_OPTIONS,
93179328
);
93189329

9330+
jest.advanceTimersByTime(STATE_DEBOUNCE_TIMEOUT);
93199331
await promise;
93209332

93219333
const result = await messenger.call(
@@ -9367,6 +9379,7 @@ describe('SnapController', () => {
93679379
true,
93689380
);
93699381

9382+
jest.advanceTimersByTime(STATE_DEBOUNCE_TIMEOUT);
93709383
await promise;
93719384

93729385
const encryptedState1 = await encrypt(
@@ -9557,6 +9570,14 @@ describe('SnapController', () => {
95579570
});
95589571

95599572
describe('SnapController:updateSnapState', () => {
9573+
beforeAll(() => {
9574+
jest.useFakeTimers();
9575+
});
9576+
9577+
afterAll(() => {
9578+
jest.useRealTimers();
9579+
});
9580+
95609581
it(`updates the snap's state`, async () => {
95619582
const messenger = getSnapControllerMessenger();
95629583

@@ -9587,6 +9608,7 @@ describe('SnapController', () => {
95879608
true,
95889609
);
95899610

9611+
jest.advanceTimersByTime(STATE_DEBOUNCE_TIMEOUT);
95909612
await promise;
95919613

95929614
expect(updateSnapStateSpy).toHaveBeenCalledTimes(1);
@@ -9611,6 +9633,8 @@ describe('SnapController', () => {
96119633

96129634
const updateSnapStateSpy = jest.spyOn(snapController, 'updateSnapState');
96139635
const state = { foo: 'bar' };
9636+
9637+
const promise = waitForStateChange(messenger);
96149638
await messenger.call(
96159639
'SnapController:updateSnapState',
96169640
MOCK_SNAP_ID,
@@ -9619,6 +9643,10 @@ describe('SnapController', () => {
96199643
);
96209644

96219645
expect(updateSnapStateSpy).toHaveBeenCalledTimes(1);
9646+
9647+
jest.advanceTimersByTime(STATE_DEBOUNCE_TIMEOUT);
9648+
await promise;
9649+
96229650
expect(
96239651
snapController.state.unencryptedSnapStates[MOCK_SNAP_ID],
96249652
).toStrictEqual(JSON.stringify(state));
@@ -9657,30 +9685,19 @@ describe('SnapController', () => {
96579685
true,
96589686
);
96599687

9688+
jest.advanceTimersByTime(STATE_DEBOUNCE_TIMEOUT);
96609689
await promise;
96619690

96629691
expect(hmacSha512).toHaveBeenCalledTimes(10);
96639692

96649693
snapController.destroy();
96659694
});
96669695

9667-
it('queues multiple state updates', async () => {
9696+
it('debounces multiple state updates', async () => {
96689697
const messenger = getSnapControllerMessenger();
96699698

9670-
jest.useFakeTimers();
9671-
96729699
const encryptor = getSnapControllerEncryptor();
9673-
const { promise, resolve } = createDeferredPromise();
9674-
const encryptWithKey = jest
9675-
.fn<
9676-
ReturnType<typeof encryptor.encryptWithKey>,
9677-
Parameters<typeof encryptor.encryptWithKey>
9678-
>()
9679-
.mockImplementation(async (...args) => {
9680-
resolve();
9681-
await sleep(1);
9682-
return await encryptor.encryptWithKey(...args);
9683-
});
9700+
const encryptWithKey = jest.spyOn(encryptor, 'encryptWithKey');
96849701

96859702
const snapController = getSnapController(
96869703
getSnapControllerOptions({
@@ -9696,41 +9713,52 @@ describe('SnapController', () => {
96969713
}),
96979714
);
96989715

9699-
const firstStateChange = waitForStateChange(messenger);
9716+
const promise = waitForStateChange(messenger);
97009717
await messenger.call(
97019718
'SnapController:updateSnapState',
97029719
MOCK_SNAP_ID,
97039720
{ foo: 'bar' },
97049721
true,
97059722
);
97069723

9724+
expect(
9725+
await messenger.call('SnapController:getSnapState', MOCK_SNAP_ID, true),
9726+
).toStrictEqual({ foo: 'bar' });
9727+
97079728
await messenger.call(
97089729
'SnapController:updateSnapState',
97099730
MOCK_SNAP_ID,
97109731
{ bar: 'baz' },
97119732
true,
97129733
);
97139734

9714-
// We await this promise to ensure the timer is queued.
9715-
await promise;
9716-
jest.advanceTimersByTime(1);
9735+
expect(
9736+
await messenger.call('SnapController:getSnapState', MOCK_SNAP_ID, true),
9737+
).toStrictEqual({ bar: 'baz' });
97179738

9718-
// After this point the second update should be queued.
9719-
await firstStateChange;
9720-
const secondStateChange = waitForStateChange(messenger);
9739+
expect(encryptWithKey).not.toHaveBeenCalled();
97219740

9722-
expect(encryptWithKey).toHaveBeenCalledTimes(1);
9741+
jest.advanceTimersByTime(STATE_DEBOUNCE_TIMEOUT);
9742+
await promise;
97239743

9724-
// This is a bit hacky, but we can't simply advance the timer by 1ms
9725-
// because the second timer is not running yet.
9726-
jest.useRealTimers();
9727-
await secondStateChange;
9744+
expect(encryptWithKey).toHaveBeenCalledTimes(1);
97289745

9729-
expect(encryptWithKey).toHaveBeenCalledTimes(2);
9746+
const nextStateChange = waitForStateChange(messenger);
9747+
await messenger.call(
9748+
'SnapController:updateSnapState',
9749+
MOCK_SNAP_ID,
9750+
{ qux: 'quux' },
9751+
true,
9752+
);
97309753

97319754
expect(
97329755
await messenger.call('SnapController:getSnapState', MOCK_SNAP_ID, true),
9733-
).toStrictEqual({ bar: 'baz' });
9756+
).toStrictEqual({ qux: 'quux' });
9757+
9758+
jest.advanceTimersByTime(STATE_DEBOUNCE_TIMEOUT);
9759+
await nextStateChange;
9760+
9761+
expect(encryptWithKey).toHaveBeenCalledTimes(2);
97349762

97359763
snapController.destroy();
97369764
});
@@ -9763,14 +9791,24 @@ describe('SnapController', () => {
97639791
true,
97649792
);
97659793

9794+
jest.advanceTimersByTime(STATE_DEBOUNCE_TIMEOUT);
97669795
await promise;
9796+
97679797
expect(error).toHaveBeenCalledWith(errorValue);
97689798

97699799
snapController.destroy();
97709800
});
97719801
});
97729802

97739803
describe('SnapController:clearSnapState', () => {
9804+
beforeAll(() => {
9805+
jest.useFakeTimers();
9806+
});
9807+
9808+
afterAll(() => {
9809+
jest.useRealTimers();
9810+
});
9811+
97749812
it('clears the state of a snap', async () => {
97759813
const messenger = getSnapControllerMessenger();
97769814

@@ -9859,7 +9897,9 @@ describe('SnapController', () => {
98599897
// eslint-disable-next-line @typescript-eslint/await-thenable
98609898
await messenger.call('SnapController:clearSnapState', MOCK_SNAP_ID, true);
98619899

9900+
jest.advanceTimersByTime(STATE_DEBOUNCE_TIMEOUT);
98629901
await promise;
9902+
98639903
expect(error).toHaveBeenCalledWith(errorValue);
98649904

98659905
snapController.destroy();

‎packages/snaps-controllers/src/snaps/SnapController.ts‎

Lines changed: 53 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ import { gt } from 'semver';
134134
import {
135135
ALLOWED_PERMISSIONS,
136136
LEGACY_ENCRYPTION_KEY_DERIVATION_OPTIONS,
137+
STATE_DEBOUNCE_TIMEOUT,
137138
} from './constants';
138139
import type { SnapLocation } from './location';
139140
import { detectSnapLocation } from './location';
@@ -167,6 +168,7 @@ import type {
167168
KeyDerivationOptions,
168169
} from '../types';
169170
import {
171+
debouncePersistState,
170172
fetchSnap,
171173
hasTimedOut,
172174
permissionsDiff,
@@ -1798,6 +1800,23 @@ export class SnapController extends BaseController<
17981800
return truncateSnap(this.getExpect(snapId));
17991801
}
18001802

1803+
/**
1804+
* Check if a given Snap has a cached encryption key stored in the runtime.
1805+
*
1806+
* @param snapId - The Snap ID.
1807+
* @param runtime - The Snap runtime data.
1808+
* @returns True if the Snap has a cached encryption key, otherwise false.
1809+
*/
1810+
#hasCachedEncryptionKey(
1811+
snapId: SnapId,
1812+
runtime = this.#getRuntimeExpect(snapId),
1813+
): runtime is SnapRuntimeData & {
1814+
encryptionKey: string;
1815+
encryptionSalt: string;
1816+
} {
1817+
return runtime.encryptionKey !== null && runtime.encryptionSalt !== null;
1818+
}
1819+
18011820
/**
18021821
* Generate an encryption key to be used for state encryption for a given Snap.
18031822
*
@@ -1821,7 +1840,7 @@ export class SnapController extends BaseController<
18211840
}): Promise<{ key: unknown; salt: string }> {
18221841
const runtime = this.#getRuntimeExpect(snapId);
18231842

1824-
if (runtime.encryptionKey && runtime.encryptionSalt && useCache) {
1843+
if (this.#hasCachedEncryptionKey(snapId, runtime) && useCache) {
18251844
return {
18261845
key: await this.#encryptor.importKey(runtime.encryptionKey),
18271846
salt: runtime.encryptionSalt,
@@ -1853,17 +1872,6 @@ export class SnapController extends BaseController<
18531872
return { key: encryptionKey, salt };
18541873
}
18551874

1856-
/**
1857-
* Check if a given Snap has a cached encryption key stored in the runtime.
1858-
*
1859-
* @param snapId - The Snap ID.
1860-
* @returns True if the Snap has a cached encryption key, otherwise false.
1861-
*/
1862-
#hasCachedEncryptionKey(snapId: SnapId) {
1863-
const runtime = this.#getRuntimeExpect(snapId);
1864-
return runtime.encryptionKey !== null && runtime.encryptionSalt !== null;
1865-
}
1866-
18671875
/**
18681876
* Decrypt the encrypted state for a given Snap.
18691877
*
@@ -1958,38 +1966,45 @@ export class SnapController extends BaseController<
19581966
/**
19591967
* Persist the state of a Snap.
19601968
*
1961-
* This is run with a mutex to ensure that only one state update per Snap is
1962-
* processed at a time, avoiding possible race conditions.
1969+
* This function is debounced per Snap, meaning that multiple calls to this
1970+
* function for the same Snap will only result in one state update. It also
1971+
* uses a mutex to ensure that only one state update per Snap is processed at
1972+
* a time, avoiding possible race conditions.
19631973
*
19641974
* @param snapId - The Snap ID.
19651975
* @param newSnapState - The new state of the Snap.
19661976
* @param encrypted - A flag to indicate whether to use encrypted storage or
19671977
* not.
19681978
*/
1969-
async #persistSnapState(
1970-
snapId: SnapId,
1971-
newSnapState: Record<string, Json> | null,
1972-
encrypted: boolean,
1973-
) {
1974-
const runtime = this.#getRuntimeExpect(snapId);
1975-
await runtime.stateMutex.runExclusive(async () => {
1976-
const newState = await this.#getStateToPersist(
1977-
snapId,
1978-
newSnapState,
1979-
encrypted,
1980-
);
1979+
readonly #persistSnapState = debouncePersistState(
1980+
(
1981+
snapId: SnapId,
1982+
newSnapState: Record<string, Json> | null,
1983+
encrypted: boolean,
1984+
) => {
1985+
const runtime = this.#getRuntimeExpect(snapId);
1986+
runtime.stateMutex
1987+
.runExclusive(async () => {
1988+
const newState = await this.#getStateToPersist(
1989+
snapId,
1990+
newSnapState,
1991+
encrypted,
1992+
);
19811993

1982-
if (encrypted) {
1983-
return this.update((state) => {
1984-
state.snapStates[snapId] = newState;
1985-
});
1986-
}
1994+
if (encrypted) {
1995+
return this.update((state) => {
1996+
state.snapStates[snapId] = newState;
1997+
});
1998+
}
19871999

1988-
return this.update((state) => {
1989-
state.unencryptedSnapStates[snapId] = newState;
1990-
});
1991-
});
1992-
}
2000+
return this.update((state) => {
2001+
state.unencryptedSnapStates[snapId] = newState;
2002+
});
2003+
})
2004+
.catch(logError);
2005+
},
2006+
STATE_DEBOUNCE_TIMEOUT,
2007+
);
19932008

19942009
/**
19952010
* Updates the own state of the snap with the given id.
@@ -2012,11 +2027,7 @@ export class SnapController extends BaseController<
20122027
runtime.unencryptedState = newSnapState;
20132028
}
20142029

2015-
// This is intentionally run asynchronously to avoid blocking the main
2016-
// thread.
2017-
this.#persistSnapState(snapId, newSnapState, encrypted).catch((error) => {
2018-
logError(error);
2019-
});
2030+
this.#persistSnapState(snapId, newSnapState, encrypted);
20202031
}
20212032

20222033
/**
@@ -2034,11 +2045,7 @@ export class SnapController extends BaseController<
20342045
runtime.unencryptedState = null;
20352046
}
20362047

2037-
// This is intentionally run asynchronously to avoid blocking the main
2038-
// thread.
2039-
this.#persistSnapState(snapId, null, encrypted).catch((error) => {
2040-
logError(error);
2041-
});
2048+
this.#persistSnapState(snapId, null, encrypted);
20422049
}
20432050

20442051
/**

0 commit comments

Comments
 (0)