diff --git a/core/src/main/java/haveno/core/trade/Trade.java b/core/src/main/java/haveno/core/trade/Trade.java index 352b7d06cc..568e58744b 100644 --- a/core/src/main/java/haveno/core/trade/Trade.java +++ b/core/src/main/java/haveno/core/trade/Trade.java @@ -50,6 +50,7 @@ import haveno.core.monetary.Volume; import haveno.core.network.MessageState; import haveno.core.offer.Offer; import haveno.core.offer.OfferDirection; +import haveno.core.offer.OpenOffer; import haveno.core.payment.payload.PaymentAccountPayload; import haveno.core.proto.CoreProtoResolver; import haveno.core.proto.network.CoreNetworkProtoResolver; @@ -73,12 +74,14 @@ import haveno.network.p2p.NodeAddress; import haveno.network.p2p.P2PService; import javafx.beans.property.DoubleProperty; import javafx.beans.property.IntegerProperty; +import javafx.beans.property.LongProperty; import javafx.beans.property.ObjectProperty; import javafx.beans.property.ReadOnlyDoubleProperty; import javafx.beans.property.ReadOnlyObjectProperty; import javafx.beans.property.ReadOnlyStringProperty; import javafx.beans.property.SimpleDoubleProperty; import javafx.beans.property.SimpleIntegerProperty; +import javafx.beans.property.SimpleLongProperty; import javafx.beans.property.SimpleObjectProperty; import javafx.beans.property.SimpleStringProperty; import javafx.beans.property.StringProperty; @@ -133,14 +136,18 @@ public abstract class Trade implements Tradable, Model { private static final String MONERO_TRADE_WALLET_PREFIX = "xmr_trade_"; private static final long SHUTDOWN_TIMEOUT_MS = 60000; - private static final long DELETE_BACKUPS_AFTER_NUM_BLOCKS = 3600; // ~5 days private static final long SYNC_EVERY_NUM_BLOCKS = 360; // ~1/2 day + private static final long DELETE_AFTER_NUM_BLOCKS = 1; // if deposit requested but not published + private static final long DELETE_AFTER_MS = TradeProtocol.TRADE_STEP_TIMEOUT_SECONDS; private final Object walletLock = new Object(); private final Object pollLock = new Object(); + private final LongProperty walletHeight = new SimpleLongProperty(0); private MoneroWallet wallet; boolean wasWalletSynced; boolean pollInProgress; boolean restartInProgress; + private Subscription protocolErrorStateSubscription; + private Subscription protocolErrorHeightSubscription; /////////////////////////////////////////////////////////////////////////////////////////// // Enums @@ -643,6 +650,7 @@ public abstract class Trade implements Tradable, Model { if (!isInitialized || isShutDownStarted) return; ThreadUtils.submitToPool(() -> { if (newValue == Trade.Phase.DEPOSIT_REQUESTED) startPolling(); + if (newValue == Trade.Phase.DEPOSITS_PUBLISHED) xmrWalletService.freezeOutputs(getSelf().getReserveTxKeyImages()); if (isDepositsPublished() && !isPayoutUnlocked()) updatePollPeriod(); if (isPaymentReceived()) { UserThread.execute(() -> { @@ -785,6 +793,10 @@ public abstract class Trade implements Tradable, Model { } } + public long getHeight() { + return walletHeight.get(); + } + private String getWalletName() { return MONERO_TRADE_WALLET_PREFIX + getShortId() + "_" + getShortUid(); } @@ -814,7 +826,7 @@ public abstract class Trade implements Tradable, Model { if (!xmrConnectionService.isSyncedWithinTolerance()) return false; Long targetHeight = xmrConnectionService.getTargetHeight(); if (targetHeight == null) return false; - if (targetHeight - wallet.getHeight() <= 3) return true; // synced if within 3 blocks of target height + if (targetHeight - walletHeight.get() <= 3) return true; // synced if within 3 blocks of target height return false; } } @@ -941,7 +953,7 @@ public abstract class Trade implements Tradable, Model { } // wallet must be synced - if (isDepositRequested() && !isSyncedWithinTolerance()) { + if (isDepositRequested() && isWalletBehind()) { log.warn("Wallet is not synced for {} {}, syncing", getClass().getSimpleName(), getId()); syncWallet(true); } @@ -966,19 +978,9 @@ public abstract class Trade implements Tradable, Model { forceCloseWallet(); // delete wallet - log.info("Deleting wallet for {} {}", getClass().getSimpleName(), getId()); + log.info("Deleting wallet and backups for {} {}", getClass().getSimpleName(), getId()); xmrWalletService.deleteWallet(getWalletName()); - - // delete trade wallet backups if empty and payout unlocked, else schedule - if (isPayoutUnlocked() || !isDepositRequested() || isDepositFailed()) { - xmrWalletService.deleteWalletBackups(getWalletName()); - } else { - - // schedule backup deletion by recording delete height - log.warn("Scheduling to delete backup wallet for " + getClass().getSimpleName() + " " + getId() + " in the small chance it becomes funded"); - processModel.setDeleteBackupsHeight(xmrConnectionService.getLastInfo().getHeight() + DELETE_BACKUPS_AFTER_NUM_BLOCKS); - maybeScheduleDeleteBackups(); - } + xmrWalletService.deleteWalletBackups(getWalletName()); } catch (Exception e) { log.warn(e.getMessage()); e.printStackTrace(); @@ -1290,9 +1292,6 @@ public abstract class Trade implements Tradable, Model { private void clearProcessData() { - // delete backup wallets after main wallet + blocks - maybeScheduleDeleteBackups(); - // delete trade wallet synchronized (walletLock) { if (!walletExists()) return; // done if already cleared @@ -1310,27 +1309,6 @@ public abstract class Trade implements Tradable, Model { } } - private void maybeScheduleDeleteBackups() { - if (processModel.getDeleteBackupsHeight() == 0) return; - if (xmrConnectionService.getLastInfo().getHeight() >= processModel.getDeleteBackupsHeight()) { - xmrWalletService.deleteWalletBackups(getWalletName()); - processModel.setDeleteBackupsHeight(0); // reset delete height - } else { - MoneroWalletListener deleteBackupsListener = new MoneroWalletListener() { - @Override - public synchronized void onNewBlock(long height) { // prevent concurrent deletion - if (processModel.getDeleteBackupsHeight() == 0) return; - if (xmrConnectionService.getLastInfo().getHeight() >= processModel.getDeleteBackupsHeight()) { - xmrWalletService.deleteWalletBackups(getWalletName()); - processModel.setDeleteBackupsHeight(0); // reset delete height - xmrWalletService.removeWalletListener(this); - } - } - }; - xmrWalletService.addWalletListener(deleteBackupsListener); - } - } - public void maybeClearSensitiveData() { String change = ""; if (removeAllChatMessages()) { @@ -1409,6 +1387,127 @@ public abstract class Trade implements Tradable, Model { }); } + /////////////////////////////////////////////////////////////////////////////////////////// + // Trade error cleanup + /////////////////////////////////////////////////////////////////////////////////////////// + + public void onProtocolError() { + + // check if deposit published + if (isDepositsPublished()) { + restorePublishedTrade(); + return; + } + + // unreserve taker key images + if (this instanceof TakerTrade) { + ThreadUtils.submitToPool(() -> { + xmrWalletService.thawOutputs(getSelf().getReserveTxKeyImages()); + }); + } + + // unreserve open offer + Optional openOffer = processModel.getOpenOfferManager().getOpenOfferById(this.getId()); + if (this instanceof MakerTrade && openOffer.isPresent()) { + processModel.getOpenOfferManager().unreserveOpenOffer(openOffer.get()); + } + + // remove if deposit not requested or is failed + if (!isDepositRequested() || isDepositFailed()) { + removeTradeOnError(); + return; + } + + // done if wallet already deleted + if (!walletExists()) return; + + // move to failed trades + processModel.getTradeManager().onMoveInvalidTradeToFailedTrades(this); + + // set error height + if (processModel.getTradeProtocolErrorHeight() == 0) { + log.warn("Scheduling to remove trade if unfunded for {} {} from height {}", getClass().getSimpleName(), getId(), xmrConnectionService.getLastInfo().getHeight()); + processModel.setTradeProtocolErrorHeight(xmrConnectionService.getLastInfo().getHeight()); + } + + // listen for deposits published to restore trade + protocolErrorStateSubscription = EasyBind.subscribe(stateProperty(), state -> { + if (isDepositsPublished()) { + restorePublishedTrade(); + if (protocolErrorStateSubscription != null) { // unsubscribe + protocolErrorStateSubscription.unsubscribe(); + protocolErrorStateSubscription = null; + } + } + }); + + // listen for block confirmations to remove trade + long startTime = System.currentTimeMillis(); + protocolErrorHeightSubscription = EasyBind.subscribe(walletHeight, lastWalletHeight -> { + if (isShutDown || isDepositsPublished()) return; + if (lastWalletHeight.longValue() < processModel.getTradeProtocolErrorHeight() + DELETE_AFTER_NUM_BLOCKS) return; + if (System.currentTimeMillis() - startTime < DELETE_AFTER_MS) return; + + // remove trade off thread + ThreadUtils.submitToPool(() -> { + + // get trade's deposit txs from daemon + MoneroTx makerDepositTx = getMaker().getDepositTxHash() == null ? null : xmrWalletService.getDaemon().getTx(getMaker().getDepositTxHash()); + MoneroTx takerDepositTx = getTaker().getDepositTxHash() == null ? null : xmrWalletService.getDaemon().getTx(getTaker().getDepositTxHash()); + + // remove trade and wallet if neither deposit tx published + if (makerDepositTx == null && takerDepositTx == null) { + log.warn("Deleting {} {} after protocol error", getClass().getSimpleName(), getId()); + if (this instanceof ArbitratorTrade && (getMaker().getReserveTxHash() != null || getTaker().getReserveTxHash() != null)) { + processModel.getTradeManager().onMoveInvalidTradeToFailedTrades(this); // arbitrator retains trades with reserved funds for analysis and penalty + deleteWallet(); + onShutDownStarted(); + ThreadUtils.submitToPool(() -> shutDown()); // run off thread + } else { + removeTradeOnError(); + } + } else if (!isPayoutPublished()) { + + // set error if wallet may be partially funded + String errorMessage = "Refusing to delete " + getClass().getSimpleName() + " " + getId() + " after protocol error because its wallet might be funded"; + prependErrorMessage(errorMessage); + log.warn(errorMessage); + } + + // unsubscribe + if (protocolErrorHeightSubscription != null) { + protocolErrorHeightSubscription.unsubscribe(); + protocolErrorHeightSubscription = null; + } + }); + }); + } + + private void restorePublishedTrade() { + + // close open offer + if (this instanceof MakerTrade && processModel.getOpenOfferManager().getOpenOfferById(getId()).isPresent()) { + log.info("Closing open offer because {} {} was restored after protocol error", getClass().getSimpleName(), getShortId()); + processModel.getOpenOfferManager().closeOpenOffer(checkNotNull(getOffer())); + } + + // re-freeze outputs + xmrWalletService.freezeOutputs(getSelf().getReserveTxKeyImages()); + + // restore trade from failed trades + processModel.getTradeManager().onMoveFailedTradeToPendingTrades(this); + } + + private void removeTradeOnError() { + log.warn("removeTradeOnError() trade={}, tradeId={}, state={}", getClass().getSimpleName(), getShortId(), getState()); + + // clear and shut down trade + clearAndShutDown(); + + // unregister trade + processModel.getTradeManager().unregisterTrade(this); + } + /////////////////////////////////////////////////////////////////////////////////////////// // Model implementation /////////////////////////////////////////////////////////////////////////////////////////// @@ -1815,6 +1914,7 @@ public abstract class Trade implements Tradable, Model { } public boolean isDepositsPublished() { + if (isDepositFailed()) return false; return getState().getPhase().ordinal() >= Phase.DEPOSITS_PUBLISHED.ordinal() && getMaker().getDepositTxHash() != null && getTaker().getDepositTxHash() != null; } @@ -1935,9 +2035,11 @@ public abstract class Trade implements Tradable, Model { public BigInteger getFrozenAmount() { BigInteger sum = BigInteger.ZERO; - for (String keyImage : getSelf().getReserveTxKeyImages()) { - List outputs = xmrWalletService.getOutputs(new MoneroOutputQuery().setIsFrozen(true).setIsSpent(false).setKeyImage(new MoneroKeyImage(keyImage))); - if (!outputs.isEmpty()) sum = sum.add(outputs.get(0).getAmount()); + if (getSelf().getReserveTxKeyImages() != null) { + for (String keyImage : getSelf().getReserveTxKeyImages()) { + List outputs = xmrWalletService.getOutputs(new MoneroOutputQuery().setIsFrozen(true).setIsSpent(false).setKeyImage(new MoneroKeyImage(keyImage))); + if (!outputs.isEmpty()) sum = sum.add(outputs.get(0).getAmount()); + } } return sum; } @@ -2169,12 +2271,12 @@ public abstract class Trade implements Tradable, Model { // skip if payout unlocked if (isPayoutUnlocked()) return; - // skip if either deposit tx id is unknown - if (processModel.getMaker().getDepositTxHash() == null || processModel.getTaker().getDepositTxHash() == null) return; + // skip if deposit txs unknown or not requested + if (processModel.getMaker().getDepositTxHash() == null || processModel.getTaker().getDepositTxHash() == null || !isDepositRequested()) return; // sync if wallet too far behind daemon if (xmrConnectionService.getTargetHeight() == null) return; - if (wallet.getHeight() < xmrConnectionService.getTargetHeight() - SYNC_EVERY_NUM_BLOCKS) syncWallet(false); + if (walletHeight.get() < xmrConnectionService.getTargetHeight() - SYNC_EVERY_NUM_BLOCKS) syncWallet(false); // update deposit txs if (!isDepositsUnlocked()) { @@ -2286,12 +2388,13 @@ public abstract class Trade implements Tradable, Model { if (isWalletBehind()) { synchronized (walletLock) { xmrWalletService.syncWallet(wallet); + walletHeight.set(wallet.getHeight()); } } } private boolean isWalletBehind() { - return wallet.getHeight() < xmrConnectionService.getTargetHeight(); + return walletHeight.get() < xmrConnectionService.getTargetHeight(); } private void setDepositTxs(List txs) { diff --git a/core/src/main/java/haveno/core/trade/TradeManager.java b/core/src/main/java/haveno/core/trade/TradeManager.java index d139bb700b..564c844e36 100644 --- a/core/src/main/java/haveno/core/trade/TradeManager.java +++ b/core/src/main/java/haveno/core/trade/TradeManager.java @@ -133,7 +133,6 @@ import monero.daemon.model.MoneroTx; import org.bitcoinj.core.Coin; import org.bouncycastle.crypto.params.KeyParameter; import org.fxmisc.easybind.EasyBind; -import org.fxmisc.easybind.Subscription; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -434,7 +433,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi Set tasks = new HashSet(); Set uids = new HashSet(); Set tradesToSkip = new HashSet(); - Set tradesToMaybeRemoveOnError = new HashSet(); + Set uninitializedTrades = new HashSet(); for (Trade trade : trades) { tasks.add(() -> { try { @@ -451,7 +450,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi // remove trade if protocol didn't initialize if (getOpenTradeByUid(trade.getUid()).isPresent() && !trade.isDepositsPublished()) { - tradesToMaybeRemoveOnError.add(trade); + uninitializedTrades.add(trade); } } catch (Exception e) { if (!isShutDownStarted) { @@ -477,9 +476,9 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi // process after all wallets initialized if (!HavenoUtils.isSeedNode()) { - // maybe remove trades on error - for (Trade trade : tradesToMaybeRemoveOnError) { - maybeRemoveTradeOnError(trade); + // handle uninitialized trades + for (Trade trade : uninitializedTrades) { + trade.onProtocolError(); } // freeze or thaw outputs @@ -623,7 +622,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); + trade.onProtocolError(); }); requestPersistence(); @@ -704,7 +703,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); + trade.onProtocolError(); }); } } @@ -797,8 +796,11 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi Optional tradeOptional = getOpenTrade(response.getOfferId()); if (!tradeOptional.isPresent()) { - log.warn("No trade with id " + response.getOfferId()); - return; + tradeOptional = getFailedTrade(response.getOfferId()); + if (!tradeOptional.isPresent()) { + log.warn("No trade with id " + response.getOfferId()); + return; + } } Trade trade = tradeOptional.get(); ((TraderProtocol) getTradeProtocol(trade)).handleDepositResponse(response, peer); @@ -885,8 +887,8 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi requestPersistence(); }, errorMessage -> { log.warn("Taker error during trade initialization: " + errorMessage); - xmrWalletService.resetAddressEntriesForOpenOffer(trade.getId()); - maybeRemoveTradeOnError(trade); + xmrWalletService.resetAddressEntriesForOpenOffer(trade.getId()); // TODO: move to maybe remove on error + trade.onProtocolError(); errorMessageHandler.handleErrorMessage(errorMessage); }); requestPersistence(); @@ -945,13 +947,32 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi if (trade.isCompleted()) throw new RuntimeException("Trade " + trade.getId() + " was already completed"); closedTradableManager.add(trade); trade.setCompleted(true); - removeTrade(trade); + removeTrade(trade, true); // TODO The address entry should have been removed already. Check and if its the case remove that. xmrWalletService.resetAddressEntriesForTrade(trade.getId()); requestPersistence(); } + public void unregisterTrade(Trade trade) { + removeTrade(trade, true); + removeFailedTrade(trade); + requestPersistence(); + } + + public void removeTrade(Trade trade, boolean removeDirectMessageListener) { + log.info("TradeManager.removeTrade() " + trade.getId()); + + // remove trade + synchronized (tradableList) { + if (!tradableList.remove(trade)) return; + } + + // unregister message listener and persist + if (removeDirectMessageListener) p2PService.removeDecryptedDirectMessageListener(getTradeProtocol(trade)); + requestPersistence(); + } + /////////////////////////////////////////////////////////////////////////////////////////// // Dispute @@ -1014,8 +1035,17 @@ 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); failedTradesManager.add(trade); + removeTrade(trade, false); + } + + public void onMoveFailedTradeToPendingTrades(Trade trade) { + addFailedTradeToPendingTrades(trade); + failedTradesManager.removeTrade(trade); + } + + public void removeFailedTrade(Trade trade) { + failedTradesManager.removeTrade(trade); } public void addFailedTradeToPendingTrades(Trade trade) { @@ -1255,132 +1285,6 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi } } - private void removeTrade(Trade trade) { - log.info("TradeManager.removeTrade() " + trade.getId()); - - // remove trade - synchronized (tradableList) { - if (!tradableList.remove(trade)) return; - } - - // unregister and persist - p2PService.removeDecryptedDirectMessageListener(getTradeProtocol(trade)); - requestPersistence(); - } - - private void maybeRemoveTradeOnError(Trade trade) { - if (trade.isDepositRequested() && !trade.isDepositFailed()) { - listenForCleanup(trade); - } else { - removeTradeOnError(trade); - } - } - - private void removeTradeOnError(Trade trade) { - log.warn("TradeManager.removeTradeOnError() trade={}, tradeId={}, state={}", trade.getClass().getSimpleName(), trade.getShortId(), trade.getState()); - - // unreserve taker key images - if (trade instanceof TakerTrade) { - xmrWalletService.thawOutputs(trade.getSelf().getReserveTxKeyImages()); - trade.getSelf().setReserveTxKeyImages(null); - } - - // unreserve open offer - Optional openOffer = openOfferManager.getOpenOfferById(trade.getId()); - if (trade instanceof MakerTrade && openOffer.isPresent()) { - openOfferManager.unreserveOpenOffer(openOffer.get()); - } - - // clear and shut down trade - trade.clearAndShutDown(); - - // remove trade from list - removeTrade(trade); - } - - private void listenForCleanup(Trade trade) { - if (getOpenTrade(trade.getId()).isPresent() && trade.isDepositRequested()) { - if (trade.isDepositsPublished()) { - cleanupPublishedTrade(trade); - } else { - log.warn("Scheduling to delete open trade if unfunded for {} {}", trade.getClass().getSimpleName(), trade.getId()); - new TradeCleanupListener(trade); // TODO: better way than creating listener? - } - } - } - - private void cleanupPublishedTrade(Trade trade) { - if (trade instanceof MakerTrade && openOfferManager.getOpenOfferById(trade.getId()).isPresent()) { - log.warn("Closing open offer as cleanup step"); - openOfferManager.closeOpenOffer(checkNotNull(trade.getOffer())); - } - } - - private class TradeCleanupListener { - - private static final long REMOVE_AFTER_MS = 60000; - private static final int REMOVE_AFTER_NUM_CONFIRMATIONS = 1; - private Long startHeight; - private Subscription stateSubscription; - private Subscription heightSubscription; - - public TradeCleanupListener(Trade trade) { - - // listen for deposits published to close open offer - stateSubscription = EasyBind.subscribe(trade.stateProperty(), state -> { - if (trade.isDepositsPublished()) { - cleanupPublishedTrade(trade); - if (stateSubscription != null) { - stateSubscription.unsubscribe(); - stateSubscription = null; - } - } - }); - - // listen for block confirmation to remove trade - long startTime = System.currentTimeMillis(); - heightSubscription = EasyBind.subscribe(xmrWalletService.getConnectionService().chainHeightProperty(), lastBlockHeight -> { - if (isShutDown) return; - if (startHeight == null) startHeight = lastBlockHeight.longValue(); - if (lastBlockHeight.longValue() >= startHeight + REMOVE_AFTER_NUM_CONFIRMATIONS) { - new Thread(() -> { - - // wait minimum time - HavenoUtils.waitFor(Math.max(0, REMOVE_AFTER_MS - (System.currentTimeMillis() - startTime))); - - // get trade's deposit txs from daemon - MoneroTx makerDepositTx = trade.getMaker().getDepositTxHash() == null ? null : xmrWalletService.getDaemon().getTx(trade.getMaker().getDepositTxHash()); - MoneroTx takerDepositTx = trade.getTaker().getDepositTxHash() == null ? null : xmrWalletService.getDaemon().getTx(trade.getTaker().getDepositTxHash()); - - // remove trade and wallet if neither deposit tx published - if (makerDepositTx == null && takerDepositTx == null) { - log.warn("Deleting {} {} after protocol error", trade.getClass().getSimpleName(), trade.getId()); - if (trade instanceof ArbitratorTrade && (trade.getMaker().getReserveTxHash() != null || trade.getTaker().getReserveTxHash() != null)) { - onMoveInvalidTradeToFailedTrades(trade); // arbitrator retains trades with reserved funds for analysis and penalty - } else { - removeTradeOnError(trade); - failedTradesManager.removeTrade(trade); - } - } else if (!trade.isPayoutPublished()) { - - // set error that wallet may be partially funded - String errorMessage = "Refusing to delete " + trade.getClass().getSimpleName() + " " + trade.getId() + " after protocol timeout because its wallet might be funded"; - trade.prependErrorMessage(errorMessage); - log.warn(errorMessage); - } - - // unsubscribe - if (heightSubscription != null) { - heightSubscription.unsubscribe(); - heightSubscription = null; - } - - }).start(); - } - }); - } - } - // TODO Remove once tradableList is refactored to a final field // (part of the persistence refactor PR) private void onTradesChanged() { diff --git a/core/src/main/java/haveno/core/trade/protocol/ProcessModel.java b/core/src/main/java/haveno/core/trade/protocol/ProcessModel.java index 8e3983ecf7..aca3211f9b 100644 --- a/core/src/main/java/haveno/core/trade/protocol/ProcessModel.java +++ b/core/src/main/java/haveno/core/trade/protocol/ProcessModel.java @@ -157,7 +157,7 @@ public class ProcessModel implements Model, PersistablePayload { private String multisigAddress; @Getter @Setter - private long deleteBackupsHeight; + private long tradeProtocolErrorHeight; // We want to indicate the user the state of the message delivery of the // PaymentSentMessage. As well we do an automatic re-send in case it was not ACKed yet. @@ -207,7 +207,7 @@ public class ProcessModel implements Model, PersistablePayload { .setPaymentSentMessageStateArbitrator(paymentSentMessageStatePropertyArbitrator.get().name()) .setBuyerPayoutAmountFromMediation(buyerPayoutAmountFromMediation) .setSellerPayoutAmountFromMediation(sellerPayoutAmountFromMediation) - .setDeleteBackupsHeight(deleteBackupsHeight); + .setTradeProtocolErrorHeight(tradeProtocolErrorHeight); Optional.ofNullable(maker).ifPresent(e -> builder.setMaker((protobuf.TradePeer) maker.toProtoMessage())); Optional.ofNullable(taker).ifPresent(e -> builder.setTaker((protobuf.TradePeer) taker.toProtoMessage())); Optional.ofNullable(arbitrator).ifPresent(e -> builder.setArbitrator((protobuf.TradePeer) arbitrator.toProtoMessage())); @@ -230,7 +230,7 @@ public class ProcessModel implements Model, PersistablePayload { processModel.setFundsNeededForTrade(proto.getFundsNeededForTrade()); processModel.setBuyerPayoutAmountFromMediation(proto.getBuyerPayoutAmountFromMediation()); processModel.setSellerPayoutAmountFromMediation(proto.getSellerPayoutAmountFromMediation()); - processModel.setDeleteBackupsHeight(proto.getDeleteBackupsHeight()); + processModel.setTradeProtocolErrorHeight(proto.getTradeProtocolErrorHeight()); // nullable processModel.setPayoutTxSignature(ProtoUtil.byteArrayOrNullFromProto(proto.getPayoutTxSignature())); diff --git a/core/src/main/java/haveno/core/trade/protocol/TradeProtocol.java b/core/src/main/java/haveno/core/trade/protocol/TradeProtocol.java index a22eb72a53..396285a487 100644 --- a/core/src/main/java/haveno/core/trade/protocol/TradeProtocol.java +++ b/core/src/main/java/haveno/core/trade/protocol/TradeProtocol.java @@ -427,15 +427,7 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D System.out.println(getClass().getSimpleName() + ".handleDepositResponse()"); ThreadUtils.execute(() -> { synchronized (trade) { - - // check trade - if (trade.hasFailed()) { - log.warn("{} {} ignoring {} from {} because trade failed with previous error: {}", trade.getClass().getSimpleName(), trade.getId(), response.getClass().getSimpleName(), sender, trade.getErrorMessage()); - return; - } Validator.checkTradeId(processModel.getOfferId(), response); - - // process message latchTrade(); processModel.setTradeMessage(response); expect(anyState(Trade.State.SENT_PUBLISH_DEPOSIT_TX_REQUEST, Trade.State.SAW_ARRIVED_PUBLISH_DEPOSIT_TX_REQUEST, Trade.State.ARBITRATOR_PUBLISHED_DEPOSIT_TXS, Trade.State.DEPOSIT_TXS_SEEN_IN_NETWORK) diff --git a/core/src/main/java/haveno/core/trade/protocol/tasks/ArbitratorProcessDepositRequest.java b/core/src/main/java/haveno/core/trade/protocol/tasks/ArbitratorProcessDepositRequest.java index dc287677d2..1feb2b6b0f 100644 --- a/core/src/main/java/haveno/core/trade/protocol/tasks/ArbitratorProcessDepositRequest.java +++ b/core/src/main/java/haveno/core/trade/protocol/tasks/ArbitratorProcessDepositRequest.java @@ -28,6 +28,7 @@ import haveno.core.trade.Trade; import haveno.core.trade.messages.DepositRequest; import haveno.core.trade.messages.DepositResponse; import haveno.core.trade.protocol.TradePeer; +import haveno.core.trade.protocol.TradeProtocol; import haveno.network.p2p.NodeAddress; import haveno.network.p2p.SendDirectMessageListener; import lombok.extern.slf4j.Slf4j; @@ -101,6 +102,10 @@ public class ArbitratorProcessDepositRequest extends TradeTask { throw new RuntimeException("Error processing deposit tx from " + (isFromTaker ? "taker " : "maker ") + trader.getNodeAddress() + ", offerId=" + offer.getId() + ": " + e.getMessage()); } + // extend timeout + if (isTimedOut()) throw new RuntimeException("Trade protocol has timed out while verifying deposit tx for {} {}" + trade.getClass().getSimpleName() + " " + trade.getShortId()); + trade.getProtocol().startTimeout(TradeProtocol.TRADE_STEP_TIMEOUT_SECONDS); + // set deposit info trader.setSecurityDeposit(securityDeposit.subtract(verifiedTx.getFee())); // subtract mining fee from security deposit trader.setDepositTxFee(verifiedTx.getFee()); @@ -109,7 +114,6 @@ public class ArbitratorProcessDepositRequest extends TradeTask { if (request.getPaymentAccountKey() != null) trader.setPaymentAccountKey(request.getPaymentAccountKey()); // relay deposit txs when both available - // TODO (woodser): add small delay so tx has head start against double spend attempts? if (processModel.getMaker().getDepositTxHex() != null && processModel.getTaker().getDepositTxHex() != null) { // update trade state @@ -147,8 +151,8 @@ public class ArbitratorProcessDepositRequest extends TradeTask { if (processModel.getTaker().getDepositTxHex() == null) log.info("Arbitrator waiting for deposit request from taker for trade " + trade.getId()); } - complete(); processModel.getTradeManager().requestPersistence(); + complete(); } catch (Throwable t) { // handle error before deposits relayed @@ -192,4 +196,8 @@ public class ArbitratorProcessDepositRequest extends TradeTask { } }); } + + private boolean isTimedOut() { + return !processModel.getTradeManager().hasOpenTrade(trade); + } } diff --git a/core/src/main/java/haveno/core/trade/protocol/tasks/MaybeSendSignContractRequest.java b/core/src/main/java/haveno/core/trade/protocol/tasks/MaybeSendSignContractRequest.java index 80ea6cd368..6870742b90 100644 --- a/core/src/main/java/haveno/core/trade/protocol/tasks/MaybeSendSignContractRequest.java +++ b/core/src/main/java/haveno/core/trade/protocol/tasks/MaybeSendSignContractRequest.java @@ -124,7 +124,6 @@ public class MaybeSendSignContractRequest extends TradeTask { throw e; } - // reset protocol timeout trade.getProtocol().startTimeout(TradeProtocol.TRADE_STEP_TIMEOUT_SECONDS); diff --git a/core/src/main/java/haveno/core/trade/protocol/tasks/ProcessDepositResponse.java b/core/src/main/java/haveno/core/trade/protocol/tasks/ProcessDepositResponse.java index 4c18800a00..1140a5d5db 100644 --- a/core/src/main/java/haveno/core/trade/protocol/tasks/ProcessDepositResponse.java +++ b/core/src/main/java/haveno/core/trade/protocol/tasks/ProcessDepositResponse.java @@ -23,6 +23,7 @@ import java.math.BigInteger; import haveno.common.taskrunner.TaskRunner; import haveno.core.trade.Trade; import haveno.core.trade.messages.DepositResponse; +import haveno.core.trade.protocol.TradeProtocol; import lombok.extern.slf4j.Slf4j; @Slf4j @@ -42,9 +43,13 @@ public class ProcessDepositResponse extends TradeTask { DepositResponse message = (DepositResponse) processModel.getTradeMessage(); if (message.getErrorMessage() != null) { trade.setStateIfValidTransitionTo(Trade.State.PUBLISH_DEPOSIT_TX_REQUEST_FAILED); + processModel.getTradeManager().unregisterTrade(trade); throw new RuntimeException(message.getErrorMessage()); } + // reset protocol timeout + trade.getProtocol().startTimeout(TradeProtocol.TRADE_STEP_TIMEOUT_SECONDS); + // record security deposits trade.getBuyer().setSecurityDeposit(BigInteger.valueOf(message.getBuyerSecurityDeposit())); trade.getSeller().setSecurityDeposit(BigInteger.valueOf(message.getSellerSecurityDeposit())); diff --git a/core/src/main/java/haveno/core/xmr/wallet/XmrWalletService.java b/core/src/main/java/haveno/core/xmr/wallet/XmrWalletService.java index a00f6f85fc..b681b2c4bb 100644 --- a/core/src/main/java/haveno/core/xmr/wallet/XmrWalletService.java +++ b/core/src/main/java/haveno/core/xmr/wallet/XmrWalletService.java @@ -549,11 +549,20 @@ public class XmrWalletService { /** * Freeze the given outputs with a lock on the wallet. * - * @param keyImages the key images to freeze + * @param keyImages the key images to freeze (ignored if null or empty) */ public void freezeOutputs(Collection keyImages) { + if (keyImages == null || keyImages.isEmpty()) return; synchronized (WALLET_LOCK) { - for (String keyImage : keyImages) wallet.freezeOutput(keyImage); + + // collect outputs to freeze + List unfrozenKeyImages = getOutputs(new MoneroOutputQuery().setIsFrozen(false).setIsSpent(false)).stream() + .map(output -> output.getKeyImage().getHex()) + .collect(Collectors.toList()); + unfrozenKeyImages.retainAll(keyImages); + + // freeze outputs + for (String keyImage : unfrozenKeyImages) wallet.freezeOutput(keyImage); cacheWalletInfo(); requestSaveMainWallet(); } @@ -567,7 +576,15 @@ public class XmrWalletService { public void thawOutputs(Collection keyImages) { if (keyImages == null || keyImages.isEmpty()) return; synchronized (WALLET_LOCK) { - for (String keyImage : keyImages) wallet.thawOutput(keyImage); + + // collect outputs to thaw + List frozenKeyImages = getOutputs(new MoneroOutputQuery().setIsFrozen(true).setIsSpent(false)).stream() + .map(output -> output.getKeyImage().getHex()) + .collect(Collectors.toList()); + frozenKeyImages.retainAll(keyImages); + + // thaw outputs + for (String keyImage : frozenKeyImages) wallet.thawOutput(keyImage); cacheWalletInfo(); requestSaveMainWallet(); } diff --git a/desktop/src/main/java/haveno/desktop/main/portfolio/failedtrades/FailedTradesView.java b/desktop/src/main/java/haveno/desktop/main/portfolio/failedtrades/FailedTradesView.java index 3449ea5650..9c351e6272 100644 --- a/desktop/src/main/java/haveno/desktop/main/portfolio/failedtrades/FailedTradesView.java +++ b/desktop/src/main/java/haveno/desktop/main/portfolio/failedtrades/FailedTradesView.java @@ -556,7 +556,7 @@ public class FailedTradesView extends ActivatableViewAndModel