Ignore GV1 records in storage service.

2026-08-05 21:07:49 +01:00 · 2026-07-29 13:05:12 -04:00
parent a28dfa40cc
commit a92304c222
10 changed files with 266 additions and 187 deletions
@@ -107,7 +107,6 @@ import org.thoughtcrime.securesms.wallpaper.WallpaperStorage
 import org.whispersystems.signalservice.api.profiles.SignalServiceProfile
 import org.whispersystems.signalservice.api.storage.SignalAccountRecord
 import org.whispersystems.signalservice.api.storage.SignalContactRecord
-import org.whispersystems.signalservice.api.storage.SignalGroupV1Record
 import org.whispersystems.signalservice.api.storage.SignalGroupV2Record
 import org.whispersystems.signalservice.api.storage.StorageId
 import org.whispersystems.signalservice.api.storage.signalAci
@@ -1018,27 +1017,6 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
    }
  }

-  fun applyStorageSyncGroupV1Insert(insert: SignalGroupV1Record) {
-    val id = writableDatabase.insertOrThrow(TABLE_NAME, null, getValuesForStorageGroupV1(insert, true))
-
-    val recipientId = RecipientId.from(id)
-    threads.applyStorageSyncUpdate(recipientId, insert)
-    AppDependencies.databaseObserver.notifyRecipientChanged(recipientId)
-  }
-
-  fun applyStorageSyncGroupV1Update(update: StorageRecordUpdate<SignalGroupV1Record>) {
-    val values = getValuesForStorageGroupV1(update.new, false)
-
-    val updateCount = writableDatabase.update(TABLE_NAME, values, STORAGE_SERVICE_ID + " = ?", arrayOf(Base64.encodeWithPadding(update.old.id.raw)))
-    if (updateCount < 1) {
-      throw AssertionError("Had an update, but it didn't match any rows!")
-    }
-
-    val recipient = Recipient.externalGroupExact(GroupId.v1orThrow(update.old.proto.id.toByteArray()))
-    threads.applyStorageSyncUpdate(recipient.id, update.new)
-    recipient.live().refresh()
-  }
-
  fun applyStorageSyncGroupV2Insert(insert: SignalGroupV2Record) {
    val masterKey = GroupMasterKey(insert.proto.masterKey.toByteArray())
    val groupId = GroupId.v2(masterKey)
@@ -1285,8 +1263,6 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
        $STORAGE_SERVICE_ID NOT NULL AND (
            ($TYPE = ${RecipientType.INDIVIDUAL.id} AND ($ACI_COLUMN NOT NULL OR $PNI_COLUMN NOT NULL) AND $ID != ${Recipient.self().id.toLong()})
            OR
-            $TYPE = ${RecipientType.GV1.id}
-            OR
            ($TYPE = ${RecipientType.DISTRIBUTION_LIST.id} AND $DISTRIBUTION_LIST_ID NOT NULL AND $DISTRIBUTION_LIST_ID IN (
              SELECT ${DistributionListTables.ListTable.ID}
              FROM ${DistributionListTables.ListTable.TABLE_NAME}
@@ -1318,7 +1294,6 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
                out[id] = StorageId.forContact(key)
              }
            }
-            RecipientType.GV1 -> out[id] = StorageId.forGroupV1(key)
            RecipientType.DISTRIBUTION_LIST -> out[id] = StorageId.forStoryDistributionList(key)
            RecipientType.CALL_LINK -> out[id] = StorageId.forCallLink(key)
            else -> throw AssertionError()
@@ -4481,29 +4456,6 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
    }
  }

