From 219cdac22e0db080a3f8d6aeafdf7c5268276b22 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Fri, 16 Aug 2024 16:23:23 +0000 Subject: [PATCH 01/20] Update tests to (maybe?) expose bug in sync state --- test-e2e/sync.js | 160 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 112 insertions(+), 48 deletions(-) diff --git a/test-e2e/sync.js b/test-e2e/sync.js index e8bd022ca..b3559f822 100644 --- a/test-e2e/sync.js +++ b/test-e2e/sync.js @@ -884,7 +884,7 @@ test('pre-haves are updated', async (t) => { }) // TODO: remove timeout -test('data sync state is properly updated as data sync is enabled and disabled', async (t) => { +test.only('data sync state is properly updated as data sync is enabled and disabled', async (t) => { const managers = await createManagers(3, t) const [invitor, ...invitees] = managers @@ -895,28 +895,43 @@ test('data sync state is properly updated as data sync is enabled and disabled', const invitorProject = await invitor.getProject(projectId) invitorProject.observation.create(valueOf(generate('observation')[0])) assert.ok( - invitorProject.$sync.getState().initial, + invitorProject.$sync.getState().initial.isSyncEnabled, 'initial sync is enabled for local device' ) await invite({ invitor, invitees, projectId }) - const [_, ...inviteesProjects] = await Promise.all( - managers.map((m) => m.getProject(projectId)) + const inviteesProjects = await Promise.all( + invitees.map((m) => m.getProject(projectId)) ) await waitForSync([invitorProject, ...inviteesProjects], 'initial') - Object.values(invitorProject.$sync.getState().remoteDeviceSyncState).forEach( - (syncState) => { - assert.ok( - syncState.initial.isSyncEnabled, - 'initial sync is also enabled for all remote devices' - ) - assert.equal( - syncState.data.want, - 1, - 'remote peers want one document from local peer' - ) - } + assert.deepEqual( + invitorProject.$sync.getState().remoteDeviceSyncState, + { + [invitees[0].deviceId]: { + initial: { isSyncEnabled: true, want: 0, wanted: 0 }, + data: { isSyncEnabled: false, want: 1, wanted: 0 }, + }, + [invitees[1].deviceId]: { + initial: { isSyncEnabled: true, want: 0, wanted: 0 }, + data: { isSyncEnabled: false, want: 1, wanted: 0 }, + }, + }, + "from the invitor's perspective, remote peers want one document and data sync is disabled" + ) + assert.deepEqual( + inviteesProjects[0].$sync.getState().remoteDeviceSyncState, + { + [invitorProject.deviceId]: { + initial: { isSyncEnabled: true, want: 0, wanted: 0 }, + data: { isSyncEnabled: false, want: 0, wanted: 1 }, + }, + [invitees[1].deviceId]: { + initial: { isSyncEnabled: true, want: 0, wanted: 0 }, + data: { isSyncEnabled: false, want: 0, wanted: 0 }, + }, + }, + "from one invitee's perspective, one remote peer has a document and data sync is disabled" ) invitorProject.$sync.start() @@ -925,13 +940,41 @@ test('data sync state is properly updated as data sync is enabled and disabled', invitorProject.$sync.getState().data.isSyncEnabled, 'after enabled sync, data sync is enabled for local device' ) + assert.deepEqual( + invitorProject.$sync.getState().remoteDeviceSyncState, + { + [invitees[0].deviceId]: { + initial: { isSyncEnabled: true, want: 0, wanted: 0 }, + data: { isSyncEnabled: false, want: 1, wanted: 0 }, + }, + [invitees[1].deviceId]: { + initial: { isSyncEnabled: true, want: 0, wanted: 0 }, + data: { isSyncEnabled: false, want: 1, wanted: 0 }, + }, + }, + 'data sync is still disabled for remote peers' + ) - Object.values(invitorProject.$sync.getState().remoteDeviceSyncState).forEach( - (syncState) => { - assert.ok( - !syncState.data.isSyncEnabled, - 'data sync is still disabled for all remote devices' - ) + const inviteeAppearsEnabledPromise = pEvent( + invitorProject.$sync, + 'sync-state', + ({ remoteDeviceSyncState }) => + remoteDeviceSyncState[invitees[0].deviceId]?.data.isSyncEnabled + ) + const invitorProjectSyncedWithFirstInviteePromise = pEvent( + invitorProject.$sync, + 'sync-state', + ({ remoteDeviceSyncState }) => { + const remoteData = remoteDeviceSyncState[invitees[0].deviceId]?.data ?? {} + return remoteData.want + remoteData.wanted === 0 + } + ) + const firstInviteeProjectSyncedWithInvitorPromise = pEvent( + inviteesProjects[0].$sync, + 'sync-state', + ({ remoteDeviceSyncState }) => { + const remoteData = remoteDeviceSyncState[invitor.deviceId]?.data ?? {} + return remoteData.want + remoteData.wanted === 0 } ) @@ -939,37 +982,58 @@ test('data sync state is properly updated as data sync is enabled and disabled', assert.ok( inviteesProjects[0].$sync.getState().data.isSyncEnabled, - 'remote peer has data sync enabled after starting sync' + 'invitee has data sync enabled after starting sync' ) - await delay(2000) - assert.ok( - Object.values(invitorProject.$sync.getState().remoteDeviceSyncState).some( - (remoteState) => { - return remoteState.data.isSyncEnabled - } - ), - 'at least one remote peer has enabled data sync' + await inviteeAppearsEnabledPromise + + assert( + invitorProject.$sync.getState().remoteDeviceSyncState[invitees[0].deviceId] + ?.data.isSyncEnabled, + 'one invitee has enabled data sync' + ) + assert( + !invitorProject.$sync.getState().remoteDeviceSyncState[invitees[1].deviceId] + ?.data.isSyncEnabled, + 'other invitee has not enabled data sync' + ) + + await invitorProjectSyncedWithFirstInviteePromise + + await Promise.all([ + invitorProjectSyncedWithFirstInviteePromise, + firstInviteeProjectSyncedWithInvitorPromise, + ]) + + const inviteeAppearsDisabledPromise = pEvent( + invitorProject.$sync, + 'sync-state', + ({ remoteDeviceSyncState }) => + !remoteDeviceSyncState[invitees[0].deviceId]?.data.isSyncEnabled ) inviteesProjects[0].$sync.stop() - const wantedDocs = Object.values( + + await inviteeAppearsDisabledPromise + + const finalRemoteDeviceSyncState = invitorProject.$sync.getState().remoteDeviceSyncState - ).reduce((finalWanted, remoteState) => { - return finalWanted + remoteState.data.want - }, 0) - assert.equal( - wantedDocs, - 1, - 'after one peer enabled data sync, only the other peer remains wanting a doc' - ) - await delay(2000) - assert.ok( - !Object.values(invitorProject.$sync.getState().remoteDeviceSyncState).some( - (remoteState) => { - return remoteState.data.isSyncEnabled - } - ), - 'no remote peer has enabled data sync now' + assert.deepEqual( + finalRemoteDeviceSyncState[invitees[0].deviceId], + { + initial: { isSyncEnabled: true, want: 0, wanted: 0 }, + data: { isSyncEnabled: false, want: 0, wanted: 0 }, + }, + 'one invitee is disabled but settled' + ) + assert.deepEqual( + finalRemoteDeviceSyncState[invitees[1].deviceId], + { + [invitees[1].deviceId]: { + initial: { isSyncEnabled: true, want: 0, wanted: 0 }, + data: { isSyncEnabled: false, want: 1, wanted: 0 }, + }, + }, + 'other invitee is still disabled, still wants something' ) }) From c2ccbe184d4d5adc0c4deab85933720d7b632ea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Fri, 16 Aug 2024 16:27:54 -0300 Subject: [PATCH 02/20] testing crash in db --- test-e2e/sync.js | 130 ++++++++++++++++++++++++++--------------------- 1 file changed, 71 insertions(+), 59 deletions(-) diff --git a/test-e2e/sync.js b/test-e2e/sync.js index b3559f822..18b29df37 100644 --- a/test-e2e/sync.js +++ b/test-e2e/sync.js @@ -919,6 +919,11 @@ test.only('data sync state is properly updated as data sync is enabled and disab }, "from the invitor's perspective, remote peers want one document and data sync is disabled" ) + //console.log('invitor id', invitorProject.deviceId) + //console.log( + // 'actual', + // inviteesProjects[0].$sync.getState().remoteDeviceSyncState + //) assert.deepEqual( inviteesProjects[0].$sync.getState().remoteDeviceSyncState, { @@ -955,12 +960,18 @@ test.only('data sync state is properly updated as data sync is enabled and disab 'data sync is still disabled for remote peers' ) + /* eslint-disable-next-line no-unused-vars */ const inviteeAppearsEnabledPromise = pEvent( invitorProject.$sync, 'sync-state', ({ remoteDeviceSyncState }) => remoteDeviceSyncState[invitees[0].deviceId]?.data.isSyncEnabled - ) + ) // this should resolve with the first invitee's data.isSyncEnabled value + + // commenting this will crash the db + await delay(2000) + + /* eslint-disable-next-line no-unused-vars */ const invitorProjectSyncedWithFirstInviteePromise = pEvent( invitorProject.$sync, 'sync-state', @@ -968,7 +979,8 @@ test.only('data sync state is properly updated as data sync is enabled and disab const remoteData = remoteDeviceSyncState[invitees[0].deviceId]?.data ?? {} return remoteData.want + remoteData.wanted === 0 } - ) + ) // this should resolve with the first invitee's data. want+wanted=0 + /* eslint-disable-next-line no-unused-vars */ const firstInviteeProjectSyncedWithInvitorPromise = pEvent( inviteesProjects[0].$sync, 'sync-state', @@ -976,64 +988,64 @@ test.only('data sync state is properly updated as data sync is enabled and disab const remoteData = remoteDeviceSyncState[invitor.deviceId]?.data ?? {} return remoteData.want + remoteData.wanted === 0 } - ) + ) // this should resolve with the invitor's data. want+wanted inviteesProjects[0].$sync.start() - assert.ok( - inviteesProjects[0].$sync.getState().data.isSyncEnabled, - 'invitee has data sync enabled after starting sync' - ) - - await inviteeAppearsEnabledPromise - - assert( - invitorProject.$sync.getState().remoteDeviceSyncState[invitees[0].deviceId] - ?.data.isSyncEnabled, - 'one invitee has enabled data sync' - ) - assert( - !invitorProject.$sync.getState().remoteDeviceSyncState[invitees[1].deviceId] - ?.data.isSyncEnabled, - 'other invitee has not enabled data sync' - ) - - await invitorProjectSyncedWithFirstInviteePromise - - await Promise.all([ - invitorProjectSyncedWithFirstInviteePromise, - firstInviteeProjectSyncedWithInvitorPromise, - ]) - - const inviteeAppearsDisabledPromise = pEvent( - invitorProject.$sync, - 'sync-state', - ({ remoteDeviceSyncState }) => - !remoteDeviceSyncState[invitees[0].deviceId]?.data.isSyncEnabled - ) - - inviteesProjects[0].$sync.stop() - - await inviteeAppearsDisabledPromise - - const finalRemoteDeviceSyncState = - invitorProject.$sync.getState().remoteDeviceSyncState - assert.deepEqual( - finalRemoteDeviceSyncState[invitees[0].deviceId], - { - initial: { isSyncEnabled: true, want: 0, wanted: 0 }, - data: { isSyncEnabled: false, want: 0, wanted: 0 }, - }, - 'one invitee is disabled but settled' - ) - assert.deepEqual( - finalRemoteDeviceSyncState[invitees[1].deviceId], - { - [invitees[1].deviceId]: { - initial: { isSyncEnabled: true, want: 0, wanted: 0 }, - data: { isSyncEnabled: false, want: 1, wanted: 0 }, - }, - }, - 'other invitee is still disabled, still wants something' - ) + //assert.ok( + // inviteesProjects[0].$sync.getState().data.isSyncEnabled, + // 'invitee has data sync enabled after starting sync' + //) + // + //await inviteeAppearsEnabledPromise + // + //assert( + // invitorProject.$sync.getState().remoteDeviceSyncState[invitees[0].deviceId] + // ?.data.isSyncEnabled, + // 'one invitee has enabled data sync' + //) + //assert( + // !invitorProject.$sync.getState().remoteDeviceSyncState[invitees[1].deviceId] + // ?.data.isSyncEnabled, + // 'other invitee has not enabled data sync' + //) + + //await invitorProjectSyncedWithFirstInviteePromise + // + //await Promise.all([ + // invitorProjectSyncedWithFirstInviteePromise, + // firstInviteeProjectSyncedWithInvitorPromise, + //]) + + //const inviteeAppearsDisabledPromise = pEvent( + // invitorProject.$sync, + // 'sync-state', + // ({ remoteDeviceSyncState }) => + // !remoteDeviceSyncState[invitees[0].deviceId]?.data.isSyncEnabled + //) + // + //inviteesProjects[0].$sync.stop() + // + //await inviteeAppearsDisabledPromise + // + //const finalRemoteDeviceSyncState = + // invitorProject.$sync.getState().remoteDeviceSyncState + //assert.deepEqual( + // finalRemoteDeviceSyncState[invitees[0].deviceId], + // { + // initial: { isSyncEnabled: true, want: 0, wanted: 0 }, + // data: { isSyncEnabled: false, want: 0, wanted: 0 }, + // }, + // 'one invitee is disabled but settled' + //) + //assert.deepEqual( + // finalRemoteDeviceSyncState[invitees[1].deviceId], + // { + // [invitees[1].deviceId]: { + // initial: { isSyncEnabled: true, want: 0, wanted: 0 }, + // data: { isSyncEnabled: false, want: 1, wanted: 0 }, + // }, + // }, + // 'other invitee is still disabled, still wants something' + //) }) From 9bf42ebf08fabcb948f0014d663fbbd2aa90fc3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ciccola?= Date: Mon, 19 Aug 2024 13:33:19 -0300 Subject: [PATCH 03/20] debugging sync tests --- test-e2e/sync.js | 51 ++++++++++++++++++++++-------------------------- 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/test-e2e/sync.js b/test-e2e/sync.js index 18b29df37..72f601627 100644 --- a/test-e2e/sync.js +++ b/test-e2e/sync.js @@ -919,11 +919,6 @@ test.only('data sync state is properly updated as data sync is enabled and disab }, "from the invitor's perspective, remote peers want one document and data sync is disabled" ) - //console.log('invitor id', invitorProject.deviceId) - //console.log( - // 'actual', - // inviteesProjects[0].$sync.getState().remoteDeviceSyncState - //) assert.deepEqual( inviteesProjects[0].$sync.getState().remoteDeviceSyncState, { @@ -992,30 +987,30 @@ test.only('data sync state is properly updated as data sync is enabled and disab inviteesProjects[0].$sync.start() - //assert.ok( - // inviteesProjects[0].$sync.getState().data.isSyncEnabled, - // 'invitee has data sync enabled after starting sync' - //) - // - //await inviteeAppearsEnabledPromise - // - //assert( - // invitorProject.$sync.getState().remoteDeviceSyncState[invitees[0].deviceId] - // ?.data.isSyncEnabled, - // 'one invitee has enabled data sync' - //) - //assert( - // !invitorProject.$sync.getState().remoteDeviceSyncState[invitees[1].deviceId] - // ?.data.isSyncEnabled, - // 'other invitee has not enabled data sync' - //) + assert.ok( + inviteesProjects[0].$sync.getState().data.isSyncEnabled, + 'invitee has data sync enabled after starting sync' + ) - //await invitorProjectSyncedWithFirstInviteePromise - // - //await Promise.all([ - // invitorProjectSyncedWithFirstInviteePromise, - // firstInviteeProjectSyncedWithInvitorPromise, - //]) + await inviteeAppearsEnabledPromise + + assert( + invitorProject.$sync.getState().remoteDeviceSyncState[invitees[0].deviceId] + ?.data.isSyncEnabled, + 'one invitee has enabled data sync' + ) + assert( + !invitorProject.$sync.getState().remoteDeviceSyncState[invitees[1].deviceId] + ?.data.isSyncEnabled, + 'other invitee has not enabled data sync' + ) + + await invitorProjectSyncedWithFirstInviteePromise + + await Promise.all([ + invitorProjectSyncedWithFirstInviteePromise, + firstInviteeProjectSyncedWithInvitorPromise, + ]) //const inviteeAppearsDisabledPromise = pEvent( // invitorProject.$sync, From ec942c63279f5822c69c8902ff8649444a7f98ac Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 19:28:15 +0000 Subject: [PATCH 04/20] Remove missing --- src/sync/core-sync-state.js | 12 +--------- src/sync/namespace-sync-state.js | 5 ++--- test-e2e/sync.js | 7 +++--- tests/sync/core-sync-state.js | 35 +++++++++--------------------- tests/sync/namespace-sync-state.js | 8 ++----- 5 files changed, 18 insertions(+), 49 deletions(-) diff --git a/src/sync/core-sync-state.js b/src/sync/core-sync-state.js index ff9517418..81501c300 100644 --- a/src/sync/core-sync-state.js +++ b/src/sync/core-sync-state.js @@ -22,7 +22,6 @@ import RemoteBitfield, { * @property {number} have blocks the peer has locally * @property {number} want blocks the peer wants, and at least one peer has * @property {number} wanted blocks the peer has that at least one peer wants - * @property {number} missing blocks the peer wants but no peer has */ /** * @typedef {CoreState & { status: 'disconnected' | 'connecting' | 'connected' }} PeerCoreState @@ -48,7 +47,6 @@ import RemoteBitfield, { * 1. `have` - number of blocks the peer has locally * 2. `want` - number of blocks the peer wants, and at least one peer has * 3. `wanted` - number of blocks the peer has that at least one peer wants - * 4. `missing` - number of blocks the peer wants but no peer has * */ export class CoreSyncState { @@ -367,7 +365,7 @@ export function deriveState(coreState) { const peerStates = new Array(peers.length) const length = coreState.length || 0 for (let i = 0; i < peerStates.length; i++) { - peerStates[i] = { want: 0, have: 0, wanted: 0, missing: 0 } + peerStates[i] = { want: 0, have: 0, wanted: 0} } const haves = new Array(peerStates.length) let want = 0 @@ -389,14 +387,6 @@ export function deriveState(coreState) { want = wouldLikeIt & someoneHasIt someoneWantsIt |= want peerStates[j].want += bitCount32(want) - // A block is missing if: - // 1. The peer wants it - // 2. The peer doesn't have it - // 3. No other peer has it - // Need to truncate to the core length, since otherwise we would get - // missing values beyond core length - const missing = wouldLikeIt & ~someoneHasIt & truncate - peerStates[j].missing += bitCount32(missing) } for (let j = 0; j < peerStates.length; j++) { // A block is wanted if: diff --git a/src/sync/namespace-sync-state.js b/src/sync/namespace-sync-state.js index e1a7a86ec..01fa4d373 100644 --- a/src/sync/namespace-sync-state.js +++ b/src/sync/namespace-sync-state.js @@ -148,9 +148,9 @@ export class NamespaceSyncState { */ export function createState(status) { if (status) { - return { want: 0, have: 0, wanted: 0, missing: 0, status } + return { want: 0, have: 0, wanted: 0, status } } else { - return { want: 0, have: 0, wanted: 0, missing: 0 } + return { want: 0, have: 0, wanted: 0 } } } @@ -178,7 +178,6 @@ function mutatingAddPeerState(accumulator, currentValue) { accumulator.have += currentValue.have accumulator.want += currentValue.want accumulator.wanted += currentValue.wanted - accumulator.missing += currentValue.missing if ('status' in accumulator && accumulator.status !== currentValue.status) { if (currentValue.status === 'disconnected') { accumulator.status === 'disconnected' diff --git a/test-e2e/sync.js b/test-e2e/sync.js index 72f601627..bf6bc41a5 100644 --- a/test-e2e/sync.js +++ b/test-e2e/sync.js @@ -649,6 +649,7 @@ test('no sync capabilities === no namespaces sync apart from auth', async (t) => assert.equal(blockedState[ns].coreCount, 1) } + // TODO: Update this comment? // "Invitor" knows blocked peer is blocked from the start, so never connects // and never creates a local copy of the blocked peer cores, but "Invitee" // does connect initially, before it realized the peer is blocked, and @@ -656,8 +657,8 @@ test('no sync capabilities === no namespaces sync apart from auth', async (t) => // data, so it considers data to be "missing" which the Invitor does not // register as missing. assert.deepEqual( - excludeKeys(invitorState[ns].localState, ['missing']), - excludeKeys(inviteeState[ns].localState, ['missing']) + invitorState[ns].localState, + inviteeState[ns].localState, ) } @@ -1005,8 +1006,6 @@ test.only('data sync state is properly updated as data sync is enabled and disab 'other invitee has not enabled data sync' ) - await invitorProjectSyncedWithFirstInviteePromise - await Promise.all([ invitorProjectSyncedWithFirstInviteePromise, firstInviteeProjectSyncedWithInvitorPromise, diff --git a/tests/sync/core-sync-state.js b/tests/sync/core-sync-state.js index 4c1ccbbd1..206fb0699 100644 --- a/tests/sync/core-sync-state.js +++ b/tests/sync/core-sync-state.js @@ -29,7 +29,7 @@ import { EventEmitter } from 'node:events' */ const scenarios = [ { - message: '3 peers, start with haves, test want, have, wanted and missing', + message: '3 peers, start with haves, test want, have, and wanted', state: { length: 4, localState: { have: 0b0111 }, @@ -37,27 +37,24 @@ const scenarios = [ }, expected: { coreLength: 4, - localState: { want: 0, have: 3, wanted: 2, missing: 1 }, + localState: { want: 0, have: 3, wanted: 2 }, remoteStates: { peer0: { want: 1, have: 2, wanted: 1, - missing: 1, status: 'disconnected', }, peer1: { want: 1, have: 2, wanted: 1, - missing: 1, status: 'disconnected', }, peer2: { want: 2, have: 1, wanted: 0, - missing: 1, status: 'disconnected', }, }, @@ -72,20 +69,18 @@ const scenarios = [ }, expected: { coreLength: 4, - localState: { want: 0, have: 0, wanted: 0, missing: 4 }, + localState: { want: 0, have: 0, wanted: 0 }, remoteStates: { peer0: { want: 0, have: 0, wanted: 0, - missing: 4, status: 'disconnected', }, peer1: { want: 0, have: 0, wanted: 0, - missing: 4, status: 'disconnected', }, }, @@ -100,9 +95,9 @@ const scenarios = [ }, expected: { coreLength: 3, - localState: { want: 0, have: 3, wanted: 1, missing: 0 }, + localState: { want: 0, have: 3, wanted: 1 }, remoteStates: { - peer0: { want: 1, have: 1, wanted: 0, missing: 0, status: 'connected' }, + peer0: { want: 1, have: 1, wanted: 0, status: 'connected' }, }, }, }, @@ -115,13 +110,12 @@ const scenarios = [ }, expected: { coreLength: 3, - localState: { want: 0, have: 3, wanted: 1, missing: 0 }, + localState: { want: 0, have: 3, wanted: 1 }, remoteStates: { peer0: { want: 1, have: 1, wanted: 0, - missing: 0, status: 'disconnected', }, }, @@ -136,13 +130,12 @@ const scenarios = [ }, expected: { coreLength: 3, - localState: { want: 0, have: 3, wanted: 1, missing: 0 }, + localState: { want: 0, have: 3, wanted: 1 }, remoteStates: { peer0: { want: 1, have: 2, wanted: 0, - missing: 0, status: 'disconnected', }, }, @@ -157,13 +150,12 @@ const scenarios = [ }, expected: { coreLength: 3, - localState: { want: 0, have: 3, wanted: 0, missing: 0 }, + localState: { want: 0, have: 3, wanted: 0 }, remoteStates: { peer0: { want: 0, have: 3, wanted: 0, - missing: 0, status: 'disconnected', }, }, @@ -182,27 +174,24 @@ const scenarios = [ }, expected: { coreLength: 72, - localState: { want: 0, have: 50, wanted: 15, missing: 22 }, + localState: { want: 0, have: 50, wanted: 15 }, remoteStates: { peer0: { want: 10, have: 40, wanted: 5, - missing: 22, status: 'disconnected', }, peer1: { want: 5, have: 40, wanted: 10, - missing: 0, status: 'disconnected', }, peer2: { want: 5, have: 40, wanted: 10, - missing: 0, status: 'disconnected', }, }, @@ -217,20 +206,18 @@ const scenarios = [ }, expected: { coreLength: 2, - localState: { want: 0, have: 2, wanted: 2, missing: 0 }, + localState: { want: 0, have: 2, wanted: 2 }, remoteStates: { peer0: { want: 1, have: 0, wanted: 0, - missing: 0, status: 'disconnected', }, peer1: { want: 2, have: 0, wanted: 0, - missing: 0, status: 'disconnected', }, }, @@ -272,14 +259,12 @@ test('deriveState() have at index beyond bitfield page size', () => { want: 1, have: 10, wanted: 10, - missing: BITS_PER_PAGE - 1, }, remoteStates: { peer0: { want: 10, have: 1, wanted: 1, - missing: BITS_PER_PAGE - 1, status: 'disconnected', }, }, diff --git a/tests/sync/namespace-sync-state.js b/tests/sync/namespace-sync-state.js index ad7545bc7..8551afe21 100644 --- a/tests/sync/namespace-sync-state.js +++ b/tests/sync/namespace-sync-state.js @@ -49,8 +49,7 @@ test('sync cores in a namespace', async () => { if ( state.localState.want === 0 && state.localState.wanted === 0 && - state.localState.have === 30 && - state.localState.missing === 10 + state.localState.have === 30 ) { syncState1Sync.resolve(state.remoteStates) } @@ -66,8 +65,7 @@ test('sync cores in a namespace', async () => { if ( state.localState.want === 0 && state.localState.wanted === 0 && - state.localState.have === 30 && - state.localState.missing === 10 + state.localState.have === 30 ) { syncState2Sync.resolve(state.remoteStates) } @@ -104,7 +102,6 @@ test('sync cores in a namespace', async () => { want: 0, wanted: 0, have: 30, - missing: 10, status: 'connected', }, }, @@ -118,7 +115,6 @@ test('sync cores in a namespace', async () => { want: 0, wanted: 0, have: 30, - missing: 10, status: 'connected', }, }, From 778524f4069b83d93f4c235f7b7bb8db389c91aa Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 19:51:44 +0000 Subject: [PATCH 05/20] Test passing with new definitions of want and wanted --- src/sync/core-sync-state.js | 165 ++++++++++++++++++++++++------------ test-e2e/sync.js | 65 +++++++------- 2 files changed, 143 insertions(+), 87 deletions(-) diff --git a/src/sync/core-sync-state.js b/src/sync/core-sync-state.js index 81501c300..c67df797a 100644 --- a/src/sync/core-sync-state.js +++ b/src/sync/core-sync-state.js @@ -20,8 +20,8 @@ import RemoteBitfield, { /** * @typedef {object} CoreState * @property {number} have blocks the peer has locally - * @property {number} want blocks the peer wants, and at least one peer has - * @property {number} wanted blocks the peer has that at least one peer wants + * @property {number} want blocks this peer wants from us TODO: derive? + * @property {number} wanted blocks we want from this peer TODO: derivce? */ /** * @typedef {CoreState & { status: 'disconnected' | 'connecting' | 'connected' }} PeerCoreState @@ -45,8 +45,8 @@ import RemoteBitfield, { * * Each peer (including the local peer) has a state of: * 1. `have` - number of blocks the peer has locally - * 2. `want` - number of blocks the peer wants, and at least one peer has - * 3. `wanted` - number of blocks the peer has that at least one peer wants + * 2. `want` - TODO + * 3. `wanted` - TODO * */ export class CoreSyncState { @@ -347,9 +347,15 @@ export class PeerState { * Only exporteed for testing */ export function deriveState(coreState) { - const peerIds = ['local'] - const peers = [coreState.localState] + // TODO: consider renaming this to `length` to make the diff smaller + const coreLength = coreState.length || 0 + /** @type {CoreState} */ + const localState = { have: 0, want: 0, wanted: 0 } + /** @type {Record} */ + const remoteStates = {} + /** @type {Map} */ + const peers = new Map() for (const [peerId, peerState] of coreState.remoteStates.entries()) { const psc = coreState.peerSyncControllers.get(peerId) const isBlocked = psc?.syncCapability[coreState.namespace] === 'blocked' @@ -357,57 +363,112 @@ export function deriveState(coreState) { // how to expose this state in a meaningful way for considering sync // completion, because blocked peers do not sync. if (isBlocked) continue - peerIds.push(peerId) - peers.push(peerState) - } + // peerIds.push(peerId) + peers.set(peerId, peerState) - /** @type {CoreState[]} */ - const peerStates = new Array(peers.length) - const length = coreState.length || 0 - for (let i = 0; i < peerStates.length; i++) { - peerStates[i] = { want: 0, have: 0, wanted: 0} - } - const haves = new Array(peerStates.length) - let want = 0 - for (let i = 0; i < length; i += 32) { - const truncate = 2 ** Math.min(32, length - i) - 1 - let someoneHasIt = 0 - for (let j = 0; j < peers.length; j++) { - haves[j] = peers[j].haveWord(i) & truncate - someoneHasIt |= haves[j] - peerStates[j].have += bitCount32(haves[j]) - } - let someoneWantsIt = 0 - for (let j = 0; j < peers.length; j++) { - // A block is a want if: - // 1. The peer wants it - // 2. They don't have it - // 3. Someone does have it - const wouldLikeIt = peers[j].wantWord(i) & ~haves[j] - want = wouldLikeIt & someoneHasIt - someoneWantsIt |= want - peerStates[j].want += bitCount32(want) - } - for (let j = 0; j < peerStates.length; j++) { - // A block is wanted if: - // 1. Someone wants it - // 2. The peer has it - const wanted = someoneWantsIt & haves[j] - peerStates[j].wanted += bitCount32(wanted) + remoteStates[peerId] = { + have: 0, + want: 0, + wanted: 0, + status: peerState.status, } } - /** @type {DerivedState} */ - const derivedState = { - coreLength: length, - localState: peerStates[0], - remoteStates: {}, + + for (let i = 0; i < coreLength; i += 32) { + const truncate = 2 ** Math.min(32, coreLength - i) - 1 + + const localHaves = coreState.localState.haveWord(i) & truncate + localState.have += bitCount32(localHaves) + + for (const [peerId, peer] of peers.entries()) { + // Haves + + const peerHaves = peer.haveWord(i) & truncate + remoteStates[peerId].have += bitCount32(peerHaves) + + // Wants (from me) + + const wantsFromMe = peer.wantWord(i) & ~peerHaves & localHaves + remoteStates[peerId].want += bitCount32(wantsFromMe) + localState.wanted += bitCount32(wantsFromMe) // TODO: does this work? + + // I want from them + + const wantedByMe = peerHaves & ~localHaves + remoteStates[peerId].wanted += bitCount32(wantedByMe) + localState.want += bitCount32(wantedByMe) // TODO: does this work? + } } - for (let j = 1; j < peerStates.length; j++) { - const peerState = /** @type {PeerCoreState} */ (peerStates[j]) - peerState.status = peers[j].status - derivedState.remoteStates[peerIds[j]] = peerState + + return { + coreLength, + localState, + remoteStates, } - return derivedState + + // ----------------- + + // const peerIds = ['local'] + // const peers = [coreState.localState] + + // for (const [peerId, peerState] of coreState.remoteStates.entries()) { + // const psc = coreState.peerSyncControllers.get(peerId) + // const isBlocked = psc?.syncCapability[coreState.namespace] === 'blocked' + // // Currently we do not include blocked peers in sync state - it's unclear + // // how to expose this state in a meaningful way for considering sync + // // completion, because blocked peers do not sync. + // if (isBlocked) continue + // peerIds.push(peerId) + // peers.push(peerState) + // } + + // /** @type {CoreState[]} */ + // const peerStates = new Array(peers.length) + // const length = coreState.length || 0 + // for (let i = 0; i < peerStates.length; i++) { + // peerStates[i] = { want: 0, have: 0, wanted: 0 } + // } + // const haves = new Array(peerStates.length) + // let want = 0 + // for (let i = 0; i < length; i += 32) { + // const truncate = 2 ** Math.min(32, length - i) - 1 + // let someoneHasIt = 0 + // for (let j = 0; j < peers.length; j++) { + // haves[j] = peers[j].haveWord(i) & truncate + // someoneHasIt |= haves[j] + // peerStates[j].have += bitCount32(haves[j]) + // } + // let someoneWantsIt = 0 + // for (let j = 0; j < peers.length; j++) { + // // A block is a want if: + // // 1. The peer wants it + // // 2. They don't have it + // // 3. Someone does have it + // const wouldLikeIt = peers[j].wantWord(i) & ~haves[j] + // want = wouldLikeIt & someoneHasIt + // someoneWantsIt |= want + // peerStates[j].want += bitCount32(want) + // } + // for (let j = 0; j < peerStates.length; j++) { + // // A block is wanted if: + // // 1. Someone wants it + // // 2. The peer has it + // const wanted = someoneWantsIt & haves[j] + // peerStates[j].wanted += bitCount32(wanted) + // } + // } + // /** @type {DerivedState} */ + // const derivedState = { + // coreLength: length, + // localState: peerStates[0], + // remoteStates: {}, + // } + // for (let j = 1; j < peerStates.length; j++) { + // const peerState = /** @type {PeerCoreState} */ (peerStates[j]) + // peerState.status = peers[j].status + // derivedState.remoteStates[peerIds[j]] = peerState + // } + // return derivedState } /** diff --git a/test-e2e/sync.js b/test-e2e/sync.js index bf6bc41a5..9cd88104a 100644 --- a/test-e2e/sync.js +++ b/test-e2e/sync.js @@ -656,10 +656,7 @@ test('no sync capabilities === no namespaces sync apart from auth', async (t) => // creates a local copy of the blocked peer's cores, but never downloads // data, so it considers data to be "missing" which the Invitor does not // register as missing. - assert.deepEqual( - invitorState[ns].localState, - inviteeState[ns].localState, - ) + assert.deepEqual(invitorState[ns].localState, inviteeState[ns].localState) } await disconnect1() @@ -1011,35 +1008,33 @@ test.only('data sync state is properly updated as data sync is enabled and disab firstInviteeProjectSyncedWithInvitorPromise, ]) - //const inviteeAppearsDisabledPromise = pEvent( - // invitorProject.$sync, - // 'sync-state', - // ({ remoteDeviceSyncState }) => - // !remoteDeviceSyncState[invitees[0].deviceId]?.data.isSyncEnabled - //) - // - //inviteesProjects[0].$sync.stop() - // - //await inviteeAppearsDisabledPromise - // - //const finalRemoteDeviceSyncState = - // invitorProject.$sync.getState().remoteDeviceSyncState - //assert.deepEqual( - // finalRemoteDeviceSyncState[invitees[0].deviceId], - // { - // initial: { isSyncEnabled: true, want: 0, wanted: 0 }, - // data: { isSyncEnabled: false, want: 0, wanted: 0 }, - // }, - // 'one invitee is disabled but settled' - //) - //assert.deepEqual( - // finalRemoteDeviceSyncState[invitees[1].deviceId], - // { - // [invitees[1].deviceId]: { - // initial: { isSyncEnabled: true, want: 0, wanted: 0 }, - // data: { isSyncEnabled: false, want: 1, wanted: 0 }, - // }, - // }, - // 'other invitee is still disabled, still wants something' - //) + const inviteeAppearsDisabledPromise = pEvent( + invitorProject.$sync, + 'sync-state', + ({ remoteDeviceSyncState }) => + !remoteDeviceSyncState[invitees[0].deviceId]?.data.isSyncEnabled + ) + + inviteesProjects[0].$sync.stop() + + await inviteeAppearsDisabledPromise + + const finalRemoteDeviceSyncState = + invitorProject.$sync.getState().remoteDeviceSyncState + assert.deepEqual( + finalRemoteDeviceSyncState[invitees[0].deviceId], + { + initial: { isSyncEnabled: true, want: 0, wanted: 0 }, + data: { isSyncEnabled: false, want: 0, wanted: 0 }, + }, + 'one invitee is disabled but settled' + ) + assert.deepEqual( + finalRemoteDeviceSyncState[invitees[1].deviceId], + { + initial: { isSyncEnabled: true, want: 0, wanted: 0 }, + data: { isSyncEnabled: false, want: 1, wanted: 0 }, + }, + 'other invitee is still disabled, still wants something' + ) }) From 565091f7d4c62dc0c55dd05ab97d53212639dca3 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 20:11:55 +0000 Subject: [PATCH 06/20] Fix tests --- test-e2e/sync.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/test-e2e/sync.js b/test-e2e/sync.js index 9cd88104a..1dce1c03c 100644 --- a/test-e2e/sync.js +++ b/test-e2e/sync.js @@ -4,7 +4,8 @@ import * as fs from 'node:fs/promises' import { pEvent } from 'p-event' import { setTimeout as delay } from 'timers/promises' import { request } from 'undici' -import { excludeKeys } from 'filter-obj' +// import { excludeKeys } from 'filter-obj' TODO +import { isDeepStrictEqual } from 'node:util' import FakeTimers from '@sinonjs/fake-timers' import Fastify from 'fastify' import { map } from 'iterpal' @@ -803,11 +804,15 @@ test('Correct sync state prior to data sync', async function (t) { } }) - // Wait for initial sharing of sync state - await delay(200) - - const syncState = await Promise.all(projects.map((p) => p.$sync.getState())) + await Promise.all( + projects.map((project, i) => + pEvent(project.$sync, 'sync-state', (syncState) => + isDeepStrictEqual(syncState, expected[i]) + ) + ) + ) + const syncState = projects.map((p) => p.$sync.getState()) assert.deepEqual(syncState, expected) await disconnect2() @@ -882,7 +887,7 @@ test('pre-haves are updated', async (t) => { }) // TODO: remove timeout -test.only('data sync state is properly updated as data sync is enabled and disabled', async (t) => { +test('data sync state is properly updated as data sync is enabled and disabled', async (t) => { const managers = await createManagers(3, t) const [invitor, ...invitees] = managers From 151c2f2fbc1cb0f658d090d12ac5ba883b42ddb2 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 20:26:56 +0000 Subject: [PATCH 07/20] fix core sync state tests --- src/sync/core-sync-state.js | 17 +++++++++-------- tests/sync/core-sync-state.js | 10 +++++----- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/sync/core-sync-state.js b/src/sync/core-sync-state.js index c67df797a..a88f5f6e8 100644 --- a/src/sync/core-sync-state.js +++ b/src/sync/core-sync-state.js @@ -363,6 +363,7 @@ export function deriveState(coreState) { // how to expose this state in a meaningful way for considering sync // completion, because blocked peers do not sync. if (isBlocked) continue + // TODO // peerIds.push(peerId) peers.set(peerId, peerState) @@ -380,24 +381,24 @@ export function deriveState(coreState) { const localHaves = coreState.localState.haveWord(i) & truncate localState.have += bitCount32(localHaves) - for (const [peerId, peer] of peers.entries()) { - // Haves + let wantFromAnyElse = 0 + let iWantFromOthers = 0 + for (const [peerId, peer] of peers.entries()) { const peerHaves = peer.haveWord(i) & truncate remoteStates[peerId].have += bitCount32(peerHaves) - // Wants (from me) - const wantsFromMe = peer.wantWord(i) & ~peerHaves & localHaves remoteStates[peerId].want += bitCount32(wantsFromMe) - localState.wanted += bitCount32(wantsFromMe) // TODO: does this work? - - // I want from them + wantFromAnyElse |= wantsFromMe const wantedByMe = peerHaves & ~localHaves remoteStates[peerId].wanted += bitCount32(wantedByMe) - localState.want += bitCount32(wantedByMe) // TODO: does this work? + iWantFromOthers |= wantedByMe } + + localState.wanted += bitCount32(wantFromAnyElse) + localState.want += bitCount32(iWantFromOthers) } return { diff --git a/tests/sync/core-sync-state.js b/tests/sync/core-sync-state.js index 206fb0699..0854899f4 100644 --- a/tests/sync/core-sync-state.js +++ b/tests/sync/core-sync-state.js @@ -42,13 +42,13 @@ const scenarios = [ peer0: { want: 1, have: 2, - wanted: 1, + wanted: 0, status: 'disconnected', }, peer1: { want: 1, have: 2, - wanted: 1, + wanted: 0, status: 'disconnected', }, peer2: { @@ -179,19 +179,19 @@ const scenarios = [ peer0: { want: 10, have: 40, - wanted: 5, + wanted: 0, status: 'disconnected', }, peer1: { want: 5, have: 40, - wanted: 10, + wanted: 0, status: 'disconnected', }, peer2: { want: 5, have: 40, - wanted: 10, + wanted: 0, status: 'disconnected', }, }, From 7b99250ad7b5ce6855d017995d833d2c51aef843 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 20:34:01 +0000 Subject: [PATCH 08/20] Remove filter-obj --- package-lock.json | 13 ------------- package.json | 1 - test-e2e/sync.js | 1 - 3 files changed, 15 deletions(-) diff --git a/package-lock.json b/package-lock.json index 414802999..fbb2851b4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -82,7 +82,6 @@ "cpy-cli": "^5.0.0", "drizzle-kit": "^0.20.14", "eslint": "^8.57.0", - "filter-obj": "^6.0.0", "husky": "^8.0.0", "iterpal": "^0.4.0", "light-my-request": "^5.10.0", @@ -4129,18 +4128,6 @@ "node": ">=8" } }, - "node_modules/filter-obj": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/filter-obj/-/filter-obj-6.0.0.tgz", - "integrity": "sha512-lscJ1kdwOyW2Nb3wcoU/LnNLoHg+Weoe7r8NT8xjNAdIZPm/JQuKoMcu3FnWrlKOjtAcf98pewuUyjeD6GoKig==", - "dev": true, - "engines": { - "node": ">=18" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, "node_modules/find-my-way": { "version": "8.1.0", "resolved": "https://registry.npmjs.org/find-my-way/-/find-my-way-8.1.0.tgz", diff --git a/package.json b/package.json index bb8767db7..c367d20f7 100644 --- a/package.json +++ b/package.json @@ -128,7 +128,6 @@ "cpy-cli": "^5.0.0", "drizzle-kit": "^0.20.14", "eslint": "^8.57.0", - "filter-obj": "^6.0.0", "husky": "^8.0.0", "iterpal": "^0.4.0", "light-my-request": "^5.10.0", diff --git a/test-e2e/sync.js b/test-e2e/sync.js index 1dce1c03c..dd075fb51 100644 --- a/test-e2e/sync.js +++ b/test-e2e/sync.js @@ -4,7 +4,6 @@ import * as fs from 'node:fs/promises' import { pEvent } from 'p-event' import { setTimeout as delay } from 'timers/promises' import { request } from 'undici' -// import { excludeKeys } from 'filter-obj' TODO import { isDeepStrictEqual } from 'node:util' import FakeTimers from '@sinonjs/fake-timers' import Fastify from 'fastify' From d2270377fbcf1c32100eebbd6f5ab58d16a3eb4e Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 20:39:03 +0000 Subject: [PATCH 09/20] generate lazily --- src/sync/sync-api.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/sync/sync-api.js b/src/sync/sync-api.js index ba854f742..7abdcf6c3 100644 --- a/src/sync/sync-api.js +++ b/src/sync/sync-api.js @@ -439,11 +439,6 @@ function getRemoteDevicesSyncState(namespaceSyncState, peerSyncControllers) { for (const psc of peerSyncControllers) { const { peerId } = psc - result[peerId] = { - initial: { isSyncEnabled: false, want: 0, wanted: 0 }, - data: { isSyncEnabled: false, want: 0, wanted: 0 }, - } - for (const namespace of NAMESPACES) { const isBlocked = psc.syncCapability[namespace] === 'blocked' if (isBlocked) continue @@ -465,6 +460,13 @@ function getRemoteDevicesSyncState(namespaceSyncState, peerSyncControllers) { throw new ExhaustivenessError(peerCoreState.status) } + if (!Object.hasOwn(result, peerId)) { + result[peerId] = { + initial: { isSyncEnabled: false, want: 0, wanted: 0 }, + data: { isSyncEnabled: false, want: 0, wanted: 0 }, + } + } + const namespaceGroup = PRESYNC_NAMESPACES.includes(namespace) ? 'initial' : 'data' From d737f00a787f62088d6eba460ab1d4367d17794a Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 20:41:25 +0000 Subject: [PATCH 10/20] test cleanup --- test-e2e/sync.js | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/test-e2e/sync.js b/test-e2e/sync.js index dd075fb51..9b92a794d 100644 --- a/test-e2e/sync.js +++ b/test-e2e/sync.js @@ -885,7 +885,6 @@ test('pre-haves are updated', async (t) => { ) }) -// TODO: remove timeout test('data sync state is properly updated as data sync is enabled and disabled', async (t) => { const managers = await createManagers(3, t) const [invitor, ...invitees] = managers @@ -957,18 +956,12 @@ test('data sync state is properly updated as data sync is enabled and disabled', 'data sync is still disabled for remote peers' ) - /* eslint-disable-next-line no-unused-vars */ const inviteeAppearsEnabledPromise = pEvent( invitorProject.$sync, 'sync-state', ({ remoteDeviceSyncState }) => remoteDeviceSyncState[invitees[0].deviceId]?.data.isSyncEnabled - ) // this should resolve with the first invitee's data.isSyncEnabled value - - // commenting this will crash the db - await delay(2000) - - /* eslint-disable-next-line no-unused-vars */ + ) const invitorProjectSyncedWithFirstInviteePromise = pEvent( invitorProject.$sync, 'sync-state', @@ -976,8 +969,7 @@ test('data sync state is properly updated as data sync is enabled and disabled', const remoteData = remoteDeviceSyncState[invitees[0].deviceId]?.data ?? {} return remoteData.want + remoteData.wanted === 0 } - ) // this should resolve with the first invitee's data. want+wanted=0 - /* eslint-disable-next-line no-unused-vars */ + ) const firstInviteeProjectSyncedWithInvitorPromise = pEvent( inviteesProjects[0].$sync, 'sync-state', @@ -985,7 +977,7 @@ test('data sync state is properly updated as data sync is enabled and disabled', const remoteData = remoteDeviceSyncState[invitor.deviceId]?.data ?? {} return remoteData.want + remoteData.wanted === 0 } - ) // this should resolve with the invitor's data. want+wanted + ) inviteesProjects[0].$sync.start() From a94def0efd7adeec0ba26905e3c38c3c13010370 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 20:42:17 +0000 Subject: [PATCH 11/20] remove a bunch of commented-out code --- src/sync/core-sync-state.js | 72 +------------------------------------ 1 file changed, 1 insertion(+), 71 deletions(-) diff --git a/src/sync/core-sync-state.js b/src/sync/core-sync-state.js index a88f5f6e8..ded826bb7 100644 --- a/src/sync/core-sync-state.js +++ b/src/sync/core-sync-state.js @@ -363,8 +363,6 @@ export function deriveState(coreState) { // how to expose this state in a meaningful way for considering sync // completion, because blocked peers do not sync. if (isBlocked) continue - // TODO - // peerIds.push(peerId) peers.set(peerId, peerState) remoteStates[peerId] = { @@ -401,75 +399,7 @@ export function deriveState(coreState) { localState.want += bitCount32(iWantFromOthers) } - return { - coreLength, - localState, - remoteStates, - } - - // ----------------- - - // const peerIds = ['local'] - // const peers = [coreState.localState] - - // for (const [peerId, peerState] of coreState.remoteStates.entries()) { - // const psc = coreState.peerSyncControllers.get(peerId) - // const isBlocked = psc?.syncCapability[coreState.namespace] === 'blocked' - // // Currently we do not include blocked peers in sync state - it's unclear - // // how to expose this state in a meaningful way for considering sync - // // completion, because blocked peers do not sync. - // if (isBlocked) continue - // peerIds.push(peerId) - // peers.push(peerState) - // } - - // /** @type {CoreState[]} */ - // const peerStates = new Array(peers.length) - // const length = coreState.length || 0 - // for (let i = 0; i < peerStates.length; i++) { - // peerStates[i] = { want: 0, have: 0, wanted: 0 } - // } - // const haves = new Array(peerStates.length) - // let want = 0 - // for (let i = 0; i < length; i += 32) { - // const truncate = 2 ** Math.min(32, length - i) - 1 - // let someoneHasIt = 0 - // for (let j = 0; j < peers.length; j++) { - // haves[j] = peers[j].haveWord(i) & truncate - // someoneHasIt |= haves[j] - // peerStates[j].have += bitCount32(haves[j]) - // } - // let someoneWantsIt = 0 - // for (let j = 0; j < peers.length; j++) { - // // A block is a want if: - // // 1. The peer wants it - // // 2. They don't have it - // // 3. Someone does have it - // const wouldLikeIt = peers[j].wantWord(i) & ~haves[j] - // want = wouldLikeIt & someoneHasIt - // someoneWantsIt |= want - // peerStates[j].want += bitCount32(want) - // } - // for (let j = 0; j < peerStates.length; j++) { - // // A block is wanted if: - // // 1. Someone wants it - // // 2. The peer has it - // const wanted = someoneWantsIt & haves[j] - // peerStates[j].wanted += bitCount32(wanted) - // } - // } - // /** @type {DerivedState} */ - // const derivedState = { - // coreLength: length, - // localState: peerStates[0], - // remoteStates: {}, - // } - // for (let j = 1; j < peerStates.length; j++) { - // const peerState = /** @type {PeerCoreState} */ (peerStates[j]) - // peerState.status = peers[j].status - // derivedState.remoteStates[peerIds[j]] = peerState - // } - // return derivedState + return { coreLength, localState, remoteStates } } /** From dbc457801738aa1a6a097d372bb15e31a529664f Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 20:44:09 +0000 Subject: [PATCH 12/20] shrink the diff --- src/sync/core-sync-state.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/sync/core-sync-state.js b/src/sync/core-sync-state.js index ded826bb7..de25b93ca 100644 --- a/src/sync/core-sync-state.js +++ b/src/sync/core-sync-state.js @@ -347,8 +347,7 @@ export class PeerState { * Only exporteed for testing */ export function deriveState(coreState) { - // TODO: consider renaming this to `length` to make the diff smaller - const coreLength = coreState.length || 0 + const length = coreState.length || 0 /** @type {CoreState} */ const localState = { have: 0, want: 0, wanted: 0 } /** @type {Record} */ @@ -373,8 +372,8 @@ export function deriveState(coreState) { } } - for (let i = 0; i < coreLength; i += 32) { - const truncate = 2 ** Math.min(32, coreLength - i) - 1 + for (let i = 0; i < length; i += 32) { + const truncate = 2 ** Math.min(32, length - i) - 1 const localHaves = coreState.localState.haveWord(i) & truncate localState.have += bitCount32(localHaves) @@ -399,7 +398,11 @@ export function deriveState(coreState) { localState.want += bitCount32(iWantFromOthers) } - return { coreLength, localState, remoteStates } + return { + coreLength: length, + localState, + remoteStates, + } } /** From 78c6bf5c52d1b8c5e3bbc893e8405bd65efbf881 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 20:45:30 +0000 Subject: [PATCH 13/20] naming cleanups --- src/sync/core-sync-state.js | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/sync/core-sync-state.js b/src/sync/core-sync-state.js index de25b93ca..225fefb77 100644 --- a/src/sync/core-sync-state.js +++ b/src/sync/core-sync-state.js @@ -363,7 +363,6 @@ export function deriveState(coreState) { // completion, because blocked peers do not sync. if (isBlocked) continue peers.set(peerId, peerState) - remoteStates[peerId] = { have: 0, want: 0, @@ -378,24 +377,24 @@ export function deriveState(coreState) { const localHaves = coreState.localState.haveWord(i) & truncate localState.have += bitCount32(localHaves) - let wantFromAnyElse = 0 - let iWantFromOthers = 0 + let someoneElseWantsFromMe = 0 + let iWantFromSomeoneElse = 0 for (const [peerId, peer] of peers.entries()) { const peerHaves = peer.haveWord(i) & truncate remoteStates[peerId].have += bitCount32(peerHaves) - const wantsFromMe = peer.wantWord(i) & ~peerHaves & localHaves - remoteStates[peerId].want += bitCount32(wantsFromMe) - wantFromAnyElse |= wantsFromMe + const theyWantFromMe = peer.wantWord(i) & ~peerHaves & localHaves + remoteStates[peerId].want += bitCount32(theyWantFromMe) + someoneElseWantsFromMe |= theyWantFromMe - const wantedByMe = peerHaves & ~localHaves - remoteStates[peerId].wanted += bitCount32(wantedByMe) - iWantFromOthers |= wantedByMe + const iWantFromThem = peerHaves & ~localHaves + remoteStates[peerId].wanted += bitCount32(iWantFromThem) + iWantFromSomeoneElse |= iWantFromThem } - localState.wanted += bitCount32(wantFromAnyElse) - localState.want += bitCount32(iWantFromOthers) + localState.wanted += bitCount32(someoneElseWantsFromMe) + localState.want += bitCount32(iWantFromSomeoneElse) } return { From 4a4cf549f10aa08e17a11e067ab9738ba688b838 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 20:46:25 +0000 Subject: [PATCH 14/20] change docs for deriveState --- src/sync/core-sync-state.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/sync/core-sync-state.js b/src/sync/core-sync-state.js index 225fefb77..636615b46 100644 --- a/src/sync/core-sync-state.js +++ b/src/sync/core-sync-state.js @@ -337,9 +337,7 @@ export class PeerState { } /** - * Derive count for each peer: "want"; "have"; "wanted". There is definitely a - * more performant and clever way of doing this, but at least with this - * implementation I can understand what I am doing. + * Derive count for each peer: "want"; "have"; "wanted". * * @param {InternalState} coreState * From 3a0b9e4d7f202fcdb4ba34b872b989d0d473a4ff Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 21:07:38 +0000 Subject: [PATCH 15/20] clean up some types and docs --- src/sync/core-sync-state.js | 18 +++++++++++------- src/sync/peer-sync-controller.js | 2 +- tsconfig.json | 2 +- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/sync/core-sync-state.js b/src/sync/core-sync-state.js index 636615b46..98de079b9 100644 --- a/src/sync/core-sync-state.js +++ b/src/sync/core-sync-state.js @@ -18,18 +18,22 @@ import RemoteBitfield, { * @property {import('../core-manager/index.js').Namespace} namespace */ /** - * @typedef {object} CoreState - * @property {number} have blocks the peer has locally - * @property {number} want blocks this peer wants from us TODO: derive? - * @property {number} wanted blocks we want from this peer TODO: derivce? + * @typedef {object} LocalCoreState + * @property {number} have blocks we have + * @property {number} want unique blocks we want from any other peer + * @property {number} wanted blocks we want from this peer */ /** - * @typedef {CoreState & { status: 'disconnected' | 'connecting' | 'connected' }} PeerCoreState + * @typedef {object} PeerCoreState + * @property {number} have blocks the peer has locally + * @property {number} want blocks this peer wants from us + * @property {number} wanted blocks we want from this peer + * @property {'disconnected' | 'connecting' | 'connected'} status */ /** * @typedef {object} DerivedState * @property {number} coreLength known (sparse) length of the core - * @property {CoreState} localState local state + * @property {LocalCoreState} localState local state * @property {{ [peerId in PeerId]: PeerCoreState }} remoteStates map of state of all known peers */ @@ -346,7 +350,7 @@ export class PeerState { */ export function deriveState(coreState) { const length = coreState.length || 0 - /** @type {CoreState} */ + /** @type {LocalCoreState} */ const localState = { have: 0, want: 0, wanted: 0 } /** @type {Record} */ const remoteStates = {} diff --git a/src/sync/peer-sync-controller.js b/src/sync/peer-sync-controller.js index 2393329f6..c669199ce 100644 --- a/src/sync/peer-sync-controller.js +++ b/src/sync/peer-sync-controller.js @@ -29,7 +29,7 @@ export class PeerSyncController { #syncCapability = createNamespaceMap('unknown') /** @type {SyncEnabledState} */ #syncEnabledState = 'none' - /** @type {Record} */ + /** @type {Record} */ #prevLocalState = createNamespaceMap(null) /** @type {SyncStatus} */ #syncStatus = createNamespaceMap('unknown') diff --git a/tsconfig.json b/tsconfig.json index 8d5195977..000250002 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -26,6 +26,6 @@ "*": ["./types", "./node_modules/@digidem/types/vendor/*/index.d.ts"] } }, - "include": ["src/**/*", "types/**/*.d.ts", "tests/**/*", "test-e2e/**/*"], + "include": ["src/**/*", "types/**/*.d.ts"], "exclude": ["node_modules", "src/generated/*.js"] } From c991d78d451d13b5d2866151159eaf7a78b52d57 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 21:10:48 +0000 Subject: [PATCH 16/20] more doc cleanup --- src/sync/core-sync-state.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/sync/core-sync-state.js b/src/sync/core-sync-state.js index 98de079b9..d12eb5aa4 100644 --- a/src/sync/core-sync-state.js +++ b/src/sync/core-sync-state.js @@ -48,10 +48,16 @@ import RemoteBitfield, { * "pull" the state when it wants it via `coreSyncState.getState()`. * * Each peer (including the local peer) has a state of: - * 1. `have` - number of blocks the peer has locally - * 2. `want` - TODO - * 3. `wanted` - TODO * + * 1. `have` - number of blocks the peer has locally + * + * 2. `want` - number of blocks this peer wants. For local state, this is the + * number of unique blocks we want from anyone else. For remote peers, it is + * the number of blocks this peer wants from us. + * + * 3. `wanted` - number of blocks this peer has that's wanted by others. For + * local state, this is the number of unique blocks any of our peers want. + * For remote peers, it is the number of blocks we want from them. */ export class CoreSyncState { /** @type {import('hypercore')<'binary', Buffer> | undefined} */ From 36f50c014362bb1ba237402703d7dc5bcd54c2fc Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 21:11:58 +0000 Subject: [PATCH 17/20] Remove a comment --- test-e2e/sync.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test-e2e/sync.js b/test-e2e/sync.js index 9b92a794d..31cf77319 100644 --- a/test-e2e/sync.js +++ b/test-e2e/sync.js @@ -648,14 +648,6 @@ test('no sync capabilities === no namespaces sync apart from auth', async (t) => assert.equal(inviteeState[ns].coreCount, 2) assert.equal(blockedState[ns].coreCount, 1) } - - // TODO: Update this comment? - // "Invitor" knows blocked peer is blocked from the start, so never connects - // and never creates a local copy of the blocked peer cores, but "Invitee" - // does connect initially, before it realized the peer is blocked, and - // creates a local copy of the blocked peer's cores, but never downloads - // data, so it considers data to be "missing" which the Invitor does not - // register as missing. assert.deepEqual(invitorState[ns].localState, inviteeState[ns].localState) } From 226d1eee9bb1b3719fa1afa9d449a9d53a96f8e2 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 21:12:08 +0000 Subject: [PATCH 18/20] remove TODO.txt --- TODO.txt | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 TODO.txt diff --git a/TODO.txt b/TODO.txt deleted file mode 100644 index a21068d7d..000000000 --- a/TODO.txt +++ /dev/null @@ -1,7 +0,0 @@ -- can we remove `have`? -- can we remove `missing`? -- can we remove `dataToSync`? -- docs -- maybe clean up `reduceSyncState` -- change names? -- all remaining TODOs From a64502517468f8ed8e82e5318575d599a136502b Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 21:13:48 +0000 Subject: [PATCH 19/20] revert tsconfig --- tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tsconfig.json b/tsconfig.json index 000250002..8d5195977 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -26,6 +26,6 @@ "*": ["./types", "./node_modules/@digidem/types/vendor/*/index.d.ts"] } }, - "include": ["src/**/*", "types/**/*.d.ts"], + "include": ["src/**/*", "types/**/*.d.ts", "tests/**/*", "test-e2e/**/*"], "exclude": ["node_modules", "src/generated/*.js"] } From 5db63eeb981f1f08e6f844f1e8cca5f8f5ed0089 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Tue, 20 Aug 2024 21:15:05 +0000 Subject: [PATCH 20/20] Renames --- src/sync/sync-api.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/sync/sync-api.js b/src/sync/sync-api.js index 7abdcf6c3..8854238b6 100644 --- a/src/sync/sync-api.js +++ b/src/sync/sync-api.js @@ -22,23 +22,25 @@ export const kRescindFullStopRequest = Symbol('foreground') */ /** - * @typedef {object} DeviceNamespaceGroupSyncState + * @internal + * @typedef {object} RemoteDeviceNamespaceGroupSyncState * @property {boolean} isSyncEnabled this device in a 'connected' state * @property {number} want number of docs wanted by this device * @property {number} wanted number of docs that other devices want from this device */ /** - * @typedef {object} DeviceSyncState state of sync for remote peer - * @property {DeviceNamespaceGroupSyncState} initial state of auth, metadata and project config - * @property {DeviceNamespaceGroupSyncState} data state of observations, map data, media attachments + * @internal + * @typedef {object} RemoteDeviceSyncState state of sync for remote peer + * @property {RemoteDeviceNamespaceGroupSyncState} initial state of auth, metadata and project config + * @property {RemoteDeviceNamespaceGroupSyncState} data state of observations, map data, media attachments */ /** * @typedef {object} State * @property {{ isSyncEnabled: boolean }} initial State of initial sync (sync of auth, metadata and project config) for local device * @property {{ isSyncEnabled: boolean }} data State of data sync (observations, map data, photos, audio, video etc.) for local device - * @property {Record} remoteDeviceSyncState map of peerId to DeviceSyncState. + * @property {Record} remoteDeviceSyncState map of peerId to DeviceSyncState. */ /**