Merge pull request #3911 from ripcurlx/redact-deposittxid-from-tradestatistics

Redact deposittxid from tradestatistics

core/src/main/java/bisq/core/offer/OfferBookService.java

@@ -33,9 +33,8 @@ import bisq.common.handlers.ResultHandler;
 import bisq.common.storage.JsonFileManager;
 import bisq.common.util.Utilities;
 
-import javax.inject.Named;
-
 import javax.inject.Inject;
+import javax.inject.Named;
 
 import java.io.File;
 
@@ -88,30 +87,26 @@ public class OfferBookService {
         p2PService.addHashSetChangedListener(new HashMapChangedListener() {
             @Override
             public void onAdded(Collection<ProtectedStorageEntry> protectedStorageEntries) {
-                protectedStorageEntries.forEach(protectedStorageEntry -> {
+                protectedStorageEntries.forEach(protectedStorageEntry -> offerBookChangedListeners.forEach(listener -> {
-                    offerBookChangedListeners.stream().forEach(listener -> {
+                    if (protectedStorageEntry.getProtectedStoragePayload() instanceof OfferPayload) {
-                        if (protectedStorageEntry.getProtectedStoragePayload() instanceof OfferPayload) {
+                        OfferPayload offerPayload = (OfferPayload) protectedStorageEntry.getProtectedStoragePayload();
-                            OfferPayload offerPayload = (OfferPayload) protectedStorageEntry.getProtectedStoragePayload();
+                        Offer offer = new Offer(offerPayload);
-                            Offer offer = new Offer(offerPayload);
+                        offer.setPriceFeedService(priceFeedService);
-                            offer.setPriceFeedService(priceFeedService);
+                        listener.onAdded(offer);
-                            listener.onAdded(offer);
+                    }
-                        }
+                }));
-                    });
-                });
             }
 
             @Override
             public void onRemoved(Collection<ProtectedStorageEntry> protectedStorageEntries) {
-                protectedStorageEntries.forEach(protectedStorageEntry -> {
+                protectedStorageEntries.forEach(protectedStorageEntry -> offerBookChangedListeners.forEach(listener -> {
-                    offerBookChangedListeners.stream().forEach(listener -> {
+                    if (protectedStorageEntry.getProtectedStoragePayload() instanceof OfferPayload) {
-                        if (protectedStorageEntry.getProtectedStoragePayload() instanceof OfferPayload) {
+                        OfferPayload offerPayload = (OfferPayload) protectedStorageEntry.getProtectedStoragePayload();
-                            OfferPayload offerPayload = (OfferPayload) protectedStorageEntry.getProtectedStoragePayload();
+                        Offer offer = new Offer(offerPayload);
-                            Offer offer = new Offer(offerPayload);
+                        offer.setPriceFeedService(priceFeedService);
-                            offer.setPriceFeedService(priceFeedService);
+                        listener.onRemoved(offer);
-                            listener.onRemoved(offer);
+                    }
-                        }
+                }));
-                    });
-                });
             }
         });
 
@@ -241,8 +236,7 @@ public class OfferBookService {
                                 offer.getId(),
                                 offer.isUseMarketBasedPrice(),
                                 offer.getMarketPriceMargin(),
-                                offer.getPaymentMethod(),
+                                offer.getPaymentMethod()
-                                offer.getOfferFeePaymentTxId()
                         );
                     } catch (Throwable t) {
                         // In case an offer was corrupted with null values we ignore it

core/src/main/java/bisq/core/offer/OfferForJson.java

@@ -35,6 +35,8 @@ import java.util.Date;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.annotation.Nullable;
+
 public class OfferForJson {
     private static final Logger log = LoggerFactory.getLogger(OfferForJson.class);
 
@@ -48,7 +50,6 @@ public class OfferForJson {
     public final double marketPriceMargin;
     public final String paymentMethod;
     public final String id;
-    public final String offerFeeTxID;
 
     // primaryMarket fields are based on industry standard where primaryMarket is always in the focus (in the app BTC is always in the focus - will be changed in a larger refactoring once)
     public String currencyPair;
@@ -78,25 +79,23 @@ public class OfferForJson {
                         String currencyCode,
                         Coin minAmount,
                         Coin amount,
-                        Price price,
+                        @Nullable Price price,
                         Date date,
                         String id,
                         boolean useMarketBasedPrice,
                         double marketPriceMargin,
-                        PaymentMethod paymentMethod,
+                        PaymentMethod paymentMethod) {
-                        String offerFeeTxID) {
 
         this.direction = direction;
         this.currencyCode = currencyCode;
         this.minAmount = minAmount.value;
         this.amount = amount.value;
-        this.price = price.getValue();
+        this.price = price != null ? price.getValue() : 0;
         this.date = date.getTime();
         this.id = id;
         this.useMarketBasedPrice = useMarketBasedPrice;
         this.marketPriceMargin = marketPriceMargin;
         this.paymentMethod = paymentMethod.getId();
-        this.offerFeeTxID = offerFeeTxID;
 
         setDisplayStrings();
     }

core/src/main/java/bisq/core/offer/availability/DisputeAgentSelection.java

@@ -42,14 +42,6 @@ import static com.google.common.base.Preconditions.checkArgument;
 
 @Slf4j
 public class DisputeAgentSelection {
-
-    public static <T extends DisputeAgent> T getLeastUsedArbitrator(TradeStatisticsManager tradeStatisticsManager,
-                                                                    DisputeAgentManager<T> disputeAgentManager) {
-        return getLeastUsedDisputeAgent(tradeStatisticsManager,
-                disputeAgentManager,
-                TradeStatistics2.ARBITRATOR_ADDRESS);
-    }
-
     public static <T extends DisputeAgent> T getLeastUsedMediator(TradeStatisticsManager tradeStatisticsManager,
                                                                   DisputeAgentManager<T> disputeAgentManager) {
         return getLeastUsedDisputeAgent(tradeStatisticsManager,

core/src/main/java/bisq/core/trade/protocol/tasks/PublishTradeStatistics.java

@@ -51,19 +51,6 @@ public class PublishTradeStatistics extends TradeTask {
                     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 a mediator.
@@ -83,7 +70,7 @@ public class PublishTradeStatistics extends TradeTask {
                         trade.getTradePrice(),
                         trade.getTradeAmount(),
                         trade.getDate(),
-                        trade.getDepositTx().getHashAsString(),
+                        null,
                         extraDataMap);
                 processModel.getP2PService().addPersistableNetworkPayload(tradeStatistics, true);
             }

core/src/main/java/bisq/core/trade/statistics/TradeStatistics2.java

@@ -25,8 +25,8 @@ import bisq.core.offer.OfferPayload;
 import bisq.core.offer.OfferUtil;
 
 import bisq.network.p2p.storage.payload.CapabilityRequiringPayload;
-import bisq.network.p2p.storage.payload.ProcessOncePersistableNetworkPayload;
 import bisq.network.p2p.storage.payload.PersistableNetworkPayload;
+import bisq.network.p2p.storage.payload.ProcessOncePersistableNetworkPayload;
 
 import bisq.common.app.Capabilities;
 import bisq.common.app.Capability;
@@ -46,6 +46,7 @@ import org.bitcoinj.utils.Fiat;
 import com.google.common.base.Charsets;
 
 import java.util.Date;
+import java.util.GregorianCalendar;
 import java.util.Map;
 import java.util.Optional;
 
@@ -64,13 +65,11 @@ import static com.google.common.base.Preconditions.checkNotNull;
 @Value
 public final class TradeStatistics2 implements ProcessOncePersistableNetworkPayload, PersistableNetworkPayload, PersistableEnvelope, CapabilityRequiringPayload {
 
-    //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";
 
+    public static final Date CUT_OFF_DATE_FOR_DEPOSIT_TX_ID = Utilities.getUTCDate(2019, GregorianCalendar.FEBRUARY, 13);
+
     private final OfferPayload.Direction direction;
     private final String baseCurrency;
     private final String counterCurrency;
@@ -86,6 +85,7 @@ public final class TradeStatistics2 implements ProcessOncePersistableNetworkPayl
     // tradeDate is different for both peers so we ignore it for hash
     @JsonExclude
     private final long tradeDate;
+    @Nullable
     private final String depositTxId;
 
     // Hash get set in constructor from json of all the other data fields (with hash = null).
@@ -141,7 +141,7 @@ public final class TradeStatistics2 implements ProcessOncePersistableNetworkPayl
                             long tradePrice,
                             long tradeAmount,
                             long tradeDate,
-                            String depositTxId,
+                            @Nullable String depositTxId,
                             @Nullable byte[] hash,
                             @Nullable Map<String, String> extraDataMap) {
         this.direction = direction;
@@ -185,9 +185,9 @@ public final class TradeStatistics2 implements ProcessOncePersistableNetworkPayl
                 .setTradePrice(tradePrice)
                 .setTradeAmount(tradeAmount)
                 .setTradeDate(tradeDate)
-                .setDepositTxId(depositTxId)
                 .setHash(ByteString.copyFrom(hash));
         Optional.ofNullable(extraDataMap).ifPresent(builder::putAllExtraData);
+        Optional.ofNullable(depositTxId).ifPresent(builder::setDepositTxId);
         return builder;
     }
 
@@ -215,7 +215,7 @@ public final class TradeStatistics2 implements ProcessOncePersistableNetworkPayl
                 proto.getTradePrice(),
                 proto.getTradeAmount(),
                 proto.getTradeDate(),
-                proto.getDepositTxId(),
+                null, // We don't want to expose this anymore
                 null,   // We want to clean up the hashes with the changed hash method in v.1.2.0 so we don't use the value from the proto
                 CollectionUtils.isEmpty(proto.getExtraDataMap()) ? null : proto.getExtraDataMap());
     }
@@ -282,7 +282,8 @@ public final class TradeStatistics2 implements ProcessOncePersistableNetworkPayl
         // Since the trade wasn't executed it's better to filter it out to avoid it having an undue influence on the
         // BSQ trade stats.
         boolean excludedFailedTrade = offerId.equals("6E5KOI6O-3a06a037-6f03-4bfa-98c2-59f49f73466a-112");
-        return tradeAmount > 0 && tradePrice > 0 && !excludedFailedTrade && !depositTxId.isEmpty();
+        boolean depositTxIdValid = depositTxId == null || (tradeDate < CUT_OFF_DATE_FOR_DEPOSIT_TX_ID.getTime() && !depositTxId.isEmpty());
+        return tradeAmount > 0 && tradePrice > 0 && !excludedFailedTrade && depositTxIdValid;
     }
 
     @Override

core/src/test/java/bisq/core/trade/statistics/TradeStatistics2Maker.java

@@ -0,0 +1,90 @@
+/*
+ * This file is part of Bisq.
+ *
+ * Bisq is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or (at
+ * your option) any later version.
+ *
+ * Bisq is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Bisq. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package bisq.core.trade.statistics;
+
+import bisq.core.monetary.Price;
+import bisq.core.offer.OfferPayload;
+
+import org.bitcoinj.core.Coin;
+
+import java.util.Calendar;
+import java.util.Collections;
+import java.util.Date;
+
+import com.natpryce.makeiteasy.Instantiator;
+import com.natpryce.makeiteasy.Maker;
+import com.natpryce.makeiteasy.Property;
+
+import static com.natpryce.makeiteasy.MakeItEasy.a;
+
+public class TradeStatistics2Maker {
+
+    public static final Property<TradeStatistics2, Date> date = new Property<>();
+    public static final Property<TradeStatistics2, String> depositTxId = new Property<>();
+
+    public static final Instantiator<TradeStatistics2> TradeStatistic2 = lookup -> {
+        Calendar calendar = Calendar.getInstance();
+        calendar.set(2016, 3, 19);
+
+        return new TradeStatistics2(
+                new OfferPayload("1234",
+                        0L,
+                        null,
+                        null,
+                        OfferPayload.Direction.BUY,
+                        100000L,
+                        0.0,
+                        false,
+                        100000L,
+                        100000L,
+                        "BTC",
+                        "USD",
+                        null,
+                        null,
+                        "SEPA",
+                        "",
+                        null,
+                        null,
+                        null,
+                        null,
+                        null,
+                        "",
+                        0L,
+                        0L,
+                        0L,
+                        false,
+                        0L,
+                        0L,
+                        0L,
+                        0L,
+                        false,
+                        false,
+                        0L,
+                        0L,
+                        false,
+                        null,
+                        null,
+                        0),
+                Price.valueOf("BTC", 100000L),
+                Coin.SATOSHI,
+                lookup.valueOf(date, new Date(calendar.getTimeInMillis())),
+                lookup.valueOf(depositTxId, "123456"),
+                Collections.emptyMap());
+    };
+    public static final Maker<TradeStatistics2> dayZeroTrade = a(TradeStatistic2);
+}

core/src/test/java/bisq/core/trade/statistics/TradeStatistics2Test.java

@@ -0,0 +1,62 @@
+/*
+ * This file is part of Bisq.
+ *
+ * Bisq is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or (at
+ * your option) any later version.
+ *
+ * Bisq is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Bisq. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package bisq.core.trade.statistics;
+
+import org.apache.commons.lang3.time.DateUtils;
+
+import org.junit.Test;
+
+import static bisq.core.trade.statistics.TradeStatistics2Maker.date;
+import static bisq.core.trade.statistics.TradeStatistics2Maker.dayZeroTrade;
+import static bisq.core.trade.statistics.TradeStatistics2Maker.depositTxId;
+import static com.natpryce.makeiteasy.MakeItEasy.make;
+import static com.natpryce.makeiteasy.MakeItEasy.with;
+import static com.natpryce.makeiteasy.MakeItEasy.withNull;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+
+public class TradeStatistics2Test {
+
+    @Test
+    public void isValid_WithDepositTxIdBeforeCutOffDate() {
+
+        TradeStatistics2 tradeStatistic = make(dayZeroTrade);
+
+        assertTrue(tradeStatistic.isValid());
+    }
+
+    @Test
+    public void isValid_Not_WithDepositTxIdAfterCutOffDate() {
+        TradeStatistics2 tradeStatistic = make(dayZeroTrade.but(
+                with(date, DateUtils.addDays(TradeStatistics2.CUT_OFF_DATE_FOR_DEPOSIT_TX_ID, 1))
+        ));
+
+        assertFalse(tradeStatistic.isValid());
+    }
+
+    @Test
+    public void isValid_WithEmptyDepositTxIdAfterCutOffDate() {
+        TradeStatistics2 tradeStatistic = make(dayZeroTrade.but(
+                with(date, DateUtils.addDays(TradeStatistics2.CUT_OFF_DATE_FOR_DEPOSIT_TX_ID, 1)),
+                withNull(depositTxId)
+        ));
+
+        assertTrue(tradeStatistic.isValid());
+    }
+}

desktop/src/main/java/bisq/desktop/main/overlays/windows/OfferDetailsWindow.java

@@ -301,8 +301,6 @@ public class OfferDetailsWindow extends Overlay<OfferDetailsWindow> {
         rows = 3;
         if (countryCode != null)
             rows++;
-        if (offer.getOfferFeePaymentTxId() != null)
-            rows++;
         if (!isF2F)
             rows++;
 
@@ -326,9 +324,6 @@ public class OfferDetailsWindow extends Overlay<OfferDetailsWindow> {
             addConfirmationLabelLabel(gridPane, ++rowIndex, Res.get("offerDetailsWindow.countryBank"),
                     CountryUtil.getNameAndCode(countryCode));
 
-        if (offer.getOfferFeePaymentTxId() != null)
-            addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.makerFeeTxId"), offer.getOfferFeePaymentTxId());
-
         if (placeOfferHandlerOptional.isPresent()) {
             addTitledGroupBg(gridPane, ++rowIndex, 1, Res.get("offerDetailsWindow.commitment"), Layout.GROUP_DISTANCE);
             final Tuple2<Label, Label> labelLabelTuple2 = addConfirmationLabelLabel(gridPane, rowIndex, Res.get("offerDetailsWindow.agree"), Res.get("createOffer.tac"),