From f61fd091277da75b733eb7d329bd00b03951c475 Mon Sep 17 00:00:00 2001 From: woodser Date: Fri, 29 Jul 2022 17:58:52 -0400 Subject: [PATCH] add grpc error handling for confirming payment sent and received --- core/src/main/java/bisq/core/api/CoreApi.java | 17 ++++----- .../java/bisq/core/api/CoreTradesService.java | 26 +++++--------- .../core/btc/wallet/XmrWalletService.java | 16 ++++----- core/src/main/java/bisq/core/trade/Trade.java | 2 +- .../core/trade/protocol/BuyerProtocol.java | 10 +++--- .../core/trade/protocol/SellerProtocol.java | 5 ++- .../bisq/daemon/grpc/GrpcTradesService.java | 36 ++++++++++++------- 7 files changed, 58 insertions(+), 54 deletions(-) diff --git a/core/src/main/java/bisq/core/api/CoreApi.java b/core/src/main/java/bisq/core/api/CoreApi.java index fb815ab5..c0e92c8b 100644 --- a/core/src/main/java/bisq/core/api/CoreApi.java +++ b/core/src/main/java/bisq/core/api/CoreApi.java @@ -549,18 +549,19 @@ public class CoreApi { Consumer resultHandler, ErrorMessageHandler errorMessageHandler) { Offer offer = coreOffersService.getOffer(offerId); - coreTradesService.takeOffer(offer, - paymentAccountId, - resultHandler, - errorMessageHandler); + coreTradesService.takeOffer(offer, paymentAccountId, resultHandler, errorMessageHandler); } - public void confirmPaymentStarted(String tradeId) { - coreTradesService.confirmPaymentStarted(tradeId); + public void confirmPaymentStarted(String tradeId, + ResultHandler resultHandler, + ErrorMessageHandler errorMessageHandler) { + coreTradesService.confirmPaymentStarted(tradeId, resultHandler, errorMessageHandler); } - public void confirmPaymentReceived(String tradeId) { - coreTradesService.confirmPaymentReceived(tradeId); + public void confirmPaymentReceived(String tradeId, + ResultHandler resultHandler, + ErrorMessageHandler errorMessageHandler) { + coreTradesService.confirmPaymentReceived(tradeId, resultHandler, errorMessageHandler); } public void keepFunds(String tradeId) { diff --git a/core/src/main/java/bisq/core/api/CoreTradesService.java b/core/src/main/java/bisq/core/api/CoreTradesService.java index c8ed9024..3264b268 100644 --- a/core/src/main/java/bisq/core/api/CoreTradesService.java +++ b/core/src/main/java/bisq/core/api/CoreTradesService.java @@ -35,7 +35,7 @@ import bisq.core.user.User; import bisq.core.util.validation.BtcAddressValidator; import bisq.common.handlers.ErrorMessageHandler; - +import bisq.common.handlers.ResultHandler; import org.bitcoinj.core.Coin; import javax.inject.Inject; @@ -131,35 +131,27 @@ class CoreTradesService { } } - void confirmPaymentStarted(String tradeId) { + void confirmPaymentStarted(String tradeId, + ResultHandler resultHandler, + ErrorMessageHandler errorMessageHandler) { var trade = getTrade(tradeId); if (isFollowingBuyerProtocol(trade)) { var tradeProtocol = tradeManager.getTradeProtocol(trade); - ((BuyerProtocol) tradeProtocol).onPaymentStarted( - () -> { - }, - errorMessage -> { - throw new IllegalStateException(errorMessage); - } - ); + ((BuyerProtocol) tradeProtocol).onPaymentStarted(resultHandler, errorMessageHandler); } else { throw new IllegalStateException("you are the seller and not sending payment"); } } - void confirmPaymentReceived(String tradeId) { + void confirmPaymentReceived(String tradeId, + ResultHandler resultHandler, + ErrorMessageHandler errorMessageHandler) { var trade = getTrade(tradeId); if (isFollowingBuyerProtocol(trade)) { throw new IllegalStateException("you are the buyer, and not receiving payment"); } else { var tradeProtocol = tradeManager.getTradeProtocol(trade); - ((SellerProtocol) tradeProtocol).onPaymentReceived( - () -> { - }, - errorMessage -> { - throw new IllegalStateException(errorMessage); - } - ); + ((SellerProtocol) tradeProtocol).onPaymentReceived(resultHandler, errorMessageHandler); } } diff --git a/core/src/main/java/bisq/core/btc/wallet/XmrWalletService.java b/core/src/main/java/bisq/core/btc/wallet/XmrWalletService.java index b22b1268..a286c565 100644 --- a/core/src/main/java/bisq/core/btc/wallet/XmrWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/XmrWalletService.java @@ -185,23 +185,21 @@ public class XmrWalletService { // TODO (woodser): test retaking failed trade. create new multisig wallet or replace? cannot reuse public MoneroWallet createMultisigWallet(String tradeId) { log.info("{}.createMultisigWallet({})", getClass().getSimpleName(), tradeId); - Trade trade = tradeManager.getOpenTrade(tradeId).get(); - if (multisigWallets.containsKey(trade.getId())) return multisigWallets.get(trade.getId()); - String path = MONERO_MULTISIG_WALLET_PREFIX + trade.getId(); + if (multisigWallets.containsKey(tradeId)) return multisigWallets.get(tradeId); + String path = MONERO_MULTISIG_WALLET_PREFIX + tradeId; MoneroWallet multisigWallet = createWallet(new MoneroWalletConfig().setPath(path).setPassword(getWalletPassword()), null, false); // auto-assign port - multisigWallets.put(trade.getId(), multisigWallet); + multisigWallets.put(tradeId, multisigWallet); return multisigWallet; } // TODO (woodser): provide progress notifications during open? public MoneroWallet getMultisigWallet(String tradeId) { log.info("{}.getMultisigWallet({})", getClass().getSimpleName(), tradeId); - Trade trade = tradeManager.getTrade(tradeId); - if (multisigWallets.containsKey(trade.getId())) return multisigWallets.get(trade.getId()); - String path = MONERO_MULTISIG_WALLET_PREFIX + trade.getId(); - if (!walletExists(path)) throw new RuntimeException("Multisig wallet does not exist for trade " + trade.getId()); + if (multisigWallets.containsKey(tradeId)) return multisigWallets.get(tradeId); + String path = MONERO_MULTISIG_WALLET_PREFIX + tradeId; + if (!walletExists(path)) throw new RuntimeException("Multisig wallet does not exist for trade " + tradeId); MoneroWallet multisigWallet = openWallet(new MoneroWalletConfig().setPath(path).setPassword(getWalletPassword()), null); - multisigWallets.put(trade.getId(), multisigWallet); + multisigWallets.put(tradeId, multisigWallet); return multisigWallet; } diff --git a/core/src/main/java/bisq/core/trade/Trade.java b/core/src/main/java/bisq/core/trade/Trade.java index efa0c5b7..84dfef0a 100644 --- a/core/src/main/java/bisq/core/trade/Trade.java +++ b/core/src/main/java/bisq/core/trade/Trade.java @@ -923,7 +923,7 @@ public abstract class Trade implements Tradable, Model { } // check if deposit txs unlocked - if (unlockHeight != null && height == unlockHeight) { + if (unlockHeight != null && height >= unlockHeight) { log.info("Multisig deposits unlocked for trade {}", getId()); setUnlockedState(); xmrWalletService.removeWalletListener(depositTxListener); // remove listener when notified diff --git a/core/src/main/java/bisq/core/trade/protocol/BuyerProtocol.java b/core/src/main/java/bisq/core/trade/protocol/BuyerProtocol.java index 1c0d1f80..8e501563 100644 --- a/core/src/main/java/bisq/core/trade/protocol/BuyerProtocol.java +++ b/core/src/main/java/bisq/core/trade/protocol/BuyerProtocol.java @@ -127,6 +127,8 @@ public abstract class BuyerProtocol extends DisputeProtocol { public void onPaymentStarted(ResultHandler resultHandler, ErrorMessageHandler errorMessageHandler) { System.out.println("BuyerProtocol.onPaymentStarted()"); synchronized (trade) { + latchTrade(); + this.errorMessageHandler = errorMessageHandler; BuyerEvent event = BuyerEvent.PAYMENT_SENT; expect(phase(Trade.Phase.DEPOSIT_UNLOCKED) .with(event) @@ -139,15 +141,16 @@ public abstract class BuyerProtocol extends DisputeProtocol { BuyerSendsPaymentSentMessage.class) // don't latch trade because this blocks and runs in background .using(new TradeTaskRunner(trade, () -> { + this.errorMessageHandler = null; resultHandler.handleResult(); handleTaskRunnerSuccess(event); }, (errorMessage) -> { - errorMessageHandler.handleErrorMessage(errorMessage); handleTaskRunnerFault(event, errorMessage); }))) .run(() -> trade.setState(Trade.State.BUYER_CONFIRMED_IN_UI_PAYMENT_SENT)) .executeTasks(); + awaitTradeLatch(); } } @@ -169,14 +172,11 @@ public abstract class BuyerProtocol extends DisputeProtocol { BuyerProcessesPaymentReceivedMessage.class) .using(new TradeTaskRunner(trade, () -> { - stopTimeout(); handleTaskRunnerSuccess(peer, message); }, errorMessage -> { - stopTimeout(); handleTaskRunnerFault(peer, message, errorMessage); - })) - .withTimeout(TRADE_TIMEOUT)) + }))) .executeTasks(true); awaitTradeLatch(); } diff --git a/core/src/main/java/bisq/core/trade/protocol/SellerProtocol.java b/core/src/main/java/bisq/core/trade/protocol/SellerProtocol.java index ab361513..d2b7fac1 100644 --- a/core/src/main/java/bisq/core/trade/protocol/SellerProtocol.java +++ b/core/src/main/java/bisq/core/trade/protocol/SellerProtocol.java @@ -125,6 +125,8 @@ public abstract class SellerProtocol extends DisputeProtocol { public void onPaymentReceived(ResultHandler resultHandler, ErrorMessageHandler errorMessageHandler) { log.info("SellerProtocol.onPaymentReceived()"); synchronized (trade) { + latchTrade(); + this.errorMessageHandler = errorMessageHandler; SellerEvent event = SellerEvent.PAYMENT_RECEIVED; expect(anyPhase(Trade.Phase.PAYMENT_SENT, Trade.Phase.PAYMENT_RECEIVED) .with(event) @@ -135,14 +137,15 @@ public abstract class SellerProtocol extends DisputeProtocol { SellerPreparesPaymentReceivedMessage.class, SellerSendsPaymentReceivedMessage.class) .using(new TradeTaskRunner(trade, () -> { + this.errorMessageHandler = null; resultHandler.handleResult(); handleTaskRunnerSuccess(event); }, (errorMessage) -> { - errorMessageHandler.handleErrorMessage(errorMessage); handleTaskRunnerFault(event, errorMessage); }))) .run(() -> trade.setState(Trade.State.SELLER_CONFIRMED_IN_UI_PAYMENT_RECEIPT)) .executeTasks(); + awaitTradeLatch(); } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java index 5ab2ac3e..cd3e4cb1 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java @@ -115,11 +115,7 @@ class GrpcTradesService extends TradesImplBase { @Override public void takeOffer(TakeOfferRequest req, StreamObserver responseObserver) { - GrpcErrorMessageHandler errorMessageHandler = - new GrpcErrorMessageHandler(getTakeOfferMethod().getFullMethodName(), - responseObserver, - exceptionHandler, - log); + GrpcErrorMessageHandler errorMessageHandler = new GrpcErrorMessageHandler(getTakeOfferMethod().getFullMethodName(), responseObserver, exceptionHandler, log); try { coreApi.takeOffer(req.getOfferId(), req.getPaymentAccountId(), @@ -144,12 +140,19 @@ class GrpcTradesService extends TradesImplBase { @Override public void confirmPaymentStarted(ConfirmPaymentStartedRequest req, StreamObserver responseObserver) { + GrpcErrorMessageHandler errorMessageHandler = new GrpcErrorMessageHandler(getConfirmPaymentStartedMethod().getFullMethodName(), responseObserver, exceptionHandler, log); try { - coreApi.confirmPaymentStarted(req.getTradeId()); - var reply = ConfirmPaymentStartedReply.newBuilder().build(); - responseObserver.onNext(reply); - responseObserver.onCompleted(); + coreApi.confirmPaymentStarted(req.getTradeId(), + () -> { + var reply = ConfirmPaymentStartedReply.newBuilder().build(); + responseObserver.onNext(reply); + responseObserver.onCompleted(); + }, + errorMessage -> { + if (!errorMessageHandler.isErrorHandled()) errorMessageHandler.handleErrorMessage(errorMessage); + }); } catch (Throwable cause) { + cause.printStackTrace(); exceptionHandler.handleException(log, cause, responseObserver); } } @@ -157,12 +160,19 @@ class GrpcTradesService extends TradesImplBase { @Override public void confirmPaymentReceived(ConfirmPaymentReceivedRequest req, StreamObserver responseObserver) { + GrpcErrorMessageHandler errorMessageHandler = new GrpcErrorMessageHandler(getConfirmPaymentReceivedMethod().getFullMethodName(), responseObserver, exceptionHandler, log); try { - coreApi.confirmPaymentReceived(req.getTradeId()); - var reply = ConfirmPaymentReceivedReply.newBuilder().build(); - responseObserver.onNext(reply); - responseObserver.onCompleted(); + coreApi.confirmPaymentReceived(req.getTradeId(), + () -> { + var reply = ConfirmPaymentReceivedReply.newBuilder().build(); + responseObserver.onNext(reply); + responseObserver.onCompleted(); + }, + errorMessage -> { + if (!errorMessageHandler.isErrorHandled()) errorMessageHandler.handleErrorMessage(errorMessage); + }); } catch (Throwable cause) { + cause.printStackTrace(); exceptionHandler.handleException(log, cause, responseObserver); } }