Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove stale prices that have not been updated for over 10 minutes. #36

Merged
merged 2 commits into from Oct 30, 2023
Merged
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
6 changes: 5 additions & 1 deletion src/main/java/bisq/price/PriceProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ public final T get() {
return cachedResult;
}

public final void put(T values) {
cachedResult = values;
}

@Override
public final void start() {
// do the initial refresh asynchronously
Expand Down Expand Up @@ -80,7 +84,7 @@ public void run() {
private void refresh() {
long ts = System.currentTimeMillis();

cachedResult = doGet();
put(doGet());

log.info("refresh took {} ms.", (System.currentTimeMillis() - ts));

Expand Down
6 changes: 2 additions & 4 deletions src/main/java/bisq/price/spot/ArsBlueRateTransformer.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,12 @@

import bisq.price.spot.providers.BlueRateProvider;
import bisq.price.util.bluelytics.ArsBlueMarketGapProvider;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Component;

import java.util.Optional;
import java.util.OptionalDouble;

@Component
@Slf4j
public class ArsBlueRateTransformer implements ExchangeRateTransformer {
private final ArsBlueMarketGapProvider blueMarketGapProvider;

Expand Down Expand Up @@ -54,8 +52,8 @@ public Optional<ExchangeRate> apply(ExchangeRateProvider provider, ExchangeRate
originalExchangeRate.getProvider()
);

log.info(String.format("%s transformed from %s to %s",
originalExchangeRate.getCurrency(), originalExchangeRate.getPrice(), blueRate));
provider.getGatedLogging().maybeLogInfo(String.format("%s transformed from %s to %s",
originalExchangeRate.getCurrency(), originalExchangeRate.getPrice(), blueRate));

return Optional.of(newExchangeRate);
}
Expand Down
23 changes: 22 additions & 1 deletion src/main/java/bisq/price/spot/ExchangeRateProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import bisq.core.locale.CurrencyUtil;
import bisq.core.locale.TradeCurrency;
import bisq.price.PriceProvider;
import bisq.price.util.GatedLogging;
import lombok.Getter;
import org.knowm.xchange.Exchange;
import org.knowm.xchange.ExchangeFactory;
import org.knowm.xchange.currency.Currency;
Expand All @@ -32,12 +34,12 @@
import org.knowm.xchange.service.marketdata.params.Params;
import org.springframework.core.env.Environment;

import javax.annotation.Nullable;
import java.io.IOException;
import java.math.BigDecimal;
import java.math.RoundingMode;
import java.time.Duration;
import java.util.*;
import java.util.concurrent.TimeUnit;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand All @@ -53,12 +55,15 @@
*/
public abstract class ExchangeRateProvider extends PriceProvider<Set<ExchangeRate>> {

private static final long STALE_PRICE_INTERVAL_MILLIS = TimeUnit.MINUTES.toMillis(10);
private static Set<String> SUPPORTED_CRYPTO_CURRENCIES = new HashSet<>();
private static Set<String> SUPPORTED_FIAT_CURRENCIES = new HashSet<>();
private final Set<String> providerExclusionList = new HashSet<>();
private final String name;
private final String prefix;
private final Environment env;
@Getter
private final GatedLogging gatedLogging = new GatedLogging();

public ExchangeRateProvider(Environment env, String name, String prefix, Duration refreshInterval) {
super(refreshInterval);
Expand Down Expand Up @@ -128,6 +133,22 @@ public String getPrefix() {
return prefix;
}

public void maybeClearStaleRates() {
// a stale rate is older than the specified interval, except:
// timestamp of 0L is used as special case re: CoinMarketCap and BitcoinAverage
// (https://github.com/bisq-network/bisq-pricenode/issues/23)
long staleTimestamp = new Date().getTime() - STALE_PRICE_INTERVAL_MILLIS;
Set<ExchangeRate> nonStaleRates = get().stream()
.filter(e -> e.getTimestamp() == 0L || e.getTimestamp() > staleTimestamp)
.collect(Collectors.toSet());
long numberOriginalRates = get().size();
if (numberOriginalRates > nonStaleRates.size()) {
put(nonStaleRates);
log.warn("{} {} stale rates removed, now {} rates",
getName(), numberOriginalRates, nonStaleRates.size());
}
}

@Override
protected void onRefresh() {
get().stream()
Expand Down
36 changes: 21 additions & 15 deletions src/main/java/bisq/price/spot/ExchangeRateService.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

import bisq.common.util.Tuple2;
import bisq.core.util.InlierUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import bisq.price.util.GatedLogging;
import lombok.extern.slf4j.Slf4j;
import org.springframework.core.env.Environment;
import org.springframework.stereotype.Service;

Expand All @@ -32,11 +32,12 @@
* High-level {@link ExchangeRate} data operations.
*/
@Service
@Slf4j
class ExchangeRateService {
protected final Logger log = LoggerFactory.getLogger(this.getClass());
private final Environment env;
private final List<ExchangeRateProvider> providers;
private final List<ExchangeRateTransformer> transformers;
private final GatedLogging gatedLogging = new GatedLogging();

/**
* Construct an {@link ExchangeRateService} with a list of all
Expand All @@ -59,6 +60,7 @@ public Map<String, Object> getAllMarketPrices() {
Map<String, ExchangeRate> aggregateExchangeRates = getAggregateExchangeRates();

providers.forEach(p -> {
p.maybeClearStaleRates();
// Specific metadata fields for specific providers are expected by the client,
// mostly for historical reasons
// Therefore, add metadata fields for all known providers
Expand Down Expand Up @@ -87,6 +89,7 @@ public Map<String, Object> getAllMarketPrices() {
* by currency code
*/
private Map<String, ExchangeRate> getAggregateExchangeRates() {
boolean maybeLogDetails = gatedLogging.gatingOperation();
Map<String, ExchangeRate> aggregateExchangeRates = new HashMap<>();

// Query all providers and collect all exchange rates, grouped by currency code
Expand All @@ -110,7 +113,7 @@ private Map<String, ExchangeRate> getAggregateExchangeRates() {
} else {
// If multiple providers have rates for this currency, then
// aggregate = average of the rates
double priceAvg = priceAverageWithOutliersRemoved(exchangeRateList, currencyCode);
double priceAvg = priceAverageWithOutliersRemoved(exchangeRateList, currencyCode, maybeLogDetails);
aggregateExchangeRate = new ExchangeRate(
currencyCode,
BigDecimal.valueOf(priceAvg),
Expand All @@ -123,7 +126,8 @@ private Map<String, ExchangeRate> getAggregateExchangeRates() {
return aggregateExchangeRates;
}

private double priceAverageWithOutliersRemoved(List<ExchangeRate> exchangeRateList, String contextInfo) {
private double priceAverageWithOutliersRemoved(
List<ExchangeRate> exchangeRateList, String contextInfo, boolean logOutliers) {
final List<Double> yValues = exchangeRateList.stream().
mapToDouble(ExchangeRate::getPrice).boxed().collect(Collectors.toList());
Tuple2<Double, Double> tuple = InlierUtil.findInlierRange(yValues, 0, getOutlierStdDeviation());
Expand All @@ -145,16 +149,18 @@ private double priceAverageWithOutliersRemoved(List<ExchangeRate> exchangeRateLi
double priceAvg = opt.orElseThrow(IllegalStateException::new);

// log the outlier prices which were removed from the average, if any.
for (ExchangeRate badRate : exchangeRateList.stream()
.filter(e -> !filteredPrices.contains(e))
.collect(Collectors.toList())) {
log.info("{} {} outlier price removed:{}, lower/upper bounds:{}/{}, consensus price:{}",
badRate.getProvider(),
badRate.getCurrency(),
badRate.getPrice(),
lowerBound,
upperBound,
priceAvg);
if (logOutliers) {
for (ExchangeRate badRate : exchangeRateList.stream()
.filter(e -> !filteredPrices.contains(e))
.collect(Collectors.toList())) {
log.info("{} {} outlier price removed:{}, lower/upper bounds:{}/{}, consensus price:{}",
badRate.getProvider(),
badRate.getCurrency(),
badRate.getPrice(),
lowerBound,
upperBound,
priceAvg);
}
}
return priceAvg;
}
Expand Down
46 changes: 46 additions & 0 deletions src/main/java/bisq/price/util/GatedLogging.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* 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 <http://www.gnu.org/licenses/>.
*/

package bisq.price.util;

import lombok.extern.slf4j.Slf4j;

import java.time.Instant;

/* Per https://github.com/bisq-network/bisq-pricenode/issues/33
* There's too much logging of outlier filtering data, fills up the logs too fast and obliterates other valid logging.
* It correlates with client requests. Change that logging so its output once per minute.
*/

@Slf4j
public class GatedLogging {
private long timestampOfLastLogMessage = 0;

public void maybeLogInfo(String format, Object... params) {
if (gatingOperation()) {
log.info(format, params);
}
}

public boolean gatingOperation() {
if (Instant.now().getEpochSecond() - timestampOfLastLogMessage > 60) {
timestampOfLastLogMessage = Instant.now().getEpochSecond();
return true;
}
return false;
}
}
34 changes: 31 additions & 3 deletions src/test/java/bisq/price/spot/ExchangeRateServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,28 @@ public void getAllMarketPrices_withMultipleProviders_overlappingCurrencyCodes()
assertNotEquals(0L, retrievedData.get(providers.get(1).getPrefix() + "Ts"));
}

@Test
public void testStaleRatesRemoved() {
String fiatCoin = "BRL";
// insert a stale rate
long staleTimestamp = 10000L;
Long validTimestamp = System.currentTimeMillis();
List<ExchangeRateProvider> providers = asList(
buildDummyExchangeRateProviderWithRateAndTimestamp("mercadoBitcoin", fiatCoin, 129000.0, staleTimestamp),
buildDummyExchangeRateProviderWithRateAndTimestamp("coinGecko", fiatCoin, 129000.0, validTimestamp),
buildDummyExchangeRateProviderWithRateAndTimestamp("binance", fiatCoin, 131000.0, validTimestamp));
Map<String, Object> retrievedData = new ExchangeRateService(new StandardEnvironment(), providers, Collections.emptyList()).getAllMarketPrices();
doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers);
// check that the provider's stale rate was removed resulting in 0 rates and a 0 timestamp
assertEquals("0", retrievedData.get("mercadoBitcoinTs").toString());
assertEquals("0", retrievedData.get("mercadoBitcoinCount").toString());
// check that other providers are unaffected
assertEquals("1", retrievedData.get("coinGeckoCount").toString());
assertEquals("1", retrievedData.get("binanceCount").toString());
assertEquals(validTimestamp.toString(), retrievedData.get("coinGeckoTs").toString());
assertEquals(validTimestamp.toString(), retrievedData.get("binanceTs").toString());
}

@Test
public void bisqIndexCalculation_oneOutlierPriceWideRange() {
String fiatCoin = "BRL";
Expand Down Expand Up @@ -517,7 +539,13 @@ protected Set<ExchangeRate> doGet() {
return dummyProvider;
}

private ExchangeRateProvider buildDummyExchangeRateProviderWithRate(String providerName, String currencyCode, Double dummyRate) {
private ExchangeRateProvider buildDummyExchangeRateProviderWithRate(
String providerName, String currencyCode, Double rate) {
return buildDummyExchangeRateProviderWithRateAndTimestamp(
providerName, currencyCode, rate, System.currentTimeMillis());
}
private ExchangeRateProvider buildDummyExchangeRateProviderWithRateAndTimestamp(
String providerName, String currencyCode, Double rate, long timestamp) {
ExchangeRateProvider dummyProvider = new ExchangeRateProvider(
new StandardEnvironment(),
providerName,
Expand All @@ -534,8 +562,8 @@ protected Set<ExchangeRate> doGet() {
HashSet<ExchangeRate> exchangeRates = new HashSet<>();
exchangeRates.add(new ExchangeRate(
currencyCode,
dummyRate,
System.currentTimeMillis(),
rate,
timestamp,
getName())); // ExchangeRateProvider name
return exchangeRates;
}
Expand Down