Add check for disputes with duplicated trade ID or payout tx ids

This commit is contained in:
chimp1984 2020-09-17 19:05:55 -05:00
parent 3d4427cdfd
commit 45cee2a272
No known key found for this signature in database
GPG key ID: 9801B4EC591F90E3
9 changed files with 159 additions and 26 deletions

View file

@ -767,7 +767,9 @@ public class DaoFacade implements DaoSetupService {
Set<String> allPastParamValues = getAllPastParamValues(Param.RECIPIENT_BTC_ADDRESS);
// If Dao is deactivated we need to add the default address as getAllPastParamValues will not return us any.
if (allPastParamValues.isEmpty()) {
allPastParamValues.add(Param.RECIPIENT_BTC_ADDRESS.getDefaultValue());
}
if (Config.baseCurrencyNetwork().isMainnet()) {
// If Dao is deactivated we need to add the past addresses used as well.

View file

@ -107,6 +107,9 @@ public final class Dispute implements NetworkPayload {
@Setter
@Nullable
private String donationAddressOfDelayedPayoutTx;
@Setter
@Nullable
private String agentsUid;
///////////////////////////////////////////////////////////////////////////////////////////
@ -234,6 +237,7 @@ public final class Dispute implements NetworkPayload {
Optional.ofNullable(mediatorsDisputeResult).ifPresent(result -> builder.setMediatorsDisputeResult(mediatorsDisputeResult));
Optional.ofNullable(delayedPayoutTxId).ifPresent(result -> builder.setDelayedPayoutTxId(delayedPayoutTxId));
Optional.ofNullable(donationAddressOfDelayedPayoutTx).ifPresent(result -> builder.setDonationAddressOfDelayedPayoutTx(donationAddressOfDelayedPayoutTx));
Optional.ofNullable(agentsUid).ifPresent(result -> builder.setAgentsUid(agentsUid));
return builder.build();
}
@ -282,6 +286,11 @@ public final class Dispute implements NetworkPayload {
dispute.setDonationAddressOfDelayedPayoutTx(donationAddressOfDelayedPayoutTx);
}
String agentsUid = proto.getAgentsUid();
if (!agentsUid.isEmpty()) {
dispute.setAgentsUid(agentsUid);
}
return dispute;
}

View file

@ -86,10 +86,11 @@ public abstract class DisputeManager<T extends DisputeList<? extends DisputeList
protected final PubKeyRing pubKeyRing;
protected final DisputeListService<T> disputeListService;
private final PriceFeedService priceFeedService;
private final DaoFacade daoFacade;
protected final DaoFacade daoFacade;
@Getter
protected final ObservableList<Dispute> disputesWithInvalidDonationAddress = FXCollections.observableArrayList();
protected final ObservableList<DelayedPayoutTxValidation.ValidationException> validationExceptions =
FXCollections.observableArrayList();
///////////////////////////////////////////////////////////////////////////////////////////
@ -219,7 +220,7 @@ public abstract class DisputeManager<T extends DisputeList<? extends DisputeList
return disputeListService.getNrOfDisputes(isBuyer, contract);
}
private T getDisputeList() {
protected T getDisputeList() {
return disputeListService.getDisputeList();
}
@ -251,6 +252,20 @@ public abstract class DisputeManager<T extends DisputeList<? extends DisputeList
tryApplyMessages();
cleanupDisputes();
getDisputeList().getList().forEach(dispute -> {
if (dispute.getAgentsUid() == null) {
dispute.setAgentsUid(UUID.randomUUID().toString());
}
try {
DelayedPayoutTxValidation.validateDonationAddress(dispute, dispute.getDonationAddressOfDelayedPayoutTx(), daoFacade);
DelayedPayoutTxValidation.testIfDisputeTriesReplay(dispute, getDisputeList().getList());
} catch (DelayedPayoutTxValidation.AddressException | DelayedPayoutTxValidation.DisputeReplayException e) {
log.error(e.toString());
validationExceptions.add(e);
}
});
}
public boolean isTrader(Dispute dispute) {
@ -282,6 +297,8 @@ public abstract class DisputeManager<T extends DisputeList<? extends DisputeList
String errorMessage = null;
Dispute dispute = openNewDisputeMessage.getDispute();
// Dispute agent sets uid to be sure to identify disputes uniquely to protect against replaying old disputes
dispute.setAgentsUid(UUID.randomUUID().toString());
dispute.setStorage(disputeListService.getStorage());
// Disputes from clients < 1.2.0 always have support type ARBITRATION in dispute as the field didn't exist before
dispute.setSupportType(openNewDisputeMessage.getSupportType());
@ -291,8 +308,10 @@ public abstract class DisputeManager<T extends DisputeList<? extends DisputeList
try {
DelayedPayoutTxValidation.validateDonationAddress(dispute.getDonationAddressOfDelayedPayoutTx(), daoFacade);
} catch (DelayedPayoutTxValidation.AddressException e) {
disputesWithInvalidDonationAddress.add(dispute);
DelayedPayoutTxValidation.testIfDisputeTriesReplay(dispute, disputeList.getList());
} catch (DelayedPayoutTxValidation.AddressException | DelayedPayoutTxValidation.DisputeReplayException e) {
log.error(e.toString());
validationExceptions.add(e);
}
PubKeyRing peersPubKeyRing = dispute.isDisputeOpenerIsBuyer() ? contract.getSellerPubKeyRing() : contract.getBuyerPubKeyRing();
@ -580,6 +599,9 @@ public abstract class DisputeManager<T extends DisputeList<? extends DisputeList
addPriceInfoMessage(dispute, 0);
// Dispute agent sets uid to be sure to identify disputes uniquely to protect against replaying old disputes
dispute.setAgentsUid(UUID.randomUUID().toString());
disputeList.add(dispute);
// We mirrored dispute already!

View file

@ -89,6 +89,7 @@ public final class MediationManager extends DisputeManager<MediationDisputeList>
openOfferManager, daoFacade, pubKeyRing, mediationDisputeListService, priceFeedService);
}
///////////////////////////////////////////////////////////////////////////////////////////
// Implement template methods
///////////////////////////////////////////////////////////////////////////////////////////

