From 1d1b63d000fd8b248d8365721d8efbd551a393a5 Mon Sep 17 00:00:00 2001 From: jmacxx <47253594+jmacxx@users.noreply.github.com> Date: Mon, 11 Sep 2023 20:21:51 -0500 Subject: [PATCH 1/2] Switch outlier price filtering to 1.1 standard deviation. Set outlier filtering to 1.1 standard deviation (was/currently 2.2). Refactor the outlier calculation code, improving layout and logging. Add 4 new tests for outlier filtering. Refactor the exchange service tests. --- .../bisq/price/spot/ExchangeRateService.java | 54 ++-- src/main/resources/application.properties | 2 +- .../price/spot/ExchangeRateServiceTest.java | 260 +++++++++++++----- 3 files changed, 224 insertions(+), 92 deletions(-) diff --git a/src/main/java/bisq/price/spot/ExchangeRateService.java b/src/main/java/bisq/price/spot/ExchangeRateService.java index 7b4fcbc..a7c0c93 100644 --- a/src/main/java/bisq/price/spot/ExchangeRateService.java +++ b/src/main/java/bisq/price/spot/ExchangeRateService.java @@ -110,26 +110,12 @@ private Map getAggregateExchangeRates() { } else { // If multiple providers have rates for this currency, then // aggregate = average of the rates - List goodPriceList = removeOutliers(exchangeRateList.stream(). - mapToDouble(ExchangeRate::getPrice).boxed().collect(Collectors.toList()), currencyCode); - OptionalDouble opt = goodPriceList.stream().mapToDouble(Double::doubleValue).average(); - // List size > 1, so opt is always set - double priceAvg = opt.orElseThrow(IllegalStateException::new); + double priceAvg = priceAverageWithOutliersRemoved(exchangeRateList, currencyCode); aggregateExchangeRate = new ExchangeRate( currencyCode, BigDecimal.valueOf(priceAvg), new Date(), // timestamp = time when avg is calculated "Bisq-Aggregate"); - // log the outlier prices which were removed from the average, if any. - for (ExchangeRate badRate : exchangeRateList.stream() - .filter(e -> !goodPriceList.contains(e.getPrice())) - .collect(Collectors.toList())) { - log.warn("outlier price removed={}, source={}, ccy={}, consensus price={}", - badRate.getPrice(), - badRate.getProvider(), - currencyCode, - aggregateExchangeRate.getPrice()); - } } aggregateExchangeRates.put(aggregateExchangeRate.getCurrency(), aggregateExchangeRate); }); @@ -137,24 +123,44 @@ private Map getAggregateExchangeRates() { return aggregateExchangeRates; } - private List removeOutliers(List yValues, String contextInfo) { + private double priceAverageWithOutliersRemoved(List exchangeRateList, String contextInfo) { + final List yValues = exchangeRateList.stream(). + mapToDouble(ExchangeRate::getPrice).boxed().collect(Collectors.toList()); Tuple2 tuple = InlierUtil.findInlierRange(yValues, 0, getOutlierStdDeviation()); double lowerBound = tuple.first; double upperBound = tuple.second; - List filteredPrices = yValues.stream() - .filter(e -> e >= lowerBound) - .filter(e -> e <= upperBound) + final List filteredPrices = exchangeRateList.stream() + .filter(e -> e.getPrice() >= lowerBound) + .filter(e -> e.getPrice() <= upperBound) .collect(Collectors.toList()); + if (filteredPrices.size() < 1) { - log.error("{}: no results after outliers removed. lowerBound={}, upperBound={}, stdDev={}, yValues={}", - contextInfo, lowerBound, upperBound, getOutlierStdDeviation(), yValues.toString()); - return yValues; // all prices cannot be removed, so revert to keep service running + log.error("{}: could not filter, revert to plain average. lowerBound={}, upperBound={}, stdDev={}, yValues={}", + contextInfo, lowerBound, upperBound, getOutlierStdDeviation(), yValues); + return exchangeRateList.stream().mapToDouble(ExchangeRate::getPrice).average().getAsDouble(); + } + + OptionalDouble opt = filteredPrices.stream().mapToDouble(ExchangeRate::getPrice).average(); + // List size > 1, so opt is always set + 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); } - return filteredPrices; + return priceAvg; } private double getOutlierStdDeviation() { - return Double.parseDouble(env.getProperty("bisq.price.outlierStdDeviation", "2.2")); + return Double.parseDouble(env.getProperty("bisq.price.outlierStdDeviation", "1.1")); } /** diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 399ab37..a6da999 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -10,4 +10,4 @@ bisq.price.mining.providers.mempoolHostname.4=mempool.bisq.services bisq.price.fiatcurrency.excluded=LBP bisq.price.fiatcurrency.excludedByProvider=HUOBI:BRL bisq.price.cryptocurrency.excluded= -bisq.price.outlierStdDeviation=2.2 +bisq.price.outlierStdDeviation=1.1 diff --git a/src/test/java/bisq/price/spot/ExchangeRateServiceTest.java b/src/test/java/bisq/price/spot/ExchangeRateServiceTest.java index c9ff1c7..7112e38 100644 --- a/src/test/java/bisq/price/spot/ExchangeRateServiceTest.java +++ b/src/test/java/bisq/price/spot/ExchangeRateServiceTest.java @@ -70,12 +70,9 @@ static void setup() { public void getAllMarketPrices_withNoExchangeRates_logs_Exception() { int numberOfCurrencyPairsOnExchange = 0; ExchangeRateProvider dummyProvider = buildDummyExchangeRateProvider(numberOfCurrencyPairsOnExchange); - ExchangeRateService service = new ExchangeRateService(new StandardEnvironment(), - Collections.singletonList(dummyProvider), Collections.emptyList()); - - Map retrievedData = service.getAllMarketPrices(); - - doSanityChecksForRetrievedDataSingleProvider(service, retrievedData, dummyProvider, numberOfCurrencyPairsOnExchange); + Map retrievedData = new ExchangeRateService( + new StandardEnvironment(), Collections.singletonList(dummyProvider)).getAllMarketPrices(); + doSanityChecksForRetrievedDataSingleProvider(retrievedData, dummyProvider, numberOfCurrencyPairsOnExchange); // No exchange rates provided by this exchange, two things should happen // A) the timestamp should be set to 0 @@ -89,20 +86,19 @@ public void getAllMarketPrices_withNoExchangeRates_logs_Exception() { // Log msg has the format: java.lang.IllegalStateException: No exchange rate data // found for ExchangeName-JzfP1 List logsList = ((ListAppender) exchangeRateServiceLogger.getAppender(LIST_APPENDER_NAME)).list; - assertEquals(Level.ERROR, logsList.get(0).getLevel()); - assertTrue(logsList.get(0).getMessage().endsWith("No exchange rate data found for " + dummyProvider.getName())); + logsList.stream().reduce((first, second) -> second).ifPresentOrElse(loggingEvent -> { + assertEquals(Level.ERROR, loggingEvent.getLevel()); + assertTrue(loggingEvent.getMessage().endsWith("No exchange rate data found for " + dummyProvider.getName())); + }, () -> fail("Expected exception log message not found")); } @Test public void getAllMarketPrices_withSingleExchangeRate() { int numberOfCurrencyPairsOnExchange = 1; ExchangeRateProvider dummyProvider = buildDummyExchangeRateProvider(numberOfCurrencyPairsOnExchange); - ExchangeRateService service = new ExchangeRateService(new StandardEnvironment(), - Collections.singletonList(dummyProvider), Collections.emptyList()); - - Map retrievedData = service.getAllMarketPrices(); - - doSanityChecksForRetrievedDataSingleProvider(service, retrievedData, dummyProvider, numberOfCurrencyPairsOnExchange); + Map retrievedData = new ExchangeRateService( + new StandardEnvironment(), Collections.singletonList(dummyProvider)).getAllMarketPrices(); + doSanityChecksForRetrievedDataSingleProvider(retrievedData, dummyProvider, numberOfCurrencyPairsOnExchange); // One rate was provided by this provider, so the timestamp should not be 0 assertNotEquals(0L, retrievedData.get(dummyProvider.getPrefix() + "Ts")); @@ -111,19 +107,18 @@ public void getAllMarketPrices_withSingleExchangeRate() { @Test public void getAllMarketPrices_withMultipleProviders_differentCurrencyCodes() { int numberOfCurrencyPairsOnExchange = 1; - ExchangeRateProvider dummyProvider1 = buildDummyExchangeRateProvider(numberOfCurrencyPairsOnExchange); - ExchangeRateProvider dummyProvider2 = buildDummyExchangeRateProvider(numberOfCurrencyPairsOnExchange); - ExchangeRateService service = new ExchangeRateService(new StandardEnvironment(), - asList(dummyProvider1, dummyProvider2), Collections.emptyList()); + List providers = asList( + buildDummyExchangeRateProvider(numberOfCurrencyPairsOnExchange), + buildDummyExchangeRateProvider(numberOfCurrencyPairsOnExchange)); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); - Map retrievedData = service.getAllMarketPrices(); - - doSanityChecksForRetrievedDataMultipleProviders(service, retrievedData, asList(dummyProvider1, dummyProvider2)); + doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); + checkBisqIndexCalculationNoOutliers(validateAndGetRetrievedRates(retrievedData), providers); // One rate was provided by each provider in this service, so the timestamp // (for both providers) should not be 0 - assertNotEquals(0L, retrievedData.get(dummyProvider1.getPrefix() + "Ts")); - assertNotEquals(0L, retrievedData.get(dummyProvider2.getPrefix() + "Ts")); + assertNotEquals(0L, retrievedData.get(providers.get(0).getPrefix() + "Ts")); + assertNotEquals(0L, retrievedData.get(providers.get(1).getPrefix() + "Ts")); } /** @@ -157,20 +152,96 @@ public void getAllMarketPrices_withMultipleProviders_overlappingCurrencyCodes() Set rateCurrencyCodes = Sets.newHashSet("CURRENCY-1", "CURRENCY-2", "CURRENCY-3"); // Create several dummy providers, each providing their own rates for the same set of currencies - ExchangeRateProvider dummyProvider1 = buildDummyExchangeRateProvider(rateCurrencyCodes, null); - ExchangeRateProvider dummyProvider2 = buildDummyExchangeRateProvider(rateCurrencyCodes, null); + List providers = asList( + buildDummyExchangeRateProvider(rateCurrencyCodes), + buildDummyExchangeRateProvider(rateCurrencyCodes)); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); + checkBisqIndexCalculationNoOutliers(validateAndGetRetrievedRates(retrievedData), providers); - ExchangeRateService service = new ExchangeRateService(new StandardEnvironment(), - asList(dummyProvider1, dummyProvider2), Collections.emptyList()); + // At least one rate was provided by each provider in this service, so the + // timestamp (for both providers) should not be 0 + assertNotEquals(0L, retrievedData.get(providers.get(0).getPrefix() + "Ts")); + assertNotEquals(0L, retrievedData.get(providers.get(1).getPrefix() + "Ts")); + } - Map retrievedData = service.getAllMarketPrices(); + @Test + public void bisqIndexCalculation_oneOutlierPriceWideRange() { + String fiatCoin = "BRL"; + List providers = asList( + buildDummyExchangeRateProviderWithRate("mercadoBitcoin", fiatCoin, 0.0), // outlier - low + buildDummyExchangeRateProviderWithRate("coinGecko", fiatCoin, 129000.0), + buildDummyExchangeRateProviderWithRate("binance", fiatCoin, 131000.0)); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); + checkBisqIndexCalculationWithOutlier(validateAndGetRetrievedRates(retrievedData), providers, Collections.singletonList("mercadoBitcoin")); + } - doSanityChecksForRetrievedDataMultipleProviders(service, retrievedData, asList(dummyProvider1, dummyProvider2)); + @Test + public void bisqIndexCalculation_oneOutlierPriceCloseRange() { + String fiatCoin = "BRL"; + List providers = asList( + buildDummyExchangeRateProviderWithRate("mercadoBitcoin", fiatCoin, 124545.5), + buildDummyExchangeRateProviderWithRate("coinGecko", fiatCoin, 124083.752), + buildDummyExchangeRateProviderWithRate("binance", fiatCoin, 124726.0)); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); + checkBisqIndexCalculationWithOutlier(validateAndGetRetrievedRates(retrievedData), providers, Collections.singletonList("coinGecko")); + } - // At least one rate was provided by each provider in this service, so the - // timestamp (for both providers) should not be 0 - assertNotEquals(0L, retrievedData.get(dummyProvider1.getPrefix() + "Ts")); - assertNotEquals(0L, retrievedData.get(dummyProvider2.getPrefix() + "Ts")); + @Test + public void bisqIndexCalculation_twoOutlierPrices() { + String fiatCoin = "GBP"; + List providers = asList( + buildDummyExchangeRateProviderWithRate("bitstamp", fiatCoin, 20788.0), + buildDummyExchangeRateProviderWithRate("bitfinex", fiatCoin, 19780.0), // outlier - low + buildDummyExchangeRateProviderWithRate("kraken", fiatCoin, 20795.0), + buildDummyExchangeRateProviderWithRate("coinGecko", fiatCoin, 20774.0), + buildDummyExchangeRateProviderWithRate("binance", fiatCoin, 21361.48), // outlier - high + buildDummyExchangeRateProviderWithRate("coinbasePro", fiatCoin, 20798.25)); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); + checkBisqIndexCalculationWithOutlier(validateAndGetRetrievedRates(retrievedData), providers, List.of("binance", "bitfinex")); + } + + @Test + public void bisqIndexCalculation_altcoinOutlierPrices() { + String altcoin = "ETH"; + List providers = asList( + buildDummyExchangeRateProviderWithRate("bitflyer", altcoin, 0.06262), + buildDummyExchangeRateProviderWithRate("bitstamp", altcoin, 0.0625939), + buildDummyExchangeRateProviderWithRate("kraken", altcoin, 0.06259), + buildDummyExchangeRateProviderWithRate("coinGecko", altcoin, 0.06264487), + buildDummyExchangeRateProviderWithRate("binance", altcoin, 0.05), // outlier - low + buildDummyExchangeRateProviderWithRate("poloniex", altcoin, 0.07), // outlier - high + buildDummyExchangeRateProviderWithRate("coinbasePro", altcoin, 0.06259)); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); + checkBisqIndexCalculationWithOutlier(validateAndGetRetrievedRates(retrievedData), providers, List.of("binance", "poloniex")); + } + + @Test + public void bisqIndexCalculation_allSamePricesAltcoin() { + String altcoin = "DOGE"; + List providers = asList( + buildDummyExchangeRateProviderWithRate("bitfinex", altcoin, 2.37E-6), + buildDummyExchangeRateProviderWithRate("kraken", altcoin, 2.37E-6), + buildDummyExchangeRateProviderWithRate("coinbasePro", altcoin, 2.37E-6)); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); + checkBisqIndexCalculationNoOutliers(validateAndGetRetrievedRates(retrievedData), providers); + } + + @Test + public void bisqIndexCalculation_allSamePricesFiat() { + String fiat = "USD"; + List providers = asList( + buildDummyExchangeRateProviderWithRate("bitfinex", fiat, 10000.0), + buildDummyExchangeRateProviderWithRate("kraken", fiat, 10000.0), + buildDummyExchangeRateProviderWithRate("coinbasePro", fiat, 10000.0)); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); + checkBisqIndexCalculationNoOutliers(validateAndGetRetrievedRates(retrievedData), providers); } /** @@ -260,8 +331,33 @@ private void doSanityChecksForRetrievedDataSingleProvider(ExchangeRateService se // Check that the amount of provided exchange rates matches expected value // For one provider, the amount of rates of that provider should be the total // amount of rates in the response - List retrievedMarketPricesData = (List) retrievedData.get("data"); - assertEquals(numberOfCurrencyPairsOnExchange, retrievedMarketPricesData.size()); + List retrievedRates = (List) retrievedData.get("data"); + assertEquals(numberOfCurrencyPairsOnExchange, retrievedRates.size()); + } + + + private List validateAndGetRetrievedRates(Map retrievedData) { + // Check validity of the data field + List retrievedRates = (List) retrievedData.get("data"); + assertNotNull(retrievedRates); + + // It should contain no duplicate ExchangeRate objects + int uniqueRates = Sets.newHashSet(retrievedRates).size(); + int totalRates = retrievedRates.size(); + assertEquals(uniqueRates, totalRates, "Found duplicate rates in data field"); + + // There should be only one ExchangeRate per currency + // In other words, even if multiple providers return rates for the same currency, + // the ExchangeRateService should expose only one (aggregate) ExchangeRate for + // that currency + Map currencyCodeToExchangeRateFromService = retrievedRates.stream() + .collect(Collectors.toMap( + ExchangeRate::getCurrency, exchangeRate -> exchangeRate + )); + int uniqueCurrencyCodes = currencyCodeToExchangeRateFromService.keySet().size(); + assertEquals(uniqueCurrencyCodes, uniqueRates, "Found currency code with multiple exchange rates"); + + return retrievedRates; } /** @@ -285,28 +381,27 @@ private void doSanityChecksForRetrievedDataMultipleProviders(ExchangeRateService assertNotNull(retrievedData.get(providerPrefix + "Ts")); assertNotNull(retrievedData.get(providerPrefix + "Count")); } + } - // Check validity of the data field - List retrievedRates = (List) retrievedData.get("data"); - assertNotNull(retrievedRates); + private void checkBisqIndexCalculationNoOutliers(List retrievedData, + List providers) { + checkBisqIndexCalculation(retrievedData, providers, Collections.emptyList()); + } - // It should contain no duplicate ExchangeRate objects - int uniqueRates = Sets.newHashSet(retrievedRates).size(); - int totalRates = retrievedRates.size(); - assertEquals(uniqueRates, totalRates, "Found duplicate rates in data field"); + private void checkBisqIndexCalculationWithOutlier(List retrievedData, + List providers, + List outlierProviderNames) { + checkBisqIndexCalculation(retrievedData, providers, outlierProviderNames); + } - // There should be only one ExchangeRate per currency - // In other words, even if multiple providers return rates for the same currency, - // the ExchangeRateService should expose only one (aggregate) ExchangeRate for - // that currency - Map currencyCodeToExchangeRateFromService = retrievedRates.stream() + // checks that the bisq index equals the average of all retrieved quotes + private void checkBisqIndexCalculation(List retrievedData, + List providers, + List optionalOutlierProviderNames) { + Map currencyCodeToExchangeRateFromService = retrievedData.stream() .collect(Collectors.toMap( ExchangeRate::getCurrency, exchangeRate -> exchangeRate )); - int uniqueCurrencyCodes = currencyCodeToExchangeRateFromService.keySet().size(); - assertEquals(uniqueCurrencyCodes, uniqueRates, "Found currency code with multiple exchange rates"); - - // Collect all ExchangeRates from all providers and group them by currency code Map> currencyCodeToExchangeRatesFromProviders = new HashMap<>(); for (ExchangeRateProvider p : providers) { for (ExchangeRate exchangeRate : p.get()) { @@ -316,31 +411,28 @@ private void doSanityChecksForRetrievedDataMultipleProviders(ExchangeRateService l.add(exchangeRate); currencyCodeToExchangeRatesFromProviders.put(currencyCode, l); } else { - currencyCodeToExchangeRatesFromProviders.put(currencyCode, asList(exchangeRate)); + currencyCodeToExchangeRatesFromProviders.put(currencyCode, Collections.singletonList(exchangeRate)); } } } - // For each ExchangeRate which is covered by multiple providers, ensure the rate // value is an average currencyCodeToExchangeRatesFromProviders.forEach((currencyCode, exchangeRateList) -> { - ExchangeRate rateFromService = currencyCodeToExchangeRateFromService.get(currencyCode); - if (rateFromService != null) { - double priceFromService = rateFromService.getPrice(); - - OptionalDouble opt = exchangeRateList.stream().mapToDouble(ExchangeRate::getPrice).average(); - double priceAvgFromProviders = opt.getAsDouble(); - - // Ensure that the ExchangeRateService correctly aggregates exchange rates - // from multiple providers. If multiple providers contain rates for a - // currency, the service should return a single aggregate rate - // Expected value for aggregate rate = avg(provider rates) - // This formula works for any number of providers for a specific currency - assertEquals(priceFromService, priceAvgFromProviders, "Service returned incorrect aggregate rate"); - } + ExchangeRate actualBisqIndex = currencyCodeToExchangeRateFromService.get(currencyCode); + OptionalDouble testBisqIndex = exchangeRateList.stream() + .filter(e -> !optionalOutlierProviderNames.contains(e.getProvider())) + .mapToDouble(ExchangeRate::getPrice) + .average(); + // Ensure that the ExchangeRateService correctly aggregates exchange rates + // from multiple providers. If multiple providers contain rates for a + // currency, the service should return a single aggregate rate + // Expected value for aggregate rate = avg(provider rates) + // This formula works for any number of providers for a specific currency + assertEquals(testBisqIndex.getAsDouble(), actualBisqIndex.getPrice(), "Service returned incorrect aggregate rate"); }); } + /** * @param numberOfRatesAvailable Number of exchange rates this provider returns * @return Dummy {@link ExchangeRateProvider} providing rates for @@ -427,6 +519,40 @@ protected Set doGet() { return dummyProvider; } + private ExchangeRateProvider buildDummyExchangeRateProviderWithRate(String providerName, String currencyCode, Double dummyRate) { + ExchangeRateProvider dummyProvider = new ExchangeRateProvider( + new StandardEnvironment(), + providerName, + providerName, + Duration.ofDays(1)) { + + @Override + public boolean isRunning() { + return true; + } + + @Override + protected Set doGet() { + HashSet exchangeRates = new HashSet<>(); + exchangeRates.add(new ExchangeRate( + currencyCode, + dummyRate, + System.currentTimeMillis(), + getName())); // ExchangeRateProvider name + return exchangeRates; + } + }; + + // Initialize provider + dummyProvider.start(); + try { + sleep(1000); + } catch (InterruptedException e) { + } + dummyProvider.stop(); + return dummyProvider; + } + private static String getRandomAlphaNumericString(int length) { return RandomStringUtils.random(length, true, true); } From ff2bed7ee3264d6a89e51c6a00bd4e1a450c644b Mon Sep 17 00:00:00 2001 From: jmacxx <47253594+jmacxx@users.noreply.github.com> Date: Tue, 12 Sep 2023 21:24:04 -0500 Subject: [PATCH 2/2] Fix merge conflicts. --- .../price/spot/ExchangeRateServiceTest.java | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/src/test/java/bisq/price/spot/ExchangeRateServiceTest.java b/src/test/java/bisq/price/spot/ExchangeRateServiceTest.java index 7112e38..a5bc01b 100644 --- a/src/test/java/bisq/price/spot/ExchangeRateServiceTest.java +++ b/src/test/java/bisq/price/spot/ExchangeRateServiceTest.java @@ -71,7 +71,7 @@ public void getAllMarketPrices_withNoExchangeRates_logs_Exception() { int numberOfCurrencyPairsOnExchange = 0; ExchangeRateProvider dummyProvider = buildDummyExchangeRateProvider(numberOfCurrencyPairsOnExchange); Map retrievedData = new ExchangeRateService( - new StandardEnvironment(), Collections.singletonList(dummyProvider)).getAllMarketPrices(); + new StandardEnvironment(), Collections.singletonList(dummyProvider), Collections.emptyList()).getAllMarketPrices(); doSanityChecksForRetrievedDataSingleProvider(retrievedData, dummyProvider, numberOfCurrencyPairsOnExchange); // No exchange rates provided by this exchange, two things should happen @@ -97,7 +97,7 @@ public void getAllMarketPrices_withSingleExchangeRate() { int numberOfCurrencyPairsOnExchange = 1; ExchangeRateProvider dummyProvider = buildDummyExchangeRateProvider(numberOfCurrencyPairsOnExchange); Map retrievedData = new ExchangeRateService( - new StandardEnvironment(), Collections.singletonList(dummyProvider)).getAllMarketPrices(); + new StandardEnvironment(), Collections.singletonList(dummyProvider), Collections.emptyList()).getAllMarketPrices(); doSanityChecksForRetrievedDataSingleProvider(retrievedData, dummyProvider, numberOfCurrencyPairsOnExchange); // One rate was provided by this provider, so the timestamp should not be 0 @@ -110,7 +110,7 @@ public void getAllMarketPrices_withMultipleProviders_differentCurrencyCodes() { List providers = asList( buildDummyExchangeRateProvider(numberOfCurrencyPairsOnExchange), buildDummyExchangeRateProvider(numberOfCurrencyPairsOnExchange)); - Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers, Collections.emptyList()).getAllMarketPrices(); doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); checkBisqIndexCalculationNoOutliers(validateAndGetRetrievedRates(retrievedData), providers); @@ -138,7 +138,7 @@ public void getAllMarketPrices_oneProvider_considerBlueUpdates() { Map retrievedData = service.getAllMarketPrices(); - doSanityChecksForRetrievedDataMultipleProviders(service, retrievedData, List.of(dummyProvider)); + doSanityChecksForRetrievedDataMultipleProviders(retrievedData, List.of(dummyProvider)); } @@ -153,9 +153,9 @@ public void getAllMarketPrices_withMultipleProviders_overlappingCurrencyCodes() // Create several dummy providers, each providing their own rates for the same set of currencies List providers = asList( - buildDummyExchangeRateProvider(rateCurrencyCodes), - buildDummyExchangeRateProvider(rateCurrencyCodes)); - Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + buildDummyExchangeRateProvider(rateCurrencyCodes, null), + buildDummyExchangeRateProvider(rateCurrencyCodes, null)); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers, Collections.emptyList()).getAllMarketPrices(); doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); checkBisqIndexCalculationNoOutliers(validateAndGetRetrievedRates(retrievedData), providers); @@ -172,7 +172,7 @@ public void bisqIndexCalculation_oneOutlierPriceWideRange() { buildDummyExchangeRateProviderWithRate("mercadoBitcoin", fiatCoin, 0.0), // outlier - low buildDummyExchangeRateProviderWithRate("coinGecko", fiatCoin, 129000.0), buildDummyExchangeRateProviderWithRate("binance", fiatCoin, 131000.0)); - Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers, Collections.emptyList()).getAllMarketPrices(); doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); checkBisqIndexCalculationWithOutlier(validateAndGetRetrievedRates(retrievedData), providers, Collections.singletonList("mercadoBitcoin")); } @@ -184,7 +184,7 @@ public void bisqIndexCalculation_oneOutlierPriceCloseRange() { buildDummyExchangeRateProviderWithRate("mercadoBitcoin", fiatCoin, 124545.5), buildDummyExchangeRateProviderWithRate("coinGecko", fiatCoin, 124083.752), buildDummyExchangeRateProviderWithRate("binance", fiatCoin, 124726.0)); - Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers, Collections.emptyList()).getAllMarketPrices(); doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); checkBisqIndexCalculationWithOutlier(validateAndGetRetrievedRates(retrievedData), providers, Collections.singletonList("coinGecko")); } @@ -199,7 +199,7 @@ public void bisqIndexCalculation_twoOutlierPrices() { buildDummyExchangeRateProviderWithRate("coinGecko", fiatCoin, 20774.0), buildDummyExchangeRateProviderWithRate("binance", fiatCoin, 21361.48), // outlier - high buildDummyExchangeRateProviderWithRate("coinbasePro", fiatCoin, 20798.25)); - Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers, Collections.emptyList()).getAllMarketPrices(); doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); checkBisqIndexCalculationWithOutlier(validateAndGetRetrievedRates(retrievedData), providers, List.of("binance", "bitfinex")); } @@ -215,7 +215,7 @@ public void bisqIndexCalculation_altcoinOutlierPrices() { buildDummyExchangeRateProviderWithRate("binance", altcoin, 0.05), // outlier - low buildDummyExchangeRateProviderWithRate("poloniex", altcoin, 0.07), // outlier - high buildDummyExchangeRateProviderWithRate("coinbasePro", altcoin, 0.06259)); - Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers, Collections.emptyList()).getAllMarketPrices(); doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); checkBisqIndexCalculationWithOutlier(validateAndGetRetrievedRates(retrievedData), providers, List.of("binance", "poloniex")); } @@ -227,7 +227,7 @@ public void bisqIndexCalculation_allSamePricesAltcoin() { buildDummyExchangeRateProviderWithRate("bitfinex", altcoin, 2.37E-6), buildDummyExchangeRateProviderWithRate("kraken", altcoin, 2.37E-6), buildDummyExchangeRateProviderWithRate("coinbasePro", altcoin, 2.37E-6)); - Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers, Collections.emptyList()).getAllMarketPrices(); doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); checkBisqIndexCalculationNoOutliers(validateAndGetRetrievedRates(retrievedData), providers); } @@ -239,7 +239,7 @@ public void bisqIndexCalculation_allSamePricesFiat() { buildDummyExchangeRateProviderWithRate("bitfinex", fiat, 10000.0), buildDummyExchangeRateProviderWithRate("kraken", fiat, 10000.0), buildDummyExchangeRateProviderWithRate("coinbasePro", fiat, 10000.0)); - Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers).getAllMarketPrices(); + Map retrievedData = new ExchangeRateService(new StandardEnvironment(), providers, Collections.emptyList()).getAllMarketPrices(); doSanityChecksForRetrievedDataMultipleProviders(retrievedData, providers); checkBisqIndexCalculationNoOutliers(validateAndGetRetrievedRates(retrievedData), providers); } @@ -315,18 +315,17 @@ public Set doGet() { /** * Performs generic sanity checks on the response format and contents. * - * @param service being tested {@link ExchangeRateService} * @param retrievedData Response data retrieved from the {@link ExchangeRateService} * @param provider {@link ExchangeRateProvider} available to the * {@link ExchangeRateService} * @param numberOfCurrencyPairsOnExchange Number of currency pairs this exchange was * initiated with */ - private void doSanityChecksForRetrievedDataSingleProvider(ExchangeRateService service, Map retrievedData, + private void doSanityChecksForRetrievedDataSingleProvider(Map retrievedData, ExchangeRateProvider provider, int numberOfCurrencyPairsOnExchange) { // Check response structure - doSanityChecksForRetrievedDataMultipleProviders(service, retrievedData, asList(provider)); + doSanityChecksForRetrievedDataMultipleProviders(retrievedData, asList(provider)); // Check that the amount of provided exchange rates matches expected value // For one provider, the amount of rates of that provider should be the total @@ -363,12 +362,11 @@ private List validateAndGetRetrievedRates(Map retr /** * Performs generic sanity checks on the response format and contents. * - * @param service being tested {@link ExchangeRateService} * @param retrievedData Response data retrieved from the {@link ExchangeRateService} * @param providers List of all {@link ExchangeRateProvider#getPrefix()} the * {@link ExchangeRateService} uses */ - private void doSanityChecksForRetrievedDataMultipleProviders(ExchangeRateService service, Map retrievedData, + private void doSanityChecksForRetrievedDataMultipleProviders(Map retrievedData, List providers) { // Check the correct amount of entries were present in the service response: // The timestamp and the count fields are per provider, so N providers means N