From 0434ba52b6674c5182eb998585ce9535409f92c0 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 3 Sep 2020 14:36:28 -0500 Subject: [PATCH] Refactor dependency structure to enable adding dispute managers If we would add DisputeManager to previous structure it would cause a circular dependency error from guice. We change dependency structure so that TradeManager does not know XmrTxProofService but XmrTxProofService gets an instance of TradeManager. It makes code cleaner in total as well as responsibility is better defined. Next commit will contain the DisputeManager addition. --- .../java/bisq/core/app/BisqExecutable.java | 2 + .../main/java/bisq/core/app/BisqSetup.java | 5 + .../java/bisq/core/trade/TradeManager.java | 44 +------ ...CounterCurrencyTransferStartedMessage.java | 6 - .../trade/txproof/AssetTxProofService.java | 12 +- .../trade/txproof/xmr/XmrTxProofService.java | 110 +++++++++++------- .../pendingtrades/PendingTradesDataModel.java | 2 +- 7 files changed, 83 insertions(+), 98 deletions(-) diff --git a/core/src/main/java/bisq/core/app/BisqExecutable.java b/core/src/main/java/bisq/core/app/BisqExecutable.java index 2cb4760f38..2fe5be9ee0 100644 --- a/core/src/main/java/bisq/core/app/BisqExecutable.java +++ b/core/src/main/java/bisq/core/app/BisqExecutable.java @@ -26,6 +26,7 @@ import bisq.core.setup.CorePersistedDataHost; import bisq.core.setup.CoreSetup; import bisq.core.support.dispute.arbitration.arbitrator.ArbitratorManager; import bisq.core.trade.TradeManager; +import bisq.core.trade.txproof.xmr.XmrTxProofService; import bisq.network.p2p.P2PService; @@ -221,6 +222,7 @@ public abstract class BisqExecutable implements GracefulShutDownHandler, BisqSet try { injector.getInstance(ArbitratorManager.class).shutDown(); injector.getInstance(TradeManager.class).shutDown(); + injector.getInstance(XmrTxProofService.class).shutDown(); injector.getInstance(DaoSetup.class).shutDown(); injector.getInstance(AvoidStandbyModeService.class).shutDown(); injector.getInstance(OpenOfferManager.class).shutDown(() -> { diff --git a/core/src/main/java/bisq/core/app/BisqSetup.java b/core/src/main/java/bisq/core/app/BisqSetup.java index 9440a98559..2bafa44b85 100644 --- a/core/src/main/java/bisq/core/app/BisqSetup.java +++ b/core/src/main/java/bisq/core/app/BisqSetup.java @@ -59,6 +59,7 @@ import bisq.core.support.traderchat.TraderChatManager; import bisq.core.trade.TradeManager; import bisq.core.trade.TradeTxException; import bisq.core.trade.statistics.TradeStatisticsManager; +import bisq.core.trade.txproof.xmr.XmrTxProofService; import bisq.core.user.Preferences; import bisq.core.user.User; import bisq.core.util.FormattingUtils; @@ -167,6 +168,7 @@ public class BisqSetup { private final PrivateNotificationManager privateNotificationManager; private final FilterManager filterManager; private final TradeStatisticsManager tradeStatisticsManager; + private final XmrTxProofService xmrTxProofService; private final ClockWatcher clockWatcher; private final FeeService feeService; private final DaoSetup daoSetup; @@ -263,6 +265,7 @@ public class BisqSetup { PrivateNotificationManager privateNotificationManager, FilterManager filterManager, TradeStatisticsManager tradeStatisticsManager, + XmrTxProofService xmrTxProofService, ClockWatcher clockWatcher, FeeService feeService, DaoSetup daoSetup, @@ -308,6 +311,7 @@ public class BisqSetup { this.privateNotificationManager = privateNotificationManager; this.filterManager = filterManager; this.tradeStatisticsManager = tradeStatisticsManager; + this.xmrTxProofService = xmrTxProofService; this.clockWatcher = clockWatcher; this.feeService = feeService; this.daoSetup = daoSetup; @@ -686,6 +690,7 @@ public class BisqSetup { traderChatManager.onAllServicesInitialized(); tradeManager.onAllServicesInitialized(); + xmrTxProofService.onAllServicesInitialized(); if (walletsSetup.downloadPercentageProperty().get() == 1) { checkForLockedUpFunds(); diff --git a/core/src/main/java/bisq/core/trade/TradeManager.java b/core/src/main/java/bisq/core/trade/TradeManager.java index 8d3c2d738c..66962f10d6 100644 --- a/core/src/main/java/bisq/core/trade/TradeManager.java +++ b/core/src/main/java/bisq/core/trade/TradeManager.java @@ -46,8 +46,6 @@ import bisq.core.trade.messages.PeerPublishedDelayedPayoutTxMessage; import bisq.core.trade.messages.TradeMessage; import bisq.core.trade.statistics.ReferralIdService; import bisq.core.trade.statistics.TradeStatisticsManager; -import bisq.core.trade.txproof.AssetTxProofResult; -import bisq.core.trade.txproof.xmr.XmrTxProofService; import bisq.core.user.User; import bisq.core.util.Validator; @@ -59,7 +57,6 @@ import bisq.network.p2p.P2PService; import bisq.network.p2p.SendMailboxMessageListener; import bisq.common.ClockWatcher; -import bisq.common.UserThread; import bisq.common.config.Config; import bisq.common.crypto.KeyRing; import bisq.common.handlers.ErrorMessageHandler; @@ -111,8 +108,6 @@ import org.jetbrains.annotations.NotNull; import javax.annotation.Nullable; -import static com.google.common.base.Preconditions.checkArgument; - public class TradeManager implements PersistedDataHost { private static final Logger log = LoggerFactory.getLogger(TradeManager.class); @@ -132,7 +127,6 @@ public class TradeManager implements PersistedDataHost { private final ReferralIdService referralIdService; private final AccountAgeWitnessService accountAgeWitnessService; @Getter - private final XmrTxProofService xmrTxProofService; private final ArbitratorManager arbitratorManager; private final MediatorManager mediatorManager; private final RefundAgentManager refundAgentManager; @@ -174,7 +168,6 @@ public class TradeManager implements PersistedDataHost { TradeStatisticsManager tradeStatisticsManager, ReferralIdService referralIdService, AccountAgeWitnessService accountAgeWitnessService, - XmrTxProofService xmrTxProofService, ArbitratorManager arbitratorManager, MediatorManager mediatorManager, RefundAgentManager refundAgentManager, @@ -197,7 +190,6 @@ public class TradeManager implements PersistedDataHost { this.tradeStatisticsManager = tradeStatisticsManager; this.referralIdService = referralIdService; this.accountAgeWitnessService = accountAgeWitnessService; - this.xmrTxProofService = xmrTxProofService; this.arbitratorManager = arbitratorManager; this.mediatorManager = mediatorManager; this.refundAgentManager = refundAgentManager; @@ -286,7 +278,7 @@ public class TradeManager implements PersistedDataHost { } public void shutDown() { - xmrTxProofService.shutDown(); + // Do nothing here } private void initPendingTrades() { @@ -329,13 +321,6 @@ public class TradeManager implements PersistedDataHost { addTradeToFailedTradesList.add(trade); } } - - if (trade.getState() == Trade.State.SELLER_RECEIVED_FIAT_PAYMENT_INITIATED_MSG) { - // This state can be only appear at a SellerTrade - checkArgument(trade instanceof SellerTrade, "Trade must be instance of SellerTrade"); - // We delay a bit as at startup lots of stuff is happening - UserThread.runAfter(() -> maybeStartXmrTxProofServices((SellerTrade) trade), 1); - } } ); @@ -360,31 +345,10 @@ public class TradeManager implements PersistedDataHost { pendingTradesInitialized.set(true); } - public void maybeStartXmrTxProofServices(SellerTrade sellerTrade) { - xmrTxProofService.maybeStartRequests(sellerTrade, tradableList.getList(), - assetTxProofResult -> { - if (assetTxProofResult == AssetTxProofResult.COMPLETED) { - log.info("###########################################################################################"); - log.info("We auto-confirm trade {} as our all our services for the tx proof completed successfully", sellerTrade.getShortId()); - log.info("###########################################################################################"); - autoConfirmFiatPaymentReceived(sellerTrade); - } - }, - (errorMessage, throwable) -> { - log.error(errorMessage); - }); - } - private void autoConfirmFiatPaymentReceived(SellerTrade sellerTrade) { - onFiatPaymentReceived(sellerTrade, - () -> { - }, errorMessage -> { - }); - } - - public void onFiatPaymentReceived(SellerTrade sellerTrade, - ResultHandler resultHandler, - ErrorMessageHandler errorMessageHandler) { + public void onUserConfirmedFiatPaymentReceived(SellerTrade sellerTrade, + ResultHandler resultHandler, + ErrorMessageHandler errorMessageHandler) { sellerTrade.onFiatPaymentReceived(resultHandler, errorMessageHandler); //TODO move to trade protocol task diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerProcessCounterCurrencyTransferStartedMessage.java b/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerProcessCounterCurrencyTransferStartedMessage.java index 29ca5bc63e..ee996ffd9c 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerProcessCounterCurrencyTransferStartedMessage.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerProcessCounterCurrencyTransferStartedMessage.java @@ -17,7 +17,6 @@ package bisq.core.trade.protocol.tasks.seller; -import bisq.core.trade.SellerTrade; import bisq.core.trade.Trade; import bisq.core.trade.messages.CounterCurrencyTransferStartedMessage; import bisq.core.trade.protocol.tasks.TradeTask; @@ -27,7 +26,6 @@ import bisq.common.taskrunner.TaskRunner; import lombok.extern.slf4j.Slf4j; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; @Slf4j @@ -62,10 +60,6 @@ public class SellerProcessCounterCurrencyTransferStartedMessage extends TradeTas trade.setCounterCurrencyExtraData(counterCurrencyExtraData); } - checkArgument(trade instanceof SellerTrade, "Trade must be instance of SellerTrade"); - // We return early in the service if its not XMR. We prefer to not have additional checks outside... - processModel.getTradeManager().maybeStartXmrTxProofServices((SellerTrade) trade); - processModel.removeMailboxMessageAfterProcessing(trade); trade.setState(Trade.State.SELLER_RECEIVED_FIAT_PAYMENT_INITIATED_MSG); diff --git a/core/src/main/java/bisq/core/trade/txproof/AssetTxProofService.java b/core/src/main/java/bisq/core/trade/txproof/AssetTxProofService.java index 56e2a7462e..c5c8476a41 100644 --- a/core/src/main/java/bisq/core/trade/txproof/AssetTxProofService.java +++ b/core/src/main/java/bisq/core/trade/txproof/AssetTxProofService.java @@ -17,18 +17,8 @@ package bisq.core.trade.txproof; -import bisq.core.trade.Trade; - -import bisq.common.handlers.FaultHandler; - -import java.util.List; -import java.util.function.Consumer; - public interface AssetTxProofService { - void maybeStartRequests(Trade trade, - List activeTrades, - Consumer resultHandler, - FaultHandler faultHandler); + void onAllServicesInitialized(); void shutDown(); } diff --git a/core/src/main/java/bisq/core/trade/txproof/xmr/XmrTxProofService.java b/core/src/main/java/bisq/core/trade/txproof/xmr/XmrTxProofService.java index bd63e1df11..5590c56c5e 100644 --- a/core/src/main/java/bisq/core/trade/txproof/xmr/XmrTxProofService.java +++ b/core/src/main/java/bisq/core/trade/txproof/xmr/XmrTxProofService.java @@ -23,6 +23,7 @@ import bisq.core.locale.Res; import bisq.core.payment.payload.AssetsAccountPayload; import bisq.core.trade.SellerTrade; import bisq.core.trade.Trade; +import bisq.core.trade.TradeManager; import bisq.core.trade.closed.ClosedTradableManager; import bisq.core.trade.failed.FailedTradesManager; import bisq.core.trade.txproof.AssetTxProofHttpClient; @@ -34,16 +35,16 @@ import bisq.core.user.Preferences; import bisq.network.p2p.P2PService; import bisq.common.app.DevEnv; -import bisq.common.handlers.FaultHandler; import javax.inject.Inject; import javax.inject.Singleton; +import javafx.collections.ListChangeListener; +import javafx.collections.ObservableList; + import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; -import java.util.function.Consumer; import java.util.stream.Stream; import lombok.extern.slf4j.Slf4j; @@ -59,12 +60,14 @@ import static com.google.common.base.Preconditions.checkNotNull; public class XmrTxProofService implements AssetTxProofService { private final FilterManager filterManager; private final Preferences preferences; + private final TradeManager tradeManager; private final ClosedTradableManager closedTradableManager; private final FailedTradesManager failedTradesManager; private final P2PService p2PService; private final WalletsSetup walletsSetup; private final AssetTxProofHttpClient httpClient; private final Map servicesByTradeId = new HashMap<>(); + private AutoConfirmSettings autoConfirmSettings; /////////////////////////////////////////////////////////////////////////////////////////// @@ -75,6 +78,7 @@ public class XmrTxProofService implements AssetTxProofService { @Inject public XmrTxProofService(FilterManager filterManager, Preferences preferences, + TradeManager tradeManager, ClosedTradableManager closedTradableManager, FailedTradesManager failedTradesManager, P2PService p2PService, @@ -82,11 +86,25 @@ public class XmrTxProofService implements AssetTxProofService { AssetTxProofHttpClient httpClient) { this.filterManager = filterManager; this.preferences = preferences; + this.tradeManager = tradeManager; this.closedTradableManager = closedTradableManager; this.failedTradesManager = failedTradesManager; this.p2PService = p2PService; this.walletsSetup = walletsSetup; this.httpClient = httpClient; + } + + + /////////////////////////////////////////////////////////////////////////////////////////// + // API + /////////////////////////////////////////////////////////////////////////////////////////// + + @Override + public void onAllServicesInitialized() { + if (!preferences.findAutoConfirmSettings("XMR").isPresent()) { + log.error("AutoConfirmSettings is not present"); + } + autoConfirmSettings = preferences.findAutoConfirmSettings("XMR").get(); filterManager.filterProperty().addListener((observable, oldValue, newValue) -> { if (isAutoConfDisabledByFilter()) { @@ -96,22 +114,39 @@ public class XmrTxProofService implements AssetTxProofService { shutDown(); } }); + + ObservableList tradableList = tradeManager.getTradableList(); + tradableList.addListener((ListChangeListener) c -> { + c.next(); + if (c.wasAdded()) { + processTrades(c.getAddedSubList()); + } + }); + processTrades(tradableList); + } + + @Override + public void shutDown() { + servicesByTradeId.values().forEach(XmrTxProofRequestsPerTrade::terminate); + servicesByTradeId.clear(); } /////////////////////////////////////////////////////////////////////////////////////////// - // API + // Private /////////////////////////////////////////////////////////////////////////////////////////// - @Override - public void maybeStartRequests(Trade trade, - List activeTrades, - Consumer resultHandler, - FaultHandler faultHandler) { - if (!isXmrBuyer(trade)) { - return; - } + private void processTrades(List trades) { + trades.stream() + .filter(trade -> trade instanceof SellerTrade) + .map(trade -> (SellerTrade) trade) + .filter(this::isExpectedTradeState) + .filter(this::isXmrBuyer) + .filter(trade -> networkAndWalletReady()) + .forEach(this::processTrade); + } + private void processTrade(SellerTrade trade) { String txId = trade.getCounterCurrencyTxId(); String txHash = trade.getCounterCurrencyExtraData(); if (is32BitHexStringInValid(txId) || is32BitHexStringInValid(txHash)) { @@ -119,25 +154,13 @@ public class XmrTxProofService implements AssetTxProofService { return; } - if (!networkAndWalletReady()) { - return; - } - - Optional optionalAutoConfirmSettings = preferences.findAutoConfirmSettings("XMR"); - if (!optionalAutoConfirmSettings.isPresent()) { - // Not expected - log.error("autoConfirmSettings is not present"); - return; - } - AutoConfirmSettings autoConfirmSettings = optionalAutoConfirmSettings.get(); - if (isAutoConfDisabledByFilter()) { trade.setAssetTxProofResult(AssetTxProofResult.FEATURE_DISABLED .details(Res.get("portfolio.pending.autoConf.state.filterDisabledFeature"))); return; } - if (wasTxKeyReUsed(trade, activeTrades)) { + if (wasTxKeyReUsed(trade, tradeManager.getTradableList())) { trade.setAssetTxProofResult(AssetTxProofResult.INVALID_DATA .details(Res.get("portfolio.pending.autoConf.state.xmr.txKeyReused"))); return; @@ -151,19 +174,22 @@ public class XmrTxProofService implements AssetTxProofService { assetTxProofResult -> { trade.setAssetTxProofResult(assetTxProofResult); + if (assetTxProofResult == AssetTxProofResult.COMPLETED) { + log.info("###########################################################################################"); + log.info("We auto-confirm trade {} as our all our services for the tx proof completed successfully", trade.getShortId()); + log.info("###########################################################################################"); + trade.onFiatPaymentReceived(() -> { + }, errorMessage -> { + }); + } + if (assetTxProofResult.isTerminal()) { servicesByTradeId.remove(trade.getId()); } - - resultHandler.accept(assetTxProofResult); }, - faultHandler); - } - - @Override - public void shutDown() { - servicesByTradeId.values().forEach(XmrTxProofRequestsPerTrade::terminate); - servicesByTradeId.clear(); + (errorMessage, throwable) -> { + log.error(errorMessage); + }); } @@ -171,6 +197,10 @@ public class XmrTxProofService implements AssetTxProofService { // Validation /////////////////////////////////////////////////////////////////////////////////////////// + private boolean isExpectedTradeState(SellerTrade sellerTrade) { + return sellerTrade.getState() == Trade.State.SELLER_RECEIVED_FIAT_PAYMENT_INITIATED_MSG; + } + private boolean isXmrBuyer(Trade trade) { if (!checkNotNull(trade.getOffer()).getCurrencyCode().equals("XMR")) { return false; @@ -183,6 +213,12 @@ public class XmrTxProofService implements AssetTxProofService { return checkNotNull(trade.getContract()).getSellerPaymentAccountPayload() instanceof AssetsAccountPayload; } + private boolean networkAndWalletReady() { + return p2PService.isBootstrapped() && + walletsSetup.isDownloadComplete() && + walletsSetup.hasSufficientPeersForBroadcast(); + } + private boolean is32BitHexStringInValid(String hexString) { if (hexString == null || hexString.isEmpty() || !hexString.matches("[a-fA-F0-9]{64}")) { log.warn("Invalid hexString: {}", hexString); @@ -192,12 +228,6 @@ public class XmrTxProofService implements AssetTxProofService { return false; } - private boolean networkAndWalletReady() { - return p2PService.isBootstrapped() && - walletsSetup.isDownloadComplete() && - walletsSetup.hasSufficientPeersForBroadcast(); - } - private boolean isAutoConfDisabledByFilter() { return filterManager.getFilter() != null && filterManager.getFilter().isDisableAutoConf(); diff --git a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesDataModel.java b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesDataModel.java index 19e2ec7e1a..fb03b413f8 100644 --- a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesDataModel.java +++ b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesDataModel.java @@ -194,7 +194,7 @@ public class PendingTradesDataModel extends ActivatableDataModel { public void onFiatPaymentReceived(ResultHandler resultHandler, ErrorMessageHandler errorMessageHandler) { checkNotNull(getTrade(), "trade must not be null"); checkArgument(getTrade() instanceof SellerTrade, "Trade must be instance of SellerTrade"); - tradeManager.onFiatPaymentReceived((SellerTrade) getTrade(), resultHandler, errorMessageHandler); + tradeManager.onUserConfirmedFiatPaymentReceived((SellerTrade) getTrade(), resultHandler, errorMessageHandler); } public void onWithdrawRequest(String toAddress,