View file

@ -83,6 +83,7 @@ public final class RefundManager extends DisputeManager<RefundDisputeList> {
openOfferManager, daoFacade, pubKeyRing, refundDisputeListService, priceFeedService);
}
///////////////////////////////////////////////////////////////////////////////////////////
// Implement template methods
///////////////////////////////////////////////////////////////////////////////////////////

View file

@ -30,9 +30,14 @@ import org.bitcoinj.core.TransactionInput;
import org.bitcoinj.core.TransactionOutPoint;
import org.bitcoinj.core.TransactionOutput;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Consumer;
import lombok.Getter;
import lombok.extern.slf4j.Slf4j;
import javax.annotation.Nullable;
@ -45,6 +50,11 @@ public class DelayedPayoutTxValidation {
public static void validateDonationAddress(String addressAsString, DaoFacade daoFacade)
throws AddressException {
validateDonationAddress(null, addressAsString, daoFacade);
}
public static void validateDonationAddress(@Nullable Dispute dispute, String addressAsString, DaoFacade daoFacade)
throws AddressException {
if (addressAsString == null) {
log.warn("address is null at validateDonationAddress. This is expected in case of an not updated trader.");
@ -57,7 +67,55 @@ public class DelayedPayoutTxValidation {
"\nAddress used in the dispute: " + addressAsString +
"\nAll DAO param donation addresses:" + allPastParamValues;
log.error(errorMsg);
throw new AddressException(errorMsg);
throw new AddressException(dispute, errorMsg);
}
}
public static void testIfDisputeTriesReplay(Dispute disputeToTest, List<Dispute> disputeList)
throws DisputeReplayException {
try {
String disputeToTestDelayedPayoutTxId = disputeToTest.getDelayedPayoutTxId();
checkNotNull(disputeToTestDelayedPayoutTxId,
"delayedPayoutTxId must not be null. Trade ID: " + disputeToTest.getTradeId());
String disputeToTestAgentsUid = checkNotNull(disputeToTest.getAgentsUid(),
"agentsUid must not be null. Trade ID: " + disputeToTest.getTradeId());
// This method can be called with the existing list and a new dispute (at opening a new dispute) or with the
// dispute already added (at close dispute). So we will consider that in the for loop.
// We have 2 disputes per trade (one per trader).
Map<String, Set<String>> disputesPerTradeId = new HashMap<>();
Map<String, Set<String>> disputesPerDelayedPayoutTxId = new HashMap<>();
disputeList.forEach(dispute -> {
String tradeId = dispute.getTradeId();
String agentsUid = dispute.getAgentsUid();
// We use an uid we have created not data delivered by the trader to protect against replay attacks
// If our dispute got already added to the list we ignore it. We will check once we build our maps
disputesPerTradeId.putIfAbsent(tradeId, new HashSet<>());
Set<String> set = disputesPerTradeId.get(tradeId);
if (!disputeToTestAgentsUid.equals(agentsUid)) {
set.add(agentsUid);
}
String delayedPayoutTxId = dispute.getDelayedPayoutTxId();
disputesPerDelayedPayoutTxId.putIfAbsent(delayedPayoutTxId, new HashSet<>());
set = disputesPerDelayedPayoutTxId.get(delayedPayoutTxId);
if (!disputeToTestAgentsUid.equals(agentsUid)) {
set.add(agentsUid);
}
});
String disputeToTestTradeId = disputeToTest.getTradeId();
checkArgument(disputesPerTradeId.get(disputeToTestTradeId).size() <= 1,
"We found more then 2 disputes with the same trade ID. " +
"Trade ID: " + disputeToTest.getTradeId());
checkArgument(disputesPerDelayedPayoutTxId.get(disputeToTestDelayedPayoutTxId).size() <= 1,
"We found more then 2 disputes with the same delayedPayoutTxId. " +
"Trade ID: " + disputeToTest.getTradeId());
} catch (IllegalArgumentException | NullPointerException e) {
throw new DisputeReplayException(disputeToTest, e.getMessage());
}
}
@ -177,7 +235,7 @@ public class DelayedPayoutTxValidation {
errorMsg = "Donation address cannot be resolved (not of type P2PKHScript or P2SH). Output: " + output;
log.error(errorMsg);
log.error(delayedPayoutTx.toString());
throw new AddressException(errorMsg);
throw new AddressException(dispute, errorMsg);
}
}
@ -220,14 +278,23 @@ public class DelayedPayoutTxValidation {
///////////////////////////////////////////////////////////////////////////////////////////
public static class ValidationException extends Exception {
@Nullable
@Getter
private final Dispute dispute;
ValidationException(String msg) {
this(null, msg);
}
ValidationException(@Nullable Dispute dispute, String msg) {
super(msg);
this.dispute = dispute;
}
}
public static class AddressException extends ValidationException {
AddressException(String msg) {
super(msg);
AddressException(@Nullable Dispute dispute, String msg) {
super(dispute, msg);
}
}
@ -260,4 +327,10 @@ public class DelayedPayoutTxValidation {
super(msg);
}
}
public static class DisputeReplayException extends ValidationException {
DisputeReplayException(Dispute dispute, String msg) {
super(dispute, msg);
}
}
}

View file

@ -746,6 +746,7 @@ public class DisputeSummaryWindow extends Overlay<DisputeSummaryWindow> {
var disputeManager = checkNotNull(getDisputeManager(dispute));
try {
DelayedPayoutTxValidation.validateDonationAddress(dispute.getDonationAddressOfDelayedPayoutTx(), daoFacade);
DelayedPayoutTxValidation.testIfDisputeTriesReplay(dispute, disputeManager.getDisputesAsObservableList());
doClose(closeTicketButton);
} catch (DelayedPayoutTxValidation.AddressException exception) {
String addressAsString = dispute.getDonationAddressOfDelayedPayoutTx();
@ -775,6 +776,21 @@ public class DisputeSummaryWindow extends Overlay<DisputeSummaryWindow> {
Res.get("support.warning.disputesWithInvalidDonationAddress.refundAgent")))
.show();
}
} catch (DelayedPayoutTxValidation.DisputeReplayException exception) {
if (disputeManager instanceof MediationManager) {
new Popup().width(900)
.warning(exception.getMessage())
.onAction(() -> {
doClose(closeTicketButton);
})
.actionButtonText(Res.get("shared.yes"))
.closeButtonText(Res.get("shared.no"))
.show();
} else {
new Popup().width(900)
.warning(exception.getMessage())
.show();
}
}
}

