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

Fix republish trade stats #4662

Merged
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
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
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
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
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
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
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
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());
}
}
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
1 change: 1 addition & 0 deletions p2p/src/main/java/bisq/network/p2p/P2PService.java
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ public void onMessage(NetworkEnvelope networkEnvelope, Connection connection) {
connection.setPeerType(Connection.PeerType.DIRECT_MSG_PEER);
try {
DecryptedMessageWithPubKey decryptedMsg = encryptionService.decryptAndVerify(sealedMsg.getSealedAndSigned());
connection.maybeHandleSupportedCapabilitiesMessage(decryptedMsg.getNetworkEnvelope());
connection.getPeersNodeAddressOptional().ifPresentOrElse(nodeAddress ->
decryptedDirectMessageListeners.forEach(e -> e.onDirectMessage(decryptedMsg, nodeAddress)),
() -> {
Expand Down
Loading