[bisq-network/bisq] (5/6) Broadcast Remove messages seen before Adds (#3664)

[Florian Reimair]

freimair requested changes on this pull request.



> @@ -501,14 +501,14 @@ public boolean remove(ProtectedStorageEntry protectedStorageEntry,
 
         maybeAddToRemoveAddOncePayloads(protectedStoragePayload, hashOfPayload);
 
-        // This means the RemoveData or RemoveMailboxData was seen prior to the AddData. We have already updated
-        // the SequenceNumberMap appropriately so the stale Add will not pass validation, but we don't want to
-        // signal listeners for state changes since no original state existed.
-        if (storedEntry == null)
-            return false;
-
-        // Valid remove entry, do the remove and signal listeners
-        removeFromMapAndDataStore(protectedStorageEntry, hashOfPayload);
+        if (storedEntry != null) {
+            // Valid remove entry, do the remove and signal listeners
+            removeFromMapAndDataStore(protectedStorageEntry, hashOfPayload);
+        } /* else {
+            // This means the RemoveData or RemoveMailboxData was seen prior to the AddData. We have already updated
+            // the SequenceNumberMap appropriately so the stale Add will not pass validation, but we still want to
+            // broadcast the remove to peers so they can update their state appropriately

The return value of the call is changed from `false` to `true` again. If I am not mistaken, we discussed this change (the other way around) quite recently. Do you see any issues that can arise from this change?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/3664#pullrequestreview-322194376
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191125/528c3aaf/attachment.html>