[WIP] Add cancel trade feature

core/src/main/java/bisq/core/account/sign/SignedWitnessService.java

@@ -494,7 +494,10 @@ void addToMap(SignedWitness signedWitness) {
     private void publishSignedWitness(SignedWitness signedWitness) {
         if (!signedWitnessMap.containsKey(signedWitness.getHashAsByteArray())) {
             log.info("broadcast signed witness {}", signedWitness.toString());
-            p2PService.addPersistableNetworkPayload(signedWitness, false);
+            // We set reBroadcast to true. The signer has broadcast the signedWitness as
+            // well, so it might be that we have received the data already, but we prefer to re-broadcast
+            // to achieve better resilience.
+            p2PService.addPersistableNetworkPayload(signedWitness, true);
             addToMap(signedWitness);
         }
     }

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

@@ -227,6 +227,9 @@ public void onFiatPaymentReceived(ResultHandler resultHandler, ErrorMessageHandl
                         handleTaskRunnerFault(errorMessage);
                     });
 
+            // SellerSendPayoutTxPublishedMessage and SellerMaybeSignWitnessAndSendWitnessMessage are sending both a
+            // msg which should have been consolidated in 1 message to avoid that the peer has to process 2 messages
+            // arriving around the same time. This breaks the concept of a strict sequential task execution.
             taskRunner.addTasks(
                     ApplyFilter.class,
                     MakerVerifyTakerAccount.class,

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

@@ -218,6 +218,9 @@ public void onFiatPaymentReceived(ResultHandler resultHandler, ErrorMessageHandl
                         handleTaskRunnerFault(errorMessage);
                     });
 
+            // SellerSendPayoutTxPublishedMessage and SellerMaybeSignWitnessAndSendWitnessMessage are sending both a
+            // msg which should have been consolidated in 1 message to avoid that the peer has to process 2 messages
+            // arriving around the same time. This breaks the concept of a strict sequential task execution.
             taskRunner.addTasks(
                     ApplyFilter.class,
                     TakerVerifyMakerAccount.class,

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

@@ -240,9 +240,13 @@ private void handle(PeerPublishedDelayedPayoutTxMessage tradeMessage, NodeAddres
     // Peer has sent a SignedWitness
     ///////////////////////////////////////////////////////////////////////////////////////////
 
-    // TODO this should not be in the trade protocol. The sending is done in the domain, so should be the handling of
-    //  the msg. If it is part of the normal trade protocol we should add the sending and handling to the steps where
-    //  it happen.
+    // It would be unsafe to use the TradeTaskRunner framework here as we expect the PeerPublishedDelayedPayoutTxMessage
+    // around the same time. The ProcessPeerPublishedDelayedPayoutTxMessage task is synchronous so there would not be
+    // an issue if we start another task runner and set the trade message to the process model, but if the code in
+    // ProcessPeerPublishedDelayedPayoutTxMessage would become async in future it would introduce inconsistent model
+    // data. The TradeTaskRunner framework does not support parallel/concurrent execution. The 2 messages should have been
+    // packed into one and the problem would have been avoided. Now it seems to be the best approach to leave the
+    // protocol framework and process the message in the AccountAgeWitness domain.
     private void handle(TraderSignedWitnessMessage tradeMessage) {
         // Publish signed witness, if it is valid and ours
         processModel.getAccountAgeWitnessService().publishOwnSignedWitness(tradeMessage.getSignedWitness());

core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerMaybeSignWitnessAndSendWitnessMessage.java

@@ -90,7 +90,13 @@ protected void run() {
 
             signedWitness = signedWitnessOptional.get();
 
-            // Message sending is handled in base class.
+            // The signer broadcasts as well the signedWitness. In case the receiver will not get the message we have
+            // better resilience that the data is in the network. We set reBroadcast to true but do not expect that the
+            // signedWitness exists already as we just created it.
+            // TODO check with @sqrrm
+            processModel.getP2PService().addPersistableNetworkPayload(signedWitness, true);
+
+            // Message sending is handled in base class (SendMailboxMessageTask).
             super.run();
         } catch (Throwable t) {
             failed(t);

p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java

@@ -479,16 +479,16 @@ public void onError(Throwable throwable) {
      * be broadcast to the P2P network.
      * @param payload PersistableNetworkPayload to add to the network
      * @param sender local NodeAddress, if available
-     * @param allowReBroadcast <code>true</code> if the PersistableNetworkPayload should be rebroadcast even if it
+     * @param reBroadcast <code>true</code> if the PersistableNetworkPayload should be rebroadcast even if it
      *                         already exists locally
      * @return <code>true</code> if the PersistableNetworkPayload passes all validation and exists in the P2PDataStore
      *         on completion
      */
     public boolean addPersistableNetworkPayload(PersistableNetworkPayload payload,
                                                 @Nullable NodeAddress sender,
-                                                boolean allowReBroadcast) {
+                                                boolean reBroadcast) {
         return addPersistableNetworkPayload(
-                payload, sender, true, allowReBroadcast, false);
+                payload, sender, true, reBroadcast, false);
     }
 
     private boolean addPersistableNetworkPayload(PersistableNetworkPayload payload,