recover from payment received nack with updated multisig info

core/src/main/java/haveno/core/network/MessageState.java

@@ -23,5 +23,6 @@ public enum MessageState {
     ARRIVED,
     STORED_IN_MAILBOX,
     ACKNOWLEDGED,
-    FAILED
+    FAILED,
+    NACKED
 }

core/src/main/java/haveno/core/trade/SellerTrade.java

@@ -19,6 +19,7 @@ package haveno.core.trade;
 
 import haveno.core.offer.Offer;
 import haveno.core.trade.protocol.ProcessModel;
+import haveno.core.trade.protocol.SellerProtocol;
 import haveno.core.xmr.wallet.XmrWalletService;
 import haveno.network.p2p.NodeAddress;
 import lombok.extern.slf4j.Slf4j;
@@ -59,5 +60,9 @@ public abstract class SellerTrade extends Trade {
     public boolean confirmPermitted() {
         return true;
     }
+
+    public boolean isFinished() {
+        return super.isFinished() && ((SellerProtocol) getProtocol()).needsToResendPaymentReceivedMessages();
+    }
 }
 

core/src/main/java/haveno/core/trade/Trade.java

@@ -787,12 +787,15 @@ public abstract class Trade extends XmrWalletBase implements Tradable, Model {
     }
 
     public boolean isFinished() {
-        return isPayoutUnlocked() && isCompleted() && !getProtocol().needsToResendPaymentReceivedMessages();
+        return isPayoutUnlocked() && isCompleted();
     }
 
     public void resetToPaymentSentState() {
         setState(Trade.State.BUYER_SENT_PAYMENT_SENT_MSG);
-        for (TradePeer peer : getAllPeers()) peer.setPaymentReceivedMessage(null);
+        for (TradePeer peer : getAllPeers()) {
+            peer.setPaymentReceivedMessage(null);
+            peer.setPaymentReceivedMessageState(MessageState.UNDEFINED);
+        }
         setPayoutTxHex(null);
     }
 
