only remove trade if not in funded state

track sent vs seen deposit request state
cleanup trade phases

core/src/main/java/bisq/core/notifications/alerts/TradeEvents.java

@@ -69,8 +69,8 @@ public class TradeEvents {
                 String shortId = trade.getShortId();
                 switch (newValue) {
                     case INIT:
-                    case TAKER_FEE_PUBLISHED:
+                    case DEPOSIT_REQUESTED:
-                    case DEPOSIT_PUBLISHED:
+                    case DEPOSITS_PUBLISHED:
                         break;
                     case DEPOSIT_UNLOCKED:
                         if (trade.getContract() != null && pubKeyRingProvider.get().equals(trade.getContract().getBuyerPubKeyRing()))

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

@@ -115,14 +115,14 @@ public abstract class Trade implements Tradable, Model {
         CONTRACT_SIGNED(Phase.INIT),
 
         // deposit requested
-        SENT_PUBLISH_DEPOSIT_TX_REQUEST(Phase.TAKER_FEE_PUBLISHED),
+        SENT_PUBLISH_DEPOSIT_TX_REQUEST(Phase.DEPOSIT_REQUESTED),
-        SAW_ARRIVED_PUBLISH_DEPOSIT_TX_REQUEST(Phase.TAKER_FEE_PUBLISHED),
+        SAW_ARRIVED_PUBLISH_DEPOSIT_TX_REQUEST(Phase.DEPOSIT_REQUESTED),
-        STORED_IN_MAILBOX_PUBLISH_DEPOSIT_TX_REQUEST(Phase.TAKER_FEE_PUBLISHED), //not a mailbox msg, not used...
+        STORED_IN_MAILBOX_PUBLISH_DEPOSIT_TX_REQUEST(Phase.DEPOSIT_REQUESTED), //not a mailbox msg, not used...
-        SEND_FAILED_PUBLISH_DEPOSIT_TX_REQUEST(Phase.TAKER_FEE_PUBLISHED),
+        SEND_FAILED_PUBLISH_DEPOSIT_TX_REQUEST(Phase.DEPOSIT_REQUESTED),
 
         // deposit published
-        SAW_DEPOSIT_TXS_IN_NETWORK(Phase.DEPOSIT_PUBLISHED), // TODO: seeing in network usually happens after arbitrator publishes
+        SAW_DEPOSIT_TXS_IN_NETWORK(Phase.DEPOSITS_PUBLISHED), // TODO: seeing in network usually happens after arbitrator publishes
-        ARBITRATOR_PUBLISHED_DEPOSIT_TXS(Phase.DEPOSIT_PUBLISHED),
+        ARBITRATOR_PUBLISHED_DEPOSIT_TXS(Phase.DEPOSITS_PUBLISHED),
 
         // deposit confirmed (TODO)
 
@@ -189,8 +189,8 @@ public abstract class Trade implements Tradable, Model {
 
     public enum Phase {
         INIT,
-        TAKER_FEE_PUBLISHED, // TODO (woodser): remove unused phases
+        DEPOSIT_REQUESTED, // TODO (woodser): remove unused phases
-        DEPOSIT_PUBLISHED,
+        DEPOSITS_PUBLISHED,
         DEPOSIT_UNLOCKED, // TODO (woodser): rename to or add DEPOSIT_UNLOCKED
         PAYMENT_SENT,
         PAYMENT_RECEIVED,
@@ -1252,11 +1252,11 @@ public abstract class Trade implements Tradable, Model {
     }
 
     public boolean isTakerFeePublished() {
-        return getState().getPhase().ordinal() >= Phase.TAKER_FEE_PUBLISHED.ordinal();
+        return getState().getPhase().ordinal() >= Phase.DEPOSIT_REQUESTED.ordinal();
     }
 
     public boolean isDepositPublished() {
-        return getState().getPhase().ordinal() >= Phase.DEPOSIT_PUBLISHED.ordinal();
+        return getState().getPhase().ordinal() >= Phase.DEPOSITS_PUBLISHED.ordinal();
     }
 
     public boolean isFundsLockedIn() {

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

@@ -478,7 +478,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
 
           ((ArbitratorProtocol) getTradeProtocol(trade)).handleInitTradeRequest(request, sender, errorMessage -> {
               log.warn("Arbitrator error during trade initialization for trade {}: {}", trade.getId(), errorMessage);
-              removeTrade(trade);
+              maybeRemoveTrade(trade);
           });
 
           requestPersistence();
@@ -554,7 +554,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
           // notify on phase changes
           // TODO (woodser): save subscription, bind on startup
           EasyBind.subscribe(trade.statePhaseProperty(), phase -> {
-              if (phase == Phase.DEPOSIT_PUBLISHED) {
+              if (phase == Phase.DEPOSITS_PUBLISHED) {
                   notificationService.sendTradeNotification(trade, "Offer Taken", "Your offer " + offer.getId() + " has been accepted"); // TODO (woodser): use language translation
               }
           });
@@ -562,7 +562,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
           ((MakerProtocol) getTradeProtocol(trade)).handleInitTradeRequest(request, sender, errorMessage -> {
               log.warn("Maker error during trade initialization: " + errorMessage);
               openOfferManager.unreserveOpenOffer(openOffer); // offer remains available // TODO: only unreserve if funds not deposited to multisig
-              removeTrade(trade);
+              maybeRemoveTrade(trade);
               if (takeOfferRequestErrorMessageHandler != null) takeOfferRequestErrorMessageHandler.handleErrorMessage(errorMessage);
           });
 
@@ -785,7 +785,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
                         }, errorMessage -> {
                             log.warn("Taker error during trade initialization: " + errorMessage);
                             errorMessageHandler.handleErrorMessage(errorMessage);
-                            removeTrade(trade);
+                            maybeRemoveTrade(trade);
                         });
                         requestPersistence();
                     }
@@ -839,7 +839,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
     public void onTradeCompleted(Trade trade) {
         closedTradableManager.add(trade);
         trade.setState(Trade.State.WITHDRAW_COMPLETED);
-        removeTrade(trade);
+        maybeRemoveTrade(trade);
 
         // TODO The address entry should have been removed already. Check and if its the case remove that.
         xmrWalletService.resetAddressEntriesForPendingTrade(trade.getId());
@@ -909,7 +909,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
     // If trade is in already in critical state (if taker role: taker fee; both roles: after deposit published)
     // we move the trade to failedTradesManager
     public void onMoveInvalidTradeToFailedTrades(Trade trade) {
-        removeTrade(trade);
+        maybeRemoveTrade(trade);
         failedTradesManager.add(trade);
     }
 
@@ -1060,27 +1060,34 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
         return closedTradableManager.getClosedTrades().stream().filter(e -> e.getId().equals(tradeId)).findFirst();
     }
 
-    private synchronized void removeTrade(Trade trade) {
+    private synchronized void maybeRemoveTrade(Trade trade) {
-        log.info("TradeManager.removeTrade()");
+        log.info("TradeManager.maybeRemoveTrade()");
         synchronized(tradableList) {
             if (!tradableList.contains(trade)) return;
-
+            
-            // remove trade
+            // delete trade if not possibly funded
-            tradableList.remove(trade);
+            if (trade.getPhase().ordinal() < Trade.Phase.DEPOSIT_REQUESTED.ordinal() || trade.getPhase().ordinal() >= Trade.Phase.PAYOUT_PUBLISHED.ordinal()) { // TODO: delete after payout unlocked
-
+                
-            // unreserve trade key images
+                // remove trade
-            if (trade instanceof TakerTrade && trade.getSelf().getReserveTxKeyImages() != null) {
+                tradableList.remove(trade);
-                for (String keyImage : trade.getSelf().getReserveTxKeyImages()) {
+                
-                    xmrWalletService.getWallet().thawOutput(keyImage);
+                // unreserve trade key images
+                if (trade instanceof TakerTrade && trade.getSelf().getReserveTxKeyImages() != null) {
+                    for (String keyImage : trade.getSelf().getReserveTxKeyImages()) {
+                        xmrWalletService.getWallet().thawOutput(keyImage);
+                    }
                 }
+                
+                // delete multisig wallet
+                deleteTradeWallet(trade);
+                
+                // unregister and persist
+                p2PService.removeDecryptedDirectMessageListener(getTradeProtocol(trade));
+                requestPersistence();
+            } else {
+                log.warn("Not deleting trade " + trade.getId() + " because its trade wallet might be funded");
+                // TODO: schedule wallet for deletion after unlock
             }
-
-            // delete trade wallet when empty
-            deleteTradeWalletWhenEmpty(trade);
-
-            // unregister and persist
-            p2PService.removeDecryptedDirectMessageListener(getTradeProtocol(trade));
-            requestPersistence();
         }
     }
 
@@ -1098,18 +1105,6 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
         this.numPendingTrades.set(getObservableList().size());
     }
 
-    private void deleteTradeWalletWhenEmpty(Trade trade) {
-
-        // delete trade wallet before funds deposited or after payout unlocked
-        // TODO: delete wallet if trade state < deposit_requested || state >= payout_unlocked (add trade states)
-        if (trade.getPhase().ordinal() < Trade.Phase.DEPOSIT_PUBLISHED.ordinal() || trade.getPhase().ordinal() >= Trade.Phase.PAYOUT_PUBLISHED.ordinal()) {
-            deleteTradeWallet(trade);
-        } else {
-            // TODO: schedule wallet for deletion after unlock
-            log.warn("Not deleting trade " + trade.getId() + " wallet because it might not be empty");
-        }
-    }
-
     private void deleteTradeWallet(Trade trade) {
         if (xmrWalletService.multisigWalletExists(trade.getId())) xmrWalletService.deleteMultisigWallet(trade.getId());
         else log.warn("Multisig wallet to delete for trade {} does not exist", trade.getId());

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

@@ -53,7 +53,7 @@ public abstract class BuyerProtocol extends DisputeProtocol {
     protected void onInitialized() {
         super.onInitialized();
         
-        given(phase(Trade.Phase.DEPOSIT_PUBLISHED)
+        given(phase(Trade.Phase.DEPOSITS_PUBLISHED)
                 .with(BuyerEvent.STARTUP))
                 .setup(tasks(SetupDepositTxsListener.class))
                 .executeTasks();

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

@@ -138,7 +138,7 @@ public abstract class DisputeProtocol extends TradeProtocol {
 
 //    public void onPublishDelayedPayoutTx(ResultHandler resultHandler, ErrorMessageHandler errorMessageHandler) {
 //        DisputeEvent event = DisputeEvent.ARBITRATION_REQUESTED;
-//        expect(anyPhase(Trade.Phase.DEPOSIT_CONFIRMED,
+//        expect(anyPhase(Trade.Phase.DEPOSITS_CONFIRMED,
 //                Trade.Phase.FIAT_SENT,
 //                Trade.Phase.FIAT_RECEIVED)
 //                .with(event)

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

@@ -48,7 +48,7 @@ public abstract class SellerProtocol extends DisputeProtocol {
     protected void onInitialized() {
         super.onInitialized();
         
-        given(phase(Trade.Phase.DEPOSIT_PUBLISHED)
+        given(phase(Trade.Phase.DEPOSITS_PUBLISHED)
                 .with(BuyerEvent.STARTUP))
                 .setup(tasks(SetupDepositTxsListener.class))
                 .executeTasks();
@@ -75,8 +75,8 @@ public abstract class SellerProtocol extends DisputeProtocol {
 
     protected void handle(PaymentSentMessage message, NodeAddress peer) {
         log.info("SellerProtocol.handle(PaymentSentMessage)");
-        // We are more tolerant with expected phase and allow also DEPOSIT_PUBLISHED as it can be the case
+        // We are more tolerant with expected phase and allow also DEPOSITS_PUBLISHED as it can be the case
-        // that the wallet is still syncing and so the DEPOSIT_CONFIRMED state to yet triggered when we received
+        // that the wallet is still syncing and so the DEPOSITS_CONFIRMED state to yet triggered when we received
         // a mailbox message with PaymentSentMessage.
         // TODO A better fix would be to add a listener for the wallet sync state and process
         // the mailbox msg once wallet is ready and trade state set.
@@ -86,7 +86,7 @@ public abstract class SellerProtocol extends DisputeProtocol {
                 return;
             }
             latchTrade();
-            expect(anyPhase(Trade.Phase.DEPOSIT_UNLOCKED, Trade.Phase.DEPOSIT_PUBLISHED)
+            expect(anyPhase(Trade.Phase.DEPOSIT_UNLOCKED, Trade.Phase.DEPOSITS_PUBLISHED)
                     .with(message)
                     .from(peer)
                     .preCondition(trade.getPayoutTx() == null,

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

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

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

@@ -84,11 +84,12 @@ public class ProcessSignContractResponse extends TradeTask {
                         processModel.getDepositTxXmr().getKey());
 
                 // send request to arbitrator
+                log.info("Sending {} to arbitrator {}; offerId={}; uid={}", request.getClass().getSimpleName(), trade.getArbitratorNodeAddress(), trade.getId(), request.getUid());
                 processModel.getP2PService().sendEncryptedDirectMessage(trade.getArbitratorNodeAddress(), trade.getArbitratorPubKeyRing(), request, new SendDirectMessageListener() {
                     @Override
                     public void onArrived() {
                         log.info("{} arrived: arbitrator={}; offerId={}; uid={}", request.getClass().getSimpleName(), trade.getArbitratorNodeAddress(), trade.getId(), request.getUid());
-                        trade.setState(Trade.State.SENT_PUBLISH_DEPOSIT_TX_REQUEST); // TODO: rename to DEPOSIT_REQUESTED
+                        trade.setState(Trade.State.SAW_ARRIVED_PUBLISH_DEPOSIT_TX_REQUEST);
                         processModel.getTradeManager().requestPersistence();
                         complete();
                     }
@@ -99,6 +100,10 @@ public class ProcessSignContractResponse extends TradeTask {
                         failed();
                     }
                 });
+
+                // deposit is requested
+                trade.setState(Trade.State.SENT_PUBLISH_DEPOSIT_TX_REQUEST);
+                processModel.getTradeManager().requestPersistence();
             } else {
                 log.info("Waiting for more contract signatures to send deposit request");
                 complete(); // does not yet have needed signatures

desktop/src/main/java/bisq/desktop/main/offer/takeoffer/TakeOfferViewModel.java

@@ -416,10 +416,10 @@ class TakeOfferViewModel extends ActivatableWithDataModel<TakeOfferDataModel> im
                 case INIT:
                     appendMsg = Res.get("takeOffer.error.noFundsLost");
                     break;
-                case TAKER_FEE_PUBLISHED:
+                case DEPOSIT_REQUESTED:
                     appendMsg = Res.get("takeOffer.error.feePaid");
                     break;
-                case DEPOSIT_PUBLISHED:
+                case DEPOSITS_PUBLISHED:
                 case PAYMENT_SENT:
                 case PAYMENT_RECEIVED:
                     appendMsg = Res.get("takeOffer.error.depositPublished");

desktop/src/main/java/bisq/desktop/main/overlays/notifications/NotificationCenter.java

@@ -187,7 +187,7 @@ public class NotificationCenter {
             message = Res.get("notification.trade.completed");
         } else {
             if (trade instanceof MakerTrade &&
-                    phase.ordinal() == Trade.Phase.DEPOSIT_PUBLISHED.ordinal()) {
+                    phase.ordinal() == Trade.Phase.DEPOSITS_PUBLISHED.ordinal()) {
                 final String role = trade instanceof BuyerTrade ? Res.get("shared.seller") : Res.get("shared.buyer");
                 message = Res.get("notification.trade.accepted", role);
             }

desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesView.java

@@ -381,7 +381,7 @@ public class PendingTradesView extends ActivatableViewAndModel<VBox, PendingTrad
 
     private boolean isMaybeInvalidTrade(Trade trade) {
         return trade.hasErrorMessage() ||
-                (Trade.Phase.DEPOSIT_PUBLISHED.ordinal() <= trade.getPhase().ordinal() && trade.isTxChainInvalid());
+                (Trade.Phase.DEPOSITS_PUBLISHED.ordinal() <= trade.getPhase().ordinal() && trade.isTxChainInvalid());
     }
 
     private void onMoveInvalidTradeToFailedTrades(Trade trade) {

desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesViewModel.java

@@ -402,6 +402,10 @@ public class PendingTradesViewModel extends ActivatableWithDataModel<PendingTrad
         switch (tradeState) {
             // preparation
             case PREPARATION:
+            case MULTISIG_PREPARED:
+            case MULTISIG_MADE:
+            case MULTISIG_EXCHANGED:
+            case MULTISIG_COMPLETED:
             case CONTRACT_SIGNATURE_REQUESTED:
             case CONTRACT_SIGNED:
                 sellerState.set(UNDEFINED);

proto/src/main/proto/pb.proto

@@ -1663,9 +1663,9 @@ message Trade {
     enum Phase {
         PB_ERROR_PHASE = 0;
         INIT = 1;
-        TAKER_FEE_PUBLISHED = 2;
+        DEPOSIT_REQUESTED = 2;
-        DEPOSIT_PUBLISHED = 3;
+        DEPOSITS_PUBLISHED = 3;
-        DEPOSIT_CONFIRMED = 4;
+        DEPOSITS_CONFIRMED = 4;
         PAYMENT_SENT = 5;
         PAYMENT_RECEIVED = 6;
         PAYOUT_PUBLISHED = 7;