Skip to content

Commit

Permalink
Fix tradestatistics (#3469)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
chimp1984 authored and ripcurlx committed Oct 29, 2019
1 parent 65703d5 commit 9c4c5ca
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 76 deletions.
7 changes: 0 additions & 7 deletions core/src/main/java/bisq/core/trade/TradeManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -266,6 +270,7 @@ public boolean isValid() {
return tradeAmount > 0 && tradePrice > 0 && !excludedFailedTrade;
}


@Override
public String toString() {
return "TradeStatistics2{" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -44,26 +40,21 @@

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 {

private final JsonFileManager jsonFileManager;
private final P2PService p2PService;
private final PriceFeedService priceFeedService;
private final TradeStatistics2StorageService tradeStatistics2StorageService;
private final ReferralIdService referralIdService;
private final boolean dumpStatistics;
private final ObservableSet<TradeStatistics2> observableTradeStatisticsSet = FXCollections.observableSet();
private int duplicates = 0;
Expand All @@ -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);

Expand Down Expand Up @@ -138,38 +127,6 @@ public void onAllServicesInitialized() {
dump();
}

public void publishTradeStatistics(List<Trade> trades) {
for (int i = 0; i < trades.size(); i++) {
Trade trade = trades.get(i);

Map<String, String> 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<TradeStatistics2> getObservableTradeStatisticsSet() {
return observableTradeStatisticsSet;
}
Expand Down

0 comments on commit 9c4c5ca

Please sign in to comment.