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

Redact deposittxid from tradestatistics #3911

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
42 changes: 18 additions & 24 deletions core/src/main/java/bisq/core/offer/OfferBookService.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@
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;

Expand Down Expand Up @@ -88,30 +87,26 @@ public OfferBookService(P2PService p2PService,
p2PService.addHashSetChangedListener(new HashMapChangedListener() {
@Override
public void onAdded(Collection<ProtectedStorageEntry> protectedStorageEntries) {
protectedStorageEntries.forEach(protectedStorageEntry -> {
offerBookChangedListeners.stream().forEach(listener -> {
if (protectedStorageEntry.getProtectedStoragePayload() instanceof OfferPayload) {
OfferPayload offerPayload = (OfferPayload) protectedStorageEntry.getProtectedStoragePayload();
Offer offer = new Offer(offerPayload);
offer.setPriceFeedService(priceFeedService);
listener.onAdded(offer);
}
});
});
protectedStorageEntries.forEach(protectedStorageEntry -> offerBookChangedListeners.forEach(listener -> {
if (protectedStorageEntry.getProtectedStoragePayload() instanceof OfferPayload) {
OfferPayload offerPayload = (OfferPayload) protectedStorageEntry.getProtectedStoragePayload();
Offer offer = new Offer(offerPayload);
offer.setPriceFeedService(priceFeedService);
listener.onAdded(offer);
}
}));
}

@Override
public void onRemoved(Collection<ProtectedStorageEntry> protectedStorageEntries) {
protectedStorageEntries.forEach(protectedStorageEntry -> {
offerBookChangedListeners.stream().forEach(listener -> {
if (protectedStorageEntry.getProtectedStoragePayload() instanceof OfferPayload) {
OfferPayload offerPayload = (OfferPayload) protectedStorageEntry.getProtectedStoragePayload();
Offer offer = new Offer(offerPayload);
offer.setPriceFeedService(priceFeedService);
listener.onRemoved(offer);
}
});
});
protectedStorageEntries.forEach(protectedStorageEntry -> offerBookChangedListeners.forEach(listener -> {
if (protectedStorageEntry.getProtectedStoragePayload() instanceof OfferPayload) {
OfferPayload offerPayload = (OfferPayload) protectedStorageEntry.getProtectedStoragePayload();
Offer offer = new Offer(offerPayload);
offer.setPriceFeedService(priceFeedService);
listener.onRemoved(offer);
}
}));
}
});

Expand Down Expand Up @@ -241,8 +236,7 @@ private void doDumpStatistics() {
offer.getId(),
offer.isUseMarketBasedPrice(),
offer.getMarketPriceMargin(),
offer.getPaymentMethod(),
offer.getOfferFeePaymentTxId()
offer.getPaymentMethod()
);
} catch (Throwable t) {
// In case an offer was corrupted with null values we ignore it
Expand Down
11 changes: 5 additions & 6 deletions core/src/main/java/bisq/core/offer/OfferForJson.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.Nullable;

public class OfferForJson {
private static final Logger log = LoggerFactory.getLogger(OfferForJson.class);

Expand All @@ -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;
Expand Down Expand Up @@ -78,25 +79,23 @@ public OfferForJson(OfferPayload.Direction direction,
String currencyCode,
Coin minAmount,
Coin amount,
Price price,
@Nullable Price price,
Date date,
String id,
boolean useMarketBasedPrice,
double marketPriceMargin,
PaymentMethod paymentMethod,
String offerFeeTxID) {
PaymentMethod paymentMethod) {

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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,6 @@

@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,
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 a mediator.
Expand All @@ -83,7 +70,7 @@ protected void run() {
trade.getTradePrice(),
trade.getTradeAmount(),
trade.getDate(),
trade.getDepositTx().getHashAsString(),
null,
extraDataMap);
processModel.getP2PService().addPersistableNetworkPayload(tradeStatistics, true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
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;
Expand All @@ -46,6 +46,7 @@
import com.google.common.base.Charsets;

import java.util.Date;
import java.util.GregorianCalendar;
import java.util.Map;
import java.util.Optional;

Expand All @@ -64,13 +65,11 @@
@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;
Expand All @@ -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).
Expand Down Expand Up @@ -141,7 +141,7 @@ public TradeStatistics2(OfferPayload.Direction direction,
long tradePrice,
long tradeAmount,
long tradeDate,
String depositTxId,
@Nullable String depositTxId,
@Nullable byte[] hash,
@Nullable Map<String, String> extraDataMap) {
this.direction = direction;
Expand Down Expand Up @@ -185,9 +185,9 @@ private protobuf.TradeStatistics2.Builder getBuilder() {
.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;
}

Expand Down Expand Up @@ -215,7 +215,7 @@ public static TradeStatistics2 fromProto(protobuf.TradeStatistics2 proto) {
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());
}
Expand Down Expand Up @@ -282,7 +282,8 @@ public boolean isValid() {
// 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
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -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());
}
}
Loading