Skip to content

Commit

Permalink
Merge pull request #2463 from HenrikJannsen/Avoid-that-currency-names…
Browse files Browse the repository at this point in the history
…-in-Market-and-TradeCurrency-can-cause-hash-conflicts

Avoid that currency names in market and trade currency can cause hash conflicts
  • Loading branch information
djing-chan authored Jul 22, 2024
2 parents 1b63760 + bbe64aa commit b5a336d
Show file tree
Hide file tree
Showing 17 changed files with 98 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public enum FiatPaymentRail implements PaymentRail {
.distinct()
.sorted(Comparator.comparingInt(TradeCurrency::hashCode))
.collect(Collectors.toList());
this.tradeCurrencies.sort(Comparator.comparing(TradeCurrency::getName));
this.tradeCurrencies.sort(Comparator.comparing(TradeCurrency::getDisplayName));

this.currencyCodes = currencyCodes != null ?
currencyCodes :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,13 @@
public abstract class PaymentMethod<R extends PaymentRail> implements Comparable<PaymentMethod<R>>, NetworkProto {
public final static int MAX_NAME_LENGTH = 50;

// Only name is used for protobuf, thus we do not need to mark the other fields with ExcludeForHash.
// Only name is used for protobuf, thus other fields are transient.
protected final String name;

// We do not persist the paymentRail but still include it in EqualsAndHashCode.
protected transient final R paymentRail;

@EqualsAndHashCode.Exclude
protected transient final String displayString;
@EqualsAndHashCode.Exclude
protected transient final String shortDisplayString;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public void onActivate() {
model.setMarketSearchTextPredicate(item ->
item != null &&
(item.getMarket().getQuoteCurrencyCode().toLowerCase().contains(search) ||
item.getMarket().getQuoteCurrencyName().toLowerCase().contains(search))
item.getMarket().getQuoteCurrencyDisplayName().toLowerCase().contains(search))
);
}
updateFilteredMarketChannelItems();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,7 @@
import javafx.geometry.Insets;
import javafx.geometry.Pos;
import javafx.scene.Cursor;
import javafx.scene.control.Label;
import javafx.scene.control.TableCell;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableRow;
import javafx.scene.control.Tooltip;
import javafx.scene.control.*;
import javafx.scene.image.ImageView;
import javafx.scene.layout.HBox;
import javafx.scene.layout.VBox;
Expand Down Expand Up @@ -111,13 +107,15 @@ protected void updateItem(MarketChannelItem item, boolean empty) {
}

if (item != null && !empty) {
marketName.setText(item.getMarket().getQuoteCurrencyName());
String quoteCurrencyDisplayName = item.getMarket().getQuoteCurrencyDisplayName();
marketName.setText(quoteCurrencyDisplayName);
marketCode.setText(item.getMarket().getQuoteCurrencyCode());
int numOffersString = item.getNumOffers().get();
StringExpression formattedNumOffers = Bindings.createStringBinding(() ->
BisqEasyOfferbookUtil.getFormattedOfferNumber(item.getNumOffers().get()), item.getNumOffers());
BisqEasyOfferbookUtil.getFormattedOfferNumber(numOffersString), item.getNumOffers());
numOffers.textProperty().bind(formattedNumOffers);
StringExpression formattedTooltip = Bindings.createStringBinding(() ->
BisqEasyOfferbookUtil.getFormattedTooltip(item.getNumOffers().get(), item.getMarket().getQuoteCurrencyName()), item.getNumOffers());
BisqEasyOfferbookUtil.getFormattedTooltip(numOffersString, quoteCurrencyDisplayName), item.getNumOffers());
marketDetailsTooltip.textProperty().bind(formattedTooltip);