@@ -2105,6 +2108,7 @@ public abstract class Trade extends XmrWalletBase implements Tradable, Model {
     private MessageState getPaymentSentMessageState() {
         if (isPaymentReceived()) return MessageState.ACKNOWLEDGED;
         if (getSeller().getPaymentSentMessageStateProperty().get() == MessageState.ACKNOWLEDGED) return MessageState.ACKNOWLEDGED;
+        if (getSeller().getPaymentSentMessageStateProperty().get() == MessageState.NACKED) return MessageState.NACKED;
         switch (state) {
             case BUYER_SENT_PAYMENT_SENT_MSG:
                 return MessageState.SENT;

core/src/main/java/haveno/core/trade/TradeManager.java

@@ -683,7 +683,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
                 if (!sender.equals(request.getTakerNodeAddress())) {
                     if (sender.equals(request.getMakerNodeAddress())) {
                         log.warn("Received InitTradeRequest from maker to arbitrator for trade that is already initializing, tradeId={}, sender={}", request.getOfferId(), sender);
-                        sendAckMessage(sender, trade.getMaker().getPubKeyRing(), request, false, "Trade is already initializing for " + getClass().getSimpleName() + " " + trade.getId());
+                        sendAckMessage(sender, trade.getMaker().getPubKeyRing(), request, false, "Trade is already initializing for " + getClass().getSimpleName() + " " + trade.getId(), null);
                     } else {
                         log.warn("Ignoring InitTradeRequest from non-taker, tradeId={}, sender={}", request.getOfferId(), sender);
                     }
@@ -1212,7 +1212,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
     // Getters, Utils
     ///////////////////////////////////////////////////////////////////////////////////////////
 
-    public void sendAckMessage(NodeAddress peer, PubKeyRing peersPubKeyRing, TradeMessage message, boolean result, @Nullable String errorMessage) {
+    public void sendAckMessage(NodeAddress peer, PubKeyRing peersPubKeyRing, TradeMessage message, boolean result, @Nullable String errorMessage, String updatedMultisigHex) {
 
         // create ack message
         String tradeId = message.getOfferId();
@@ -1223,7 +1223,8 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
                 sourceUid,
                 tradeId,
                 result,
-                errorMessage);
+                errorMessage,
+                updatedMultisigHex);
 
         // send ack message
         log.info("Send AckMessage for {} to peer {}. tradeId={}, sourceUid={}",

core/src/main/java/haveno/core/trade/protocol/ProcessModel.java

@@ -311,7 +311,7 @@ public class ProcessModel implements Model, PersistablePayload {
     void setDepositTxSentAckMessage(AckMessage ackMessage) {
         MessageState messageState = ackMessage.isSuccess() ?
                 MessageState.ACKNOWLEDGED :
-                MessageState.FAILED;
+                MessageState.NACKED;
         setDepositTxMessageState(messageState);
     }
 

core/src/main/java/haveno/core/trade/protocol/TradePeer.java

@@ -208,21 +208,21 @@ public final class TradePeer implements PersistablePayload {
     void setDepositsConfirmedAckMessage(AckMessage ackMessage) {
         MessageState messageState = ackMessage.isSuccess() ?
                 MessageState.ACKNOWLEDGED :
-                MessageState.FAILED;
+                MessageState.NACKED;
         setDepositsConfirmedMessageState(messageState);
     }
 
     void setPaymentSentAckMessage(AckMessage ackMessage) {
         MessageState messageState = ackMessage.isSuccess() ?
                 MessageState.ACKNOWLEDGED :
-                MessageState.FAILED;
+                MessageState.NACKED;
         setPaymentSentMessageState(messageState);
     }
 
     void setPaymentReceivedAckMessage(AckMessage ackMessage) {
         MessageState messageState = ackMessage.isSuccess() ?
                 MessageState.ACKNOWLEDGED :
-                MessageState.FAILED;
+                MessageState.NACKED;
         setPaymentReceivedMessageState(messageState);
     }
 
@@ -256,7 +256,7 @@ public final class TradePeer implements PersistablePayload {
     }
 
     public boolean isPaymentReceivedMessageReceived() {
-        return paymentReceivedMessageStateProperty.get() == MessageState.ACKNOWLEDGED || paymentReceivedMessageStateProperty.get() == MessageState.STORED_IN_MAILBOX;
+        return paymentReceivedMessageStateProperty.get() == MessageState.ACKNOWLEDGED || paymentReceivedMessageStateProperty.get() == MessageState.STORED_IN_MAILBOX || paymentReceivedMessageStateProperty.get() == MessageState.NACKED;
     }
 
     @Override

core/src/main/java/haveno/core/trade/protocol/TradeProtocol.java

@@ -42,6 +42,7 @@ import haveno.common.crypto.PubKeyRing;
 import haveno.common.handlers.ErrorMessageHandler;
 import haveno.common.proto.network.NetworkEnvelope;
 import haveno.common.taskrunner.Task;
+import haveno.core.network.MessageState;
 import haveno.core.trade.ArbitratorTrade;
 import haveno.core.trade.BuyerTrade;
 import haveno.core.trade.HavenoUtils;
@@ -272,7 +273,7 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
                                     handleTaskRunnerSuccess(null, null, "maybeSendDepositsConfirmedMessages");
                                 },
                                 (errorMessage) -> {
-                                    handleTaskRunnerFault(null, null, "maybeSendDepositsConfirmedMessages", errorMessage);
+                                    handleTaskRunnerFault(null, null, "maybeSendDepositsConfirmedMessages", errorMessage, null);
                                 })))
                         .executeTasks(true);
                 awaitTradeLatch();
@@ -280,10 +281,6 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
         }, trade.getId());
     }
 
-    public boolean needsToResendPaymentReceivedMessages() {
-        return false; // seller protocol overrides
-    }
-
     public void maybeReprocessPaymentSentMessage(boolean reprocessOnError) {
         if (trade.isShutDownStarted()) return;
         ThreadUtils.execute(() -> {
@@ -627,6 +624,11 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
             ThreadUtils.execute(() -> {
                 synchronized (trade.getLock()) {
                     if (!trade.isInitialized() || trade.isShutDownStarted()) return;
+                    if (trade.getPhase().ordinal() >= Trade.Phase.PAYMENT_RECEIVED.ordinal()) {
+                        log.warn("Received another PaymentReceivedMessage which was already processed for {} {}, ACKing", trade.getClass().getSimpleName(), trade.getId());
+                        handleTaskRunnerSuccess(peer, message);
+                        return;
+                    }
                     latchTrade();
                     Validator.checkTradeId(processModel.getOfferId(), message);
                     processModel.setTradeMessage(message);
@@ -665,7 +667,7 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
                                             maybeReprocessPaymentReceivedMessage(reprocessOnError);
                                         }, trade.getReprocessDelayInSeconds(reprocessPaymentReceivedMessageCount));
                                     } else {
-                                        handleTaskRunnerFault(peer, message, errorMessage); // otherwise send nack
+                                        handleTaskRunnerFault(peer, message, null, errorMessage, trade.getSelf().getUpdatedMultisigHex()); // otherwise send nack
                                     }
                                     unlatchTrade();
                                 })))
