From ab0e61a9286d7660ed9b716c01c4e3860e704d43 Mon Sep 17 00:00:00 2001
From: Evan Hahn <me@evanhahn.com>
Date: Wed, 18 Sep 2024 22:49:40 +0000
Subject: [PATCH] fix: more lenient initial sync

When you join a project, you wait for some data to sync, such as the
project config. That effectively happens in two steps:

1. If sync is already done, return.
2. Otherwise, listen for sync state updates, and return when sync is
  done.

Before this change, these two steps answered "is sync done?" in slightly
different ways. After this change, they answer that in the same way.

Here's a diff in pseudocode:

```diff
 function waitForInitialSync() {
   if (!wantsDataFromOtherPeers()) return 'done'

   onSyncState(() => {
-    if (isSyncCompletelyFinished()) {
+    if (!wantsDataFromOtherPeers()) {
       return 'done'
     }
   })
 }
```

More specifically, this makes both parts check "is sync done?" by
looking at the `auth` and `config` namespaces. If they want nothing but
have something, we consider them done with initial sync.
---
 src/mapeo-manager.js | 45 +++++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/src/mapeo-manager.js b/src/mapeo-manager.js
index cdc253f9..db75a537 100644
--- a/src/mapeo-manager.js
+++ b/src/mapeo-manager.js
@@ -49,10 +49,11 @@ import {
   kRescindFullStopRequest,
 } from './sync/sync-api.js'
 /** @import { ProjectSettingsValue as ProjectValue } from '@comapeo/schema' */
-/** @import { SetNonNullable } from 'type-fest' */
+/** @import { ReadonlyDeep, SetNonNullable } from 'type-fest' */
 /** @import { CoreStorage, Namespace } from './types.js' */
 /** @import { DeviceInfoParam } from './schema/client.js' */
 /** @import { OpenedNoiseStream } from './lib/noise-secret-stream-helpers.js' */
+/** @import { State as SyncStateState } from './sync/sync-state.js' */
 
 /** @typedef {SetNonNullable<ProjectKeys, 'encryptionKeys'>} ValidatedProjectKeys */
 
@@ -654,25 +655,14 @@ export class MapeoManager extends TypedEmitter {
       project.$getOwnRole(),
       project.$getProjectSettings(),
     ])
-    const {
-      auth: { localState: authState },
-      config: { localState: configState },
-    } = project.$sync[kSyncState].getState()
+    const syncState = project.$sync[kSyncState].getState()
     const isRoleSynced = ownRole !== Roles.NO_ROLE
     const isProjectSettingsSynced =
       projectSettings !== MapeoProject.EMPTY_PROJECT_SETTINGS
-    // Assumes every project that someone is invited to has at least one record
-    // in the auth store - the row record for the invited device
-    const isAuthSynced = authState.want === 0 && authState.have > 0
-    // Assumes every project that someone is invited to has at least one record
-    // in the config store - defining the name of the project.
-    // TODO: Enforce adding a project name in the invite method
-    const isConfigSynced = configState.want === 0 && configState.have > 0
     if (
       isRoleSynced &&
       isProjectSettingsSynced &&
-      isAuthSynced &&
-      isConfigSynced
+      isSyncStateDoneWithInitialSync(syncState)
     ) {
       return true
     }
@@ -680,7 +670,7 @@ export class MapeoManager extends TypedEmitter {
       /** @param {import('./sync/sync-state.js').State} syncState */
       const onSyncState = (syncState) => {
         clearTimeout(timeoutId)
-        if (syncState.auth.dataToSync || syncState.config.dataToSync) {
+        if (!isSyncStateDoneWithInitialSync(syncState)) {
           timeoutId = setTimeout(onTimeout, timeoutMs)
           return
         }
@@ -870,6 +860,31 @@ export class MapeoManager extends TypedEmitter {
   }
 }
 
+/**
+ * Is the sync state done with initial sync?
+ *
+ * Assumes every project that someone is invited to has at least one record in
+ * the auth store (a record for the invited device) and the config store (the
+ * project name).
+ *
+ * @param {ReadonlyDeep<SyncStateState>} syncState
+ * @returns {boolean}
+ */
+function isSyncStateDoneWithInitialSync(syncState) {
+  return (
+    isNamespaceDoneWithInitialSync(syncState.auth.localState) &&
+    isNamespaceDoneWithInitialSync(syncState.config.localState)
+  )
+}
+
+/**
+ * @param {ReadonlyDeep<{ have: number, want: number }>} namespaceSyncState
+ * @returns {boolean}
+ */
+function isNamespaceDoneWithInitialSync({ want, have }) {
+  return want === 0 && have > 0
+}
+
 // We use the `protomux` property of connected peers internally, but we don't
 // expose it to the API. I have avoided using a private symbol for this for fear
 // that we could accidentally keep references around of protomux instances,