From 2e50e4c5d7e96790c4812c825b386761e01d48a7 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 14:55:01 -0500 Subject: [PATCH 01/29] Add PersistableNetworkPayloadStore --- .../PersistableNetworkPayloadStore.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadStore.java diff --git a/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadStore.java b/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadStore.java new file mode 100644 index 00000000000..751e0b63411 --- /dev/null +++ b/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadStore.java @@ -0,0 +1,20 @@ +package bisq.network.p2p.storage.persistence; + +import bisq.network.p2p.storage.P2PDataStorage; +import bisq.network.p2p.storage.payload.PersistableNetworkPayload; + +import bisq.common.proto.persistable.ThreadedPersistableEnvelope; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +import lombok.Getter; + +/** + * Base class for store implementations using a map with a PersistableNetworkPayload + * as the type of the map value. + */ +public abstract class PersistableNetworkPayloadStore implements ThreadedPersistableEnvelope { + @Getter + public Map map = new ConcurrentHashMap<>(); +} From 240f0b903c81c559f55fa186badf107fb5c05a97 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 14:57:48 -0500 Subject: [PATCH 02/29] Use PersistableNetworkPayloadStore as base class for stores which had a map with PersistableNetworkPayloads --- .../bisq/core/account/sign/SignedWitnessStore.java | 11 ++--------- .../core/account/witness/AccountAgeWitnessStore.java | 11 ++--------- .../governance/blindvote/storage/BlindVoteStore.java | 11 ++--------- .../proposal/storage/appendonly/ProposalStore.java | 11 ++--------- .../core/trade/statistics/TradeStatistics2Store.java | 11 ++--------- 5 files changed, 10 insertions(+), 45 deletions(-) diff --git a/core/src/main/java/bisq/core/account/sign/SignedWitnessStore.java b/core/src/main/java/bisq/core/account/sign/SignedWitnessStore.java index 8b0e340c41f..a1e8372f8e1 100644 --- a/core/src/main/java/bisq/core/account/sign/SignedWitnessStore.java +++ b/core/src/main/java/bisq/core/account/sign/SignedWitnessStore.java @@ -19,18 +19,13 @@ import bisq.network.p2p.storage.P2PDataStorage; -import bisq.network.p2p.storage.payload.PersistableNetworkPayload; - -import bisq.common.proto.persistable.ThreadedPersistableEnvelope; +import bisq.network.p2p.storage.persistence.PersistableNetworkPayloadStore; import com.google.protobuf.Message; import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; -import lombok.Getter; import lombok.extern.slf4j.Slf4j; @@ -40,9 +35,7 @@ * definition and provide a hashMap for the domain access. */ @Slf4j -public class SignedWitnessStore implements ThreadedPersistableEnvelope { - @Getter - private Map map = new ConcurrentHashMap<>(); +public class SignedWitnessStore extends PersistableNetworkPayloadStore { SignedWitnessStore() { } diff --git a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessStore.java b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessStore.java index 4731cb9e624..0da7f9d211d 100644 --- a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessStore.java +++ b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessStore.java @@ -18,18 +18,13 @@ package bisq.core.account.witness; import bisq.network.p2p.storage.P2PDataStorage; -import bisq.network.p2p.storage.payload.PersistableNetworkPayload; - -import bisq.common.proto.persistable.ThreadedPersistableEnvelope; +import bisq.network.p2p.storage.persistence.PersistableNetworkPayloadStore; import com.google.protobuf.Message; import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; -import lombok.Getter; import lombok.extern.slf4j.Slf4j; @@ -39,9 +34,7 @@ * definition and provide a hashMap for the domain access. */ @Slf4j -public class AccountAgeWitnessStore implements ThreadedPersistableEnvelope { - @Getter - private Map map = new ConcurrentHashMap<>(); +public class AccountAgeWitnessStore extends PersistableNetworkPayloadStore { AccountAgeWitnessStore() { } diff --git a/core/src/main/java/bisq/core/dao/governance/blindvote/storage/BlindVoteStore.java b/core/src/main/java/bisq/core/dao/governance/blindvote/storage/BlindVoteStore.java index 3b7266fca3e..32e5b256b91 100644 --- a/core/src/main/java/bisq/core/dao/governance/blindvote/storage/BlindVoteStore.java +++ b/core/src/main/java/bisq/core/dao/governance/blindvote/storage/BlindVoteStore.java @@ -18,18 +18,13 @@ package bisq.core.dao.governance.blindvote.storage; import bisq.network.p2p.storage.P2PDataStorage; -import bisq.network.p2p.storage.payload.PersistableNetworkPayload; - -import bisq.common.proto.persistable.ThreadedPersistableEnvelope; +import bisq.network.p2p.storage.persistence.PersistableNetworkPayloadStore; import com.google.protobuf.Message; import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; -import lombok.Getter; import lombok.extern.slf4j.Slf4j; @@ -39,9 +34,7 @@ * definition and provide a hashMap for the domain access. */ @Slf4j -public class BlindVoteStore implements ThreadedPersistableEnvelope { - @Getter - private Map map = new ConcurrentHashMap<>(); +public class BlindVoteStore extends PersistableNetworkPayloadStore { BlindVoteStore() { } diff --git a/core/src/main/java/bisq/core/dao/governance/proposal/storage/appendonly/ProposalStore.java b/core/src/main/java/bisq/core/dao/governance/proposal/storage/appendonly/ProposalStore.java index 2173f7c4770..6652362d14a 100644 --- a/core/src/main/java/bisq/core/dao/governance/proposal/storage/appendonly/ProposalStore.java +++ b/core/src/main/java/bisq/core/dao/governance/proposal/storage/appendonly/ProposalStore.java @@ -18,18 +18,13 @@ package bisq.core.dao.governance.proposal.storage.appendonly; import bisq.network.p2p.storage.P2PDataStorage; -import bisq.network.p2p.storage.payload.PersistableNetworkPayload; - -import bisq.common.proto.persistable.ThreadedPersistableEnvelope; +import bisq.network.p2p.storage.persistence.PersistableNetworkPayloadStore; import com.google.protobuf.Message; import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; -import lombok.Getter; import lombok.extern.slf4j.Slf4j; @@ -39,9 +34,7 @@ * definition and provide a hashMap for the domain access. */ @Slf4j -public class ProposalStore implements ThreadedPersistableEnvelope { - @Getter - private Map map = new ConcurrentHashMap<>(); +public class ProposalStore extends PersistableNetworkPayloadStore { ProposalStore() { } diff --git a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2Store.java b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2Store.java index db1c7943f28..8a4b8fb8648 100644 --- a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2Store.java +++ b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2Store.java @@ -18,18 +18,13 @@ package bisq.core.trade.statistics; import bisq.network.p2p.storage.P2PDataStorage; -import bisq.network.p2p.storage.payload.PersistableNetworkPayload; - -import bisq.common.proto.persistable.ThreadedPersistableEnvelope; +import bisq.network.p2p.storage.persistence.PersistableNetworkPayloadStore; import com.google.protobuf.Message; import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; -import lombok.Getter; import lombok.extern.slf4j.Slf4j; /** @@ -38,9 +33,7 @@ * definition and provide a hashMap for the domain access. */ @Slf4j -public class TradeStatistics2Store implements ThreadedPersistableEnvelope { - @Getter - private final Map map = new ConcurrentHashMap<>(); +public class TradeStatistics2Store extends PersistableNetworkPayloadStore { TradeStatistics2Store() { } From 431debe05e757f2e5da0294462f521aa897a749e Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 14:59:27 -0500 Subject: [PATCH 03/29] Unrelated to PR topic fix: Only log warnings if banned object is not isEmpty --- .../main/java/bisq/core/provider/ProvidersRepository.java | 5 +++-- .../bisq/core/support/dispute/agent/DisputeAgentService.java | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/bisq/core/provider/ProvidersRepository.java b/core/src/main/java/bisq/core/provider/ProvidersRepository.java index 716a0c8d209..13a67acb98d 100644 --- a/core/src/main/java/bisq/core/provider/ProvidersRepository.java +++ b/core/src/main/java/bisq/core/provider/ProvidersRepository.java @@ -79,11 +79,12 @@ public void applyBannedNodes(@Nullable List bannedNodes) { fillProviderList(); selectNextProviderBaseUrl(); - if (bannedNodes == null) + if (bannedNodes == null) { log.info("Selected provider baseUrl={}, providerList={}", baseUrl, providerList); - else + } else if (!bannedNodes.isEmpty()) { log.warn("We have banned provider nodes: bannedNodes={}, selected provider baseUrl={}, providerList={}", bannedNodes, baseUrl, providerList); + } } public void selectNextProviderBaseUrl() { diff --git a/core/src/main/java/bisq/core/support/dispute/agent/DisputeAgentService.java b/core/src/main/java/bisq/core/support/dispute/agent/DisputeAgentService.java index e50a4abeb61..335a6f7d535 100644 --- a/core/src/main/java/bisq/core/support/dispute/agent/DisputeAgentService.java +++ b/core/src/main/java/bisq/core/support/dispute/agent/DisputeAgentService.java @@ -100,8 +100,11 @@ public Map getDisputeAgents() { } else { bannedDisputeAgents = null; } - if (bannedDisputeAgents != null) + + if (bannedDisputeAgents != null && !bannedDisputeAgents.isEmpty()) { log.warn("bannedDisputeAgents=" + bannedDisputeAgents); + } + Set disputeAgentSet = getDisputeAgentSet(bannedDisputeAgents); Map map = new HashMap<>(); From b90fd3968d3e21da912cdc7743fc607053fa806f Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 15:00:32 -0500 Subject: [PATCH 04/29] Unrelated to PR topic: Improve logs for windows: \n is not recognized and reading logs from windows users makes it harder without line breaks. --- .../network/p2p/peers/getdata/RequestDataHandler.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/p2p/src/main/java/bisq/network/p2p/peers/getdata/RequestDataHandler.java b/p2p/src/main/java/bisq/network/p2p/peers/getdata/RequestDataHandler.java index 7923420031b..e00fdd67bf9 100644 --- a/p2p/src/main/java/bisq/network/p2p/peers/getdata/RequestDataHandler.java +++ b/p2p/src/main/java/bisq/network/p2p/peers/getdata/RequestDataHandler.java @@ -260,15 +260,16 @@ private void logContents(NetworkEnvelope networkEnvelope, // Log different data types StringBuilder sb = new StringBuilder(); - sb.append("\n#################################################################\n"); - sb.append("Connected to node: " + peersNodeAddress.getFullAddress() + "\n"); + String sep = System.lineSeparator(); + sb.append(sep).append("#################################################################").append(sep); + sb.append("Connected to node: ").append(peersNodeAddress.getFullAddress()).append(sep); int items = dataSet.size() + persistableNetworkPayloadSet.size(); sb.append("Received ").append(items).append(" instances from a ") - .append(getDataRequestType).append("\n"); + .append(getDataRequestType).append(sep); payloadByClassName.forEach((key, value) -> sb.append(key) .append(": ") .append(value.size()) - .append("\n")); + .append(sep)); sb.append("#################################################################"); log.info(sb.toString()); } From 7122ef0356add96a11c533880f76ee6f248d329d Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 15:01:33 -0500 Subject: [PATCH 05/29] Refactoring: Rename variables --- .../p2p/peers/getdata/GetDataRequestHandler.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/p2p/src/main/java/bisq/network/p2p/peers/getdata/GetDataRequestHandler.java b/p2p/src/main/java/bisq/network/p2p/peers/getdata/GetDataRequestHandler.java index cc80bbda066..93fca1d8083 100644 --- a/p2p/src/main/java/bisq/network/p2p/peers/getdata/GetDataRequestHandler.java +++ b/p2p/src/main/java/bisq/network/p2p/peers/getdata/GetDataRequestHandler.java @@ -88,22 +88,22 @@ public void handle(GetDataRequest getDataRequest, final Connection connection) { .map(e -> "node address " + e.getFullAddress()) .orElseGet(() -> "connection UID " + connection.getUid()); - AtomicBoolean outPersistableNetworkPayloadOutputTruncated = new AtomicBoolean(false); - AtomicBoolean outProtectedStoragePayloadOutputTruncated = new AtomicBoolean(false); + AtomicBoolean wasPersistableNetworkPayloadsTruncated = new AtomicBoolean(false); + AtomicBoolean wasProtectedStorageEntriesTruncated = new AtomicBoolean(false); GetDataResponse getDataResponse = dataStorage.buildGetDataResponse( getDataRequest, MAX_ENTRIES, - outPersistableNetworkPayloadOutputTruncated, - outProtectedStoragePayloadOutputTruncated, + wasPersistableNetworkPayloadsTruncated, + wasProtectedStorageEntriesTruncated, connection.getCapabilities()); - if (outPersistableNetworkPayloadOutputTruncated.get()) { + if (wasPersistableNetworkPayloadsTruncated.get()) { log.warn("The getData request from peer with {} caused too much PersistableNetworkPayload " + "entries to get delivered. We limited the entries for the response to {} entries", connectionInfo, MAX_ENTRIES); } - if (outProtectedStoragePayloadOutputTruncated.get()) { + if (wasProtectedStorageEntriesTruncated.get()) { log.warn("The getData request from peer with {} caused too much ProtectedStorageEntry " + "entries to get delivered. We limited the entries for the response to {} entries", connectionInfo, MAX_ENTRIES); From df90b2440ad525191e865258cd6603fbc88db92f Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 15:02:44 -0500 Subject: [PATCH 06/29] Add new methods and refactor APIs. Needed later for new classes.... --- common/src/main/java/bisq/common/storage/Storage.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/common/src/main/java/bisq/common/storage/Storage.java b/common/src/main/java/bisq/common/storage/Storage.java index 209cdff82bb..3521d8c6844 100644 --- a/common/src/main/java/bisq/common/storage/Storage.java +++ b/common/src/main/java/bisq/common/storage/Storage.java @@ -77,12 +77,17 @@ public Storage(@Named(Config.STORAGE_DIR) File dir, this.corruptedDatabaseFilesHandler = corruptedDatabaseFilesHandler; } + @Nullable + public T getPersisted(String fileName) { + return getPersisted(new File(dir, fileName)); + } + @Nullable public T initAndGetPersistedWithFileName(String fileName, long delay) { this.fileName = fileName; storageFile = new File(dir, fileName); fileManager = new FileManager<>(dir, storageFile, delay, persistenceProtoResolver); - return getPersisted(); + return getPersisted(storageFile); } @Nullable @@ -96,7 +101,7 @@ public T initAndGetPersisted(T persistable, String fileName, long delay) { this.fileName = fileName; storageFile = new File(dir, fileName); fileManager = new FileManager<>(dir, storageFile, delay, persistenceProtoResolver); - return getPersisted(); + return getPersisted(storageFile); } public void queueUpForSave() { @@ -144,7 +149,7 @@ public void remove(String fileName) { // We do the file read on the UI thread to avoid problems from multi threading. // Data are small and read is done only at startup, so it is no performance issue. @Nullable - private T getPersisted() { + private T getPersisted(File storageFile) { if (storageFile.exists()) { long now = System.currentTimeMillis(); try { From 9446f2807d0d55297e0f60e5909d2a3447ec9154 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 15:04:08 -0500 Subject: [PATCH 07/29] Split read store and get store so it can be reused from new class (future commit) Change API --- .../p2p/storage/persistence/StoreService.java | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/p2p/src/main/java/bisq/network/p2p/storage/persistence/StoreService.java b/p2p/src/main/java/bisq/network/p2p/storage/persistence/StoreService.java index 5476ad9403e..625123184d0 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/persistence/StoreService.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/persistence/StoreService.java @@ -81,33 +81,33 @@ protected T getStore() { /////////////////////////////////////////////////////////////////////////////////////////// protected void readFromResources(String postFix) { - makeFileFromResourceFile(postFix); + String fileName = getFileName(); + makeFileFromResourceFile(fileName, postFix); try { readStore(); } catch (Throwable t) { try { - String fileName = getFileName(); storage.removeAndBackupFile(fileName); } catch (IOException e) { log.error(e.toString()); } - makeFileFromResourceFile(postFix); + makeFileFromResourceFile(fileName, postFix); readStore(); } } - protected void makeFileFromResourceFile(String postFix) { - final String fileName = getFileName(); + protected boolean makeFileFromResourceFile(String fileName, String postFix) { String resourceFileName = fileName + postFix; File dbDir = new File(absolutePathOfStorageDir); if (!dbDir.exists() && !dbDir.mkdir()) log.warn("make dir failed.\ndbDir=" + dbDir.getAbsolutePath()); - final File destinationFile = new File(Paths.get(absolutePathOfStorageDir, fileName).toString()); + File destinationFile = new File(Paths.get(absolutePathOfStorageDir, fileName).toString()); if (!destinationFile.exists()) { try { log.info("We copy resource to file: resourceFileName={}, destinationFile={}", resourceFileName, destinationFile); FileUtil.resourceToFile(resourceFileName, destinationFile); + return true; } catch (ResourceNotFoundException e) { log.info("Could not find resourceFile " + resourceFileName + ". That is expected if none is provided yet."); } catch (Throwable e) { @@ -116,14 +116,13 @@ protected void makeFileFromResourceFile(String postFix) { e.printStackTrace(); } } else { - log.debug(fileName + " file exists already."); + log.info("No resource file have been copied. {} exists already.", fileName); } + return false; } - - protected void readStore() { - final String fileName = getFileName(); - store = storage.initAndGetPersistedWithFileName(fileName, 100); + protected T getStore(String fileName) { + T store = storage.initAndGetPersistedWithFileName(fileName, 100); if (store != null) { log.info("{}: size of {}: {} MB", this.getClass().getSimpleName(), storage.getClass().getSimpleName(), @@ -131,6 +130,11 @@ protected void readStore() { } else { store = createStore(); } + return store; + } + + protected void readStore() { + store = getStore(getFileName()); } protected abstract T createStore(); From 384152fb6be8917c92dc825030b7355063964f18 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 15:05:31 -0500 Subject: [PATCH 08/29] Add version field to data requests classes --- .../getdata/messages/GetDataRequest.java | 11 +++++- .../messages/GetUpdatedDataRequest.java | 24 ++++++++----- .../messages/PreliminaryGetDataRequest.java | 34 ++++++++++++------- proto/src/main/proto/pb.proto | 2 ++ 4 files changed, 50 insertions(+), 21 deletions(-) diff --git a/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetDataRequest.java b/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetDataRequest.java index cfb0b301925..65cc5177dfd 100644 --- a/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetDataRequest.java +++ b/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetDataRequest.java @@ -27,6 +27,8 @@ import lombok.Getter; import lombok.ToString; +import javax.annotation.Nullable; + @EqualsAndHashCode(callSuper = true) @Getter @ToString @@ -35,11 +37,18 @@ public abstract class GetDataRequest extends NetworkEnvelope implements Extended // Keys for ProtectedStorageEntry items to be excluded from the request because the peer has them already protected final Set excludedKeys; + // Added at v1.4.0 + // The version of the requester. Used for response to send potentially missing historical data + @Nullable + protected final String version; + public GetDataRequest(int messageVersion, int nonce, - Set excludedKeys) { + Set excludedKeys, + @Nullable String version) { super(messageVersion); this.nonce = nonce; this.excludedKeys = excludedKeys; + this.version = version; } } diff --git a/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetUpdatedDataRequest.java b/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetUpdatedDataRequest.java index 0539fa02bc6..25befecab42 100644 --- a/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetUpdatedDataRequest.java +++ b/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetUpdatedDataRequest.java @@ -27,6 +27,7 @@ import com.google.protobuf.ByteString; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -34,7 +35,7 @@ import lombok.Value; import lombok.extern.slf4j.Slf4j; -import static com.google.common.base.Preconditions.checkNotNull; +import javax.annotation.Nullable; @Slf4j @EqualsAndHashCode(callSuper = true) @@ -48,6 +49,7 @@ public GetUpdatedDataRequest(NodeAddress senderNodeAddress, this(senderNodeAddress, nonce, excludedKeys, + Version.VERSION, Version.getP2PMessageVersion()); } @@ -59,35 +61,41 @@ public GetUpdatedDataRequest(NodeAddress senderNodeAddress, private GetUpdatedDataRequest(NodeAddress senderNodeAddress, int nonce, Set excludedKeys, + @Nullable String version, int messageVersion) { super(messageVersion, nonce, - excludedKeys); - checkNotNull(senderNodeAddress, "senderNodeAddress must not be null at GetUpdatedDataRequest"); + excludedKeys, + version); this.senderNodeAddress = senderNodeAddress; } @Override public protobuf.NetworkEnvelope toProtoNetworkEnvelope() { - final protobuf.GetUpdatedDataRequest.Builder builder = protobuf.GetUpdatedDataRequest.newBuilder() + protobuf.GetUpdatedDataRequest.Builder builder = protobuf.GetUpdatedDataRequest.newBuilder() .setSenderNodeAddress(senderNodeAddress.toProtoMessage()) .setNonce(nonce) .addAllExcludedKeys(excludedKeys.stream() .map(ByteString::copyFrom) .collect(Collectors.toList())); - + Optional.ofNullable(version).ifPresent(builder::setVersion); NetworkEnvelope proto = getNetworkEnvelopeBuilder() .setGetUpdatedDataRequest(builder) .build(); - log.info("Sending a GetUpdatedDataRequest with {} kB", proto.getSerializedSize() / 1000d); + log.info("Sending a GetUpdatedDataRequest with {} kB and {} excluded key entries. Requesters version={}", + proto.getSerializedSize() / 1000d, excludedKeys.size(), version); return proto; } public static GetUpdatedDataRequest fromProto(protobuf.GetUpdatedDataRequest proto, int messageVersion) { - log.info("Received a GetUpdatedDataRequest with {} kB", proto.getSerializedSize() / 1000d); + Set excludedKeys = ProtoUtil.byteSetFromProtoByteStringList(proto.getExcludedKeysList()); + String requestersVersion = ProtoUtil.stringOrNullFromProto(proto.getVersion()); + log.info("Received a GetUpdatedDataRequest with {} kB and {} excluded key entries. Requesters version={}", + proto.getSerializedSize() / 1000d, excludedKeys.size(), requestersVersion); return new GetUpdatedDataRequest(NodeAddress.fromProto(proto.getSenderNodeAddress()), proto.getNonce(), - ProtoUtil.byteSetFromProtoByteStringList(proto.getExcludedKeysList()), + excludedKeys, + requestersVersion, messageVersion); } } diff --git a/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/PreliminaryGetDataRequest.java b/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/PreliminaryGetDataRequest.java index a47a2f86054..f36cb198072 100644 --- a/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/PreliminaryGetDataRequest.java +++ b/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/PreliminaryGetDataRequest.java @@ -28,6 +28,7 @@ import com.google.protobuf.ByteString; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -35,7 +36,7 @@ import lombok.Value; import lombok.extern.slf4j.Slf4j; -import org.jetbrains.annotations.NotNull; +import javax.annotation.Nullable; @Slf4j @EqualsAndHashCode(callSuper = true) @@ -43,9 +44,12 @@ public final class PreliminaryGetDataRequest extends GetDataRequest implements AnonymousMessage, SupportedCapabilitiesMessage { private final Capabilities supportedCapabilities; - public PreliminaryGetDataRequest(int nonce, - @NotNull Set excludedKeys) { - this(nonce, excludedKeys, Capabilities.app, Version.getP2PMessageVersion()); + public PreliminaryGetDataRequest(int nonce, Set excludedKeys) { + this(nonce, + excludedKeys, + Version.VERSION, + Capabilities.app, + Version.getP2PMessageVersion()); } @@ -54,34 +58,40 @@ public PreliminaryGetDataRequest(int nonce, /////////////////////////////////////////////////////////////////////////////////////////// private PreliminaryGetDataRequest(int nonce, - @NotNull Set excludedKeys, - @NotNull Capabilities supportedCapabilities, + Set excludedKeys, + @Nullable String version, + Capabilities supportedCapabilities, int messageVersion) { - super(messageVersion, nonce, excludedKeys); + super(messageVersion, nonce, excludedKeys, version); this.supportedCapabilities = supportedCapabilities; } @Override public protobuf.NetworkEnvelope toProtoNetworkEnvelope() { - final protobuf.PreliminaryGetDataRequest.Builder builder = protobuf.PreliminaryGetDataRequest.newBuilder() + protobuf.PreliminaryGetDataRequest.Builder builder = protobuf.PreliminaryGetDataRequest.newBuilder() .addAllSupportedCapabilities(Capabilities.toIntList(supportedCapabilities)) .setNonce(nonce) .addAllExcludedKeys(excludedKeys.stream() .map(ByteString::copyFrom) .collect(Collectors.toList())); - + Optional.ofNullable(version).ifPresent(builder::setVersion); NetworkEnvelope proto = getNetworkEnvelopeBuilder() .setPreliminaryGetDataRequest(builder) .build(); - log.info("Sending a PreliminaryGetDataRequest with {} kB", proto.getSerializedSize() / 1000d); + log.info("Sending a PreliminaryGetDataRequest with {} kB and {} excluded key entries. Requesters version={}", + proto.getSerializedSize() / 1000d, excludedKeys.size(), version); return proto; } public static PreliminaryGetDataRequest fromProto(protobuf.PreliminaryGetDataRequest proto, int messageVersion) { - log.info("Received a PreliminaryGetDataRequest with {} kB", proto.getSerializedSize() / 1000d); + Set excludedKeys = ProtoUtil.byteSetFromProtoByteStringList(proto.getExcludedKeysList()); + String requestersVersion = ProtoUtil.stringOrNullFromProto(proto.getVersion()); + log.info("Received a PreliminaryGetDataRequest with {} kB and {} excluded key entries. Requesters version={}", + proto.getSerializedSize() / 1000d, excludedKeys.size(), requestersVersion); return new PreliminaryGetDataRequest(proto.getNonce(), - ProtoUtil.byteSetFromProtoByteStringList(proto.getExcludedKeysList()), + excludedKeys, + requestersVersion, Capabilities.fromIntList(proto.getSupportedCapabilitiesList()), messageVersion); } diff --git a/proto/src/main/proto/pb.proto b/proto/src/main/proto/pb.proto index 9a1b55250da..4c858b18a97 100644 --- a/proto/src/main/proto/pb.proto +++ b/proto/src/main/proto/pb.proto @@ -95,6 +95,7 @@ message PreliminaryGetDataRequest { int32 nonce = 21; repeated bytes excluded_keys = 2; repeated int32 supported_capabilities = 3; + string version = 4; } message GetDataResponse { @@ -109,6 +110,7 @@ message GetUpdatedDataRequest { NodeAddress sender_node_address = 1; int32 nonce = 2; repeated bytes excluded_keys = 3; + string version = 4; } // peers From c79504c8410db24cef4912f9eb7212c5987e86a4 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 15:05:46 -0500 Subject: [PATCH 09/29] Add getter (needed later) --- .../p2p/storage/persistence/AppendOnlyDataStoreService.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/p2p/src/main/java/bisq/network/p2p/storage/persistence/AppendOnlyDataStoreService.java b/p2p/src/main/java/bisq/network/p2p/storage/persistence/AppendOnlyDataStoreService.java index c7ea20c9438..0a87288f53f 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/persistence/AppendOnlyDataStoreService.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/persistence/AppendOnlyDataStoreService.java @@ -29,6 +29,7 @@ import java.util.Map; import java.util.stream.Collectors; +import lombok.Getter; import lombok.extern.slf4j.Slf4j; /** @@ -36,7 +37,8 @@ */ @Slf4j public class AppendOnlyDataStoreService { - private List> services = new ArrayList<>(); + @Getter + private final List> services = new ArrayList<>(); /////////////////////////////////////////////////////////////////////////////////////////// From 3fb73ac0a3e8722c7d215f56dde32f6a12b04944 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 15:06:04 -0500 Subject: [PATCH 10/29] Add HistoricalDataStoreService --- .../HistoricalDataStoreService.java | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java diff --git a/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java b/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java new file mode 100644 index 00000000000..e0b8f3078d6 --- /dev/null +++ b/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java @@ -0,0 +1,187 @@ +package bisq.network.p2p.storage.persistence; + +import bisq.network.p2p.storage.P2PDataStorage; +import bisq.network.p2p.storage.payload.PersistableNetworkPayload; + +import bisq.common.app.Version; +import bisq.common.storage.Storage; + +import com.google.common.collect.ImmutableMap; + +import java.io.File; + +import java.util.HashMap; +import java.util.Map; + +import lombok.extern.slf4j.Slf4j; + +/** + * Manages historical data stores tagged with the release versions. + * New data is added to the default map in the store (live data). Historical data is created from resource files. + * For initial data requests we only use the live data as the users version is sent with the + * request so the responding (seed)node can figure out if we miss any of the historical data. + */ +@Slf4j +public abstract class HistoricalDataStoreService extends MapStoreService { + private ImmutableMap storesByVersion; + // Cache to avoid that we have to recreate the historical data at each request + private ImmutableMap allHistoricalPayloads; + + + /////////////////////////////////////////////////////////////////////////////////////////// + // Constructor + /////////////////////////////////////////////////////////////////////////////////////////// + + public HistoricalDataStoreService(File storageDir, Storage storage) { + super(storageDir, storage); + } + + + /////////////////////////////////////////////////////////////////////////////////////////// + // API + /////////////////////////////////////////////////////////////////////////////////////////// + + // We give back a map of our live map and all historical maps newer than the requested version. + // If requestersVersion is null we return all historical data. + public Map getMapSinceVersion(String requestersVersion) { + // We add all our live data + Map result = new HashMap<>(store.getMap()); + + // If we have a store with a newer version than the requesters version we will add those as well. + storesByVersion.entrySet().stream() + .filter(entry -> { + // Old nodes not sending the version will get delivered all data + if (requestersVersion == null) { + log.info("The requester did not send a version. This is expected for not updated nodes."); + return true; + } + + // Otherwise we only add data if the requesters version is older then + // the version of the particular store. + String storeVersion = entry.getKey(); + boolean newVersion = Version.isNewVersion(storeVersion, requestersVersion); + String details = newVersion ? + "As our historical store is a newer version we add the data to our result map." : + "As the requester version is not older as our historical store we do not " + + "add the data to the result map."; + log.info("The requester had version {}. Our historical data store has version {}.\n{}", + requestersVersion, storeVersion, details); + return newVersion; + }) + .map(e -> e.getValue().getMap()) + .forEach(result::putAll); + + log.info("We found {} entries since requesters version {}", + result.size(), requestersVersion); + return result; + } + + public Map getMapOfLiveData() { + return store.getMap(); + } + + public Map getMapOfAllData() { + Map result = new HashMap<>(getMapOfLiveData()); + result.putAll(allHistoricalPayloads); + return result; + } + + + /////////////////////////////////////////////////////////////////////////////////////////// + // MapStoreService + /////////////////////////////////////////////////////////////////////////////////////////// + + // TODO optimize so that callers to AppendOnlyDataStoreService are not invoking that often getMap + // ProposalService is one of the main callers and could avoid it by using the ProposalStoreService directly + // instead of AppendOnlyDataStoreService + + // By default we return the live data only. This method should not be used by domain clients but rather the + // custom methods getMapOfAllData, getMapOfLiveData or getMapSinceVersion + @Override + public Map getMap() { + return store.getMap(); + } + + @Override + protected void put(P2PDataStorage.ByteArray hash, PersistableNetworkPayload payload) { + if (anyMapContainsKey(hash)) { + return; + } + + getMapOfLiveData().put(hash, payload); + persist(); + } + + @Override + protected PersistableNetworkPayload putIfAbsent(P2PDataStorage.ByteArray hash, PersistableNetworkPayload payload) { + if (anyMapContainsKey(hash)) { + return null; + } + + PersistableNetworkPayload previous = getMapOfLiveData().put(hash, payload); + persist(); + return previous; + } + + + @Override + protected void readFromResources(String postFix) { + readStore(); + log.info("We have created the {} store for the live data and filled it with {} entries from the persisted data.", + getFileName(), getMapOfLiveData().size()); + + // Now we add our historical data stores. As they are immutable after created we use an ImmutableMap + ImmutableMap.Builder allHistoricalPayloadsBuilder = ImmutableMap.builder(); + ImmutableMap.Builder storesByVersionBuilder = ImmutableMap.builder(); + + Version.HISTORY.forEach(version -> readHistoricalStoreFromResources(version, postFix, allHistoricalPayloadsBuilder, storesByVersionBuilder)); + + allHistoricalPayloads = allHistoricalPayloadsBuilder.build(); + storesByVersion = storesByVersionBuilder.build(); + } + + + /////////////////////////////////////////////////////////////////////////////////////////// + // Private + /////////////////////////////////////////////////////////////////////////////////////////// + + private void readHistoricalStoreFromResources(String version, + String postFix, + ImmutableMap.Builder allHistoricalDataBuilder, + ImmutableMap.Builder storesByVersionBuilder) { + String fileName = getFileName() + "_" + version; + boolean wasCreatedFromResources = makeFileFromResourceFile(fileName, postFix); + + // If resource file does not exist we return null. We do not create a new store as it would never get filled. + PersistableNetworkPayloadStore historicalStore = storage.getPersisted(fileName); + if (historicalStore == null) { + log.warn("Resource file with file name {} does not exits.", fileName); + return; + } + + storesByVersionBuilder.put(version, historicalStore); + allHistoricalDataBuilder.putAll(historicalStore.getMap()); + + if (wasCreatedFromResources) { + pruneStore(historicalStore, version); + } + } + + private void pruneStore(PersistableNetworkPayloadStore historicalStore, String version) { + int preLive = getMapOfLiveData().keySet().size(); + getMapOfLiveData().keySet().removeAll(historicalStore.getMap().keySet()); + int postLive = getMapOfLiveData().size(); + if (preLive > postLive) { + log.info("We pruned data from our live data store which are already contained in the historical data store with version {}. " + + "The live map had {} entries before pruning and has {} entries afterwards.", + version, preLive, postLive); + } else { + log.info("No pruning from historical data store with version {} was applied", version); + } + storage.queueUpForSave(store); + } + + private boolean anyMapContainsKey(P2PDataStorage.ByteArray hash) { + return getMapOfLiveData().containsKey(hash) || allHistoricalPayloads.containsKey(hash); + } +} From 3df2f7e177edcafb437ee10ffcd84c7a5152edd6 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 15:06:34 -0500 Subject: [PATCH 11/29] Add HISTORY field. Make isNewVersion public --- common/src/main/java/bisq/common/app/Version.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/common/src/main/java/bisq/common/app/Version.java b/common/src/main/java/bisq/common/app/Version.java index 0e4d7ce8115..809e1e9bf0e 100644 --- a/common/src/main/java/bisq/common/app/Version.java +++ b/common/src/main/java/bisq/common/app/Version.java @@ -17,6 +17,9 @@ package bisq.common.app; +import java.util.Arrays; +import java.util.List; + import lombok.extern.slf4j.Slf4j; import static com.google.common.base.Preconditions.checkArgument; @@ -29,6 +32,11 @@ public class Version { // We use semantic versioning with major, minor and patch public static final String VERSION = "1.3.9"; + /** + * Holds a list of the versions of tagged resource files for optimizing the getData requests. + */ + public static final List HISTORY = Arrays.asList("1.4.0"); + public static int getMajorVersion(String version) { return getSubVersion(version, 0); } @@ -45,7 +53,7 @@ public static boolean isNewVersion(String newVersion) { return isNewVersion(newVersion, VERSION); } - static boolean isNewVersion(String newVersion, String currentVersion) { + public static boolean isNewVersion(String newVersion, String currentVersion) { if (newVersion.equals(currentVersion)) return false; else if (getMajorVersion(newVersion) > getMajorVersion(currentVersion)) From e44fdbdea247bb30143c22904e2cdabafdef4be7 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 15:06:57 -0500 Subject: [PATCH 12/29] Refactoring: Rename variable --- .../bisq/core/trade/statistics/TradeStatisticsManager.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java b/core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java index a92d8ae3c71..dfa64953000 100644 --- a/core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java +++ b/core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java @@ -79,7 +79,7 @@ public void onAllServicesInitialized() { addToSet((TradeStatistics2) payload); }); - Set collect = tradeStatistics2StorageService.getMap().values().stream() + Set set = tradeStatistics2StorageService.getMapOfAllData().values().stream() .filter(e -> e instanceof TradeStatistics2) .map(e -> (TradeStatistics2) e) .map(WrapperTradeStatistics2::new) @@ -87,7 +87,7 @@ public void onAllServicesInitialized() { .map(WrapperTradeStatistics2::unwrap) .filter(TradeStatistics2::isValid) .collect(Collectors.toSet()); - observableTradeStatisticsSet.addAll(collect); + observableTradeStatisticsSet.addAll(set); priceFeedService.applyLatestBisqMarketPrice(observableTradeStatisticsSet); @@ -99,7 +99,6 @@ public ObservableSet getObservableTradeStatisticsSet() { } private void addToSet(TradeStatistics2 tradeStatistics) { - if (!observableTradeStatisticsSet.contains(tradeStatistics)) { Optional duplicate = observableTradeStatisticsSet.stream().filter( e -> e.getOfferId().equals(tradeStatistics.getOfferId())).findAny(); From 62836d79f5e6b15c14546069106abaf57e813abf Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 15:08:43 -0500 Subject: [PATCH 13/29] Add support for HistoricalDataStoreService --- .../network/p2p/storage/P2PDataStorage.java | 112 ++++++++++++++---- 1 file changed, 86 insertions(+), 26 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 47317fa2b7e..b78fa10be19 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java @@ -47,6 +47,8 @@ import bisq.network.p2p.storage.payload.RequiresOwnerIsOnlinePayload; import bisq.network.p2p.storage.persistence.AppendOnlyDataStoreListener; import bisq.network.p2p.storage.persistence.AppendOnlyDataStoreService; +import bisq.network.p2p.storage.persistence.HistoricalDataStoreService; +import bisq.network.p2p.storage.persistence.PersistableNetworkPayloadStore; import bisq.network.p2p.storage.persistence.ProtectedDataStoreService; import bisq.network.p2p.storage.persistence.ResourceDataStoreService; import bisq.network.p2p.storage.persistence.SequenceNumberMap; @@ -195,14 +197,14 @@ public synchronized void readFromResources(String postFix) { * Returns a PreliminaryGetDataRequest that can be sent to a peer node to request missing Payload data. */ public PreliminaryGetDataRequest buildPreliminaryGetDataRequest(int nonce) { - return new PreliminaryGetDataRequest(nonce, this.getKnownPayloadHashes()); + return new PreliminaryGetDataRequest(nonce, getKnownPayloadHashes()); } /** * Returns a GetUpdatedDataRequest that can be sent to a peer node to request missing Payload data. */ public GetUpdatedDataRequest buildGetUpdatedDataRequest(NodeAddress senderNodeAddress, int nonce) { - return new GetUpdatedDataRequest(senderNodeAddress, nonce, this.getKnownPayloadHashes()); + return new GetUpdatedDataRequest(senderNodeAddress, nonce, getKnownPayloadHashes()); } /** @@ -213,44 +215,40 @@ private Set getKnownPayloadHashes() { // PersistedStoragePayload items don't get removed, so we don't have an issue with the case that // an object gets removed in between PreliminaryGetDataRequest and the GetUpdatedDataRequest and we would // miss that event if we do not load the full set or use some delta handling. - Set excludedKeys = this.appendOnlyDataStoreService.getMap().keySet().stream() - .map(e -> e.bytes) - .collect(Collectors.toSet()); - Set excludedKeysFromPersistedEntryMap = this.map.keySet() - .stream() - .map(e -> e.bytes) - .collect(Collectors.toSet()); + Set excludedKeys = getKeysAsByteSet(getMapForDataRequest()); + Set excludedKeysFromPersistedEntryMap = getKeysAsByteSet(map); excludedKeys.addAll(excludedKeysFromPersistedEntryMap); - return excludedKeys; } + /** * Generic function that can be used to filter a Map * by a given set of keys and peer capabilities. */ static private Set filterKnownHashes( - Map toFilter, - Function objToPayload, + Map mapToFilter, + Function objToPayloadFunction, Set knownHashes, Capabilities peerCapabilities, int maxEntries, - AtomicBoolean outTruncated) { + AtomicBoolean wasTruncated) { AtomicInteger limit = new AtomicInteger(maxEntries); - Set filteredResults = toFilter.entrySet().stream() + Set filteredResults = mapToFilter.entrySet().stream() .filter(e -> !knownHashes.contains(e.getKey())) .filter(e -> limit.decrementAndGet() >= 0) .map(Map.Entry::getValue) .filter(networkPayload -> shouldTransmitPayloadToPeer(peerCapabilities, - objToPayload.apply(networkPayload))) + objToPayloadFunction.apply(networkPayload))) .collect(Collectors.toSet()); - if (limit.get() < 0) - outTruncated.set(true); + if (limit.get() < 0) { + wasTruncated.set(true); + } return filteredResults; } @@ -261,30 +259,39 @@ static private Set filterKnownHashes( public GetDataResponse buildGetDataResponse( GetDataRequest getDataRequest, int maxEntriesPerType, - AtomicBoolean outPersistableNetworkPayloadOutputTruncated, - AtomicBoolean outProtectedStorageEntryOutputTruncated, + AtomicBoolean wasPersistableNetworkPayloadsTruncated, + AtomicBoolean wasProtectedStorageEntriesTruncated, Capabilities peerCapabilities) { Set excludedKeysAsByteArray = P2PDataStorage.ByteArray.convertBytesSetToByteArraySet(getDataRequest.getExcludedKeys()); + // Pre v 1.4.0 requests do not have set the requesters version field so it is null. + // The methods in HistoricalDataStoreService will return all historical data in that case. + // mapForDataResponse contains the filtered by version data from HistoricalDataStoreService as well as all other + // maps of the remaining appendOnlyDataStoreServices. + Map mapForDataResponse = getMapForDataResponse(getDataRequest.getVersion()); Set filteredPersistableNetworkPayloads = filterKnownHashes( - this.appendOnlyDataStoreService.getMap(), + mapForDataResponse, Function.identity(), excludedKeysAsByteArray, peerCapabilities, maxEntriesPerType, - outPersistableNetworkPayloadOutputTruncated); + wasPersistableNetworkPayloadsTruncated); + log.info("{} PersistableNetworkPayload entries remained after filtered by excluded keys. Original map had {} entries.", + filteredPersistableNetworkPayloads.size(), mapForDataResponse.size()); Set filteredProtectedStorageEntries = filterKnownHashes( - this.map, + map, ProtectedStorageEntry::getProtectedStoragePayload, excludedKeysAsByteArray, peerCapabilities, maxEntriesPerType, - outProtectedStorageEntryOutputTruncated); + wasProtectedStorageEntriesTruncated); + log.info("{} ProtectedStorageEntry entries remained after filtered by excluded keys. Original map had {} entries.", + filteredProtectedStorageEntries.size(), map.size()); return new GetDataResponse( filteredProtectedStorageEntries, @@ -293,6 +300,57 @@ public GetDataResponse buildGetDataResponse( getDataRequest instanceof GetUpdatedDataRequest); } + + /////////////////////////////////////////////////////////////////////////////////////////// + // Utils for collecting the exclude hashes + /////////////////////////////////////////////////////////////////////////////////////////// + + private Map getMapForDataRequest() { + Map map = new HashMap<>(); + appendOnlyDataStoreService.getServices() + .forEach(service -> { + Map serviceMap; + if (service instanceof HistoricalDataStoreService) { + var historicalDataStoreService = (HistoricalDataStoreService) service; + // As we add the version to our request we only use the live data. Eventually missing data will be + // derived from the version. + serviceMap = historicalDataStoreService.getMapOfLiveData(); + map.putAll(serviceMap); + } else { + serviceMap = service.getMap(); + map.putAll(serviceMap); + } + log.info("We added {} entries from {} to the excluded key set of our request", + serviceMap.size(), service.getClass().getSimpleName()); + }); + return map; + } + + private Map getMapForDataResponse(String requestersVersion) { + Map map = new HashMap<>(); + appendOnlyDataStoreService.getServices() + .forEach(service -> { + Map serviceMap; + if (service instanceof HistoricalDataStoreService) { + var historicalDataStoreService = (HistoricalDataStoreService) service; + serviceMap = historicalDataStoreService.getMapSinceVersion(requestersVersion); + map.putAll(serviceMap); + } else { + serviceMap = service.getMap(); + map.putAll(serviceMap); + } + log.info("We added {} entries from {} to be filtered by excluded keys", + serviceMap.size(), service.getClass().getSimpleName()); + }); + return map; + } + + private Set getKeysAsByteSet(Map map) { + return map.keySet().stream() + .map(e -> e.bytes) + .collect(Collectors.toSet()); + } + /** * Returns true if a Payload should be transmit to a peer given the peer's supported capabilities. */ @@ -402,6 +460,7 @@ public Map getAppendOnlyDataStoreMap() { return appendOnlyDataStoreService.getMap(); } + /////////////////////////////////////////////////////////////////////////////////////////// // MessageListener implementation /////////////////////////////////////////////////////////////////////////////////////////// @@ -579,10 +638,11 @@ private boolean addProtectedStorageEntry(ProtectedStorageEntry protectedStorageE // To avoid that expired data get stored and broadcast we check early for expire date. if (protectedStorageEntry.isExpired(clock)) { - log.warn("We received an expired protectedStorageEntry from peer {}", - sender != null ? sender.getFullAddress() : "sender is null"); + String peer = sender != null ? sender.getFullAddress() : "sender is null"; + log.warn("We received an expired protectedStorageEntry from peer {}. ProtectedStoragePayload={}", + peer, protectedStorageEntry.getProtectedStoragePayload().getClass().getSimpleName()); log.debug("Expired protectedStorageEntry from peer {}. getCreationTimeStamp={}, protectedStorageEntry={}", - sender != null ? sender.getFullAddress() : "sender is null", + peer, new Date(protectedStorageEntry.getCreationTimeStamp()), protectedStorageEntry); return false; From 58efb62b84b25e000c5a54383f105b9f518b72a3 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 15:12:15 -0500 Subject: [PATCH 14/29] Refactoring: Rearrange method (moved method) --- .../network/p2p/storage/P2PDataStorage.java | 59 +++++++++---------- 1 file changed, 29 insertions(+), 30 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 b78fa10be19..59cd65e13c5 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java @@ -223,36 +223,6 @@ private Set getKnownPayloadHashes() { return excludedKeys; } - - /** - * Generic function that can be used to filter a Map - * by a given set of keys and peer capabilities. - */ - static private Set filterKnownHashes( - Map mapToFilter, - Function objToPayloadFunction, - Set knownHashes, - Capabilities peerCapabilities, - int maxEntries, - AtomicBoolean wasTruncated) { - - AtomicInteger limit = new AtomicInteger(maxEntries); - - Set filteredResults = mapToFilter.entrySet().stream() - .filter(e -> !knownHashes.contains(e.getKey())) - .filter(e -> limit.decrementAndGet() >= 0) - .map(Map.Entry::getValue) - .filter(networkPayload -> shouldTransmitPayloadToPeer(peerCapabilities, - objToPayloadFunction.apply(networkPayload))) - .collect(Collectors.toSet()); - - if (limit.get() < 0) { - wasTruncated.set(true); - } - - return filteredResults; - } - /** * Returns a GetDataResponse object that contains the Payloads known locally, but not remotely. */ @@ -345,6 +315,35 @@ private Map getMapForDataResponse(String r return map; } + /** + * Generic function that can be used to filter a Map + * by a given set of keys and peer capabilities. + */ + static private Set filterKnownHashes( + Map mapToFilter, + Function objToPayloadFunction, + Set knownHashes, + Capabilities peerCapabilities, + int maxEntries, + AtomicBoolean wasTruncated) { + + AtomicInteger limit = new AtomicInteger(maxEntries); + + Set filteredResults = mapToFilter.entrySet().stream() + .filter(e -> !knownHashes.contains(e.getKey())) + .filter(e -> limit.decrementAndGet() >= 0) + .map(Map.Entry::getValue) + .filter(networkPayload -> shouldTransmitPayloadToPeer(peerCapabilities, + objToPayloadFunction.apply(networkPayload))) + .collect(Collectors.toSet()); + + if (limit.get() < 0) { + wasTruncated.set(true); + } + + return filteredResults; + } + private Set getKeysAsByteSet(Map map) { return map.keySet().stream() .map(e -> e.bytes) From 65de106df057ecd17689099c13392b044a1c2a8a Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 15:13:11 -0500 Subject: [PATCH 15/29] Use HistoricalDataStoreService for TradeStatistics2StorageService --- .../TradeStatistics2StorageService.java | 20 +++---------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2StorageService.java b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2StorageService.java index c732a077f2c..92f5719d22c 100644 --- a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2StorageService.java +++ b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2StorageService.java @@ -17,25 +17,21 @@ package bisq.core.trade.statistics; -import bisq.network.p2p.storage.P2PDataStorage; import bisq.network.p2p.storage.payload.PersistableNetworkPayload; -import bisq.network.p2p.storage.persistence.MapStoreService; +import bisq.network.p2p.storage.persistence.HistoricalDataStoreService; import bisq.common.config.Config; import bisq.common.storage.Storage; -import javax.inject.Named; - import javax.inject.Inject; +import javax.inject.Named; import java.io.File; -import java.util.Map; - import lombok.extern.slf4j.Slf4j; @Slf4j -public class TradeStatistics2StorageService extends MapStoreService { +public class TradeStatistics2StorageService extends HistoricalDataStoreService { private static final String FILE_NAME = "TradeStatistics2Store"; @@ -59,11 +55,6 @@ public String getFileName() { return FILE_NAME; } - @Override - public Map getMap() { - return store.getMap(); - } - @Override public boolean canHandle(PersistableNetworkPayload payload) { return payload instanceof TradeStatistics2; @@ -78,9 +69,4 @@ public boolean canHandle(PersistableNetworkPayload payload) { protected TradeStatistics2Store createStore() { return new TradeStatistics2Store(); } - - @Override - protected void readStore() { - super.readStore(); - } } From 8ea6da01da1623c67b863df1f32e8881d377e56d Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 15:13:31 -0500 Subject: [PATCH 16/29] Apply changes to test classes --- .../P2PDataStorageBuildGetDataResponseTest.java | 13 +++++++------ .../p2p/storage/P2PDataStorageRequestDataTest.java | 8 +++++--- .../mocks/AppendOnlyDataStoreServiceFake.java | 8 +++----- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageBuildGetDataResponseTest.java b/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageBuildGetDataResponseTest.java index 5a65f435c45..ef11f5e07f6 100644 --- a/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageBuildGetDataResponseTest.java +++ b/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageBuildGetDataResponseTest.java @@ -44,6 +44,9 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -53,11 +56,6 @@ import static org.mockito.Mockito.when; import static org.mockito.Mockito.withSettings; - - -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; - public class P2PDataStorageBuildGetDataResponseTest { abstract static class P2PDataStorageBuildGetDataResponseTestBase { // GIVEN null & non-null supportedCapabilities @@ -240,7 +238,10 @@ public void buildGetDataResponse_unknownPNPSendBackTruncation() { Assert.assertEquals(getDataRequest instanceof GetUpdatedDataRequest, getDataResponse.isGetUpdatedDataResponse()); Assert.assertEquals(getDataResponse.getSupportedCapabilities(), Capabilities.app); Assert.assertEquals(1, getDataResponse.getPersistableNetworkPayloadSet().size()); - Assert.assertTrue(getDataResponse.getPersistableNetworkPayloadSet().contains(onlyLocal1)); + Set persistableNetworkPayloadSet = getDataResponse.getPersistableNetworkPayloadSet(); + + // We use a set at the filter so it is not deterministic which item get truncated + Assert.assertEquals(1, persistableNetworkPayloadSet.size()); Assert.assertTrue(getDataResponse.getDataSet().isEmpty()); } diff --git a/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageRequestDataTest.java b/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageRequestDataTest.java index 27e522daaad..c8976913719 100644 --- a/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageRequestDataTest.java +++ b/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageRequestDataTest.java @@ -35,13 +35,15 @@ import java.util.Set; +import org.mockito.MockitoAnnotations; + import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import org.mockito.MockitoAnnotations; - -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class P2PDataStorageRequestDataTest { private TestState testState; diff --git a/p2p/src/test/java/bisq/network/p2p/storage/mocks/AppendOnlyDataStoreServiceFake.java b/p2p/src/test/java/bisq/network/p2p/storage/mocks/AppendOnlyDataStoreServiceFake.java index 155e4708be7..00d893b8f2c 100644 --- a/p2p/src/test/java/bisq/network/p2p/storage/mocks/AppendOnlyDataStoreServiceFake.java +++ b/p2p/src/test/java/bisq/network/p2p/storage/mocks/AppendOnlyDataStoreServiceFake.java @@ -21,7 +21,6 @@ import bisq.network.p2p.storage.payload.PersistableNetworkPayload; import bisq.network.p2p.storage.persistence.AppendOnlyDataStoreService; -import java.util.HashMap; import java.util.Map; /** @@ -31,17 +30,16 @@ * @see Reference */ public class AppendOnlyDataStoreServiceFake extends AppendOnlyDataStoreService { - private final Map map; public AppendOnlyDataStoreServiceFake() { - map = new HashMap<>(); + addService(new MapStoreServiceFake()); } public Map getMap() { - return map; + return super.getMap(); } public void put(P2PDataStorage.ByteArray hashAsByteArray, PersistableNetworkPayload payload) { - map.put(hashAsByteArray, payload); + super.put(hashAsByteArray, payload); } } From 6f5bfde92fd2904dda9566bb2f73a97263c10327 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 15:20:32 -0500 Subject: [PATCH 17/29] Deactivate usage of HistoricalDataStoreService for now. This is intended to be able to merge the code base before https://github.com/bisq-network/bisq/pull/4577 is merged. We can add the code base but not using the feature yet. Once we are ready for deployment we can revert this commit and have the feature activated. --- .../TradeStatistics2StorageService.java | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2StorageService.java b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2StorageService.java index 92f5719d22c..b5a9699e134 100644 --- a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2StorageService.java +++ b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2StorageService.java @@ -17,8 +17,9 @@ package bisq.core.trade.statistics; +import bisq.network.p2p.storage.P2PDataStorage; import bisq.network.p2p.storage.payload.PersistableNetworkPayload; -import bisq.network.p2p.storage.persistence.HistoricalDataStoreService; +import bisq.network.p2p.storage.persistence.MapStoreService; import bisq.common.config.Config; import bisq.common.storage.Storage; @@ -28,10 +29,12 @@ import java.io.File; +import java.util.Map; + import lombok.extern.slf4j.Slf4j; @Slf4j -public class TradeStatistics2StorageService extends HistoricalDataStoreService { +public class TradeStatistics2StorageService extends MapStoreService { private static final String FILE_NAME = "TradeStatistics2Store"; @@ -55,6 +58,15 @@ public String getFileName() { return FILE_NAME; } + @Override + public Map getMap() { + return store.getMap(); + } + + public Map getMapOfAllData() { + return getMap(); + } + @Override public boolean canHandle(PersistableNetworkPayload payload) { return payload instanceof TradeStatistics2; From a1debd807087f47152c4f7588dbba459cbd15eb9 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 16:33:21 -0500 Subject: [PATCH 18/29] Fix grammar --- .../java/bisq/network/p2p/storage/persistence/StoreService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/src/main/java/bisq/network/p2p/storage/persistence/StoreService.java b/p2p/src/main/java/bisq/network/p2p/storage/persistence/StoreService.java index 625123184d0..3cbbb036538 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/persistence/StoreService.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/persistence/StoreService.java @@ -116,7 +116,7 @@ protected boolean makeFileFromResourceFile(String fileName, String postFix) { e.printStackTrace(); } } else { - log.info("No resource file have been copied. {} exists already.", fileName); + log.info("No resource file was copied. {} exists already.", fileName); } return false; } From 5027c861e3558014cfea5ed28e93c6de1d375e01 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 16:33:33 -0500 Subject: [PATCH 19/29] Fix proto field index --- proto/src/main/proto/pb.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/src/main/proto/pb.proto b/proto/src/main/proto/pb.proto index 4c858b18a97..1516c62f3c4 100644 --- a/proto/src/main/proto/pb.proto +++ b/proto/src/main/proto/pb.proto @@ -92,7 +92,7 @@ message BundleOfEnvelopes { // get data message PreliminaryGetDataRequest { - int32 nonce = 21; + int32 nonce = 1; repeated bytes excluded_keys = 2; repeated int32 supported_capabilities = 3; string version = 4; From 3ee60d5cdcf71ff893709de305f64c67e967aee4 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 16:35:41 -0500 Subject: [PATCH 20/29] Add license --- .../persistence/HistoricalDataStoreService.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java b/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java index e0b8f3078d6..b087325a8f1 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java @@ -1,3 +1,20 @@ +/* + * 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 . + */ + package bisq.network.p2p.storage.persistence; import bisq.network.p2p.storage.P2PDataStorage; From c308791321827a2cebc19b04e5582d0197a887f9 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 1 Oct 2020 16:36:12 -0500 Subject: [PATCH 21/29] Add license --- .../PersistableNetworkPayloadStore.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadStore.java b/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadStore.java index 751e0b63411..a617faad083 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadStore.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadStore.java @@ -1,3 +1,20 @@ +/* + * 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 . + */ + package bisq.network.p2p.storage.persistence; import bisq.network.p2p.storage.P2PDataStorage; From ef7f5a7f635445519b8ef85fcd5de7a0efc73738 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Fri, 2 Oct 2020 13:13:45 -0500 Subject: [PATCH 22/29] Revert nonce index at PreliminaryGetDataRequest protobuf entry to 21 as it was the version used in master as well. We cannot change that without breaking compatibility. --- proto/src/main/proto/pb.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/src/main/proto/pb.proto b/proto/src/main/proto/pb.proto index 1516c62f3c4..2dd32a983c5 100644 --- a/proto/src/main/proto/pb.proto +++ b/proto/src/main/proto/pb.proto @@ -92,7 +92,7 @@ message BundleOfEnvelopes { // get data message PreliminaryGetDataRequest { - int32 nonce = 1; + int32 nonce = 21; // This was set to 21 instead of 1 in some old commit so we cannot change it. repeated bytes excluded_keys = 2; repeated int32 supported_capabilities = 3; string version = 4; From d3384e66e5f92fffb50218503d27357884239417 Mon Sep 17 00:00:00 2001 From: chimp1984 <54558767+chimp1984@users.noreply.github.com> Date: Fri, 2 Oct 2020 13:15:51 -0500 Subject: [PATCH 23/29] Update p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java Co-authored-by: sqrrm --- .../p2p/storage/persistence/HistoricalDataStoreService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java b/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java index b087325a8f1..4880eae32a5 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java @@ -73,7 +73,7 @@ public Map getMapSinceVersi return true; } - // Otherwise we only add data if the requesters version is older then + // Otherwise we only add data if the requesters version is older than // the version of the particular store. String storeVersion = entry.getKey(); boolean newVersion = Version.isNewVersion(storeVersion, requestersVersion); From 9d517140b28606cc212052521e26c6f5753fc249 Mon Sep 17 00:00:00 2001 From: chimp1984 <54558767+chimp1984@users.noreply.github.com> Date: Fri, 2 Oct 2020 13:16:04 -0500 Subject: [PATCH 24/29] Update p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java Co-authored-by: sqrrm --- .../p2p/storage/persistence/HistoricalDataStoreService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java b/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java index 4880eae32a5..90b878744c8 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java @@ -79,7 +79,7 @@ public Map getMapSinceVersi boolean newVersion = Version.isNewVersion(storeVersion, requestersVersion); String details = newVersion ? "As our historical store is a newer version we add the data to our result map." : - "As the requester version is not older as our historical store we do not " + + "As the requester version is not older than our historical store we do not " + "add the data to the result map."; log.info("The requester had version {}. Our historical data store has version {}.\n{}", requestersVersion, storeVersion, details); From 35d13fb018f0652d184b387b6eab32b1076f87ba Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Fri, 2 Oct 2020 13:22:25 -0500 Subject: [PATCH 25/29] Make putIfAbsent method more clear --- .../persistence/HistoricalDataStoreService.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java b/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java index 90b878744c8..c12f24f1472 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java @@ -73,13 +73,13 @@ public Map getMapSinceVersi return true; } - // Otherwise we only add data if the requesters version is older than + // Otherwise we only add data if the requesters version is older then // the version of the particular store. String storeVersion = entry.getKey(); boolean newVersion = Version.isNewVersion(storeVersion, requestersVersion); String details = newVersion ? "As our historical store is a newer version we add the data to our result map." : - "As the requester version is not older than our historical store we do not " + + "As the requester version is not older as our historical store we do not " + "add the data to the result map."; log.info("The requester had version {}. Our historical data store has version {}.\n{}", requestersVersion, storeVersion, details); @@ -135,9 +135,12 @@ protected PersistableNetworkPayload putIfAbsent(P2PDataStorage.ByteArray hash, P return null; } - PersistableNetworkPayload previous = getMapOfLiveData().put(hash, payload); + // We do not return the value from getMapOfLiveData().put as we checked before that it does not contain any value. + // So it will be always null. We still keep the return type as we override the method from MapStoreService which + // follow the Map.putIfAbsent signature. + getMapOfLiveData().put(hash, payload); persist(); - return previous; + return null; } From 611bcef46111fba823c5226aebed97bb0216ec52 Mon Sep 17 00:00:00 2001 From: chimp1984 <54558767+chimp1984@users.noreply.github.com> Date: Fri, 2 Oct 2020 13:23:12 -0500 Subject: [PATCH 26/29] Update p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java Co-authored-by: sqrrm --- .../p2p/storage/persistence/HistoricalDataStoreService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java b/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java index c12f24f1472..5f1346f20cf 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/persistence/HistoricalDataStoreService.java @@ -188,7 +188,7 @@ private void readHistoricalStoreFromResources(String version, } private void pruneStore(PersistableNetworkPayloadStore historicalStore, String version) { - int preLive = getMapOfLiveData().keySet().size(); + int preLive = getMapOfLiveData().size(); getMapOfLiveData().keySet().removeAll(historicalStore.getMap().keySet()); int postLive = getMapOfLiveData().size(); if (preLive > postLive) { From e9c57b1a4b4ac34c203f8860f23269dd138da835 Mon Sep 17 00:00:00 2001 From: chimp1984 <54558767+chimp1984@users.noreply.github.com> Date: Fri, 2 Oct 2020 13:25:51 -0500 Subject: [PATCH 27/29] Update p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java Co-authored-by: sqrrm --- p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java | 3 +-- 1 file changed, 1 insertion(+), 2 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 59cd65e13c5..f695eafa683 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java @@ -285,11 +285,10 @@ private Map getMapForDataRequest() { // As we add the version to our request we only use the live data. Eventually missing data will be // derived from the version. serviceMap = historicalDataStoreService.getMapOfLiveData(); - map.putAll(serviceMap); } else { serviceMap = service.getMap(); - map.putAll(serviceMap); } + map.putAll(serviceMap); log.info("We added {} entries from {} to the excluded key set of our request", serviceMap.size(), service.getClass().getSimpleName()); }); From 308aa162e8e652502843308a75a37a08d1c8cc6d Mon Sep 17 00:00:00 2001 From: chimp1984 <54558767+chimp1984@users.noreply.github.com> Date: Fri, 2 Oct 2020 13:26:11 -0500 Subject: [PATCH 28/29] Update p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java Co-authored-by: sqrrm --- p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java | 3 +-- 1 file changed, 1 insertion(+), 2 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 f695eafa683..8540000f61f 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java @@ -303,11 +303,10 @@ private Map getMapForDataResponse(String r if (service instanceof HistoricalDataStoreService) { var historicalDataStoreService = (HistoricalDataStoreService) service; serviceMap = historicalDataStoreService.getMapSinceVersion(requestersVersion); - map.putAll(serviceMap); } else { serviceMap = service.getMap(); - map.putAll(serviceMap); } + map.putAll(serviceMap); log.info("We added {} entries from {} to be filtered by excluded keys", serviceMap.size(), service.getClass().getSimpleName()); }); From 807b9fc5be2c6ed852534f35fec86c3e3a7c18c0 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Fri, 2 Oct 2020 13:29:35 -0500 Subject: [PATCH 29/29] Remove unused method, fix typo, remove unneeded annotations --- .../network/p2p/storage/P2PDataStorage.java | 35 +------------------ 1 file changed, 1 insertion(+), 34 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 8540000f61f..6643c37ae7f 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java @@ -681,7 +681,7 @@ private boolean addProtectedStorageEntry(ProtectedStorageEntry protectedStorageE if (allowBroadcast) broadcaster.broadcast(new AddDataMessage(protectedStorageEntry), sender, listener); - // Persist ProtectedStorageEntrys carrying PersistablePayload payloads + // Persist ProtectedStorageEntries carrying PersistablePayload payloads if (protectedStoragePayload instanceof PersistablePayload) protectedDataStoreService.put(hashOfPayload, protectedStorageEntry); @@ -790,37 +790,6 @@ public boolean remove(ProtectedStorageEntry protectedStorageEntry, return true; } - - /** - * This method must be called only from client code not from network messages! We omit the ownership checks - * so we must apply it only if it comes from our trusted application code. It is used from client code which detects - * that the domain object violates specific domain rules. - * We could make it more generic by adding an Interface with a generic validation method. - * - * @param protectedStorageEntry The entry to be removed - */ - public void removeInvalidProtectedStorageEntry(ProtectedStorageEntry protectedStorageEntry) { - log.warn("We remove an invalid protectedStorageEntry: {}", protectedStorageEntry); - ProtectedStoragePayload protectedStoragePayload = protectedStorageEntry.getProtectedStoragePayload(); - ByteArray hashOfPayload = get32ByteHashAsByteArray(protectedStoragePayload); - - if (!map.containsKey(hashOfPayload)) { - return; - } - - removeFromMapAndDataStore(protectedStorageEntry, hashOfPayload); - - // We do not update the sequence number as that method is only called if we have received an invalid - // protectedStorageEntry from a previous add operation. - - // We do not call maybeAddToRemoveAddOncePayloads to avoid that an invalid object might block a valid object - // which we might receive in future (could be potential attack). - - // We do not broadcast as this is a local operation only to avoid our maps get polluted with invalid objects - // and as we do not check for ownership a node would not accept such a procedure if it would come from untrusted - // source (network). - } - public ProtectedStorageEntry getProtectedStorageEntry(ProtectedStoragePayload protectedStoragePayload, KeyPair ownerStoragePubKey) throws CryptoException { @@ -880,7 +849,6 @@ public void addAppendOnlyDataStoreListener(AppendOnlyDataStoreListener listener) appendOnlyDataStoreListeners.add(listener); } - @SuppressWarnings("unused") public void removeAppendOnlyDataStoreListener(AppendOnlyDataStoreListener listener) { appendOnlyDataStoreListeners.remove(listener); } @@ -1075,7 +1043,6 @@ public static ByteArray fromProto(protobuf.ByteArray proto) { // Util /////////////////////////////////////////////////////////////////////////////////////////// - @SuppressWarnings("unused") public String getHex() { return Utilities.encodeToHex(bytes); }