arbitrator sends nack if trade already taken

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

@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList;
 import common.utils.GenUtils;
 import haveno.common.ClockWatcher;
 import haveno.common.crypto.KeyRing;
+import haveno.common.crypto.PubKeyRing;
 import haveno.common.handlers.ErrorMessageHandler;
 import haveno.common.handlers.FaultHandler;
 import haveno.common.handlers.ResultHandler;
@@ -69,11 +70,14 @@ import haveno.core.user.User;
 import haveno.core.util.Validator;
 import haveno.core.xmr.model.XmrAddressEntry;
 import haveno.core.xmr.wallet.XmrWalletService;
+import haveno.network.p2p.AckMessage;
+import haveno.network.p2p.AckMessageSourceType;
 import haveno.network.p2p.BootstrapListener;
 import haveno.network.p2p.DecryptedDirectMessageListener;
 import haveno.network.p2p.DecryptedMessageWithPubKey;
 import haveno.network.p2p.NodeAddress;
 import haveno.network.p2p.P2PService;
+import haveno.network.p2p.SendMailboxMessageListener;
 import haveno.network.p2p.network.TorNetworkNode;
 import javafx.beans.property.BooleanProperty;
 import javafx.beans.property.LongProperty;
@@ -553,6 +557,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
             return;
         }
 
+        // handle trade
         Trade trade;
         Optional<Trade> tradeOptional = getOpenTrade(offer.getId());
         if (tradeOptional.isPresent()) {
@@ -560,7 +565,11 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
 
             // verify request is from maker
             if (!sender.equals(request.getMakerNodeAddress())) {
-                  log.warn("Trade is already taken"); // TODO (woodser): need to respond with bad ack
+
+                // send nack if trade already taken
+                String errMsg = "Trade is already taken, tradeId=" + request.getTradeId();
+                log.warn(errMsg);
+                sendAckMessage(sender, request.getPubKeyRing(), request, false, errMsg);
                 return;
             }
         } else {
@@ -600,6 +609,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
             }
           }
 
+          // process with protocol
           ((ArbitratorProtocol) getTradeProtocol(trade)).handleInitTradeRequest(request, sender, errorMessage -> {
               log.warn("Arbitrator error during trade initialization for trade {}: {}", trade.getId(), errorMessage);
               maybeRemoveTradeOnError(trade);
@@ -643,6 +653,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
           // get expected taker fee
           BigInteger takerFee = HavenoUtils.getTakerFee(BigInteger.valueOf(request.getTradeAmount()));
 
+          // initialize trade
           Trade trade;
           if (offer.isBuyOffer())
               trade = new BuyerAsMakerTrade(offer,
@@ -687,6 +698,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
               }
           });
 
