From 0b38fccfd2585a8ade78b1db0ac8545ed2e84e7e Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Fri, 16 Oct 2020 13:27:21 -0500 Subject: [PATCH 1/3] Add file name to temp file at write to disk --- .../main/java/bisq/common/persistence/PersistenceManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/main/java/bisq/common/persistence/PersistenceManager.java b/common/src/main/java/bisq/common/persistence/PersistenceManager.java index ec8d1ee005f..6cdf98c9661 100644 --- a/common/src/main/java/bisq/common/persistence/PersistenceManager.java +++ b/common/src/main/java/bisq/common/persistence/PersistenceManager.java @@ -305,7 +305,7 @@ public void writeToDisk(protobuf.PersistableEnvelope serialized, @Nullable Runna tempFile = usedTempFilePath != null ? FileUtil.createNewFile(usedTempFilePath) - : File.createTempFile("temp", null, dir); + : File.createTempFile("temp_" + fileName, null, dir); // Don't use a new temp file path each time, as that causes the delete-on-exit hook to leak memory: tempFile.deleteOnExit(); From 8cb6a053341f69d85453cd31971330278d8952c0 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Fri, 16 Oct 2020 14:02:57 -0500 Subject: [PATCH 2/3] Fix min height for trade statistics table --- .../java/bisq/desktop/main/market/trades/TradesChartsView.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/desktop/src/main/java/bisq/desktop/main/market/trades/TradesChartsView.java b/desktop/src/main/java/bisq/desktop/main/market/trades/TradesChartsView.java index 604d572f47d..fbd035ff82d 100644 --- a/desktop/src/main/java/bisq/desktop/main/market/trades/TradesChartsView.java +++ b/desktop/src/main/java/bisq/desktop/main/market/trades/TradesChartsView.java @@ -544,7 +544,7 @@ private ToggleButton getToggleButton(String label, TradesChartsViewModel.TickUni private void createTable() { tableView = new TableView<>(); - tableView.setMinHeight(120); + tableView.setMinHeight(80); tableView.setPrefHeight(130); VBox.setVgrow(tableView, Priority.ALWAYS); From 294b45dbad69f0391c64bcf814b20f5a1c53d82c Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Fri, 16 Oct 2020 14:13:01 -0500 Subject: [PATCH 3/3] Republish trade statistics if not found in existing trade stats. Remove republishing at SellerProtocol as we don't know the capability of the peer at that moment and as we also want to republish for completed trades. --- .../java/bisq/core/trade/TradeManager.java | 13 +++++ .../core/trade/protocol/SellerProtocol.java | 26 +--------- .../SellerPublishesTradeStatistics.java | 38 ++------------ .../trade/statistics/TradeStatistics2.java | 34 +++++++++++++ .../trade/statistics/TradeStatistics3.java | 42 ++++++++++++++++ .../statistics/TradeStatisticsManager.java | 49 +++++++++++++++++++ 6 files changed, 142 insertions(+), 60 deletions(-) diff --git a/core/src/main/java/bisq/core/trade/TradeManager.java b/core/src/main/java/bisq/core/trade/TradeManager.java index 42cb6b9e000..0d8d02a2414 100644 --- a/core/src/main/java/bisq/core/trade/TradeManager.java +++ b/core/src/main/java/bisq/core/trade/TradeManager.java @@ -40,6 +40,7 @@ import bisq.core.trade.protocol.TakerProtocol; import bisq.core.trade.protocol.TradeProtocol; import bisq.core.trade.protocol.TradeProtocolFactory; +import bisq.core.trade.statistics.ReferralIdService; import bisq.core.trade.statistics.TradeStatisticsManager; import bisq.core.user.User; import bisq.core.util.Validator; @@ -49,6 +50,7 @@ import bisq.network.p2p.DecryptedMessageWithPubKey; import bisq.network.p2p.NodeAddress; import bisq.network.p2p.P2PService; +import bisq.network.p2p.network.TorNetworkNode; import bisq.common.ClockWatcher; import bisq.common.config.Config; @@ -83,6 +85,7 @@ import java.util.Date; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -133,6 +136,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi private ErrorMessageHandler takeOfferRequestErrorMessageHandler; @Getter private final LongProperty numPendingTrades = new SimpleLongProperty(); + private final ReferralIdService referralIdService; private final DumpDelayedPayoutTx dumpDelayedPayoutTx; @Getter private final boolean allowFaultyDelayedTxs; @@ -158,6 +162,7 @@ public TradeManager(User user, ProcessModelServiceProvider processModelServiceProvider, ClockWatcher clockWatcher, PersistenceManager> persistenceManager, + ReferralIdService referralIdService, DumpDelayedPayoutTx dumpDelayedPayoutTx, @Named(Config.ALLOW_FAULTY_DELAYED_TXS) boolean allowFaultyDelayedTxs) { this.user = user; @@ -174,6 +179,7 @@ public TradeManager(User user, this.mediatorManager = mediatorManager; this.processModelServiceProvider = processModelServiceProvider; this.clockWatcher = clockWatcher; + this.referralIdService = referralIdService; this.dumpDelayedPayoutTx = dumpDelayedPayoutTx; this.allowFaultyDelayedTxs = allowFaultyDelayedTxs; this.persistenceManager = persistenceManager; @@ -324,6 +330,13 @@ public TradeProtocol getTradeProtocol(Trade trade) { private void initPersistedTrades() { tradableList.forEach(this::initPersistedTrade); persistedTradesInitialized.set(true); + + // We do not include failed trades as they should not be counted anyway in the trade statistics + Set allTrades = new HashSet<>(closedTradableManager.getClosedTrades()); + allTrades.addAll(tradableList.getList()); + String referralId = referralIdService.getOptionalReferralId().orElse(null); + boolean isTorNetworkNode = p2PService.getNetworkNode() instanceof TorNetworkNode; + tradeStatisticsManager.maybeRepublishTradeStatistics(allTrades, referralId, isTorNetworkNode); } private void initPersistedTrade(Trade trade) { diff --git a/core/src/main/java/bisq/core/trade/protocol/SellerProtocol.java b/core/src/main/java/bisq/core/trade/protocol/SellerProtocol.java index 9f1acab3689..dd138fc0f9b 100644 --- a/core/src/main/java/bisq/core/trade/protocol/SellerProtocol.java +++ b/core/src/main/java/bisq/core/trade/protocol/SellerProtocol.java @@ -39,10 +39,6 @@ import bisq.common.handlers.ErrorMessageHandler; import bisq.common.handlers.ResultHandler; -import bisq.common.util.Utilities; - -import java.util.Date; -import java.util.GregorianCalendar; import lombok.extern.slf4j.Slf4j; @@ -57,27 +53,6 @@ public SellerProtocol(SellerTrade trade) { super(trade); } - @Override - protected void onInitialized() { - super.onInitialized(); - - // We get called the constructor with any possible state and phase. As we don't want to log an error for such - // cases we use the alternative 'given' method instead of 'expect'. - - // We only re-publish for about 2 weeks after 1.4.0 release until most nodes have updated to - // achieve sufficient resilience. - boolean currentDateBeforeCutOffDate = new Date().before(Utilities.getUTCDate(2020, GregorianCalendar.NOVEMBER, 1)); - given(anyPhase(Trade.Phase.DEPOSIT_PUBLISHED, - Trade.Phase.DEPOSIT_CONFIRMED, - Trade.Phase.FIAT_SENT, - Trade.Phase.FIAT_RECEIVED, - Trade.Phase.PAYOUT_PUBLISHED) - .with(SellerEvent.STARTUP) - .preCondition(currentDateBeforeCutOffDate)) - .setup(tasks(SellerPublishesTradeStatistics.class)) - .executeTasks(); - } - /////////////////////////////////////////////////////////////////////////////////////////// // Mailbox @@ -92,6 +67,7 @@ public void onMailboxMessage(TradeMessage message, NodeAddress peerNodeAddress) } } + /////////////////////////////////////////////////////////////////////////////////////////// // Incoming messages /////////////////////////////////////////////////////////////////////////////////////////// diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerPublishesTradeStatistics.java b/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerPublishesTradeStatistics.java index 0489ac0e325..89b6ac5a14a 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerPublishesTradeStatistics.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerPublishesTradeStatistics.java @@ -17,22 +17,15 @@ package bisq.core.trade.protocol.tasks.seller; -import bisq.core.offer.Offer; -import bisq.core.offer.OfferPayload; import bisq.core.trade.Trade; import bisq.core.trade.protocol.tasks.TradeTask; import bisq.core.trade.statistics.TradeStatistics3; -import bisq.network.p2p.NodeAddress; -import bisq.network.p2p.network.NetworkNode; import bisq.network.p2p.network.TorNetworkNode; import bisq.common.app.Capability; import bisq.common.taskrunner.TaskRunner; -import java.util.HashMap; -import java.util.Map; - import lombok.extern.slf4j.Slf4j; import static com.google.common.base.Preconditions.checkNotNull; @@ -56,34 +49,9 @@ protected void run() { // Our peer has updated, so as we are the seller we will publish the trade statistics. // The peer as buyer does not publish anymore with v.1.4.0 (where Capability.TRADE_STATISTICS_3 was added) - Map extraDataMap = new HashMap<>(); - if (processModel.getReferralIdService().getOptionalReferralId().isPresent()) { - extraDataMap.put(OfferPayload.REFERRAL_ID, processModel.getReferralIdService().getOptionalReferralId().get()); - } - - NodeAddress mediatorNodeAddress = checkNotNull(trade.getMediatorNodeAddress()); - // The first 4 chars are sufficient to identify a mediator. - // For testing with regtest/localhost we use the full address as its localhost and would result in - // same values for multiple mediators. - NetworkNode networkNode = model.getProcessModel().getP2PService().getNetworkNode(); - String truncatedMediatorNodeAddress = networkNode instanceof TorNetworkNode ? - mediatorNodeAddress.getFullAddress().substring(0, 4) : - mediatorNodeAddress.getFullAddress(); - - NodeAddress refundAgentNodeAddress = checkNotNull(trade.getRefundAgentNodeAddress()); - String truncatedRefundAgentNodeAddress = networkNode instanceof TorNetworkNode ? - refundAgentNodeAddress.getFullAddress().substring(0, 4) : - refundAgentNodeAddress.getFullAddress(); - - Offer offer = checkNotNull(trade.getOffer()); - TradeStatistics3 tradeStatistics = new TradeStatistics3(offer.getCurrencyCode(), - trade.getTradePrice().getValue(), - trade.getTradeAmountAsLong(), - offer.getPaymentMethod().getId(), - trade.getTakeOfferDate().getTime(), - truncatedMediatorNodeAddress, - truncatedRefundAgentNodeAddress, - extraDataMap); + String referralId = processModel.getReferralIdService().getOptionalReferralId().orElse(null); + boolean isTorNetworkNode = model.getProcessModel().getP2PService().getNetworkNode() instanceof TorNetworkNode; + TradeStatistics3 tradeStatistics = TradeStatistics3.from(trade, referralId, isTorNetworkNode); if (tradeStatistics.isValid()) { log.info("Publishing trade statistics"); processModel.getP2PService().addPersistableNetworkPayload(tradeStatistics, true); 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 41f753bc3b6..0b7830a34cb 100644 --- a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2.java +++ b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2.java @@ -21,9 +21,12 @@ import bisq.core.monetary.AltcoinExchangeRate; import bisq.core.monetary.Price; import bisq.core.monetary.Volume; +import bisq.core.offer.Offer; import bisq.core.offer.OfferPayload; import bisq.core.offer.OfferUtil; +import bisq.core.trade.Trade; +import bisq.network.p2p.NodeAddress; import bisq.network.p2p.storage.payload.CapabilityRequiringPayload; import bisq.network.p2p.storage.payload.PersistableNetworkPayload; import bisq.network.p2p.storage.payload.ProcessOncePersistableNetworkPayload; @@ -46,6 +49,7 @@ import com.google.common.base.Charsets; import java.util.Date; +import java.util.HashMap; import java.util.Map; import java.util.Optional; @@ -67,6 +71,36 @@ public final class TradeStatistics2 implements ProcessOncePersistableNetworkPayload, PersistableNetworkPayload, CapabilityRequiringPayload, Comparable { + public static TradeStatistics2 from(Trade trade, + @Nullable String referralId, + boolean isTorNetworkNode) { + Map extraDataMap = new HashMap<>(); + if (referralId != null) { + extraDataMap.put(OfferPayload.REFERRAL_ID, referralId); + } + + NodeAddress mediatorNodeAddress = trade.getMediatorNodeAddress(); + if (mediatorNodeAddress != null) { + // The first 4 chars are sufficient to identify a mediator. + // For testing with regtest/localhost we use the full address as its localhost and would result in + // same values for multiple mediators. + String address = isTorNetworkNode ? + mediatorNodeAddress.getFullAddress().substring(0, 4) : + mediatorNodeAddress.getFullAddress(); + extraDataMap.put(TradeStatistics2.MEDIATOR_ADDRESS, address); + } + + Offer offer = trade.getOffer(); + checkNotNull(offer, "offer must not ne null"); + checkNotNull(trade.getTradeAmount(), "trade.getTradeAmount() must not ne null"); + return new TradeStatistics2(offer.getOfferPayload(), + trade.getTradePrice(), + trade.getTradeAmount(), + trade.getDate(), + trade.getDepositTxId(), + extraDataMap); + } + @SuppressWarnings("SpellCheckingInspection") public static final String MEDIATOR_ADDRESS = "medAddr"; @SuppressWarnings("SpellCheckingInspection") diff --git a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics3.java b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics3.java index 0fb4032f0aa..a3f3e996a07 100644 --- a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics3.java +++ b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics3.java @@ -21,8 +21,12 @@ import bisq.core.monetary.AltcoinExchangeRate; import bisq.core.monetary.Price; import bisq.core.monetary.Volume; +import bisq.core.offer.Offer; +import bisq.core.offer.OfferPayload; import bisq.core.offer.OfferUtil; +import bisq.core.trade.Trade; +import bisq.network.p2p.NodeAddress; import bisq.network.p2p.storage.payload.CapabilityRequiringPayload; import bisq.network.p2p.storage.payload.DateSortedTruncatablePayload; import bisq.network.p2p.storage.payload.PersistableNetworkPayload; @@ -48,6 +52,7 @@ import java.util.Arrays; import java.util.Date; +import java.util.HashMap; import java.util.Map; import java.util.Optional; @@ -66,6 +71,43 @@ public final class TradeStatistics3 implements ProcessOncePersistableNetworkPayload, PersistableNetworkPayload, CapabilityRequiringPayload, DateSortedTruncatablePayload { + + public static TradeStatistics3 from(Trade trade, + @Nullable String referralId, + boolean isTorNetworkNode) { + Map extraDataMap = new HashMap<>(); + if (referralId != null) { + extraDataMap.put(OfferPayload.REFERRAL_ID, referralId); + } + + NodeAddress mediatorNodeAddress = checkNotNull(trade.getMediatorNodeAddress()); + // The first 4 chars are sufficient to identify a mediator. + // For testing with regtest/localhost we use the full address as its localhost and would result in + // same values for multiple mediators. + String truncatedMediatorNodeAddress = isTorNetworkNode ? + mediatorNodeAddress.getFullAddress().substring(0, 4) : + mediatorNodeAddress.getFullAddress(); + + // RefundAgentNodeAddress can be null if converted from old version. + String truncatedRefundAgentNodeAddress = null; + NodeAddress refundAgentNodeAddress = trade.getRefundAgentNodeAddress(); + if (refundAgentNodeAddress != null) { + truncatedRefundAgentNodeAddress = isTorNetworkNode ? + refundAgentNodeAddress.getFullAddress().substring(0, 4) : + refundAgentNodeAddress.getFullAddress(); + } + + Offer offer = checkNotNull(trade.getOffer()); + return new TradeStatistics3(offer.getCurrencyCode(), + trade.getTradePrice().getValue(), + trade.getTradeAmountAsLong(), + offer.getPaymentMethod().getId(), + trade.getTakeOfferDate().getTime(), + truncatedMediatorNodeAddress, + truncatedRefundAgentNodeAddress, + extraDataMap); + } + // This enum must not change the order as we use the ordinal for storage to reduce data size. // The payment method string can be quite long and would consume 15% more space. // When we get a new payment method we can add it to the enum at the end. Old users would add it as string if not 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 01a978c1a9e..83ae72b85b9 100644 --- a/core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java +++ b/core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java @@ -21,8 +21,11 @@ import bisq.core.locale.CurrencyUtil; import bisq.core.locale.Res; import bisq.core.provider.price.PriceFeedService; +import bisq.core.trade.BuyerTrade; +import bisq.core.trade.Trade; import bisq.network.p2p.P2PService; +import bisq.network.p2p.storage.P2PDataStorage; import bisq.network.p2p.storage.persistence.AppendOnlyDataStoreService; import bisq.common.config.Config; @@ -46,6 +49,8 @@ import lombok.extern.slf4j.Slf4j; +import javax.annotation.Nullable; + @Singleton @Slf4j public class TradeStatisticsManager { @@ -140,4 +145,48 @@ private void maybeDumpStatistics() { list.toArray(array); jsonFileManager.writeToDiscThreaded(Utilities.objectToJson(array), "trade_statistics"); } + + public void maybeRepublishTradeStatistics(Set trades, + @Nullable String referralId, + boolean isTorNetworkNode) { + long ts = System.currentTimeMillis(); + Set hashes = tradeStatistics3StorageService.getMapOfAllData().keySet(); + trades.forEach(trade -> { + if (trade instanceof BuyerTrade) { + log.debug("Trade: {} is a buyer trade, we only republish we have been seller.", + trade.getShortId()); + return; + } + + TradeStatistics3 tradeStatistics3 = TradeStatistics3.from(trade, referralId, isTorNetworkNode); + boolean hasTradeStatistics3 = hashes.contains(new P2PDataStorage.ByteArray(tradeStatistics3.getHash())); + if (hasTradeStatistics3) { + log.debug("Trade: {}. We have already a tradeStatistics matching the hash of tradeStatistics3.", + trade.getShortId()); + return; + } + + // If we did not find a TradeStatistics3 we look up if we find a TradeStatistics3 converted from + // TradeStatistics2 where we used the original hash, which is not the native hash of the + // TradeStatistics3 but of TradeStatistics2. + TradeStatistics2 tradeStatistics2 = TradeStatistics2.from(trade, referralId, isTorNetworkNode); + boolean hasTradeStatistics2 = hashes.contains(new P2PDataStorage.ByteArray(tradeStatistics2.getHash())); + if (hasTradeStatistics2) { + log.debug("Trade: {}. We have already a tradeStatistics matching the hash of tradeStatistics2. ", + trade.getShortId()); + return; + } + + if (!tradeStatistics3.isValid()) { + log.warn("Trade: {}. Trade statistics is invalid. We do not publish it.", tradeStatistics3); + return; + } + + log.info("Trade: {}. We republish tradeStatistics3 as we did not find it in the existing trade statistics. ", + trade.getShortId()); + p2PService.addPersistableNetworkPayload(tradeStatistics3, true); + }); + log.info("maybeRepublishTradeStatistics took {} ms. Number of tradeStatistics: {}. Number of own trades: {}", + System.currentTimeMillis() - ts, hashes.size(), trades.size()); + } }