From 9c4c5ca0da5065d9380a8b090122a20014f3a73f Mon Sep 17 00:00:00 2001 From: chimp1984 <54558767+chimp1984@users.noreply.github.com> Date: Tue, 29 Oct 2019 06:04:00 -0500 Subject: [PATCH] Fix tradestatistics (#3469) * Remove delayed re-publishing of tradeStatistics This was done earlier when only maker was publishing trade statistics. Now both traders do it so we get already higher resilience. * Remove unused method Forgot in prev. commit to remove also the method. * Remove support for TradeStatistics2.ARBITRATOR_ADDRESS * Add comment and set ARBITRATOR_ADDRESS deprecated * Remove setting of arbitrator data from makers side The 2 arbitrator related fields in Trade are only set by the maker and not used anymore for reading, so it can be removed. The whole arbitrator domain should be cleaned out some day, but because of backward compatibility issues it id difficult to do it entirely at release date. With release after v 1.2. when no old offers are out anymore we are able to clean up that domain. * Remove dev log --- .../java/bisq/core/trade/TradeManager.java | 7 --- .../tasks/PublishTradeStatistics.java | 13 ------ ...kerProcessesInputsForDepositTxRequest.java | 13 ------ .../trade/statistics/TradeStatistics2.java | 5 +++ .../statistics/TradeStatisticsManager.java | 43 ------------------- 5 files changed, 5 insertions(+), 76 deletions(-) diff --git a/core/src/main/java/bisq/core/trade/TradeManager.java b/core/src/main/java/bisq/core/trade/TradeManager.java index d3612d95165..1faf2d9a8a9 100644 --- a/core/src/main/java/bisq/core/trade/TradeManager.java +++ b/core/src/main/java/bisq/core/trade/TradeManager.java @@ -56,7 +56,6 @@ import bisq.network.p2p.SendMailboxMessageListener; import bisq.common.ClockWatcher; -import bisq.common.UserThread; import bisq.common.crypto.KeyRing; import bisq.common.handlers.ErrorMessageHandler; import bisq.common.handlers.FaultHandler; @@ -90,7 +89,6 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; -import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -288,11 +286,6 @@ private void initPendingTrades() { cleanUpAddressEntries(); - // TODO remove once we support Taker side publishing at take offer process - // We start later to have better connectivity to the network - UserThread.runAfter(() -> tradeStatisticsManager.publishTradeStatistics(tradesForStatistics), - 30, TimeUnit.SECONDS); - pendingTradesInitialized.set(true); } diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/PublishTradeStatistics.java b/core/src/main/java/bisq/core/trade/protocol/tasks/PublishTradeStatistics.java index e6c7342d6dd..ab083218d27 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/PublishTradeStatistics.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/PublishTradeStatistics.java @@ -51,19 +51,6 @@ protected void run() { extraDataMap.put(OfferPayload.REFERRAL_ID, processModel.getReferralIdService().getOptionalReferralId().get()); } - NodeAddress arbitratorNodeAddress = trade.getArbitratorNodeAddress(); - if (arbitratorNodeAddress != null) { - - // The first 4 chars are sufficient to identify an arbitrator. - // For testing with regtest/localhost we use the full address as its localhost and would result in - // same values for multiple arbitrators. - NetworkNode networkNode = model.getProcessModel().getP2PService().getNetworkNode(); - String address = networkNode instanceof TorNetworkNode ? - arbitratorNodeAddress.getFullAddress().substring(0, 4) : - arbitratorNodeAddress.getFullAddress(); - extraDataMap.put(TradeStatistics2.ARBITRATOR_ADDRESS, address); - } - NodeAddress mediatorNodeAddress = trade.getMediatorNodeAddress(); if (mediatorNodeAddress != null) { // The first 4 chars are sufficient to identify an arbitrator. diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/maker/MakerProcessesInputsForDepositTxRequest.java b/core/src/main/java/bisq/core/trade/protocol/tasks/maker/MakerProcessesInputsForDepositTxRequest.java index 6c5c1ea05e7..68fbbc51780 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/maker/MakerProcessesInputsForDepositTxRequest.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/maker/MakerProcessesInputsForDepositTxRequest.java @@ -19,7 +19,6 @@ import bisq.core.exceptions.TradePriceOutOfToleranceException; import bisq.core.offer.Offer; -import bisq.core.support.dispute.arbitration.arbitrator.Arbitrator; import bisq.core.support.dispute.mediation.mediator.Mediator; import bisq.core.trade.Trade; import bisq.core.trade.messages.InputsForDepositTxRequest; @@ -80,18 +79,6 @@ protected void run() { User user = checkNotNull(processModel.getUser(), "User must not be null"); - NodeAddress arbitratorNodeAddress = inputsForDepositTxRequest.getArbitratorNodeAddress(); - if (arbitratorNodeAddress != null) { - trade.setArbitratorNodeAddress(arbitratorNodeAddress); - Arbitrator arbitrator = user.getAcceptedArbitratorByAddress(arbitratorNodeAddress); - if (arbitrator != null) { - trade.setArbitratorBtcPubKey(checkNotNull(arbitrator.getBtcPubKey(), - "arbitrator.getBtcPubKey() must not be null")); - trade.setArbitratorPubKeyRing(checkNotNull(arbitrator.getPubKeyRing(), - "arbitrator.getPubKeyRing() must not be null")); - } - } - NodeAddress mediatorNodeAddress = checkNotNull(inputsForDepositTxRequest.getMediatorNodeAddress(), "payDepositRequest.getMediatorNodeAddress() must not be null"); trade.setMediatorNodeAddress(mediatorNodeAddress); diff --git a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2.java b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2.java index 7a12248d76e..72e9a6fe9c5 100644 --- a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2.java +++ b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2.java @@ -61,7 +61,11 @@ @Slf4j @Value public final class TradeStatistics2 implements LazyProcessedPayload, PersistableNetworkPayload, PersistableEnvelope { + + //We don't support arbitrators anymore so this entry will be only for pre v1.2. trades + @Deprecated public static final String ARBITRATOR_ADDRESS = "arbAddr"; + public static final String MEDIATOR_ADDRESS = "medAddr"; public static final String REFUND_AGENT_ADDRESS = "refAddr"; @@ -266,6 +270,7 @@ public boolean isValid() { return tradeAmount > 0 && tradePrice > 0 && !excludedFailedTrade; } + @Override public String toString() { return "TradeStatistics2{" + diff --git a/core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java b/core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java index 78d6b845044..5c423eca6bc 100644 --- a/core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java +++ b/core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java @@ -21,15 +21,11 @@ import bisq.core.locale.CurrencyTuple; import bisq.core.locale.CurrencyUtil; import bisq.core.locale.Res; -import bisq.core.offer.Offer; -import bisq.core.offer.OfferPayload; import bisq.core.provider.price.PriceFeedService; -import bisq.core.trade.Trade; import bisq.network.p2p.P2PService; import bisq.network.p2p.storage.persistence.AppendOnlyDataStoreService; -import bisq.common.UserThread; import bisq.common.storage.JsonFileManager; import bisq.common.storage.Storage; import bisq.common.util.Utilities; @@ -44,18 +40,14 @@ import java.util.ArrayList; import java.util.Collection; -import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; -import static com.google.common.base.Preconditions.checkNotNull; - @Slf4j public class TradeStatisticsManager { @@ -63,7 +55,6 @@ public class TradeStatisticsManager { private final P2PService p2PService; private final PriceFeedService priceFeedService; private final TradeStatistics2StorageService tradeStatistics2StorageService; - private final ReferralIdService referralIdService; private final boolean dumpStatistics; private final ObservableSet observableTradeStatisticsSet = FXCollections.observableSet(); private int duplicates = 0; @@ -73,13 +64,11 @@ public TradeStatisticsManager(P2PService p2PService, PriceFeedService priceFeedService, TradeStatistics2StorageService tradeStatistics2StorageService, AppendOnlyDataStoreService appendOnlyDataStoreService, - ReferralIdService referralIdService, @Named(Storage.STORAGE_DIR) File storageDir, @Named(AppOptionKeys.DUMP_STATISTICS) boolean dumpStatistics) { this.p2PService = p2PService; this.priceFeedService = priceFeedService; this.tradeStatistics2StorageService = tradeStatistics2StorageService; - this.referralIdService = referralIdService; this.dumpStatistics = dumpStatistics; jsonFileManager = new JsonFileManager(storageDir); @@ -138,38 +127,6 @@ public void onAllServicesInitialized() { dump(); } - public void publishTradeStatistics(List trades) { - for (int i = 0; i < trades.size(); i++) { - Trade trade = trades.get(i); - - Map extraDataMap = null; - if (referralIdService.getOptionalReferralId().isPresent()) { - extraDataMap = new HashMap<>(); - extraDataMap.put(OfferPayload.REFERRAL_ID, referralIdService.getOptionalReferralId().get()); - } - Offer offer = trade.getOffer(); - checkNotNull(offer, "offer must not ne null"); - checkNotNull(trade.getTradeAmount(), "trade.getTradeAmount() must not ne null"); - TradeStatistics2 tradeStatistics = new TradeStatistics2(offer.getOfferPayload(), - trade.getTradePrice(), - trade.getTradeAmount(), - trade.getDate(), - (trade.getDepositTx() != null ? trade.getDepositTx().getHashAsString() : ""), - extraDataMap); - addToSet(tradeStatistics); - - // We only republish trades from last 10 days - if ((new Date().getTime() - trade.getDate().getTime()) < TimeUnit.DAYS.toMillis(10)) { - long delay = 5000; - long minDelay = (i + 1) * delay; - long maxDelay = (i + 2) * delay; - UserThread.runAfterRandomDelay(() -> { - p2PService.addPersistableNetworkPayload(tradeStatistics, true); - }, minDelay, maxDelay, TimeUnit.MILLISECONDS); - } - } - } - public ObservableSet getObservableTradeStatisticsSet() { return observableTradeStatisticsSet; }