+          // process with protocol
           ((MakerProtocol) getTradeProtocol(trade)).handleInitTradeRequest(request, sender, errorMessage -> {
               log.warn("Maker error during trade initialization: " + errorMessage);
               maybeRemoveTradeOnError(trade);
@@ -872,7 +884,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
 
                         initTradeAndProtocol(trade, tradeProtocol);
 
-                        // take offer and persist trade on success
+                        // process with protocol
                         ((TakerProtocol) tradeProtocol).onTakeOffer(result -> {
                             tradeResultHandler.handleResult(trade);
                             requestPersistence();
@@ -1115,6 +1127,48 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
     // Getters, Utils
     ///////////////////////////////////////////////////////////////////////////////////////////
 
+    public void sendAckMessage(NodeAddress peer, PubKeyRing peersPubKeyRing, TradeMessage message, boolean result, @Nullable String errorMessage) {
+        
+        // create ack message
+        String tradeId = message.getTradeId();
+        String sourceUid = message.getUid();
+        AckMessage ackMessage = new AckMessage(P2PService.getMyNodeAddress(),
+                AckMessageSourceType.TRADE_MESSAGE,
+                message.getClass().getSimpleName(),
+                sourceUid,
+                tradeId,
+                result,
+                errorMessage);
+
+        // send ack message
+        log.info("Send AckMessage for {} to peer {}. tradeId={}, sourceUid={}",
+                ackMessage.getSourceMsgClassName(), peer, tradeId, sourceUid);
+        p2PService.getMailboxMessageService().sendEncryptedMailboxMessage(
+                peer,
+                peersPubKeyRing,
+                ackMessage,
+                new SendMailboxMessageListener() {
+                    @Override
+                    public void onArrived() {
+                        log.info("AckMessage for {} arrived at peer {}. tradeId={}, sourceUid={}",
+                                ackMessage.getSourceMsgClassName(), peer, tradeId, sourceUid);
+                    }
+
+                    @Override
+                    public void onStoredInMailbox() {
+                        log.info("AckMessage for {} stored in mailbox for peer {}. tradeId={}, sourceUid={}",
+                                ackMessage.getSourceMsgClassName(), peer, tradeId, sourceUid);
+                    }
+
+                    @Override
+                    public void onFault(String errorMessage) {
+                        log.error("AckMessage for {} failed. Peer {}. tradeId={}, sourceUid={}, errorMessage={}",
+                                ackMessage.getSourceMsgClassName(), peer, tradeId, sourceUid, errorMessage);
+                    }
+                }
+        );
+    }
+
     public ObservableList<Trade> getObservableList() {
         synchronized (tradableList) {
             return tradableList.getObservableList();

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

@@ -62,7 +62,6 @@ import haveno.network.p2p.AckMessageSourceType;
 import haveno.network.p2p.DecryptedDirectMessageListener;
 import haveno.network.p2p.DecryptedMessageWithPubKey;
 import haveno.network.p2p.NodeAddress;
-import haveno.network.p2p.SendMailboxMessageListener;
 import haveno.network.p2p.mailbox.MailboxMessage;
 import haveno.network.p2p.mailbox.MailboxMessageService;
 import haveno.network.p2p.messaging.DecryptedMailboxListener;
@@ -666,7 +665,7 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
                 processModel.getTradeManager().requestPersistence();
             }
 
-            handleError(err);
+            handleError(ackMessage.getErrorMessage());
         }
     }
 
@@ -679,43 +678,10 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
             return;
         }
 
-        String tradeId = message.getTradeId();
+        // send ack message
-        String sourceUid = message.getUid();
+        processModel.getTradeManager().sendAckMessage(peer, peersPubKeyRing, message, result, errorMessage);
-        AckMessage ackMessage = new AckMessage(processModel.getMyNodeAddress(),
-                AckMessageSourceType.TRADE_MESSAGE,
-                message.getClass().getSimpleName(),
-                sourceUid,
-                tradeId,
-                result,
-                errorMessage);
-
-        log.info("Send AckMessage for {} to peer {}. tradeId={}, sourceUid={}",
-                ackMessage.getSourceMsgClassName(), peer, tradeId, sourceUid);
-        processModel.getP2PService().getMailboxMessageService().sendEncryptedMailboxMessage(
-                peer,
-                peersPubKeyRing,
-                ackMessage,
-                new SendMailboxMessageListener() {
-                    @Override
-                    public void onArrived() {
-                        log.info("AckMessage for {} arrived at peer {}. tradeId={}, sourceUid={}",
-                                ackMessage.getSourceMsgClassName(), peer, tradeId, sourceUid);
     }
 
-                    @Override
-                    public void onStoredInMailbox() {
-                        log.info("AckMessage for {} stored in mailbox for peer {}. tradeId={}, sourceUid={}",
-                                ackMessage.getSourceMsgClassName(), peer, tradeId, sourceUid);
-                    }
-
-                    @Override
-                    public void onFault(String errorMessage) {
-                        log.error("AckMessage for {} failed. Peer {}. tradeId={}, sourceUid={}, errorMessage={}",
-                                ackMessage.getSourceMsgClassName(), peer, tradeId, sourceUid, errorMessage);
-                    }
-                }
-        );
-    }
 
     ///////////////////////////////////////////////////////////////////////////////////////////
     // Timeout