fix potential double spend by locking wallet when thawing outputs

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

@@ -30,6 +30,7 @@ import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -287,6 +288,17 @@ public class XmrWalletService {
         }
     }
 
+    /**
+     * Thaw the given outputs with a lock on the wallet.
+     * 
+     * @param keyImages the key images to thaw
+     */
+    public void thawOutputs(Collection<String> keyImages) {
+        synchronized (getWallet()) {
+            for (String keyImage : keyImages) wallet.thawOutput(keyImage);
+        }
+    }
+
     /**
      * Create the reserve tx and freeze its inputs. The full amount is returned
      * to the sender's payout address less the trade fee.
@@ -314,8 +326,19 @@ public class XmrWalletService {
         BigInteger tradeFee = HavenoUtils.coinToAtomicUnits(trade instanceof MakerTrade ? trade.getOffer().getMakerFee() : trade.getTakerFee());
         BigInteger peerAmount = HavenoUtils.coinToAtomicUnits(trade instanceof BuyerTrade ? Coin.ZERO : offer.getAmount());
         BigInteger securityDeposit = HavenoUtils.coinToAtomicUnits(trade instanceof BuyerTrade ? offer.getBuyerSecurityDeposit() : offer.getSellerSecurityDeposit());
-        log.info("Creating deposit tx with fee={}, peerAmount={}, securityDeposit={}", tradeFee, peerAmount, securityDeposit);
+
-        return createTradeTx(tradeFee, peerAmount, securityDeposit, multisigAddress);
+        // thaw reserved outputs then create deposit tx
+        MoneroWallet wallet = getWallet();
+        synchronized (wallet) {
+
+            // thaw reserved outputs
+            if (trade.getSelf().getReserveTxKeyImages() != null) {
+                thawOutputs(trade.getSelf().getReserveTxKeyImages());
+            }
+
+            log.info("Creating deposit tx with fee={}, peerAmount={}, securityDeposit={}", tradeFee, peerAmount, securityDeposit);
+            return createTradeTx(tradeFee, peerAmount, securityDeposit, multisigAddress);
+        }
     }
 
     private MoneroTxWallet createTradeTx(BigInteger tradeFee, BigInteger peerAmount, BigInteger securityDeposit, String address) {

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

@@ -587,7 +587,7 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe
     private void onRemoved(@NotNull OpenOffer openOffer) {
         Offer offer = openOffer.getOffer();
         if (offer.getOfferPayload().getReserveTxKeyImages() != null) {
-            for (String frozenKeyImage : offer.getOfferPayload().getReserveTxKeyImages()) xmrWalletService.getWallet().thawOutput(frozenKeyImage);
+            xmrWalletService.thawOutputs(offer.getOfferPayload().getReserveTxKeyImages());
             xmrWalletService.saveMainWallet();
         }
         offer.setState(Offer.State.REMOVED);

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

@@ -306,18 +306,18 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
         }
 
         // thaw unreserved outputs
-        Set<String> frozenKeyImages = xmrWalletService.getWallet().getOutputs(new MoneroOutputQuery()
+        Set<String> unreservedFrozenKeyImages = xmrWalletService.getWallet().getOutputs(new MoneroOutputQuery()
                 .setIsFrozen(true)
                 .setIsSpent(false))
                 .stream()
                 .map(output -> output.getKeyImage().getHex())
                 .collect(Collectors.toSet());
-        frozenKeyImages.removeAll(reservedKeyImages);
+        unreservedFrozenKeyImages.removeAll(reservedKeyImages);
-        for (String unreservedFrozenKeyImage : frozenKeyImages) {
+        if (!unreservedFrozenKeyImages.isEmpty()) {
-            log.info("Thawing output which is not reserved for offer or trade: " + unreservedFrozenKeyImage);
+            log.info("Thawing outputs which are not reserved for offer or trade: " + unreservedFrozenKeyImages);
-            xmrWalletService.getWallet().thawOutput(unreservedFrozenKeyImage);
+            xmrWalletService.thawOutputs(unreservedFrozenKeyImages);
+            xmrWalletService.saveMainWallet();
         }
-        if (!frozenKeyImages.isEmpty()) xmrWalletService.saveMainWallet();
     }
 
     public TradeProtocol getTradeProtocol(Trade trade) {
@@ -1059,21 +1059,22 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
         synchronized(tradableList) {
             if (!tradableList.contains(trade)) return;
 
-            // skip if trade wallet possibly funded
+            // unreserve key images
+            if (trade instanceof TakerTrade && trade.getSelf().getReserveTxKeyImages() != null) {
+                xmrWalletService.thawOutputs(trade.getSelf().getReserveTxKeyImages());
+                xmrWalletService.saveMainWallet();
+                trade.getSelf().setReserveTxKeyImages(null);
+            }
+
+            // stop if trade wallet possibly funded
             if (xmrWalletService.multisigWalletExists(trade.getId()) && trade.isDepositRequested()) {
-                log.warn("Not removing trade {} because trade wallet could be funded", trade.getId());
+                log.warn("Refusing to delete {} {} because trade wallet could be funded", trade.getClass().getSimpleName(), trade.getId());
                 return;
             }
 
             // delete trade wallet if exists
             if (xmrWalletService.multisigWalletExists(trade.getId())) trade.deleteWallet();
 
-            // unreserve key images
-            if (trade instanceof TakerTrade && trade.getSelf().getReserveTxKeyImages() != null) {
-                for (String keyImage : trade.getSelf().getReserveTxKeyImages()) xmrWalletService.getWallet().thawOutput(keyImage);
-                xmrWalletService.saveMainWallet();
-            }
-
             // remove trade
             removeTrade(trade);
         }

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

@@ -66,17 +66,9 @@ public class MaybeSendSignContractRequest extends TradeTask {
               return;
           }
 
-          // thaw reserved outputs
-          MoneroWallet wallet = trade.getXmrWalletService().getWallet();
-          for (String reserveTxKeyImage : trade.getSelf().getReserveTxKeyImages()) {
-              wallet.thawOutput(reserveTxKeyImage);
-          }
-
           // create deposit tx and freeze inputs
           MoneroTxWallet depositTx = trade.getXmrWalletService().createDepositTx(trade);
 
-          // TODO (woodser): save frozen key images and unfreeze if trade fails before deposited to multisig
-
           // save process state
           processModel.setDepositTxXmr(depositTx); // TODO: trade.getSelf().setDepositTx()
           trade.getSelf().setDepositTxHash(depositTx.getHash());