@@ -694,7 +696,8 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
                         handleTaskRunnerFault(null,
                                 null,
                                 result.name(),
-                                result.getInfo());
+                                result.getInfo(),
+                                null);
                     }
                 });
     }
@@ -755,7 +758,7 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
             peer.setNodeAddress(sender);
         }
 
-        // set trade state on deposit request nack
+        // handle nack of deposit request
         if (ackMessage.getSourceMsgClassName().equals(DepositRequest.class.getSimpleName())) {
             if (!ackMessage.isSuccess()) {
                 trade.setStateIfValidTransitionTo(Trade.State.PUBLISH_DEPOSIT_TX_REQUEST_FAILED);
@@ -763,13 +766,13 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
             }
         }
 
-        // handle ack for DepositsConfirmedMessage, which automatically re-sends if not ACKed in a certain time
+        // handle ack message for DepositsConfirmedMessage, which automatically re-sends if not ACKed in a certain time
         if (ackMessage.getSourceMsgClassName().equals(DepositsConfirmedMessage.class.getSimpleName())) {
             peer.setDepositsConfirmedAckMessage(ackMessage);
             processModel.getTradeManager().requestPersistence();
         }
 
-        // handle ack for PaymentSentMessage, which automatically re-sends if not ACKed in a certain time
+        // handle ack message for PaymentSentMessage, which automatically re-sends if not ACKed in a certain time
         if (ackMessage.getSourceMsgClassName().equals(PaymentSentMessage.class.getSimpleName())) {
             if (trade.getTradePeer(sender) == trade.getSeller()) {
                 trade.getSeller().setPaymentSentAckMessage(ackMessage);
@@ -785,15 +788,55 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
             }
         }
 
-        // handle ack for PaymentReceivedMessage, which automatically re-sends if not ACKed in a certain time
+        // handle ack message for PaymentReceivedMessage, which automatically re-sends if not ACKed in a certain time
         if (ackMessage.getSourceMsgClassName().equals(PaymentReceivedMessage.class.getSimpleName())) {
+
+            // ack message from buyer
             if (trade.getTradePeer(sender) == trade.getBuyer()) {
                 trade.getBuyer().setPaymentReceivedAckMessage(ackMessage);
-                if (ackMessage.isSuccess()) trade.setStateIfValidTransitionTo(Trade.State.BUYER_RECEIVED_PAYMENT_RECEIVED_MSG);
+
-                else trade.setState(Trade.State.SELLER_SEND_FAILED_PAYMENT_RECEIVED_MSG);
+                // handle successful ack
+                if (ackMessage.isSuccess()) {
+                    trade.setStateIfValidTransitionTo(Trade.State.BUYER_RECEIVED_PAYMENT_RECEIVED_MSG);
+                }
+                
+                // handle nack
+                else {
+                    log.warn("We received a NACK for our PaymentReceivedMessage to the buyer for {} {}", trade.getClass().getSimpleName(), trade.getId());
+                    
+                    // update multisig hex
+                    if (ackMessage.getUpdatedMultisigHex() != null) {
+                        trade.getBuyer().setUpdatedMultisigHex(ackMessage.getUpdatedMultisigHex());
+                    }
+
+                    // reset state if not processed
+                    if (trade.isPaymentReceived() && !trade.isPayoutPublished() && !isPaymentReceivedMessageAckedByEither()) {
+                        log.warn("Resetting state to payment sent for {} {}", trade.getClass().getSimpleName(), trade.getId());
+                        trade.resetToPaymentSentState();
+                    }
+                }
                 processModel.getTradeManager().requestPersistence();
-            } else if (trade.getTradePeer(sender) == trade.getArbitrator()) {
+            }
+            
+            // ack message from arbitrator
+            else if (trade.getTradePeer(sender) == trade.getArbitrator()) {
                 trade.getArbitrator().setPaymentReceivedAckMessage(ackMessage);
+
+                // handle nack
+                if (!ackMessage.isSuccess()) {
+                    log.warn("We received a NACK for our PaymentReceivedMessage to the arbitrator for {} {}", trade.getClass().getSimpleName(), trade.getId());
+
+                    // update multisig hex
+                    if (ackMessage.getUpdatedMultisigHex() != null) {
+                        trade.getArbitrator().setUpdatedMultisigHex(ackMessage.getUpdatedMultisigHex());
+                    }
+
+                    // reset state if not processed
+                    if (trade.isPaymentReceived() && !trade.isPayoutPublished() && !isPaymentReceivedMessageAckedByEither()) {
+                        log.warn("Resetting state to payment sent for {} {}", trade.getClass().getSimpleName(), trade.getId());
+                        trade.resetToPaymentSentState();
+                    }
+                }
                 processModel.getTradeManager().requestPersistence();
             } else {
                 log.warn("Received AckMessage from unexpected peer for {}, sender={}, trade={} {}, messageUid={}, success={}, errorMsg={}", ackMessage.getSourceMsgClassName(), sender, trade.getClass().getSimpleName(), trade.getId(), ackMessage.getSourceUid(), ackMessage.isSuccess(), ackMessage.getErrorMessage());
@@ -813,7 +856,17 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
         trade.onAckMessage(ackMessage, sender);
     }
 
+    private boolean isPaymentReceivedMessageAckedByEither() {
+        if (trade.getBuyer().getPaymentReceivedMessageStateProperty().get() == MessageState.ACKNOWLEDGED) return true;
+        if (trade.getArbitrator().getPaymentReceivedMessageStateProperty().get() == MessageState.ACKNOWLEDGED) return true;
+        return false;
+    }
+
     protected void sendAckMessage(NodeAddress peer, TradeMessage message, boolean result, @Nullable String errorMessage) {
+        sendAckMessage(peer, message, result, errorMessage, null);
+    }
+
+    protected void sendAckMessage(NodeAddress peer, TradeMessage message, boolean result, @Nullable String errorMessage, String updatedMultisigHex) {
 
         // get peer's pub key ring
         PubKeyRing peersPubKeyRing = getPeersPubKeyRing(peer);
@@ -823,7 +876,7 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
         }
 
         // send ack message
-        processModel.getTradeManager().sendAckMessage(peer, peersPubKeyRing, message, result, errorMessage);
+        processModel.getTradeManager().sendAckMessage(peer, peersPubKeyRing, message, result, errorMessage, updatedMultisigHex);
     }
 
 
