From a9aaba87fa7f8565ce516d4ca37aa90a3bc80525 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Wed, 5 Jan 2022 12:10:46 -0300 Subject: [PATCH 1/3] Rename API method keepfunds -> closetrade Trade proceeds and deposits have already been transfered to Bisq wallets before the `keepfunds` command is (was) executed; `keepfunds` merely moves open trades to closed trades lists and persistence files. Renaming `keepfunds` as `closetrade` makes its purpose clear to API users. The commit modifies only method names and comments in api server+cli classes, apitest cases, and api trade simulation scripts. Based on `master` --- apitest/scripts/trade-simulation-utils.sh | 16 +++++----- .../method/trade/TakeBuyBSQOfferTest.java | 6 ++-- .../method/trade/TakeBuyBTCOfferTest.java | 6 ++-- ...keBuyBTCOfferWithNationalBankAcctTest.java | 6 ++-- .../method/trade/TakeBuyXMROfferTest.java | 6 ++-- .../method/trade/TakeSellBSQOfferTest.java | 9 ++++-- .../method/trade/TakeSellBTCOfferTest.java | 2 +- .../method/trade/TakeSellXMROfferTest.java | 2 +- .../bisq/apitest/scenario/bot/BotClient.java | 6 ++-- .../scenario/bot/protocol/BotProtocol.java | 8 ++--- .../bot/protocol/MakerBotProtocol.java | 2 +- .../scenario/bot/protocol/ProtocolStep.java | 2 +- .../bot/protocol/TakerBotProtocol.java | 2 +- .../bot/script/BashScriptGenerator.java | 4 +-- cli/src/main/java/bisq/cli/CliMain.java | 10 +++--- cli/src/main/java/bisq/cli/GrpcClient.java | 4 +-- cli/src/main/java/bisq/cli/Method.java | 2 +- .../cli/request/TradesServiceRequest.java | 8 ++--- core/src/main/java/bisq/core/api/CoreApi.java | 4 +-- .../java/bisq/core/api/CoreTradesService.java | 4 +-- .../main/resources/help/closetrade-help.txt | 29 +++++++++++++++++ .../main/resources/help/keepfunds-help.txt | 31 ------------------- .../resources/help/withdrawfunds-help.txt | 14 ++++----- .../bisq/daemon/grpc/GrpcTradesService.java | 15 +++++---- proto/src/main/proto/grpc.proto | 6 ++-- 25 files changed, 102 insertions(+), 102 deletions(-) create mode 100644 core/src/main/resources/help/closetrade-help.txt delete mode 100644 core/src/main/resources/help/keepfunds-help.txt diff --git a/apitest/scripts/trade-simulation-utils.sh b/apitest/scripts/trade-simulation-utils.sh index 91e5f2bdd97..fee096fd17a 100755 --- a/apitest/scripts/trade-simulation-utils.sh +++ b/apitest/scripts/trade-simulation-utils.sh @@ -536,22 +536,22 @@ executetrade() { printbreak # Complete the trade on both sides - printdate "BOB $BOB_ROLE: Closing trade by keeping funds in Bisq wallet." - CMD="$CLI_BASE --port=$BOB_PORT keepfunds --trade-id=$OFFER_ID" + printdate "BOB $BOB_ROLE: Closing trade and keeping funds in Bisq wallet." + CMD="$CLI_BASE --port=$BOB_PORT closetrade --trade-id=$OFFER_ID" printdate "BOB CLI: $CMD" KEEP_FUNDS_MSG=$($CMD) - commandalert $? "Could close trade with keepfunds command." - # Print the keepfunds command's console output. + commandalert $? "Closed trade with closetrade command." + # Print the closetrade command's console output. printdate "$KEEP_FUNDS_MSG" sleeptraced 3 printbreak - printdate "ALICE (taker): Closing trade by keeping funds in Bisq wallet." - CMD="$CLI_BASE --port=$ALICE_PORT keepfunds --trade-id=$OFFER_ID" + printdate "ALICE (taker): Closing trade and keeping funds in Bisq wallet." + CMD="$CLI_BASE --port=$ALICE_PORT closetrade --trade-id=$OFFER_ID" printdate "ALICE CLI: $CMD" KEEP_FUNDS_MSG=$($CMD) - commandalert $? "Could close trade with keepfunds command." - # Print the keepfunds command's console output. + commandalert $? "Closed trade with closetrade command." + # Print the closetrade command's console output. printdate "$KEEP_FUNDS_MSG" sleeptraced 3 printbreak diff --git a/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBSQOfferTest.java b/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBSQOfferTest.java index 5d43b935778..6a37a3d1217 100644 --- a/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBSQOfferTest.java +++ b/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBSQOfferTest.java @@ -162,10 +162,10 @@ public void testKeepFunds(final TestInfo testInfo) { genBtcBlocksThenWait(1, 1_000); var trade = bobClient.getTrade(tradeId); - logTrade(log, testInfo, "Alice's view before keeping funds", trade); + logTrade(log, testInfo, "Alice's view before closing trade", trade); - aliceClient.keepFunds(tradeId); - bobClient.keepFunds(tradeId); + aliceClient.closeTrade(tradeId); + bobClient.closeTrade(tradeId); genBtcBlocksThenWait(1, 1_000); trade = bobClient.getTrade(tradeId); diff --git a/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBTCOfferTest.java b/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBTCOfferTest.java index b9c761d3561..26ac9d24666 100644 --- a/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBTCOfferTest.java +++ b/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBTCOfferTest.java @@ -137,9 +137,9 @@ public void testKeepFunds(final TestInfo testInfo) { try { genBtcBlocksThenWait(1, 1_000); var trade = aliceClient.getTrade(tradeId); - logTrade(log, testInfo, "Alice's view before keeping funds", trade); - aliceClient.keepFunds(tradeId); - bobClient.keepFunds(tradeId); + logTrade(log, testInfo, "Alice's view before closing trade and keeping funds", trade); + aliceClient.closeTrade(tradeId); + bobClient.closeTrade(tradeId); genBtcBlocksThenWait(1, 1_000); trade = aliceClient.getTrade(tradeId); EXPECTED_PROTOCOL_STATUS.setState(BUYER_RECEIVED_PAYOUT_TX_PUBLISHED_MSG).setPhase(PAYOUT_PUBLISHED); diff --git a/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBTCOfferWithNationalBankAcctTest.java b/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBTCOfferWithNationalBankAcctTest.java index 210d58192ec..e0e2ee746bd 100644 --- a/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBTCOfferWithNationalBankAcctTest.java +++ b/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBTCOfferWithNationalBankAcctTest.java @@ -213,9 +213,9 @@ public void testKeepFunds(final TestInfo testInfo) { genBtcBlocksThenWait(1, 1_000); var trade = aliceClient.getTrade(tradeId); - logTrade(log, testInfo, "Alice's view before keeping funds", trade); - aliceClient.keepFunds(tradeId); - bobClient.keepFunds(tradeId); + logTrade(log, testInfo, "Alice's view before closing trade and keeping funds", trade); + aliceClient.closeTrade(tradeId); + bobClient.closeTrade(tradeId); genBtcBlocksThenWait(1, 1_000); trade = aliceClient.getTrade(tradeId); EXPECTED_PROTOCOL_STATUS.setState(BUYER_RECEIVED_PAYOUT_TX_PUBLISHED_MSG) diff --git a/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyXMROfferTest.java b/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyXMROfferTest.java index 868d5c83b5c..d59bc02bc6c 100644 --- a/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyXMROfferTest.java +++ b/apitest/src/test/java/bisq/apitest/method/trade/TakeBuyXMROfferTest.java @@ -156,9 +156,9 @@ public void testKeepFunds(final TestInfo testInfo) { try { genBtcBlocksThenWait(1, 1_000); var trade = bobClient.getTrade(tradeId); - logTrade(log, testInfo, "Alice's view before keeping funds", trade); - aliceClient.keepFunds(tradeId); - bobClient.keepFunds(tradeId); + logTrade(log, testInfo, "Alice's view before closing trade and keeping funds", trade); + aliceClient.closeTrade(tradeId); + bobClient.closeTrade(tradeId); genBtcBlocksThenWait(1, 1_000); trade = bobClient.getTrade(tradeId); diff --git a/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBSQOfferTest.java b/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBSQOfferTest.java index d830a78e13f..f4411c51789 100644 --- a/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBSQOfferTest.java +++ b/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBSQOfferTest.java @@ -155,11 +155,14 @@ public void testAlicesBtcWithdrawalToExternalAddress(final TestInfo testInfo) { genBtcBlocksThenWait(1, 1_000); var trade = aliceClient.getTrade(tradeId); - logTrade(log, testInfo, "Alice's view before withdrawing BTC funds to external wallet", trade); + logTrade(log, + testInfo, + "Alice's view before closing trade and withdrawing BTC funds to external wallet", + trade); String toAddress = bitcoinCli.getNewBtcAddress(); aliceClient.withdrawFunds(tradeId, toAddress, WITHDRAWAL_TX_MEMO); - // Bob keeps funds. - bobClient.keepFunds(tradeId); + // Bob closes trade and keeps funds. + bobClient.closeTrade(tradeId); genBtcBlocksThenWait(1, 1_000); trade = aliceClient.getTrade(tradeId); EXPECTED_PROTOCOL_STATUS.setState(WITHDRAW_COMPLETED) diff --git a/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBTCOfferTest.java b/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBTCOfferTest.java index 8bd13fff096..9e205f5ef37 100644 --- a/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBTCOfferTest.java +++ b/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBTCOfferTest.java @@ -144,7 +144,7 @@ public void testBobsBtcWithdrawalToExternalAddress(final TestInfo testInfo) { logTrade(log, testInfo, "Bob's view before withdrawing funds to external wallet", trade); String toAddress = bitcoinCli.getNewBtcAddress(); bobClient.withdrawFunds(tradeId, toAddress, WITHDRAWAL_TX_MEMO); - aliceClient.keepFunds(tradeId); + aliceClient.closeTrade(tradeId); genBtcBlocksThenWait(1, 1_000); trade = bobClient.getTrade(tradeId); EXPECTED_PROTOCOL_STATUS.setState(WITHDRAW_COMPLETED) diff --git a/apitest/src/test/java/bisq/apitest/method/trade/TakeSellXMROfferTest.java b/apitest/src/test/java/bisq/apitest/method/trade/TakeSellXMROfferTest.java index 247b5320164..ec048da8923 100644 --- a/apitest/src/test/java/bisq/apitest/method/trade/TakeSellXMROfferTest.java +++ b/apitest/src/test/java/bisq/apitest/method/trade/TakeSellXMROfferTest.java @@ -165,7 +165,7 @@ public void testAlicesBtcWithdrawalToExternalAddress(final TestInfo testInfo) { String toAddress = bitcoinCli.getNewBtcAddress(); aliceClient.withdrawFunds(tradeId, toAddress, WITHDRAWAL_TX_MEMO); - bobClient.keepFunds(tradeId); + bobClient.closeTrade(tradeId); genBtcBlocksThenWait(1, 1_000); trade = aliceClient.getTrade(tradeId); diff --git a/apitest/src/test/java/bisq/apitest/scenario/bot/BotClient.java b/apitest/src/test/java/bisq/apitest/scenario/bot/BotClient.java index 062ee742b19..3ae53d7d55c 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/bot/BotClient.java +++ b/apitest/src/test/java/bisq/apitest/scenario/bot/BotClient.java @@ -282,12 +282,12 @@ public void sendConfirmPaymentReceivedMessage(String tradeId) { } /** - * Sends a 'keep funds in wallet message' for a trade with the given tradeId, + * Sends a 'closetrade' for a trade with the given tradeId, * or throws an exception. * @param tradeId */ - public void sendKeepFundsMessage(String tradeId) { - grpcClient.keepFunds(tradeId); + public void sendCloseTradeMessage(String tradeId) { + grpcClient.closeTrade(tradeId); } /** diff --git a/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/BotProtocol.java b/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/BotProtocol.java index 51d59e7537d..b090742a917 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/BotProtocol.java +++ b/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/BotProtocol.java @@ -219,8 +219,8 @@ protected void printBotProtocolStep() { } }; - protected final Function keepFundsFromTrade = (trade) -> { - initProtocolStep.accept(KEEP_FUNDS); + protected final Function closeTrade = (trade) -> { + initProtocolStep.accept(CLOSE_TRADE); var isBuy = trade.getOffer().getDirection().equalsIgnoreCase(BUY); var isSell = trade.getOffer().getDirection().equalsIgnoreCase(SELL); var cliUserIsSeller = (this instanceof MakerBotProtocol && isBuy) || (this instanceof TakerBotProtocol && isSell); @@ -229,8 +229,8 @@ protected void printBotProtocolStep() { } else { createGetBalanceScript(); } - checkIfShutdownCalled("Interrupted before closing trade with 'keep funds' command."); - this.getBotClient().sendKeepFundsMessage(trade.getTradeId()); + checkIfShutdownCalled("Interrupted before closing trade with 'closetrade' command."); + this.getBotClient().sendCloseTradeMessage(trade.getTradeId()); return trade; }; diff --git a/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/MakerBotProtocol.java b/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/MakerBotProtocol.java index e251150cf16..3f757dbbd28 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/MakerBotProtocol.java +++ b/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/MakerBotProtocol.java @@ -56,7 +56,7 @@ public void run() { : waitForPaymentStartedMessage.andThen(sendPaymentReceivedMessage); completeFiatTransaction.apply(trade); - Function closeTrade = waitForPayoutTx.andThen(keepFundsFromTrade); + Function closeTrade = waitForPayoutTx.andThen(this.closeTrade); closeTrade.apply(trade); currentProtocolStep = DONE; diff --git a/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/ProtocolStep.java b/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/ProtocolStep.java index def2a0bb663..2c8c8cd07f7 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/ProtocolStep.java +++ b/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/ProtocolStep.java @@ -12,6 +12,6 @@ public enum ProtocolStep { SEND_PAYMENT_RECEIVED_CONFIRMATION_MESSAGE, WAIT_FOR_PAYMENT_RECEIVED_CONFIRMATION_MESSAGE, WAIT_FOR_PAYOUT_TX, - KEEP_FUNDS, + CLOSE_TRADE, DONE } diff --git a/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/TakerBotProtocol.java b/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/TakerBotProtocol.java index 22964d0f160..dd04b5c5bff 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/TakerBotProtocol.java +++ b/apitest/src/test/java/bisq/apitest/scenario/bot/protocol/TakerBotProtocol.java @@ -55,7 +55,7 @@ public void run() { : sendPaymentStartedMessage.andThen(waitForPaymentReceivedConfirmation); completeFiatTransaction.apply(trade); - Function closeTrade = waitForPayoutTx.andThen(keepFundsFromTrade); + Function closeTrade = waitForPayoutTx.andThen(this.closeTrade); closeTrade.apply(trade); currentProtocolStep = DONE; diff --git a/apitest/src/test/java/bisq/apitest/scenario/bot/script/BashScriptGenerator.java b/apitest/src/test/java/bisq/apitest/scenario/bot/script/BashScriptGenerator.java index d41e8a1acd3..4c6b73f8084 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/bot/script/BashScriptGenerator.java +++ b/apitest/src/test/java/bisq/apitest/scenario/bot/script/BashScriptGenerator.java @@ -167,10 +167,10 @@ public File createPaymentReceivedScript(TradeInfo trade) { } public File createKeepFundsScript(TradeInfo trade) { - String paymentStartedCmd = format("%s keepfunds --trade-id=%s", cliBase, trade.getTradeId()); + String paymentStartedCmd = format("%s closetrade --trade-id=%s", cliBase, trade.getTradeId()); String getTradeCmd = format("%s gettrade --trade-id=%s", cliBase, trade.getTradeId()); String getBalanceCmd = format("%s getbalance", cliBase); - return createCliScript("keepfunds.sh", + return createCliScript("closetrade.sh", paymentStartedCmd, "sleep 2", getTradeCmd, diff --git a/cli/src/main/java/bisq/cli/CliMain.java b/cli/src/main/java/bisq/cli/CliMain.java index 1170774e3e7..b0bd24b84c8 100644 --- a/cli/src/main/java/bisq/cli/CliMain.java +++ b/cli/src/main/java/bisq/cli/CliMain.java @@ -525,15 +525,15 @@ public static void run(String[] args) { out.printf("trade %s payment received message sent%n", tradeId); return; } - case keepfunds: { + case closetrade: { var opts = new GetTradeOptionParser(args).parse(); if (opts.isForHelp()) { out.println(client.getMethodHelp(method)); return; } var tradeId = opts.getTradeId(); - client.keepFunds(tradeId); - out.printf("funds from trade %s saved in bisq wallet%n", tradeId); + client.closeTrade(tradeId); + out.printf("trade %s is closed%n", tradeId); return; } case withdrawfunds: { @@ -864,10 +864,10 @@ private static void printHelp(OptionParser parser, @SuppressWarnings("SameParame stream.println(); stream.format(rowFormat, confirmpaymentreceived.name(), "--trade-id=", "Confirm payment received"); stream.println(); - stream.format(rowFormat, keepfunds.name(), "--trade-id=", "Keep received funds in Bisq wallet"); + stream.format(rowFormat, closetrade.name(), "--trade-id=", "Close completed trade"); stream.println(); stream.format(rowFormat, withdrawfunds.name(), "--trade-id= --address= \\", - "Withdraw received funds to external wallet address"); + "Withdraw received trade funds to external wallet address"); stream.format(rowFormat, "", "[--memo=<\"memo\">]", ""); stream.println(); stream.format(rowFormat, getpaymentmethods.name(), "", "Get list of supported payment account method ids"); diff --git a/cli/src/main/java/bisq/cli/GrpcClient.java b/cli/src/main/java/bisq/cli/GrpcClient.java index 99af68ca30d..57312abc034 100644 --- a/cli/src/main/java/bisq/cli/GrpcClient.java +++ b/cli/src/main/java/bisq/cli/GrpcClient.java @@ -372,8 +372,8 @@ public void confirmPaymentReceived(String tradeId) { tradesServiceRequest.confirmPaymentReceived(tradeId); } - public void keepFunds(String tradeId) { - tradesServiceRequest.keepFunds(tradeId); + public void closeTrade(String tradeId) { + tradesServiceRequest.closeTrade(tradeId); } public void withdrawFunds(String tradeId, String address, String memo) { diff --git a/cli/src/main/java/bisq/cli/Method.java b/cli/src/main/java/bisq/cli/Method.java index 273150aad5d..3e097238a86 100644 --- a/cli/src/main/java/bisq/cli/Method.java +++ b/cli/src/main/java/bisq/cli/Method.java @@ -22,6 +22,7 @@ */ public enum Method { canceloffer, + closetrade, confirmpaymentreceived, confirmpaymentstarted, createoffer, @@ -45,7 +46,6 @@ public enum Method { gettxfeerate, getunusedbsqaddress, getversion, - keepfunds, lockwallet, registerdisputeagent, removewalletpassword, diff --git a/cli/src/main/java/bisq/cli/request/TradesServiceRequest.java b/cli/src/main/java/bisq/cli/request/TradesServiceRequest.java index cf95506615e..080be70e026 100644 --- a/cli/src/main/java/bisq/cli/request/TradesServiceRequest.java +++ b/cli/src/main/java/bisq/cli/request/TradesServiceRequest.java @@ -17,10 +17,10 @@ package bisq.cli.request; +import bisq.proto.grpc.CloseTradeRequest; import bisq.proto.grpc.ConfirmPaymentReceivedRequest; import bisq.proto.grpc.ConfirmPaymentStartedRequest; import bisq.proto.grpc.GetTradeRequest; -import bisq.proto.grpc.KeepFundsRequest; import bisq.proto.grpc.TakeOfferReply; import bisq.proto.grpc.TakeOfferRequest; import bisq.proto.grpc.TradeInfo; @@ -86,12 +86,12 @@ public void confirmPaymentReceived(String tradeId) { grpcStubs.tradesService.confirmPaymentReceived(request); } - public void keepFunds(String tradeId) { - var request = KeepFundsRequest.newBuilder() + public void closeTrade(String tradeId) { + var request = CloseTradeRequest.newBuilder() .setTradeId(tradeId) .build(); //noinspection ResultOfMethodCallIgnored - grpcStubs.tradesService.keepFunds(request); + grpcStubs.tradesService.closeTrade(request); } public void withdrawFunds(String tradeId, String address, String memo) { diff --git a/core/src/main/java/bisq/core/api/CoreApi.java b/core/src/main/java/bisq/core/api/CoreApi.java index 9808d2e7d9f..2794db47bbb 100644 --- a/core/src/main/java/bisq/core/api/CoreApi.java +++ b/core/src/main/java/bisq/core/api/CoreApi.java @@ -315,8 +315,8 @@ public void confirmPaymentReceived(String tradeId) { coreTradesService.confirmPaymentReceived(tradeId); } - public void keepFunds(String tradeId) { - coreTradesService.keepFunds(tradeId); + public void closeTrade(String tradeId) { + coreTradesService.closeTrade(tradeId); } public void withdrawFunds(String tradeId, String address, String memo) { diff --git a/core/src/main/java/bisq/core/api/CoreTradesService.java b/core/src/main/java/bisq/core/api/CoreTradesService.java index 3184ff31477..f28e2f36cc2 100644 --- a/core/src/main/java/bisq/core/api/CoreTradesService.java +++ b/core/src/main/java/bisq/core/api/CoreTradesService.java @@ -180,14 +180,14 @@ void confirmPaymentReceived(String tradeId) { } } - void keepFunds(String tradeId) { + void closeTrade(String tradeId) { coreWalletsService.verifyWalletsAreAvailable(); coreWalletsService.verifyEncryptedWalletIsUnlocked(); verifyTradeIsNotClosed(tradeId); var trade = getOpenTrade(tradeId).orElseThrow(() -> new IllegalArgumentException(format("trade with id '%s' not found", tradeId))); - log.info("Keeping funds received from trade {}", tradeId); + log.info("Closing trade {}", tradeId); tradeManager.onTradeCompleted(trade); } diff --git a/core/src/main/resources/help/closetrade-help.txt b/core/src/main/resources/help/closetrade-help.txt new file mode 100644 index 00000000000..a26d4310b2c --- /dev/null +++ b/core/src/main/resources/help/closetrade-help.txt @@ -0,0 +1,29 @@ +closetrade + +NAME +---- +closetrade - close open trade + +SYNOPSIS +-------- +closetrade + --trade-id= + +DESCRIPTION +----------- +This final step in the trade protocol moves a completed trade from the open trades list +to the closed trade list, where it becomes part of the user's trade history. +The step is necessary for correct transition of a trade's state to CLOSED. + +The alternative way to close a trade is to send the received BTC to an external +BTC wallet, using the withdrawfunds command. + +OPTIONS +------- +--trade-id + The ID of the completed trade (the full offer-id). + +EXAMPLES +-------- +The BTC buyer or seller closes out trade 83e8b2e2-51b6-4f39-a748-3ebd29c22aea: +$ ./bisq-cli --password=xyz --port=9998 closetrade --trade-id=83e8b2e2-51b6-4f39-a748-3ebd29c22aea diff --git a/core/src/main/resources/help/keepfunds-help.txt b/core/src/main/resources/help/keepfunds-help.txt deleted file mode 100644 index 4e8232df043..00000000000 --- a/core/src/main/resources/help/keepfunds-help.txt +++ /dev/null @@ -1,31 +0,0 @@ -keepfunds - -NAME ----- -keepfunds - keep BTC received during a trade in Bisq wallet - -SYNOPSIS --------- -keepfunds - --trade-id= - -DESCRIPTION ------------ -A BTC buyer completes the final step in the trade protocol by keeping received BTC in his -Bisq wallet. This step may not seem necessary from the buyer's perspective, but it is -necessary for correct transition of a trade's state to CLOSED, within the Bisq server. - -The alternative way to close out the trade is to send the received BTC to an external -BTC wallet, using the withdrawfunds command. - -OPTIONS -------- ---trade-id - The ID of the trade (the full offer-id). - -EXAMPLES --------- -A BTC seller has informed the buyer that fiat payment has been received for trade with ID -83e8b2e2-51b6-4f39-a748-3ebd29c22aea, and locked BTC has been released to the buyer. -The BTC buyer closes out the trade by keeping the received BTC in her Bisq wallet: -$ ./bisq-cli --password=xyz --port=9998 keepfunds --trade-id=83e8b2e2-51b6-4f39-a748-3ebd29c22aea diff --git a/core/src/main/resources/help/withdrawfunds-help.txt b/core/src/main/resources/help/withdrawfunds-help.txt index edc69dddcb8..cdee6362f4a 100644 --- a/core/src/main/resources/help/withdrawfunds-help.txt +++ b/core/src/main/resources/help/withdrawfunds-help.txt @@ -2,7 +2,7 @@ withdrawfunds NAME ---- -withdrawfunds - send BTC received during a trade to an external BTC wallet +withdrawfunds - send BTC received during a trade to an external BTC wallet and close trade SYNOPSIS -------- @@ -16,10 +16,10 @@ DESCRIPTION A BTC buyer completes the final step in the trade protocol by sending received BTC to an external BTC wallet. -The alternative way to close out the trade is to keep the received BTC in the Bisq wallet, -using the keepfunds command. +The alternative way to close out the trade is to keep the received BTC in the Bisq wallet +with the closetrade command. -The buyer needs to complete the trade protocol using the keepfunds or withdrawfunds or command. +The buyer needs to complete the trade protocol using the closetrade or withdrawfunds command. This step may not seem necessary from the buyer's perspective, but it is necessary for correct transition of a trade's state to CLOSED, within the Bisq server. @@ -29,11 +29,11 @@ OPTIONS The ID of the trade (the full offer-id). --address - The destination btc address for the send btc transaction. + The destination btc address for the btc transaction. --memo - An optional memo to be saved with the send btc transaction. - A multi word memo must be enclosed in double quotes. + An optional memo to be saved with the btc transaction. + A multi-word memo must be enclosed in double quotes. EXAMPLES -------- diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java index 4d2605b109a..38d03382552 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java @@ -23,14 +23,14 @@ import bisq.core.trade.model.bisq_v1.Trade; import bisq.core.trade.model.bsq_swap.BsqSwapTrade; +import bisq.proto.grpc.CloseTradeReply; +import bisq.proto.grpc.CloseTradeRequest; import bisq.proto.grpc.ConfirmPaymentReceivedReply; import bisq.proto.grpc.ConfirmPaymentReceivedRequest; import bisq.proto.grpc.ConfirmPaymentStartedReply; import bisq.proto.grpc.ConfirmPaymentStartedRequest; import bisq.proto.grpc.GetTradeReply; import bisq.proto.grpc.GetTradeRequest; -import bisq.proto.grpc.KeepFundsReply; -import bisq.proto.grpc.KeepFundsRequest; import bisq.proto.grpc.TakeOfferReply; import bisq.proto.grpc.TakeOfferRequest; import bisq.proto.grpc.WithdrawFundsReply; @@ -152,11 +152,11 @@ public void confirmPaymentReceived(ConfirmPaymentReceivedRequest req, } @Override - public void keepFunds(KeepFundsRequest req, - StreamObserver responseObserver) { + public void closeTrade(CloseTradeRequest req, + StreamObserver responseObserver) { try { - coreApi.keepFunds(req.getTradeId()); - var reply = KeepFundsReply.newBuilder().build(); + coreApi.closeTrade(req.getTradeId()); + var reply = CloseTradeReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); } catch (Throwable cause) { @@ -189,10 +189,9 @@ final Optional rateMeteringInterceptor() { new HashMap<>() {{ put(getGetTradeMethod().getFullMethodName(), new GrpcCallRateMeter(1, SECONDS)); put(getTakeOfferMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); - // put(getTakeBsqSwapOfferMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); put(getConfirmPaymentStartedMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); put(getConfirmPaymentReceivedMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); - put(getKeepFundsMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); + put(getCloseTradeMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); put(getWithdrawFundsMethod().getFullMethodName(), new GrpcCallRateMeter(1, MINUTES)); }} ))); diff --git a/proto/src/main/proto/grpc.proto b/proto/src/main/proto/grpc.proto index 646ab0a85c8..1af7e69ab01 100644 --- a/proto/src/main/proto/grpc.proto +++ b/proto/src/main/proto/grpc.proto @@ -394,7 +394,7 @@ service Trades { } rpc ConfirmPaymentReceived (ConfirmPaymentReceivedRequest) returns (ConfirmPaymentReceivedReply) { } - rpc KeepFunds (KeepFundsRequest) returns (KeepFundsReply) { + rpc CloseTrade (CloseTradeRequest) returns (CloseTradeReply) { } rpc WithdrawFunds (WithdrawFundsRequest) returns (WithdrawFundsReply) { } @@ -433,11 +433,11 @@ message GetTradeReply { TradeInfo trade = 1; } -message KeepFundsRequest { +message CloseTradeRequest { string tradeId = 1; } -message KeepFundsReply { +message CloseTradeReply { } message WithdrawFundsRequest { From 905841b2b2cbfbdbfb049a6e4442f001213917b1 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Wed, 5 Jan 2022 12:19:36 -0300 Subject: [PATCH 2/3] Adjust api beta test guide to new closetrade cmd --- apitest/docs/api-beta-test-guide.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/apitest/docs/api-beta-test-guide.md b/apitest/docs/api-beta-test-guide.md index 9929f4df75c..3b6396fcf61 100644 --- a/apitest/docs/api-beta-test-guide.md +++ b/apitest/docs/api-beta-test-guide.md @@ -637,19 +637,19 @@ protocol completed. There are three CLI commands that must be performed in coor ``` confirmpaymentstarted Buyer sends seller a message confirming payment has been sent. confirmpaymentreceived Seller sends buyer a message confirming payment has been received. -keepfunds Keep trade proceeds in their Bisq wallets. +closetrade Set trade state to CLOSED, and keep trade proceeds in user's Bisq wallet. OR -withdrawfunds Send trade proceeds to an external wallet. +withdrawfunds Set trade state to CLOSED, and send trade proceeds to an external wallet. ``` -The last two mutually exclusive commands (`keepfunds` or `withdrawfunds`) may seem unnecessary, but they are critical -because they inform the Bisq node that a trade’s state can be set to `CLOSED`. Please close out your trades with one +The last two mutually exclusive commands (`closetrade` or `withdrawfunds`) may seem unnecessary, but they are critical +because they tell the Bisq node to set a completed trade’s state `CLOSED`. Please close out your trades with one or the other command. Each of the CLI commands above takes one argument: `--trade-id=`: ``` $ ./bisq-cli --password=xyz --port=9998 confirmpaymentstarted --trade-id= $ ./bisq-cli --password=xyz --port=9999 confirmpaymentreceived --trade-id= -$ ./bisq-cli --password=xyz --port=9998 keepfunds --trade-id= +$ ./bisq-cli --password=xyz --port=9998 closetrade --trade-id= $ ./bisq-cli --password=xyz --port=9999 withdrawfunds --trade-id= --address= [--memo=<"memo">] ``` From 7690ffd9530b15f1d5dce91e9b674b09111def19 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 7 Jan 2022 19:11:34 -0300 Subject: [PATCH 3/3] Add API methods 'failtrade', 'unfailtrade' Prerequisite for next PR: Add API method 'gettrades' The `gettrades` method will show 'open', 'closed', and 'failed' trades. Users already needed to be able to fail and unfail trades for the same reasons they do in the UI. API test cases will need to be able to fail and unfail trades to check correct behavior of 'gettrades' method. Based on branch `rename-keepfunds2closetrade`. --- .../method/trade/AbstractTradeTest.java | 3 + .../method/trade/FailUnfailTradeTest.java | 143 ++++++++++++++++++ .../java/bisq/apitest/scenario/TradeTest.java | 11 ++ cli/src/main/java/bisq/cli/CliMain.java | 26 ++++ cli/src/main/java/bisq/cli/GrpcClient.java | 8 + cli/src/main/java/bisq/cli/Method.java | 2 + .../cli/request/TradesServiceRequest.java | 18 +++ core/src/main/java/bisq/core/api/CoreApi.java | 8 + .../java/bisq/core/api/CoreTradesService.java | 61 ++++++++ .../trade/bisq_v1/FailedTradesManager.java | 74 +++++++-- .../main/resources/help/failtrade-help.txt | 28 ++++ .../main/resources/help/unfailtrade-help.txt | 32 ++++ .../bisq/daemon/grpc/GrpcTradesService.java | 30 ++++ proto/src/main/proto/grpc.proto | 18 +++ 14 files changed, 452 insertions(+), 10 deletions(-) create mode 100644 apitest/src/test/java/bisq/apitest/method/trade/FailUnfailTradeTest.java create mode 100644 core/src/main/resources/help/failtrade-help.txt create mode 100644 core/src/main/resources/help/unfailtrade-help.txt diff --git a/apitest/src/test/java/bisq/apitest/method/trade/AbstractTradeTest.java b/apitest/src/test/java/bisq/apitest/method/trade/AbstractTradeTest.java index 625bcc0ab18..2c1fb29d320 100644 --- a/apitest/src/test/java/bisq/apitest/method/trade/AbstractTradeTest.java +++ b/apitest/src/test/java/bisq/apitest/method/trade/AbstractTradeTest.java @@ -8,6 +8,8 @@ import org.slf4j.Logger; +import lombok.Getter; + import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.TestInfo; @@ -36,6 +38,7 @@ public class AbstractTradeTest extends AbstractOfferTest { public static final ExpectedProtocolStatus EXPECTED_PROTOCOL_STATUS = new ExpectedProtocolStatus(); // A Trade ID cache for use in @Test sequences. + @Getter protected static String tradeId; protected final Supplier maxTradeStateAndPhaseChecks = () -> isLongRunningTest ? 10 : 2; diff --git a/apitest/src/test/java/bisq/apitest/method/trade/FailUnfailTradeTest.java b/apitest/src/test/java/bisq/apitest/method/trade/FailUnfailTradeTest.java new file mode 100644 index 00000000000..5985ea81285 --- /dev/null +++ b/apitest/src/test/java/bisq/apitest/method/trade/FailUnfailTradeTest.java @@ -0,0 +1,143 @@ +/* + * This file is part of Bisq. + * + * Bisq is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or (at + * your option) any later version. + * + * Bisq is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public + * License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with Bisq. If not, see . + */ + +package bisq.apitest.method.trade; + +import io.grpc.StatusRuntimeException; + +import lombok.extern.slf4j.Slf4j; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.TestMethodOrder; + +import static java.lang.String.format; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; + + + +import bisq.apitest.method.offer.AbstractOfferTest; + +@Disabled +@Slf4j +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) +public class FailUnfailTradeTest extends AbstractTradeTest { + + @BeforeAll + public static void setUp() { + AbstractOfferTest.setUp(); + } + + @BeforeEach + public void init() { + EXPECTED_PROTOCOL_STATUS.init(); + } + + + @Test + @Order(1) + public void testFailAndUnFailBuyBTCTrade(final TestInfo testInfo) { + TakeBuyBTCOfferTest test = new TakeBuyBTCOfferTest(); + test.testTakeAlicesBuyOffer(testInfo); + + var tradeId = test.getTradeId(); + aliceClient.failTrade(tradeId); + + Throwable exception = assertThrows(StatusRuntimeException.class, () -> aliceClient.getTrade(tradeId)); + String expectedExceptionMessage = format("INVALID_ARGUMENT: trade with id '%s' not found", tradeId); + assertEquals(expectedExceptionMessage, exception.getMessage()); + + try { + aliceClient.unFailTrade(tradeId); + aliceClient.getTrade(tradeId); //Throws ex if trade is still failed. + } catch (Exception ex) { + fail(ex); + } + } + + @Test + @Order(2) + public void testFailAndUnFailSellBTCTrade(final TestInfo testInfo) { + TakeSellBTCOfferTest test = new TakeSellBTCOfferTest(); + test.testTakeAlicesSellOffer(testInfo); + + var tradeId = test.getTradeId(); + aliceClient.failTrade(tradeId); + + Throwable exception = assertThrows(StatusRuntimeException.class, () -> aliceClient.getTrade(tradeId)); + String expectedExceptionMessage = format("INVALID_ARGUMENT: trade with id '%s' not found", tradeId); + assertEquals(expectedExceptionMessage, exception.getMessage()); + + try { + aliceClient.unFailTrade(tradeId); + aliceClient.getTrade(tradeId); //Throws ex if trade is still failed. + } catch (Exception ex) { + fail(ex); + } + } + + @Test + @Order(3) + public void testFailAndUnFailBuyXmrTrade(final TestInfo testInfo) { + TakeBuyXMROfferTest test = new TakeBuyXMROfferTest(); + test.createXmrPaymentAccounts(); + test.testTakeAlicesSellBTCForXMROffer(testInfo); + + var tradeId = test.getTradeId(); + aliceClient.failTrade(tradeId); + + Throwable exception = assertThrows(StatusRuntimeException.class, () -> aliceClient.getTrade(tradeId)); + String expectedExceptionMessage = format("INVALID_ARGUMENT: trade with id '%s' not found", tradeId); + assertEquals(expectedExceptionMessage, exception.getMessage()); + + try { + aliceClient.unFailTrade(tradeId); + aliceClient.getTrade(tradeId); //Throws ex if trade is still failed. + } catch (Exception ex) { + fail(ex); + } + } + + @Test + @Order(4) + public void testFailAndUnFailTakeSellXMRTrade(final TestInfo testInfo) { + TakeSellXMROfferTest test = new TakeSellXMROfferTest(); + test.createXmrPaymentAccounts(); + test.testTakeAlicesBuyBTCForXMROffer(testInfo); + + var tradeId = test.getTradeId(); + aliceClient.failTrade(tradeId); + + Throwable exception = assertThrows(StatusRuntimeException.class, () -> aliceClient.getTrade(tradeId)); + String expectedExceptionMessage = format("INVALID_ARGUMENT: trade with id '%s' not found", tradeId); + assertEquals(expectedExceptionMessage, exception.getMessage()); + + try { + aliceClient.unFailTrade(tradeId); + aliceClient.getTrade(tradeId); //Throws ex if trade is still failed. + } catch (Exception ex) { + fail(ex); + } + } +} diff --git a/apitest/src/test/java/bisq/apitest/scenario/TradeTest.java b/apitest/src/test/java/bisq/apitest/scenario/TradeTest.java index 69d5ba6dd80..6e91705b4d8 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/TradeTest.java +++ b/apitest/src/test/java/bisq/apitest/scenario/TradeTest.java @@ -30,6 +30,7 @@ import bisq.apitest.method.trade.AbstractTradeTest; import bisq.apitest.method.trade.BsqSwapTradeTest; +import bisq.apitest.method.trade.FailUnfailTradeTest; import bisq.apitest.method.trade.TakeBuyBSQOfferTest; import bisq.apitest.method.trade.TakeBuyBTCOfferTest; import bisq.apitest.method.trade.TakeBuyBTCOfferWithNationalBankAcctTest; @@ -130,4 +131,14 @@ public void testBsqSwapTradeTest(final TestInfo testInfo) { test.testBobTakesBsqSwapOffer(); test.testGetBalancesAfterTrade(); } + + @Test + @Order(9) + public void testFailUnfailTrade(final TestInfo testInfo) { + FailUnfailTradeTest test = new FailUnfailTradeTest(); + test.testFailAndUnFailBuyBTCTrade(testInfo); + test.testFailAndUnFailSellBTCTrade(testInfo); + test.testFailAndUnFailBuyXmrTrade(testInfo); + test.testFailAndUnFailTakeSellXMRTrade(testInfo); + } } diff --git a/cli/src/main/java/bisq/cli/CliMain.java b/cli/src/main/java/bisq/cli/CliMain.java index b0bd24b84c8..e42619df6b8 100644 --- a/cli/src/main/java/bisq/cli/CliMain.java +++ b/cli/src/main/java/bisq/cli/CliMain.java @@ -559,6 +559,28 @@ public static void run(String[] args) { paymentMethods.forEach(p -> out.println(p.getId())); return; } + case failtrade: { + var opts = new GetTradeOptionParser(args).parse(); + if (opts.isForHelp()) { + out.println(client.getMethodHelp(method)); + return; + } + var tradeId = opts.getTradeId(); + client.failTrade(tradeId); + out.printf("open trade %s changed to failed trade%n", tradeId); + return; + } + case unfailtrade: { + var opts = new GetTradeOptionParser(args).parse(); + if (opts.isForHelp()) { + out.println(client.getMethodHelp(method)); + return; + } + var tradeId = opts.getTradeId(); + client.unFailTrade(tradeId); + out.printf("failed trade %s changed to open trade%n", tradeId); + return; + } case getpaymentacctform: { var opts = new GetPaymentAcctFormOptionParser(args).parse(); if (opts.isForHelp()) { @@ -870,6 +892,10 @@ private static void printHelp(OptionParser parser, @SuppressWarnings("SameParame "Withdraw received trade funds to external wallet address"); stream.format(rowFormat, "", "[--memo=<\"memo\">]", ""); stream.println(); + stream.format(rowFormat, failtrade.name(), "--trade-id=", "Change open trade to failed trade"); + stream.println(); + stream.format(rowFormat, unfailtrade.name(), "--trade-id=", "Change failed trade to open trade"); + stream.println(); stream.format(rowFormat, getpaymentmethods.name(), "", "Get list of supported payment account method ids"); stream.println(); stream.format(rowFormat, getpaymentacctform.name(), "--payment-method-id=", "Get a new payment account form"); diff --git a/cli/src/main/java/bisq/cli/GrpcClient.java b/cli/src/main/java/bisq/cli/GrpcClient.java index 57312abc034..1d93187353d 100644 --- a/cli/src/main/java/bisq/cli/GrpcClient.java +++ b/cli/src/main/java/bisq/cli/GrpcClient.java @@ -380,6 +380,14 @@ public void withdrawFunds(String tradeId, String address, String memo) { tradesServiceRequest.withdrawFunds(tradeId, address, memo); } + public void failTrade(String tradeId) { + tradesServiceRequest.failTrade(tradeId); + } + + public void unFailTrade(String tradeId) { + tradesServiceRequest.unFailTrade(tradeId); + } + public List getPaymentMethods() { return paymentAccountsServiceRequest.getPaymentMethods(); } diff --git a/cli/src/main/java/bisq/cli/Method.java b/cli/src/main/java/bisq/cli/Method.java index 3e097238a86..55207203846 100644 --- a/cli/src/main/java/bisq/cli/Method.java +++ b/cli/src/main/java/bisq/cli/Method.java @@ -42,6 +42,8 @@ public enum Method { getpaymentaccts, getpaymentmethods, gettrade, + failtrade, + unfailtrade, gettransaction, gettxfeerate, getunusedbsqaddress, diff --git a/cli/src/main/java/bisq/cli/request/TradesServiceRequest.java b/cli/src/main/java/bisq/cli/request/TradesServiceRequest.java index 080be70e026..69b535a77e0 100644 --- a/cli/src/main/java/bisq/cli/request/TradesServiceRequest.java +++ b/cli/src/main/java/bisq/cli/request/TradesServiceRequest.java @@ -20,10 +20,12 @@ import bisq.proto.grpc.CloseTradeRequest; import bisq.proto.grpc.ConfirmPaymentReceivedRequest; import bisq.proto.grpc.ConfirmPaymentStartedRequest; +import bisq.proto.grpc.FailTradeRequest; import bisq.proto.grpc.GetTradeRequest; import bisq.proto.grpc.TakeOfferReply; import bisq.proto.grpc.TakeOfferRequest; import bisq.proto.grpc.TradeInfo; +import bisq.proto.grpc.UnFailTradeRequest; import bisq.proto.grpc.WithdrawFundsRequest; @@ -103,4 +105,20 @@ public void withdrawFunds(String tradeId, String address, String memo) { //noinspection ResultOfMethodCallIgnored grpcStubs.tradesService.withdrawFunds(request); } + + public void failTrade(String tradeId) { + var request = FailTradeRequest.newBuilder() + .setTradeId(tradeId) + .build(); + //noinspection ResultOfMethodCallIgnored + grpcStubs.tradesService.failTrade(request); + } + + public void unFailTrade(String tradeId) { + var request = UnFailTradeRequest.newBuilder() + .setTradeId(tradeId) + .build(); + //noinspection ResultOfMethodCallIgnored + grpcStubs.tradesService.unFailTrade(request); + } } diff --git a/core/src/main/java/bisq/core/api/CoreApi.java b/core/src/main/java/bisq/core/api/CoreApi.java index 2794db47bbb..57c9a07a503 100644 --- a/core/src/main/java/bisq/core/api/CoreApi.java +++ b/core/src/main/java/bisq/core/api/CoreApi.java @@ -335,6 +335,14 @@ public String getBsqSwapTradeRole(BsqSwapTrade bsqSwapTrade) { return coreTradesService.getBsqSwapTradeRole(bsqSwapTrade); } + public void failTrade(String tradeId) { + coreTradesService.failTrade(tradeId); + } + + public void unFailTrade(String tradeId) { + coreTradesService.unFailTrade(tradeId); + } + /////////////////////////////////////////////////////////////////////////////////////////// // Wallets /////////////////////////////////////////////////////////////////////////////////////////// diff --git a/core/src/main/java/bisq/core/api/CoreTradesService.java b/core/src/main/java/bisq/core/api/CoreTradesService.java index f28e2f36cc2..d28ce99c736 100644 --- a/core/src/main/java/bisq/core/api/CoreTradesService.java +++ b/core/src/main/java/bisq/core/api/CoreTradesService.java @@ -25,6 +25,7 @@ import bisq.core.offer.bsq_swap.BsqSwapTakeOfferModel; import bisq.core.trade.ClosedTradableManager; import bisq.core.trade.TradeManager; +import bisq.core.trade.bisq_v1.FailedTradesManager; import bisq.core.trade.bisq_v1.TradeResultHandler; import bisq.core.trade.bisq_v1.TradeUtil; import bisq.core.trade.model.Tradable; @@ -63,6 +64,7 @@ class CoreTradesService { private final BtcWalletService btcWalletService; private final OfferUtil offerUtil; private final ClosedTradableManager closedTradableManager; + private final FailedTradesManager failedTradesManager; private final TakeOfferModel takeOfferModel; private final BsqSwapTakeOfferModel bsqSwapTakeOfferModel; private final TradeManager tradeManager; @@ -75,6 +77,7 @@ public CoreTradesService(CoreContext coreContext, BtcWalletService btcWalletService, OfferUtil offerUtil, ClosedTradableManager closedTradableManager, + FailedTradesManager failedTradesManager, TakeOfferModel takeOfferModel, BsqSwapTakeOfferModel bsqSwapTakeOfferModel, TradeManager tradeManager, @@ -85,6 +88,7 @@ public CoreTradesService(CoreContext coreContext, this.btcWalletService = btcWalletService; this.offerUtil = offerUtil; this.closedTradableManager = closedTradableManager; + this.failedTradesManager = failedTradesManager; this.takeOfferModel = takeOfferModel; this.bsqSwapTakeOfferModel = bsqSwapTakeOfferModel; this.tradeManager = tradeManager; @@ -269,6 +273,37 @@ Trade getTrade(String tradeId) { )); } + void failTrade(String tradeId) { + // TODO Recommend that API users should use this method with extra care because + // the API lacks methods for diagnosing trade problems, and does not support + // interaction with mediators. Users may accidentally fail valid trades, + // although they can easily be un-failed with the 'unfailtrade' method. + // The 'failtrade' and 'unfailtrade' methods are implemented at this early + // stage of API development to help efficiently test a new + // 'gettrades --category=' + // method currently in development. + coreWalletsService.verifyWalletsAreAvailable(); + coreWalletsService.verifyEncryptedWalletIsUnlocked(); + + var trade = getTrade(tradeId); + tradeManager.onMoveInvalidTradeToFailedTrades(trade); + log.info("Trade {} changed to failed trade.", tradeId); + } + + void unFailTrade(String tradeId) { + coreWalletsService.verifyWalletsAreAvailable(); + coreWalletsService.verifyEncryptedWalletIsUnlocked(); + + failedTradesManager.getTradeById(tradeId).ifPresentOrElse(failedTrade -> { + verifyCanUnfailTrade(failedTrade); + failedTradesManager.removeTrade(failedTrade); + tradeManager.addFailedTradeToPendingTrades(failedTrade); + log.info("Failed trade {} changed to open trade.", tradeId); + }, () -> { + throw new IllegalArgumentException(format("failed trade '%s' not found", tradeId)); + }); + } + private Optional getOpenTrade(String tradeId) { return tradeManager.getTradeById(tradeId); } @@ -318,4 +353,30 @@ private void verifyFundsNotWithdrawn(AddressEntry fromAddressEntry) { throw new IllegalStateException(format("funds already withdrawn from address '%s'", fromAddressEntry.getAddressString())); } + + // Throws a RuntimeException if failed trade cannot be changed to OPEN for any reason. + private void verifyCanUnfailTrade(Trade failedTrade) { + if (tradeUtil.getTradeAddresses(failedTrade) == null) + throw new IllegalStateException( + format("cannot change failed trade to open because no trade addresses found for '%s'", + failedTrade.getId())); + + if (!failedTradesManager.hasDepositTx(failedTrade)) + throw new IllegalStateException( + format("cannot change failed trade to open, no deposit tx found for '%s'", + failedTrade.getId())); + + if (!failedTradesManager.hasDelayedPayoutTxBytes(failedTrade)) + throw new IllegalStateException( + format("cannot change failed trade to open, no delayed payout tx found for '%s'", + failedTrade.getId())); + + failedTradesManager.getBlockingTradeIds(failedTrade).ifPresent(tradeIds -> { + throw new IllegalStateException( + format("cannot change failed trade '%s' to open at this time," + + "%ntry again after completing trade(s):%n\t%s", + failedTrade.getId(), + String.join(", ", tradeIds))); + }); + } } diff --git a/core/src/main/java/bisq/core/trade/bisq_v1/FailedTradesManager.java b/core/src/main/java/bisq/core/trade/bisq_v1/FailedTradesManager.java index 412bacb03df..2ab852f9728 100644 --- a/core/src/main/java/bisq/core/trade/bisq_v1/FailedTradesManager.java +++ b/core/src/main/java/bisq/core/trade/bisq_v1/FailedTradesManager.java @@ -24,14 +24,20 @@ import bisq.core.trade.model.TradableList; import bisq.core.trade.model.bisq_v1.Trade; +import bisq.common.config.Config; import bisq.common.crypto.KeyRing; import bisq.common.persistence.PersistenceManager; import bisq.common.proto.persistable.PersistedDataHost; import com.google.inject.Inject; +import javax.inject.Named; + import javafx.collections.ObservableList; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.function.Predicate; import java.util.stream.Stream; @@ -41,6 +47,8 @@ import lombok.Setter; +import static bisq.core.btc.model.AddressEntry.Context.AVAILABLE; + public class FailedTradesManager implements PersistedDataHost { private static final Logger log = LoggerFactory.getLogger(FailedTradesManager.class); private final TradableList failedTrades = new TradableList<>(); @@ -51,6 +59,8 @@ public class FailedTradesManager implements PersistedDataHost { private final PersistenceManager> persistenceManager; private final TradeUtil tradeUtil; private final DumpDelayedPayoutTx dumpDelayedPayoutTx; + private final boolean allowFaultyDelayedTxs; + @Setter private Predicate unFailTradeCallback; @@ -61,7 +71,8 @@ public FailedTradesManager(KeyRing keyRing, PersistenceManager> persistenceManager, TradeUtil tradeUtil, CleanupMailboxMessagesService cleanupMailboxMessagesService, - DumpDelayedPayoutTx dumpDelayedPayoutTx) { + DumpDelayedPayoutTx dumpDelayedPayoutTx, + @Named(Config.ALLOW_FAULTY_DELAYED_TXS) boolean allowFaultyDelayedTxs) { this.keyRing = keyRing; this.priceFeedService = priceFeedService; this.btcWalletService = btcWalletService; @@ -69,6 +80,7 @@ public FailedTradesManager(KeyRing keyRing, this.dumpDelayedPayoutTx = dumpDelayedPayoutTx; this.persistenceManager = persistenceManager; this.tradeUtil = tradeUtil; + this.allowFaultyDelayedTxs = allowFaultyDelayedTxs; this.persistenceManager.initialize(failedTrades, "FailedTrades", PersistenceManager.Source.PRIVATE); } @@ -136,17 +148,59 @@ public String checkUnFail(Trade trade) { if (addresses == null) { return "Addresses not found"; } - StringBuilder blockingTrades = new StringBuilder(); - for (var entry : btcWalletService.getAddressEntryListAsImmutableList()) { - if (entry.getContext() == AddressEntry.Context.AVAILABLE) - continue; - if (entry.getAddressString() != null && - (entry.getAddressString().equals(addresses.first) || - entry.getAddressString().equals(addresses.second))) { - blockingTrades.append(entry.getOfferId()).append(", "); + Optional> blockingTradeIds = getBlockingTradeIds(trade); + return blockingTradeIds.map(strings -> String.join(",", strings)).orElse(""); + } + + public Optional> getBlockingTradeIds(Trade trade) { + var tradeAddresses = tradeUtil.getTradeAddresses(trade); + if (tradeAddresses == null) { + return Optional.empty(); + } + + Predicate isBeingUsedForOtherTrade = (addressEntry) -> { + if (addressEntry.getContext() == AVAILABLE) { + return false; } + String address = addressEntry.getAddressString(); + return address != null + && (address.equals(tradeAddresses.first) || address.equals(tradeAddresses.second)); + }; + + List blockingTradeIds = new ArrayList<>(); + for (var addressEntry : btcWalletService.getAddressEntryListAsImmutableList()) { + if (isBeingUsedForOtherTrade.test(addressEntry)) { + var offerId = addressEntry.getOfferId(); + // TODO Be certain 'List blockingTrades' should NOT be populated + // with the trade parameter's tradeId. The 'var addressEntry' will + // always be found in the 'var tradeAddresses' tuple, so check + // offerId != trade.getId() to avoid the bug being fixed by the next if + // statement (if it was a bug). + if (!Objects.equals(offerId, trade.getId()) && !blockingTradeIds.contains(offerId)) + blockingTradeIds.add(offerId); + } + } + return blockingTradeIds.isEmpty() + ? Optional.empty() + : Optional.of(blockingTradeIds); + } + + public boolean hasDepositTx(Trade failedTrade) { + if (failedTrade.getDepositTx() == null) { + log.warn("Failed trade {} has no deposit tx.", failedTrade.getId()); + return false; + } else { + return true; + } + } + + public boolean hasDelayedPayoutTxBytes(Trade failedTrade) { + if (failedTrade.getDelayedPayoutTxBytes() != null) { + return true; + } else { + log.warn("Failed trade {} has no delayedPayoutTxBytes.", failedTrade.getId()); + return allowFaultyDelayedTxs; } - return blockingTrades.toString(); } private void requestPersistence() { diff --git a/core/src/main/resources/help/failtrade-help.txt b/core/src/main/resources/help/failtrade-help.txt new file mode 100644 index 00000000000..39d250d153e --- /dev/null +++ b/core/src/main/resources/help/failtrade-help.txt @@ -0,0 +1,28 @@ +failtrade + +NAME +---- +failtrade - change an open trade to a failed trade + +SYNOPSIS +-------- +failtrade + --trade-id= + +DESCRIPTION +----------- +If there are problems with an existing trade, and it cannot be completed, it can be moved +from the open trades list to the failed trades list. + +You cannot open mediation or arbitration for a failed trade, but you can change a failed +trade back to an open trade with the 'unfailtrade' command. + +OPTIONS +------- +--trade-id + The ID of the trade (the full offer-id). + +EXAMPLES +-------- +Change the status of an active, open trade to failed: +$ ./bisq-cli --password=xyz --port=9998 failtrade --trade-id=83e8b2e2-51b6-4f39-a748-3ebd29c22aea diff --git a/core/src/main/resources/help/unfailtrade-help.txt b/core/src/main/resources/help/unfailtrade-help.txt new file mode 100644 index 00000000000..f82144dcf38 --- /dev/null +++ b/core/src/main/resources/help/unfailtrade-help.txt @@ -0,0 +1,32 @@ +unfailtrade + +NAME +---- +unfailtrade - change a failed trade to an open trade + +SYNOPSIS +-------- +unfailtrade + --trade-id= + +DESCRIPTION +----------- +This is a possible way to unlock funds stuck in a failed trade. + +The operation could fail for any of the following reasons: + + The trade's deposit transaction is missing (null). + The trade's delayed payout transaction is missing (null). + The trade is using wallet addresses also being used one or more other trades. + +Before proceeding, make sure you have a backup of your data directory. + +OPTIONS +------- +--trade-id + The ID of the trade (the full offer-id). + +EXAMPLES +-------- +Change the status of failed trade back to an open trade: +$ ./bisq-cli --password=xyz --port=9998 unfailtrade --trade-id=83e8b2e2-51b6-4f39-a748-3ebd29c22aea diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java index 38d03382552..dbcb4c6c51d 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java @@ -29,10 +29,14 @@ import bisq.proto.grpc.ConfirmPaymentReceivedRequest; import bisq.proto.grpc.ConfirmPaymentStartedReply; import bisq.proto.grpc.ConfirmPaymentStartedRequest; +import bisq.proto.grpc.FailTradeReply; +import bisq.proto.grpc.FailTradeRequest; import bisq.proto.grpc.GetTradeReply; import bisq.proto.grpc.GetTradeRequest; import bisq.proto.grpc.TakeOfferReply; import bisq.proto.grpc.TakeOfferRequest; +import bisq.proto.grpc.UnFailTradeReply; +import bisq.proto.grpc.UnFailTradeRequest; import bisq.proto.grpc.WithdrawFundsReply; import bisq.proto.grpc.WithdrawFundsRequest; @@ -177,6 +181,32 @@ public void withdrawFunds(WithdrawFundsRequest req, } } + @Override + public void failTrade(FailTradeRequest req, + StreamObserver responseObserver) { + try { + coreApi.failTrade(req.getTradeId()); + var reply = FailTradeReply.newBuilder().build(); + responseObserver.onNext(reply); + responseObserver.onCompleted(); + } catch (Throwable cause) { + exceptionHandler.handleException(log, cause, responseObserver); + } + } + + @Override + public void unFailTrade(UnFailTradeRequest req, + StreamObserver responseObserver) { + try { + coreApi.unFailTrade(req.getTradeId()); + var reply = UnFailTradeReply.newBuilder().build(); + responseObserver.onNext(reply); + responseObserver.onCompleted(); + } catch (Throwable cause) { + exceptionHandler.handleException(log, cause, responseObserver); + } + } + final ServerInterceptor[] interceptors() { Optional rateMeteringInterceptor = rateMeteringInterceptor(); return rateMeteringInterceptor.map(serverInterceptor -> diff --git a/proto/src/main/proto/grpc.proto b/proto/src/main/proto/grpc.proto index 1af7e69ab01..bb4504c74c9 100644 --- a/proto/src/main/proto/grpc.proto +++ b/proto/src/main/proto/grpc.proto @@ -396,6 +396,10 @@ service Trades { } rpc CloseTrade (CloseTradeRequest) returns (CloseTradeReply) { } + rpc FailTrade (FailTradeRequest) returns (FailTradeReply) { + } + rpc UnFailTrade (UnFailTradeRequest) returns (UnFailTradeReply) { + } rpc WithdrawFunds (WithdrawFundsRequest) returns (WithdrawFundsReply) { } } @@ -440,6 +444,20 @@ message CloseTradeRequest { message CloseTradeReply { } +message FailTradeRequest { + string tradeId = 1; +} + +message FailTradeReply { +} + +message UnFailTradeRequest { + string tradeId = 1; +} + +message UnFailTradeReply { +} + message WithdrawFundsRequest { string tradeId = 1; string address = 2;