diff --git a/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java b/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java index 788c369eed6..69a7d49feb0 100644 --- a/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java +++ b/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java @@ -51,6 +51,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -61,7 +62,6 @@ import java.util.Stack; import java.util.stream.Collectors; -import lombok.Getter; import lombok.extern.slf4j.Slf4j; @Slf4j @@ -74,10 +74,14 @@ public class SignedWitnessService { private final P2PService p2PService; private final ArbitratorManager arbitratorManager; private final User user; + private final FilterManager filterManager; - @Getter private final Map signedWitnessMap = new HashMap<>(); - private final FilterManager filterManager; + + // This map keeps all SignedWitnesses with the same AccountAgeWitnessHash in a Set. + // This avoids iterations over the signedWitnessMap for getting the set of such SignedWitnesses. + private final Map> signedWitnessSetByAccountAgeWitnessHash = new HashMap<>(); + /////////////////////////////////////////////////////////////////////////////////////////// // Constructor @@ -142,6 +146,10 @@ private void onBootstrapComplete() { // API /////////////////////////////////////////////////////////////////////////////////////////// + public Collection getSignedWitnessMapValues() { + return signedWitnessMap.values(); + } + /** * List of dates as long when accountAgeWitness was signed * @@ -199,7 +207,7 @@ public String ownerPubKeyAsString(AccountAgeWitness accountAgeWitness) { @VisibleForTesting public Set getSignedWitnessSetByOwnerPubKey(byte[] ownerPubKey) { - return signedWitnessMap.values().stream() + return getSignedWitnessMapValues().stream() .filter(e -> Arrays.equals(e.getWitnessOwnerPubKey(), ownerPubKey)) .collect(Collectors.toSet()); } @@ -344,30 +352,27 @@ private boolean verifySignatureWithDSAKey(SignedWitness signedWitness) { } } - private Set getSignedWitnessSet(AccountAgeWitness accountAgeWitness) { - return signedWitnessMap.values().stream() - .filter(e -> Arrays.equals(e.getAccountAgeWitnessHash(), accountAgeWitness.getHash())) - .collect(Collectors.toSet()); + public Set getSignedWitnessSet(AccountAgeWitness accountAgeWitness) { + P2PDataStorage.ByteArray key = new P2PDataStorage.ByteArray(accountAgeWitness.getHash()); + return signedWitnessSetByAccountAgeWitnessHash.getOrDefault(key, new HashSet<>()); } // SignedWitness objects signed by arbitrators public Set getArbitratorsSignedWitnessSet(AccountAgeWitness accountAgeWitness) { - return signedWitnessMap.values().stream() + return getSignedWitnessSet(accountAgeWitness).stream() .filter(SignedWitness::isSignedByArbitrator) - .filter(e -> Arrays.equals(e.getAccountAgeWitnessHash(), accountAgeWitness.getHash())) .collect(Collectors.toSet()); } // SignedWitness objects signed by any other peer public Set getTrustedPeerSignedWitnessSet(AccountAgeWitness accountAgeWitness) { - return signedWitnessMap.values().stream() + return getSignedWitnessSet(accountAgeWitness).stream() .filter(e -> !e.isSignedByArbitrator()) - .filter(e -> Arrays.equals(e.getAccountAgeWitnessHash(), accountAgeWitness.getHash())) .collect(Collectors.toSet()); } public Set getRootSignedWitnessSet(boolean includeSignedByArbitrator) { - return signedWitnessMap.values().stream() + return getSignedWitnessMapValues().stream() .filter(witness -> getSignedWitnessSetByOwnerPubKey(witness.getSignerPubKey(), new Stack<>()).isEmpty()) .filter(witness -> includeSignedByArbitrator || witness.getVerificationMethod() != SignedWitness.VerificationMethod.ARBITRATOR) @@ -388,7 +393,7 @@ public Set getUnsignedSignerPubKeys() { // witnessOwnerPubKey private Set getSignedWitnessSetByOwnerPubKey(byte[] ownerPubKey, Stack excluded) { - return signedWitnessMap.values().stream() + return getSignedWitnessMapValues().stream() .filter(e -> Arrays.equals(e.getWitnessOwnerPubKey(), ownerPubKey)) .filter(e -> !excluded.contains(new P2PDataStorage.ByteArray(e.getSignerPubKey()))) .collect(Collectors.toSet()); @@ -487,8 +492,12 @@ private boolean verifyDate(SignedWitness signedWitness, long childSignedWitnessD /////////////////////////////////////////////////////////////////////////////////////////// @VisibleForTesting - void addToMap(SignedWitness signedWitness) { + public void addToMap(SignedWitness signedWitness) { signedWitnessMap.putIfAbsent(signedWitness.getHashAsByteArray(), signedWitness); + + P2PDataStorage.ByteArray accountAgeWitnessHash = new P2PDataStorage.ByteArray(signedWitness.getAccountAgeWitnessHash()); + signedWitnessSetByAccountAgeWitnessHash.putIfAbsent(accountAgeWitnessHash, new HashSet<>()); + signedWitnessSetByAccountAgeWitnessHash.get(accountAgeWitnessHash).add(signedWitness); } private void publishSignedWitness(SignedWitness signedWitness) { @@ -501,7 +510,22 @@ private void publishSignedWitness(SignedWitness signedWitness) { } private void doRepublishAllSignedWitnesses() { - signedWitnessMap.forEach((e, signedWitness) -> p2PService.addPersistableNetworkPayload(signedWitness, true)); + getSignedWitnessMapValues() + .forEach(signedWitness -> p2PService.addPersistableNetworkPayload(signedWitness, true)); + } + + @VisibleForTesting + public void removeSignedWitness(SignedWitness signedWitness) { + signedWitnessMap.remove(signedWitness.getHashAsByteArray()); + + P2PDataStorage.ByteArray accountAgeWitnessHash = new P2PDataStorage.ByteArray(signedWitness.getAccountAgeWitnessHash()); + if (signedWitnessSetByAccountAgeWitnessHash.containsKey(accountAgeWitnessHash)) { + Set set = signedWitnessSetByAccountAgeWitnessHash.get(accountAgeWitnessHash); + set.remove(signedWitness); + if (set.isEmpty()) { + signedWitnessSetByAccountAgeWitnessHash.remove(accountAgeWitnessHash); + } + } } // Remove SignedWitnesses that are signed by TRADE that also have an ARBITRATOR signature diff --git a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java index b7678c7fa0c..cf113513c34 100644 --- a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java +++ b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java @@ -73,6 +73,7 @@ import java.util.Optional; import java.util.Random; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -138,9 +139,13 @@ public String getDisplayString() { @Getter private final AccountAgeWitnessUtils accountAgeWitnessUtils; - @Getter private final Map accountAgeWitnessMap = new HashMap<>(); + // The accountAgeWitnessMap is very large (70k items) and access is a bit expensive. We usually only access less + // than 100 items, those who have offers online. So we use a cache for a fast lookup and only if + // not found there we use the accountAgeWitnessMap and put then the new item into our cache. + private final Map accountAgeWitnessCache = new ConcurrentHashMap<>(); + /////////////////////////////////////////////////////////////////////////////////////////// // Constructor @@ -233,8 +238,17 @@ public void addToMap(AccountAgeWitness accountAgeWitness) { public void publishMyAccountAgeWitness(PaymentAccountPayload paymentAccountPayload) { AccountAgeWitness accountAgeWitness = getMyWitness(paymentAccountPayload); - if (!accountAgeWitnessMap.containsKey(accountAgeWitness.getHashAsByteArray())) + P2PDataStorage.ByteArray hash = accountAgeWitness.getHashAsByteArray(); + + // We use first our fast lookup cache. If its in accountAgeWitnessCache it is also in accountAgeWitnessMap + // and we do not publish. + if (accountAgeWitnessCache.containsKey(hash)) { + return; + } + + if (!accountAgeWitnessMap.containsKey(hash)) { p2PService.addPersistableNetworkPayload(accountAgeWitness, false); + } } public byte[] getPeerAccountAgeWitnessHash(Trade trade) { @@ -284,12 +298,21 @@ private Optional findTradePeerWitness(Trade trade) { private Optional getWitnessByHash(byte[] hash) { P2PDataStorage.ByteArray hashAsByteArray = new P2PDataStorage.ByteArray(hash); - final boolean containsKey = accountAgeWitnessMap.containsKey(hashAsByteArray); - if (!containsKey) - log.debug("hash not found in accountAgeWitnessMap"); + // First we look up in our fast lookup cache + if (accountAgeWitnessCache.containsKey(hashAsByteArray)) { + return Optional.of(accountAgeWitnessCache.get(hashAsByteArray)); + } + + if (accountAgeWitnessMap.containsKey(hashAsByteArray)) { + AccountAgeWitness accountAgeWitness = accountAgeWitnessMap.get(hashAsByteArray); + + // We add it to our fast lookup cache + accountAgeWitnessCache.put(hashAsByteArray, accountAgeWitness); + + return Optional.of(accountAgeWitness); + } - return accountAgeWitnessMap.containsKey(hashAsByteArray) ? - Optional.of(accountAgeWitnessMap.get(hashAsByteArray)) : Optional.empty(); + return Optional.empty(); } private Optional getWitnessByHashAsHex(String hashAsHex) { @@ -656,16 +679,20 @@ public void arbitratorSignAccountAgeWitness(Coin tradeAmount, } public String arbitratorSignOrphanWitness(AccountAgeWitness accountAgeWitness, - ECKey key, + ECKey ecKey, long time) { - // Find AccountAgeWitness as signedwitness - var signedWitness = signedWitnessService.getSignedWitnessMap().values().stream() - .filter(sw -> Arrays.equals(sw.getAccountAgeWitnessHash(), accountAgeWitness.getHash())) + // TODO Is not found signedWitness considered an error case? + // Previous code version was throwing an exception in case no signedWitness was found... + + // signAndPublishAccountAgeWitness returns an empty string in success case and error otherwise + return signedWitnessService.getSignedWitnessSet(accountAgeWitness).stream() .findAny() - .orElse(null); - checkNotNull(signedWitness); - return signedWitnessService.signAndPublishAccountAgeWitness(accountAgeWitness, key, - signedWitness.getWitnessOwnerPubKey(), time); + .map(SignedWitness::getWitnessOwnerPubKey) + .map(witnessOwnerPubKey -> + signedWitnessService.signAndPublishAccountAgeWitness(accountAgeWitness, ecKey, + witnessOwnerPubKey, time) + ) + .orElse("No signedWitness found"); } public String arbitratorSignOrphanPubKey(ECKey key, diff --git a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessUtils.java b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessUtils.java index 6bc4b27218b..628f0a81862 100644 --- a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessUtils.java +++ b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessUtils.java @@ -30,6 +30,7 @@ import bisq.common.util.Utilities; import java.util.Arrays; +import java.util.Collection; import java.util.Optional; import java.util.Stack; @@ -67,12 +68,11 @@ public void logSignedWitnesses() { } private void logChild(SignedWitness sigWit, String initString, Stack excluded) { - var allSig = signedWitnessService.getSignedWitnessMap(); log.info("{}AEW: {} PKH: {} time: {}", initString, Utilities.bytesAsHexString(sigWit.getAccountAgeWitnessHash()).substring(0, 7), Utilities.bytesAsHexString(Hash.getRipemd160hash(sigWit.getWitnessOwnerPubKey())).substring(0, 7), sigWit.getDate()); - allSig.values().forEach(w -> { + signedWitnessService.getSignedWitnessMapValues().forEach(w -> { if (!excluded.contains(new P2PDataStorage.ByteArray(w.getWitnessOwnerPubKey())) && Arrays.equals(w.getSignerPubKey(), sigWit.getWitnessOwnerPubKey())) { excluded.push(new P2PDataStorage.ByteArray(w.getWitnessOwnerPubKey())); @@ -85,10 +85,10 @@ private void logChild(SignedWitness sigWit, String initString, Stack { + Collection signedWitnessMapValues = signedWitnessService.getSignedWitnessMapValues(); + signedWitnessMapValues.forEach(w -> { log.info("AEW {}", Utilities.bytesAsHexString(w.getAccountAgeWitnessHash())); - allSig.values().forEach(ww -> { + signedWitnessMapValues.forEach(ww -> { if (Arrays.equals(w.getSignerPubKey(), ww.getWitnessOwnerPubKey())) { log.info(" {}", Utilities.bytesAsHexString(ww.getAccountAgeWitnessHash())); } diff --git a/core/src/test/java/bisq/core/account/witness/AccountAgeWitnessServiceTest.java b/core/src/test/java/bisq/core/account/witness/AccountAgeWitnessServiceTest.java index 06ffd4470c1..946e4db64d4 100644 --- a/core/src/test/java/bisq/core/account/witness/AccountAgeWitnessServiceTest.java +++ b/core/src/test/java/bisq/core/account/witness/AccountAgeWitnessServiceTest.java @@ -310,12 +310,12 @@ public void testArbitratorSignDummyWitness() throws CryptoException { // Remove SignedWitness signed by arbitrator @SuppressWarnings("OptionalGetWithoutIsPresent") - var signedWitnessArb = signedWitnessService.getSignedWitnessMap().values().stream() + var signedWitnessArb = signedWitnessService.getSignedWitnessMapValues().stream() .filter(sw -> sw.getVerificationMethod() == SignedWitness.VerificationMethod.ARBITRATOR) .findAny() .get(); - signedWitnessService.getSignedWitnessMap().remove(signedWitnessArb.getHashAsByteArray()); - assertEquals(signedWitnessService.getSignedWitnessMap().size(), 2); + signedWitnessService.removeSignedWitness(signedWitnessArb); + assertEquals(signedWitnessService.getSignedWitnessMapValues().size(), 2); // Check that no account age witness is a signer assertFalse(service.accountIsSigner(aew1)); @@ -354,7 +354,7 @@ private void signAccountAgeWitness(AccountAgeWitness accountAgeWitness, witnessOwnerPubKey.getEncoded(), time, SignedWitnessService.MINIMUM_TRADE_AMOUNT_FOR_SIGNING.value); - signedWitnessService.getSignedWitnessMap().putIfAbsent(signedWitness.getHashAsByteArray(), signedWitness); + signedWitnessService.addToMap(signedWitness); } }