Proper handling of GV1->GV2 migrations in storage service.

app/src/main/java/org/thoughtcrime/securesms/database/GroupDatabase.java

@@ -41,10 +41,12 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.UUID;
 
 @Trace
@@ -677,7 +679,7 @@ public final class GroupDatabase extends Database {
     return RecipientId.toSerializedList(groupMembers);
   }
 
-  public List<GroupId.V2> getAllGroupV2Ids() {
+  public @NonNull List<GroupId.V2> getAllGroupV2Ids() {
     List<GroupId.V2> result = new LinkedList<>();
 
     try (Cursor cursor = databaseHelper.getReadableDatabase().query(TABLE_NAME, new String[]{ GROUP_ID }, null, null, null, null, null)) {
@@ -692,6 +694,28 @@ public final class GroupDatabase extends Database {
     return result;
   }
 
+  /**
+   * Key: The 'expected' V2 ID (i.e. what a V1 ID would map to when migrated)
+   * Value: The matching V1 ID
+   */
+  public @NonNull Map<GroupId.V2, GroupId.V1> getAllExpectedV2Ids() {
+    Map<GroupId.V2, GroupId.V1> result = new HashMap<>();
+
+    String[] projection = new String[]{ GROUP_ID, EXPECTED_V2_ID };
+    String   query      = EXPECTED_V2_ID + " NOT NULL";
+
+    try (Cursor cursor = databaseHelper.getReadableDatabase().query(TABLE_NAME, projection, query, null, null, null, null)) {
+      while (cursor.moveToNext()) {
+        GroupId.V1 groupId    = GroupId.parseOrThrow(cursor.getString(cursor.getColumnIndexOrThrow(GROUP_ID))).requireV1();
+        GroupId.V2 expectedId = GroupId.parseOrThrow(cursor.getString(cursor.getColumnIndexOrThrow(EXPECTED_V2_ID))).requireV2();
+
+        result.put(expectedId, groupId);
+      }
+    }
+
+    return result;
+  }
+
   public static class Reader implements Closeable {
 
     private final Cursor cursor;

app/src/main/java/org/thoughtcrime/securesms/jobs/GroupV1MigrationJob.java

@@ -11,6 +11,7 @@ import com.annimon.stream.Stream;
 import org.signal.storageservice.protos.groups.local.DecryptedGroup;
 import org.signal.zkgroup.groups.GroupMasterKey;
 import org.thoughtcrime.securesms.database.DatabaseFactory;
+import org.thoughtcrime.securesms.database.GroupDatabase;
 import org.thoughtcrime.securesms.database.model.ThreadRecord;
 import org.thoughtcrime.securesms.dependencies.ApplicationDependencies;
 import org.thoughtcrime.securesms.groups.GroupAlreadyExistsException;
@@ -61,6 +62,8 @@ public class GroupV1MigrationJob extends BaseJob {
   private static final int  ROUTINE_LIMIT     = 50;
   private static final long REFRESH_INTERVAL = TimeUnit.HOURS.toMillis(3);
 
+  private static final Object MIGRATION_LOCK = new Object();
+
   private final RecipientId recipientId;
   private final boolean     forced;
 
@@ -161,6 +164,7 @@ public class GroupV1MigrationJob extends BaseJob {
   protected void onRun() throws IOException, RetryLaterException {
     Recipient     groupRecipient = Recipient.resolved(recipientId);
     Long          threadId       = DatabaseFactory.getThreadDatabase(context).getThreadIdFor(recipientId);
+    GroupDatabase groupDatabase  = DatabaseFactory.getGroupDatabase(context);
 
     if (threadId == null) {
       warn(TAG, "No thread found!");
@@ -182,6 +186,11 @@ public class GroupV1MigrationJob extends BaseJob {
     GroupMasterKey gv2MasterKey = gv1Id.deriveV2MigrationMasterKey();
     boolean        newlyCreated = false;
 
+    if (groupDatabase.groupExists(gv2Id)) {
+      warn(TAG, "We already have a V2 group for this V1 group! Must have been added before we were migration-capable.");
+      return;
+    }
+
     switch (GroupManager.v2GroupStatus(context, gv2MasterKey)) {
       case DOES_NOT_EXIST:
         log(TAG, "Group does not exist on the service.");
@@ -259,13 +268,16 @@ public class GroupV1MigrationJob extends BaseJob {
   }
 
   public static void performLocalMigration(@NonNull Context context, @NonNull GroupId.V1 gv1Id) throws IOException {
+    synchronized (MIGRATION_LOCK) {
       Recipient recipient = Recipient.externalGroupExact(context, gv1Id);
       long      threadId  = DatabaseFactory.getThreadDatabase(context).getThreadIdFor(recipient);
 
       performLocalMigration(context, gv1Id, threadId, recipient);
     }
+  }
 
   private static @Nullable DecryptedGroup performLocalMigration(@NonNull Context context, @NonNull GroupId.V1 gv1Id, long threadId, @NonNull Recipient groupRecipient) throws IOException {
+    synchronized (MIGRATION_LOCK) {
       DecryptedGroup decryptedGroup;
       try {
         decryptedGroup = GroupManager.addedGroupVersion(context, gv1Id.deriveV2MigrationMasterKey());
@@ -296,6 +308,7 @@ public class GroupV1MigrationJob extends BaseJob {
 
       return decryptedGroup;
     }
+  }
 
   private static void handleLeftBehind(@NonNull Context context, @NonNull GroupId.V1 gv1Id, @NonNull Recipient groupRecipient, long threadId) {
     DatabaseFactory.getGroupDatabase(context).setActive(gv1Id, false);

app/src/main/java/org/thoughtcrime/securesms/jobs/StorageSyncJob.java

@@ -6,11 +6,13 @@ import androidx.annotation.NonNull;
 
 import com.annimon.stream.Stream;
 
+import org.signal.zkgroup.groups.GroupMasterKey;
 import org.thoughtcrime.securesms.database.DatabaseFactory;
 import org.thoughtcrime.securesms.database.RecipientDatabase;
 import org.thoughtcrime.securesms.database.RecipientDatabase.RecipientSettings;
 import org.thoughtcrime.securesms.database.StorageKeyDatabase;
 import org.thoughtcrime.securesms.dependencies.ApplicationDependencies;
+import org.thoughtcrime.securesms.groups.GroupId;
 import org.thoughtcrime.securesms.jobmanager.Data;
 import org.thoughtcrime.securesms.jobmanager.Job;
 import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint;
@@ -18,6 +20,8 @@ import org.thoughtcrime.securesms.keyvalue.SignalStore;
 import org.thoughtcrime.securesms.logging.Log;
 import org.thoughtcrime.securesms.recipients.Recipient;
 import org.thoughtcrime.securesms.recipients.RecipientId;
+import org.thoughtcrime.securesms.storage.GroupV2ExistenceChecker;
+import org.thoughtcrime.securesms.storage.StaticGroupV2ExistenceChecker;
 import org.thoughtcrime.securesms.storage.StorageSyncHelper;
 import org.thoughtcrime.securesms.storage.StorageSyncHelper.KeyDifferenceResult;
 import org.thoughtcrime.securesms.storage.StorageSyncHelper.LocalWriteResult;
@@ -28,6 +32,7 @@ import org.thoughtcrime.securesms.storage.StorageSyncValidations;
 import org.thoughtcrime.securesms.tracing.Trace;
 import org.thoughtcrime.securesms.transport.RetryLaterException;
 import org.thoughtcrime.securesms.util.FeatureFlags;
+import org.thoughtcrime.securesms.util.GroupUtil;
 import org.thoughtcrime.securesms.util.TextSecurePreferences;
 import org.thoughtcrime.securesms.util.Util;
 import org.whispersystems.libsignal.InvalidKeyException;
@@ -35,6 +40,7 @@ import org.whispersystems.libsignal.util.guava.Optional;
 import org.whispersystems.signalservice.api.SignalServiceAccountManager;
 import org.whispersystems.signalservice.api.push.exceptions.PushNetworkException;
 import org.whispersystems.signalservice.api.storage.SignalAccountRecord;
+import org.whispersystems.signalservice.api.storage.SignalGroupV2Record;
 import org.whispersystems.signalservice.api.storage.SignalStorageManifest;
 import org.whispersystems.signalservice.api.storage.SignalStorageRecord;
 import org.whispersystems.signalservice.api.storage.StorageId;
@@ -44,9 +50,12 @@ import org.whispersystems.signalservice.internal.storage.protos.ManifestRecord;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
@@ -156,7 +165,8 @@ public class StorageSyncJob extends BaseJob {
 
         List<SignalStorageRecord> localOnly            = buildLocalStorageRecords(context, keyDifference.getLocalOnlyKeys());
         List<SignalStorageRecord> remoteOnly           = accountManager.readStorageRecords(storageServiceKey, keyDifference.getRemoteOnlyKeys());
-        MergeResult               mergeResult          = StorageSyncHelper.resolveConflict(remoteOnly, localOnly);
+        GroupV2ExistenceChecker   gv2ExistenceChecker  = new StaticGroupV2ExistenceChecker(DatabaseFactory.getGroupDatabase(context).getAllGroupV2Ids());
+        MergeResult               mergeResult          = StorageSyncHelper.resolveConflict(remoteOnly, localOnly, gv2ExistenceChecker);
         WriteOperationResult      writeOperationResult = StorageSyncHelper.createWriteOperation(remoteManifest.get().getVersion(), allLocalStorageKeys, mergeResult);
 
         if (remoteOnly.size() != keyDifference.getRemoteOnlyKeys().size()) {
@@ -191,6 +201,7 @@ public class StorageSyncJob extends BaseJob {
           Log.i(TAG, "[Remote Newer] After resolving the conflict, all changes are local. No remote writes needed.");
         }
 
+        migrateToGv2IfNecessary(context, mergeResult.getLocalGroupV2Inserts());
         recipientDatabase.applyStorageSyncUpdates(mergeResult.getLocalContactInserts(), mergeResult.getLocalContactUpdates(), mergeResult.getLocalGroupV1Inserts(), mergeResult.getLocalGroupV1Updates(), mergeResult.getLocalGroupV2Inserts(), mergeResult.getLocalGroupV2Updates());
         storageKeyDatabase.applyStorageSyncUpdates(mergeResult.getLocalUnknownInserts(), mergeResult.getLocalUnknownDeletes());
         StorageSyncHelper.applyAccountStorageSyncUpdates(context, mergeResult.getLocalAccountUpdate());
@@ -267,6 +278,27 @@ public class StorageSyncJob extends BaseJob {
     return needsMultiDeviceSync;
   }
 
+  /**
+   * Migrates any of the provided V2 IDs that map a local V1 ID. If a match is found, we remove the
+   * record from the collection of V2 IDs.
+   */
+  private static void migrateToGv2IfNecessary(@NonNull Context context, @NonNull Collection<SignalGroupV2Record> inserts)
+      throws IOException
+  {
+    Map<GroupId.V2, GroupId.V1>   idMap          = DatabaseFactory.getGroupDatabase(context).getAllExpectedV2Ids();
+    Iterator<SignalGroupV2Record> recordIterator = inserts.iterator();
+
+    while (recordIterator.hasNext()) {
+      GroupId.V2 id = GroupId.v2(GroupUtil.requireMasterKey(recordIterator.next().getMasterKeyBytes()));
+
+      if (idMap.containsKey(id)) {
+        Log.i(TAG, "Discovered a new GV2 ID that is actually a migrated V1 group! Migrating now.");
+        GroupV1MigrationJob.performLocalMigration(context, idMap.get(id));
+        recordIterator.remove();
+      }
+    }
+  }
+
   private static @NonNull List<StorageId> getAllLocalStorageIds(@NonNull Context context, @NonNull Recipient self) {
     return Util.concatenatedList(DatabaseFactory.getRecipientDatabase(context).getContactStorageSyncIds(),
                                  Collections.singletonList(StorageId.forAccount(self.getStorageServiceId())),

app/src/main/java/org/thoughtcrime/securesms/storage/GroupV1ConflictMerger.java

@@ -17,9 +17,12 @@ import java.util.Map;
 final class GroupV1ConflictMerger implements StorageSyncHelper.ConflictMerger<SignalGroupV1Record> {
 
   private final Map<GroupId, SignalGroupV1Record> localByGroupId;
+  private final GroupV2ExistenceChecker groupExistenceChecker;
 
-  GroupV1ConflictMerger(@NonNull Collection<SignalGroupV1Record> localOnly) {
+  GroupV1ConflictMerger(@NonNull Collection<SignalGroupV1Record> localOnly, @NonNull GroupV2ExistenceChecker groupExistenceChecker) {
     localByGroupId = Stream.of(localOnly).collect(Collectors.toMap(g -> GroupId.v1orThrow(g.getGroupId()), g -> g));
+
+    this.groupExistenceChecker = groupExistenceChecker;
   }
 
   @Override
@@ -30,8 +33,14 @@ final class GroupV1ConflictMerger implements StorageSyncHelper.ConflictMerger<Si
   @Override
   public @NonNull Collection<SignalGroupV1Record> getInvalidEntries(@NonNull Collection<SignalGroupV1Record> remoteRecords) {
     return Stream.of(remoteRecords)
-                 .filterNot(GroupV1ConflictMerger::isValidGroupId)
+                 .filter(record -> {
-                 .toList();
+                   try {
+                     GroupId.V1 id = GroupId.v1Exact(record.getGroupId());
+                     return groupExistenceChecker.exists(id.deriveV2MigrationGroupId());
+                   } catch (BadGroupIdException e) {
+                     return true;
+                   }
+                 }).toList();
   }
 
   @Override
@@ -58,13 +67,4 @@ final class GroupV1ConflictMerger implements StorageSyncHelper.ConflictMerger<Si
                                     .build();
     }
   }
-
-  private static boolean isValidGroupId(@NonNull SignalGroupV1Record record) {
-    try {
-      GroupId.v1Exact(record.getGroupId());
-      return true;
-    } catch (BadGroupIdException e) {
-      return false;
-    }
-  }
 }

app/src/main/java/org/thoughtcrime/securesms/storage/GroupV2ExistenceChecker.java

@@ -0,0 +1,13 @@
+package org.thoughtcrime.securesms.storage;
+
+import androidx.annotation.NonNull;
+
+import org.thoughtcrime.securesms.groups.GroupId;
+
+/**
+ * Allows a caller to determine if a group exists in the local data store already. Needed primarily
+ * to check if a local GV2 group already exists for a remote GV1 group.
+ */
+public interface GroupV2ExistenceChecker {
+  boolean exists(@NonNull GroupId.V2 groupId);
+}

app/src/main/java/org/thoughtcrime/securesms/storage/StaticGroupV2ExistenceChecker.java

@@ -0,0 +1,26 @@
+package org.thoughtcrime.securesms.storage;
+
+import androidx.annotation.NonNull;
+
+import org.thoughtcrime.securesms.groups.GroupId;
+
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * Implementation that is backed by a static set of GV2 IDs.
+ */
+public final class StaticGroupV2ExistenceChecker implements GroupV2ExistenceChecker {
+
+  private final Set<GroupId.V2> ids;
+
+  public StaticGroupV2ExistenceChecker(@NonNull Collection<GroupId.V2> ids) {
+    this.ids = new HashSet<>(ids);
+  }
+
+  @Override
+  public boolean exists(@NonNull GroupId.V2 groupId) {
+    return ids.contains(groupId);
+  }
+}

app/src/main/java/org/thoughtcrime/securesms/storage/StorageSyncHelper.java

@@ -12,7 +12,6 @@ import com.annimon.stream.Stream;
 import org.thoughtcrime.securesms.database.DatabaseFactory;
 import org.thoughtcrime.securesms.database.RecipientDatabase;
 import org.thoughtcrime.securesms.database.RecipientDatabase.RecipientSettings;
-import org.thoughtcrime.securesms.database.ThreadDatabase;
 import org.thoughtcrime.securesms.dependencies.ApplicationDependencies;
 import org.thoughtcrime.securesms.jobs.RetrieveProfileAvatarJob;
 import org.thoughtcrime.securesms.jobs.StorageSyncJob;
@@ -36,7 +35,6 @@ import org.whispersystems.signalservice.api.storage.SignalStorageManifest;
 import org.whispersystems.signalservice.api.storage.SignalStorageRecord;
 import org.whispersystems.signalservice.api.storage.StorageId;
 import org.whispersystems.signalservice.api.util.OptionalUtil;
-import org.whispersystems.signalservice.internal.storage.protos.AccountRecord;
 import org.whispersystems.signalservice.internal.storage.protos.ManifestRecord;
 
 import java.nio.ByteBuffer;
@@ -45,7 +43,6 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -256,7 +253,8 @@ public final class StorageSyncHelper {
    * @return A set of actions that should be applied to resolve the conflict.
    */
   public static @NonNull MergeResult resolveConflict(@NonNull Collection<SignalStorageRecord> remoteOnlyRecords,
-                                                     @NonNull Collection<SignalStorageRecord> localOnlyRecords)
+                                                     @NonNull Collection<SignalStorageRecord> localOnlyRecords,
+                                                     @NonNull GroupV2ExistenceChecker groupExistenceChecker)
   {
     List<SignalContactRecord> remoteOnlyContacts = Stream.of(remoteOnlyRecords).filter(r -> r.getContact().isPresent()).map(r -> r.getContact().get()).toList();
     List<SignalContactRecord> localOnlyContacts  = Stream.of(localOnlyRecords).filter(r -> r.getContact().isPresent()).map(r -> r.getContact().get()).toList();
@@ -280,7 +278,7 @@ public final class StorageSyncHelper {
     }
 
     RecordMergeResult<SignalContactRecord> contactMergeResult = resolveRecordConflict(remoteOnlyContacts, localOnlyContacts, new ContactConflictMerger(localOnlyContacts, Recipient.self()));
-    RecordMergeResult<SignalGroupV1Record> groupV1MergeResult = resolveRecordConflict(remoteOnlyGroupV1, localOnlyGroupV1, new GroupV1ConflictMerger(localOnlyGroupV1));
+    RecordMergeResult<SignalGroupV1Record> groupV1MergeResult = resolveRecordConflict(remoteOnlyGroupV1, localOnlyGroupV1, new GroupV1ConflictMerger(localOnlyGroupV1, groupExistenceChecker));
     RecordMergeResult<SignalGroupV2Record> groupV2MergeResult = resolveRecordConflict(remoteOnlyGroupV2, localOnlyGroupV2, new GroupV2ConflictMerger(localOnlyGroupV2));
     RecordMergeResult<SignalAccountRecord> accountMergeResult = resolveRecordConflict(remoteOnlyAccount, localOnlyAccount, new AccountConflictMerger(localOnlyAccount.isEmpty() ? Optional.absent() : Optional.of(localOnlyAccount.get(0))));
 

app/src/test/java/org/thoughtcrime/securesms/storage/GroupV1ConflictMergerTest.java

@@ -1,6 +1,7 @@
 package org.thoughtcrime.securesms.storage;
 
 import org.junit.Test;
+import org.thoughtcrime.securesms.groups.GroupId;
 import org.thoughtcrime.securesms.storage.StorageSyncHelper.KeyGenerator;
 import org.whispersystems.signalservice.api.storage.SignalGroupV1Record;
 
@@ -39,7 +40,7 @@ public final class GroupV1ConflictMergerTest {
                                                         .setForcedUnread(true)
                                                         .build();
 
-    SignalGroupV1Record merged = new GroupV1ConflictMerger(Collections.singletonList(local)).merge(remote, local, KEY_GENERATOR);
+    SignalGroupV1Record merged = new GroupV1ConflictMerger(Collections.singletonList(local), id -> false).merge(remote, local, KEY_GENERATOR);
 
     assertArrayEquals(remote.getId().getRaw(), merged.getId().getRaw());
     assertArrayEquals(byteArray(100), merged.getGroupId());
@@ -62,7 +63,7 @@ public final class GroupV1ConflictMergerTest {
                                                         .setArchived(false)
                                                         .build();
 
-    SignalGroupV1Record merged = new GroupV1ConflictMerger(Collections.singletonList(local)).merge(remote, local, mock(KeyGenerator.class));
+    SignalGroupV1Record merged = new GroupV1ConflictMerger(Collections.singletonList(local), id -> false).merge(remote, local, mock(KeyGenerator.class));
 
     assertEquals(remote, merged);
   }
@@ -81,7 +82,29 @@ public final class GroupV1ConflictMergerTest {
                                                             .setArchived(true)
                                                             .build();
 
-    Collection<SignalGroupV1Record> invalid = new GroupV1ConflictMerger(Collections.emptyList()).getInvalidEntries(Arrays.asList(badRemote, goodRemote));
+    Collection<SignalGroupV1Record> invalid = new GroupV1ConflictMerger(Collections.emptyList(), id -> false).getInvalidEntries(Arrays.asList(badRemote, goodRemote));
+
+    assertEquals(Collections.singletonList(badRemote), invalid);
+  }
+
+  @Test
+  public void merge_excludeMigratedGroupId() {
+    GroupId.V1 v1Id = GroupId.v1orThrow(groupKey(1));
+    GroupId.V2 v2Id = v1Id.deriveV2MigrationGroupId();
+
+    SignalGroupV1Record badRemote  = new SignalGroupV1Record.Builder(byteArray(1), v1Id.getDecodedId())
+                                                            .setBlocked(false)
+                                                            .setProfileSharingEnabled(true)
+                                                            .setArchived(true)
+                                                            .build();
+
+    SignalGroupV1Record goodRemote = new SignalGroupV1Record.Builder(byteArray(1), groupKey(99))
+                                                            .setBlocked(false)
+                                                            .setProfileSharingEnabled(true)
+                                                            .setArchived(true)
+                                                            .build();
+
+    Collection<SignalGroupV1Record> invalid = new GroupV1ConflictMerger(Collections.emptyList(), id -> id.equals(v2Id)).getInvalidEntries(Arrays.asList(badRemote, goodRemote));
 
     assertEquals(Collections.singletonList(badRemote), invalid);
   }

app/src/test/java/org/thoughtcrime/securesms/storage/StorageSyncHelperTest.java

@@ -6,8 +6,7 @@ import com.annimon.stream.Stream;
 
 import org.junit.Before;
 import org.junit.Test;
-import org.signal.zkgroup.InvalidInputException;
+import org.powermock.core.classloader.annotations.PowerMockIgnore;
-import org.signal.zkgroup.groups.GroupMasterKey;
 import org.junit.runner.RunWith;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
@@ -51,6 +50,7 @@ import static org.thoughtcrime.securesms.testutil.TestHelpers.setOf;
 
 @RunWith(PowerMockRunner.class)
 @PrepareForTest({ Recipient.class, FeatureFlags.class})
+@PowerMockIgnore("javax.crypto.*")
 public final class StorageSyncHelperTest {
 
   private static final UUID UUID_A    = UuidUtil.parseOrThrow("ebef429e-695e-4f51-bcc4-526a60ac68c7");
@@ -145,7 +145,7 @@ public final class StorageSyncHelperTest {
     SignalContactRecord remote1 = contact(1, UUID_A, E164_A, "a");
     SignalContactRecord local1  = contact(2, UUID_B, E164_B, "b");
 
-    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1));
+    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1), r -> false);
 
     assertEquals(setOf(remote1), result.getLocalContactInserts());
     assertTrue(result.getLocalContactUpdates().isEmpty());
@@ -159,7 +159,7 @@ public final class StorageSyncHelperTest {
     SignalContactRecord remote1 = contact(1, UUID_SELF, E164_SELF, "self");
     SignalContactRecord local1  = contact(2, UUID_A, E164_A, "a");
 
-    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1));
+    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1), r -> false);
 
     assertTrue(result.getLocalContactInserts().isEmpty());
     assertTrue(result.getLocalContactUpdates().isEmpty());
@@ -173,7 +173,7 @@ public final class StorageSyncHelperTest {
     SignalGroupV1Record remote1 = badGroupV1(1, 1, true, false);
     SignalGroupV1Record local1  = groupV1(2, 1, true, true);
 
-    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1));
+    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1), r -> false);
 
     assertTrue(result.getLocalContactInserts().isEmpty());
     assertTrue(result.getLocalContactUpdates().isEmpty());
@@ -187,7 +187,7 @@ public final class StorageSyncHelperTest {
     SignalGroupV2Record remote1 = badGroupV2(1, 2, true, false);
     SignalGroupV2Record local1  = groupV2(2, 2, true, false);
 
-    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1));
+    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1), r -> false);
 
     assertTrue(result.getLocalContactInserts().isEmpty());
     assertTrue(result.getLocalContactUpdates().isEmpty());
@@ -201,7 +201,7 @@ public final class StorageSyncHelperTest {
     SignalContactRecord remote1 = contact(1, UUID_A, E164_A, "a");
     SignalContactRecord local1  = contact(2, UUID_A, E164_A, "a");
 
-    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1));
+    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1), r -> false);
 
     SignalContactRecord expectedMerge = contact(1, UUID_A, E164_A, "a");
 
@@ -217,7 +217,7 @@ public final class StorageSyncHelperTest {
     SignalGroupV1Record remote1 = groupV1(1, 1, true, false);
     SignalGroupV1Record local1  = groupV1(2, 1, true, false);
 
-    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1));
+    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1), r -> false);
 
     SignalGroupV1Record expectedMerge = groupV1(1, 1, true, false);
 
@@ -233,7 +233,7 @@ public final class StorageSyncHelperTest {
     SignalGroupV2Record remote1 = groupV2(1, 2, true, false);
     SignalGroupV2Record local1  = groupV2(2, 2, true, false);
 
-    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1));
+    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1), r -> false);
 
     SignalGroupV2Record expectedMerge = groupV2(1, 2, true, false);
 
