fixes based on refactored tests (#454)

log trade id with error
open dispute chat on user thread
fix null arbitrator signature by copying offer payload before modifying

core/src/main/java/bisq/core/api/CoreOffersService.java

@@ -111,7 +111,9 @@ public class CoreOffersService {
                 .filter(o -> !o.isMyOffer(keyRing))
                 .filter(o -> {
                     Result result = offerFilter.canTakeOffer(o, coreContext.isApiUser());
-                    return result.isValid() || result == Result.HAS_NO_PAYMENT_ACCOUNT_VALID_FOR_OFFER;
+                    boolean valid = result.isValid() || result == Result.HAS_NO_PAYMENT_ACCOUNT_VALID_FOR_OFFER;
+                    if (!valid) log.warn("Cannot take offer " + o.getId() + " with invalid state : " + result);
+                    return valid;
                 })
                 .findAny().orElseThrow(() ->
                         new IllegalStateException(format("offer with id '%s' not found", id)));

core/src/main/java/bisq/core/btc/wallet/XmrWalletService.java

@@ -155,7 +155,7 @@ public class XmrWalletService {
     
     public MoneroWallet getWallet() {
         State state = walletsSetup.getWalletConfig().state();
-        checkState(state == State.STARTING || state == State.RUNNING, "Cannot call until startup is complete");
+        checkState(state == State.STARTING || state == State.RUNNING, "Cannot call until startup is complete, but state is: " + state);
         return wallet;
     }
 

core/src/main/java/bisq/core/offer/OfferFilterService.java

@@ -224,6 +224,6 @@ public class OfferFilterService {
     public boolean hasValidSignature(Offer offer) {
         Arbitrator arbitrator = user.getAcceptedArbitratorByAddress(offer.getOfferPayload().getArbitratorSigner());
         if (arbitrator == null) return false; // invalid arbitrator
-        return TradeUtils.isArbitratorSignatureValid(offer.getOfferPayload(), arbitrator);
+        return TradeUtils.isArbitratorSignatureValid(offer, arbitrator);
     }
 }

core/src/main/java/bisq/core/offer/placeoffer/tasks/AddToOfferBook.java

@@ -23,6 +23,8 @@ import bisq.core.offer.placeoffer.PlaceOfferModel;
 import bisq.common.taskrunner.Task;
 import bisq.common.taskrunner.TaskRunner;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 public class AddToOfferBook extends Task<PlaceOfferModel> {
 
     public AddToOfferBook(TaskRunner<PlaceOfferModel> taskHandler, PlaceOfferModel model) {
@@ -33,6 +35,7 @@ public class AddToOfferBook extends Task<PlaceOfferModel> {
     protected void run() {
         try {
             runInterceptHook();
+            checkNotNull(model.getSignOfferResponse().getSignedOfferPayload().getArbitratorSignature(), "Offer's arbitrator signature is null: " + model.getOffer().getId());
             model.getOfferBookService().addOffer(new Offer(model.getSignOfferResponse().getSignedOfferPayload()),
                     () -> {
                         model.setOfferAddedToOfferBook(true);

core/src/main/java/bisq/core/offer/placeoffer/tasks/MakerProcessSignOfferResponse.java

@@ -38,9 +38,11 @@ public class MakerProcessSignOfferResponse extends Task<PlaceOfferModel> {
         try {
             runInterceptHook();
             
-            // validate arbitrator signature
+            // get arbitrator
             Arbitrator arbitrator = checkNotNull(model.getUser().getAcceptedArbitratorByAddress(offer.getOfferPayload().getArbitratorSigner()), "user.getAcceptedArbitratorByAddress(arbitratorSigner) must not be null");
-            if (!TradeUtils.isArbitratorSignatureValid(model.getSignOfferResponse().getSignedOfferPayload(), arbitrator)) {
+            
+            // validate arbitrator signature
+            if (!TradeUtils.isArbitratorSignatureValid(new Offer(model.getSignOfferResponse().getSignedOfferPayload()), arbitrator)) {
                 throw new RuntimeException("Offer payload has invalid arbitrator signature");
             }
             

core/src/main/java/bisq/core/support/dispute/arbitration/ArbitrationManager.java

@@ -669,7 +669,7 @@ public final class ArbitrationManager extends DisputeManager<ArbitrationDisputeL
     public static MoneroTxWallet arbitratorCreatesDisputedPayoutTx(Contract contract, Dispute dispute, DisputeResult disputeResult, MoneroWallet multisigWallet) {
 
         // multisig wallet must be synced
-        if (multisigWallet.isMultisigImportNeeded()) throw new RuntimeException("Arbitrator's wallet needs updated multisig hex to create payout tx which means a trader must have already broadcast the payout tx");
+        if (multisigWallet.isMultisigImportNeeded()) throw new RuntimeException("Arbitrator's wallet needs updated multisig hex to create payout tx which means a trader must have already broadcast the payout tx for trade " + dispute.getTradeId());
 
         // collect winner and loser payout address and amounts
         String winnerPayoutAddress = disputeResult.getWinner() == Winner.BUYER ?

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

@@ -127,8 +127,8 @@ public abstract class Trade implements Tradable, Model {
         SEND_FAILED_PUBLISH_DEPOSIT_TX_REQUEST(Phase.DEPOSIT_REQUESTED),
 
         // deposit published
-        DEPOSIT_TXS_SEEN_IN_BLOCKCHAIN(Phase.DEPOSITS_PUBLISHED), // TODO: seeing in network usually happens after arbitrator publishes
         ARBITRATOR_PUBLISHED_DEPOSIT_TXS(Phase.DEPOSITS_PUBLISHED),
+        DEPOSIT_TXS_SEEN_IN_BLOCKCHAIN(Phase.DEPOSITS_PUBLISHED),
 
         // deposit confirmed
         DEPOSIT_TXS_CONFIRMED_IN_BLOCKCHAIN(Phase.DEPOSITS_CONFIRMED),

core/src/main/java/bisq/core/trade/TradeUtils.java

@@ -23,6 +23,7 @@ import bisq.common.crypto.PubKeyRing;
 import bisq.common.crypto.Sig;
 import bisq.common.util.Tuple2;
 import bisq.core.btc.wallet.XmrWalletService;
+import bisq.core.offer.Offer;
 import bisq.core.offer.OfferPayload;
 import bisq.core.support.dispute.arbitration.arbitrator.Arbitrator;
 import bisq.core.trade.messages.InitTradeRequest;
@@ -74,18 +75,21 @@ public class TradeUtils {
     /**
      * Check if the arbitrator signature for an offer is valid.
      * 
-     * @param signedOfferPayload is a signed offer payload
+     * @param offer is a signed offer with payload
      * @param arbitrator is the possible original arbitrator
      * @return true if the arbitrator's signature is valid for the offer
      */
-    public static boolean isArbitratorSignatureValid(OfferPayload signedOfferPayload, Arbitrator arbitrator) {
+    public static boolean isArbitratorSignatureValid(Offer offer, Arbitrator arbitrator) {
+        
+        // copy offer payload
+        OfferPayload offerPayloadCopy = OfferPayload.fromProto(offer.toProtoMessage().getOfferPayload());
         
         // remove arbitrator signature from signed payload
-        String signature = signedOfferPayload.getArbitratorSignature();
+        String signature = offerPayloadCopy.getArbitratorSignature();
-        signedOfferPayload.setArbitratorSignature(null);
+        offerPayloadCopy.setArbitratorSignature(null);
         
         // get unsigned offer payload as json string
-        String unsignedOfferAsJson = JsonUtil.objectToJson(signedOfferPayload);
+        String unsignedOfferAsJson = JsonUtil.objectToJson(offerPayloadCopy);
         
         // verify arbitrator signature
         boolean isValid = true;
@@ -95,9 +99,6 @@ public class TradeUtils {
             isValid = false;
         }
         
-        // replace signature
-        signedOfferPayload.setArbitratorSignature(signature);
-        
         // return result
         return isValid;
     }

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

@@ -335,7 +335,7 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
                 latchTrade();
                 Validator.checkTradeId(processModel.getOfferId(), response);
                 processModel.setTradeMessage(response);
-                expect(state(Trade.State.SAW_ARRIVED_PUBLISH_DEPOSIT_TX_REQUEST)
+                expect(anyState(Trade.State.SAW_ARRIVED_PUBLISH_DEPOSIT_TX_REQUEST, Trade.State.ARBITRATOR_PUBLISHED_DEPOSIT_TXS, Trade.State.DEPOSIT_TXS_SEEN_IN_BLOCKCHAIN)
                         .with(response)
                         .from(sender)) // TODO (woodser): ensure this asserts sender == response.getSenderNodeAddress()
                         .setup(tasks(

core/src/main/java/bisq/core/trade/protocol/tasks/ProcessDepositResponse.java

@@ -34,7 +34,7 @@ public class ProcessDepositResponse extends TradeTask {
     protected void run() {
         try {
           runInterceptHook();
-          trade.setState(Trade.State.ARBITRATOR_PUBLISHED_DEPOSIT_TXS);
+          trade.setStateIfValidTransitionTo(Trade.State.ARBITRATOR_PUBLISHED_DEPOSIT_TXS);
           processModel.getTradeManager().requestPersistence();
           complete();
         } catch (Throwable t) {

core/src/main/java/bisq/core/trade/protocol/tasks/ProcessSignContractResponse.java

@@ -50,6 +50,7 @@ public class ProcessSignContractResponse extends TradeTask {
             if (!contractAsJson.equals(response.getContractAsJson())) {
                 trade.getContract().printDiff(response.getContractAsJson());
                 failed("Contracts are not matching");
+                return;
             }
 
             // get peer info

desktop/src/main/java/bisq/desktop/main/support/dispute/DisputeChatPopup.java

@@ -85,7 +85,6 @@ public class DisputeChatPopup {
     }
 
     public void openChat(Dispute selectedDispute, DisputeSession concreteDisputeSession, String counterpartyName) {
-        UserThread.execute(() -> {
         closeChat();
         this.selectedDispute = selectedDispute;
         selectedDispute.getChatMessages().forEach(m -> m.setWasDisplayed(true));
@@ -157,6 +156,5 @@ public class DisputeChatPopup {
         // Delay display to next render frame to avoid that the popup is first quickly displayed in default position
         // and after a short moment in the correct position
         UserThread.execute(() -> chatPopupStage.setOpacity(1));
-        });
     }
 }

p2p/src/main/java/bisq/network/p2p/network/Connection.java

@@ -112,7 +112,6 @@ public class Connection implements HasCapabilities, Runnable, MessageListener {
     private static final int MAX_PERMITTED_MESSAGE_SIZE = 10 * 1024 * 1024;             // 10 MB (425 offers resulted in about 660 kb, mailbox msg will add more to it) offer has usually 2 kb, mailbox 3kb.
     //TODO decrease limits again after testing
     private static final int SOCKET_TIMEOUT = (int) TimeUnit.SECONDS.toMillis(180);
-    private static final int MAX_CONNECTION_THREADS = 10;
 
     public static int getPermittedMessageSize() {
         return PERMITTED_MESSAGE_SIZE;
@@ -131,7 +130,6 @@ public class Connection implements HasCapabilities, Runnable, MessageListener {
     @Getter
     private final String uid;
     private final ExecutorService singleThreadExecutor = Executors.newSingleThreadExecutor(runnable -> new Thread(runnable, "Connection.java executor-service"));
-    private final ExecutorService connectionThreadPool = Executors.newFixedThreadPool(MAX_CONNECTION_THREADS);
 
     // holder of state shared between InputHandler and Connection
     @Getter