Skip to content

Commit

Permalink
Merge pull request #4662 from chimp1984/fix-republish-trade-stats
Browse files Browse the repository at this point in the history 
Fix republish trade stats
  • Loading branch information
@sqrrm
sqrrm committed Oct 16, 2020
2 parents b924107 + 294b45d commit 0dea954
Show file tree
Hide file tree
Showing 8 changed files with 144 additions and 62 deletions.
2 changes: 1 addition & 1 deletion common/src/main/java/bisq/common/persistence/PersistenceManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
13 changes: 13 additions & 0 deletions core/src/main/java/bisq/core/trade/TradeManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -158,6 +162,7 @@ public TradeManager(User user,
ProcessModelServiceProvider processModelServiceProvider,
ClockWatcher clockWatcher,
PersistenceManager<TradableList<Trade>> persistenceManager,
ReferralIdService referralIdService,
DumpDelayedPayoutTx dumpDelayedPayoutTx,
@Named(Config.ALLOW_FAULTY_DELAYED_TXS) boolean allowFaultyDelayedTxs) {
this.user = user;
Expand All @@ -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;
Expand Down Expand Up @@ -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<Trade> 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) {
Expand Down
26 changes: 1 addition & 25 deletions core/src/main/java/bisq/core/trade/protocol/SellerProtocol.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand All @@ -92,6 +67,7 @@ public void onMailboxMessage(TradeMessage message, NodeAddress peerNodeAddress)
}
}


///////////////////////////////////////////////////////////////////////////////////////////
// Incoming messages
///////////////////////////////////////////////////////////////////////////////////////////
Expand Down
38 changes: 3 additions & 35 deletions core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerPublishesTradeStatistics.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<String, String> 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);
Expand Down
34 changes: 34 additions & 0 deletions core/src/main/java/bisq/core/trade/statistics/TradeStatistics2.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -67,6 +71,36 @@
public final class TradeStatistics2 implements ProcessOncePersistableNetworkPayload, PersistableNetworkPayload,
CapabilityRequiringPayload, Comparable<TradeStatistics2> {

public static TradeStatistics2 from(Trade trade,
@Nullable String referralId,
boolean isTorNetworkNode) {
Map<String, String> 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")
Expand Down
42 changes: 42 additions & 0 deletions core/src/main/java/bisq/core/trade/statistics/TradeStatistics3.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -48,6 +52,7 @@

import java.util.Arrays;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;

Expand All @@ -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<String, String> 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
Expand Down
49 changes: 49 additions & 0 deletions core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -46,6 +49,8 @@

import lombok.extern.slf4j.Slf4j;

import javax.annotation.Nullable;

@Singleton
@Slf4j
public class TradeStatisticsManager {
Expand Down Expand Up @@ -140,4 +145,48 @@ private void maybeDumpStatistics() {
list.toArray(array);
jsonFileManager.writeToDiscThreaded(Utilities.objectToJson(array), "trade_statistics");
}

public void maybeRepublishTradeStatistics(Set<Trade> trades,
@Nullable String referralId,
boolean isTorNetworkNode) {
long ts = System.currentTimeMillis();
Set<P2PDataStorage.ByteArray> 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());
}
}
2 changes: 1 addition & 1 deletion desktop/src/main/java/bisq/desktop/main/market/trades/TradesChartsView.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit 0dea954

Please sign in to comment.