[BUGFIX] Fix duplicate sequence number use case (startup)

Fix a bug introduced in d484617 that
did not properly handle a valid use case for duplicate sequence numbers.

For in-memory-only ProtectedStoragePayloads, the client nodes need a way
to reconstruct the Payloads after startup from peer and seed nodes. This
involves sending a ProtectedStorageEntry with a sequence number that
is equal to the last one the client had already seen.

This patch adds tests to confirm the bug and fix as well as the changes
necessary to allow adding of Payloads that were previously seen, but
removed during a restart.

p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java

@@ -390,16 +390,26 @@ public boolean addProtectedStorageEntry(ProtectedStorageEntry protectedStorageEn
             return false;
         }
 
-        // If we have seen a more recent operation for this payload, we ignore the current one
-        if(!hasSequenceNrIncreased(protectedStorageEntry.getSequenceNumber(), hashOfPayload))
+        ProtectedStorageEntry storedEntry = map.get(hashOfPayload);
+
+        // If we have seen a more recent operation for this payload and we have a payload locally, ignore it
+        if (storedEntry != null &&
+                !hasSequenceNrIncreased(protectedStorageEntry.getSequenceNumber(), hashOfPayload)) {
             return false;
+        }
+
+        // We want to allow add operations for equal sequence numbers if we don't have the payload locally. This is
+        // the case for non-persistent Payloads that need to be reconstructed from peer and seed nodes each startup.
+        MapValue sequenceNumberMapValue = sequenceNumberMap.get(hashOfPayload);
+        if (sequenceNumberMapValue != null &&
+                protectedStorageEntry.getSequenceNumber() < sequenceNumberMapValue.sequenceNr) {
+            return false;
+        }
 
         // Verify the ProtectedStorageEntry is well formed and valid for the add operation
         if (!protectedStorageEntry.isValidForAddOperation())
             return false;
 
-        ProtectedStorageEntry storedEntry = map.get(hashOfPayload);
-
         // If we have already seen an Entry with the same hash, verify the metadata is equal
         if (storedEntry != null && !protectedStorageEntry.matchesRelevantPubKey(storedEntry))
             return false;

p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageProtectedStorageEntryTest.java

@@ -576,6 +576,34 @@ public void refreshTTL_refreshEntryOwnerOriginalOwnerMismatch() throws CryptoExc
             KeyPair notOwner = TestUtils.generateKeyPair();
             doRefreshTTLAndVerify(buildRefreshOfferMessage(entry, notOwner, 2), false, false);
         }
+
+        // TESTCASE: After restart, identical sequence numbers are accepted ONCE. We need a way to reconstruct
+        // in-memory ProtectedStorageEntrys from seed and peer nodes around startup time.
+        @Test
+        public void addProtectedStorageEntry_afterRestartCanAddDuplicateSeqNr() {
+            ProtectedStorageEntry toAdd1 = this.getProtectedStorageEntryForAdd(1);
+            doProtectedStorageAddAndVerify(toAdd1, true, true);
+
+            this.testState.simulateRestart();
+
+            // Can add equal seqNr only once
+            doProtectedStorageAddAndVerify(toAdd1, true, true);
+
+            // Can't add equal seqNr twice
+            doProtectedStorageAddAndVerify(toAdd1, false, false);
+        }
+
+        // TESTCASE: After restart, old sequence numbers are not accepted
+        @Test
+        public void addProtectedStorageEntry_afterRestartCanNotAddLowerSeqNr() {
+            ProtectedStorageEntry toAdd1 = this.getProtectedStorageEntryForAdd(1);
+            ProtectedStorageEntry toAdd2 = this.getProtectedStorageEntryForAdd(2);
+            doProtectedStorageAddAndVerify(toAdd2, true, true);
+
+            this.testState.simulateRestart();
+
+            doProtectedStorageAddAndVerify(toAdd1, false, false);
+        }
     }
 
     /**
@@ -608,6 +636,18 @@ public void addProtectedStorageEntry_afterReadFromResourcesWithDuplicate_3629Reg
 
             Assert.assertEquals(beforeRestart, this.testState.mockedStorage.getMap());
         }
+
+        // TESTCASE: After restart, identical sequence numbers are not accepted for persistent payloads
+        @Test
+        public void addProtectedStorageEntry_afterRestartCanNotAddDuplicateSeqNr() {
+            ProtectedStorageEntry toAdd1 = this.getProtectedStorageEntryForAdd(1);
+            doProtectedStorageAddAndVerify(toAdd1, true, true);
+
+            this.testState.simulateRestart();
+
+            // Can add equal seqNr only once
+            doProtectedStorageAddAndVerify(toAdd1, false, false);
+        }
     }
 
     /**

p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageRemoveExpiredTest.java

@@ -188,15 +188,5 @@ public void removeExpiredEntries_PurgeSeqNrMap() throws CryptoException, NoSuchA
         SavedTestState beforeState = this.testState.saveTestState(purgedProtectedStorageEntry);
         this.testState.mockedStorage.removeExpiredEntries();
         this.testState.verifyProtectedStorageRemove(beforeState, expectedRemoves, true, true, false, false, false);
-
-        // Which means that an addition of a purged entry should succeed.
-        beforeState = this.testState.saveTestState(purgedProtectedStorageEntry);
-        Assert.assertTrue(this.testState.mockedStorage.addProtectedStorageEntry(purgedProtectedStorageEntry, TestState.getTestNodeAddress(), null, false));
-        this.testState.verifyProtectedStorageAdd(beforeState, purgedProtectedStorageEntry, true, false);
-
-        // The last entry (5 days old) should still exist in the SequenceNumberMap which means trying to add it again should fail.
-        beforeState = this.testState.saveTestState(keepProtectedStorageEntry);
-        Assert.assertFalse(this.testState.mockedStorage.addProtectedStorageEntry(keepProtectedStorageEntry, TestState.getTestNodeAddress(), null, false));
-        this.testState.verifyProtectedStorageAdd(beforeState, keepProtectedStorageEntry, false, false);
     }
 }