Update tests to (maybe?) expose bug in sync state

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",

src/sync/core-sync-state.js

@@ -18,19 +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 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 {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
  */
 
@@ -45,11 +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` - 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
  *
+ * 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} */
@@ -339,85 +347,69 @@ 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
  *
  * @private
  * Only exporteed for testing
  */
 export function deriveState(coreState) {
-  const peerIds = ['local']
-  const peers = [coreState.localState]
+  const length = coreState.length || 0
+  /** @type {LocalCoreState} */
+  const localState = { have: 0, want: 0, wanted: 0 }
+  /** @type {Record<PeerId, PeerCoreState>} */
+  const remoteStates = {}
 
+  /** @type {Map<PeerId, PeerState>} */
+  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'
     // 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)
+    peers.set(peerId, peerState)
+    remoteStates[peerId] = {
+      have: 0,
+      want: 0,
+      wanted: 0,
+      status: peerState.status,
+    }
   }
 
-  /** @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, missing: 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)
-      // 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:
-      //   1. Someone wants it
-      //   2. The peer has it
-      const wanted = someoneWantsIt & haves[j]
-      peerStates[j].wanted += bitCount32(wanted)
+
+    const localHaves = coreState.localState.haveWord(i) & truncate
+    localState.have += bitCount32(localHaves)
+
+    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 theyWantFromMe = peer.wantWord(i) & ~peerHaves & localHaves
+      remoteStates[peerId].want += bitCount32(theyWantFromMe)
+      someoneElseWantsFromMe |= theyWantFromMe
+
+      const iWantFromThem = peerHaves & ~localHaves
+      remoteStates[peerId].wanted += bitCount32(iWantFromThem)
+      iWantFromSomeoneElse |= iWantFromThem
     }
+
+    localState.wanted += bitCount32(someoneElseWantsFromMe)
+    localState.want += bitCount32(iWantFromSomeoneElse)
   }
-  /** @type {DerivedState} */
-  const derivedState = {
+
+  return {
     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
+    localState,
+    remoteStates,
   }
-  return derivedState
 }
 
 /**

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'

src/sync/peer-sync-controller.js

@@ -29,7 +29,7 @@ export class PeerSyncController {
   #syncCapability = createNamespaceMap('unknown')
   /** @type {SyncEnabledState} */
   #syncEnabledState = 'none'
-  /** @type {Record<Namespace, import('./core-sync-state.js').CoreState | null>} */
+  /** @type {Record<Namespace, import('./core-sync-state.js').LocalCoreState | null>} */
   #prevLocalState = createNamespaceMap(null)
   /** @type {SyncStatus} */
   #syncStatus = createNamespaceMap('unknown')

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<string, DeviceSyncState>} remoteDeviceSyncState  map of peerId to DeviceSyncState.
+ * @property {Record<string, RemoteDeviceSyncState>} remoteDeviceSyncState  map of peerId to DeviceSyncState.
  */
 
 /**
@@ -439,11 +441,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 +462,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'