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 8bcf2749a87..17b33d075d7 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 @@ -75,10 +75,6 @@ public class SignedWitnessService { private final ArbitratorManager arbitratorManager; private final User user; - // TODO signedWitnessMap could be a HashSet as clients access the map values only. - // Only one test uses the key for removing an entry but seems that use case does not exist otherwise. - // As there are only about 5000 objects it will probably not have a significatn impact to change that... - @Getter private final Map signedWitnessMap = new HashMap<>(); // The getSignedWitnessSet is called very often and is a bit expensive. We cache the result in that map but we @@ -151,6 +147,10 @@ private void onBootstrapComplete() { // API /////////////////////////////////////////////////////////////////////////////////////////// + public Collection getSignedWitnessMapValues() { + return signedWitnessMap.values(); + } + /** * List of dates as long when accountAgeWitness was signed * @@ -208,7 +208,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()); } @@ -362,7 +362,7 @@ private Set getSignedWitnessSet(AccountAgeWitness accountAgeWitne return getSignedWitnessSetCache.get(key); } - Set result = signedWitnessMap.values().stream() + Set result = getSignedWitnessMapValues().stream() .filter(e -> Arrays.equals(e.getAccountAgeWitnessHash(), hash)) .collect(Collectors.toSet()); @@ -373,7 +373,7 @@ private Set getSignedWitnessSet(AccountAgeWitness accountAgeWitne // SignedWitness objects signed by arbitrators public Set getArbitratorsSignedWitnessSet(AccountAgeWitness accountAgeWitness) { - return signedWitnessMap.values().stream() + return getSignedWitnessMapValues().stream() .filter(SignedWitness::isSignedByArbitrator) .filter(e -> Arrays.equals(e.getAccountAgeWitnessHash(), accountAgeWitness.getHash())) .collect(Collectors.toSet()); @@ -381,14 +381,14 @@ public Set getArbitratorsSignedWitnessSet(AccountAgeWitness accou // SignedWitness objects signed by any other peer public Set getTrustedPeerSignedWitnessSet(AccountAgeWitness accountAgeWitness) { - return signedWitnessMap.values().stream() + return getSignedWitnessMapValues().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) @@ -409,7 +409,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()); @@ -508,12 +508,18 @@ private boolean verifyDate(SignedWitness signedWitness, long childSignedWitnessD /////////////////////////////////////////////////////////////////////////////////////////// @VisibleForTesting - void addToMap(SignedWitness signedWitness) { - signedWitnessMap.putIfAbsent(signedWitness.getHashAsByteArray(), signedWitness); + public void addToMap(SignedWitness signedWitness) { + P2PDataStorage.ByteArray hash = signedWitness.getHashAsByteArray(); + signedWitnessMap.putIfAbsent(hash, signedWitness); // We remove the entry with that hash in case we have cached it, so at the next getSignedWitnessSet // call we use the updated signedWitnessMap to re-fill our cache. getSignedWitnessSetCache.remove(new P2PDataStorage.ByteArray(signedWitness.getAccountAgeWitnessHash())); + + // Not sure if that is needed as well, tests did succeed in both cases, but seems to be more safe to remove + // potential entries with hash as well. A removed item in getSignedWitnessSetCache carries no risk, though a + // remaining item would. + getSignedWitnessSetCache.remove(hash); } private void publishSignedWitness(SignedWitness signedWitness) { @@ -526,7 +532,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) { + P2PDataStorage.ByteArray hash = signedWitness.getHashAsByteArray(); + signedWitnessMap.remove(hash); + + // Need to remove the entry matching signedWitness.getAccountAgeWitnessHash() (test would fail otherwise) + getSignedWitnessSetCache.remove(new P2PDataStorage.ByteArray(signedWitness.getAccountAgeWitnessHash())); + + // Not sure if that is needed as well, tests did succeed in both cases, but seems to be more safe to remove + // potential entries with hash as well. A removed item in getSignedWitnessSetCache carries no risk, though a + // remaining item would. + getSignedWitnessSetCache.remove(hash); } // 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 9b8a0daab8c..c3f4f196c1a 100644 --- a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java +++ b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java @@ -681,7 +681,7 @@ public String arbitratorSignOrphanWitness(AccountAgeWitness accountAgeWitness, ECKey key, long time) { // Find AccountAgeWitness as signedwitness - var signedWitness = signedWitnessService.getSignedWitnessMap().values().stream() + var signedWitness = signedWitnessService.getSignedWitnessMapValues().stream() .filter(sw -> Arrays.equals(sw.getAccountAgeWitnessHash(), accountAgeWitness.getHash())) .findAny() .orElse(null); 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); } }