-  private fun getValuesForStorageGroupV1(groupV1: SignalGroupV1Record, isInsert: Boolean): ContentValues {
-    return ContentValues().apply {
-      val groupId = GroupId.v1orThrow(groupV1.proto.id.toByteArray())
-
-      put(GROUP_ID, groupId.toString())
-      put(TYPE, RecipientType.GV1.id)
-      put(PROFILE_SHARING, if (groupV1.proto.whitelisted) "1" else "0")
-      put(BLOCKED, if (groupV1.proto.blocked) "1" else "0")
-      put(MUTE_UNTIL, groupV1.proto.mutedUntilTimestamp)
-      put(STORAGE_SERVICE_ID, Base64.encodeWithPadding(groupV1.id.raw))
-
-      if (groupV1.proto.hasUnknownFields()) {
-        put(STORAGE_SERVICE_PROTO, Base64.encodeWithPadding(groupV1.serializedUnknowns!!))
-      } else {
-        putNull(STORAGE_SERVICE_PROTO)
-      }
-
-      if (isInsert) {
-        put(AVATAR_COLOR, AvatarColorHash.forGroupId(groupId).serialize())
-      }
-    }
-  }
-
  private fun getValuesForStorageGroupV2(groupV2: SignalGroupV2Record, isInsert: Boolean): ContentValues {
    return ContentValues().apply {
      val groupId = GroupId.v2(GroupMasterKey(groupV2.proto.masterKey.toByteArray()))
@@ -82,7 +82,6 @@ import org.thoughtcrime.securesms.util.isPoll
 import org.thoughtcrime.securesms.util.isScheduled
 import org.whispersystems.signalservice.api.storage.SignalAccountRecord
 import org.whispersystems.signalservice.api.storage.SignalContactRecord
-import org.whispersystems.signalservice.api.storage.SignalGroupV1Record
 import org.whispersystems.signalservice.api.storage.SignalGroupV2Record
 import org.whispersystems.signalservice.api.storage.toSignalServiceAddress
 import org.whispersystems.signalservice.internal.storage.protos.AccountRecord
@@ -1608,10 +1607,6 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
    applyStorageSyncUpdate(recipientId, record.proto.archived, record.proto.markedUnread, isGroup = false)
  }

-  fun applyStorageSyncUpdate(recipientId: RecipientId, record: SignalGroupV1Record) {
-    applyStorageSyncUpdate(recipientId, record.proto.archived, record.proto.markedUnread, isGroup = true)
-  }
-
  fun applyStorageSyncUpdate(recipientId: RecipientId, record: SignalGroupV2Record) {
    applyStorageSyncUpdate(recipientId, record.proto.archived, record.proto.markedUnread, isGroup = true)
  }
@@ -176,6 +176,7 @@ import org.thoughtcrime.securesms.database.helpers.migration.V320_AddAttachmentT
 import org.thoughtcrime.securesms.database.helpers.migration.V321_AddScheduledMessageIndex
 import org.thoughtcrime.securesms.database.helpers.migration.V322_NormalizeStickerTable
 import org.thoughtcrime.securesms.database.helpers.migration.V323_AddStickerPackStorageSync
+import org.thoughtcrime.securesms.database.helpers.migration.V324_MoveGroupV1StorageIdsToUnknownIds
 import org.thoughtcrime.securesms.database.SQLiteDatabase as SignalSqliteDatabase

 /**
@@ -359,10 +360,11 @@ object SignalDatabaseMigrations {
    320 to V320_AddAttachmentThumbnailFileAndUuidIndexes,
    321 to V321_AddScheduledMessageIndex,
    322 to V322_NormalizeStickerTable,
-    323 to V323_AddStickerPackStorageSync
+    323 to V323_AddStickerPackStorageSync,
+    324 to V324_MoveGroupV1StorageIdsToUnknownIds
  )

-  const val DATABASE_VERSION = 323
+  const val DATABASE_VERSION = 324

  @JvmStatic
  fun migrate(context: Application, db: SignalSqliteDatabase, oldVersion: Int, newVersion: Int) {
@@ -0,0 +1,33 @@
+/*
+ * Copyright 2026 Signal Messenger, LLC
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+package org.thoughtcrime.securesms.database.helpers.migration
+
+import android.app.Application
+import org.thoughtcrime.securesms.database.SQLiteDatabase
+
+/**
+ * Copies the storage id off of every gv1 recipient row into the unknown id table so the ids stay in the storage
+ * service manifest, then clears the storage columns on those rows.
+ *
+ * A gv1 row can still be given a storage id after this runs, it is just never read, so this is not a lasting invariant.
+ */
+@Suppress("ClassName")
+object V324_MoveGroupV1StorageIdsToUnknownIds : SignalDatabaseMigration {
+
+  private const val RECIPIENT_TYPE_GV1 = 2
+  private const val MANIFEST_TYPE_GROUPV1 = 2
+
+  override fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) {
+    db.execSQL(
+      """
+      INSERT OR IGNORE INTO storage_key (type, key)
+      SELECT $MANIFEST_TYPE_GROUPV1, storage_service_id FROM recipient WHERE type = $RECIPIENT_TYPE_GV1 AND storage_service_id NOT NULL
+      """
+    )
+
+    db.execSQL("UPDATE recipient SET storage_service_id = NULL, storage_service_proto = NULL WHERE type = $RECIPIENT_TYPE_GV1")
+  }
+}
@@ -26,7 +26,6 @@ import org.thoughtcrime.securesms.storage.AccountRecordProcessor
 import org.thoughtcrime.securesms.storage.CallLinkRecordProcessor
 import org.thoughtcrime.securesms.storage.ChatFolderRecordProcessor
 import org.thoughtcrime.securesms.storage.ContactRecordProcessor
