From c7a3a9740ff7386ba2a6dfdefe1c2a02372d79e5 Mon Sep 17 00:00:00 2001 From: woodser Date: Sat, 19 Apr 2025 16:54:01 -0400 Subject: [PATCH] fixes when cloned offers are taken at the same time --- .../main/java/haveno/core/trade/Trade.java | 9 ++++- .../core/trade/protocol/TradeProtocol.java | 16 +++++++-- .../ArbitratorProcessDepositRequest.java | 33 +++++++++++-------- .../tasks/ProcessDepositResponse.java | 9 ++--- .../haveno/core/xmr/wallet/XmrWalletBase.java | 1 - .../main/funds/deposit/DepositView.java | 3 +- 6 files changed, 47 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/haveno/core/trade/Trade.java b/core/src/main/java/haveno/core/trade/Trade.java index 9d4112ff3f..2dfa5b0f1f 100644 --- a/core/src/main/java/haveno/core/trade/Trade.java +++ b/core/src/main/java/haveno/core/trade/Trade.java @@ -2648,7 +2648,7 @@ public abstract class Trade extends XmrWalletBase implements Tradable, Model { } } setDepositTxs(txs); - if (getMaker().getDepositTx() == null || (getTaker().getDepositTx() == null && !hasBuyerAsTakerWithoutDeposit())) return; // skip if either deposit tx not seen + if (!isPublished(getMaker().getDepositTx()) || (!hasBuyerAsTakerWithoutDeposit() && !isPublished(getTaker().getDepositTx()))) return; // skip if deposit txs not published successfully setStateDepositsSeen(); // set actual security deposits @@ -2750,6 +2750,13 @@ public abstract class Trade extends XmrWalletBase implements Tradable, Model { } } + private static boolean isPublished(MoneroTx tx) { + if (tx == null) return false; + if (Boolean.TRUE.equals(tx.isFailed())) return false; + if (!Boolean.TRUE.equals(tx.inTxPool()) && !Boolean.TRUE.equals(tx.isConfirmed())) return false; + return true; + } + private void syncWalletIfBehind() { synchronized (walletLock) { if (isWalletBehind()) { 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 a1ad25ddaf..d87d5f3d5d 100644 --- a/core/src/main/java/haveno/core/trade/protocol/TradeProtocol.java +++ b/core/src/main/java/haveno/core/trade/protocol/TradeProtocol.java @@ -460,9 +460,19 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D .using(new TradeTaskRunner(trade, () -> { stopTimeout(); - this.errorMessageHandler = null; // TODO: set this when trade state is >= DEPOSIT_PUBLISHED - handleTaskRunnerSuccess(sender, response); - if (tradeResultHandler != null) tradeResultHandler.handleResult(trade); // trade is initialized + + // tasks may complete successfully but process an error + if (trade.getInitError() == null) { + this.errorMessageHandler = null; // TODO: set this when trade state is >= DEPOSIT_PUBLISHED + handleTaskRunnerSuccess(sender, response); + if (tradeResultHandler != null) tradeResultHandler.handleResult(trade); // trade is initialized + } else { + handleTaskRunnerSuccess(sender, response); + if (errorMessageHandler != null) errorMessageHandler.handleErrorMessage(trade.getInitError().getMessage()); + } + + this.tradeResultHandler = null; + this.errorMessageHandler = null; }, errorMessage -> { handleTaskRunnerFault(sender, response, errorMessage); 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 3a7fc7ace9..c11ed57ee4 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 @@ -95,6 +95,18 @@ public class ArbitratorProcessDepositRequest extends TradeTask { // set peer's signature sender.setContractSignature(signature); + // subscribe to trade state once to send responses with ack or nack + if (!hasBothContractSignatures()) { + trade.stateProperty().addListener((obs, oldState, newState) -> { + if (oldState == newState) return; + if (newState == Trade.State.PUBLISH_DEPOSIT_TX_REQUEST_FAILED) { + sendDepositResponsesOnce(trade.getProcessModel().error == null ? "Arbitrator failed to publish deposit txs within timeout for trade " + trade.getId() : trade.getProcessModel().error.getMessage()); + } else if (newState.ordinal() >= Trade.State.ARBITRATOR_PUBLISHED_DEPOSIT_TXS.ordinal()) { + sendDepositResponsesOnce(null); + } + }); + } + // collect expected values Offer offer = trade.getOffer(); boolean isFromTaker = sender == trade.getTaker(); @@ -138,7 +150,7 @@ public class ArbitratorProcessDepositRequest extends TradeTask { // relay deposit txs when both requests received MoneroDaemon daemon = trade.getXmrWalletService().getDaemon(); - if (processModel.getMaker().getContractSignature() != null && processModel.getTaker().getContractSignature() != null) { + if (hasBothContractSignatures()) { // check timeout and extend just before relaying if (isTimedOut()) throw new RuntimeException("Trade protocol has timed out before relaying deposit txs for {} {}" + trade.getClass().getSimpleName() + " " + trade.getShortId()); @@ -182,22 +194,15 @@ public class ArbitratorProcessDepositRequest extends TradeTask { throw e; } } else { - - // subscribe to trade state once to send responses with ack or nack - trade.stateProperty().addListener((obs, oldState, newState) -> { - if (oldState == newState) return; - if (newState == Trade.State.PUBLISH_DEPOSIT_TX_REQUEST_FAILED) { - sendDepositResponsesOnce(trade.getProcessModel().error == null ? "Arbitrator failed to publish deposit txs within timeout for trade " + trade.getId() : trade.getProcessModel().error.getMessage()); - } else if (newState.ordinal() >= Trade.State.ARBITRATOR_PUBLISHED_DEPOSIT_TXS.ordinal()) { - sendDepositResponsesOnce(null); - } - }); - if (processModel.getMaker().getDepositTxHex() == null) log.info("Arbitrator waiting for deposit request from maker for trade " + trade.getId()); if (processModel.getTaker().getDepositTxHex() == null && !trade.hasBuyerAsTakerWithoutDeposit()) log.info("Arbitrator waiting for deposit request from taker for trade " + trade.getId()); } } + private boolean hasBothContractSignatures() { + return processModel.getMaker().getContractSignature() != null && processModel.getTaker().getContractSignature() != null; + } + private boolean isTimedOut() { return !processModel.getTradeManager().hasOpenTrade(trade); } @@ -210,7 +215,7 @@ public class ArbitratorProcessDepositRequest extends TradeTask { // log error if (errorMessage != null) { - log.warn("Sending deposit responses with error={}", errorMessage, new Throwable("Stack trace")); + log.warn("Sending deposit responses for tradeId={}, error={}", trade.getId(), errorMessage); } // create deposit response @@ -229,7 +234,7 @@ public class ArbitratorProcessDepositRequest extends TradeTask { } private void sendDepositResponse(NodeAddress nodeAddress, PubKeyRing pubKeyRing, DepositResponse response) { - log.info("Sending deposit response to trader={}; offerId={}, error={}", nodeAddress, trade.getId(), trade.getProcessModel().error); + log.info("Sending deposit response to trader={}; offerId={}, error={}", nodeAddress, trade.getId(), response.getErrorMessage()); processModel.getP2PService().sendEncryptedDirectMessage(nodeAddress, pubKeyRing, response, new SendDirectMessageListener() { @Override public void onArrived() { 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 dcaf1a7e76..454763e15b 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 @@ -38,13 +38,14 @@ public class ProcessDepositResponse extends TradeTask { try { runInterceptHook(); - // throw if error + // handle error DepositResponse message = (DepositResponse) processModel.getTradeMessage(); if (message.getErrorMessage() != null) { - log.warn("Unregistering trade {} {} because deposit response has error message={}", trade.getClass().getSimpleName(), trade.getShortId(), message.getErrorMessage()); + log.warn("Deposit response for {} {} has error message={}", trade.getClass().getSimpleName(), trade.getShortId(), message.getErrorMessage()); trade.setStateIfValidTransitionTo(Trade.State.PUBLISH_DEPOSIT_TX_REQUEST_FAILED); - processModel.getTradeManager().unregisterTrade(trade); - throw new RuntimeException(message.getErrorMessage()); + trade.setInitError(new RuntimeException(message.getErrorMessage())); + complete(); + return; } // record security deposits diff --git a/core/src/main/java/haveno/core/xmr/wallet/XmrWalletBase.java b/core/src/main/java/haveno/core/xmr/wallet/XmrWalletBase.java index 9646f5e3f4..594f58b6fb 100644 --- a/core/src/main/java/haveno/core/xmr/wallet/XmrWalletBase.java +++ b/core/src/main/java/haveno/core/xmr/wallet/XmrWalletBase.java @@ -105,7 +105,6 @@ public abstract class XmrWalletBase { // start polling wallet for progress syncProgressLatch = new CountDownLatch(1); syncProgressLooper = new TaskLooper(() -> { - if (wallet == null) return; long height; try { height = wallet.getHeight(); // can get read timeout while syncing diff --git a/desktop/src/main/java/haveno/desktop/main/funds/deposit/DepositView.java b/desktop/src/main/java/haveno/desktop/main/funds/deposit/DepositView.java index 884df454e7..e035a04f23 100644 --- a/desktop/src/main/java/haveno/desktop/main/funds/deposit/DepositView.java +++ b/desktop/src/main/java/haveno/desktop/main/funds/deposit/DepositView.java @@ -346,7 +346,8 @@ public class DepositView extends ActivatableView { List addressEntries = xmrWalletService.getAddressEntries(); List items = new ArrayList<>(); for (XmrAddressEntry addressEntry : addressEntries) { - if (addressEntry.isTradePayout()) continue; // do not show trade payout addresses + DepositListItem item = new DepositListItem(addressEntry, xmrWalletService, formatter); + if (addressEntry.isTradePayout() && BigInteger.ZERO.equals(item.getBalanceAsBI())) continue; // do not show empty trade payout addresses items.add(new DepositListItem(addressEntry, xmrWalletService, formatter)); }