From 6dca11f471a9369ec8101822fbfb1807e17eefc5 Mon Sep 17 00:00:00 2001 From: woodser Date: Sat, 25 Feb 2023 08:13:44 -0500 Subject: [PATCH] arbitrator sends original unsigned payout tx if published synchronize on trade when processing dispute messages --- .../bisq/core/api/CoreDisputesService.java | 23 +- .../core/support/dispute/DisputeManager.java | 212 +++++++++--------- .../core/trade/protocol/ProcessModel.java | 3 + .../ResendDisputeClosedMessageWithPayout.java | 3 +- .../windows/DisputeSummaryWindow.java | 17 +- 5 files changed, 135 insertions(+), 123 deletions(-) diff --git a/core/src/main/java/bisq/core/api/CoreDisputesService.java b/core/src/main/java/bisq/core/api/CoreDisputesService.java index c3c97eaec2..3c23d09273 100644 --- a/core/src/main/java/bisq/core/api/CoreDisputesService.java +++ b/core/src/main/java/bisq/core/api/CoreDisputesService.java @@ -36,7 +36,6 @@ import java.util.List; import java.util.Optional; import lombok.extern.slf4j.Slf4j; -import monero.wallet.model.MoneroTxWallet; import static com.google.common.base.Preconditions.checkNotNull; import static java.lang.String.format; @@ -171,25 +170,25 @@ public class CoreDisputesService { applyPayoutAmountsToDisputeResult(payout, winningDispute, disputeResult, customWinnerAmount); // create dispute payout tx - MoneroTxWallet disputePayoutTx = arbitrationManager.createDisputePayoutTx(trade, winningDispute.getContract(), disputeResult, false); + trade.getProcessModel().setUnsignedPayoutTx(arbitrationManager.createDisputePayoutTx(trade, winningDispute.getContract(), disputeResult, false)); // close winning dispute ticket - closeDisputeTicket(arbitrationManager, winningDispute, disputeResult, disputePayoutTx, () -> { + closeDisputeTicket(arbitrationManager, winningDispute, disputeResult, () -> { arbitrationManager.requestPersistence(); }, (errMessage, err) -> { throw new IllegalStateException(errMessage, err); }); // close loser's dispute ticket - var peersDisputeOptional = arbitrationManager.getDisputesAsObservableList().stream() + var loserDisputeOptional = arbitrationManager.getDisputesAsObservableList().stream() .filter(d -> tradeId.equals(d.getTradeId()) && winningDispute.getTraderId() != d.getTraderId()) .findFirst(); - if (!peersDisputeOptional.isPresent()) throw new IllegalStateException("could not find peer dispute"); - var peerDispute = peersDisputeOptional.get(); - var peerDisputeResult = createDisputeResult(peerDispute, winner, reason, summaryNotes, closeDate); - peerDisputeResult.setBuyerPayoutAmount(disputeResult.getBuyerPayoutAmount()); - peerDisputeResult.setSellerPayoutAmount(disputeResult.getSellerPayoutAmount()); - closeDisputeTicket(arbitrationManager, peerDispute, peerDisputeResult, disputePayoutTx, () -> { + if (!loserDisputeOptional.isPresent()) throw new IllegalStateException("could not find peer dispute"); + var loserDispute = loserDisputeOptional.get(); + var loserDisputeResult = createDisputeResult(loserDispute, winner, reason, summaryNotes, closeDate); + loserDisputeResult.setBuyerPayoutAmount(disputeResult.getBuyerPayoutAmount()); + loserDisputeResult.setSellerPayoutAmount(disputeResult.getSellerPayoutAmount()); + closeDisputeTicket(arbitrationManager, loserDispute, loserDisputeResult, () -> { arbitrationManager.requestPersistence(); }, (errMessage, err) -> { throw new IllegalStateException(errMessage, err); @@ -248,7 +247,7 @@ public class CoreDisputesService { } } - public void closeDisputeTicket(DisputeManager disputeManager, Dispute dispute, DisputeResult disputeResult, MoneroTxWallet payoutTx, ResultHandler resultHandler, FaultHandler faultHandler) { + public void closeDisputeTicket(DisputeManager disputeManager, Dispute dispute, DisputeResult disputeResult, ResultHandler resultHandler, FaultHandler faultHandler) { DisputeResult.Reason reason = disputeResult.getReason(); String role = Res.get("shared.arbitrator"); @@ -279,7 +278,7 @@ public class CoreDisputesService { String summaryText = DisputeSummaryVerification.signAndApply(disputeManager, disputeResult, textToSign); summaryText += Res.get("disputeSummaryWindow.close.nextStepsForRefundAgentArbitration"); - disputeManager.closeDisputeTicket(disputeResult, dispute, summaryText, payoutTx, () -> { + disputeManager.closeDisputeTicket(disputeResult, dispute, summaryText, () -> { dispute.setDisputeResult(disputeResult); dispute.setIsClosed(); resultHandler.handleResult(); 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 5ef6406bea..fe623168d5 100644 --- a/core/src/main/java/bisq/core/support/dispute/DisputeManager.java +++ b/core/src/main/java/bisq/core/support/dispute/DisputeManager.java @@ -443,105 +443,106 @@ public abstract class DisputeManager> extends Sup Dispute dispute = message.getDispute(); log.info("{}.onDisputeOpenedMessage() with trade {}, dispute {}", getClass().getSimpleName(), dispute.getTradeId(), dispute.getId()); - Trade trade = null; - String errorMessage = null; - PubKeyRing senderPubKeyRing = null; - try { - - // initialize - T disputeList = getDisputeList(); - if (disputeList == null) { - log.warn("disputes is null"); - return; - } - dispute.setSupportType(message.getSupportType()); - dispute.setState(Dispute.State.NEW); - Contract contract = dispute.getContract(); - - // validate dispute + // get trade + Trade trade = tradeManager.getTrade(dispute.getTradeId()); + if (trade == null) { + log.warn("Dispute trade {} does not exist", dispute.getTradeId()); + return; + } + + synchronized (trade) { + String errorMessage = null; + PubKeyRing senderPubKeyRing = null; try { - DisputeValidation.validateDisputeData(dispute); - DisputeValidation.validateNodeAddresses(dispute, config); - DisputeValidation.validateSenderNodeAddress(dispute, message.getSenderNodeAddress()); - DisputeValidation.validatePaymentAccountPayload(dispute); - //DisputeValidation.testIfDisputeTriesReplay(dispute, disputeList.getList()); - } catch (DisputeValidation.ValidationException e) { - validationExceptions.add(e); - throw e; - } - - // get trade - trade = tradeManager.getTrade(dispute.getTradeId()); - if (trade == null) { - log.warn("Dispute trade {} does not exist", dispute.getTradeId()); - return; - } - - // get sender - senderPubKeyRing = trade.isArbitrator() ? (dispute.isDisputeOpenerIsBuyer() ? contract.getBuyerPubKeyRing() : contract.getSellerPubKeyRing()) : trade.getArbitrator().getPubKeyRing(); - TradePeer sender = trade.getTradePeer(senderPubKeyRing); - if (sender == null) throw new RuntimeException("Pub key ring is not from arbitrator, buyer, or seller"); - - // message to trader is expected from arbitrator - if (!trade.isArbitrator() && sender != trade.getArbitrator()) { - throw new RuntimeException(message.getClass().getSimpleName() + " to trader is expected only from arbitrator"); - } - - // arbitrator verifies signature of payment sent message if given - if (trade.isArbitrator() && message.getPaymentSentMessage() != null) { - HavenoUtils.verifyPaymentSentMessage(trade, message.getPaymentSentMessage()); - trade.getBuyer().setUpdatedMultisigHex(message.getPaymentSentMessage().getUpdatedMultisigHex()); - trade.advanceState(sender == trade.getBuyer() ? Trade.State.BUYER_SENT_PAYMENT_SENT_MSG : Trade.State.SELLER_RECEIVED_PAYMENT_SENT_MSG); - } - - // update multisig hex - if (message.getUpdatedMultisigHex() != null) sender.setUpdatedMultisigHex(message.getUpdatedMultisigHex()); - - // update peer node address - // TODO: tests can reuse the same addresses so nullify equal peer - sender.setNodeAddress(message.getSenderNodeAddress()); - - // add chat message with price info - if (trade instanceof ArbitratorTrade) addPriceInfoMessage(dispute, 0); - - // add dispute - synchronized (disputeList) { - if (!disputeList.contains(dispute)) { - Optional storedDisputeOptional = findDispute(dispute); - if (!storedDisputeOptional.isPresent()) { - disputeList.add(dispute); - trade.advanceDisputeState(Trade.DisputeState.DISPUTE_OPENED); - - // send dispute opened message to peer if arbitrator - if (trade.isArbitrator()) sendDisputeOpenedMessageToPeer(dispute, contract, dispute.isDisputeOpenerIsBuyer() ? contract.getSellerPubKeyRing() : contract.getBuyerPubKeyRing(), trade.getSelf().getUpdatedMultisigHex()); - tradeManager.requestPersistence(); - errorMessage = null; - } else { - // valid case if both have opened a dispute and agent was not online - log.debug("We got a dispute already open for that trade and trading peer. TradeId = {}", - dispute.getTradeId()); - } - - // add chat message with mediation info if applicable - addMediationResultMessage(dispute); - } else { - throw new RuntimeException("We got a dispute msg that we have already stored. TradeId = " + dispute.getTradeId()); + + // initialize + T disputeList = getDisputeList(); + if (disputeList == null) { + log.warn("disputes is null"); + return; } + dispute.setSupportType(message.getSupportType()); + dispute.setState(Dispute.State.NEW); + Contract contract = dispute.getContract(); + + // validate dispute + try { + DisputeValidation.validateDisputeData(dispute); + DisputeValidation.validateNodeAddresses(dispute, config); + DisputeValidation.validateSenderNodeAddress(dispute, message.getSenderNodeAddress()); + DisputeValidation.validatePaymentAccountPayload(dispute); + //DisputeValidation.testIfDisputeTriesReplay(dispute, disputeList.getList()); + } catch (DisputeValidation.ValidationException e) { + validationExceptions.add(e); + throw e; + } + + // get sender + senderPubKeyRing = trade.isArbitrator() ? (dispute.isDisputeOpenerIsBuyer() ? contract.getBuyerPubKeyRing() : contract.getSellerPubKeyRing()) : trade.getArbitrator().getPubKeyRing(); + TradePeer sender = trade.getTradePeer(senderPubKeyRing); + if (sender == null) throw new RuntimeException("Pub key ring is not from arbitrator, buyer, or seller"); + + // message to trader is expected from arbitrator + if (!trade.isArbitrator() && sender != trade.getArbitrator()) { + throw new RuntimeException(message.getClass().getSimpleName() + " to trader is expected only from arbitrator"); + } + + // arbitrator verifies signature of payment sent message if given + if (trade.isArbitrator() && message.getPaymentSentMessage() != null) { + HavenoUtils.verifyPaymentSentMessage(trade, message.getPaymentSentMessage()); + trade.getBuyer().setUpdatedMultisigHex(message.getPaymentSentMessage().getUpdatedMultisigHex()); + trade.advanceState(sender == trade.getBuyer() ? Trade.State.BUYER_SENT_PAYMENT_SENT_MSG : Trade.State.SELLER_RECEIVED_PAYMENT_SENT_MSG); + } + + // update multisig hex + if (message.getUpdatedMultisigHex() != null) sender.setUpdatedMultisigHex(message.getUpdatedMultisigHex()); + + // update peer node address + // TODO: tests can reuse the same addresses so nullify equal peer + sender.setNodeAddress(message.getSenderNodeAddress()); + + // add chat message with price info + if (trade instanceof ArbitratorTrade) addPriceInfoMessage(dispute, 0); + + // add dispute + synchronized (disputeList) { + if (!disputeList.contains(dispute)) { + Optional storedDisputeOptional = findDispute(dispute); + if (!storedDisputeOptional.isPresent()) { + disputeList.add(dispute); + trade.advanceDisputeState(Trade.DisputeState.DISPUTE_OPENED); + + // send dispute opened message to peer if arbitrator + if (trade.isArbitrator()) sendDisputeOpenedMessageToPeer(dispute, contract, dispute.isDisputeOpenerIsBuyer() ? contract.getSellerPubKeyRing() : contract.getBuyerPubKeyRing(), trade.getSelf().getUpdatedMultisigHex()); + tradeManager.requestPersistence(); + errorMessage = null; + } else { + // valid case if both have opened a dispute and agent was not online + log.debug("We got a dispute already open for that trade and trading peer. TradeId = {}", + dispute.getTradeId()); + } + + // add chat message with mediation info if applicable + addMediationResultMessage(dispute); + } else { + throw new RuntimeException("We got a dispute msg that we have already stored. TradeId = " + dispute.getTradeId()); + } + } + } catch (Exception e) { + errorMessage = e.getMessage(); + log.warn(errorMessage); + if (trade != null) trade.setErrorMessage(errorMessage); } - } catch (Exception e) { - errorMessage = e.getMessage(); - log.warn(errorMessage); - if (trade != null) trade.setErrorMessage(errorMessage); + + // use chat message instead of open dispute message for the ack + ObservableList messages = message.getDispute().getChatMessages(); + if (!messages.isEmpty()) { + ChatMessage msg = messages.get(0); + sendAckMessage(msg, senderPubKeyRing, errorMessage == null, errorMessage); + } + + requestPersistence(); } - - // use chat message instead of open dispute message for the ack - ObservableList messages = message.getDispute().getChatMessages(); - if (!messages.isEmpty()) { - ChatMessage msg = messages.get(0); - sendAckMessage(msg, senderPubKeyRing, errorMessage == null, errorMessage); - } - - requestPersistence(); } // arbitrator sends dispute opened message to opener's peer @@ -700,16 +701,13 @@ public abstract class DisputeManager> extends Sup } // arbitrator sends result to trader when their dispute is closed - public void closeDisputeTicket(DisputeResult disputeResult, Dispute dispute, String summaryText, MoneroTxWallet payoutTx, ResultHandler resultHandler, FaultHandler faultHandler) { + public void closeDisputeTicket(DisputeResult disputeResult, Dispute dispute, String summaryText, ResultHandler resultHandler, FaultHandler faultHandler) { try { // get trade Trade trade = tradeManager.getTrade(dispute.getTradeId()); if (trade == null) throw new RuntimeException("Dispute trade " + dispute.getTradeId() + " does not exist"); - // create dispute payout tx if not given - if (payoutTx == null) payoutTx = createDisputePayoutTx(trade, dispute.getContract(), disputeResult, false); // can be null if already published or we don't have receiver's multisig hex - // persist result in dispute's chat message once boolean resending = disputeResult.getChatMessage() != null; if (!resending) { @@ -724,9 +722,15 @@ public abstract class DisputeManager> extends Sup dispute.addAndPersistChatMessage(chatMessage); } - // create dispute closed message + // create dispute payout tx if not published TradePeer receiver = trade.getTradePeer(dispute.getTraderPubKeyRing()); - String unsignedPayoutTxHex = payoutTx == null ? null : payoutTx.getTxSet().getMultisigTxHex(); + if (!trade.isPayoutPublished() && receiver.getUpdatedMultisigHex() != null) { + trade.getProcessModel().setUnsignedPayoutTx(createDisputePayoutTx(trade, dispute.getContract(), disputeResult, false)); // can be null if we don't have receiver's multisig hex + } + + // create dispute closed message + MoneroTxWallet unsignedPayoutTx = receiver.getUpdatedMultisigHex() == null ? null : trade.getProcessModel().getUnsignedPayoutTx(); + String unsignedPayoutTxHex = unsignedPayoutTx == null ? null : unsignedPayoutTx.getTxSet().getMultisigTxHex(); TradePeer receiverPeer = receiver == trade.getBuyer() ? trade.getSeller() : trade.getBuyer(); boolean deferPublishPayout = !resending && unsignedPayoutTxHex != null && receiverPeer.getUpdatedMultisigHex() != null && trade.getDisputeState().ordinal() >= Trade.DisputeState.ARBITRATOR_SAW_ARRIVED_DISPUTE_CLOSED_MSG.ordinal() ; DisputeClosedMessage disputeClosedMessage = new DisputeClosedMessage(disputeResult, @@ -799,9 +803,9 @@ public abstract class DisputeManager> extends Sup ); // save state - if (payoutTx != null) { - trade.setPayoutTx(payoutTx); - trade.setPayoutTxHex(payoutTx.getTxSet().getMultisigTxHex()); + if (unsignedPayoutTx != null) { + trade.setPayoutTx(unsignedPayoutTx); + trade.setPayoutTxHex(unsignedPayoutTx.getTxSet().getMultisigTxHex()); } trade.advanceDisputeState(Trade.DisputeState.ARBITRATOR_SENT_DISPUTE_CLOSED_MSG); requestPersistence(); diff --git a/core/src/main/java/bisq/core/trade/protocol/ProcessModel.java b/core/src/main/java/bisq/core/trade/protocol/ProcessModel.java index 82ce683efe..989a2d6a08 100644 --- a/core/src/main/java/bisq/core/trade/protocol/ProcessModel.java +++ b/core/src/main/java/bisq/core/trade/protocol/ProcessModel.java @@ -169,6 +169,9 @@ public class ProcessModel implements Model, PersistablePayload { @Getter @Setter transient private MoneroTxWallet depositTxXmr; + @Getter + @Setter + transient private MoneroTxWallet unsignedPayoutTx; @Nullable @Getter @Setter diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/ResendDisputeClosedMessageWithPayout.java b/core/src/main/java/bisq/core/trade/protocol/tasks/ResendDisputeClosedMessageWithPayout.java index 69791c061b..9961d3c071 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/ResendDisputeClosedMessageWithPayout.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/ResendDisputeClosedMessageWithPayout.java @@ -58,7 +58,8 @@ public class ResendDisputeClosedMessageWithPayout extends TradeTask { for (Dispute dispute : disputes) { if (!dispute.isClosed()) continue; // dispute must be closed if (sender.getPubKeyRing().equals(dispute.getTraderPubKeyRing())) { - HavenoUtils.arbitrationManager.closeDisputeTicket(dispute.getDisputeResultProperty().get(), dispute, dispute.getDisputeResultProperty().get().summaryNotesProperty().get(), null, () -> { + log.info("Arbitrator resending DisputeClosedMessage for trade {} after receiving updated multisig hex", trade.getId()); + HavenoUtils.arbitrationManager.closeDisputeTicket(dispute.getDisputeResultProperty().get(), dispute, dispute.getDisputeResultProperty().get().summaryNotesProperty().get(), () -> { completeAux(); }, (errMessage, err) -> { err.printStackTrace(); 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 6f856c25bc..ce85389fa9 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 @@ -590,8 +590,13 @@ public class DisputeSummaryWindow extends Overlay { closeTicketButton.setOnAction(e -> { - // get or create payout tx - MoneroTxWallet payoutTx = trade.isPayoutPublished() ? trade.getPayoutTx() : arbitrationManager.createDisputePayoutTx(trade, dispute.getContract(), disputeResult, false); + // get or create dispute payout tx + MoneroTxWallet payoutTx = null; + if (trade.isPayoutPublished()) payoutTx = trade.getPayoutTx(); + else { + payoutTx = arbitrationManager.createDisputePayoutTx(trade, dispute.getContract(), disputeResult, false); + trade.getProcessModel().setUnsignedPayoutTx(payoutTx); + } // show confirmation if (dispute.getSupportType() == SupportType.ARBITRATION && @@ -600,9 +605,9 @@ public class DisputeSummaryWindow extends Overlay { showPayoutTxConfirmation(contract, disputeResult, payoutTx, - () -> doClose(closeTicketButton, payoutTx)); + () -> doClose(closeTicketButton)); } else { - doClose(closeTicketButton, payoutTx); + doClose(closeTicketButton); } }); @@ -654,7 +659,7 @@ public class DisputeSummaryWindow extends Overlay { } } - private void doClose(Button closeTicketButton, MoneroTxWallet payoutTx) { + private void doClose(Button closeTicketButton) { DisputeManager> disputeManager = getDisputeManager(dispute); if (disputeManager == null) { return; @@ -663,7 +668,7 @@ public class DisputeSummaryWindow extends Overlay { summaryNotesTextArea.textProperty().unbindBidirectional(disputeResult.summaryNotesProperty()); disputeResult.setCloseDate(new Date()); - disputesService.closeDisputeTicket(disputeManager, dispute, disputeResult, payoutTx, () -> { + disputesService.closeDisputeTicket(disputeManager, dispute, disputeResult, () -> { if (peersDisputeOptional.isPresent() && !peersDisputeOptional.get().isClosed() && !DevEnv.isDevMode()) { new Popup().attention(Res.get("disputeSummaryWindow.close.closePeer")).show(); }