// Set up new row
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ public void onActivate() {
String search = searchText.toLowerCase();
model.getFilteredList().setPredicate(item ->
item != null &&
(item.getQuoteCurrencyName().toLowerCase().contains(search) ||
item.getMarket().getQuoteCurrencyName().toLowerCase().contains(search))
(item.getQuoteCurrencyDisplayName().toLowerCase().contains(search) ||
item.getMarket().getQuoteCurrencyDisplayName().toLowerCase().contains(search))
);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ private void configTableView() {
tableView.getColumns().add(new BisqTableColumn.Builder<MarketListItem>()
.left()
.minWidth(120)
.comparator(Comparator.comparing(MarketListItem::getQuoteCurrencyName))
.comparator(Comparator.comparing(MarketListItem::getQuoteCurrencyDisplayName))
.setCellFactory(getNameCellFactory())
.build());
tableView.getColumns().add(new BisqTableColumn.Builder<MarketListItem>()
Expand Down Expand Up @@ -154,7 +154,7 @@ public void updateItem(final MarketListItem item, boolean empty) {

if (item != null && !empty) {
label.setGraphic(item.getMarketLogo());
String quoteCurrencyName = item.getQuoteCurrencyName();
String quoteCurrencyName = item.getQuoteCurrencyDisplayName();
label.setText(quoteCurrencyName);
if (quoteCurrencyName.length() > 20) {
Tooltip tooltip = new BisqTooltip(quoteCurrencyName);
Expand All @@ -176,7 +176,7 @@ public void updateItem(final MarketListItem item, boolean empty) {
@Getter
static class MarketListItem {
private final Market market;
private final String quoteCurrencyName;
private final String quoteCurrencyDisplayName;
private final int numOffersAsInteger;
private final int numUsersAsInteger;
private final String numOffers;
Expand All @@ -186,7 +186,7 @@ static class MarketListItem {

MarketListItem(Market market, int numOffersAsInteger, int numUsersAsInteger) {
this.market = market;
quoteCurrencyName = new FiatCurrency(market.getQuoteCurrencyCode()).getCodeAndName();
quoteCurrencyDisplayName = new FiatCurrency(market.getQuoteCurrencyCode()).getCodeAndDisplayName();
this.numOffers = String.valueOf(numOffersAsInteger);
this.numOffersAsInteger = numOffersAsInteger;
this.numUsers = String.valueOf(numUsersAsInteger);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ public final class AuthorizedBondedRole implements AuthorizedDistributedData {

// 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, HIGHEST_PRIORITY, getClass().getSimpleName(), MAX_MAP_SIZE_100);
@EqualsAndHashCode.Exclude
@ExcludeForHash
@EqualsAndHashCode.Exclude
private final int version;
private final String profileId;
private final String authorizedPublicKey;
Expand All @@ -59,6 +59,8 @@ public final class AuthorizedBondedRole implements AuthorizedDistributedData {
private final Optional<AddressByTransportTypeMap> addressByTransportTypeMap;
private final NetworkId networkId;
// The oracle node which did the validation and publishing
@ExcludeForHash(excludeOnlyInVersions = {1, 2, 3})
@EqualsAndHashCode.Exclude
private final Optional<AuthorizedOracleNode> authorizingOracleNode;

// ExcludeForHash from version 1 on to not treat data from different oracle nodes with different staticPublicKeysProvided value as duplicate data.
Expand Down
15 changes: 6 additions & 9 deletions common/src/main/java/bisq/common/currency/CryptoCurrency.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@
@EqualsAndHashCode(callSuper = true)
@ToString(callSuper = true)
public final class CryptoCurrency extends TradeCurrency {
// http://boschista.deviantart.com/journal/Cool-ASCII-Symbols-214218618
private final static String PREFIX = "✦ ";

public CryptoCurrency(String code, String name) {
super(code, name);
}
Expand All @@ -41,16 +38,16 @@ public bisq.common.protobuf.TradeCurrency toProto(boolean serializeForHash) {
return resolveProto(serializeForHash);
}

public static CryptoCurrency fromProto(bisq.common.protobuf.TradeCurrency baseProto, bisq.common.protobuf.CryptoCurrency proto) {
public static CryptoCurrency fromProto(bisq.common.protobuf.TradeCurrency baseProto) {
return new CryptoCurrency(baseProto.getCode(), baseProto.getName());
}

@Override
public String getDisplayPrefix() {
return PREFIX;
}

public boolean isFiat() {
return false;
}

@Override
public String getDisplayName() {
return name;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public class CryptoCurrencyRepository {
minorCurrencies = new ArrayList<>(currencyByCode.values());
minorCurrencies.remove(defaultCurrency);
minorCurrencies.removeAll(majorCurrencies);
minorCurrencies.sort(Comparator.comparing(TradeCurrency::getNameAndCode));
minorCurrencies.sort(Comparator.comparing(TradeCurrency::getDisplayNameAndCode));

allCurrencies = new ArrayList<>();
allCurrencies.add(defaultCurrency);
Expand Down
26 changes: 12 additions & 14 deletions common/src/main/java/bisq/common/currency/FiatCurrency.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,17 @@
@EqualsAndHashCode(callSuper = true)
@ToString(callSuper = true)
public final class FiatCurrency extends TradeCurrency {
// http://boschista.deviantart.com/journal/Cool-ASCII-Symbols-214218618
private final static String PREFIX = "★ ";

@Getter
// transient fields are excluded by default for EqualsAndHashCode
private transient final Currency currency;
private transient String displayName;

public FiatCurrency(String code) {
this(Currency.getInstance(code), LocaleRepository.getDefaultLocale());
}

public FiatCurrency(String code, Locale locale) {
this(Currency.getInstance(code), locale);
this(Currency.getInstance(code));
}

public FiatCurrency(Currency currency, Locale locale) {
super(currency.getCurrencyCode(), currency.getDisplayName(locale));

public FiatCurrency(Currency currency) {
super(currency.getCurrencyCode(), currency.getDisplayName(Locale.US));
this.currency = currency;
}

Expand All @@ -65,13 +58,18 @@ public bisq.common.protobuf.TradeCurrency toProto(boolean serializeForHash) {
return resolveProto(serializeForHash);
}

public static FiatCurrency fromProto(bisq.common.protobuf.TradeCurrency baseProto, bisq.common.protobuf.FiatCurrency proto) {
public static FiatCurrency fromProto(bisq.common.protobuf.TradeCurrency baseProto) {
return new FiatCurrency(baseProto.getCode(), baseProto.getName());
}

// The name field is the displayName using the US locale. For display purpose we use the name based on the user's locale.
@Override
public String getDisplayPrefix() {
return PREFIX;
public String getDisplayName() {
if (displayName == null) {
Locale defaultLocale = LocaleRepository.getDefaultLocale();
displayName = currency.getDisplayName(defaultLocale);
}
return displayName;
}

public boolean isFiat() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public static void setLocale(Locale locale) {
minorCurrencies = new ArrayList<>(currencyByCode.values());
minorCurrencies.remove(defaultCurrency);
minorCurrencies.removeAll(majorCurrencies);
minorCurrencies.sort(Comparator.comparing(TradeCurrency::getNameAndCode));
minorCurrencies.sort(Comparator.comparing(TradeCurrency::getDisplayNameAndCode));

allCurrencies = new ArrayList<>();
allCurrencies.add(defaultCurrency);
Expand All @@ -79,11 +79,13 @@ public static FiatCurrency getCurrencyByCountryCode(String countryCode) {

public static FiatCurrency getCurrencyByCountryCode(String countryCode, Locale locale) {
if (countryCode.equals("XK")) {
return new FiatCurrency("EUR", locale);
return new FiatCurrency("EUR");
}

Currency currency = Currency.getInstance(new Locale(locale.getLanguage(), countryCode));
return new FiatCurrency(currency.getCurrencyCode(), locale);
// The language and variant components of the locale at Currency.getInstance are ignored.
Locale countryLocale = new Locale(locale.getLanguage(), countryCode);
Currency currency = Currency.getInstance(countryLocale);
return new FiatCurrency(currency);
}

public static Map<String, FiatCurrency> getCurrencyByCodeMap() {
Expand All @@ -98,6 +100,10 @@ public static Optional<String> getName(String code) {
return Optional.ofNullable(currencyByCode.get(code)).map(TradeCurrency::getName);
}

public static Optional<String> getDisplayName(String code) {
return Optional.ofNullable(currencyByCode.get(code)).map(TradeCurrency::getDisplayName);
}

public static List<String> getAllFiatCurrencyCodes() {
return getAllCurrencies().stream().map(e -> e.getCurrency().getCurrencyCode()).collect(Collectors.toList());
}
Expand Down
26 changes: 21 additions & 5 deletions common/src/main/java/bisq/common/currency/Market.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ public final class Market implements NetworkProto, PersistableProto, Comparable<

private final String baseCurrencyCode;
private final String quoteCurrencyCode;

// The baseCurrencyName and quoteCurrencyName are using the US locale in case they are Fiat currencies, thus they
// are immutable with the code (therefor we don't need the ExcludeForHash annotation)
// For display purposes we use getQuoteCurrencyDisplayName() and getBaseCurrencyDisplayName()
@EqualsAndHashCode.Exclude
private final String baseCurrencyName;
@EqualsAndHashCode.Exclude
Expand Down Expand Up @@ -80,9 +84,21 @@ public static Market fromProto(bisq.common.protobuf.Market proto) {
proto.getQuoteCurrencyName());
}

public String getQuoteCurrencyDisplayName() {
return FiatCurrency.isFiat(quoteCurrencyCode)
? FiatCurrencyRepository.getDisplayName(quoteCurrencyCode).orElse(quoteCurrencyName)
: quoteCurrencyName;
}

public String getBaseCurrencyDisplayName() {
return FiatCurrency.isFiat(baseCurrencyCode)
? FiatCurrencyRepository.getDisplayName(baseCurrencyCode).orElse(baseCurrencyName)
: baseCurrencyName;
}

//todo (refactor, low prio) make static utils
public String getNonBitcoinCurrency() {
return isFiat() ? quoteCurrencyName : baseCurrencyName;
public String getFiatCurrencyName() {
return isFiat() ? getQuoteCurrencyDisplayName() : getBaseCurrencyDisplayName();
}

public boolean isFiat() {
Expand All @@ -93,13 +109,13 @@ public String getMarketCodes() {
return baseCurrencyCode + QUOTE_SEPARATOR + quoteCurrencyCode;
}

public String getMarketName() {
return baseCurrencyName + QUOTE_SEPARATOR + quoteCurrencyName;
public String getMarketDisplayName() {
return getBaseCurrencyDisplayName() + QUOTE_SEPARATOR + getQuoteCurrencyDisplayName();
}

@Override
public String toString() {
return getNonBitcoinCurrency() + " (" + getMarketCodes() + ")";
return getFiatCurrencyName() + " (" + getMarketCodes() + ")";
}

@Override
Expand Down
23 changes: 11 additions & 12 deletions common/src/main/java/bisq/common/currency/TradeCurrency.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package bisq.common.currency;

import bisq.common.annotation.ExcludeForHash;
import bisq.common.proto.PersistableProto;
import bisq.common.proto.UnresolvableProtobufMessageException;
import bisq.common.validation.NetworkDataValidation;
Expand All @@ -31,6 +32,7 @@ public abstract class TradeCurrency implements Comparable<TradeCurrency>, Persis
public final static int MAX_NAME_LENGTH = 100;

protected final String code;
@ExcludeForHash
@EqualsAndHashCode.Exclude
protected final String name;

Expand Down Expand Up @@ -61,37 +63,34 @@ public TradeCurrency(String code, String name) {
public bisq.common.protobuf.TradeCurrency.Builder getTradeCurrencyBuilder() {
return bisq.common.protobuf.TradeCurrency.newBuilder()
.setCode(code)
.setCode(name);
.setName(name);
}

public static TradeCurrency fromProto(bisq.common.protobuf.TradeCurrency proto) {
switch (proto.getMessageCase()) {
case CRYPTOCURRENCY:
return CryptoCurrency.fromProto(proto, proto.getCryptoCurrency());
return CryptoCurrency.fromProto(proto);
case FIATCURRENCY:
return FiatCurrency.fromProto(proto, proto.getFiatCurrency());
return FiatCurrency.fromProto(proto);
case MESSAGE_NOT_SET:
throw new UnresolvableProtobufMessageException(proto);
}
throw new UnresolvableProtobufMessageException(proto);
}

public abstract String getDisplayName();

public String getDisplayPrefix() {
return "";
public String getDisplayNameAndCode() {
return getDisplayName() + " (" + code + ")";
}

public String getNameAndCode() {
return name + " (" + code + ")";
}

public String getCodeAndName() {
return code + " (" + name + ")";
public String getCodeAndDisplayName() {
return code + " (" + getDisplayName() + ")";
}

@Override
public int compareTo(TradeCurrency other) {
return this.name.compareTo(other.name);
return this.getDisplayName().compareTo(other.getDisplayName());
}

public abstract boolean isFiat();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ public DataStorageResult add(AddAuthenticatedDataRequest request) {
// If we had already the data (only updated seq nr) we return true to broadcast the message but do not
// notify listeners as data has not changed.
if (requestFromMap != null) {
log.warn("requestFromMap != null. request={}", request);
return new DataStorageResult(true).payloadAlreadyStored();
}
}
Expand Down
Loading

0 comments on commit b5a336d

Please sign in to comment.