@@ -870,11 +923,11 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
     }
 
     protected void handleTaskRunnerFault(NodeAddress sender, TradeMessage message, String errorMessage) {
-        handleTaskRunnerFault(sender, message, message.getClass().getSimpleName(), errorMessage);
+        handleTaskRunnerFault(sender, message, message.getClass().getSimpleName(), errorMessage, null);
     }
 
     protected void handleTaskRunnerFault(FluentProtocol.Event event, String errorMessage) {
-        handleTaskRunnerFault(null, null, event.name(), errorMessage);
+        handleTaskRunnerFault(null, null, event.name(), errorMessage, null);
     }
 
 
@@ -936,11 +989,11 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
         unlatchTrade();
     }
 
-    void handleTaskRunnerFault(NodeAddress ackReceiver, @Nullable TradeMessage message, String source, String errorMessage) {
+    void handleTaskRunnerFault(NodeAddress ackReceiver, @Nullable TradeMessage message, String source, String errorMessage, String updatedMultisigHex) {
         log.error("Task runner failed with error {}. Triggered from {}. Monerod={}" , errorMessage, source, trade.getXmrWalletService().getXmrConnectionService().getConnection());
 
         if (message != null) {
-            sendAckMessage(ackReceiver, message, false, errorMessage);
+            sendAckMessage(ackReceiver, message, false, errorMessage, updatedMultisigHex);
         }
 
         handleError(errorMessage);

core/src/main/java/haveno/core/trade/protocol/tasks/SellerSendPaymentReceivedMessage.java

@@ -90,8 +90,8 @@ public abstract class SellerSendPaymentReceivedMessage extends SendMailboxMessag
         try {
             runInterceptHook();
 
-            // skip if already received
+            // skip if stopped
-            if (isReceived()) {
+            if (stopSending()) {
                 if (!isCompleted()) complete();
                 return;
             }
@@ -191,8 +191,8 @@ public abstract class SellerSendPaymentReceivedMessage extends SendMailboxMessag
 
     private void tryToSendAgainLater() {
 
-        // skip if already received
+        // skip if stopped
-        if (isReceived()) return;
+        if (stopSending()) return;
 
         if (resendCounter >= MAX_RESEND_ATTEMPTS) {
             cleanup();
@@ -226,12 +226,16 @@ public abstract class SellerSendPaymentReceivedMessage extends SendMailboxMessag
     }
 
     private void onMessageStateChange(MessageState newValue) {
-        if (isReceived()) {
+        if (isMessageReceived()) {
             cleanup();
         }
     }
 
-    protected boolean isReceived() {
+    protected boolean isMessageReceived() {
         return getReceiver().isPaymentReceivedMessageReceived();
     }
+
+    protected boolean stopSending() {
+        return isMessageReceived() || !trade.isPaymentReceived(); // stop if received or trade state reset // TODO: also stop after some number of blocks?
+    }
 }

desktop/src/main/java/haveno/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep3View.java

@@ -112,6 +112,7 @@ public class BuyerStep3View extends TradeStepView {
                 iconLabel.getStyleClass().add("trade-msg-state-stored");
                 break;
             case FAILED:
+            case NACKED:
                 textFieldWithIcon.setIcon(AwesomeIcon.EXCLAMATION_SIGN);
                 iconLabel.getStyleClass().add("trade-msg-state-acknowledged");
                 break;

desktop/src/main/java/haveno/desktop/main/portfolio/pendingtrades/steps/seller/SellerStep3View.java

@@ -151,6 +151,9 @@ public class SellerStep3View extends TradeStepView {
                         break;
                 }
             }
+
+            // update confirm button state
+            confirmButton.setDisable(!confirmPaymentReceivedPermitted());
         });
     }
 

p2p/src/main/java/haveno/network/p2p/AckMessage.java

@@ -53,6 +53,8 @@ public final class AckMessage extends NetworkEnvelope implements MailboxMessage,
     private final boolean success;
     @Nullable
     private final String errorMessage;
+    @Nullable
+    private final String updatedMultisigHex;
 
     /**
      *
@@ -79,6 +81,27 @@ public final class AckMessage extends NetworkEnvelope implements MailboxMessage,
                 sourceId,
                 success,
                 errorMessage,
+                null,
+                Version.getP2PMessageVersion());
+    }
+
+    public AckMessage(NodeAddress senderNodeAddress,
+                      AckMessageSourceType sourceType,
+                      String sourceMsgClassName,
+                      String sourceUid,
+                      String sourceId,
+                      boolean success,
+                      String errorMessage,
+                      String updatedMultisigHex) {
+        this(UUID.randomUUID().toString(),
+                senderNodeAddress,
+                sourceType,
+                sourceMsgClassName,
+                sourceUid,
+                sourceId,
+                success,
+                errorMessage,
+                updatedMultisigHex,
                 Version.getP2PMessageVersion());
     }
 
@@ -95,6 +118,7 @@ public final class AckMessage extends NetworkEnvelope implements MailboxMessage,
                        String sourceId,
                        boolean success,
                        @Nullable String errorMessage,
+                       String updatedMultisigInfo,
                        String messageVersion) {
         super(messageVersion);
         this.uid = uid;
@@ -105,6 +129,7 @@ public final class AckMessage extends NetworkEnvelope implements MailboxMessage,
         this.sourceId = sourceId;
         this.success = success;
         this.errorMessage = errorMessage;
+        this.updatedMultisigHex = updatedMultisigInfo;
     }
 
     public protobuf.AckMessage toProtoMessage() {
@@ -126,6 +151,7 @@ public final class AckMessage extends NetworkEnvelope implements MailboxMessage,
                 .setSuccess(success);
         Optional.ofNullable(sourceUid).ifPresent(builder::setSourceUid);
         Optional.ofNullable(errorMessage).ifPresent(builder::setErrorMessage);
+        Optional.ofNullable(updatedMultisigHex).ifPresent(builder::setUpdatedMultisigHex);
         return builder;
     }
 
@@ -139,6 +165,7 @@ public final class AckMessage extends NetworkEnvelope implements MailboxMessage,
                 proto.getSourceId(),
                 proto.getSuccess(),
                 proto.getErrorMessage().isEmpty() ? null : proto.getErrorMessage(),
+                proto.getUpdatedMultisigHex().isEmpty() ? null : proto.getUpdatedMultisigHex(),
                 messageVersion);
     }
 
@@ -163,6 +190,7 @@ public final class AckMessage extends NetworkEnvelope implements MailboxMessage,
                 ",\n     sourceId='" + sourceId + '\'' +
                 ",\n     success=" + success +
                 ",\n     errorMessage='" + errorMessage + '\'' +
+                ",\n     updatedMultisigInfo='" + updatedMultisigHex + '\'' +
                 "\n} " + super.toString();
     }
 }

proto/src/main/proto/pb.proto

@@ -216,6 +216,7 @@ message AckMessage {
     string source_id = 6; // id of source (tradeId, disputeId)
     bool success = 7; // true if source message was processed successfully
     string error_message = 8; // optional error message if source message processing failed
+    string updated_multisig_hex = 9; // data to update the multisig state
 }
 
 message PrefixedSealedAndSignedMessage {