-import org.thoughtcrime.securesms.storage.GroupV1RecordProcessor
 import org.thoughtcrime.securesms.storage.GroupV2RecordProcessor
 import org.thoughtcrime.securesms.storage.NotificationProfileRecordProcessor
 import org.thoughtcrime.securesms.storage.StickerPackRecordProcessor
@@ -45,7 +44,6 @@ import org.whispersystems.signalservice.api.storage.SignalAccountRecord
 import org.whispersystems.signalservice.api.storage.SignalCallLinkRecord
 import org.whispersystems.signalservice.api.storage.SignalChatFolderRecord
 import org.whispersystems.signalservice.api.storage.SignalContactRecord
-import org.whispersystems.signalservice.api.storage.SignalGroupV1Record
 import org.whispersystems.signalservice.api.storage.SignalGroupV2Record
 import org.whispersystems.signalservice.api.storage.SignalNotificationProfileRecord
 import org.whispersystems.signalservice.api.storage.SignalStickerPackRecord
@@ -57,7 +55,6 @@ import org.whispersystems.signalservice.api.storage.toSignalAccountRecord
 import org.whispersystems.signalservice.api.storage.toSignalCallLinkRecord
 import org.whispersystems.signalservice.api.storage.toSignalChatFolderRecord
 import org.whispersystems.signalservice.api.storage.toSignalContactRecord
-import org.whispersystems.signalservice.api.storage.toSignalGroupV1Record
 import org.whispersystems.signalservice.api.storage.toSignalGroupV2Record
 import org.whispersystems.signalservice.api.storage.toSignalNotificationProfileRecord
 import org.whispersystems.signalservice.api.storage.toSignalStickerPackRecord
