From 05e1039423c265b4319bf40d065746ccb8e1c829 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Fri, 11 Sep 2020 20:28:23 -0500 Subject: [PATCH 1/4] Call validatePayoutTx only after trades are initialized --- core/src/main/java/bisq/core/trade/Trade.java | 16 ++++-- .../java/bisq/core/trade/TradeManager.java | 1 + .../steps/buyer/BuyerStep1View.java | 53 ++++++++++++++----- .../steps/buyer/BuyerStep2View.java | 45 +++++++++++----- 4 files changed, 88 insertions(+), 27 deletions(-) diff --git a/core/src/main/java/bisq/core/trade/Trade.java b/core/src/main/java/bisq/core/trade/Trade.java index a00fe2dca8..3242bd535e 100644 --- a/core/src/main/java/bisq/core/trade/Trade.java +++ b/core/src/main/java/bisq/core/trade/Trade.java @@ -725,9 +725,19 @@ public abstract class Trade implements Tradable, Model { @Nullable public Transaction getDelayedPayoutTx() { if (delayedPayoutTx == null) { - delayedPayoutTx = delayedPayoutTxBytes != null && processModel.getBtcWalletService() != null ? - processModel.getBtcWalletService().getTxFromSerializedTx(delayedPayoutTxBytes) : - null; + BtcWalletService btcWalletService = processModel.getBtcWalletService(); + if (btcWalletService == null) { + log.warn("btcWalletService is null. You might call that method before the tradeManager has " + + "initialized all trades"); + return null; + } + + if (delayedPayoutTxBytes == null) { + log.warn("delayedPayoutTxBytes are null"); + return null; + } + + delayedPayoutTx = btcWalletService.getTxFromSerializedTx(delayedPayoutTxBytes); } return delayedPayoutTx; } diff --git a/core/src/main/java/bisq/core/trade/TradeManager.java b/core/src/main/java/bisq/core/trade/TradeManager.java index 6c0f3d79a3..ea9bcdf293 100644 --- a/core/src/main/java/bisq/core/trade/TradeManager.java +++ b/core/src/main/java/bisq/core/trade/TradeManager.java @@ -135,6 +135,7 @@ public class TradeManager implements PersistedDataHost { private final Storage> tradableListStorage; private TradableList tradableList; + @Getter private final BooleanProperty pendingTradesInitialized = new SimpleBooleanProperty(); private List tradesForStatistics; @Setter diff --git a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep1View.java b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep1View.java index 75dfb517c4..716d6bae04 100644 --- a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep1View.java +++ b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep1View.java @@ -24,7 +24,13 @@ import bisq.desktop.main.portfolio.pendingtrades.steps.TradeStepView; import bisq.core.locale.Res; import bisq.core.trade.DelayedPayoutTxValidation; +import bisq.common.UserThread; + +import javafx.beans.property.BooleanProperty; +import javafx.beans.value.ChangeListener; + public class BuyerStep1View extends TradeStepView { + private ChangeListener pendingTradesInitializedListener; /////////////////////////////////////////////////////////////////////////////////////////// // Constructor, Initialisation @@ -38,18 +44,18 @@ public class BuyerStep1View extends TradeStepView { public void activate() { super.activate(); - try { - DelayedPayoutTxValidation.validatePayoutTx(trade, - trade.getDelayedPayoutTx(), - model.dataModel.daoFacade, - model.dataModel.btcWalletService); - } catch (DelayedPayoutTxValidation.MissingTxException ignore) { - // We don't react on those errors as a failed trade might get listed initially but getting removed from the - // trade manager after initPendingTrades which happens after activate might be called. - } catch (DelayedPayoutTxValidation.ValidationException e) { - if (!model.dataModel.tradeManager.isAllowFaultyDelayedTxs()) { - new Popup().warning(Res.get("portfolio.pending.invalidDelayedPayoutTx", e.getMessage())).show(); - } + // We need to have the trades initialized before we can call validatePayoutTx. + BooleanProperty pendingTradesInitialized = model.dataModel.tradeManager.getPendingTradesInitialized(); + if (pendingTradesInitialized.get()) { + validatePayoutTx(); + } else { + pendingTradesInitializedListener = (observable, oldValue, newValue) -> { + if (newValue) { + validatePayoutTx(); + UserThread.execute(() -> pendingTradesInitialized.removeListener(pendingTradesInitializedListener)); + } + }; + pendingTradesInitialized.addListener(pendingTradesInitializedListener); } } @@ -85,6 +91,29 @@ public class BuyerStep1View extends TradeStepView { protected String getPeriodOverWarnText() { return Res.get("portfolio.pending.step1.openForDispute"); } + + + /////////////////////////////////////////////////////////////////////////////////////////// + // Private + /////////////////////////////////////////////////////////////////////////////////////////// + + private void validatePayoutTx() { + try { + DelayedPayoutTxValidation.validatePayoutTx(trade, + trade.getDelayedPayoutTx(), + model.dataModel.daoFacade, + model.dataModel.btcWalletService); + } catch (DelayedPayoutTxValidation.MissingTxException ignore) { + // We don't react on those errors as a failed trade might get listed initially but getting removed from the + // trade manager after initPendingTrades which happens after activate might be called. + log.error(""); + } catch (DelayedPayoutTxValidation.ValidationException e) { + if (!model.dataModel.tradeManager.isAllowFaultyDelayedTxs()) { + new Popup().warning(Res.get("portfolio.pending.invalidDelayedPayoutTx", e.getMessage())).show(); + } + } + } + } diff --git a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java index 9011390f59..6096e0cf3d 100644 --- a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java +++ b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java @@ -88,6 +88,9 @@ import javafx.scene.layout.Priority; import org.fxmisc.easybind.EasyBind; import org.fxmisc.easybind.Subscription; +import javafx.beans.property.BooleanProperty; +import javafx.beans.value.ChangeListener; + import java.util.List; import java.util.concurrent.TimeUnit; @@ -104,6 +107,7 @@ public class BuyerStep2View extends TradeStepView { private BusyAnimation busyAnimation; private Subscription tradeStatePropertySubscription; private Timer timeoutTimer; + private ChangeListener pendingTradesInitializedListener; /////////////////////////////////////////////////////////////////////////////////////////// // Constructor, Initialisation @@ -117,18 +121,18 @@ public class BuyerStep2View extends TradeStepView { public void activate() { super.activate(); - try { - DelayedPayoutTxValidation.validatePayoutTx(trade, - trade.getDelayedPayoutTx(), - model.dataModel.daoFacade, - model.dataModel.btcWalletService); - } catch (DelayedPayoutTxValidation.MissingTxException ignore) { - // We don't react on those errors as a failed trade might get listed initially but getting removed from the - // trade manager after initPendingTrades which happens after activate might be called. - } catch (DelayedPayoutTxValidation.ValidationException e) { - if (!model.dataModel.tradeManager.isAllowFaultyDelayedTxs()) { - new Popup().warning(Res.get("portfolio.pending.invalidDelayedPayoutTx", e.getMessage())).show(); - } + // We need to have the trades initialized before we can call validatePayoutTx. + BooleanProperty pendingTradesInitialized = model.dataModel.tradeManager.getPendingTradesInitialized(); + if (pendingTradesInitialized.get()) { + validatePayoutTx(); + } else { + pendingTradesInitializedListener = (observable, oldValue, newValue) -> { + if (newValue) { + validatePayoutTx(); + UserThread.execute(() -> pendingTradesInitialized.removeListener(pendingTradesInitializedListener)); + } + }; + pendingTradesInitialized.addListener(pendingTradesInitializedListener); } if (timeoutTimer != null) @@ -629,6 +633,23 @@ public class BuyerStep2View extends TradeStepView { } } + private void validatePayoutTx() { + try { + DelayedPayoutTxValidation.validatePayoutTx(trade, + trade.getDelayedPayoutTx(), + model.dataModel.daoFacade, + model.dataModel.btcWalletService); + } catch (DelayedPayoutTxValidation.MissingTxException ignore) { + // We don't react on those errors as a failed trade might get listed initially but getting removed from the + // trade manager after initPendingTrades which happens after activate might be called. + log.error(""); + } catch (DelayedPayoutTxValidation.ValidationException e) { + if (!model.dataModel.tradeManager.isAllowFaultyDelayedTxs()) { + new Popup().warning(Res.get("portfolio.pending.invalidDelayedPayoutTx", e.getMessage())).show(); + } + } + } + @Override protected void updateConfirmButtonDisableState(boolean isDisabled) { confirmButton.setDisable(isDisabled); From 1c41db4a767e1d70aabe99c9af5d47bd0e3b6112 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 17 Sep 2020 13:46:00 -0500 Subject: [PATCH 2/4] Fix wrong handling of mainnet RECIPIENT_BTC_ADDRESSes We must not add main net addresses if not on mainnet --- core/src/main/java/bisq/core/dao/DaoFacade.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/bisq/core/dao/DaoFacade.java b/core/src/main/java/bisq/core/dao/DaoFacade.java index bfc542a862..2f20a802ba 100644 --- a/core/src/main/java/bisq/core/dao/DaoFacade.java +++ b/core/src/main/java/bisq/core/dao/DaoFacade.java @@ -73,6 +73,7 @@ import bisq.core.dao.state.model.governance.Vote; import bisq.asset.Asset; +import bisq.common.config.Config; import bisq.common.handlers.ErrorMessageHandler; import bisq.common.handlers.ExceptionHandler; import bisq.common.handlers.ResultHandler; @@ -768,10 +769,12 @@ public class DaoFacade implements DaoSetupService { // If Dao is deactivated we need to add the default address as getAllPastParamValues will not return us any. allPastParamValues.add(Param.RECIPIENT_BTC_ADDRESS.getDefaultValue()); - // If Dao is deactivated we need to add the past addresses used as well. - // This list need to be updated once a new address gets defined. - allPastParamValues.add("3EtUWqsGThPtjwUczw27YCo6EWvQdaPUyp"); // burning man 2019 - allPastParamValues.add("3A8Zc1XioE2HRzYfbb5P8iemCS72M6vRJV"); // burningman2 + if (Config.baseCurrencyNetwork().isMainnet()) { + // If Dao is deactivated we need to add the past addresses used as well. + // This list need to be updated once a new address gets defined. + allPastParamValues.add("3EtUWqsGThPtjwUczw27YCo6EWvQdaPUyp"); // burning man 2019 + allPastParamValues.add("3A8Zc1XioE2HRzYfbb5P8iemCS72M6vRJV"); // burningman2 + } return allPastParamValues; } From 3d4427cdfd43edc803cc5a44a9e93b20447bc828 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 17 Sep 2020 13:54:45 -0500 Subject: [PATCH 3/4] Add result of filter match. Add more filter data (tx ids, json) --- .../main/support/dispute/DisputeView.java | 110 ++++++++++++++++-- .../dispute/agent/DisputeAgentView.java | 18 ++- .../dispute/client/DisputeClientView.java | 14 +-- 3 files changed, 111 insertions(+), 31 deletions(-) diff --git a/desktop/src/main/java/bisq/desktop/main/support/dispute/DisputeView.java b/desktop/src/main/java/bisq/desktop/main/support/dispute/DisputeView.java index fcba3c3b65..4c4e06f5d9 100644 --- a/desktop/src/main/java/bisq/desktop/main/support/dispute/DisputeView.java +++ b/desktop/src/main/java/bisq/desktop/main/support/dispute/DisputeView.java @@ -89,6 +89,7 @@ import javafx.collections.transformation.FilteredList; import javafx.collections.transformation.SortedList; import javafx.util.Callback; +import javafx.util.Duration; import java.text.DateFormat; import java.text.ParseException; @@ -102,6 +103,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import lombok.Getter; @@ -110,6 +112,29 @@ import javax.annotation.Nullable; import static bisq.desktop.util.FormBuilder.getIconForLabel; public abstract class DisputeView extends ActivatableView { + public enum FilterResult { + NO_MATCH("No Match"), + NO_FILTER("No filter text"), + OPEN_DISPUTES("Open disputes"), + TRADE_ID("Trade ID"), + OPENING_DATE("Opening date"), + BUYER_NODE_ADDRESS("Buyer node address"), + SELLER_NODE_ADDRESS("Seller node address"), + BUYER_ACCOUNT_DETAILS("Buyer account details"), + SELLER_ACCOUNT_DETAILS("Seller account details"), + DEPOSIT_TX("Deposit tx ID"), + PAYOUT_TX("Payout tx ID"), + DEL_PAYOUT_TX("Delayed payout tx ID"), + JSON("Contract as json"); + + @Getter + private final String displayString; + + FilterResult(String displayString) { + + this.displayString = displayString; + } + } protected final DisputeManager> disputeManager; protected final KeyRing keyRing; @@ -177,6 +202,10 @@ public abstract class DisputeView extends ActivatableView { HBox.setHgrow(label, Priority.NEVER); filterTextField = new InputTextField(); + Tooltip tooltip = new Tooltip(); + tooltip.setShowDelay(Duration.millis(100)); + tooltip.setShowDuration(Duration.seconds(10)); + filterTextField.setTooltip(tooltip); filterTextFieldListener = (observable, oldValue, newValue) -> applyFilteredListPredicate(filterTextField.getText()); HBox.setHgrow(filterTextField, Priority.NEVER); @@ -385,10 +414,77 @@ public abstract class DisputeView extends ActivatableView { protected abstract DisputeSession getConcreteDisputeChatSession(Dispute dispute); protected void applyFilteredListPredicate(String filterString) { - // If in trader view we must not display arbitrators own disputes as trader (must not happen anyway) - filteredList.setPredicate(dispute -> !dispute.getAgentPubKeyRing().equals(keyRing.getPubKeyRing())); + AtomicReference filterResult = new AtomicReference<>(FilterResult.NO_FILTER); + filteredList.setPredicate(dispute -> { + FilterResult filterResult1 = getFilterResult(dispute, filterString); + filterResult.set(filterResult1); + boolean b = filterResult.get() != FilterResult.NO_MATCH; + log.error("filterResult1 {} {} {}, {}", filterResult1, dispute.getTraderId(), b, filterResult); + return b; + }); + + if (filterResult.get() == FilterResult.NO_MATCH) { + filterTextField.getTooltip().setText("No matches found"); + } else if (filterResult.get() == FilterResult.NO_FILTER) { + filterTextField.getTooltip().setText("No filter applied"); + } else if (filterResult.get() == FilterResult.OPEN_DISPUTES) { + filterTextField.getTooltip().setText("Show all open disputes"); + } else { + filterTextField.getTooltip().setText("Data matching filter string: " + filterResult.get().getDisplayString()); + } } + protected FilterResult getFilterResult(Dispute dispute, String filterString) { + if (filterString.isEmpty()) { + return FilterResult.NO_FILTER; + } + if (!dispute.isClosed() && filterString.toLowerCase().equals("open")) { + return FilterResult.OPEN_DISPUTES; + } + + if (dispute.getTradeId().contains(filterString)) { + return FilterResult.TRADE_ID; + } + + if (DisplayUtils.formatDate(dispute.getOpeningDate()).contains(filterString)) { + return FilterResult.OPENING_DATE; + } + + if (dispute.getContract().getBuyerNodeAddress().getFullAddress().contains(filterString)) { + return FilterResult.BUYER_NODE_ADDRESS; + } + + if (dispute.getContract().getSellerNodeAddress().getFullAddress().contains(filterString)) { + return FilterResult.SELLER_NODE_ADDRESS; + } + + if (dispute.getContract().getBuyerPaymentAccountPayload().getPaymentDetails().contains(filterString)) { + return FilterResult.BUYER_ACCOUNT_DETAILS; + } + + if (dispute.getContract().getSellerPaymentAccountPayload().getPaymentDetails().contains(filterString)) { + return FilterResult.SELLER_ACCOUNT_DETAILS; + } + + if (dispute.getDepositTxId() != null && dispute.getDepositTxId().contains(filterString)) { + return FilterResult.DEPOSIT_TX; + } + if (dispute.getPayoutTxId() != null && dispute.getPayoutTxId().contains(filterString)) { + return FilterResult.PAYOUT_TX; + } + + if (dispute.getDelayedPayoutTxId() != null && dispute.getDelayedPayoutTxId().contains(filterString)) { + return FilterResult.DEL_PAYOUT_TX; + } + + if (dispute.getContractAsJson().contains(filterString)) { + return FilterResult.JSON; + } + + return FilterResult.NO_MATCH; + } + + protected void reOpenDisputeFromButton() { reOpenDispute(); } @@ -412,16 +508,6 @@ public abstract class DisputeView extends ActivatableView { } } - protected boolean anyMatchOfFilterString(Dispute dispute, String filterString) { - boolean matchesTradeId = dispute.getTradeId().contains(filterString); - boolean matchesDate = DisplayUtils.formatDate(dispute.getOpeningDate()).contains(filterString); - boolean isBuyerOnion = dispute.getContract().getBuyerNodeAddress().getFullAddress().contains(filterString); - boolean isSellerOnion = dispute.getContract().getSellerNodeAddress().getFullAddress().contains(filterString); - boolean matchesBuyersPaymentAccountData = dispute.getContract().getBuyerPaymentAccountPayload().getPaymentDetails().contains(filterString); - boolean matchesSellersPaymentAccountData = dispute.getContract().getSellerPaymentAccountPayload().getPaymentDetails().contains(filterString); - return matchesTradeId || matchesDate || isBuyerOnion || isSellerOnion || - matchesBuyersPaymentAccountData || matchesSellersPaymentAccountData; - } /////////////////////////////////////////////////////////////////////////////////////////// // UI actions diff --git a/desktop/src/main/java/bisq/desktop/main/support/dispute/agent/DisputeAgentView.java b/desktop/src/main/java/bisq/desktop/main/support/dispute/agent/DisputeAgentView.java index f7d059b110..c8661f2bb1 100644 --- a/desktop/src/main/java/bisq/desktop/main/support/dispute/agent/DisputeAgentView.java +++ b/desktop/src/main/java/bisq/desktop/main/support/dispute/agent/DisputeAgentView.java @@ -174,17 +174,13 @@ public abstract class DisputeAgentView extends DisputeView implements MultipleHo /////////////////////////////////////////////////////////////////////////////////////////// @Override - protected void applyFilteredListPredicate(String filterString) { - filteredList.setPredicate(dispute -> { - // If in arbitrator view we must only display disputes where we are selected as arbitrator (must not receive others anyway) - if (!dispute.getAgentPubKeyRing().equals(keyRing.getPubKeyRing())) { - return false; - } - boolean isOpen = !dispute.isClosed() && filterString.toLowerCase().equals("open"); - return filterString.isEmpty() || - isOpen || - anyMatchOfFilterString(dispute, filterString); - }); + protected DisputeView.FilterResult getFilterResult(Dispute dispute, String filterString) { + // If in arbitrator view we must only display disputes where we are selected as arbitrator (must not receive others anyway) + if (!dispute.getAgentPubKeyRing().equals(keyRing.getPubKeyRing())) { + return FilterResult.NO_MATCH; + } + + return super.getFilterResult(dispute, filterString); } @Override diff --git a/desktop/src/main/java/bisq/desktop/main/support/dispute/client/DisputeClientView.java b/desktop/src/main/java/bisq/desktop/main/support/dispute/client/DisputeClientView.java index f06875202e..265f5838ec 100644 --- a/desktop/src/main/java/bisq/desktop/main/support/dispute/client/DisputeClientView.java +++ b/desktop/src/main/java/bisq/desktop/main/support/dispute/client/DisputeClientView.java @@ -55,14 +55,12 @@ public abstract class DisputeClientView extends DisputeView { } @Override - protected void applyFilteredListPredicate(String filterString) { - filteredList.setPredicate(dispute -> { - // As we are in the client view we hide disputes where we are the agent - if (dispute.getAgentPubKeyRing().equals(keyRing.getPubKeyRing())) { - return false; - } + protected DisputeView.FilterResult getFilterResult(Dispute dispute, String filterString) { + // As we are in the client view we hide disputes where we are the agent + if (dispute.getAgentPubKeyRing().equals(keyRing.getPubKeyRing())) { + return FilterResult.NO_MATCH; + } - return filterString.isEmpty() || anyMatchOfFilterString(dispute, filterString); - }); + return super.getFilterResult(dispute, filterString); } } From 45cee2a2729c91dc4a627a5ba2747c45b4673b26 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 17 Sep 2020 19:05:55 -0500 Subject: [PATCH 4/4] Add check for disputes with duplicated trade ID or payout tx ids --- .../main/java/bisq/core/dao/DaoFacade.java | 4 +- .../bisq/core/support/dispute/Dispute.java | 9 +++ .../core/support/dispute/DisputeManager.java | 32 ++++++-- .../dispute/mediation/MediationManager.java | 1 + .../support/dispute/refund/RefundManager.java | 1 + .../core/trade/DelayedPayoutTxValidation.java | 81 ++++++++++++++++++- .../windows/DisputeSummaryWindow.java | 16 ++++ .../dispute/agent/DisputeAgentView.java | 40 +++++---- proto/src/main/proto/pb.proto | 1 + 9 files changed, 159 insertions(+), 26 deletions(-) diff --git a/core/src/main/java/bisq/core/dao/DaoFacade.java b/core/src/main/java/bisq/core/dao/DaoFacade.java index 2f20a802ba..7d9d39aaa0 100644 --- a/core/src/main/java/bisq/core/dao/DaoFacade.java +++ b/core/src/main/java/bisq/core/dao/DaoFacade.java @@ -767,7 +767,9 @@ public class DaoFacade implements DaoSetupService { Set allPastParamValues = getAllPastParamValues(Param.RECIPIENT_BTC_ADDRESS); // If Dao is deactivated we need to add the default address as getAllPastParamValues will not return us any. - allPastParamValues.add(Param.RECIPIENT_BTC_ADDRESS.getDefaultValue()); + 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. diff --git a/core/src/main/java/bisq/core/support/dispute/Dispute.java b/core/src/main/java/bisq/core/support/dispute/Dispute.java index 6a79a9f740..df80aa8c3f 100644 --- a/core/src/main/java/bisq/core/support/dispute/Dispute.java +++ b/core/src/main/java/bisq/core/support/dispute/Dispute.java @@ -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; } diff --git a/core/src/main/java/bisq/core/support/dispute/DisputeManager.java b/core/src/main/java/bisq/core/support/dispute/DisputeManager.java index 3fc8538e15..ea1ff28c54 100644 --- a/core/src/main/java/bisq/core/support/dispute/DisputeManager.java +++ b/core/src/main/java/bisq/core/support/dispute/DisputeManager.java @@ -86,10 +86,11 @@ public abstract class DisputeManager disputeListService; private final PriceFeedService priceFeedService; - private final DaoFacade daoFacade; + protected final DaoFacade daoFacade; @Getter - protected final ObservableList disputesWithInvalidDonationAddress = FXCollections.observableArrayList(); + protected final ObservableList validationExceptions = + FXCollections.observableArrayList(); /////////////////////////////////////////////////////////////////////////////////////////// @@ -219,7 +220,7 @@ public abstract class DisputeManager { + 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 openOfferManager, daoFacade, pubKeyRing, mediationDisputeListService, priceFeedService); } + /////////////////////////////////////////////////////////////////////////////////////////// // Implement template methods /////////////////////////////////////////////////////////////////////////////////////////// diff --git a/core/src/main/java/bisq/core/support/dispute/refund/RefundManager.java b/core/src/main/java/bisq/core/support/dispute/refund/RefundManager.java index f00aca5fbd..ab3008d8b1 100644 --- a/core/src/main/java/bisq/core/support/dispute/refund/RefundManager.java +++ b/core/src/main/java/bisq/core/support/dispute/refund/RefundManager.java @@ -83,6 +83,7 @@ public final class RefundManager extends DisputeManager { openOfferManager, daoFacade, pubKeyRing, refundDisputeListService, priceFeedService); } + /////////////////////////////////////////////////////////////////////////////////////////// // Implement template methods /////////////////////////////////////////////////////////////////////////////////////////// diff --git a/core/src/main/java/bisq/core/trade/DelayedPayoutTxValidation.java b/core/src/main/java/bisq/core/trade/DelayedPayoutTxValidation.java index 023f9d608d..25eb077120 100644 --- a/core/src/main/java/bisq/core/trade/DelayedPayoutTxValidation.java +++ b/core/src/main/java/bisq/core/trade/DelayedPayoutTxValidation.java @@ -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 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> disputesPerTradeId = new HashMap<>(); + Map> 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 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); + } + } } diff --git a/desktop/src/main/java/bisq/desktop/main/overlays/windows/DisputeSummaryWindow.java b/desktop/src/main/java/bisq/desktop/main/overlays/windows/DisputeSummaryWindow.java index 3e349efc4d..2fd0b33e8e 100644 --- a/desktop/src/main/java/bisq/desktop/main/overlays/windows/DisputeSummaryWindow.java +++ b/desktop/src/main/java/bisq/desktop/main/overlays/windows/DisputeSummaryWindow.java @@ -746,6 +746,7 @@ public class DisputeSummaryWindow extends Overlay { 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 { 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(); + } } } diff --git a/desktop/src/main/java/bisq/desktop/main/support/dispute/agent/DisputeAgentView.java b/desktop/src/main/java/bisq/desktop/main/support/dispute/agent/DisputeAgentView.java index c8661f2bb1..b2802c28b4 100644 --- a/desktop/src/main/java/bisq/desktop/main/support/dispute/agent/DisputeAgentView.java +++ b/desktop/src/main/java/bisq/desktop/main/support/dispute/agent/DisputeAgentView.java @@ -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 disputesWithInvalidDonationAddressListener; + private ListChangeListener validationExceptionListener; public DisputeAgentView(DisputeManager> 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 disputes) { - disputes.stream() - .filter(dispute -> !dispute.isClosed()) - .forEach(dispute -> { - new Popup().warning(Res.get("support.warning.disputesWithInvalidDonationAddress", - dispute.getDonationAddressOfDelayedPayoutTx(), - daoFacade.getAllDonationAddresses(), - dispute.getTradeId(), - "")) - .show(); + protected void showWarningForValidationExceptions(List 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); } diff --git a/proto/src/main/proto/pb.proto b/proto/src/main/proto/pb.proto index b10a4a02da..29efbd465b 100644 --- a/proto/src/main/proto/pb.proto +++ b/proto/src/main/proto/pb.proto @@ -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 {