View file

@ -34,6 +34,7 @@ import bisq.core.support.dispute.DisputeList;
import bisq.core.support.dispute.DisputeManager;
import bisq.core.support.dispute.DisputeSession;
import bisq.core.support.dispute.agent.MultipleHolderNameDetection;
import bisq.core.trade.DelayedPayoutTxValidation;
import bisq.core.trade.TradeManager;
import bisq.core.user.DontShowAgainLookup;
import bisq.core.util.coin.CoinFormatter;
@ -59,13 +60,14 @@ import javafx.collections.ListChangeListener;
import java.util.List;
import static bisq.core.trade.DelayedPayoutTxValidation.ValidationException;
import static bisq.desktop.util.FormBuilder.getIconForLabel;
public abstract class DisputeAgentView extends DisputeView implements MultipleHolderNameDetection.Listener {
private final MultipleHolderNameDetection multipleHolderNameDetection;
private final DaoFacade daoFacade;
private ListChangeListener<Dispute> disputesWithInvalidDonationAddressListener;
private ListChangeListener<ValidationException> validationExceptionListener;
public DisputeAgentView(DisputeManager<? extends DisputeList<? extends DisputeList>> disputeManager,
KeyRing keyRing,
@ -115,24 +117,30 @@ public abstract class DisputeAgentView extends DisputeView implements MultipleHo
multipleHolderNameDetection.detectMultipleHolderNames();
disputesWithInvalidDonationAddressListener = c -> {
validationExceptionListener = c -> {
c.next();
if (c.wasAdded()) {
showWarningForInvalidDonationAddress(c.getAddedSubList());
showWarningForValidationExceptions(c.getAddedSubList());
}
};
}
protected void showWarningForInvalidDonationAddress(List<? extends Dispute> disputes) {
disputes.stream()
.filter(dispute -> !dispute.isClosed())
.forEach(dispute -> {
new Popup().warning(Res.get("support.warning.disputesWithInvalidDonationAddress",
protected void showWarningForValidationExceptions(List<? extends ValidationException> exceptions) {
exceptions.stream()
.filter(ex -> ex.getDispute() != null)
.filter(ex -> !ex.getDispute().isClosed())
.forEach(ex -> {
Dispute dispute = ex.getDispute();
if (ex instanceof DelayedPayoutTxValidation.AddressException) {
new Popup().width(900).warning(Res.get("support.warning.disputesWithInvalidDonationAddress",
dispute.getDonationAddressOfDelayedPayoutTx(),
daoFacade.getAllDonationAddresses(),
dispute.getTradeId(),
""))
.show();
} else {
new Popup().width(900).warning(ex.getMessage()).show();
}
});
}
@ -145,8 +153,8 @@ public abstract class DisputeAgentView extends DisputeView implements MultipleHo
suspiciousDisputeDetected();
}
disputeManager.getDisputesWithInvalidDonationAddress().addListener(disputesWithInvalidDonationAddressListener);
showWarningForInvalidDonationAddress(disputeManager.getDisputesWithInvalidDonationAddress());
disputeManager.getValidationExceptions().addListener(validationExceptionListener);
showWarningForValidationExceptions(disputeManager.getValidationExceptions());
}
@Override
@ -155,7 +163,7 @@ public abstract class DisputeAgentView extends DisputeView implements MultipleHo
multipleHolderNameDetection.removeListener(this);
disputeManager.getDisputesWithInvalidDonationAddress().removeListener(disputesWithInvalidDonationAddressListener);
disputeManager.getValidationExceptions().removeListener(validationExceptionListener);
}

View file

@ -793,6 +793,7 @@ message Dispute {
string mediators_dispute_result = 25;
string delayed_payout_tx_id = 26;
string donation_address_of_delayed_payout_tx = 27;
string agents_uid = 28;
}
message Attachment {