@@ -336,7 +333,7 @@ class StorageSyncJob private constructor(parameters: Parameters, private var loc

        db.beginTransaction()
        try {
-          Log.i(TAG, "[Remote Sync] Remote-Only :: Contacts: ${remoteOnly.contacts.size}, GV1: ${remoteOnly.gv1.size}, GV2: ${remoteOnly.gv2.size}, Account: ${remoteOnly.account.size}, DLists: ${remoteOnly.storyDistributionLists.size}, call links: ${remoteOnly.callLinkRecords.size}, chat folders: ${remoteOnly.chatFolderRecords.size}, notification profiles: ${remoteOnly.notificationProfileRecords.size}, sticker packs: ${remoteOnly.stickerPackRecords.size}")
+          Log.i(TAG, "[Remote Sync] Remote-Only :: Contacts: ${remoteOnly.contacts.size}, GV2: ${remoteOnly.gv2.size}, Account: ${remoteOnly.account.size}, DLists: ${remoteOnly.storyDistributionLists.size}, call links: ${remoteOnly.callLinkRecords.size}, chat folders: ${remoteOnly.chatFolderRecords.size}, notification profiles: ${remoteOnly.notificationProfileRecords.size}, sticker packs: ${remoteOnly.stickerPackRecords.size}")

          processKnownRecords(context, remoteOnly)

@@ -409,8 +406,20 @@ class StorageSyncJob private constructor(parameters: Parameters, private var loc
        Log.i(TAG, "Removed $removedUnregistered unregistered, $removedDeletedFolders folders, $removedDeletedProfiles notification profiles, $removedDeletedPacks sticker packs from storage service that have been deleted for longer than ${RemoteConfig.messageQueueTime.milliseconds.inWholeDays} days.")
      }

-      val localStorageIds = getAllLocalStorageIds(self)
-      val idDifference = StorageSyncHelper.findIdDifference(remoteManifest.storageIds, localStorageIds)
+      var localStorageIds = getAllLocalStorageIds(self)
+      var idDifference = StorageSyncHelper.findIdDifference(remoteManifest.storageIds, localStorageIds)
+
+      // We can't build a record for an unknown id, so declaring one the remote doesn't already have would fail validation.
+      val localOnlyUnknownIds = idDifference.localOnlyIds.filter { it.isUnknown }
+      if (localOnlyUnknownIds.isNotEmpty()) {
+        Log.w(TAG, "Found ${localOnlyUnknownIds.size} unknown ids that aren't in the remote manifest. Removed them. Recalculating diff.")
+
+        SignalDatabase.unknownStorageIds.delete(localOnlyUnknownIds)
+
+        localStorageIds = getAllLocalStorageIds(self)
+        idDifference = StorageSyncHelper.findIdDifference(remoteManifest.storageIds, localStorageIds)
+      }
+
      val remoteInserts = buildLocalStorageRecords(context, self, idDifference.localOnlyIds.stream().filter { it: StorageId -> !it.isUnknown }.collect(Collectors.toList()))
      val remoteDeletes = idDifference.remoteOnlyIds.stream().map { obj: StorageId -> obj.raw }.collect(Collectors.toList())

@@ -469,7 +478,6 @@ class StorageSyncJob private constructor(parameters: Parameters, private var loc
  @Throws(IOException::class)
  private fun processKnownRecords(context: Context, records: StorageRecordCollection) {
    ContactRecordProcessor().process(records.contacts, StorageSyncHelper.KEY_GENERATOR)
-    GroupV1RecordProcessor().process(records.gv1, StorageSyncHelper.KEY_GENERATOR)
    GroupV2RecordProcessor().process(records.gv2, StorageSyncHelper.KEY_GENERATOR)
    NotificationProfileRecordProcessor().process(records.notificationProfileRecords, StorageSyncHelper.KEY_GENERATOR)
    AccountRecordProcessor(context, freshSelf()).process(records.account, StorageSyncHelper.KEY_GENERATOR)
@@ -502,7 +510,7 @@ class StorageSyncJob private constructor(parameters: Parameters, private var loc
      }

      when (type) {
-        ManifestRecord.Identifier.Type.CONTACT, ManifestRecord.Identifier.Type.GROUPV1, ManifestRecord.Identifier.Type.GROUPV2 -> {
+        ManifestRecord.Identifier.Type.CONTACT, ManifestRecord.Identifier.Type.GROUPV2 -> {
          val settings = SignalDatabase.recipients.getByStorageId(id.raw)
          if (settings != null) {
            if (settings.recipientType == RecipientTable.RecipientType.GV2 && settings.syncExtras.groupMasterKey == null) {
@@ -599,13 +607,12 @@ class StorageSyncJob private constructor(parameters: Parameters, private var loc

  private fun getKnownTypes(): List<Int> {
    return ManifestRecord.Identifier.Type.entries
-      .filter { it != ManifestRecord.Identifier.Type.UNKNOWN }
      .map { it.value }
+      .filter { StorageId.isKnownType(it) }
  }

  private class StorageRecordCollection(records: Collection<SignalStorageRecord>) {
    val contacts: MutableList<SignalContactRecord> = mutableListOf()
-    val gv1: MutableList<SignalGroupV1Record> = mutableListOf()
    val gv2: MutableList<SignalGroupV2Record> = mutableListOf()
    val account: MutableList<SignalAccountRecord> = mutableListOf()
    val unknown: MutableList<SignalStorageRecord> = mutableListOf()
@@ -619,8 +626,6 @@ class StorageSyncJob private constructor(parameters: Parameters, private var loc
      for (record in records) {
        if (record.proto.contact != null) {
          contacts += record.proto.contact!!.toSignalContactRecord(record.id)
-        } else if (record.proto.groupV1 != null) {
-          gv1 += record.proto.groupV1!!.toSignalGroupV1Record(record.id)
        } else if (record.proto.groupV2 != null) {
          gv2 += record.proto.groupV2!!.toSignalGroupV2Record(record.id)
        } else if (record.proto.account != null) {
@@ -1,99 +0,0 @@
-package org.thoughtcrime.securesms.storage
-
-import org.signal.core.util.logging.Log
-import org.thoughtcrime.securesms.database.GroupTable
-import org.thoughtcrime.securesms.database.RecipientTable
-import org.thoughtcrime.securesms.database.SignalDatabase
-import org.thoughtcrime.securesms.database.model.RecipientRecord
-import org.thoughtcrime.securesms.groups.BadGroupIdException
-import org.thoughtcrime.securesms.groups.GroupId
-import org.whispersystems.signalservice.api.storage.SignalGroupV1Record
-import org.whispersystems.signalservice.api.storage.SignalStorageRecord
-import org.whispersystems.signalservice.api.storage.StorageId
-import org.whispersystems.signalservice.api.storage.toSignalGroupV1Record
-import java.util.Optional
-
-/**
- * Record processor for [SignalGroupV1Record].
- * Handles merging and updating our local store when processing remote gv1 storage records.
- */
-class GroupV1RecordProcessor(private val groupDatabase: GroupTable, private val recipientTable: RecipientTable) : DefaultStorageRecordProcessor<SignalGroupV1Record>() {
-  companion object {
-    private val TAG = Log.tag(GroupV1RecordProcessor::class.java)
-  }
-
-  constructor() : this(SignalDatabase.groups, SignalDatabase.recipients)
-
-  /**
-   * We want to catch:
-   * - Invalid group ID's
-   * - GV1 ID's that map to GV2 ID's, meaning we've already migrated them.
-   *
-   * Note: This method could be written more succinctly, but the logs are useful :)
-   */
-  override fun isInvalid(remote: SignalGroupV1Record): Boolean {
-    try {
-      val id = GroupId.v1(remote.proto.id.toByteArray())
-      val v2Record = groupDatabase.getGroup(id.deriveV2MigrationGroupId())
-
-      if (v2Record.isPresent) {
-        Log.w(TAG, "We already have an upgraded V2 group for this V1 group -- marking as invalid.")
-        return true
-      } else {
-        return false
-      }
-    } catch (e: BadGroupIdException) {
-      Log.w(TAG, "Bad Group ID -- marking as invalid.")
-      return true
-    }
-  }
-
-  override fun getMatching(remote: SignalGroupV1Record, keyGenerator: StorageKeyGenerator): Optional<SignalGroupV1Record> {
-    val groupId = GroupId.v1orThrow(remote.proto.id.toByteArray())
-
-    val recipientId = recipientTable.getByGroupId(groupId)
-
-    return recipientId
-      .map { recipientTable.getRecordForSync(it)!! }
-      .map { settings: RecipientRecord -> StorageSyncModels.localToRemoteRecord(settings) }
-      .map { record: SignalStorageRecord -> record.proto.groupV1!!.toSignalGroupV1Record(record.id) }
-  }
-
-  override fun merge(remote: SignalGroupV1Record, local: SignalGroupV1Record, keyGenerator: StorageKeyGenerator): SignalGroupV1Record {
-    val merged = SignalGroupV1Record.newBuilder(remote.serializedUnknowns).apply {
-      id = remote.proto.id
-      blocked = remote.proto.blocked
-      whitelisted = remote.proto.whitelisted
-      archived = remote.proto.archived
-      markedUnread = remote.proto.markedUnread
-      mutedUntilTimestamp = remote.proto.mutedUntilTimestamp
-    }.build().toSignalGroupV1Record(StorageId.forGroupV1(keyGenerator.generate()))
-
-    val matchesRemote = doParamsMatch(remote, merged)
-    val matchesLocal = doParamsMatch(local, merged)
-
-    return if (matchesRemote) {
-      remote
-    } else if (matchesLocal) {
-      local
-    } else {
-      merged
-    }
-  }
-
-  override fun insertLocal(record: SignalGroupV1Record) {
-    recipientTable.applyStorageSyncGroupV1Insert(record)
-  }
-
-  override fun updateLocal(update: StorageRecordUpdate<SignalGroupV1Record>) {
-    recipientTable.applyStorageSyncGroupV1Update(update)
-  }
-
-  override fun compare(lhs: SignalGroupV1Record, rhs: SignalGroupV1Record): Int {
-    return if (lhs.proto.id == rhs.proto.id) {
-      0
-    } else {
-      1
-    }
-  }
-}
@@ -38,7 +38,6 @@ import org.whispersystems.signalservice.api.storage.IAPSubscriptionId
 import org.whispersystems.signalservice.api.storage.SignalCallLinkRecord
 import org.whispersystems.signalservice.api.storage.SignalChatFolderRecord
 import org.whispersystems.signalservice.api.storage.SignalContactRecord
-import org.whispersystems.signalservice.api.storage.SignalGroupV1Record
 import org.whispersystems.signalservice.api.storage.SignalGroupV2Record
 import org.whispersystems.signalservice.api.storage.SignalNotificationProfileRecord
 import org.whispersystems.signalservice.api.storage.SignalStickerPackRecord
@@ -48,7 +47,6 @@ import org.whispersystems.signalservice.api.storage.StorageId
 import org.whispersystems.signalservice.api.storage.toSignalCallLinkRecord
 import org.whispersystems.signalservice.api.storage.toSignalChatFolderRecord
 import org.whispersystems.signalservice.api.storage.toSignalContactRecord
-import org.whispersystems.signalservice.api.storage.toSignalGroupV1Record
 import org.whispersystems.signalservice.api.storage.toSignalGroupV2Record
 import org.whispersystems.signalservice.api.storage.toSignalNotificationProfileRecord
 import org.whispersystems.signalservice.api.storage.toSignalStickerPackRecord
@@ -90,7 +88,6 @@ object StorageSyncModels {
  fun localToRemoteRecord(settings: RecipientRecord, rawStorageId: ByteArray): SignalStorageRecord {
    return when (settings.recipientType) {
      RecipientType.INDIVIDUAL -> localToRemoteContact(settings, rawStorageId).toSignalStorageRecord()
-      RecipientType.GV1 -> localToRemoteGroupV1(settings, rawStorageId).toSignalStorageRecord()
      RecipientType.GV2 -> localToRemoteGroupV2(settings, rawStorageId, settings.syncExtras.groupMasterKey!!).toSignalStorageRecord()
      RecipientType.DISTRIBUTION_LIST -> localToRemoteStoryDistributionList(settings, rawStorageId).toSignalStorageRecord()
      RecipientType.CALL_LINK -> localToRemoteCallLink(settings, rawStorageId).toSignalStorageRecord()
@@ -229,23 +226,6 @@ object StorageSyncModels {
    }.build().toSignalContactRecord(StorageId.forContact(rawStorageId))
  }

-  private fun localToRemoteGroupV1(recipient: RecipientRecord, rawStorageId: ByteArray): SignalGroupV1Record {
-    val groupId = recipient.groupId ?: throw AssertionError("Must have a groupId!")
-
-    if (!groupId.isV1) {
-      throw AssertionError("Group is not V1")
-    }
-
-    return SignalGroupV1Record.newBuilder(recipient.syncExtras.storageProto).apply {
-      id = recipient.groupId.requireV1().decodedId.toByteString()
-      blocked = recipient.isBlocked
-      whitelisted = recipient.profileSharing
-      archived = recipient.syncExtras.isArchived
-      markedUnread = recipient.syncExtras.isForcedUnread
-      mutedUntilTimestamp = recipient.muteUntil
-    }.build().toSignalGroupV1Record(StorageId.forGroupV1(rawStorageId))
-  }
-
  private fun localToRemoteGroupV2(recipient: RecipientRecord, rawStorageId: ByteArray?, groupMasterKey: GroupMasterKey): SignalGroupV2Record {
    val groupId = recipient.groupId ?: throw AssertionError("Must have a groupId!")

@@ -0,0 +1,123 @@
+/*
+ * Copyright 2026 Signal Messenger, LLC
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+package org.thoughtcrime.securesms.database.helpers.migration
+
+import android.app.Application
+import androidx.test.core.app.ApplicationProvider
+import assertk.assertThat
+import assertk.assertions.isEqualTo
+import assertk.assertions.isNull
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+import org.signal.core.util.insertInto
+import org.signal.core.util.readToList
+import org.signal.core.util.readToSingleObject
+import org.signal.core.util.requireInt
+import org.signal.core.util.requireNonNullString
+import org.signal.core.util.requireString
+import org.signal.core.util.select
+import org.thoughtcrime.securesms.testutil.SignalDatabaseMigrationRule
+
+@Suppress("ClassName")
+@RunWith(RobolectricTestRunner::class)
+@Config(manifest = Config.NONE, application = Application::class)
+class V324_MoveGroupV1StorageIdsToUnknownIdsTest {
+
+  companion object {
+    private const val RECIPIENT_TYPE_GV1 = 2
+    private const val RECIPIENT_TYPE_GV2 = 3
+    private const val MANIFEST_TYPE_GROUPV1 = 2
+  }
+
+  @get:Rule val signalDatabaseRule = SignalDatabaseMigrationRule(323)
+
+  private val db get() = signalDatabaseRule.database
+
+  @Test
+  fun migrate_movesGroupV1StorageIdIntoUnknownIds() {
+    insertRecipient(groupId = "gv1", type = RECIPIENT_TYPE_GV1, storageId = "storage-id-gv1")
+
+    migrate()
+
+    assertThat(unknownIdKeys()).isEqualTo(listOf("storage-id-gv1"))
+    assertThat(unknownIdTypeOf("storage-id-gv1")).isEqualTo(MANIFEST_TYPE_GROUPV1)
+  }
+
+  @Test
+  fun migrate_clearsGroupV1StorageIdAndProto() {
+    insertRecipient(groupId = "gv1", type = RECIPIENT_TYPE_GV1, storageId = "storage-id-gv1", storageProto = "proto")
+
+    migrate()
+
+    assertThat(storageIdOf("gv1")).isNull()
+    assertThat(storageProtoOf("gv1")).isNull()
+  }
+
+  @Test
+  fun migrate_leavesOtherRecipientTypesAlone() {
+    insertRecipient(groupId = "gv2", type = RECIPIENT_TYPE_GV2, storageId = "storage-id-gv2")
+
+    migrate()
+
+    assertThat(storageIdOf("gv2")).isEqualTo("storage-id-gv2")
+    assertThat(unknownIdKeys()).isEqualTo(emptyList())
+  }
+
+  @Test
+  fun migrate_ignoresGroupV1RecipientsWithoutAStorageId() {
+    insertRecipient(groupId = "gv1", type = RECIPIENT_TYPE_GV1, storageId = null)
+
+    migrate()
+
+    assertThat(unknownIdKeys()).isEqualTo(emptyList())
+  }
+
+  /** The unknown ID table has a UNIQUE constraint on key, so a pre-existing row must not blow up the insert. */
+  @Test
+  fun migrate_toleratesAnIdThatIsAlreadyTracked() {
+    insertRecipient(groupId = "gv1", type = RECIPIENT_TYPE_GV1, storageId = "storage-id-gv1")
+    db.insertInto("storage_key").values("type" to MANIFEST_TYPE_GROUPV1, "key" to "storage-id-gv1").run()
+
+    migrate()
+
+    assertThat(unknownIdKeys()).isEqualTo(listOf("storage-id-gv1"))
+    assertThat(storageIdOf("gv1")).isNull()
+  }
+
+  private fun migrate() {
+    V324_MoveGroupV1StorageIdsToUnknownIds.migrate(ApplicationProvider.getApplicationContext(), db, 323, 324)
+  }
+
+  private fun insertRecipient(groupId: String, type: Int, storageId: String?, storageProto: String? = null) {
+    db.insertInto("recipient")
+      .values(
+        "group_id" to groupId,
+        "type" to type,
+        "storage_service_id" to storageId,
+        "storage_service_proto" to storageProto
+      )
+      .run()
+  }
+
+  private fun unknownIdKeys(): List<String> {
+    return db.select("key").from("storage_key").run().readToList { it.requireNonNullString("key") }
+  }
+
+  private fun unknownIdTypeOf(key: String): Int? {
+    return db.select("type").from("storage_key").where("key = ?", key).run().readToSingleObject { it.requireInt("type") }
+  }
+
+  private fun storageIdOf(groupId: String): String? {
+    return db.select("storage_service_id").from("recipient").where("group_id = ?", groupId).run().readToSingleObject { it.requireString("storage_service_id") }
+  }
+
+  private fun storageProtoOf(groupId: String): String? {
+    return db.select("storage_service_proto").from("recipient").where("group_id = ?", groupId).run().readToSingleObject { it.requireString("storage_service_proto") }
+  }
+}
@@ -11,6 +11,7 @@ import io.mockk.every
 import okio.ByteString.Companion.toByteString
 import org.junit.Assert.assertArrayEquals
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertTrue
 import org.junit.Before
@@ -27,6 +28,7 @@ import org.signal.core.util.logging.Log
 import org.signal.core.util.withinTransaction
 import org.thoughtcrime.securesms.database.SignalDatabase
 import org.thoughtcrime.securesms.database.model.StickerPackId
+import org.thoughtcrime.securesms.groups.GroupId
 import org.thoughtcrime.securesms.jobmanager.Job
 import org.thoughtcrime.securesms.profiles.ProfileName
 import org.thoughtcrime.securesms.recipients.Recipient
@@ -37,6 +39,7 @@ import org.thoughtcrime.securesms.testutil.SystemOutLogger
 import org.whispersystems.signalservice.api.storage.SignalStorageRecord
 import org.whispersystems.signalservice.api.storage.StorageId
 import org.whispersystems.signalservice.internal.storage.protos.ContactRecord
+import org.whispersystems.signalservice.internal.storage.protos.GroupV1Record
 import org.whispersystems.signalservice.internal.storage.protos.StickerPackRecord
 import org.whispersystems.signalservice.internal.storage.protos.StorageRecord
 import java.util.UUID
@@ -222,6 +225,74 @@ class StorageSyncJobTest {
    assertArrayEquals(record.id.raw, pack!!.storageServiceId!!.raw)
  }

+  @Test
+  fun `given a remote GV1 record, when I run, then I keep it in the manifest`() {
+    val record = groupV1Record()
+    remoteStorage.addRemoteRecords(listOf(record))
+
+    val result = runJob(StorageSyncJob.forRemoteChange())
+
+    assertTrue(result.isSuccess)
+    assertEquals(0, remoteStorage.writeCount)
+    assertTrue(remoteStorage.manifest!!.storageIds.contains(record.id))
+    assertTrue(SignalDatabase.unknownStorageIds.allUnknownIds.contains(record.id))
+  }
+
+  @Test
+  fun `given a remote GV1 record, when I run, then I do not apply it locally`() {
+    val groupId = GroupId.v1(Util.getSecretBytes(16))
+    remoteStorage.addRemoteRecords(listOf(groupV1Record(groupId)))
+
+    val result = runJob(StorageSyncJob.forRemoteChange())
+
+    assertTrue(result.isSuccess)
+    assertFalse(SignalDatabase.recipients.getByGroupId(groupId).isPresent)
+  }
+
+  @Test
+  fun `given a GV1 recipient with a storage ID, when I run, then I ignore it entirely`() {
+    val storageId = StorageId.forGroupV1(Util.getSecretBytes(16))
+    val recipientId = SignalDatabase.recipients.getOrInsertFromGroupId(GroupId.v1(Util.getSecretBytes(16)))
+    SignalDatabase.recipients.updateStorageId(recipientId, storageId.raw)
+
+    val result = runJob(StorageSyncJob.forLocalChange())
+
+    assertTrue(result.isSuccess)
+    assertEquals(0, remoteStorage.writeCount)
+    assertFalse(remoteStorage.manifest!!.storageIds.contains(storageId))
+  }
+
+  @Test
+  fun `given a newer remote manifest without my GV1 ID, when I run, then I stop tracking it`() {
+    val record = groupV1Record()
+    remoteStorage.addRemoteRecords(listOf(record))
+    check(runJob(StorageSyncJob.forRemoteChange()).isSuccess)
+    check(SignalDatabase.unknownStorageIds.allUnknownIds.contains(record.id))
+
+    val withoutGroupV1 = remoteStorage.records.filterNot { it.id == record.id }
+    remoteStorage.setRemoteState(withoutGroupV1, version = remoteStorage.manifest!!.version + 1)
+
+    val result = runJob(StorageSyncJob.forRemoteChange())
+
+    assertTrue(result.isSuccess)
+    assertFalse(SignalDatabase.unknownStorageIds.allUnknownIds.contains(record.id))
+  }
+
+  @Test
+  fun `given a local-only unknown ID and a local contact, when I run, then I write only the contact and drop the unknown ID`() {
+    val strandedId = StorageId.forGroupV1(Util.getSecretBytes(16))
+    insertUnknownStorageId(strandedId)
+    SignalDatabase.recipients.rotateStorageId(recipients.createRecipient("Local Contact"))
+
+    val result = runJob(StorageSyncJob.forLocalChange())
+
+    assertTrue(result.isSuccess)
+    assertEquals(1, remoteStorage.writeCount)
+    assertEquals(1, remoteStorage.records.count { it.proto.contact?.givenName == "Local" })
+    assertFalse(remoteStorage.manifest!!.storageIds.contains(strandedId))
+    assertFalse(SignalDatabase.unknownStorageIds.allUnknownIds.contains(strandedId))
+  }
+
  /**
   * Gets us to a steady state: remote holds our account record at version 1, then a sync pushes up everything else
   * the fresh database came with (the default chat folder), leaving both sides at [BASE_MANIFEST_VERSION].
@@ -278,6 +349,19 @@ class StorageSyncJobTest {
    }
  }

+  private fun groupV1Record(groupId: GroupId.V1 = GroupId.v1(Util.getSecretBytes(16)), storageId: StorageId = StorageId.forGroupV1(Util.getSecretBytes(16))): SignalStorageRecord {
+    return SignalStorageRecord(
+      id = storageId,
+      proto = StorageRecord(
+        groupV1 = GroupV1Record(
+          id = groupId.decodedId.toByteString(),
+          blocked = true,
+          whitelisted = true
+        )
+      )
+    )
+  }
+
  private fun runJob(job: StorageSyncJob): Job.Result {
    job.setContext(ApplicationProvider.getApplicationContext())
    return job.run()
@@ -75,9 +75,13 @@ public class StorageId {
    return new StorageId(type, key);
  }

+  /**
+   * GROUPV1 is deliberately excluded. We no longer read or write gv1 records, so treating them as unknown lets us keep
+   * their ids in the manifest instead of deleting them.
+   */
  public static boolean isKnownType(int val) {
    for (ManifestRecord.Identifier.Type type : ManifestRecord.Identifier.Type.values()) {
-      if (type != ManifestRecord.Identifier.Type.UNKNOWN && type.getValue() == val) {
+      if (type != ManifestRecord.Identifier.Type.UNKNOWN && type != ManifestRecord.Identifier.Type.GROUPV1 && type.getValue() == val) {
        return true;
      }
    }