From 18f75869a2cfa4a66ee243648e8881591f4aff36 Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Tue, 3 Mar 2020 21:40:31 +0800 Subject: [PATCH 1/5] Make 'fromProto' static methods return most specific type Minor change for consistency: narrow the signature of some remaining such methods, which have return type 'PersistableEnvelope'. (This excludes some other cases with return type 'NetworkEnvelope'.) --- .../bisq/common/proto/persistable/NavigationPath.java | 2 +- .../java/bisq/core/account/sign/SignedWitnessStore.java | 2 +- .../bisq/core/account/witness/AccountAgeWitnessStore.java | 2 +- .../core/dao/governance/blindvote/MyBlindVoteList.java | 3 +-- .../dao/governance/blindvote/storage/BlindVoteStore.java | 2 +- .../java/bisq/core/dao/governance/myvote/MyVoteList.java | 5 ++--- .../proposal/storage/appendonly/ProposalStore.java | 2 +- .../proposal/storage/temp/TempProposalStore.java | 2 +- core/src/main/java/bisq/core/dao/state/DaoStateStore.java | 2 +- .../state/unconfirmed/UnconfirmedBsqChangeOutputList.java | 3 +-- .../main/java/bisq/core/payment/PaymentAccountList.java | 3 +-- .../bisq/core/trade/statistics/TradeStatistics2Store.java | 2 +- core/src/main/java/bisq/core/user/PreferencesPayload.java | 5 ++--- .../bisq/network/p2p/peers/peerexchange/PeerList.java | 3 +-- .../persistence/PersistableNetworkPayloadList.java | 8 ++++---- 15 files changed, 20 insertions(+), 26 deletions(-) diff --git a/common/src/main/java/bisq/common/proto/persistable/NavigationPath.java b/common/src/main/java/bisq/common/proto/persistable/NavigationPath.java index a1cc7566874..998498deb70 100644 --- a/common/src/main/java/bisq/common/proto/persistable/NavigationPath.java +++ b/common/src/main/java/bisq/common/proto/persistable/NavigationPath.java @@ -45,7 +45,7 @@ public Message toProtoMessage() { return protobuf.PersistableEnvelope.newBuilder().setNavigationPath(builder).build(); } - public static PersistableEnvelope fromProto(protobuf.NavigationPath proto) { + public static NavigationPath fromProto(protobuf.NavigationPath proto) { return new NavigationPath(new ArrayList<>(proto.getPathList())); } } 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 1f05e66ff59..cedd570edcc 100644 --- a/core/src/main/java/bisq/core/account/sign/SignedWitnessStore.java +++ b/core/src/main/java/bisq/core/account/sign/SignedWitnessStore.java @@ -70,7 +70,7 @@ private protobuf.SignedWitnessStore.Builder getBuilder() { return protobuf.SignedWitnessStore.newBuilder().addAllItems(protoList); } - public static PersistableEnvelope fromProto(protobuf.SignedWitnessStore proto) { + public static SignedWitnessStore fromProto(protobuf.SignedWitnessStore proto) { List list = proto.getItemsList().stream() .map(SignedWitness::fromProto).collect(Collectors.toList()); return new SignedWitnessStore(list); 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 e25f73e28b8..24146fe4610 100644 --- a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessStore.java +++ b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessStore.java @@ -69,7 +69,7 @@ private protobuf.AccountAgeWitnessStore.Builder getBuilder() { return protobuf.AccountAgeWitnessStore.newBuilder().addAllItems(protoList); } - public static PersistableEnvelope fromProto(protobuf.AccountAgeWitnessStore proto) { + public static AccountAgeWitnessStore fromProto(protobuf.AccountAgeWitnessStore proto) { List list = proto.getItemsList().stream() .map(AccountAgeWitness::fromProto).collect(Collectors.toList()); return new AccountAgeWitnessStore(list); diff --git a/core/src/main/java/bisq/core/dao/governance/blindvote/MyBlindVoteList.java b/core/src/main/java/bisq/core/dao/governance/blindvote/MyBlindVoteList.java index 47dca594eb0..be0890b653a 100644 --- a/core/src/main/java/bisq/core/dao/governance/blindvote/MyBlindVoteList.java +++ b/core/src/main/java/bisq/core/dao/governance/blindvote/MyBlindVoteList.java @@ -19,7 +19,6 @@ import bisq.core.dao.governance.ConsensusCritical; -import bisq.common.proto.persistable.PersistableEnvelope; import bisq.common.proto.persistable.PersistableList; import com.google.protobuf.Message; @@ -59,7 +58,7 @@ public Message toProtoMessage() { .build(); } - public static PersistableEnvelope fromProto(protobuf.MyBlindVoteList proto) { + public static MyBlindVoteList fromProto(protobuf.MyBlindVoteList proto) { return new MyBlindVoteList(new ArrayList<>(proto.getBlindVoteList().stream() .map(BlindVote::fromProto) .collect(Collectors.toList()))); 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 e7f11038065..7c7f9c05131 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 @@ -69,7 +69,7 @@ private protobuf.BlindVoteStore.Builder getBuilder() { return protobuf.BlindVoteStore.newBuilder().addAllItems(protoList); } - public static PersistableEnvelope fromProto(protobuf.BlindVoteStore proto) { + public static BlindVoteStore fromProto(protobuf.BlindVoteStore proto) { List list = proto.getItemsList().stream() .map(BlindVotePayload::fromProto).collect(Collectors.toList()); return new BlindVoteStore(list); diff --git a/core/src/main/java/bisq/core/dao/governance/myvote/MyVoteList.java b/core/src/main/java/bisq/core/dao/governance/myvote/MyVoteList.java index ef66f624c99..06eb206a113 100644 --- a/core/src/main/java/bisq/core/dao/governance/myvote/MyVoteList.java +++ b/core/src/main/java/bisq/core/dao/governance/myvote/MyVoteList.java @@ -17,7 +17,6 @@ package bisq.core.dao.governance.myvote; -import bisq.common.proto.persistable.PersistableEnvelope; import bisq.common.proto.persistable.PersistableList; import com.google.protobuf.Message; @@ -31,7 +30,7 @@ @EqualsAndHashCode(callSuper = true) public class MyVoteList extends PersistableList { - public MyVoteList() { + MyVoteList() { super(); } @@ -53,7 +52,7 @@ public Message toProtoMessage() { .build(); } - public static PersistableEnvelope fromProto(protobuf.MyVoteList proto) { + public static MyVoteList fromProto(protobuf.MyVoteList proto) { return new MyVoteList(new ArrayList<>(proto.getMyVoteList().stream() .map(MyVote::fromProto) .collect(Collectors.toList()))); 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 cd18dc8274b..9d425a92eb1 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 @@ -69,7 +69,7 @@ private protobuf.ProposalStore.Builder getBuilder() { return protobuf.ProposalStore.newBuilder().addAllItems(protoList); } - public static PersistableEnvelope fromProto(protobuf.ProposalStore proto) { + public static ProposalStore fromProto(protobuf.ProposalStore proto) { List list = proto.getItemsList().stream() .map(ProposalPayload::fromProto).collect(Collectors.toList()); return new ProposalStore(list); diff --git a/core/src/main/java/bisq/core/dao/governance/proposal/storage/temp/TempProposalStore.java b/core/src/main/java/bisq/core/dao/governance/proposal/storage/temp/TempProposalStore.java index 169ba028a6b..8bc112b7e93 100644 --- a/core/src/main/java/bisq/core/dao/governance/proposal/storage/temp/TempProposalStore.java +++ b/core/src/main/java/bisq/core/dao/governance/proposal/storage/temp/TempProposalStore.java @@ -72,7 +72,7 @@ private protobuf.TempProposalStore.Builder getBuilder() { return protobuf.TempProposalStore.newBuilder().addAllItems(protoList); } - public static PersistableEnvelope fromProto(protobuf.TempProposalStore proto, NetworkProtoResolver networkProtoResolver) { + public static TempProposalStore fromProto(protobuf.TempProposalStore proto, NetworkProtoResolver networkProtoResolver) { List list = proto.getItemsList().stream() .map(entry -> ProtectedStorageEntry.fromProto(entry, networkProtoResolver)) .collect(Collectors.toList()); diff --git a/core/src/main/java/bisq/core/dao/state/DaoStateStore.java b/core/src/main/java/bisq/core/dao/state/DaoStateStore.java index a0a28e66004..b089d0a59ab 100644 --- a/core/src/main/java/bisq/core/dao/state/DaoStateStore.java +++ b/core/src/main/java/bisq/core/dao/state/DaoStateStore.java @@ -67,7 +67,7 @@ public Message toProtoMessage() { .build(); } - public static PersistableEnvelope fromProto(protobuf.DaoStateStore proto) { + public static DaoStateStore fromProto(protobuf.DaoStateStore proto) { LinkedList daoStateHashList = proto.getDaoStateHashList().isEmpty() ? new LinkedList<>() : new LinkedList<>(proto.getDaoStateHashList().stream() diff --git a/core/src/main/java/bisq/core/dao/state/unconfirmed/UnconfirmedBsqChangeOutputList.java b/core/src/main/java/bisq/core/dao/state/unconfirmed/UnconfirmedBsqChangeOutputList.java index 141bc1a853a..2deae256979 100644 --- a/core/src/main/java/bisq/core/dao/state/unconfirmed/UnconfirmedBsqChangeOutputList.java +++ b/core/src/main/java/bisq/core/dao/state/unconfirmed/UnconfirmedBsqChangeOutputList.java @@ -17,7 +17,6 @@ package bisq.core.dao.state.unconfirmed; -import bisq.common.proto.persistable.PersistableEnvelope; import bisq.common.proto.persistable.PersistableList; import com.google.protobuf.Message; @@ -52,7 +51,7 @@ public Message toProtoMessage() { .build(); } - public static PersistableEnvelope fromProto(protobuf.UnconfirmedBsqChangeOutputList proto) { + public static UnconfirmedBsqChangeOutputList fromProto(protobuf.UnconfirmedBsqChangeOutputList proto) { return new UnconfirmedBsqChangeOutputList(new ArrayList<>(proto.getUnconfirmedTxOutputList().stream() .map(UnconfirmedTxOutput::fromProto) .collect(Collectors.toList()))); diff --git a/core/src/main/java/bisq/core/payment/PaymentAccountList.java b/core/src/main/java/bisq/core/payment/PaymentAccountList.java index edd21695389..a35801705ce 100644 --- a/core/src/main/java/bisq/core/payment/PaymentAccountList.java +++ b/core/src/main/java/bisq/core/payment/PaymentAccountList.java @@ -19,7 +19,6 @@ import bisq.core.proto.CoreProtoResolver; -import bisq.common.proto.persistable.PersistableEnvelope; import bisq.common.proto.persistable.PersistableList; import com.google.protobuf.Message; @@ -45,7 +44,7 @@ public Message toProtoMessage() { .build(); } - public static PersistableEnvelope fromProto(protobuf.PaymentAccountList proto, CoreProtoResolver coreProtoResolver) { + public static PaymentAccountList fromProto(protobuf.PaymentAccountList proto, CoreProtoResolver coreProtoResolver) { return new PaymentAccountList(new ArrayList<>(proto.getPaymentAccountList().stream() .map(e -> PaymentAccount.fromProto(e, coreProtoResolver)) .collect(Collectors.toList()))); 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 f84be42cf26..e1a9ae979d6 100644 --- a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2Store.java +++ b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2Store.java @@ -68,7 +68,7 @@ private protobuf.TradeStatistics2Store.Builder getBuilder() { return protobuf.TradeStatistics2Store.newBuilder().addAllItems(protoList); } - public static PersistableEnvelope fromProto(protobuf.TradeStatistics2Store proto) { + public static TradeStatistics2Store fromProto(protobuf.TradeStatistics2Store proto) { List list = proto.getItemsList().stream() .map(TradeStatistics2::fromProto).collect(Collectors.toList()); return new TradeStatistics2Store(list); diff --git a/core/src/main/java/bisq/core/user/PreferencesPayload.java b/core/src/main/java/bisq/core/user/PreferencesPayload.java index 15b4bbe4665..fa04855db68 100644 --- a/core/src/main/java/bisq/core/user/PreferencesPayload.java +++ b/core/src/main/java/bisq/core/user/PreferencesPayload.java @@ -132,7 +132,7 @@ public final class PreferencesPayload implements PersistableEnvelope { // Constructor /////////////////////////////////////////////////////////////////////////////////////////// - public PreferencesPayload() { + PreferencesPayload() { } @@ -207,7 +207,7 @@ public Message toProtoMessage() { return protobuf.PersistableEnvelope.newBuilder().setPreferencesPayload(builder).build(); } - public static PersistableEnvelope fromProto(protobuf.PreferencesPayload proto, CoreProtoResolver coreProtoResolver) { + public static PreferencesPayload fromProto(protobuf.PreferencesPayload proto, CoreProtoResolver coreProtoResolver) { final protobuf.Country userCountry = proto.getUserCountry(); PaymentAccount paymentAccount = null; if (proto.hasSelectedPaymentAccountForCreateOffer() && proto.getSelectedPaymentAccountForCreateOffer().hasPaymentMethod()) @@ -275,6 +275,5 @@ public static PersistableEnvelope fromProto(protobuf.PreferencesPayload proto, C proto.getBuyerSecurityDepositAsPercentForCrypto(), proto.getBlockNotifyPort(), proto.getTacAcceptedV120()); - } } diff --git a/p2p/src/main/java/bisq/network/p2p/peers/peerexchange/PeerList.java b/p2p/src/main/java/bisq/network/p2p/peers/peerexchange/PeerList.java index 03acd3f9307..64e2352ecfe 100644 --- a/p2p/src/main/java/bisq/network/p2p/peers/peerexchange/PeerList.java +++ b/p2p/src/main/java/bisq/network/p2p/peers/peerexchange/PeerList.java @@ -17,7 +17,6 @@ package bisq.network.p2p.peers.peerexchange; -import bisq.common.proto.persistable.PersistableEnvelope; import bisq.common.proto.persistable.PersistableList; import com.google.protobuf.Message; @@ -43,7 +42,7 @@ public Message toProtoMessage() { .build(); } - public static PersistableEnvelope fromProto(protobuf.PeerList proto) { + public static PeerList fromProto(protobuf.PeerList proto) { return new PeerList(new ArrayList<>(proto.getPeerList().stream() .map(Peer::fromProto) .collect(Collectors.toList()))); diff --git a/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadList.java b/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadList.java index e3e74aa4797..fea9e360541 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadList.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadList.java @@ -46,7 +46,7 @@ public class PersistableNetworkPayloadList implements PersistableEnvelope { @Getter private Map map = new ConcurrentHashMap<>(); - public PersistableNetworkPayloadList() { + PersistableNetworkPayloadList() { } @@ -54,7 +54,7 @@ public PersistableNetworkPayloadList() { // PROTO BUFFER /////////////////////////////////////////////////////////////////////////////////////////// - public PersistableNetworkPayloadList(Map map) { + private PersistableNetworkPayloadList(Map map) { this.map.putAll(map); } @@ -69,8 +69,8 @@ public Message toProtoMessage() { .build(); } - public static PersistableEnvelope fromProto(protobuf.PersistableNetworkPayloadList proto, - PersistenceProtoResolver resolver) { + public static PersistableNetworkPayloadList fromProto(protobuf.PersistableNetworkPayloadList proto, + PersistenceProtoResolver resolver) { Map map = new HashMap<>(); proto.getItemsList() .forEach(e -> { From 6487f92d84257d80b4b6795c53ab7380378f7e18 Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Wed, 4 Mar 2020 14:49:45 +0800 Subject: [PATCH 2/5] Make serialisation in FileManager::saveToFile thread-safe Add toProtoMessageSynchronized() default method to PersistableEnvelope, which performs (blocking) protobuf serialisation in the user thread, regardless of the calling thread. This should prevent data races like the ConcurrentModificationException observed in #3752, under the reasonable assumption that shared persistable objects are only mutated in the user thread. Also add a ThreadedPersistableEnvelope sub-interface overriding the default method above, to let objects which are expensive to serialise (like DaoStateStore) be selectively serialised in the 'save-file-task-X' thread as before, but directly synchronised with each mutating op. As most objects are cheap to serialise, this avoids a noticeable perf drop without having to track down every mutating method for each store. In all cases but one, classes implementing ThreadedPersistableEnvelope are stores like TradeStatistic2Store, with a single ConcurrentHashMap field. These require no further serialisation, since the map entries are immutable, so the only mutating operations are map.put(..) calls which are already synchronised with map reads. (Even if map.values().stream() sees updates @ different keys happen out-of-order, it should be benign.) The remaining case is DaoStateStore, which is only ever reset or modified via a single persist(..) call with a cloned DaoState instance and hash chain from DaoStateSnapshotService, so there is no aliasing risk from the various DAO state mutations done in DaoStateService and elsewhere. This should fix #3752. --- .../persistable/PersistableEnvelope.java | 14 ++++++ .../ThreadedPersistableEnvelope.java | 44 +++++++++++++++++++ .../java/bisq/common/storage/FileManager.java | 2 +- .../core/account/sign/SignedWitnessStore.java | 4 +- .../witness/AccountAgeWitnessStore.java | 4 +- .../blindvote/storage/BlindVoteStore.java | 4 +- .../storage/appendonly/ProposalStore.java | 4 +- .../storage/temp/TempProposalStore.java | 4 +- .../dao/state/DaoStateStorageService.java | 6 ++- .../bisq/core/dao/state/DaoStateStore.java | 4 +- .../statistics/TradeStatistics2Store.java | 4 +- .../PersistableNetworkPayloadList.java | 4 +- .../persistence/SequenceNumberMap.java | 6 +-- 13 files changed, 82 insertions(+), 22 deletions(-) create mode 100644 common/src/main/java/bisq/common/proto/persistable/ThreadedPersistableEnvelope.java diff --git a/common/src/main/java/bisq/common/proto/persistable/PersistableEnvelope.java b/common/src/main/java/bisq/common/proto/persistable/PersistableEnvelope.java index dfab4f0b4dd..6fd622ca75f 100644 --- a/common/src/main/java/bisq/common/proto/persistable/PersistableEnvelope.java +++ b/common/src/main/java/bisq/common/proto/persistable/PersistableEnvelope.java @@ -18,9 +18,23 @@ package bisq.common.proto.persistable; import bisq.common.Envelope; +import bisq.common.UserThread; + +import com.google.protobuf.Message; + +import com.google.common.util.concurrent.Futures; + +import java.util.concurrent.FutureTask; /** * Interface for the outside envelope object persisted to disk. */ public interface PersistableEnvelope extends Envelope { + + default Message toProtoMessageSynchronized() { + FutureTask toProtoOnUserThread = new FutureTask<>(this::toProtoMessage); + UserThread.execute(toProtoOnUserThread); + //noinspection UnstableApiUsage + return Futures.getUnchecked(toProtoOnUserThread); + } } diff --git a/common/src/main/java/bisq/common/proto/persistable/ThreadedPersistableEnvelope.java b/common/src/main/java/bisq/common/proto/persistable/ThreadedPersistableEnvelope.java new file mode 100644 index 00000000000..9652b5a5cfa --- /dev/null +++ b/common/src/main/java/bisq/common/proto/persistable/ThreadedPersistableEnvelope.java @@ -0,0 +1,44 @@ +/* + * 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.common.proto.persistable; + +import com.google.protobuf.Message; + +/** + * Interface for the outer envelope object persisted to disk, where its serialization + * during persistence takes place on a separate thread (for performance). + *

+ * To make the serialization thread-safe, all modifications of the object must be + * synchronized with it. This may be achieved by wrapping such modifications with the + * provided {@link ThreadedPersistableEnvelope#modifySynchronized(Runnable)} method. + */ +public interface ThreadedPersistableEnvelope extends PersistableEnvelope { + + @Override + default Message toProtoMessageSynchronized() { + synchronized (this) { + return toProtoMessage(); + } + } + + default void modifySynchronized(Runnable modifyTask) { + synchronized (this) { + modifyTask.run(); + } + } +} diff --git a/common/src/main/java/bisq/common/storage/FileManager.java b/common/src/main/java/bisq/common/storage/FileManager.java index 764fdcde6fa..de5c0a63adf 100644 --- a/common/src/main/java/bisq/common/storage/FileManager.java +++ b/common/src/main/java/bisq/common/storage/FileManager.java @@ -189,7 +189,7 @@ private synchronized void saveToFile(T persistable, File dir, File storageFile) log.debug("Write to disc: {}", storageFile.getName()); protobuf.PersistableEnvelope protoPersistable; try { - protoPersistable = (protobuf.PersistableEnvelope) persistable.toProtoMessage(); + protoPersistable = (protobuf.PersistableEnvelope) persistable.toProtoMessageSynchronized(); if (protoPersistable.toByteArray().length == 0) log.error("protoPersistable is empty. persistable=" + persistable.getClass().getSimpleName()); } catch (Throwable e) { 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 cedd570edcc..8b0e340c41f 100644 --- a/core/src/main/java/bisq/core/account/sign/SignedWitnessStore.java +++ b/core/src/main/java/bisq/core/account/sign/SignedWitnessStore.java @@ -21,7 +21,7 @@ import bisq.network.p2p.storage.P2PDataStorage; import bisq.network.p2p.storage.payload.PersistableNetworkPayload; -import bisq.common.proto.persistable.PersistableEnvelope; +import bisq.common.proto.persistable.ThreadedPersistableEnvelope; import com.google.protobuf.Message; @@ -40,7 +40,7 @@ * definition and provide a hashMap for the domain access. */ @Slf4j -public class SignedWitnessStore implements PersistableEnvelope { +public class SignedWitnessStore implements ThreadedPersistableEnvelope { @Getter private Map map = new ConcurrentHashMap<>(); 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 24146fe4610..4731cb9e624 100644 --- a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessStore.java +++ b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessStore.java @@ -20,7 +20,7 @@ import bisq.network.p2p.storage.P2PDataStorage; import bisq.network.p2p.storage.payload.PersistableNetworkPayload; -import bisq.common.proto.persistable.PersistableEnvelope; +import bisq.common.proto.persistable.ThreadedPersistableEnvelope; import com.google.protobuf.Message; @@ -39,7 +39,7 @@ * definition and provide a hashMap for the domain access. */ @Slf4j -public class AccountAgeWitnessStore implements PersistableEnvelope { +public class AccountAgeWitnessStore implements ThreadedPersistableEnvelope { @Getter private Map map = new ConcurrentHashMap<>(); 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 7c7f9c05131..3b7266fca3e 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 @@ -20,7 +20,7 @@ import bisq.network.p2p.storage.P2PDataStorage; import bisq.network.p2p.storage.payload.PersistableNetworkPayload; -import bisq.common.proto.persistable.PersistableEnvelope; +import bisq.common.proto.persistable.ThreadedPersistableEnvelope; import com.google.protobuf.Message; @@ -39,7 +39,7 @@ * definition and provide a hashMap for the domain access. */ @Slf4j -public class BlindVoteStore implements PersistableEnvelope { +public class BlindVoteStore implements ThreadedPersistableEnvelope { @Getter private Map map = new ConcurrentHashMap<>(); 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 9d425a92eb1..2173f7c4770 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 @@ -20,7 +20,7 @@ import bisq.network.p2p.storage.P2PDataStorage; import bisq.network.p2p.storage.payload.PersistableNetworkPayload; -import bisq.common.proto.persistable.PersistableEnvelope; +import bisq.common.proto.persistable.ThreadedPersistableEnvelope; import com.google.protobuf.Message; @@ -39,7 +39,7 @@ * definition and provide a hashMap for the domain access. */ @Slf4j -public class ProposalStore implements PersistableEnvelope { +public class ProposalStore implements ThreadedPersistableEnvelope { @Getter private Map map = new ConcurrentHashMap<>(); diff --git a/core/src/main/java/bisq/core/dao/governance/proposal/storage/temp/TempProposalStore.java b/core/src/main/java/bisq/core/dao/governance/proposal/storage/temp/TempProposalStore.java index 8bc112b7e93..229f173368b 100644 --- a/core/src/main/java/bisq/core/dao/governance/proposal/storage/temp/TempProposalStore.java +++ b/core/src/main/java/bisq/core/dao/governance/proposal/storage/temp/TempProposalStore.java @@ -21,7 +21,7 @@ import bisq.network.p2p.storage.payload.ProtectedStorageEntry; import bisq.common.proto.network.NetworkProtoResolver; -import bisq.common.proto.persistable.PersistableEnvelope; +import bisq.common.proto.persistable.ThreadedPersistableEnvelope; import com.google.protobuf.Message; @@ -42,7 +42,7 @@ * definition and provide a hashMap for the domain access. */ @Slf4j -public class TempProposalStore implements PersistableEnvelope { +public class TempProposalStore implements ThreadedPersistableEnvelope { @Getter private Map map = new ConcurrentHashMap<>(); diff --git a/core/src/main/java/bisq/core/dao/state/DaoStateStorageService.java b/core/src/main/java/bisq/core/dao/state/DaoStateStorageService.java index 669a0c5666d..2e1e9abd1dc 100644 --- a/core/src/main/java/bisq/core/dao/state/DaoStateStorageService.java +++ b/core/src/main/java/bisq/core/dao/state/DaoStateStorageService.java @@ -86,8 +86,10 @@ public void persist(DaoState daoState, LinkedList daoStateHashChai } private void persist(DaoState daoState, LinkedList daoStateHashChain, long delayInMilli) { - store.setDaoState(daoState); - store.setDaoStateHashChain(daoStateHashChain); + store.modifySynchronized(() -> { + store.setDaoState(daoState); + store.setDaoStateHashChain(daoStateHashChain); + }); storage.queueUpForSave(store, delayInMilli); } diff --git a/core/src/main/java/bisq/core/dao/state/DaoStateStore.java b/core/src/main/java/bisq/core/dao/state/DaoStateStore.java index b089d0a59ab..144479c2817 100644 --- a/core/src/main/java/bisq/core/dao/state/DaoStateStore.java +++ b/core/src/main/java/bisq/core/dao/state/DaoStateStore.java @@ -20,7 +20,7 @@ import bisq.core.dao.monitoring.model.DaoStateHash; import bisq.core.dao.state.model.DaoState; -import bisq.common.proto.persistable.PersistableEnvelope; +import bisq.common.proto.persistable.ThreadedPersistableEnvelope; import com.google.protobuf.Message; @@ -35,7 +35,7 @@ @Slf4j -public class DaoStateStore implements PersistableEnvelope { +public class DaoStateStore implements ThreadedPersistableEnvelope { // DaoState is always a clone and must not be used for read access beside initial read from disc when we apply // the snapshot! @Getter 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 e1a9ae979d6..d3ea468907c 100644 --- a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2Store.java +++ b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2Store.java @@ -20,7 +20,7 @@ import bisq.network.p2p.storage.P2PDataStorage; import bisq.network.p2p.storage.payload.PersistableNetworkPayload; -import bisq.common.proto.persistable.PersistableEnvelope; +import bisq.common.proto.persistable.ThreadedPersistableEnvelope; import com.google.protobuf.Message; @@ -38,7 +38,7 @@ * definition and provide a hashMap for the domain access. */ @Slf4j -public class TradeStatistics2Store implements PersistableEnvelope { +public class TradeStatistics2Store implements ThreadedPersistableEnvelope { @Getter private Map map = new ConcurrentHashMap<>(); diff --git a/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadList.java b/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadList.java index fea9e360541..50cb5d7b20d 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadList.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/persistence/PersistableNetworkPayloadList.java @@ -20,8 +20,8 @@ import bisq.network.p2p.storage.P2PDataStorage; import bisq.network.p2p.storage.payload.PersistableNetworkPayload; -import bisq.common.proto.persistable.PersistableEnvelope; import bisq.common.proto.persistable.PersistenceProtoResolver; +import bisq.common.proto.persistable.ThreadedPersistableEnvelope; import com.google.protobuf.Message; @@ -42,7 +42,7 @@ // TODO at next hard fork we can rename the PB definition and class name. @Deprecated @Slf4j -public class PersistableNetworkPayloadList implements PersistableEnvelope { +public class PersistableNetworkPayloadList implements ThreadedPersistableEnvelope { @Getter private Map map = new ConcurrentHashMap<>(); diff --git a/p2p/src/main/java/bisq/network/p2p/storage/persistence/SequenceNumberMap.java b/p2p/src/main/java/bisq/network/p2p/storage/persistence/SequenceNumberMap.java index cd82336000a..cbb4f77398a 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/persistence/SequenceNumberMap.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/persistence/SequenceNumberMap.java @@ -19,7 +19,7 @@ import bisq.network.p2p.storage.P2PDataStorage; -import bisq.common.proto.persistable.PersistableEnvelope; +import bisq.common.proto.persistable.ThreadedPersistableEnvelope; import java.util.HashMap; import java.util.Map; @@ -34,7 +34,7 @@ * in protobuffer the map construct can't be anything, so the straightforward mapping was not possible. * Hence this Persistable class. */ -public class SequenceNumberMap implements PersistableEnvelope { +public class SequenceNumberMap implements ThreadedPersistableEnvelope { @Getter @Setter private Map map = new ConcurrentHashMap<>(); @@ -70,7 +70,7 @@ public protobuf.PersistableEnvelope toProtoMessage() { public static SequenceNumberMap fromProto(protobuf.SequenceNumberMap proto) { HashMap map = new HashMap<>(); - proto.getSequenceNumberEntriesList().stream() + proto.getSequenceNumberEntriesList() .forEach(e -> map.put(P2PDataStorage.ByteArray.fromProto(e.getBytes()), P2PDataStorage.MapValue.fromProto(e.getMapValue()))); return new SequenceNumberMap(map); } From 9e9fc6ab5723a3bbce66ebba9670dc0b76b00991 Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Tue, 10 Mar 2020 10:36:52 +0800 Subject: [PATCH 3/5] Rename toProtoMessageSynchronized to toPersistableMessage Make PersistableEnvelope method name less confusing, as it is only synchronised in ThreadedPersistableEnvelope. --- .../java/bisq/common/proto/persistable/PersistableEnvelope.java | 2 +- .../common/proto/persistable/ThreadedPersistableEnvelope.java | 2 +- common/src/main/java/bisq/common/storage/FileManager.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/src/main/java/bisq/common/proto/persistable/PersistableEnvelope.java b/common/src/main/java/bisq/common/proto/persistable/PersistableEnvelope.java index 6fd622ca75f..46c5443df19 100644 --- a/common/src/main/java/bisq/common/proto/persistable/PersistableEnvelope.java +++ b/common/src/main/java/bisq/common/proto/persistable/PersistableEnvelope.java @@ -31,7 +31,7 @@ */ public interface PersistableEnvelope extends Envelope { - default Message toProtoMessageSynchronized() { + default Message toPersistableMessage() { FutureTask toProtoOnUserThread = new FutureTask<>(this::toProtoMessage); UserThread.execute(toProtoOnUserThread); //noinspection UnstableApiUsage diff --git a/common/src/main/java/bisq/common/proto/persistable/ThreadedPersistableEnvelope.java b/common/src/main/java/bisq/common/proto/persistable/ThreadedPersistableEnvelope.java index 9652b5a5cfa..7cee389f07e 100644 --- a/common/src/main/java/bisq/common/proto/persistable/ThreadedPersistableEnvelope.java +++ b/common/src/main/java/bisq/common/proto/persistable/ThreadedPersistableEnvelope.java @@ -30,7 +30,7 @@ public interface ThreadedPersistableEnvelope extends PersistableEnvelope { @Override - default Message toProtoMessageSynchronized() { + default Message toPersistableMessage() { synchronized (this) { return toProtoMessage(); } diff --git a/common/src/main/java/bisq/common/storage/FileManager.java b/common/src/main/java/bisq/common/storage/FileManager.java index de5c0a63adf..06ae86b8695 100644 --- a/common/src/main/java/bisq/common/storage/FileManager.java +++ b/common/src/main/java/bisq/common/storage/FileManager.java @@ -189,7 +189,7 @@ private synchronized void saveToFile(T persistable, File dir, File storageFile) log.debug("Write to disc: {}", storageFile.getName()); protobuf.PersistableEnvelope protoPersistable; try { - protoPersistable = (protobuf.PersistableEnvelope) persistable.toProtoMessageSynchronized(); + protoPersistable = (protobuf.PersistableEnvelope) persistable.toPersistableMessage(); if (protoPersistable.toByteArray().length == 0) log.error("protoPersistable is empty. persistable=" + persistable.getClass().getSimpleName()); } catch (Throwable e) { From 91fa07343c1f23cba16937a58f0f67f7cebe2bd5 Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Tue, 10 Mar 2020 11:44:23 +0800 Subject: [PATCH 4/5] Add UserThreadMappedPersistableEnvelope for greater explicitness Make the default toPersistableMessage() method of PersistableEnvelope simply delegate to Proto.toProtoMessage for speed, so that stores can explicitly implement (Threaded|UserThreadMapped)PersistableEnvelope if they actually need concurrency control. As part of this, make PeerList implement PersistableEnvelope directly instead of extending PersistableList, as it is non-critical & cloned on the user thread prior to storage anyway, so doesn't need be thread-safe. In this way, only PaymentAccountList & small DAO-related stores extend PersistableList, so they can all be made user-thread-mapped. After this change, the only concrete store classes not implementing (Threaded|UserThreadMapped)PersistableEnvelope are: AccountAgeWitness, BlindVotePayload, ProposalPayload, SignedWitness, TradeStatistics2, NavigationPath & PeerList The first five appear to erroneously implement PersistableEnvelope and can be cleaned up in a separate commit. The last two are non-critical. (Make NavigationPath.path an immutable list, for slightly better thread safety anyway - that way it will never be observed half-constructed.) --- .../proto/persistable/NavigationPath.java | 5 +-- .../persistable/PersistableEnvelope.java | 10 +---- .../proto/persistable/PersistableList.java | 2 +- .../UserThreadMappedPersistableEnvelope.java | 45 +++++++++++++++++++ .../bisq/core/btc/model/AddressEntryList.java | 4 +- .../core/support/dispute/DisputeList.java | 3 +- .../java/bisq/core/trade/TradableList.java | 12 ++--- .../bisq/core/user/PreferencesPayload.java | 4 +- .../main/java/bisq/core/user/UserPayload.java | 4 +- .../main/java/bisq/desktop/Navigation.java | 2 +- .../p2p/peers/peerexchange/PeerList.java | 17 ++++--- 11 files changed, 76 insertions(+), 32 deletions(-) create mode 100644 common/src/main/java/bisq/common/proto/persistable/UserThreadMappedPersistableEnvelope.java diff --git a/common/src/main/java/bisq/common/proto/persistable/NavigationPath.java b/common/src/main/java/bisq/common/proto/persistable/NavigationPath.java index 998498deb70..1cb22d428ae 100644 --- a/common/src/main/java/bisq/common/proto/persistable/NavigationPath.java +++ b/common/src/main/java/bisq/common/proto/persistable/NavigationPath.java @@ -21,7 +21,6 @@ import com.google.protobuf.Message; -import java.util.ArrayList; import java.util.List; import lombok.AllArgsConstructor; @@ -36,7 +35,7 @@ @Getter @Setter public class NavigationPath implements PersistableEnvelope { - private List path = new ArrayList<>(); + private List path = List.of(); @Override public Message toProtoMessage() { @@ -46,6 +45,6 @@ public Message toProtoMessage() { } public static NavigationPath fromProto(protobuf.NavigationPath proto) { - return new NavigationPath(new ArrayList<>(proto.getPathList())); + return new NavigationPath(List.copyOf(proto.getPathList())); } } diff --git a/common/src/main/java/bisq/common/proto/persistable/PersistableEnvelope.java b/common/src/main/java/bisq/common/proto/persistable/PersistableEnvelope.java index 46c5443df19..6700839eaf6 100644 --- a/common/src/main/java/bisq/common/proto/persistable/PersistableEnvelope.java +++ b/common/src/main/java/bisq/common/proto/persistable/PersistableEnvelope.java @@ -18,23 +18,15 @@ package bisq.common.proto.persistable; import bisq.common.Envelope; -import bisq.common.UserThread; import com.google.protobuf.Message; -import com.google.common.util.concurrent.Futures; - -import java.util.concurrent.FutureTask; - /** * Interface for the outside envelope object persisted to disk. */ public interface PersistableEnvelope extends Envelope { default Message toPersistableMessage() { - FutureTask toProtoOnUserThread = new FutureTask<>(this::toProtoMessage); - UserThread.execute(toProtoOnUserThread); - //noinspection UnstableApiUsage - return Futures.getUnchecked(toProtoOnUserThread); + return toProtoMessage(); } } diff --git a/common/src/main/java/bisq/common/proto/persistable/PersistableList.java b/common/src/main/java/bisq/common/proto/persistable/PersistableList.java index 8c9b26fc5aa..d1c30613278 100644 --- a/common/src/main/java/bisq/common/proto/persistable/PersistableList.java +++ b/common/src/main/java/bisq/common/proto/persistable/PersistableList.java @@ -31,7 +31,7 @@ import lombok.experimental.Delegate; @EqualsAndHashCode -public class PersistableList implements PersistableEnvelope, Iterable { +public class PersistableList implements UserThreadMappedPersistableEnvelope, Iterable { @Delegate(excludes = ExcludesDelegateMethods.class) @Getter @Setter diff --git a/common/src/main/java/bisq/common/proto/persistable/UserThreadMappedPersistableEnvelope.java b/common/src/main/java/bisq/common/proto/persistable/UserThreadMappedPersistableEnvelope.java new file mode 100644 index 00000000000..aa5e84cda75 --- /dev/null +++ b/common/src/main/java/bisq/common/proto/persistable/UserThreadMappedPersistableEnvelope.java @@ -0,0 +1,45 @@ +/* + * 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.common.proto.persistable; + +import bisq.common.UserThread; + +import com.google.protobuf.Message; + +import com.google.common.util.concurrent.Futures; + +import java.util.concurrent.FutureTask; + +/** + * Interface for the outer envelope object persisted to disk, where its serialization + * during persistence is forced to take place on the user thread. + *

+ * To avoid jitter, this should be only be used for small, safely critical stores. Larger + * or frequently written stores should either implement {@link PersistableEnvelope} + * directly (where thread-safety isn't needed) or use {@link ThreadedPersistableEnvelope}. + */ +public interface UserThreadMappedPersistableEnvelope extends PersistableEnvelope { + + @Override + default Message toPersistableMessage() { + FutureTask toProtoOnUserThread = new FutureTask<>(this::toProtoMessage); + UserThread.execute(toProtoOnUserThread); + //noinspection UnstableApiUsage + return Futures.getUnchecked(toProtoOnUserThread); + } +} diff --git a/core/src/main/java/bisq/core/btc/model/AddressEntryList.java b/core/src/main/java/bisq/core/btc/model/AddressEntryList.java index 867474132e7..becb46dfbef 100644 --- a/core/src/main/java/bisq/core/btc/model/AddressEntryList.java +++ b/core/src/main/java/bisq/core/btc/model/AddressEntryList.java @@ -17,8 +17,8 @@ package bisq.core.btc.model; -import bisq.common.proto.persistable.PersistableEnvelope; import bisq.common.proto.persistable.PersistedDataHost; +import bisq.common.proto.persistable.UserThreadMappedPersistableEnvelope; import bisq.common.storage.Storage; import com.google.protobuf.Message; @@ -44,7 +44,7 @@ */ @ToString @Slf4j -public final class AddressEntryList implements PersistableEnvelope, PersistedDataHost { +public final class AddressEntryList implements UserThreadMappedPersistableEnvelope, PersistedDataHost { transient private Storage storage; transient private Wallet wallet; @Getter diff --git a/core/src/main/java/bisq/core/support/dispute/DisputeList.java b/core/src/main/java/bisq/core/support/dispute/DisputeList.java index d2dd6cca12f..468699c4abf 100644 --- a/core/src/main/java/bisq/core/support/dispute/DisputeList.java +++ b/core/src/main/java/bisq/core/support/dispute/DisputeList.java @@ -19,6 +19,7 @@ import bisq.common.proto.persistable.PersistableEnvelope; import bisq.common.proto.persistable.PersistedDataHost; +import bisq.common.proto.persistable.UserThreadMappedPersistableEnvelope; import bisq.common.storage.Storage; import javafx.collections.FXCollections; @@ -39,7 +40,7 @@ * Calls to the List are delegated because this class intercepts the add/remove calls so changes * can be saved to disc. */ -public abstract class DisputeList implements PersistableEnvelope, PersistedDataHost { +public abstract class DisputeList implements UserThreadMappedPersistableEnvelope, PersistedDataHost { transient protected final Storage storage; @Getter diff --git a/core/src/main/java/bisq/core/trade/TradableList.java b/core/src/main/java/bisq/core/trade/TradableList.java index 9e0b9cf3b38..eae2c3a60b1 100644 --- a/core/src/main/java/bisq/core/trade/TradableList.java +++ b/core/src/main/java/bisq/core/trade/TradableList.java @@ -23,7 +23,7 @@ import bisq.common.proto.ProtoUtil; import bisq.common.proto.ProtobufferRuntimeException; -import bisq.common.proto.persistable.PersistableEnvelope; +import bisq.common.proto.persistable.UserThreadMappedPersistableEnvelope; import bisq.common.storage.Storage; import com.google.protobuf.Message; @@ -41,7 +41,7 @@ import lombok.extern.slf4j.Slf4j; @Slf4j -public final class TradableList implements PersistableEnvelope { +public final class TradableList implements UserThreadMappedPersistableEnvelope { transient final private Storage> storage; @Getter private final ObservableList list = FXCollections.observableArrayList(); @@ -78,10 +78,10 @@ public Message toProtoMessage() { .build(); } - public static TradableList fromProto(protobuf.TradableList proto, - CoreProtoResolver coreProtoResolver, - Storage> storage, - BtcWalletService btcWalletService) { + public static TradableList fromProto(protobuf.TradableList proto, + CoreProtoResolver coreProtoResolver, + Storage> storage, + BtcWalletService btcWalletService) { List list = proto.getTradableList().stream() .map(tradable -> { switch (tradable.getMessageCase()) { diff --git a/core/src/main/java/bisq/core/user/PreferencesPayload.java b/core/src/main/java/bisq/core/user/PreferencesPayload.java index fa04855db68..3e0997d68b4 100644 --- a/core/src/main/java/bisq/core/user/PreferencesPayload.java +++ b/core/src/main/java/bisq/core/user/PreferencesPayload.java @@ -25,7 +25,7 @@ import bisq.core.proto.CoreProtoResolver; import bisq.common.proto.ProtoUtil; -import bisq.common.proto.persistable.PersistableEnvelope; +import bisq.common.proto.persistable.UserThreadMappedPersistableEnvelope; import com.google.protobuf.Message; @@ -49,7 +49,7 @@ @Slf4j @Data @AllArgsConstructor -public final class PreferencesPayload implements PersistableEnvelope { +public final class PreferencesPayload implements UserThreadMappedPersistableEnvelope { private String userLanguage; private Country userCountry; @SuppressWarnings("MismatchedQueryAndUpdateOfCollection") diff --git a/core/src/main/java/bisq/core/user/UserPayload.java b/core/src/main/java/bisq/core/user/UserPayload.java index 212570a6f72..f7bbb5ab03d 100644 --- a/core/src/main/java/bisq/core/user/UserPayload.java +++ b/core/src/main/java/bisq/core/user/UserPayload.java @@ -28,7 +28,7 @@ import bisq.core.support.dispute.refund.refundagent.RefundAgent; import bisq.common.proto.ProtoUtil; -import bisq.common.proto.persistable.PersistableEnvelope; +import bisq.common.proto.persistable.UserThreadMappedPersistableEnvelope; import java.util.ArrayList; import java.util.HashSet; @@ -46,7 +46,7 @@ @Slf4j @Data @AllArgsConstructor -public class UserPayload implements PersistableEnvelope { +public class UserPayload implements UserThreadMappedPersistableEnvelope { @Nullable private String accountId; @Nullable diff --git a/desktop/src/main/java/bisq/desktop/Navigation.java b/desktop/src/main/java/bisq/desktop/Navigation.java index 020b0720a48..0efeb8cc987 100644 --- a/desktop/src/main/java/bisq/desktop/Navigation.java +++ b/desktop/src/main/java/bisq/desktop/Navigation.java @@ -141,7 +141,7 @@ public void navigateTo(ViewPath newPath, @Nullable Object data) { private void queueUpForSave() { if (currentPath.tip() != null) { - navigationPath.setPath(currentPath.stream().map(Class::getName).collect(Collectors.toList())); + navigationPath.setPath(currentPath.stream().map(Class::getName).collect(Collectors.toUnmodifiableList())); } storage.queueUpForSave(navigationPath, 1000); } diff --git a/p2p/src/main/java/bisq/network/p2p/peers/peerexchange/PeerList.java b/p2p/src/main/java/bisq/network/p2p/peers/peerexchange/PeerList.java index 64e2352ecfe..b09c67df06f 100644 --- a/p2p/src/main/java/bisq/network/p2p/peers/peerexchange/PeerList.java +++ b/p2p/src/main/java/bisq/network/p2p/peers/peerexchange/PeerList.java @@ -17,7 +17,7 @@ package bisq.network.p2p.peers.peerexchange; -import bisq.common.proto.persistable.PersistableList; +import bisq.common.proto.persistable.PersistableEnvelope; import com.google.protobuf.Message; @@ -26,19 +26,26 @@ import java.util.stream.Collectors; import lombok.EqualsAndHashCode; +import lombok.Getter; -@EqualsAndHashCode(callSuper = true) -public class PeerList extends PersistableList { +@EqualsAndHashCode +public class PeerList implements PersistableEnvelope { + @Getter + private final List list; public PeerList(List list) { - super(list); + this.list = list; + } + + public int size() { + return list.size(); } @Override public Message toProtoMessage() { return protobuf.PersistableEnvelope.newBuilder() .setPeerList(protobuf.PeerList.newBuilder() - .addAllPeer(getList().stream().map(Peer::toProtoMessage).collect(Collectors.toList()))) + .addAllPeer(list.stream().map(Peer::toProtoMessage).collect(Collectors.toList()))) .build(); } From 9ab649ec058b4d621a2db0923bb46c70a41df70c Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Sun, 22 Mar 2020 14:12:57 +0800 Subject: [PATCH 5/5] Restore type hierarchy of MeritList & VoteWithProposalTxIdList Provide UserThreadMappedPersistableList subclass for persistable lists which need to implement UserThreadMappedPersistableEnvelope, instead of putting the interface on the base class. Make the (non-storage) classes MeritList and VoteWithProposalTxIdList keep the original PersistableList superclass, deriving the remaining subclasses of PersistableList from the new class instead. In this way, further persistence-related changes are less likely to inadvertently alter the behaviour of those two consensus-critical classes. Removing the superfluous PersistableEnvelope interface from the two classes (via the base class) will be done in a separate PR. --- .../proto/persistable/PersistableList.java | 2 +- .../UserThreadMappedPersistableList.java | 31 +++++++++++++++++++ .../governance/blindvote/MyBlindVoteList.java | 4 +-- .../bond/reputation/MyReputationList.java | 5 ++- .../dao/governance/myvote/MyVoteList.java | 5 ++- .../proofofburn/MyProofOfBurnList.java | 5 ++- .../governance/proposal/MyProposalList.java | 5 ++- .../state/model/governance/BallotList.java | 5 ++- .../UnconfirmedBsqChangeOutputList.java | 4 +-- .../bisq/core/payment/PaymentAccountList.java | 4 +-- 10 files changed, 48 insertions(+), 22 deletions(-) create mode 100644 common/src/main/java/bisq/common/proto/persistable/UserThreadMappedPersistableList.java diff --git a/common/src/main/java/bisq/common/proto/persistable/PersistableList.java b/common/src/main/java/bisq/common/proto/persistable/PersistableList.java index d1c30613278..8c9b26fc5aa 100644 --- a/common/src/main/java/bisq/common/proto/persistable/PersistableList.java +++ b/common/src/main/java/bisq/common/proto/persistable/PersistableList.java @@ -31,7 +31,7 @@ import lombok.experimental.Delegate; @EqualsAndHashCode -public class PersistableList implements UserThreadMappedPersistableEnvelope, Iterable { +public class PersistableList implements PersistableEnvelope, Iterable { @Delegate(excludes = ExcludesDelegateMethods.class) @Getter @Setter diff --git a/common/src/main/java/bisq/common/proto/persistable/UserThreadMappedPersistableList.java b/common/src/main/java/bisq/common/proto/persistable/UserThreadMappedPersistableList.java new file mode 100644 index 00000000000..5337af5efe1 --- /dev/null +++ b/common/src/main/java/bisq/common/proto/persistable/UserThreadMappedPersistableList.java @@ -0,0 +1,31 @@ +/* + * 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.common.proto.persistable; + +import java.util.List; + +public class UserThreadMappedPersistableList extends PersistableList + implements UserThreadMappedPersistableEnvelope { + + public UserThreadMappedPersistableList(List list) { + super(list); + } + + public UserThreadMappedPersistableList() { + } +} diff --git a/core/src/main/java/bisq/core/dao/governance/blindvote/MyBlindVoteList.java b/core/src/main/java/bisq/core/dao/governance/blindvote/MyBlindVoteList.java index be0890b653a..313fc07d349 100644 --- a/core/src/main/java/bisq/core/dao/governance/blindvote/MyBlindVoteList.java +++ b/core/src/main/java/bisq/core/dao/governance/blindvote/MyBlindVoteList.java @@ -19,7 +19,7 @@ import bisq.core.dao.governance.ConsensusCritical; -import bisq.common.proto.persistable.PersistableList; +import bisq.common.proto.persistable.UserThreadMappedPersistableList; import com.google.protobuf.Message; @@ -33,7 +33,7 @@ * List of my own blind votes. Blind votes received from other voters are stored in the BlindVoteStore. */ @EqualsAndHashCode(callSuper = true) -public class MyBlindVoteList extends PersistableList implements ConsensusCritical { +public class MyBlindVoteList extends UserThreadMappedPersistableList implements ConsensusCritical { MyBlindVoteList() { super(); diff --git a/core/src/main/java/bisq/core/dao/governance/bond/reputation/MyReputationList.java b/core/src/main/java/bisq/core/dao/governance/bond/reputation/MyReputationList.java index 729ef5ae314..9e705d2642f 100644 --- a/core/src/main/java/bisq/core/dao/governance/bond/reputation/MyReputationList.java +++ b/core/src/main/java/bisq/core/dao/governance/bond/reputation/MyReputationList.java @@ -17,7 +17,7 @@ package bisq.core.dao.governance.bond.reputation; -import bisq.common.proto.persistable.PersistableList; +import bisq.common.proto.persistable.UserThreadMappedPersistableList; import java.util.ArrayList; import java.util.List; @@ -29,7 +29,7 @@ * PersistableEnvelope wrapper for list of MyReputations. */ @EqualsAndHashCode(callSuper = true) -public class MyReputationList extends PersistableList { +public class MyReputationList extends UserThreadMappedPersistableList { private MyReputationList(List list) { super(list); @@ -69,4 +69,3 @@ public String toString() { .collect(Collectors.toList()); } } - diff --git a/core/src/main/java/bisq/core/dao/governance/myvote/MyVoteList.java b/core/src/main/java/bisq/core/dao/governance/myvote/MyVoteList.java index 06eb206a113..c840faf7e60 100644 --- a/core/src/main/java/bisq/core/dao/governance/myvote/MyVoteList.java +++ b/core/src/main/java/bisq/core/dao/governance/myvote/MyVoteList.java @@ -17,7 +17,7 @@ package bisq.core.dao.governance.myvote; -import bisq.common.proto.persistable.PersistableList; +import bisq.common.proto.persistable.UserThreadMappedPersistableList; import com.google.protobuf.Message; @@ -28,7 +28,7 @@ import lombok.EqualsAndHashCode; @EqualsAndHashCode(callSuper = true) -public class MyVoteList extends PersistableList { +public class MyVoteList extends UserThreadMappedPersistableList { MyVoteList() { super(); @@ -65,4 +65,3 @@ public String toString() { .collect(Collectors.toList()); } } - diff --git a/core/src/main/java/bisq/core/dao/governance/proofofburn/MyProofOfBurnList.java b/core/src/main/java/bisq/core/dao/governance/proofofburn/MyProofOfBurnList.java index ed0fb32e16a..0845b4efb74 100644 --- a/core/src/main/java/bisq/core/dao/governance/proofofburn/MyProofOfBurnList.java +++ b/core/src/main/java/bisq/core/dao/governance/proofofburn/MyProofOfBurnList.java @@ -17,7 +17,7 @@ package bisq.core.dao.governance.proofofburn; -import bisq.common.proto.persistable.PersistableList; +import bisq.common.proto.persistable.UserThreadMappedPersistableList; import java.util.ArrayList; import java.util.List; @@ -29,7 +29,7 @@ * PersistableEnvelope wrapper for list of MyProofOfBurn objects. */ @EqualsAndHashCode(callSuper = true) -public class MyProofOfBurnList extends PersistableList { +public class MyProofOfBurnList extends UserThreadMappedPersistableList { private MyProofOfBurnList(List list) { super(list); @@ -69,4 +69,3 @@ public String toString() { .collect(Collectors.toList()); } } - diff --git a/core/src/main/java/bisq/core/dao/governance/proposal/MyProposalList.java b/core/src/main/java/bisq/core/dao/governance/proposal/MyProposalList.java index 5ec0126d36b..0729043232c 100644 --- a/core/src/main/java/bisq/core/dao/governance/proposal/MyProposalList.java +++ b/core/src/main/java/bisq/core/dao/governance/proposal/MyProposalList.java @@ -20,7 +20,7 @@ import bisq.core.dao.governance.ConsensusCritical; import bisq.core.dao.state.model.governance.Proposal; -import bisq.common.proto.persistable.PersistableList; +import bisq.common.proto.persistable.UserThreadMappedPersistableList; import java.util.ArrayList; import java.util.List; @@ -32,7 +32,7 @@ * PersistableEnvelope wrapper for list of proposals. Used in vote consensus, so changes can break consensus! */ @EqualsAndHashCode(callSuper = true) -public class MyProposalList extends PersistableList implements ConsensusCritical { +public class MyProposalList extends UserThreadMappedPersistableList implements ConsensusCritical { public MyProposalList(List list) { super(list); @@ -72,4 +72,3 @@ public String toString() { .collect(Collectors.toList()); } } - diff --git a/core/src/main/java/bisq/core/dao/state/model/governance/BallotList.java b/core/src/main/java/bisq/core/dao/state/model/governance/BallotList.java index b5a4651a1a7..d3b4a1f97eb 100644 --- a/core/src/main/java/bisq/core/dao/state/model/governance/BallotList.java +++ b/core/src/main/java/bisq/core/dao/state/model/governance/BallotList.java @@ -20,7 +20,7 @@ import bisq.core.dao.governance.ConsensusCritical; import bisq.core.dao.state.model.ImmutableDaoStateModel; -import bisq.common.proto.persistable.PersistableList; +import bisq.common.proto.persistable.UserThreadMappedPersistableList; import java.util.ArrayList; import java.util.List; @@ -35,7 +35,7 @@ */ @Immutable @EqualsAndHashCode(callSuper = true) -public class BallotList extends PersistableList implements ConsensusCritical, ImmutableDaoStateModel { +public class BallotList extends UserThreadMappedPersistableList implements ConsensusCritical, ImmutableDaoStateModel { public BallotList(List list) { super(list); @@ -75,4 +75,3 @@ public String toString() { .collect(Collectors.toList()); } } - diff --git a/core/src/main/java/bisq/core/dao/state/unconfirmed/UnconfirmedBsqChangeOutputList.java b/core/src/main/java/bisq/core/dao/state/unconfirmed/UnconfirmedBsqChangeOutputList.java index 2deae256979..391afda22f7 100644 --- a/core/src/main/java/bisq/core/dao/state/unconfirmed/UnconfirmedBsqChangeOutputList.java +++ b/core/src/main/java/bisq/core/dao/state/unconfirmed/UnconfirmedBsqChangeOutputList.java @@ -17,7 +17,7 @@ package bisq.core.dao.state.unconfirmed; -import bisq.common.proto.persistable.PersistableList; +import bisq.common.proto.persistable.UserThreadMappedPersistableList; import com.google.protobuf.Message; @@ -28,7 +28,7 @@ import lombok.EqualsAndHashCode; @EqualsAndHashCode(callSuper = true) -public class UnconfirmedBsqChangeOutputList extends PersistableList { +public class UnconfirmedBsqChangeOutputList extends UserThreadMappedPersistableList { UnconfirmedBsqChangeOutputList() { super(); diff --git a/core/src/main/java/bisq/core/payment/PaymentAccountList.java b/core/src/main/java/bisq/core/payment/PaymentAccountList.java index a35801705ce..efe9cccc229 100644 --- a/core/src/main/java/bisq/core/payment/PaymentAccountList.java +++ b/core/src/main/java/bisq/core/payment/PaymentAccountList.java @@ -19,7 +19,7 @@ import bisq.core.proto.CoreProtoResolver; -import bisq.common.proto.persistable.PersistableList; +import bisq.common.proto.persistable.UserThreadMappedPersistableList; import com.google.protobuf.Message; @@ -30,7 +30,7 @@ import lombok.EqualsAndHashCode; @EqualsAndHashCode(callSuper = true) -public class PaymentAccountList extends PersistableList { +public class PaymentAccountList extends UserThreadMappedPersistableList { public PaymentAccountList(List list) { super(list);