From 2c2a57ef9d8fd227278fe2c5a39d72bfc0ac2533 Mon Sep 17 00:00:00 2001
From: Julian Knutsen <julianknutsen@users.noreply.github.com>
Date: Fri, 8 Nov 2019 10:32:49 -0800
Subject: [PATCH] [REFACTOR] Remove duplicated code in refreshTTL

The custom code to verify the refreshTTLMessage's signature and update
an entry isn't necessary. Just have the code construct an updated
ProtectedStorageEntry from the existing and new data, verify it,
and add it to the map.

This also allows the removal of the ProtectedStorageEntry APIs
that modify internal state.
---
 .../network/p2p/storage/P2PDataStorage.java   | 36 +++++++++----------
 .../payload/ProtectedStorageEntry.java        | 11 ++----
 2 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java b/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java
index 7696bd406db..613f9bcdc91 100644
--- a/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java
+++ b/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java
@@ -67,8 +67,6 @@
 
 import com.google.common.annotations.VisibleForTesting;
 
-import org.apache.commons.lang3.StringUtils;
-
 import java.security.KeyPair;
 import java.security.PublicKey;
 
@@ -444,6 +442,7 @@ private void broadcastProtectedStorageEntry(ProtectedStorageEntry protectedStora
     public boolean refreshTTL(RefreshOfferMessage refreshTTLMessage,
                               @Nullable NodeAddress sender,
                               boolean isDataOwner) {
+
         ByteArray hashOfPayload = new ByteArray(refreshTTLMessage.getHashOfPayload());
 
         if (!map.containsKey((hashOfPayload))) {
@@ -452,34 +451,33 @@ public boolean refreshTTL(RefreshOfferMessage refreshTTLMessage,
             return false;
         }
 
-        int sequenceNumber = refreshTTLMessage.getSequenceNumber();
+        ProtectedStorageEntry storedEntry = map.get(hashOfPayload);
+        ProtectedStorageEntry updatedEntry = new ProtectedStorageEntry(
+                storedEntry.getProtectedStoragePayload(),
+                storedEntry.getOwnerPubKey(),
+                refreshTTLMessage.getSequenceNumber(),
+                refreshTTLMessage.getSignature(),
+                this.clock);
 
-        if(!hasSequenceNrIncreased(sequenceNumber, hashOfPayload))
-            return false;
 
-        ProtectedStorageEntry storedData = map.get(hashOfPayload);
-        PublicKey ownerPubKey = storedData.getProtectedStoragePayload().getOwnerPubKey();
-        byte[] hashOfDataAndSeqNr = refreshTTLMessage.getHashOfDataAndSeqNr();
-        byte[] signature = refreshTTLMessage.getSignature();
+        // If we have seen a more recent operation for this payload, we ignore the current one
+        if(!hasSequenceNrIncreased(updatedEntry.getSequenceNumber(), hashOfPayload))
+            return false;
 
-        // Verify the RefreshOfferMessage is well formed and valid for the refresh operation
-        if (!checkSignature(ownerPubKey, hashOfDataAndSeqNr, signature))
+        // Verify the updated ProtectedStorageEntry is well formed and valid for update
+        if (!checkSignature(updatedEntry))
             return false;
 
         // Verify the Payload owner and the Entry owner for the stored Entry are the same
         // TODO: This is also checked in the validation for the original add(), investigate if this can be removed
-        if (!checkIfStoredDataPubKeyMatchesNewDataPubKey(ownerPubKey, hashOfPayload))
+        if (!checkIfStoredDataPubKeyMatchesNewDataPubKey(updatedEntry.getOwnerPubKey(), hashOfPayload))
             return false;
 
-        // This is a valid refresh, update the payload for it
-        log.debug("refreshDate called for storedData:\n\t" + StringUtils.abbreviate(storedData.toString(), 100));
-        storedData.refreshTTL(this.clock);
-        storedData.updateSequenceNumber(sequenceNumber);
-        storedData.updateSignature(signature);
-        printData("after refreshTTL");
+        // Update the hash map with the updated entry
+        map.put(hashOfPayload, updatedEntry);
 
         // Record the latest sequence number and persist it
-        sequenceNumberMap.put(hashOfPayload, new MapValue(sequenceNumber, this.clock.millis()));
+        sequenceNumberMap.put(hashOfPayload, new MapValue(updatedEntry.getSequenceNumber(), this.clock.millis()));
         sequenceNumberMapStorage.queueUpForSave(SequenceNumberMap.clone(sequenceNumberMap), 1000);
 
         // Always broadcast refreshes
diff --git a/p2p/src/main/java/bisq/network/p2p/storage/payload/ProtectedStorageEntry.java b/p2p/src/main/java/bisq/network/p2p/storage/payload/ProtectedStorageEntry.java
index 67d71d5031d..71f264d386e 100644
--- a/p2p/src/main/java/bisq/network/p2p/storage/payload/ProtectedStorageEntry.java
+++ b/p2p/src/main/java/bisq/network/p2p/storage/payload/ProtectedStorageEntry.java
@@ -40,7 +40,7 @@ public class ProtectedStorageEntry implements NetworkPayload, PersistablePayload
     private final ProtectedStoragePayload protectedStoragePayload;
     private final byte[] ownerPubKeyBytes;
     transient private final PublicKey ownerPubKey;
-    private int sequenceNumber;
+    private final int sequenceNumber;
     private byte[] signature;
     private long creationTimeStamp;
 
@@ -133,19 +133,12 @@ public void maybeAdjustCreationTimeStamp(Clock clock) {
             creationTimeStamp = clock.millis();
     }
 
-    public void refreshTTL(Clock clock) {
-        creationTimeStamp = clock.millis();
-    }
-
     public void backDate() {
         if (protectedStoragePayload instanceof ExpirablePayload)
             creationTimeStamp -= ((ExpirablePayload) protectedStoragePayload).getTTL() / 2;
     }
 
-    public void updateSequenceNumber(int sequenceNumber) {
-        this.sequenceNumber = sequenceNumber;
-    }
-
+    // TODO: only used in tests so find a better way to test and delete public API
     public void updateSignature(byte[] signature) {
         this.signature = signature;
     }