Fix small p2p network issues

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

@@ -26,6 +26,7 @@
 import bisq.network.p2p.peers.BroadcastHandler;
 import bisq.network.p2p.peers.Broadcaster;
 import bisq.network.p2p.storage.messages.AddDataMessage;
+import bisq.network.p2p.storage.messages.AddOncePayload;
 import bisq.network.p2p.storage.messages.AddPersistableNetworkPayloadMessage;
 import bisq.network.p2p.storage.messages.BroadcastMessage;
 import bisq.network.p2p.storage.messages.RefreshOfferMessage;
@@ -112,6 +113,7 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
 
     @Getter
     private final Map<ByteArray, ProtectedStorageEntry> map = new ConcurrentHashMap<>();
+    private final Set<ByteArray> removedAddOncePayloads = new HashSet<>();
     private final Set<HashMapChangedListener> hashMapChangedListeners = new CopyOnWriteArraySet<>();
     private Timer removeExpiredEntriesTimer;
 
@@ -366,18 +368,30 @@ public boolean addProtectedStorageEntry(ProtectedStorageEntry protectedStorageEn
         return addProtectedStorageEntry(protectedStorageEntry, sender, listener, isDataOwner, true);
     }
 
-    public boolean addProtectedStorageEntry(ProtectedStorageEntry protectedStorageEntry, @Nullable NodeAddress sender,
-                                            @Nullable BroadcastHandler.Listener listener, boolean isDataOwner, boolean allowBroadcast) {
-        final ProtectedStoragePayload protectedStoragePayload = protectedStorageEntry.getProtectedStoragePayload();
+    public boolean addProtectedStorageEntry(ProtectedStorageEntry protectedStorageEntry,
+                                            @Nullable NodeAddress sender,
+                                            @Nullable BroadcastHandler.Listener listener,
+                                            boolean isDataOwner,
+                                            boolean allowBroadcast) {
+        ProtectedStoragePayload protectedStoragePayload = protectedStorageEntry.getProtectedStoragePayload();
         ByteArray hashOfPayload = get32ByteHashAsByteArray(protectedStoragePayload);
+
+        if (protectedStoragePayload instanceof AddOncePayload &&
+                removedAddOncePayloads.contains(hashOfPayload)) {
+            log.warn("We have already removed that AddOncePayload by a previous removeDataMessage. " +
+                    "We ignore that message. ProtectedStoragePayload: {}", protectedStoragePayload.toString());
+            return false;
+        }
+
         boolean sequenceNrValid = isSequenceNrValid(protectedStorageEntry.getSequenceNumber(), hashOfPayload);
         boolean result = checkPublicKeys(protectedStorageEntry, true)
                 && checkSignature(protectedStorageEntry)
                 && sequenceNrValid;
 
         boolean containsKey = map.containsKey(hashOfPayload);
-        if (containsKey)
+        if (containsKey) {
             result = result && checkIfStoredDataPubKeyMatchesNewDataPubKey(protectedStorageEntry.getOwnerPubKey(), hashOfPayload);
+        }
 
         // printData("before add");
         if (result) {
@@ -422,7 +436,9 @@ public void broadcastProtectedStorageEntry(ProtectedStorageEntry protectedStorag
         broadcast(new AddDataMessage(protectedStorageEntry), sender, broadcastListener, isDataOwner);
     }
 
-    public boolean refreshTTL(RefreshOfferMessage refreshTTLMessage, @Nullable NodeAddress sender, boolean isDataOwner) {
+    public boolean refreshTTL(RefreshOfferMessage refreshTTLMessage,
+                              @Nullable NodeAddress sender,
+                              boolean isDataOwner) {
         byte[] hashOfDataAndSeqNr = refreshTTLMessage.getHashOfDataAndSeqNr();
         byte[] signature = refreshTTLMessage.getSignature();
         ByteArray hashOfPayload = new ByteArray(refreshTTLMessage.getHashOfPayload());
@@ -464,7 +480,9 @@ public boolean refreshTTL(RefreshOfferMessage refreshTTLMessage, @Nullable NodeA
         }
     }
 
-    public boolean remove(ProtectedStorageEntry protectedStorageEntry, @Nullable NodeAddress sender, boolean isDataOwner) {
+    public boolean remove(ProtectedStorageEntry protectedStorageEntry,
+                          @Nullable NodeAddress sender,
+                          boolean isDataOwner) {
         final ProtectedStoragePayload protectedStoragePayload = protectedStorageEntry.getProtectedStoragePayload();
         ByteArray hashOfPayload = get32ByteHashAsByteArray(protectedStoragePayload);
         boolean containsKey = map.containsKey(hashOfPayload);
@@ -483,6 +501,8 @@ && checkSignature(protectedStorageEntry)
             sequenceNumberMap.put(hashOfPayload, new MapValue(protectedStorageEntry.getSequenceNumber(), System.currentTimeMillis()));
             sequenceNumberMapStorage.queueUpForSave(SequenceNumberMap.clone(sequenceNumberMap), 300);
 
+            maybeAddToRemoveAddOncePayloads(protectedStoragePayload, hashOfPayload);
+
             broadcast(new RemoveDataMessage(protectedStorageEntry), sender, null, isDataOwner);
 
             removeFromProtectedDataStore(protectedStorageEntry);
@@ -506,33 +526,47 @@ private void removeFromProtectedDataStore(ProtectedStorageEntry protectedStorage
     }
 
     @SuppressWarnings("UnusedReturnValue")
-    public boolean removeMailboxData(ProtectedMailboxStorageEntry protectedMailboxStorageEntry, @Nullable NodeAddress sender, boolean isDataOwner) {
-        ByteArray hashOfData = get32ByteHashAsByteArray(protectedMailboxStorageEntry.getProtectedStoragePayload());
-        boolean containsKey = map.containsKey(hashOfData);
+    public boolean removeMailboxData(ProtectedMailboxStorageEntry protectedMailboxStorageEntry,
+                                     @Nullable NodeAddress sender,
+                                     boolean isDataOwner) {
+        ProtectedStoragePayload protectedStoragePayload = protectedMailboxStorageEntry.getProtectedStoragePayload();
+        ByteArray hashOfPayload = get32ByteHashAsByteArray(protectedStoragePayload);
+        boolean containsKey = map.containsKey(hashOfPayload);
         if (!containsKey)
             log.debug("Remove data ignored as we don't have an entry for that data.");
+
         boolean result = containsKey
                 && checkPublicKeys(protectedMailboxStorageEntry, false)
-                && isSequenceNrValid(protectedMailboxStorageEntry.getSequenceNumber(), hashOfData)
+                && isSequenceNrValid(protectedMailboxStorageEntry.getSequenceNumber(), hashOfPayload)
                 && protectedMailboxStorageEntry.getMailboxStoragePayload().getOwnerPubKey().equals(protectedMailboxStorageEntry.getReceiversPubKey()) // at remove both keys are the same (only receiver is able to remove data)
                 && checkSignature(protectedMailboxStorageEntry)
-                && checkIfStoredMailboxDataMatchesNewMailboxData(protectedMailboxStorageEntry.getReceiversPubKey(), hashOfData);
+                && checkIfStoredMailboxDataMatchesNewMailboxData(protectedMailboxStorageEntry.getReceiversPubKey(), hashOfPayload);
 
         // printData("before removeMailboxData");
         if (result) {
-            doRemoveProtectedExpirableData(protectedMailboxStorageEntry, hashOfData);
+            doRemoveProtectedExpirableData(protectedMailboxStorageEntry, hashOfPayload);
             printData("after removeMailboxData");
-            sequenceNumberMap.put(hashOfData, new MapValue(protectedMailboxStorageEntry.getSequenceNumber(), System.currentTimeMillis()));
+            sequenceNumberMap.put(hashOfPayload, new MapValue(protectedMailboxStorageEntry.getSequenceNumber(), System.currentTimeMillis()));
             sequenceNumberMapStorage.queueUpForSave(SequenceNumberMap.clone(sequenceNumberMap), 300);
 
+            maybeAddToRemoveAddOncePayloads(protectedStoragePayload, hashOfPayload);
+
             broadcast(new RemoveMailboxDataMessage(protectedMailboxStorageEntry), sender, null, isDataOwner);
         } else {
             log.debug("removeMailboxData failed");
         }
         return result;
     }
 
-    public ProtectedStorageEntry getProtectedStorageEntry(ProtectedStoragePayload protectedStoragePayload, KeyPair ownerStoragePubKey)
+    private void maybeAddToRemoveAddOncePayloads(ProtectedStoragePayload protectedStoragePayload,
+                                                 ByteArray hashOfData) {
+        if (protectedStoragePayload instanceof AddOncePayload) {
+            removedAddOncePayloads.add(hashOfData);
+        }
+    }
+
+    public ProtectedStorageEntry getProtectedStorageEntry(ProtectedStoragePayload protectedStoragePayload,
+                                                          KeyPair ownerStoragePubKey)
             throws CryptoException {
         ByteArray hashOfData = get32ByteHashAsByteArray(protectedStoragePayload);
         int sequenceNumber;
@@ -546,7 +580,8 @@ public ProtectedStorageEntry getProtectedStorageEntry(ProtectedStoragePayload pr
         return new ProtectedStorageEntry(protectedStoragePayload, ownerStoragePubKey.getPublic(), sequenceNumber, signature);
     }
 
-    public RefreshOfferMessage getRefreshTTLMessage(ProtectedStoragePayload protectedStoragePayload, KeyPair ownerStoragePubKey)
+    public RefreshOfferMessage getRefreshTTLMessage(ProtectedStoragePayload protectedStoragePayload,
+                                                    KeyPair ownerStoragePubKey)
             throws CryptoException {
         ByteArray hashOfPayload = get32ByteHashAsByteArray(protectedStoragePayload);
         int sequenceNumber;
@@ -561,7 +596,8 @@ public RefreshOfferMessage getRefreshTTLMessage(ProtectedStoragePayload protecte
     }
 
     public ProtectedMailboxStorageEntry getMailboxDataWithSignedSeqNr(MailboxStoragePayload expirableMailboxStoragePayload,
-                                                                      KeyPair storageSignaturePubKey, PublicKey receiversPublicKey)
+                                                                      KeyPair storageSignaturePubKey,
+                                                                      PublicKey receiversPublicKey)
             throws CryptoException {
         ByteArray hashOfData = get32ByteHashAsByteArray(expirableMailboxStoragePayload);
         int sequenceNumber;

p2p/src/main/java/bisq/network/p2p/storage/messages/AddOncePayload.java

@@ -0,0 +1,24 @@
+/*
+ * This file is part of Bisq.
+ *
+ * Bisq is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or (at
+ * your option) any later version.
+ *
+ * Bisq is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Bisq. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package bisq.network.p2p.storage.messages;
+
+/**
+ * Marker interface for messages which must not be added again after a remove message has been received (e.g. MailboxMessages).
+ */
+public interface AddOncePayload {
+}

p2p/src/main/java/bisq/network/p2p/storage/payload/MailboxStoragePayload.java

@@ -18,6 +18,7 @@
 package bisq.network.p2p.storage.payload;
 
 import bisq.network.p2p.PrefixedSealedAndSignedMessage;
+import bisq.network.p2p.storage.messages.AddOncePayload;
 
 import bisq.common.crypto.Sig;
 import bisq.common.util.ExtraDataMapValidator;
@@ -49,7 +50,7 @@
 @Getter
 @EqualsAndHashCode
 @Slf4j
-public final class MailboxStoragePayload implements ProtectedStoragePayload, ExpirablePayload {
+public final class MailboxStoragePayload implements ProtectedStoragePayload, ExpirablePayload, AddOncePayload {
     private final PrefixedSealedAndSignedMessage prefixedSealedAndSignedMessage;
     private PublicKey senderPubKeyForAddOperation;
     private final byte[] senderPubKeyForAddOperationBytes;