Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

import static bisq.network.p2p.services.data.storage.MetaData.DEFAULT_PRIORITY;
import static bisq.network.p2p.services.data.storage.MetaData.*;
import static com.google.common.base.Preconditions.checkArgument;

@Slf4j
Expand All @@ -49,7 +49,7 @@ public final class AuthorizedMarketPriceData implements AuthorizedDistributedDat
public static final long TTL = TimeUnit.MINUTES.toMillis(10);

// MetaData is transient as it will be used indirectly by low level network classes. Only some low level network classes write the metaData to their protobuf representations.
private transient final MetaData metaData = new MetaData(TTL, DEFAULT_PRIORITY, getClass().getSimpleName());
private transient final MetaData metaData = new MetaData(TTL, DEFAULT_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_100);
@EqualsAndHashCode.Exclude
@ExcludeForHash
private final int version;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,15 @@

import java.util.Optional;

import static bisq.network.p2p.services.data.storage.MetaData.MAX_MAP_SIZE_100;
import static bisq.network.p2p.services.data.storage.MetaData.TTL_10_DAYS;
import static bisq.network.p2p.services.data.storage.MetaData.*;

@Slf4j
@Getter
@ToString
@EqualsAndHashCode
public final class BondedRoleRegistrationRequest implements MailboxMessage, ExternalNetworkMessage {
// MetaData is transient as it will be used indirectly by low level network classes. Only some low level network classes write the metaData to their protobuf representations.
private transient final MetaData metaData = new MetaData(TTL_10_DAYS, getClass().getSimpleName(), MAX_MAP_SIZE_100);
private transient final MetaData metaData = new MetaData(TTL_10_DAYS, LOW_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_100);
private final String profileId;
private final String authorizedPublicKey;
private final BondedRoleType bondedRoleType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public final class ReleaseNotification implements AuthorizedDistributedData {
public final static int MAX_MESSAGE_LENGTH = 10_000;

// MetaData is transient as it will be used indirectly by low level network classes. Only some low level network classes write the metaData to their protobuf representations.
private transient final MetaData metaData = new MetaData(TTL_100_DAYS, HIGH_PRIORITY, getClass().getSimpleName());
private transient final MetaData metaData = new MetaData(TTL_100_DAYS, HIGH_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_100);
@EqualsAndHashCode.Exclude
@ExcludeForHash
private final int version;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public final class AuthorizedAlertData implements AuthorizedDistributedData {
public final static int MAX_BANNED_ACCOUNT_DATA_LENGTH = 10_000;

// MetaData is transient as it will be used indirectly by low level network classes. Only some low level network classes write the metaData to their protobuf representations.
private transient final MetaData metaData = new MetaData(TTL_30_DAYS, HIGH_PRIORITY, getClass().getSimpleName());
private transient final MetaData metaData = new MetaData(TTL_30_DAYS, HIGH_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_1000);
@EqualsAndHashCode.Exclude
@ExcludeForHash
private final int version;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@

import java.util.Set;

import static bisq.network.p2p.services.data.storage.MetaData.HIGH_PRIORITY;
import static bisq.network.p2p.services.data.storage.MetaData.TTL_100_DAYS;
import static bisq.network.p2p.services.data.storage.MetaData.*;

@Slf4j
@ToString
Expand All @@ -45,7 +44,7 @@ public final class AuthorizedDifficultyAdjustmentData implements AuthorizedDistr
private static final int VERSION = 1;

// MetaData is transient as it will be used indirectly by low level network classes. Only some low level network classes write the metaData to their protobuf representations.
private transient final MetaData metaData = new MetaData(TTL_100_DAYS, HIGH_PRIORITY, getClass().getSimpleName());
private transient final MetaData metaData = new MetaData(TTL_100_DAYS, HIGH_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_100);
@EqualsAndHashCode.Exclude
@ExcludeForHash
private final int version;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@

import java.util.Set;

import static bisq.network.p2p.services.data.storage.MetaData.HIGH_PRIORITY;
import static bisq.network.p2p.services.data.storage.MetaData.TTL_100_DAYS;
import static bisq.network.p2p.services.data.storage.MetaData.*;

@Slf4j
@ToString
Expand All @@ -46,7 +45,7 @@ public final class AuthorizedMinRequiredReputationScoreData implements Authorize
private static final int VERSION = 1;

// MetaData is transient as it will be used indirectly by low level network classes. Only some low level network classes write the metaData to their protobuf representations.
private transient final MetaData metaData = new MetaData(TTL_100_DAYS, HIGH_PRIORITY, getClass().getSimpleName());
private transient final MetaData metaData = new MetaData(TTL_100_DAYS, HIGH_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_100);
@EqualsAndHashCode.Exclude
@ExcludeForHash
private final int version;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public static BisqEasyOpenTradeMessage createTakeOfferMessage(String tradeId,

// Metadata needs to be symmetric with BisqEasyOpenTradeMessageReaction.
// MetaData is transient as it will be used indirectly by low level network classes. Only some low level network classes write the metaData to their protobuf representations.
private transient final MetaData metaData = new MetaData(TTL_30_DAYS, HIGH_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_100);
private transient final MetaData metaData = new MetaData(TTL_30_DAYS, HIGH_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_10_000);

private final String tradeId;
private final Optional<UserProfile> mediator;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,16 @@
import lombok.ToString;
import lombok.extern.slf4j.Slf4j;

import static bisq.network.p2p.services.data.storage.MetaData.*;

@Slf4j
@Getter
@ToString(callSuper = true)
@EqualsAndHashCode(callSuper = true)
public class CommonPublicChatMessageReaction extends ChatMessageReaction implements DistributedData {
// Metadata needs to be symmetric with CommonPublicChatMessage.
// MetaData is transient as it will be used indirectly by low level network classes. Only some low level network classes write the metaData to their protobuf representations.
private transient final MetaData metaData = new MetaData(MetaData.TTL_10_DAYS, MetaData.LOW_PRIORITY, getClass().getSimpleName(), MetaData.MAX_MAP_SIZE_10_000);
private transient final MetaData metaData = new MetaData(TTL_10_DAYS, LOW_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_10_000);

public CommonPublicChatMessageReaction(String id,
String userProfileId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@
import lombok.ToString;
import lombok.extern.slf4j.Slf4j;

import static bisq.network.p2p.services.data.storage.MetaData.MAX_MAP_SIZE_100;
import static bisq.network.p2p.services.data.storage.MetaData.TTL_30_DAYS;
import static bisq.network.p2p.services.data.storage.MetaData.*;

@Slf4j
@Getter
Expand All @@ -36,7 +35,7 @@
public final class TwoPartyPrivateChatMessageReaction extends PrivateChatMessageReaction {
// Metadata needs to be symmetric with TwoPartyPrivateChatMessage.
// MetaData is transient as it will be used indirectly by low level network classes. Only some low level network classes write the metaData to their protobuf representations.
private transient final MetaData metaData = new MetaData(TTL_30_DAYS, getClass().getSimpleName(), MAX_MAP_SIZE_100);
private transient final MetaData metaData = new MetaData(TTL_30_DAYS, LOW_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_10_000);
Comment on lines 36 to +38
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Inconsistent maxMapSize violates symmetry requirement.

The comment on Line 36 states "Metadata needs to be symmetric with TwoPartyPrivateChatMessage," but MAX_MAP_SIZE_10_000 is used here while the DEFAULT_MAX_MAP_SIZE_BY_CLASS_NAME in MetaData.java specifies MAX_MAP_SIZE_1000 for both TwoPartyPrivateChatMessage and TwoPartyPrivateChatMessageReaction. This 10x discrepancy breaks the intended symmetry.

Apply this diff to fix the inconsistency:

-    private transient final MetaData metaData = new MetaData(TTL_30_DAYS, LOW_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_10_000);
+    private transient final MetaData metaData = new MetaData(TTL_30_DAYS, LOW_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_1000);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Metadata needs to be symmetric with TwoPartyPrivateChatMessage.
// MetaData is transient as it will be used indirectly by low level network classes. Only some low level network classes write the metaData to their protobuf representations.
private transient final MetaData metaData = new MetaData(TTL_30_DAYS, getClass().getSimpleName(), MAX_MAP_SIZE_100);
private transient final MetaData metaData = new MetaData(TTL_30_DAYS, LOW_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_10_000);
// Metadata needs to be symmetric with TwoPartyPrivateChatMessage.
// MetaData is transient as it will be used indirectly by low level network classes. Only some low level network classes write the metaData to their protobuf representations.
private transient final MetaData metaData = new MetaData(TTL_30_DAYS, LOW_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_1000);
🤖 Prompt for AI Agents
In
chat/src/main/java/bisq/chat/reactions/TwoPartyPrivateChatMessageReaction.java
around lines 36-38, the MetaData is created with MAX_MAP_SIZE_10_000 which
breaks symmetry with TwoPartyPrivateChatMessage and
MetaData.DEFAULT_MAX_MAP_SIZE_BY_CLASS_NAME; replace MAX_MAP_SIZE_10_000 with
MAX_MAP_SIZE_1000 (or otherwise use the DEFAULT_MAX_MAP_SIZE_BY_CLASS_NAME
lookup for class defaults) so the maxMapSize matches the value used for
TwoPartyPrivateChatMessage.


public TwoPartyPrivateChatMessageReaction(String id,
UserProfile senderUserProfile,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
public final class TwoPartyPrivateChatMessage extends PrivateChatMessage<TwoPartyPrivateChatMessageReaction> {
// Metadata needs to be symmetric with TwoPartyPrivateChatMessageReaction.
// MetaData is transient as it will be used indirectly by low level network classes. Only some low level network classes write the metaData to their protobuf representations.
private transient final MetaData metaData = new MetaData(TTL_30_DAYS, getClass().getSimpleName(), MAX_MAP_SIZE_100);
private transient final MetaData metaData = new MetaData(TTL_30_DAYS, getClass().getSimpleName(), MAX_MAP_SIZE_1000);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify symmetry with TwoPartyPrivateChatMessageReaction.

The comment on line 45 explicitly states that metadata must be symmetric with TwoPartyPrivateChatMessageReaction. Ensure that the related class also uses MAX_MAP_SIZE_1000 to maintain this symmetry requirement.

Run the following script to verify the MetaData configuration in the related class:


🏁 Script executed:

#!/bin/bash
# Description: Verify TwoPartyPrivateChatMessageReaction uses MAX_MAP_SIZE_1000 for symmetry

# Search for MetaData initialization in TwoPartyPrivateChatMessageReaction
rg -n -A2 -B2 'class TwoPartyPrivateChatMessageReaction' --type java
rg -n 'new MetaData.*MAX_MAP_SIZE' chat/src/main/java/bisq/chat/reactions/TwoPartyPrivateChatMessageReaction.java

Length of output: 956


Fix asymmetric MetaData configuration between related classes.

Verification found that TwoPartyPrivateChatMessage.java (line 47) uses MAX_MAP_SIZE_1000, while TwoPartyPrivateChatMessageReaction.java (line 38) uses MAX_MAP_SIZE_10_000. Both classes have explicit comments requiring symmetry. Update one of these values to match the other and maintain the required symmetry.

🤖 Prompt for AI Agents
In chat/src/main/java/bisq/chat/two_party/TwoPartyPrivateChatMessage.java around
line 47, the MetaData is configured with MAX_MAP_SIZE_1000 which is asymmetric
with TwoPartyPrivateChatMessageReaction (which uses MAX_MAP_SIZE_10_000); update
the MetaData instantiation to use MAX_MAP_SIZE_10_000 so both related classes
use the same map size constant, keeping the rest of the constructor parameters
unchanged.


public TwoPartyPrivateChatMessage(String messageId,
ChatChannelDomain chatChannelDomain,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
@ToString
public final class AckMessage implements MailboxMessage, Response {
// MetaData is transient as it will be used indirectly by low level network classes. Only some low level network classes write the metaData to their protobuf representations.
private transient final MetaData metaData = new MetaData(TTL_2_DAYS, LOW_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_100);
private transient final MetaData metaData = new MetaData(TTL_2_DAYS, LOW_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_10_000);

private final String id;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@
import bisq.common.data.ByteArray;
import bisq.common.observable.collection.ObservableSet;
import bisq.network.p2p.services.data.DataRequest;
import bisq.network.p2p.services.data.storage.append.AddAppendOnlyDataRequest;
import bisq.network.p2p.services.data.storage.auth.AddAuthenticatedDataRequest;
import bisq.network.p2p.services.data.storage.auth.RefreshAuthenticatedDataRequest;
import bisq.network.p2p.services.data.storage.auth.RemoveAuthenticatedDataRequest;
import bisq.network.p2p.services.data.storage.mailbox.AddMailboxRequest;
import bisq.network.p2p.services.data.storage.mailbox.RemoveMailboxRequest;
import bisq.persistence.DbSubDirectory;
import bisq.persistence.Persistence;
import bisq.persistence.PersistenceService;
Expand All @@ -35,10 +29,12 @@
import lombok.extern.slf4j.Slf4j;

import java.io.File;
import java.util.Map;
import java.util.Optional;
import java.util.*;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;

import static bisq.network.p2p.services.data.storage.MetaData.MAX_MAP_SIZE_10_000;

@Slf4j
public abstract class DataStorageService<T extends DataRequest> extends RateLimitedPersistenceClient<DataStore<T>> {
public static final String STORE_POST_FIX = "Store";
Expand All @@ -48,7 +44,7 @@ public abstract class DataStorageService<T extends DataRequest> extends RateLimi
@Getter
protected final DataStore<T> persistableStore = new DataStore<>();
@Getter
private final String storeKey;
protected final String storeKey;
@Getter
protected final String subDirectory;
@Getter
Expand Down Expand Up @@ -80,81 +76,76 @@ public DataStore<T> prunePersisted(DataStore<T> persisted) {
}

int maxSize = getMaxMapSize();
Map<ByteArray, T> pruned = map.entrySet().stream()
int originalSize = map.size();
log.debug("Maybe pruning persisted data for {}: size={}, maxSize={}", storeKey, originalSize, maxSize);

Map<String, AtomicInteger> numEntriesByDataRequest = new TreeMap<>();
Map<ByteArray, T> newMap = map.entrySet().stream()
.filter(entry -> {
T dataRequest = entry.getValue();
boolean isExpired = dataRequest.isExpired();
if (isExpired) {
prunedAndExpiredDataRequests.add(dataRequest);
String dataRequestName = dataRequest.getClass().getSimpleName();
numEntriesByDataRequest
.computeIfAbsent(dataRequestName, k -> new AtomicInteger(0))
.incrementAndGet();
if (dataRequest.isExpired()) {
// log.debug("{} is expired (was created: {})", storeKey, new Date(dataRequest.getCreated()));
return false;
}
return !isExpired;
return true;
})
.sorted((o1, o2) -> Long.compare(o2.getValue().getCreated(), o1.getValue().getCreated()))
.limit(maxSize)
.sorted(Comparator.comparingLong((Map.Entry<ByteArray, T> e) -> e.getValue().getCreated()).reversed())
.limit(getMaxMapSize())
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
map.clear();
map.putAll(pruned);
return persisted;
}

protected int getMaxMapSize() {
if (maxMapSize.isEmpty()) {
int size = persistableStore.getMap().values().stream()
.map(DataRequest::getMaxMapSize)
.findFirst()
.orElse(100_000);
// Until the too low values in some MetaData are fixed we use 10000 as min size
maxMapSize = Optional.of(Math.max(MetaData.MAX_MAP_SIZE_10_000, size));
if (numEntriesByDataRequest.values().stream().mapToInt(AtomicInteger::get).sum() > 0) {
log.info("Add/Remove distribution for {}: {}", storeKey, numEntriesByDataRequest);
}
return maxMapSize.get();

// If no change, nothing to do
if (newMap.size() == originalSize) {
return persisted;
}

// Determine pruned/expired entries efficiently
Set<ByteArray> retainedKeys = newMap.keySet();
List<T> removedEntries = map.entrySet().stream()
.filter(e -> !retainedKeys.contains(e.getKey()))
.map(Map.Entry::getValue)
.toList();

prunedAndExpiredDataRequests.addAll(removedEntries);
log.info("Pruned {} entries for {}", removedEntries.size(), storeKey);

// Apply changes atomically
map.clear();
map.putAll(newMap);

// We do not call persist and delegate it to the event when the map gets updated at initial data requests.

return persisted;
}

protected boolean isExceedingMapSize() {
int size = persistableStore.getMap().size();
Map<ByteArray, T> map = persistableStore.getMap();
int size = map.size();
boolean isExceeding = size > getMaxMapSize();
if (isExceeding) {
String className = persistableStore.getMap().values().stream()
.findFirst()
.map(dataRequest -> {
if (dataRequest instanceof AddAuthenticatedDataRequest addRequest) {
return addRequest.getDistributedData().getClass().getSimpleName();
} else if (dataRequest instanceof RemoveAuthenticatedDataRequest removeRequest) {
return removeRequest.getClassName();
} else if (dataRequest instanceof RefreshAuthenticatedDataRequest request) {
return request.getClassName();
} else if (dataRequest instanceof AddAppendOnlyDataRequest addRequest) {
return addRequest.getAppendOnlyData().getClass().getSimpleName();
} else if (dataRequest instanceof AddMailboxRequest addRequest) {
return addRequest.getMailboxSequentialData().getMailboxData().getClassName();
} else if (dataRequest instanceof RemoveMailboxRequest removeRequest) {
return removeRequest.getClassName();
}
return "N/A";
}).orElse("N/A");
log.warn("Max. map size reached for {}. map.size()={}, getMaxMapSize={}",
className, size, getMaxMapSize());
log.debug("Max. map size reached for {}. map.size()={}, getMaxMapSize={}",
storeKey, size, getMaxMapSize());
}
if (size > 20_000) {
String className = persistableStore.getMap().values().stream()
.findFirst()
.map(dataRequest -> {
if (dataRequest instanceof AddAuthenticatedDataRequest addRequest) {
return addRequest.getDistributedData().getClass().getSimpleName();
} else if (dataRequest instanceof RemoveAuthenticatedDataRequest removeRequest) {
return removeRequest.getClassName();
} else if (dataRequest instanceof RefreshAuthenticatedDataRequest request) {
return request.getClassName();
} else if (dataRequest instanceof AddAppendOnlyDataRequest addRequest) {
return addRequest.getAppendOnlyData().getClass().getSimpleName();
} else if (dataRequest instanceof AddMailboxRequest addRequest) {
return addRequest.getMailboxSequentialData().getMailboxData().getClassName();
} else if (dataRequest instanceof RemoveMailboxRequest removeRequest) {
return removeRequest.getClassName();
}
return "N/A";
}).orElse("N/A");
log.info("Map size for {} reached > 20 000 entries. map.size()={}", className, size);
log.info("Map size for {} reached > 20 000 entries. map.size()={}", storeKey, size);
}
return isExceeding;
}

protected int getMaxMapSize() {
return Optional.ofNullable(MetaData.MAX_MAP_SIZE_BY_CLASS_NAME.get(storeKey))
.orElseGet(() -> Optional.ofNullable(MetaData.DEFAULT_MAX_MAP_SIZE_BY_CLASS_NAME.get(storeKey))
.orElseGet(() -> {
log.warn("We did not find the map size for {} and use the default value of {}", storeKey, MAX_MAP_SIZE_10_000);
return MAX_MAP_SIZE_10_000;
}));
}

}
Loading