From 68a4c21b1743e7de87450b0dda1b64966c4097f6 Mon Sep 17 00:00:00 2001 From: woodser Date: Thu, 23 May 2024 10:23:51 -0400 Subject: [PATCH] enable cancel button while placing an offer --- .../java/haveno/core/offer/OpenOffer.java | 4 ++ .../haveno/core/offer/OpenOfferManager.java | 71 ++++++++++++------- .../offer/placeoffer/PlaceOfferProtocol.java | 10 ++- .../tasks/MakerReserveOfferFunds.java | 12 ++-- .../tasks/MakerSendSignOfferRequest.java | 6 ++ .../java/haveno/core/trade/HavenoUtils.java | 5 +- .../desktop/main/offer/MutableOfferView.java | 9 ++- .../main/offer/MutableOfferViewModel.java | 30 +++++++- .../overlays/windows/OfferDetailsWindow.java | 2 +- 9 files changed, 106 insertions(+), 43 deletions(-) diff --git a/core/src/main/java/haveno/core/offer/OpenOffer.java b/core/src/main/java/haveno/core/offer/OpenOffer.java index bf45657e20..27f9c82916 100644 --- a/core/src/main/java/haveno/core/offer/OpenOffer.java +++ b/core/src/main/java/haveno/core/offer/OpenOffer.java @@ -246,6 +246,10 @@ public final class OpenOffer implements Tradable { return state == State.DEACTIVATED; } + public boolean isCanceled() { + return state == State.CANCELED; + } + @Override public String toString() { return "OpenOffer{" + diff --git a/core/src/main/java/haveno/core/offer/OpenOfferManager.java b/core/src/main/java/haveno/core/offer/OpenOfferManager.java index d6ce814788..a283755a89 100644 --- a/core/src/main/java/haveno/core/offer/OpenOfferManager.java +++ b/core/src/main/java/haveno/core/offer/OpenOfferManager.java @@ -218,6 +218,7 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe this.persistenceManager = persistenceManager; this.signedOfferPersistenceManager = signedOfferPersistenceManager; this.accountAgeWitnessService = accountAgeWitnessService; + HavenoUtils.openOfferManager = this; this.persistenceManager.initialize(openOffers, "OpenOffers", PersistenceManager.Source.PRIVATE); this.signedOfferPersistenceManager.initialize(signedOffers, "SignedOffers", PersistenceManager.Source.PRIVATE); // arbitrator stores reserve tx for signed offers @@ -548,11 +549,14 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe latch.countDown(); resultHandler.handleResult(transaction); }, (errorMessage) -> { - log.warn("Error processing unposted offer {}: {}", openOffer.getId(), errorMessage); - onCancelled(openOffer); - offer.setErrorMessage(errorMessage); - latch.countDown(); - errorMessageHandler.handleErrorMessage(errorMessage); + if (openOffer.isCanceled()) latch.countDown(); + else { + log.warn("Error processing unposted offer {}: {}", openOffer.getId(), errorMessage); + doCancel(openOffer); + offer.setErrorMessage(errorMessage); + latch.countDown(); + errorMessageHandler.handleErrorMessage(errorMessage); + } }); HavenoUtils.awaitLatch(latch); } @@ -612,21 +616,22 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe public void cancelOpenOffer(OpenOffer openOffer, ResultHandler resultHandler, ErrorMessageHandler errorMessageHandler) { + log.info("Canceling open offer: {}", openOffer.getId()); if (!offersToBeEdited.containsKey(openOffer.getId())) { - if (openOffer.isDeactivated()) { - ThreadUtils.execute(() -> { - onCancelled(openOffer); - resultHandler.handleResult(); - }, THREAD_ID); - } else { + if (openOffer.isAvailable()) { offerBookService.removeOffer(openOffer.getOffer().getOfferPayload(), () -> { - ThreadUtils.execute(() -> { // TODO: this runs off thread and then shows popup when done. should show overlay spinner until done - onCancelled(openOffer); + ThreadUtils.submitToPool(() -> { // TODO: this runs off thread and then shows popup when done. should show overlay spinner until done + doCancel(openOffer); resultHandler.handleResult(); - }, THREAD_ID); + }); }, errorMessageHandler); + } else { + ThreadUtils.submitToPool(() -> { + doCancel(openOffer); + resultHandler.handleResult(); + }); } } else { errorMessageHandler.handleErrorMessage("You can't remove an offer that is currently edited."); @@ -703,12 +708,12 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe } // remove open offer which thaws its key images - private void onCancelled(@NotNull OpenOffer openOffer) { + private void doCancel(@NotNull OpenOffer openOffer) { Offer offer = openOffer.getOffer(); offer.setState(Offer.State.REMOVED); openOffer.setState(OpenOffer.State.CANCELED); removeOpenOffer(openOffer); - closedTradableManager.add(openOffer); + closedTradableManager.add(openOffer); // TODO: don't add these to closed tradables? xmrWalletService.resetAddressEntriesForOpenOffer(offer.getId()); requestPersistence(); xmrWalletService.thawOutputs(offer.getOfferPayload().getReserveTxKeyImages()); @@ -785,6 +790,10 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe } } + public boolean hasOpenOffer(String offerId) { + return getOpenOfferById(offerId).isPresent(); + } + public Optional getSignedOfferById(String offerId) { synchronized (signedOffers) { return signedOffers.stream().filter(e -> e.getOfferId().equals(offerId)).findFirst(); @@ -803,6 +812,10 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe synchronized (openOffers) { openOffers.remove(openOffer); } + synchronized (placeOfferProtocols) { + PlaceOfferProtocol protocol = placeOfferProtocols.remove(openOffer.getId()); + if (protocol != null) protocol.cancelOffer(); + } } private void addSignedOffer(SignedOffer signedOffer) { @@ -856,13 +869,15 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe processUnpostedOffer(openOffers, scheduledOffer, (transaction) -> { latch.countDown(); }, errorMessage -> { - log.warn("Error processing unposted offer, offerId={}, attempt={}/{}, error={}", scheduledOffer.getId(), scheduledOffer.getNumProcessingAttempts(), MAX_PROCESS_ATTEMPTS, errorMessage); - if (scheduledOffer.getNumProcessingAttempts() >= MAX_PROCESS_ATTEMPTS) { - log.warn("Offer canceled after {} attempts, offerId={}, error={}", scheduledOffer.getNumProcessingAttempts(), scheduledOffer.getId(), errorMessage); - HavenoUtils.havenoSetup.getTopErrorMsg().set("Offer canceled after " + scheduledOffer.getNumProcessingAttempts() + " attempts. Please switch to a better Monero connection and try again.\n\nOffer ID: " + scheduledOffer.getId() + "\nError: " + errorMessage); - onCancelled(scheduledOffer); + if (!scheduledOffer.isCanceled()) { + log.warn("Error processing unposted offer, offerId={}, attempt={}/{}, error={}", scheduledOffer.getId(), scheduledOffer.getNumProcessingAttempts(), MAX_PROCESS_ATTEMPTS, errorMessage); + if (scheduledOffer.getNumProcessingAttempts() >= MAX_PROCESS_ATTEMPTS) { + log.warn("Offer canceled after {} attempts, offerId={}, error={}", scheduledOffer.getNumProcessingAttempts(), scheduledOffer.getId(), errorMessage); + HavenoUtils.havenoSetup.getTopErrorMsg().set("Offer canceled after " + scheduledOffer.getNumProcessingAttempts() + " attempts. Please switch to a better Monero connection and try again.\n\nOffer ID: " + scheduledOffer.getId() + "\nError: " + errorMessage); + doCancel(scheduledOffer); + } + errorMessages.add(errorMessage); } - errorMessages.add(errorMessage); latch.countDown(); }); HavenoUtils.awaitLatch(latch); @@ -941,7 +956,7 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe // handle result resultHandler.handleResult(null); } catch (Exception e) { - e.printStackTrace(); + if (!openOffer.isCanceled()) e.printStackTrace(); errorMessageHandler.handleErrorMessage(e.getMessage()); } }).start(); @@ -1737,7 +1752,7 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe } else { // cancel and recreate offer - onCancelled(openOffer); + doCancel(openOffer); Offer updatedOffer = new Offer(openOffer.getOffer().getOfferPayload()); updatedOffer.setPriceFeedService(priceFeedService); OpenOffer updatedOpenOffer = new OpenOffer(updatedOffer, openOffer.getTriggerPrice()); @@ -1751,9 +1766,11 @@ public class OpenOfferManager implements PeerManager.Listener, DecryptedDirectMe latch.countDown(); if (completeHandler != null) completeHandler.run(); }, (errorMessage) -> { - log.warn("Error reposting offer {}: {}", updatedOpenOffer.getId(), errorMessage); - onCancelled(updatedOpenOffer); - updatedOffer.setErrorMessage(errorMessage); + if (!updatedOpenOffer.isCanceled()) { + log.warn("Error reposting offer {}: {}", updatedOpenOffer.getId(), errorMessage); + doCancel(updatedOpenOffer); + updatedOffer.setErrorMessage(errorMessage); + } latch.countDown(); if (completeHandler != null) completeHandler.run(); }); diff --git a/core/src/main/java/haveno/core/offer/placeoffer/PlaceOfferProtocol.java b/core/src/main/java/haveno/core/offer/placeoffer/PlaceOfferProtocol.java index 32426b80d5..bb5480d0ac 100644 --- a/core/src/main/java/haveno/core/offer/placeoffer/PlaceOfferProtocol.java +++ b/core/src/main/java/haveno/core/offer/placeoffer/PlaceOfferProtocol.java @@ -84,6 +84,10 @@ public class PlaceOfferProtocol { taskRunner.run(); } + + public void cancelOffer() { + handleError("Offer was canceled: " + model.getOpenOffer().getOffer().getId()); // cancel is treated as error for callers to handle + } // TODO (woodser): switch to fluent public void handleSignOfferResponse(SignOfferResponse response, NodeAddress sender) { @@ -147,9 +151,11 @@ public class PlaceOfferProtocol { private void handleError(String errorMessage) { if (timeoutTimer != null) { taskRunner.cancel(); - log.error(errorMessage); + if (!model.getOpenOffer().isCanceled()) { + log.error(errorMessage); + model.getOpenOffer().getOffer().setErrorMessage(errorMessage); + } stopTimeoutTimer(); - model.getOpenOffer().getOffer().setErrorMessage(errorMessage); errorMessageHandler.handleErrorMessage(errorMessage); } } diff --git a/core/src/main/java/haveno/core/offer/placeoffer/tasks/MakerReserveOfferFunds.java b/core/src/main/java/haveno/core/offer/placeoffer/tasks/MakerReserveOfferFunds.java index bda7ba99fc..7dba99715a 100644 --- a/core/src/main/java/haveno/core/offer/placeoffer/tasks/MakerReserveOfferFunds.java +++ b/core/src/main/java/haveno/core/offer/placeoffer/tasks/MakerReserveOfferFunds.java @@ -66,7 +66,7 @@ public class MakerReserveOfferFunds extends Task { synchronized (XmrWalletService.WALLET_LOCK) { // reset protocol timeout - verifyOpen(); + verifyScheduled(); model.getProtocol().startTimeoutTimer(); // collect relevant info @@ -92,7 +92,7 @@ public class MakerReserveOfferFunds extends Task { } // verify still open - verifyOpen(); + verifyScheduled(); if (reserveTx != null) break; } } @@ -119,11 +119,7 @@ public class MakerReserveOfferFunds extends Task { } } - public void verifyOpen() { - if (!isOpen()) throw new RuntimeException("Offer " + model.getOpenOffer().getOffer().getId() + " is no longer open"); - } - - public boolean isOpen() { - return model.getOpenOfferManager().getOpenOfferById(model.getOpenOffer().getId()).isPresent(); + public void verifyScheduled() { + if (!model.getOpenOffer().isScheduled()) throw new RuntimeException("Offer " + model.getOpenOffer().getOffer().getId() + " is canceled"); } } diff --git a/core/src/main/java/haveno/core/offer/placeoffer/tasks/MakerSendSignOfferRequest.java b/core/src/main/java/haveno/core/offer/placeoffer/tasks/MakerSendSignOfferRequest.java index 8f08b77685..e252e51d34 100644 --- a/core/src/main/java/haveno/core/offer/placeoffer/tasks/MakerSendSignOfferRequest.java +++ b/core/src/main/java/haveno/core/offer/placeoffer/tasks/MakerSendSignOfferRequest.java @@ -137,6 +137,12 @@ public class MakerSendSignOfferRequest extends Task { log.warn("Arbitrator unavailable: address={}, error={}", arbitratorNodeAddress, errorMessage); excludedArbitrators.add(arbitratorNodeAddress); + // check if offer still scheduled + if (!model.getOpenOffer().isScheduled()) { + errorMessageHandler.handleErrorMessage("Offer is no longer scheduled, offerId=" + model.getOpenOffer().getId()); + return; + } + // get alternative arbitrator Arbitrator altArbitrator = DisputeAgentSelection.getRandomArbitrator(model.getArbitratorManager(), excludedArbitrators); if (altArbitrator == null) { diff --git a/core/src/main/java/haveno/core/trade/HavenoUtils.java b/core/src/main/java/haveno/core/trade/HavenoUtils.java index 04cc22a89d..d9acf03d7e 100644 --- a/core/src/main/java/haveno/core/trade/HavenoUtils.java +++ b/core/src/main/java/haveno/core/trade/HavenoUtils.java @@ -30,6 +30,7 @@ import haveno.common.crypto.Sig; import haveno.common.util.Utilities; import haveno.core.app.HavenoSetup; import haveno.core.offer.OfferPayload; +import haveno.core.offer.OpenOfferManager; import haveno.core.support.dispute.arbitration.ArbitrationManager; import haveno.core.support.dispute.arbitration.arbitrator.Arbitrator; import haveno.core.trade.messages.PaymentReceivedMessage; @@ -97,9 +98,11 @@ public class HavenoUtils { public static final DecimalFormat XMR_FORMATTER = new DecimalFormat("##############0.000000000000", DECIMAL_FORMAT_SYMBOLS); public static final SimpleDateFormat DATE_FORMAT = new SimpleDateFormat("dd-MM-yyyy HH:mm:ss"); + // TODO: better way to share references? public static HavenoSetup havenoSetup; - public static ArbitrationManager arbitrationManager; // TODO: better way to share references? + public static ArbitrationManager arbitrationManager; public static XmrWalletService xmrWalletService; + public static OpenOfferManager openOfferManager; public static boolean isSeedNode() { return havenoSetup == null; diff --git a/desktop/src/main/java/haveno/desktop/main/offer/MutableOfferView.java b/desktop/src/main/java/haveno/desktop/main/offer/MutableOfferView.java index d9b41071ee..16c4c8194d 100644 --- a/desktop/src/main/java/haveno/desktop/main/offer/MutableOfferView.java +++ b/desktop/src/main/java/haveno/desktop/main/offer/MutableOfferView.java @@ -348,9 +348,12 @@ public abstract class MutableOfferView> exten if (model.getDataModel().canPlaceOffer()) { Offer offer = model.createAndGetOffer(); if (!DevEnv.isDevMode()) { - offerDetailsWindow.onPlaceOffer(() -> - model.onPlaceOffer(offer, offerDetailsWindow::hide)) - .show(offer); + offerDetailsWindow.onPlaceOffer(() -> { + model.onPlaceOffer(offer, offerDetailsWindow::hide); + }).show(offer); + offerDetailsWindow.onClose(() -> { + model.onCancelOffer(null, null); + }); } else { balanceSubscription.unsubscribe(); model.onPlaceOffer(offer, () -> { diff --git a/desktop/src/main/java/haveno/desktop/main/offer/MutableOfferViewModel.java b/desktop/src/main/java/haveno/desktop/main/offer/MutableOfferViewModel.java index 7dbbd23a4f..fa0dfbbf72 100644 --- a/desktop/src/main/java/haveno/desktop/main/offer/MutableOfferViewModel.java +++ b/desktop/src/main/java/haveno/desktop/main/offer/MutableOfferViewModel.java @@ -21,6 +21,8 @@ import com.google.inject.Inject; import com.google.inject.name.Named; import haveno.common.UserThread; import haveno.common.app.DevEnv; +import haveno.common.handlers.ErrorMessageHandler; +import haveno.common.handlers.ResultHandler; import haveno.common.util.MathUtils; import haveno.core.account.witness.AccountAgeWitnessService; import haveno.core.locale.CurrencyUtil; @@ -34,6 +36,8 @@ import haveno.core.offer.Offer; import haveno.core.offer.OfferDirection; import haveno.core.offer.OfferRestrictions; import haveno.core.offer.OfferUtil; +import haveno.core.offer.OpenOffer; +import haveno.core.offer.OpenOfferManager; import haveno.core.payment.PaymentAccount; import haveno.core.payment.payload.PaymentMethod; import haveno.core.payment.validation.FiatVolumeValidator; @@ -68,6 +72,7 @@ import java.math.BigInteger; import java.text.SimpleDateFormat; import java.util.Calendar; import java.util.Date; +import java.util.Optional; import java.util.concurrent.TimeUnit; import static javafx.beans.binding.Bindings.createStringBinding; import javafx.beans.property.BooleanProperty; @@ -617,7 +622,6 @@ public abstract class MutableOfferViewModel ext updateButtonDisableState(); updateSpinnerInfo(); resultHandler.run(); - }); }); @@ -625,6 +629,30 @@ public abstract class MutableOfferViewModel ext updateSpinnerInfo(); } + public void onCancelOffer(ResultHandler resultHandler, ErrorMessageHandler errorMessageHandler) { + createOfferRequested = false; + OpenOfferManager openOfferManager = HavenoUtils.openOfferManager; + Optional openOffer = openOfferManager.getOpenOfferById(offer.getId()); + if (openOffer.isPresent()) { + openOfferManager.cancelOpenOffer(openOffer.get(), () -> { + UserThread.execute(() -> { + updateButtonDisableState(); + updateSpinnerInfo(); + }); + if (resultHandler != null) resultHandler.handleResult(); + }, errorMessage -> { + UserThread.execute(() -> { + updateButtonDisableState(); + updateSpinnerInfo(); + if (errorMessageHandler != null) errorMessageHandler.handleErrorMessage(errorMessage); + }); + }); + } else { + if (resultHandler != null) resultHandler.handleResult(); + return; + } + } + public void onPaymentAccountSelected(PaymentAccount paymentAccount) { dataModel.onPaymentAccountSelected(paymentAccount); if (amount.get() != null) diff --git a/desktop/src/main/java/haveno/desktop/main/overlays/windows/OfferDetailsWindow.java b/desktop/src/main/java/haveno/desktop/main/overlays/windows/OfferDetailsWindow.java index 1814fc8f62..4764930dba 100644 --- a/desktop/src/main/java/haveno/desktop/main/overlays/windows/OfferDetailsWindow.java +++ b/desktop/src/main/java/haveno/desktop/main/overlays/windows/OfferDetailsWindow.java @@ -417,7 +417,7 @@ public class OfferDetailsWindow extends Overlay { button.setOnAction(e -> { if (GUIUtil.canCreateOrTakeOfferOrShowPopup(user, navigation)) { button.setDisable(true); - cancelButton.setDisable(true); + cancelButton.setDisable(isPlaceOffer ? false : true); // TODO: enable cancel button for taking an offer until messages sent // temporarily disabled due to high CPU usage (per issue #4649) // busyAnimation.play(); if (isPlaceOffer) {