@@ -249,7 +249,7 @@ public final class StorageSyncHelperTest {
     SignalContactRecord remote1 = contact(1, UUID_A, E164_A, null);
     SignalContactRecord local1  = contact(2, UUID_A, E164_A, "a");
 
-    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1));
+    MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1), r -> false);
 
     SignalContactRecord expectedMerge = contact(2, UUID_A, E164_A, "a");
 
@@ -268,7 +268,7 @@ public final class StorageSyncHelperTest {
     SignalStorageRecord local1  = unknown(1);
     SignalStorageRecord local2  = unknown(2);
 
-    MergeResult result = StorageSyncHelper.resolveConflict(setOf(remote1, remote2, account), setOf(local1, local2, account));
+    MergeResult result = StorageSyncHelper.resolveConflict(setOf(remote1, remote2, account), setOf(local1, local2, account), r -> false);
 
     assertTrue(result.getLocalContactInserts().isEmpty());
     assertTrue(result.getLocalContactUpdates().isEmpty());
@@ -305,7 +305,7 @@ public final class StorageSyncHelperTest {
     Set<SignalStorageRecord> remoteOnly = recordSetOf(remote1, remote2, remote3, remote4, remote5, remote6, unknownRemote);
     Set<SignalStorageRecord> localOnly  = recordSetOf(local1, local2, local3, local4, local5, local6, unknownLocal);
 
-    MergeResult result = StorageSyncHelper.resolveConflict(remoteOnly, localOnly);
+    MergeResult result = StorageSyncHelper.resolveConflict(remoteOnly, localOnly, r -> false);
 
     SignalContactRecord merge1 = contact(2, UUID_A, E164_A, "a");
     SignalContactRecord merge2 = contact(111, UUID_B, E164_B, "b");