From dc6144d337ae57d7d8c445f04758585acd7c3136 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 4 Dec 2020 14:17:24 -0300 Subject: [PATCH 01/20] Refactor BtcWalletService to let api override fee rates BtcWalletService was changed to allow the api to override tx fee rates from the sendbsq and sendbtc methods. The api methods will still be able to use the network fee service and custom tx fee rate preference, and set / unset the custom tx fee rate preference, but the change will permit the addition of an optional txFeeRate parameter to the sendbsq and sendbtc methods (todo). A few other minor changes (style and removal of never thrown ex spec) were also made to this class. Two BtcWalletService methods were refactored. - The redundant (was always true) boolean isSendTx argument was removed from the completePreparedVoteRevealTx method signature. - The redundant (was always true) boolean useCustomTxFee was removed from the completePreparedBsqTx method signature. - The completePreparedSendBsqTx method was overloaded with a 2nd parameter (Coin txFeePerVbyte) to allow api to override fee service and custom tx fee rate when sending BSQ or BTC. - The completePreparedBsqTx method was overloaded with a 3rd parameter (Coin txFeePerVbyte) to allow api to override fee service and custom tx fee rate when sending BSQ or BTC. The following line was deleted from the completePreparedBsqTx method because txFeePerVbyte is now an argument: Coin txFeePerVbyte = useCustomTxFee ? getTxFeeForWithdrawalPerVbyte() : feeService.getTxFeePerVbyte(); This useCustomTxFee value was always true, and redudant here because getTxFeeForWithdrawalPerVbyte() returns feeService.getTxFeePerVbyte() or the custom fee rate preference. i.e., Coin txFeePerVbyte = useCustomTxFee ? getTxFeeForWithdrawalPerVbyte() : feeService.getTxFeePerVbyte(); is equivalent to Coin txFeePerVbyte = getTxFeeForWithdrawalPerVbyte(); LockupTxService, UnlockTxService, BsqSendView, and BsqTransferService were adjusted to this BtcWalletService refactoring. --- .../core/btc/wallet/BsqTransferService.java | 2 +- .../core/btc/wallet/BtcWalletService.java | 40 ++++++++++++------- .../bond/lockup/LockupTxService.java | 2 +- .../bond/unlock/UnlockTxService.java | 2 +- .../main/dao/wallet/send/BsqSendView.java | 4 +- 5 files changed, 30 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/BsqTransferService.java b/core/src/main/java/bisq/core/btc/wallet/BsqTransferService.java index b6cc83e8c77..6bb9c79f039 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqTransferService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqTransferService.java @@ -40,7 +40,7 @@ public BsqTransferModel getBsqTransferModel(LegacyAddress address, InsufficientMoneyException { Transaction preparedSendTx = bsqWalletService.getPreparedSendBsqTx(address.toString(), receiverAmount); - Transaction txWithBtcFee = btcWalletService.completePreparedSendBsqTx(preparedSendTx, true); + Transaction txWithBtcFee = btcWalletService.completePreparedSendBsqTx(preparedSendTx); Transaction signedTx = bsqWalletService.signTx(txWithBtcFee); return new BsqTransferModel(address, diff --git a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java index 3d45c491878..59ea556d591 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -439,8 +439,7 @@ public Transaction completePreparedVoteRevealTx(Transaction preparedTx, byte[] o // Add fee input to prepared BSQ send tx /////////////////////////////////////////////////////////////////////////////////////////// - - public Transaction completePreparedSendBsqTx(Transaction preparedBsqTx, boolean isSendTx) throws + public Transaction completePreparedSendBsqTx(Transaction preparedBsqTx) throws TransactionVerificationException, WalletException, InsufficientMoneyException { // preparedBsqTx has following structure: // inputs [1-n] BSQ inputs @@ -454,13 +453,26 @@ public Transaction completePreparedSendBsqTx(Transaction preparedBsqTx, boolean // outputs [0-1] BSQ change output // outputs [0-1] BTC change output // mining fee: BTC mining fee - return completePreparedBsqTx(preparedBsqTx, isSendTx, null); + Coin txFeePerVbyte = getTxFeeForWithdrawalPerVbyte(); + return completePreparedBsqTx(preparedBsqTx, null, txFeePerVbyte); + } + + public Transaction completePreparedSendBsqTx(Transaction preparedBsqTx, Coin txFeePerVbyte) throws + TransactionVerificationException, WalletException, InsufficientMoneyException { + return completePreparedBsqTx(preparedBsqTx, null, txFeePerVbyte); } public Transaction completePreparedBsqTx(Transaction preparedBsqTx, - boolean useCustomTxFee, @Nullable byte[] opReturnData) throws TransactionVerificationException, WalletException, InsufficientMoneyException { + Coin txFeePerVbyte = getTxFeeForWithdrawalPerVbyte(); + return completePreparedBsqTx(preparedBsqTx, opReturnData, txFeePerVbyte); + } + + public Transaction completePreparedBsqTx(Transaction preparedBsqTx, + @Nullable byte[] opReturnData, + Coin txFeePerVbyte) throws + TransactionVerificationException, WalletException, InsufficientMoneyException { // preparedBsqTx has following structure: // inputs [1-n] BSQ inputs @@ -487,8 +499,6 @@ public Transaction completePreparedBsqTx(Transaction preparedBsqTx, int sigSizePerInput = 106; // typical size for a tx with 2 inputs int txVsizeWithUnsignedInputs = 203; - // If useCustomTxFee we allow overriding the estimated fee from preferences - Coin txFeePerVbyte = useCustomTxFee ? getTxFeeForWithdrawalPerVbyte() : feeService.getTxFeePerVbyte(); // In case there are no change outputs we force a change by adding min dust to the BTC input Coin forcedChangeValue = Coin.ZERO; @@ -532,8 +542,8 @@ public Transaction completePreparedBsqTx(Transaction preparedBsqTx, sendRequest.signInputs = false; sendRequest.fee = txFeePerVbyte.multiply(txVsizeWithUnsignedInputs + - sigSizePerInput * numLegacyInputs + - sigSizePerInput * numSegwitInputs / 4); + sigSizePerInput * numLegacyInputs + + sigSizePerInput * numSegwitInputs / 4); sendRequest.feePerKb = Coin.ZERO; sendRequest.ensureMinRequiredFee = false; @@ -558,8 +568,8 @@ public Transaction completePreparedBsqTx(Transaction preparedBsqTx, numSegwitInputs = numInputs.second; txVsizeWithUnsignedInputs = resultTx.getVsize(); final long estimatedFeeAsLong = txFeePerVbyte.multiply(txVsizeWithUnsignedInputs + - sigSizePerInput * numLegacyInputs + - sigSizePerInput * numSegwitInputs / 4).value; + sigSizePerInput * numLegacyInputs + + sigSizePerInput * numSegwitInputs / 4).value; // calculated fee must be inside of a tolerance range with tx fee isFeeOutsideTolerance = Math.abs(resultTx.getFee().value - estimatedFeeAsLong) > 1000; } @@ -933,7 +943,7 @@ public void doubleSpendTransaction(String txId, Runnable resultHandler, ErrorMes } if (sendResult != null) { log.info("Broadcasting double spending transaction. " + sendResult.tx); - Futures.addCallback(sendResult.broadcastComplete, new FutureCallback() { + Futures.addCallback(sendResult.broadcastComplete, new FutureCallback<>() { @Override public void onSuccess(Transaction result) { log.info("Double spending transaction published. " + result); @@ -1163,7 +1173,7 @@ private SendRequest getSendRequestForMultipleAddresses(Set fromAddresses Coin fee, @Nullable String changeAddress, @Nullable KeyParameter aesKey) throws - AddressFormatException, AddressEntryException, InsufficientMoneyException { + AddressFormatException, AddressEntryException { Transaction tx = new Transaction(params); final Coin netValue = amount.subtract(fee); checkArgument(Restrictions.isAboveDust(netValue), @@ -1196,12 +1206,12 @@ private SendRequest getSendRequestForMultipleAddresses(Set fromAddresses sendRequest.coinSelector = new BtcCoinSelector(walletsSetup.getAddressesFromAddressEntries(addressEntries), preferences.getIgnoreDustThreshold()); - Optional addressEntryOptional = Optional.empty(); - AddressEntry changeAddressAddressEntry = null; + Optional addressEntryOptional = Optional.empty(); + if (changeAddress != null) addressEntryOptional = findAddressEntry(changeAddress, AddressEntry.Context.AVAILABLE); - changeAddressAddressEntry = addressEntryOptional.orElseGet(() -> getFreshAddressEntry()); + AddressEntry changeAddressAddressEntry = addressEntryOptional.orElseGet(this::getFreshAddressEntry); checkNotNull(changeAddressAddressEntry, "change address must not be null"); sendRequest.changeAddress = changeAddressAddressEntry.getAddress(); return sendRequest; diff --git a/core/src/main/java/bisq/core/dao/governance/bond/lockup/LockupTxService.java b/core/src/main/java/bisq/core/dao/governance/bond/lockup/LockupTxService.java index 4a466811e0c..901793c74de 100644 --- a/core/src/main/java/bisq/core/dao/governance/bond/lockup/LockupTxService.java +++ b/core/src/main/java/bisq/core/dao/governance/bond/lockup/LockupTxService.java @@ -103,7 +103,7 @@ private Transaction getLockupTx(Coin lockupAmount, int lockTime, LockupReason lo throws InsufficientMoneyException, WalletException, TransactionVerificationException, IOException { byte[] opReturnData = BondConsensus.getLockupOpReturnData(lockTime, lockupReason, hash); Transaction preparedTx = bsqWalletService.getPreparedLockupTx(lockupAmount); - Transaction txWithBtcFee = btcWalletService.completePreparedBsqTx(preparedTx, true, opReturnData); + Transaction txWithBtcFee = btcWalletService.completePreparedBsqTx(preparedTx, opReturnData); Transaction transaction = bsqWalletService.signTx(txWithBtcFee); log.info("Lockup tx: " + transaction); return transaction; diff --git a/core/src/main/java/bisq/core/dao/governance/bond/unlock/UnlockTxService.java b/core/src/main/java/bisq/core/dao/governance/bond/unlock/UnlockTxService.java index 0defe5e4752..a4c6691cf67 100644 --- a/core/src/main/java/bisq/core/dao/governance/bond/unlock/UnlockTxService.java +++ b/core/src/main/java/bisq/core/dao/governance/bond/unlock/UnlockTxService.java @@ -103,7 +103,7 @@ private Transaction getUnlockTx(String lockupTxId) checkArgument(optionalLockupTxOutput.isPresent(), "lockupTxOutput must be present"); TxOutput lockupTxOutput = optionalLockupTxOutput.get(); Transaction preparedTx = bsqWalletService.getPreparedUnlockTx(lockupTxOutput); - Transaction txWithBtcFee = btcWalletService.completePreparedBsqTx(preparedTx, true, null); + Transaction txWithBtcFee = btcWalletService.completePreparedBsqTx(preparedTx, null); Transaction transaction = bsqWalletService.signTx(txWithBtcFee); log.info("Unlock tx: " + transaction); return transaction; diff --git a/desktop/src/main/java/bisq/desktop/main/dao/wallet/send/BsqSendView.java b/desktop/src/main/java/bisq/desktop/main/dao/wallet/send/BsqSendView.java index 917d1290aae..533064b62c1 100644 --- a/desktop/src/main/java/bisq/desktop/main/dao/wallet/send/BsqSendView.java +++ b/desktop/src/main/java/bisq/desktop/main/dao/wallet/send/BsqSendView.java @@ -247,7 +247,7 @@ private void addSendBsqGroup() { Coin receiverAmount = ParsingUtils.parseToCoin(amountInputTextField.getText(), bsqFormatter); try { Transaction preparedSendTx = bsqWalletService.getPreparedSendBsqTx(receiversAddressString, receiverAmount); - Transaction txWithBtcFee = btcWalletService.completePreparedSendBsqTx(preparedSendTx, true); + Transaction txWithBtcFee = btcWalletService.completePreparedSendBsqTx(preparedSendTx); Transaction signedTx = bsqWalletService.signTx(txWithBtcFee); Coin miningFee = signedTx.getFee(); int txVsize = signedTx.getVsize(); @@ -305,7 +305,7 @@ private void addSendBtcGroup() { Coin receiverAmount = bsqFormatter.parseToBTC(btcAmountInputTextField.getText()); try { Transaction preparedSendTx = bsqWalletService.getPreparedSendBtcTx(receiversAddressString, receiverAmount); - Transaction txWithBtcFee = btcWalletService.completePreparedSendBsqTx(preparedSendTx, true); + Transaction txWithBtcFee = btcWalletService.completePreparedSendBsqTx(preparedSendTx); Transaction signedTx = bsqWalletService.signTx(txWithBtcFee); Coin miningFee = signedTx.getFee(); From 159d4cc6f505a178b5ab304888a276a35098c1b1 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 4 Dec 2020 17:17:37 -0300 Subject: [PATCH 02/20] Add optional txFeeRate parameter to api sendbsq If present in the sendbsq command, the parameter will override the fee service and custom fee rate setting for the BSQ transaction. Also changed the sendbsq grpc return type to a lightweight TX proto wrapper. Besides some small refactoring in the CLI, all the changes are adjustments for this new sendbsq parameter and its new grpc return value. --- .../java/bisq/apitest/method/MethodTest.java | 27 +++++- .../apitest/method/wallet/BsqWalletTest.java | 2 +- cli/src/main/java/bisq/cli/CliMain.java | 57 ++++++++----- core/src/main/java/bisq/core/api/CoreApi.java | 7 +- .../bisq/core/api/CoreWalletsService.java | 18 +++- .../main/java/bisq/core/api/model/TxInfo.java | 84 +++++++++++++++++++ .../core/btc/wallet/BsqTransferService.java | 5 +- .../bisq/daemon/grpc/GrpcWalletsService.java | 41 +++++---- proto/src/main/proto/grpc.proto | 27 ++++-- 9 files changed, 212 insertions(+), 56 deletions(-) create mode 100644 core/src/main/java/bisq/core/api/model/TxInfo.java diff --git a/apitest/src/test/java/bisq/apitest/method/MethodTest.java b/apitest/src/test/java/bisq/apitest/method/MethodTest.java index 0a43258d9a5..f275e885d19 100644 --- a/apitest/src/test/java/bisq/apitest/method/MethodTest.java +++ b/apitest/src/test/java/bisq/apitest/method/MethodTest.java @@ -52,6 +52,7 @@ import bisq.proto.grpc.SetWalletPasswordRequest; import bisq.proto.grpc.TakeOfferRequest; import bisq.proto.grpc.TradeInfo; +import bisq.proto.grpc.TxInfo; import bisq.proto.grpc.UnlockWalletRequest; import bisq.proto.grpc.UnsetTxFeeRatePreferenceRequest; import bisq.proto.grpc.WithdrawFundsRequest; @@ -160,8 +161,14 @@ protected final GetUnusedBsqAddressRequest createGetUnusedBsqAddressRequest() { return GetUnusedBsqAddressRequest.newBuilder().build(); } - protected final SendBsqRequest createSendBsqRequest(String address, String amount) { - return SendBsqRequest.newBuilder().setAddress(address).setAmount(amount).build(); + protected final SendBsqRequest createSendBsqRequest(String address, + String amount, + String txFeeRate) { + return SendBsqRequest.newBuilder() + .setAddress(address) + .setAmount(amount) + .setTxFeeRate(txFeeRate) + .build(); } protected final GetFundingAddressesRequest createGetFundingAddressesRequest() { @@ -247,9 +254,21 @@ protected final String getUnusedBsqAddress(BisqAppConfig bisqAppConfig) { return grpcStubs(bisqAppConfig).walletsService.getUnusedBsqAddress(createGetUnusedBsqAddressRequest()).getAddress(); } - protected final void sendBsq(BisqAppConfig bisqAppConfig, String address, String amount) { + protected final TxInfo sendBsq(BisqAppConfig bisqAppConfig, + String address, + String amount) { + return sendBsq(bisqAppConfig, address, amount, ""); + } + + protected final TxInfo sendBsq(BisqAppConfig bisqAppConfig, + String address, + String amount, + String txFeeRate) { //noinspection ResultOfMethodCallIgnored - grpcStubs(bisqAppConfig).walletsService.sendBsq(createSendBsqRequest(address, amount)); + return grpcStubs(bisqAppConfig).walletsService.sendBsq(createSendBsqRequest(address, + amount, + txFeeRate)) + .getTxInfo(); } protected final String getUnusedBtcAddress(BisqAppConfig bisqAppConfig) { diff --git a/apitest/src/test/java/bisq/apitest/method/wallet/BsqWalletTest.java b/apitest/src/test/java/bisq/apitest/method/wallet/BsqWalletTest.java index f9202af778a..2884555e3c7 100644 --- a/apitest/src/test/java/bisq/apitest/method/wallet/BsqWalletTest.java +++ b/apitest/src/test/java/bisq/apitest/method/wallet/BsqWalletTest.java @@ -108,7 +108,7 @@ public void testInitialBsqBalances(final TestInfo testInfo) { @Order(3) public void testSendBsqAndCheckBalancesBeforeGeneratingBtcBlock(final TestInfo testInfo) { String bobsBsqAddress = getUnusedBsqAddress(bobdaemon); - sendBsq(alicedaemon, bobsBsqAddress, SEND_BSQ_AMOUNT); + sendBsq(alicedaemon, bobsBsqAddress, SEND_BSQ_AMOUNT, "100"); sleep(2000); BsqBalanceInfo alicesBsqBalances = getBsqBalances(alicedaemon); diff --git a/cli/src/main/java/bisq/cli/CliMain.java b/cli/src/main/java/bisq/cli/CliMain.java index 44590cfbd6a..dc9c1204584 100644 --- a/cli/src/main/java/bisq/cli/CliMain.java +++ b/cli/src/main/java/bisq/cli/CliMain.java @@ -43,6 +43,7 @@ import bisq.proto.grpc.SetTxFeeRatePreferenceRequest; import bisq.proto.grpc.SetWalletPasswordRequest; import bisq.proto.grpc.TakeOfferRequest; +import bisq.proto.grpc.TxInfo; import bisq.proto.grpc.UnlockWalletRequest; import bisq.proto.grpc.UnsetTxFeeRatePreferenceRequest; import bisq.proto.grpc.WithdrawFundsRequest; @@ -259,19 +260,20 @@ public static void run(String[] args) { throw new IllegalArgumentException("no bsq amount specified"); var amount = nonOptionArgs.get(2); + verifyStringIsValidDecimal(amount); - try { - Double.parseDouble(amount); - } catch (NumberFormatException e) { - throw new IllegalArgumentException(format("'%s' is not a number", amount)); - } + var txFeeRate = nonOptionArgs.size() == 4 ? nonOptionArgs.get(3) : ""; + if (!txFeeRate.isEmpty()) + verifyStringIsValidLong(txFeeRate); var request = SendBsqRequest.newBuilder() .setAddress(address) .setAmount(amount) + .setTxFeeRate(txFeeRate) .build(); - walletsService.sendBsq(request); - out.printf("%s BSQ sent to %s%n", amount, address); + var reply = walletsService.sendBsq(request); + TxInfo txInfo = reply.getTxInfo(); + out.printf("%s bsq sent to %s in tx %s%n", amount, address, txInfo.getId()); return; } case gettxfeerate: { @@ -284,13 +286,7 @@ public static void run(String[] args) { if (nonOptionArgs.size() < 2) throw new IllegalArgumentException("no tx fee rate specified"); - long txFeeRate; - try { - txFeeRate = Long.parseLong(nonOptionArgs.get(2)); - } catch (NumberFormatException e) { - throw new IllegalArgumentException(format("'%s' is not a number", nonOptionArgs.get(2))); - } - + var txFeeRate = toLong(nonOptionArgs.get(2)); var request = SetTxFeeRatePreferenceRequest.newBuilder() .setTxFeeRatePreference(txFeeRate) .build(); @@ -560,12 +556,7 @@ public static void run(String[] args) { if (nonOptionArgs.size() < 3) throw new IllegalArgumentException("no unlock timeout specified"); - long timeout; - try { - timeout = Long.parseLong(nonOptionArgs.get(2)); - } catch (NumberFormatException e) { - throw new IllegalArgumentException(format("'%s' is not a number", nonOptionArgs.get(2))); - } + var timeout = toLong(nonOptionArgs.get(2)); var request = UnlockWalletRequest.newBuilder() .setPassword(nonOptionArgs.get(1)) .setTimeout(timeout).build(); @@ -627,6 +618,30 @@ private static Method getMethodFromCmd(String methodName) { return Method.valueOf(methodName.toLowerCase()); } + private static void verifyStringIsValidDecimal(String param) { + try { + Double.parseDouble(param); + } catch (NumberFormatException ex) { + throw new IllegalArgumentException(format("'%s' is not a number", param)); + } + } + + private static void verifyStringIsValidLong(String param) { + try { + Long.parseLong(param); + } catch (NumberFormatException ex) { + throw new IllegalArgumentException(format("'%s' is not a number", param)); + } + } + + private static long toLong(String param) { + try { + return Long.parseLong(param); + } catch (NumberFormatException ex) { + throw new IllegalArgumentException(format("'%s' is not a number", param)); + } + } + private static File saveFileToDisk(String prefix, @SuppressWarnings("SameParameterValue") String suffix, String text) { @@ -663,7 +678,7 @@ private static void printHelp(OptionParser parser, PrintStream stream) { stream.format(rowFormat, "getaddressbalance", "address", "Get server wallet address balance"); stream.format(rowFormat, "getfundingaddresses", "", "Get BTC funding addresses"); stream.format(rowFormat, "getunusedbsqaddress", "", "Get unused BSQ address"); - stream.format(rowFormat, "sendbsq", "address, amount", "Send BSQ"); + stream.format(rowFormat, "sendbsq", "address, amount [,tx fee rate (sats/byte)]", "Send BSQ"); stream.format(rowFormat, "gettxfeerate", "", "Get current tx fee rate in sats/byte"); stream.format(rowFormat, "settxfeerate", "satoshis (per byte)", "Set custom tx fee rate in sats/byte"); stream.format(rowFormat, "unsettxfeerate", "", "Unset custom tx fee rate"); diff --git a/core/src/main/java/bisq/core/api/CoreApi.java b/core/src/main/java/bisq/core/api/CoreApi.java index 9b7b25bc8f2..3319db51649 100644 --- a/core/src/main/java/bisq/core/api/CoreApi.java +++ b/core/src/main/java/bisq/core/api/CoreApi.java @@ -244,8 +244,11 @@ public String getUnusedBsqAddress() { return walletsService.getUnusedBsqAddress(); } - public void sendBsq(String address, String amount, TxBroadcaster.Callback callback) { - walletsService.sendBsq(address, amount, callback); + public void sendBsq(String address, + String amount, + String txFeeRate, + TxBroadcaster.Callback callback) { + walletsService.sendBsq(address, amount, txFeeRate, callback); } public void getTxFeeRate(ResultHandler resultHandler) { diff --git a/core/src/main/java/bisq/core/api/CoreWalletsService.java b/core/src/main/java/bisq/core/api/CoreWalletsService.java index 346c97a78de..5d52d9f9022 100644 --- a/core/src/main/java/bisq/core/api/CoreWalletsService.java +++ b/core/src/main/java/bisq/core/api/CoreWalletsService.java @@ -189,13 +189,27 @@ String getUnusedBsqAddress() { void sendBsq(String address, String amount, + String txFeeRate, TxBroadcaster.Callback callback) { try { LegacyAddress legacyAddress = getValidBsqLegacyAddress(address); Coin receiverAmount = getValidBsqTransferAmount(amount); - BsqTransferModel model = bsqTransferService.getBsqTransferModel(legacyAddress, receiverAmount); + + // A non txFeeRate String value overrides the fee service and custom fee. + Coin txFeePerVbyte = txFeeRate.isEmpty() + ? btcWalletService.getTxFeeForWithdrawalPerVbyte() + : Coin.valueOf(Long.parseLong(txFeeRate)); + + BsqTransferModel model = bsqTransferService.getBsqTransferModel(legacyAddress, + receiverAmount, + txFeePerVbyte); + log.info("Sending {} BSQ to {} with tx fee rate {} sats/byte).", + amount, + address, + txFeePerVbyte.value); bsqTransferService.sendFunds(model, callback); - } catch (InsufficientMoneyException + } catch (NumberFormatException + | InsufficientMoneyException | BsqChangeBelowDustException | TransactionVerificationException | WalletException ex) { diff --git a/core/src/main/java/bisq/core/api/model/TxInfo.java b/core/src/main/java/bisq/core/api/model/TxInfo.java new file mode 100644 index 00000000000..f1b24f6b0fa --- /dev/null +++ b/core/src/main/java/bisq/core/api/model/TxInfo.java @@ -0,0 +1,84 @@ +/* + * 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.core.api.model; + +import bisq.common.Payload; + +import org.bitcoinj.core.Transaction; + +import lombok.EqualsAndHashCode; +import lombok.Getter; + +@EqualsAndHashCode +@Getter +public class TxInfo implements Payload { + + private final String id; + private final long outputSum; + private final long fee; + private final int size; + + public TxInfo(String id, long outputSum, long fee, int size) { + this.id = id; + this.outputSum = outputSum; + this.fee = fee; + this.size = size; + } + + public static TxInfo toTxInfo(Transaction transaction) { + if (transaction == null) + throw new IllegalStateException("server created a null transaction"); + + return new TxInfo(transaction.getTxId().toString(), + transaction.getOutputSum().value, + transaction.getFee().value, + transaction.getMessageSize()); + } + + ////////////////////////////////////////////////////////////////////////////////////// + // PROTO BUFFER + ////////////////////////////////////////////////////////////////////////////////////// + + @Override + public bisq.proto.grpc.TxInfo toProtoMessage() { + return bisq.proto.grpc.TxInfo.newBuilder() + .setId(id) + .setOutputSum(outputSum) + .setFee(fee) + .setSize(size) + .build(); + } + + @SuppressWarnings("unused") + public static TxInfo fromProto(bisq.proto.grpc.TxInfo proto) { + return new TxInfo(proto.getId(), + proto.getOutputSum(), + proto.getFee(), + proto.getSize()); + } + + @Override + public String toString() { + return "TxInfo{" + "\n" + + " id='" + id + '\'' + "\n" + + ", outputSum=" + outputSum + " sats" + "\n" + + ", fee=" + fee + " sats" + "\n" + + ", size=" + size + " bytes" + "\n" + + '}'; + } +} diff --git a/core/src/main/java/bisq/core/btc/wallet/BsqTransferService.java b/core/src/main/java/bisq/core/btc/wallet/BsqTransferService.java index 6bb9c79f039..4558b0acf74 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqTransferService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqTransferService.java @@ -33,14 +33,15 @@ public BsqTransferService(WalletsManager walletsManager, } public BsqTransferModel getBsqTransferModel(LegacyAddress address, - Coin receiverAmount) + Coin receiverAmount, + Coin txFeePerVbyte) throws TransactionVerificationException, WalletException, BsqChangeBelowDustException, InsufficientMoneyException { Transaction preparedSendTx = bsqWalletService.getPreparedSendBsqTx(address.toString(), receiverAmount); - Transaction txWithBtcFee = btcWalletService.completePreparedSendBsqTx(preparedSendTx); + Transaction txWithBtcFee = btcWalletService.completePreparedSendBsqTx(preparedSendTx, txFeePerVbyte); Transaction signedTx = bsqWalletService.signTx(txWithBtcFee); return new BsqTransferModel(address, diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java index b138a6c6931..c38d5209d1b 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java @@ -62,6 +62,8 @@ import lombok.extern.slf4j.Slf4j; +import static bisq.core.api.model.TxInfo.toTxInfo; + @Slf4j class GrpcWalletsService extends WalletsGrpc.WalletsImplBase { @@ -145,24 +147,29 @@ public void getUnusedBsqAddress(GetUnusedBsqAddressRequest req, public void sendBsq(SendBsqRequest req, StreamObserver responseObserver) { try { - coreApi.sendBsq(req.getAddress(), req.getAmount(), new TxBroadcaster.Callback() { - @Override - public void onSuccess(Transaction tx) { - log.info("Successfully published BSQ tx: id {}, output sum {} sats, fee {} sats, size {} bytes", - tx.getTxId().toString(), - tx.getOutputSum(), - tx.getFee(), - tx.getMessageSize()); - var reply = SendBsqReply.newBuilder().build(); - responseObserver.onNext(reply); - responseObserver.onCompleted(); - } + coreApi.sendBsq(req.getAddress(), + req.getAmount(), + req.getTxFeeRate(), + new TxBroadcaster.Callback() { + @Override + public void onSuccess(Transaction tx) { + log.info("Successfully published BSQ tx: id {}, output sum {} sats, fee {} sats, size {} bytes", + tx.getTxId().toString(), + tx.getOutputSum(), + tx.getFee(), + tx.getMessageSize()); + var reply = SendBsqReply.newBuilder() + .setTxInfo(toTxInfo(tx).toProtoMessage()) + .build(); + responseObserver.onNext(reply); + responseObserver.onCompleted(); + } - @Override - public void onFailure(TxBroadcastException ex) { - throw new IllegalStateException(ex); - } - }); + @Override + public void onFailure(TxBroadcastException ex) { + throw new IllegalStateException(ex); + } + }); } catch (IllegalStateException cause) { var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); responseObserver.onError(ex); diff --git a/proto/src/main/proto/grpc.proto b/proto/src/main/proto/grpc.proto index 517cfc4ea46..b4950b96a25 100644 --- a/proto/src/main/proto/grpc.proto +++ b/proto/src/main/proto/grpc.proto @@ -287,6 +287,24 @@ message TradeInfo { string contractAsJson = 24; } +/////////////////////////////////////////////////////////////////////////////////////////// +// Transactions +/////////////////////////////////////////////////////////////////////////////////////////// + +message TxFeeRateInfo { + bool useCustomTxFeeRate = 1; + uint64 customTxFeeRate = 2; + uint64 feeServiceRate = 3; + uint64 lastFeeServiceRequestTs = 4; +} + +message TxInfo { + string id = 1; + uint64 outputSum = 2; + uint64 fee = 3; + int32 size = 4; +} + /////////////////////////////////////////////////////////////////////////////////////////// // Wallets /////////////////////////////////////////////////////////////////////////////////////////// @@ -344,9 +362,11 @@ message GetUnusedBsqAddressReply { message SendBsqRequest { string address = 1; string amount = 2; + string txFeeRate = 3; } message SendBsqReply { + TxInfo txInfo = 1; } message GetTxFeeRateRequest { @@ -437,13 +457,6 @@ message AddressBalanceInfo { int64 numConfirmations = 3; } -message TxFeeRateInfo { - bool useCustomTxFeeRate = 1; - uint64 customTxFeeRate = 2; - uint64 feeServiceRate = 3; - uint64 lastFeeServiceRequestTs = 4; -} - /////////////////////////////////////////////////////////////////////////////////////////// // Version /////////////////////////////////////////////////////////////////////////////////////////// From 6c9f0c252d061bd713e4bd6c399e8473c5649f91 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Tue, 8 Dec 2020 21:12:02 -0300 Subject: [PATCH 03/20] Add new api method 'sendbtc' and test Takes an address, amount, and optional txfeerate param, returns a lightweight TxInfo proto. Also overloaded two BtcWalletService methods to allow sendbtc to pass in the tx fee rate -- overriding the fee service and custom fee rate setting. --- .../java/bisq/apitest/method/MethodTest.java | 26 +++++ .../apitest/method/wallet/BtcWalletTest.java | 33 +++++++ .../bisq/apitest/scenario/WalletTest.java | 1 + cli/src/main/java/bisq/cli/CliMain.java | 29 ++++++ core/src/main/java/bisq/core/api/CoreApi.java | 10 ++ .../bisq/core/api/CoreWalletsService.java | 97 ++++++++++++++++--- .../core/btc/wallet/BtcWalletService.java | 15 ++- .../bisq/daemon/grpc/GrpcWalletsService.java | 45 +++++++++ proto/src/main/proto/grpc.proto | 12 +++ 9 files changed, 253 insertions(+), 15 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/method/MethodTest.java b/apitest/src/test/java/bisq/apitest/method/MethodTest.java index f275e885d19..f64c2a5fe92 100644 --- a/apitest/src/test/java/bisq/apitest/method/MethodTest.java +++ b/apitest/src/test/java/bisq/apitest/method/MethodTest.java @@ -48,6 +48,7 @@ import bisq.proto.grpc.RegisterDisputeAgentRequest; import bisq.proto.grpc.RemoveWalletPasswordRequest; import bisq.proto.grpc.SendBsqRequest; +import bisq.proto.grpc.SendBtcRequest; import bisq.proto.grpc.SetTxFeeRatePreferenceRequest; import bisq.proto.grpc.SetWalletPasswordRequest; import bisq.proto.grpc.TakeOfferRequest; @@ -171,6 +172,16 @@ protected final SendBsqRequest createSendBsqRequest(String address, .build(); } + protected final SendBtcRequest createSendBtcRequest(String address, + String amount, + String txFeeRate) { + return SendBtcRequest.newBuilder() + .setAddress(address) + .setAmount(amount) + .setTxFeeRate(txFeeRate) + .build(); + } + protected final GetFundingAddressesRequest createGetFundingAddressesRequest() { return GetFundingAddressesRequest.newBuilder().build(); } @@ -271,6 +282,21 @@ protected final TxInfo sendBsq(BisqAppConfig bisqAppConfig, .getTxInfo(); } + protected final TxInfo sendBtc(BisqAppConfig bisqAppConfig, String address, String amount) { + return sendBtc(bisqAppConfig, address, amount, ""); + } + + protected final TxInfo sendBtc(BisqAppConfig bisqAppConfig, + String address, + String amount, + String txFeeRate) { + //noinspection ResultOfMethodCallIgnored + return grpcStubs(bisqAppConfig).walletsService.sendBtc(createSendBtcRequest(address, + amount, + txFeeRate)) + .getTxInfo(); + } + protected final String getUnusedBtcAddress(BisqAppConfig bisqAppConfig) { //noinspection OptionalGetWithoutIsPresent return grpcStubs(bisqAppConfig).walletsService.getFundingAddresses(createGetFundingAddressesRequest()) diff --git a/apitest/src/test/java/bisq/apitest/method/wallet/BtcWalletTest.java b/apitest/src/test/java/bisq/apitest/method/wallet/BtcWalletTest.java index daee479b89a..a97e9a690da 100644 --- a/apitest/src/test/java/bisq/apitest/method/wallet/BtcWalletTest.java +++ b/apitest/src/test/java/bisq/apitest/method/wallet/BtcWalletTest.java @@ -20,6 +20,7 @@ import static bisq.cli.TableFormat.formatBtcBalanceInfoTbl; import static java.util.Collections.singletonList; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.MethodOrderer.OrderAnnotation; @@ -92,6 +93,38 @@ public void testFundAlicesBtcWallet(final TestInfo testInfo) { formatBtcBalanceInfoTbl(btcBalanceInfo)); } + @Test + @Order(3) + public void testAliceSendBTCToBob(TestInfo testInfo) { + String bobsBtcAddress = getUnusedBtcAddress(bobdaemon); + log.debug("Sending 5.5 BTC From Alice to Bob @ {}", bobsBtcAddress); + + sendBtc(alicedaemon, bobsBtcAddress, "5.50", "100"); + genBtcBlocksThenWait(1, 3000); + + BtcBalanceInfo alicesBalances = getBtcBalances(alicedaemon); + + log.debug("{} Alice's BTC Balances:\n{}", + testName(testInfo), + formatBtcBalanceInfoTbl(alicesBalances)); + bisq.core.api.model.BtcBalanceInfo alicesExpectedBalances = + bisq.core.api.model.BtcBalanceInfo.valueOf(700000000, + 0, + 700000000, + 0); + verifyBtcBalances(alicesExpectedBalances, alicesBalances); + + BtcBalanceInfo bobsBalances = getBtcBalances(bobdaemon); + log.debug("{} Bob's BTC Balances:\n{}", + testName(testInfo), + formatBtcBalanceInfoTbl(bobsBalances)); + // We cannot (?) predict the exact tx size and calculate how much in tx fees were + // deducted from the 5.5 BTC sent to Bob, but we do know Bob should have something + // between 15.49978000 and 15.49978100 BTC. + assertTrue(bobsBalances.getAvailableBalance() >= 1549978000); + assertTrue(bobsBalances.getAvailableBalance() <= 1549978100); + } + @AfterAll public static void tearDown() { tearDownScaffold(); diff --git a/apitest/src/test/java/bisq/apitest/scenario/WalletTest.java b/apitest/src/test/java/bisq/apitest/scenario/WalletTest.java index 6fadf3cc251..0fff4bf694d 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/WalletTest.java +++ b/apitest/src/test/java/bisq/apitest/scenario/WalletTest.java @@ -67,6 +67,7 @@ public void testBtcWalletFunding(final TestInfo testInfo) { btcWalletTest.testInitialBtcBalances(testInfo); btcWalletTest.testFundAlicesBtcWallet(testInfo); + btcWalletTest.testAliceSendBTCToBob(testInfo); } @Test diff --git a/cli/src/main/java/bisq/cli/CliMain.java b/cli/src/main/java/bisq/cli/CliMain.java index dc9c1204584..5a98f9a7eb8 100644 --- a/cli/src/main/java/bisq/cli/CliMain.java +++ b/cli/src/main/java/bisq/cli/CliMain.java @@ -40,6 +40,7 @@ import bisq.proto.grpc.RegisterDisputeAgentRequest; import bisq.proto.grpc.RemoveWalletPasswordRequest; import bisq.proto.grpc.SendBsqRequest; +import bisq.proto.grpc.SendBtcRequest; import bisq.proto.grpc.SetTxFeeRatePreferenceRequest; import bisq.proto.grpc.SetWalletPasswordRequest; import bisq.proto.grpc.TakeOfferRequest; @@ -111,6 +112,7 @@ private enum Method { getfundingaddresses, getunusedbsqaddress, sendbsq, + sendbtc, gettxfeerate, settxfeerate, unsettxfeerate, @@ -276,6 +278,32 @@ public static void run(String[] args) { out.printf("%s bsq sent to %s in tx %s%n", amount, address, txInfo.getId()); return; } + case sendbtc: { + if (nonOptionArgs.size() < 2) + throw new IllegalArgumentException("no btc address specified"); + + var address = nonOptionArgs.get(1); + + if (nonOptionArgs.size() < 3) + throw new IllegalArgumentException("no btc amount specified"); + + var amount = nonOptionArgs.get(2); + verifyStringIsValidDecimal(amount); + + var txFeeRate = nonOptionArgs.size() == 4 ? nonOptionArgs.get(3) : ""; + if (!txFeeRate.isEmpty()) + verifyStringIsValidLong(txFeeRate); + + var request = SendBtcRequest.newBuilder() + .setAddress(address) + .setAmount(amount) + .setTxFeeRate(txFeeRate) + .build(); + var reply = walletsService.sendBtc(request); + TxInfo txInfo = reply.getTxInfo(); + out.printf("%s btc sent to %s in tx %s%n", amount, address, txInfo.getId()); + return; + } case gettxfeerate: { var request = GetTxFeeRateRequest.newBuilder().build(); var reply = walletsService.getTxFeeRate(request); @@ -679,6 +707,7 @@ private static void printHelp(OptionParser parser, PrintStream stream) { stream.format(rowFormat, "getfundingaddresses", "", "Get BTC funding addresses"); stream.format(rowFormat, "getunusedbsqaddress", "", "Get unused BSQ address"); stream.format(rowFormat, "sendbsq", "address, amount [,tx fee rate (sats/byte)]", "Send BSQ"); + stream.format(rowFormat, "sendbtc", "address, amount [,tx fee rate (sats/byte)]", "Send BTC"); stream.format(rowFormat, "gettxfeerate", "", "Get current tx fee rate in sats/byte"); stream.format(rowFormat, "settxfeerate", "satoshis (per byte)", "Set custom tx fee rate in sats/byte"); stream.format(rowFormat, "unsettxfeerate", "", "Unset custom tx fee rate"); diff --git a/core/src/main/java/bisq/core/api/CoreApi.java b/core/src/main/java/bisq/core/api/CoreApi.java index af8a5a4bcee..522fe8fb243 100644 --- a/core/src/main/java/bisq/core/api/CoreApi.java +++ b/core/src/main/java/bisq/core/api/CoreApi.java @@ -34,10 +34,13 @@ import bisq.common.handlers.ResultHandler; import org.bitcoinj.core.Coin; +import org.bitcoinj.core.Transaction; import javax.inject.Inject; import javax.inject.Singleton; +import com.google.common.util.concurrent.FutureCallback; + import java.util.ArrayList; import java.util.List; import java.util.Set; @@ -253,6 +256,13 @@ public void sendBsq(String address, walletsService.sendBsq(address, amount, txFeeRate, callback); } + public void sendBtc(String address, + String amount, + String txFeeRate, + FutureCallback callback) { + walletsService.sendBtc(address, amount, txFeeRate, callback); + } + public void getTxFeeRate(ResultHandler resultHandler) { walletsService.getTxFeeRate(resultHandler); } diff --git a/core/src/main/java/bisq/core/api/CoreWalletsService.java b/core/src/main/java/bisq/core/api/CoreWalletsService.java index 5d52d9f9022..2862f6c7e9b 100644 --- a/core/src/main/java/bisq/core/api/CoreWalletsService.java +++ b/core/src/main/java/bisq/core/api/CoreWalletsService.java @@ -23,7 +23,9 @@ import bisq.core.api.model.BtcBalanceInfo; import bisq.core.api.model.TxFeeRateInfo; import bisq.core.btc.Balances; +import bisq.core.btc.exceptions.AddressEntryException; import bisq.core.btc.exceptions.BsqChangeBelowDustException; +import bisq.core.btc.exceptions.InsufficientFundsException; import bisq.core.btc.exceptions.TransactionVerificationException; import bisq.core.btc.exceptions.WalletException; import bisq.core.btc.model.AddressEntry; @@ -35,7 +37,9 @@ import bisq.core.btc.wallet.WalletsManager; import bisq.core.provider.fee.FeeService; import bisq.core.user.Preferences; +import bisq.core.util.FormattingUtils; import bisq.core.util.coin.BsqFormatter; +import bisq.core.util.coin.CoinFormatter; import bisq.common.Timer; import bisq.common.UserThread; @@ -46,10 +50,12 @@ import org.bitcoinj.core.Coin; import org.bitcoinj.core.InsufficientMoneyException; import org.bitcoinj.core.LegacyAddress; +import org.bitcoinj.core.Transaction; import org.bitcoinj.core.TransactionConfidence; import org.bitcoinj.crypto.KeyCrypterScrypt; import javax.inject.Inject; +import javax.inject.Named; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; @@ -64,6 +70,7 @@ import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -85,6 +92,7 @@ class CoreWalletsService { private final BsqTransferService bsqTransferService; private final BsqFormatter bsqFormatter; private final BtcWalletService btcWalletService; + private final CoinFormatter btcFormatter; private final FeeService feeService; private final Preferences preferences; @@ -103,6 +111,7 @@ public CoreWalletsService(Balances balances, BsqTransferService bsqTransferService, BsqFormatter bsqFormatter, BtcWalletService btcWalletService, + @Named(FormattingUtils.BTC_FORMATTER_KEY) CoinFormatter btcFormatter, FeeService feeService, Preferences preferences) { this.balances = balances; @@ -111,6 +120,7 @@ public CoreWalletsService(Balances balances, this.bsqTransferService = bsqTransferService; this.bsqFormatter = bsqFormatter; this.btcWalletService = btcWalletService; + this.btcFormatter = btcFormatter; this.feeService = feeService; this.preferences = preferences; } @@ -191,25 +201,25 @@ void sendBsq(String address, String amount, String txFeeRate, TxBroadcaster.Callback callback) { + verifyWalletsAreAvailable(); + verifyEncryptedWalletIsUnlocked(); + try { LegacyAddress legacyAddress = getValidBsqLegacyAddress(address); - Coin receiverAmount = getValidBsqTransferAmount(amount); - - // A non txFeeRate String value overrides the fee service and custom fee. - Coin txFeePerVbyte = txFeeRate.isEmpty() - ? btcWalletService.getTxFeeForWithdrawalPerVbyte() - : Coin.valueOf(Long.parseLong(txFeeRate)); - + Coin receiverAmount = getValidTransferAmount(amount, bsqFormatter); + Coin txFeePerVbyte = getTxFeeRateFromParamOrPreferenceOrFeeService(txFeeRate); BsqTransferModel model = bsqTransferService.getBsqTransferModel(legacyAddress, receiverAmount, txFeePerVbyte); - log.info("Sending {} BSQ to {} with tx fee rate {} sats/byte).", + log.info("Sending {} BSQ to {} with tx fee rate {} sats/byte.", amount, address, txFeePerVbyte.value); bsqTransferService.sendFunds(model, callback); + } catch (InsufficientMoneyException ex) { + log.error("", ex); + throw new IllegalStateException("cannot send bsq due to insufficient funds", ex); } catch (NumberFormatException - | InsufficientMoneyException | BsqChangeBelowDustException | TransactionVerificationException | WalletException ex) { @@ -218,6 +228,60 @@ void sendBsq(String address, } } + void sendBtc(String address, + String amount, + String txFeeRate, + FutureCallback callback) { + verifyWalletsAreAvailable(); + verifyEncryptedWalletIsUnlocked(); + + try { + Set fromAddresses = btcWalletService.getAddressEntriesForAvailableBalanceStream() + .map(AddressEntry::getAddressString) + .collect(Collectors.toSet()); + Coin receiverAmount = getValidTransferAmount(amount, btcFormatter); + Coin txFeePerVbyte = getTxFeeRateFromParamOrPreferenceOrFeeService(txFeeRate); + + // TODO Support feeExcluded (or included), default is fee included. + // See WithdrawalView # onWithdraw (and refactor). + Transaction feeEstimationTransaction = + btcWalletService.getFeeEstimationTransactionForMultipleAddresses(fromAddresses, + receiverAmount, + txFeePerVbyte); + if (feeEstimationTransaction == null) + throw new IllegalStateException("could not estimate the transaction fee"); + + Coin dust = btcWalletService.getDust(feeEstimationTransaction); + Coin fee = feeEstimationTransaction.getFee().add(dust); + if (dust.isPositive()) { + fee = feeEstimationTransaction.getFee().add(dust); + log.info("Dust txo ({} sats) was detected, the dust amount has been added to the fee (was {}, now {})", + dust.value, + feeEstimationTransaction.getFee(), + fee.value); + } + log.info("Sending {} BTC to {} with tx fee of {} sats (fee rate {} sats/byte).", + amount, + address, + fee.value, + txFeePerVbyte.value); + btcWalletService.sendFundsForMultipleAddresses(fromAddresses, + address, + receiverAmount, + fee, + null, + tempAesKey, + null, /* memo todo */ + callback); + } catch (AddressEntryException ex) { + log.error("", ex); + throw new IllegalStateException("cannot send btc from any addresses in wallet", ex); + } catch (InsufficientFundsException | InsufficientMoneyException ex) { + log.error("", ex); + throw new IllegalStateException("cannot send btc due to insufficient funds", ex); + } + } + void getTxFeeRate(ResultHandler resultHandler) { try { @SuppressWarnings({"unchecked", "Convert2MethodRef"}) @@ -437,15 +501,22 @@ private LegacyAddress getValidBsqLegacyAddress(String address) { } } - // Returns a Coin for the amount string, or a RuntimeException if invalid. - private Coin getValidBsqTransferAmount(String amount) { - Coin amountAsCoin = parseToCoin(amount, bsqFormatter); + // Returns a Coin for the transfer amount string, or a RuntimeException if invalid. + private Coin getValidTransferAmount(String amount, CoinFormatter coinFormatter) { + Coin amountAsCoin = parseToCoin(amount, coinFormatter); if (amountAsCoin.isLessThan(getMinNonDustOutput())) - throw new IllegalStateException(format("%s bsq is an invalid send amount", amount)); + throw new IllegalStateException(format("%s is an invalid transfer amount", amount)); return amountAsCoin; } + private Coin getTxFeeRateFromParamOrPreferenceOrFeeService(String txFeeRate) { + // A non txFeeRate String value overrides the fee service and custom fee. + return txFeeRate.isEmpty() + ? btcWalletService.getTxFeeForWithdrawalPerVbyte() + : Coin.valueOf(Long.parseLong(txFeeRate)); + } + private KeyCrypterScrypt getKeyCrypterScrypt() { KeyCrypterScrypt keyCrypterScrypt = walletsManager.getKeyCrypterScrypt(); if (keyCrypterScrypt == null) diff --git a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java index f6b7ac05f4b..2d534caf4e3 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -1023,6 +1023,14 @@ public Transaction getFeeEstimationTransaction(String fromAddress, public Transaction getFeeEstimationTransactionForMultipleAddresses(Set fromAddresses, Coin amount) throws AddressFormatException, AddressEntryException, InsufficientFundsException { + Coin txFeeForWithdrawalPerVbyte = getTxFeeForWithdrawalPerVbyte(); + return getFeeEstimationTransactionForMultipleAddresses(fromAddresses, amount, txFeeForWithdrawalPerVbyte); + } + + public Transaction getFeeEstimationTransactionForMultipleAddresses(Set fromAddresses, + Coin amount, + Coin txFeeForWithdrawalPerVbyte) + throws AddressFormatException, AddressEntryException, InsufficientFundsException { Set addressEntries = fromAddresses.stream() .map(address -> { Optional addressEntryOptional = findAddressEntry(address, AddressEntry.Context.AVAILABLE); @@ -1045,7 +1053,6 @@ public Transaction getFeeEstimationTransactionForMultipleAddresses(Set f int counter = 0; int txVsize = 0; Transaction tx; - Coin txFeeForWithdrawalPerVbyte = getTxFeeForWithdrawalPerVbyte(); do { counter++; fee = txFeeForWithdrawalPerVbyte.multiply(txVsize); @@ -1072,7 +1079,11 @@ public Transaction getFeeEstimationTransactionForMultipleAddresses(Set f } private boolean feeEstimationNotSatisfied(int counter, Transaction tx) { - long targetFee = getTxFeeForWithdrawalPerVbyte().multiply(tx.getVsize()).value; + return feeEstimationNotSatisfied(counter, tx, getTxFeeForWithdrawalPerVbyte()); + } + + private boolean feeEstimationNotSatisfied(int counter, Transaction tx, Coin txFeeForWithdrawalPerVbyte) { + long targetFee = txFeeForWithdrawalPerVbyte.multiply(tx.getVsize()).value; return counter < 10 && (tx.getFee().value < targetFee || tx.getFee().value - targetFee > 1000); diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java index c38d5209d1b..b902e805edd 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java @@ -39,6 +39,8 @@ import bisq.proto.grpc.RemoveWalletPasswordRequest; import bisq.proto.grpc.SendBsqReply; import bisq.proto.grpc.SendBsqRequest; +import bisq.proto.grpc.SendBtcReply; +import bisq.proto.grpc.SendBtcRequest; import bisq.proto.grpc.SetTxFeeRatePreferenceReply; import bisq.proto.grpc.SetTxFeeRatePreferenceRequest; import bisq.proto.grpc.SetWalletPasswordReply; @@ -57,11 +59,15 @@ import javax.inject.Inject; +import com.google.common.util.concurrent.FutureCallback; + import java.util.List; import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; +import org.jetbrains.annotations.NotNull; + import static bisq.core.api.model.TxInfo.toTxInfo; @Slf4j @@ -177,6 +183,45 @@ public void onFailure(TxBroadcastException ex) { } } + @Override + public void sendBtc(SendBtcRequest req, + StreamObserver responseObserver) { + try { + coreApi.sendBtc(req.getAddress(), + req.getAmount(), + req.getTxFeeRate(), + new FutureCallback<>() { + @Override + public void onSuccess(Transaction tx) { + if (tx != null) { + log.info("Successfully published BTC tx: id {}, output sum {} sats, fee {} sats, size {} bytes", + tx.getTxId().toString(), + tx.getOutputSum(), + tx.getFee(), + tx.getMessageSize()); + var reply = SendBtcReply.newBuilder() + .setTxInfo(toTxInfo(tx).toProtoMessage()) + .build(); + responseObserver.onNext(reply); + responseObserver.onCompleted(); + } else { + throw new IllegalStateException("btc transaction is null"); + } + } + + @Override + public void onFailure(@NotNull Throwable t) { + log.error("", t); + throw new IllegalStateException(t); + } + }); + } catch (IllegalStateException | IllegalArgumentException cause) { + var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); + responseObserver.onError(ex); + throw ex; + } + } + @Override public void getTxFeeRate(GetTxFeeRateRequest req, StreamObserver responseObserver) { diff --git a/proto/src/main/proto/grpc.proto b/proto/src/main/proto/grpc.proto index b4950b96a25..7bfaf219f73 100644 --- a/proto/src/main/proto/grpc.proto +++ b/proto/src/main/proto/grpc.proto @@ -318,6 +318,8 @@ service Wallets { } rpc SendBsq (SendBsqRequest) returns (SendBsqReply) { } + rpc SendBtc (SendBtcRequest) returns (SendBtcReply) { + } rpc GetTxFeeRate (GetTxFeeRateRequest) returns (GetTxFeeRateReply) { } rpc SetTxFeeRatePreference (SetTxFeeRatePreferenceRequest) returns (SetTxFeeRatePreferenceReply) { @@ -369,6 +371,16 @@ message SendBsqReply { TxInfo txInfo = 1; } +message SendBtcRequest { + string address = 1; + string amount = 2; + string txFeeRate = 3; +} + +message SendBtcReply { + TxInfo txInfo = 1; +} + message GetTxFeeRateRequest { } From bd66008062027b3677c276f5bbf99a8d1b2bc9d2 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Wed, 9 Dec 2020 16:51:56 -0300 Subject: [PATCH 04/20] Support tx memo field for btc withdrawals from api - Added optional memo parameter to the api's sendbtc and withdrawfunds commands. - Removed the @Nullable annotation was removed because protobuf does not support null. - Visibility in two wallet check methods were changed from private to pkg protected so the CoreTradeService could use them. - Adjusted affected tests. (Asserting the memo field was set on a transaction cannot be checked from apitest yet.) --- .../java/bisq/apitest/method/MethodTest.java | 26 ++++++++++++------- .../method/trade/AbstractTradeTest.java | 10 ++++--- .../method/trade/TakeSellBTCOfferTest.java | 4 +-- .../apitest/method/wallet/BtcWalletTest.java | 6 ++++- cli/src/main/java/bisq/cli/CliMain.java | 20 ++++++++++---- core/src/main/java/bisq/core/api/CoreApi.java | 7 +++-- .../java/bisq/core/api/CoreTradesService.java | 25 ++++++++++++------ .../bisq/core/api/CoreWalletsService.java | 9 ++++--- .../bisq/daemon/grpc/GrpcTradesService.java | 6 +++-- .../bisq/daemon/grpc/GrpcWalletsService.java | 1 + proto/src/main/proto/grpc.proto | 2 ++ 11 files changed, 77 insertions(+), 39 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/method/MethodTest.java b/apitest/src/test/java/bisq/apitest/method/MethodTest.java index f64c2a5fe92..95918189424 100644 --- a/apitest/src/test/java/bisq/apitest/method/MethodTest.java +++ b/apitest/src/test/java/bisq/apitest/method/MethodTest.java @@ -174,11 +174,13 @@ protected final SendBsqRequest createSendBsqRequest(String address, protected final SendBtcRequest createSendBtcRequest(String address, String amount, - String txFeeRate) { + String txFeeRate, + String memo) { return SendBtcRequest.newBuilder() .setAddress(address) .setAmount(amount) .setTxFeeRate(txFeeRate) + .setMemo(memo) .build(); } @@ -226,10 +228,13 @@ protected final KeepFundsRequest createKeepFundsRequest(String tradeId) { .build(); } - protected final WithdrawFundsRequest createWithdrawFundsRequest(String tradeId, String address) { + protected final WithdrawFundsRequest createWithdrawFundsRequest(String tradeId, + String address, + String memo) { return WithdrawFundsRequest.newBuilder() .setTradeId(tradeId) .setAddress(address) + .setMemo(memo) .build(); } @@ -283,17 +288,17 @@ protected final TxInfo sendBsq(BisqAppConfig bisqAppConfig, } protected final TxInfo sendBtc(BisqAppConfig bisqAppConfig, String address, String amount) { - return sendBtc(bisqAppConfig, address, amount, ""); + return sendBtc(bisqAppConfig, address, amount, "", ""); } protected final TxInfo sendBtc(BisqAppConfig bisqAppConfig, String address, String amount, - String txFeeRate) { + String txFeeRate, + String memo) { //noinspection ResultOfMethodCallIgnored - return grpcStubs(bisqAppConfig).walletsService.sendBtc(createSendBtcRequest(address, - amount, - txFeeRate)) + return grpcStubs(bisqAppConfig).walletsService.sendBtc( + createSendBtcRequest(address, amount, txFeeRate, memo)) .getTxInfo(); } @@ -399,8 +404,11 @@ protected final void keepFunds(BisqAppConfig bisqAppConfig, String tradeId) { } @SuppressWarnings("ResultOfMethodCallIgnored") - protected final void withdrawFunds(BisqAppConfig bisqAppConfig, String tradeId, String address) { - var req = createWithdrawFundsRequest(tradeId, address); + protected final void withdrawFunds(BisqAppConfig bisqAppConfig, + String tradeId, + String address, + String memo) { + var req = createWithdrawFundsRequest(tradeId, address, memo); grpcStubs(bisqAppConfig).tradesService.withdrawFunds(req); } 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 2b88e4f2700..7f4038a0344 100644 --- a/apitest/src/test/java/bisq/apitest/method/trade/AbstractTradeTest.java +++ b/apitest/src/test/java/bisq/apitest/method/trade/AbstractTradeTest.java @@ -64,9 +64,11 @@ protected final void logTrade(Logger log, TestInfo testInfo, String description, TradeInfo trade) { - log.info(String.format("%s %s%n%s", - testName(testInfo), - description.toUpperCase(), - format(trade))); + if (log.isDebugEnabled()) { + log.debug(String.format("%s %s%n%s", + testName(testInfo), + description.toUpperCase(), + format(trade))); + } } } 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 2278ce315cd..69445657416 100644 --- a/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBTCOfferTest.java +++ b/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBTCOfferTest.java @@ -147,7 +147,7 @@ public void testBobsBtcWithdrawalToExternalAddress(final TestInfo testInfo) { logTrade(log, testInfo, "Bob's view before withdrawing funds to external wallet", trade); String toAddress = bitcoinCli.getNewBtcAddress(); - withdrawFunds(bobdaemon, tradeId, toAddress); + withdrawFunds(bobdaemon, tradeId, toAddress, "to whom it may concern"); genBtcBlocksThenWait(1, 2250); @@ -158,7 +158,7 @@ public void testBobsBtcWithdrawalToExternalAddress(final TestInfo testInfo) { verifyExpectedProtocolStatus(trade); logTrade(log, testInfo, "Bob's view after withdrawing funds to external wallet", trade); BtcBalanceInfo currentBalance = getBtcBalances(bobdaemon); - log.info("{} Bob's current available balance: {} BTC", + log.debug("{} Bob's current available balance: {} BTC", testName(testInfo), formatSatoshis(currentBalance.getAvailableBalance())); } diff --git a/apitest/src/test/java/bisq/apitest/method/wallet/BtcWalletTest.java b/apitest/src/test/java/bisq/apitest/method/wallet/BtcWalletTest.java index a97e9a690da..7af31dcb3fc 100644 --- a/apitest/src/test/java/bisq/apitest/method/wallet/BtcWalletTest.java +++ b/apitest/src/test/java/bisq/apitest/method/wallet/BtcWalletTest.java @@ -99,7 +99,11 @@ public void testAliceSendBTCToBob(TestInfo testInfo) { String bobsBtcAddress = getUnusedBtcAddress(bobdaemon); log.debug("Sending 5.5 BTC From Alice to Bob @ {}", bobsBtcAddress); - sendBtc(alicedaemon, bobsBtcAddress, "5.50", "100"); + sendBtc(alicedaemon, + bobsBtcAddress, + "5.50", + "100", + "to whom it may concern"); genBtcBlocksThenWait(1, 3000); BtcBalanceInfo alicesBalances = getBtcBalances(alicedaemon); diff --git a/cli/src/main/java/bisq/cli/CliMain.java b/cli/src/main/java/bisq/cli/CliMain.java index 5a98f9a7eb8..b9cb4ede0fd 100644 --- a/cli/src/main/java/bisq/cli/CliMain.java +++ b/cli/src/main/java/bisq/cli/CliMain.java @@ -290,14 +290,18 @@ public static void run(String[] args) { var amount = nonOptionArgs.get(2); verifyStringIsValidDecimal(amount); - var txFeeRate = nonOptionArgs.size() == 4 ? nonOptionArgs.get(3) : ""; + // TODO Find a better way to handle the two optional parameters. + var txFeeRate = nonOptionArgs.size() >= 4 ? nonOptionArgs.get(3) : ""; if (!txFeeRate.isEmpty()) verifyStringIsValidLong(txFeeRate); + var memo = nonOptionArgs.size() == 5 ? nonOptionArgs.get(4) : ""; + var request = SendBtcRequest.newBuilder() .setAddress(address) .setAmount(amount) .setTxFeeRate(txFeeRate) + .setMemo(memo) .build(); var reply = walletsService.sendBtc(request); TxInfo txInfo = reply.getTxInfo(); @@ -496,16 +500,21 @@ public static void run(String[] args) { case withdrawfunds: { if (nonOptionArgs.size() < 3) throw new IllegalArgumentException("incorrect parameter count, " - + " expecting trade id, bitcoin wallet address"); + + " expecting trade id, bitcoin wallet address [,\"memo\"]"); var tradeId = nonOptionArgs.get(1); var address = nonOptionArgs.get(2); + // A multi-word memo must be double quoted. + var memo = nonOptionArgs.size() == 4 + ? nonOptionArgs.get(3) + : ""; var request = WithdrawFundsRequest.newBuilder() .setTradeId(tradeId) .setAddress(address) + .setMemo(memo) .build(); tradesService.withdrawFunds(request); - out.printf("funds from trade %s sent to btc address %s%n", tradeId, address); + out.printf("trade %s funds sent to btc address %s%n", tradeId, address); return; } case getpaymentmethods: { @@ -707,7 +716,7 @@ private static void printHelp(OptionParser parser, PrintStream stream) { stream.format(rowFormat, "getfundingaddresses", "", "Get BTC funding addresses"); stream.format(rowFormat, "getunusedbsqaddress", "", "Get unused BSQ address"); stream.format(rowFormat, "sendbsq", "address, amount [,tx fee rate (sats/byte)]", "Send BSQ"); - stream.format(rowFormat, "sendbtc", "address, amount [,tx fee rate (sats/byte)]", "Send BTC"); + stream.format(rowFormat, "sendbtc", "address, amount [,tx fee rate (sats/byte), \"memo\"]", "Send BTC"); stream.format(rowFormat, "gettxfeerate", "", "Get current tx fee rate in sats/byte"); stream.format(rowFormat, "settxfeerate", "satoshis (per byte)", "Set custom tx fee rate in sats/byte"); stream.format(rowFormat, "unsettxfeerate", "", "Unset custom tx fee rate"); @@ -723,7 +732,8 @@ private static void printHelp(OptionParser parser, PrintStream stream) { stream.format(rowFormat, "confirmpaymentstarted", "trade id", "Confirm payment started"); stream.format(rowFormat, "confirmpaymentreceived", "trade id", "Confirm payment received"); stream.format(rowFormat, "keepfunds", "trade id", "Keep received funds in Bisq wallet"); - stream.format(rowFormat, "withdrawfunds", "trade id, bitcoin wallet address", "Withdraw received funds to external wallet address"); + stream.format(rowFormat, "withdrawfunds", "trade id, bitcoin wallet address [,\"memo\"]", + "Withdraw received funds to external wallet address"); stream.format(rowFormat, "getpaymentmethods", "", "Get list of supported payment account method ids"); stream.format(rowFormat, "getpaymentacctform", "payment method id", "Get a new payment account form"); stream.format(rowFormat, "createpaymentacct", "path to payment account form", "Create a new payment account"); diff --git a/core/src/main/java/bisq/core/api/CoreApi.java b/core/src/main/java/bisq/core/api/CoreApi.java index 522fe8fb243..dbba310f5dd 100644 --- a/core/src/main/java/bisq/core/api/CoreApi.java +++ b/core/src/main/java/bisq/core/api/CoreApi.java @@ -48,8 +48,6 @@ import lombok.extern.slf4j.Slf4j; -import javax.annotation.Nullable; - /** * Provides high level interface to functionality of core Bisq features. * E.g. useful for different APIs to access data of different domains of Bisq. @@ -213,7 +211,7 @@ public void keepFunds(String tradeId) { coreTradesService.keepFunds(tradeId); } - public void withdrawFunds(String tradeId, String address, @Nullable String memo) { + public void withdrawFunds(String tradeId, String address, String memo) { coreTradesService.withdrawFunds(tradeId, address, memo); } @@ -259,8 +257,9 @@ public void sendBsq(String address, public void sendBtc(String address, String amount, String txFeeRate, + String memo, FutureCallback callback) { - walletsService.sendBtc(address, amount, txFeeRate, callback); + walletsService.sendBtc(address, amount, txFeeRate, memo, callback); } public void getTxFeeRate(ResultHandler resultHandler) { diff --git a/core/src/main/java/bisq/core/api/CoreTradesService.java b/core/src/main/java/bisq/core/api/CoreTradesService.java index 2f75f241407..10d21d6415d 100644 --- a/core/src/main/java/bisq/core/api/CoreTradesService.java +++ b/core/src/main/java/bisq/core/api/CoreTradesService.java @@ -41,8 +41,6 @@ import lombok.extern.slf4j.Slf4j; -import javax.annotation.Nullable; - import static bisq.core.btc.model.AddressEntry.Context.TRADE_PAYOUT; import static java.lang.String.format; @@ -85,6 +83,8 @@ void takeOffer(Offer offer, String paymentAccountId, String takerFeeCurrencyCode, Consumer resultHandler) { + coreWalletsService.verifyWalletsAreAvailable(); + coreWalletsService.verifyEncryptedWalletIsUnlocked(); offerUtil.maybeSetFeePaymentCurrencyPreference(takerFeeCurrencyCode); @@ -149,6 +149,9 @@ void confirmPaymentReceived(String tradeId) { } void keepFunds(String tradeId) { + coreWalletsService.verifyWalletsAreAvailable(); + coreWalletsService.verifyEncryptedWalletIsUnlocked(); + verifyTradeIsNotClosed(tradeId); var trade = getOpenTrade(tradeId).orElseThrow(() -> new IllegalArgumentException(format("trade with id '%s' not found", tradeId))); @@ -156,8 +159,10 @@ void keepFunds(String tradeId) { tradeManager.onTradeCompleted(trade); } - void withdrawFunds(String tradeId, String toAddress, @Nullable String memo) { - // An encrypted wallet must be unlocked for this operation. + void withdrawFunds(String tradeId, String toAddress, String memo) { + coreWalletsService.verifyWalletsAreAvailable(); + coreWalletsService.verifyEncryptedWalletIsUnlocked(); + verifyTradeIsNotClosed(tradeId); var trade = getOpenTrade(tradeId).orElseThrow(() -> new IllegalArgumentException(format("trade with id '%s' not found", tradeId))); @@ -172,21 +177,21 @@ void withdrawFunds(String tradeId, String toAddress, @Nullable String memo) { var receiverAmount = amount.subtract(fee); log.info(format("Withdrawing funds received from trade %s:" - + "%n From %s%n To %s%n Amt %s%n Tx Fee %s%n Receiver Amt %s", + + "%n From %s%n To %s%n Amt %s%n Tx Fee %s%n Receiver Amt %s%n Memo %s%n", tradeId, fromAddressEntry.getAddressString(), toAddress, amount.toFriendlyString(), fee.toFriendlyString(), - receiverAmount.toFriendlyString())); - + receiverAmount.toFriendlyString(), + memo)); tradeManager.onWithdrawRequest( toAddress, amount, fee, coreWalletsService.getKey(), trade, - memo, + memo.isEmpty() ? null : memo, () -> { }, (errorMessage, throwable) -> { @@ -196,10 +201,14 @@ void withdrawFunds(String tradeId, String toAddress, @Nullable String memo) { } String getTradeRole(String tradeId) { + coreWalletsService.verifyWalletsAreAvailable(); + coreWalletsService.verifyEncryptedWalletIsUnlocked(); return tradeUtil.getRole(getTrade(tradeId)); } Trade getTrade(String tradeId) { + coreWalletsService.verifyWalletsAreAvailable(); + coreWalletsService.verifyEncryptedWalletIsUnlocked(); return getOpenTrade(tradeId).orElseGet(() -> getClosedTrade(tradeId).orElseThrow(() -> new IllegalArgumentException(format("trade with id '%s' not found", tradeId)) diff --git a/core/src/main/java/bisq/core/api/CoreWalletsService.java b/core/src/main/java/bisq/core/api/CoreWalletsService.java index 2862f6c7e9b..2bbd585f8dd 100644 --- a/core/src/main/java/bisq/core/api/CoreWalletsService.java +++ b/core/src/main/java/bisq/core/api/CoreWalletsService.java @@ -231,6 +231,7 @@ void sendBsq(String address, void sendBtc(String address, String amount, String txFeeRate, + String memo, FutureCallback callback) { verifyWalletsAreAvailable(); verifyEncryptedWalletIsUnlocked(); @@ -271,7 +272,7 @@ void sendBtc(String address, fee, null, tempAesKey, - null, /* memo todo */ + memo.isEmpty() ? null : memo, callback); } catch (AddressEntryException ex) { log.error("", ex); @@ -420,13 +421,13 @@ void removeWalletPassword(String password) { } // Throws a RuntimeException if wallets are not available (encrypted or not). - private void verifyWalletsAreAvailable() { + void verifyWalletsAreAvailable() { if (!walletsManager.areWalletsAvailable()) throw new IllegalStateException("wallet is not yet available"); } // Throws a RuntimeException if wallets are not available or not encrypted. - private void verifyWalletIsAvailableAndEncrypted() { + void verifyWalletIsAvailableAndEncrypted() { if (!walletsManager.areWalletsAvailable()) throw new IllegalStateException("wallet is not yet available"); @@ -435,7 +436,7 @@ private void verifyWalletIsAvailableAndEncrypted() { } // Throws a RuntimeException if wallets are encrypted and locked. - private void verifyEncryptedWalletIsUnlocked() { + void verifyEncryptedWalletIsUnlocked() { if (walletsManager.areWalletsEncrypted() && tempAesKey == null) throw new IllegalStateException("wallet is locked"); } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java index 1dbb453a22a..8c04f67b059 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java @@ -39,11 +39,14 @@ import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; +import org.bitcoinj.core.Transaction; + import javax.inject.Inject; import lombok.extern.slf4j.Slf4j; import static bisq.core.api.model.TradeInfo.toTradeInfo; +import static bisq.core.api.model.TxInfo.toTxInfo; @Slf4j class GrpcTradesService extends TradesGrpc.TradesImplBase { @@ -144,8 +147,7 @@ public void keepFunds(KeepFundsRequest req, public void withdrawFunds(WithdrawFundsRequest req, StreamObserver responseObserver) { try { - //TODO @ghubstan Feel free to add a memo param for withdrawal requests (was just added in UI) - coreApi.withdrawFunds(req.getTradeId(), req.getAddress(), null); + coreApi.withdrawFunds(req.getTradeId(), req.getAddress(), req.getMemo()); var reply = WithdrawFundsReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java index b902e805edd..c7bbb4623da 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java @@ -190,6 +190,7 @@ public void sendBtc(SendBtcRequest req, coreApi.sendBtc(req.getAddress(), req.getAmount(), req.getTxFeeRate(), + req.getMemo(), new FutureCallback<>() { @Override public void onSuccess(Transaction tx) { diff --git a/proto/src/main/proto/grpc.proto b/proto/src/main/proto/grpc.proto index 7bfaf219f73..a6e30cdb473 100644 --- a/proto/src/main/proto/grpc.proto +++ b/proto/src/main/proto/grpc.proto @@ -255,6 +255,7 @@ message KeepFundsReply { message WithdrawFundsRequest { string tradeId = 1; string address = 2; + string memo = 3; } message WithdrawFundsReply { @@ -375,6 +376,7 @@ message SendBtcRequest { string address = 1; string amount = 2; string txFeeRate = 3; + string memo = 4; } message SendBtcReply { From 478c8f4eb23c510a06e0fdde1be9342684005902 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Wed, 9 Dec 2020 17:38:20 -0300 Subject: [PATCH 05/20] Remove unused imports --- daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java index 8c04f67b059..1028c0cf9d7 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java @@ -39,14 +39,11 @@ import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; -import org.bitcoinj.core.Transaction; - import javax.inject.Inject; import lombok.extern.slf4j.Slf4j; import static bisq.core.api.model.TradeInfo.toTradeInfo; -import static bisq.core.api.model.TxInfo.toTxInfo; @Slf4j class GrpcTradesService extends TradesGrpc.TradesImplBase { From 150e2f685126e7ab7f7501d3f212ce5fa58452fd Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Fri, 11 Dec 2020 18:33:19 -0300 Subject: [PATCH 06/20] Use Bisq's UserThread.executor in gRPC server --- daemon/src/main/java/bisq/daemon/grpc/GrpcServer.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcServer.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcServer.java index bb9dbebd273..40efdd07ac2 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcServer.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcServer.java @@ -17,6 +17,7 @@ package bisq.daemon.grpc; +import bisq.common.UserThread; import bisq.common.config.Config; import io.grpc.Server; @@ -48,6 +49,7 @@ public GrpcServer(Config config, GrpcTradesService tradesService, GrpcWalletsService walletsService) { this.server = ServerBuilder.forPort(config.apiPort) + .executor(UserThread.getExecutor()) .addService(disputeAgentsService) .addService(offersService) .addService(paymentAccountsService) From 6aa385e494eb8fa870257c76e078108607503d03 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Sat, 12 Dec 2020 12:01:55 -0300 Subject: [PATCH 07/20] Append nullable withdrawalTxId field to Trade proto message The withdrawalTxId field will be set in TradeManager#onWithdrawRequest upon successful trade completion and withdrawal of funds. Persisting withdrawalTxId will allow the api and ui to find the withdrawalTxId for a completed trade after the seller withdraws funds to an external wallet. In turn, the withdrawal tx's memo field will be accessible in a new (todo) api getTx(txID) api method. Changed: - Appended field 'string withdrawal_tx_id = 40' to pb.proto's Trade message. - Added nullable 'String withdrawalTxId' to Trade entity class. - Added trade.setWithdrawalTxId(transaction.getTxId().toString()) in TradeManager#onWithdrawRequest's callback. --- core/src/main/java/bisq/core/trade/Trade.java | 7 +++++++ core/src/main/java/bisq/core/trade/TradeManager.java | 1 + proto/src/main/proto/pb.proto | 1 + 3 files changed, 9 insertions(+) diff --git a/core/src/main/java/bisq/core/trade/Trade.java b/core/src/main/java/bisq/core/trade/Trade.java index de5c1949bb7..b25587eba97 100644 --- a/core/src/main/java/bisq/core/trade/Trade.java +++ b/core/src/main/java/bisq/core/trade/Trade.java @@ -317,6 +317,10 @@ public static protobuf.Trade.TradePeriodState toProtoMessage(Trade.TradePeriodSt @Getter @Setter private String payoutTxId; + @Nullable + @Getter + @Setter + private String withdrawalTxId; @Getter @Setter private long tradeAmountAsLong; @@ -554,6 +558,7 @@ public Message toProtoMessage() { Optional.ofNullable(takerFeeTxId).ifPresent(builder::setTakerFeeTxId); Optional.ofNullable(depositTxId).ifPresent(builder::setDepositTxId); Optional.ofNullable(payoutTxId).ifPresent(builder::setPayoutTxId); + Optional.ofNullable(withdrawalTxId).ifPresent(builder::setWithdrawalTxId); Optional.ofNullable(tradingPeerNodeAddress).ifPresent(e -> builder.setTradingPeerNodeAddress(tradingPeerNodeAddress.toProtoMessage())); Optional.ofNullable(contract).ifPresent(e -> builder.setContract(contract.toProtoMessage())); Optional.ofNullable(contractAsJson).ifPresent(builder::setContractAsJson); @@ -587,6 +592,7 @@ public static Trade fromProto(Trade trade, protobuf.Trade proto, CoreProtoResolv trade.setTakerFeeTxId(ProtoUtil.stringOrNullFromProto(proto.getTakerFeeTxId())); trade.setDepositTxId(ProtoUtil.stringOrNullFromProto(proto.getDepositTxId())); trade.setPayoutTxId(ProtoUtil.stringOrNullFromProto(proto.getPayoutTxId())); + trade.setWithdrawalTxId(ProtoUtil.stringOrNullFromProto(proto.getWithdrawalTxId())); trade.setContract(proto.hasContract() ? Contract.fromProto(proto.getContract(), coreProtoResolver) : null); trade.setContractAsJson(ProtoUtil.stringOrNullFromProto(proto.getContractAsJson())); trade.setContractHash(ProtoUtil.byteArrayOrNullFromProto(proto.getContractHash())); @@ -1124,6 +1130,7 @@ public String toString() { ",\n takerFeeTxId='" + takerFeeTxId + '\'' + ",\n depositTxId='" + depositTxId + '\'' + ",\n payoutTxId='" + payoutTxId + '\'' + + ",\n withdrawalTxId='" + withdrawalTxId + '\'' + ",\n tradeAmountAsLong=" + tradeAmountAsLong + ",\n tradePrice=" + tradePrice + ",\n tradingPeerNodeAddress=" + tradingPeerNodeAddress + diff --git a/core/src/main/java/bisq/core/trade/TradeManager.java b/core/src/main/java/bisq/core/trade/TradeManager.java index dc87d4a98cc..d6c99e28e3e 100644 --- a/core/src/main/java/bisq/core/trade/TradeManager.java +++ b/core/src/main/java/bisq/core/trade/TradeManager.java @@ -497,6 +497,7 @@ public void onSuccess(@javax.annotation.Nullable Transaction transaction) { onTradeCompleted(trade); trade.setState(Trade.State.WITHDRAW_COMPLETED); getTradeProtocol(trade).onWithdrawCompleted(); + trade.setWithdrawalTxId(transaction.getTxId().toString()); requestPersistence(); resultHandler.handleResult(); } diff --git a/proto/src/main/proto/pb.proto b/proto/src/main/proto/pb.proto index 22d81d40de9..a4579016f0c 100644 --- a/proto/src/main/proto/pb.proto +++ b/proto/src/main/proto/pb.proto @@ -1450,6 +1450,7 @@ message Trade { string counter_currency_extra_data = 37; string asset_tx_proof_result = 38; // name of AssetTxProofResult enum string uid = 39; + string withdrawal_tx_id = 40; } message BuyerAsMakerTrade { From 5522d0c53e96312a75dbb88da72bec8480c97bc8 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 14 Dec 2020 15:01:03 -0300 Subject: [PATCH 08/20] Add new api method gettransaction This change was prompted by the recent changes in the main branch to allow a tx memo field to be set from the UI and API. This and the prior PR address the API's need to be able to fetch a tx (with a memo). The API can now get a completed trade's withdrawal txid and pass it as a gettransaction parameter. See previous PR "Append nullable withdrawalTxId field to Trade". https://github.com/bisq-network/bisq/pull/4937 A summary of changes by file: grpc.proto - Added withdrawalTxId field to existing TradeInfo proto & wrapper. - Reordered fields in TradeInfo proto. - Added new fields to be displayed by TxInfo proto in CLI. - Fixed typo: unsetTxFeeRatePreference -> UnsetTxFeeRatePreference. - Added new GetTransaction rpc. GrpcWalletsService - Added new getTransaction gRPC boilerplate. CoreWalletsService - Added new getTransaction implementation. TxInfo - Added the new fields for displaying a tx summary from CLI. This is not intended to be more than a brief summary; a block explorer or bitcoin-core client should be used to see the complete definition. TradeInfo - Added the new withdrawalTxId field defined in grpc.proto. CliMain - Added new 'case gettransaction'. TransactionFormat - Formats a TxInfo sent from the server to CLI. ColumnHeaderConstants - Added console headers used by TransactionFormat. TradeFormat - Displays a completed trade's WithdrawalTxId if present. Apitest - Adjusted affected tests: assert tx memo is persisted and test gettransaction. --- .../java/bisq/apitest/method/MethodTest.java | 6 + .../payment/CreatePaymentAccountTest.java | 7 +- .../method/trade/TakeSellBTCOfferTest.java | 23 +++- .../apitest/method/wallet/BtcWalletTest.java | 18 ++- .../java/bisq/apitest/scenario/TradeTest.java | 1 + cli/src/main/java/bisq/cli/CliMain.java | 27 ++++- .../java/bisq/cli/ColumnHeaderConstants.java | 10 ++ cli/src/main/java/bisq/cli/TradeFormat.java | 33 ++--- .../main/java/bisq/cli/TransactionFormat.java | 59 +++++++++ core/src/main/java/bisq/core/api/CoreApi.java | 4 + .../bisq/core/api/CoreWalletsService.java | 20 +++ .../java/bisq/core/api/model/TradeInfo.java | 11 ++ .../main/java/bisq/core/api/model/TxInfo.java | 114 +++++++++++++++--- .../bisq/daemon/grpc/GrpcWalletsService.java | 19 +++ proto/src/main/proto/grpc.proto | 50 +++++--- 15 files changed, 341 insertions(+), 61 deletions(-) create mode 100644 cli/src/main/java/bisq/cli/TransactionFormat.java diff --git a/apitest/src/test/java/bisq/apitest/method/MethodTest.java b/apitest/src/test/java/bisq/apitest/method/MethodTest.java index 95918189424..791e22ef6c2 100644 --- a/apitest/src/test/java/bisq/apitest/method/MethodTest.java +++ b/apitest/src/test/java/bisq/apitest/method/MethodTest.java @@ -39,6 +39,7 @@ import bisq.proto.grpc.GetPaymentAccountsRequest; import bisq.proto.grpc.GetPaymentMethodsRequest; import bisq.proto.grpc.GetTradeRequest; +import bisq.proto.grpc.GetTransactionRequest; import bisq.proto.grpc.GetTxFeeRateRequest; import bisq.proto.grpc.GetUnusedBsqAddressRequest; import bisq.proto.grpc.KeepFundsRequest; @@ -432,6 +433,11 @@ protected final TxFeeRateInfo unsetTxFeeRate(BisqAppConfig bisqAppConfig) { grpcStubs(bisqAppConfig).walletsService.unsetTxFeeRatePreference(req).getTxFeeRateInfo()); } + protected final TxInfo getTransaction(BisqAppConfig bisqAppConfig, String txId) { + var req = GetTransactionRequest.newBuilder().setTxId(txId).build(); + return grpcStubs(bisqAppConfig).walletsService.getTransaction(req).getTxInfo(); + } + // Static conveniences for test methods and test case fixture setups. protected static RegisterDisputeAgentRequest createRegisterDisputeAgentRequest(String disputeAgentType) { diff --git a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java index 9795eec28c4..ac66cc7993e 100644 --- a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java @@ -746,7 +746,12 @@ public void testCreateTransferwiseAccount(TestInfo testInfo) { String jsonString = getCompletedFormAsJsonString(); TransferwiseAccount paymentAccount = (TransferwiseAccount) createPaymentAccount(alicedaemon, jsonString); verifyUserPayloadHasPaymentAccountWithId(paymentAccount.getId()); - verifyAccountTradeCurrencies(getAllTransferwiseCurrencies(), paymentAccount); + verifyUserPayloadHasPaymentAccountWithId(paymentAccount.getId()); + // As per commit 88f26f93241af698ae689bf081205d0f9dc929fa + // Do not autofill all currencies by default but keep all unselected. + // verifyAccountTradeCurrencies(getAllTransferwiseCurrencies(), paymentAccount); + // TODO uncomment after master/merge + // assertEquals(0, paymentAccount.getTradeCurrencies().size()); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); 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 69445657416..ce5bceff7a1 100644 --- a/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBTCOfferTest.java +++ b/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBTCOfferTest.java @@ -23,7 +23,6 @@ import lombok.extern.slf4j.Slf4j; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; @@ -33,6 +32,7 @@ import static bisq.apitest.config.BisqAppConfig.alicedaemon; import static bisq.apitest.config.BisqAppConfig.bobdaemon; import static bisq.cli.CurrencyFormat.formatSatoshis; +import static bisq.cli.TransactionFormat.format; import static bisq.core.trade.Trade.Phase.*; import static bisq.core.trade.Trade.State.*; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -42,7 +42,7 @@ import static protobuf.Offer.State.OFFER_FEE_PAID; import static protobuf.OpenOffer.State.AVAILABLE; -@Disabled +// @Disabled @Slf4j @TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class TakeSellBTCOfferTest extends AbstractTradeTest { @@ -52,6 +52,8 @@ public class TakeSellBTCOfferTest extends AbstractTradeTest { // Maker and Taker fees are in BTC. private static final String TRADE_FEE_CURRENCY_CODE = "btc"; + private static final String WITHDRAWAL_TX_MEMO = "Bob's trade withdrawal"; + @Test @Order(1) public void testTakeAlicesSellOffer(final TestInfo testInfo) { @@ -147,7 +149,7 @@ public void testBobsBtcWithdrawalToExternalAddress(final TestInfo testInfo) { logTrade(log, testInfo, "Bob's view before withdrawing funds to external wallet", trade); String toAddress = bitcoinCli.getNewBtcAddress(); - withdrawFunds(bobdaemon, tradeId, toAddress, "to whom it may concern"); + withdrawFunds(bobdaemon, tradeId, toAddress, WITHDRAWAL_TX_MEMO); genBtcBlocksThenWait(1, 2250); @@ -162,4 +164,19 @@ public void testBobsBtcWithdrawalToExternalAddress(final TestInfo testInfo) { testName(testInfo), formatSatoshis(currentBalance.getAvailableBalance())); } + + @Test + @Order(5) + public void testGetTradeWithdrawalTx(final TestInfo testInfo) { + var trade = getTrade(bobdaemon, tradeId); + var withdrawalTxId = trade.getWithdrawalTxId(); + assertNotNull(withdrawalTxId); + + var txInfo = getTransaction(bobdaemon, withdrawalTxId); + assertEquals(WITHDRAWAL_TX_MEMO, txInfo.getMemo()); + + log.debug("{} Trade withdrawal Tx:\n{}", + testName(testInfo), + format(txInfo)); + } } diff --git a/apitest/src/test/java/bisq/apitest/method/wallet/BtcWalletTest.java b/apitest/src/test/java/bisq/apitest/method/wallet/BtcWalletTest.java index 7af31dcb3fc..90c46a3c814 100644 --- a/apitest/src/test/java/bisq/apitest/method/wallet/BtcWalletTest.java +++ b/apitest/src/test/java/bisq/apitest/method/wallet/BtcWalletTest.java @@ -1,6 +1,7 @@ package bisq.apitest.method.wallet; import bisq.proto.grpc.BtcBalanceInfo; +import bisq.proto.grpc.TxInfo; import lombok.extern.slf4j.Slf4j; @@ -20,6 +21,7 @@ import static bisq.cli.TableFormat.formatBtcBalanceInfoTbl; import static java.util.Collections.singletonList; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.MethodOrderer.OrderAnnotation; @@ -32,6 +34,8 @@ @TestMethodOrder(OrderAnnotation.class) public class BtcWalletTest extends MethodTest { + private static final String TX_MEMO = "tx memo"; + // All api tests depend on the DAO / regtest environment, and Bob & Alice's wallets // are initialized with 10 BTC during the scaffolding setup. private static final bisq.core.api.model.BtcBalanceInfo INITIAL_BTC_BALANCES = @@ -99,15 +103,23 @@ public void testAliceSendBTCToBob(TestInfo testInfo) { String bobsBtcAddress = getUnusedBtcAddress(bobdaemon); log.debug("Sending 5.5 BTC From Alice to Bob @ {}", bobsBtcAddress); - sendBtc(alicedaemon, + TxInfo txInfo = sendBtc(alicedaemon, bobsBtcAddress, "5.50", "100", - "to whom it may concern"); + TX_MEMO); + assertTrue(txInfo.getIsPending()); + + // Note that the memo is not set on the tx yet. + assertTrue(txInfo.getMemo().isEmpty()); genBtcBlocksThenWait(1, 3000); - BtcBalanceInfo alicesBalances = getBtcBalances(alicedaemon); + // Fetch the tx and check for confirmation and memo. + txInfo = getTransaction(alicedaemon, txInfo.getTxId()); + assertFalse(txInfo.getIsPending()); + assertEquals(TX_MEMO, txInfo.getMemo()); + BtcBalanceInfo alicesBalances = getBtcBalances(alicedaemon); log.debug("{} Alice's BTC Balances:\n{}", testName(testInfo), formatBtcBalanceInfoTbl(alicesBalances)); diff --git a/apitest/src/test/java/bisq/apitest/scenario/TradeTest.java b/apitest/src/test/java/bisq/apitest/scenario/TradeTest.java index 4c07452abc6..410b72ca6cb 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/TradeTest.java +++ b/apitest/src/test/java/bisq/apitest/scenario/TradeTest.java @@ -60,5 +60,6 @@ public void testTakeSellBTCOffer(final TestInfo testInfo) { test.testBobsConfirmPaymentStarted(testInfo); test.testAlicesConfirmPaymentReceived(testInfo); test.testBobsBtcWithdrawalToExternalAddress(testInfo); + test.testGetTradeWithdrawalTx(testInfo); } } diff --git a/cli/src/main/java/bisq/cli/CliMain.java b/cli/src/main/java/bisq/cli/CliMain.java index b9cb4ede0fd..4853e3a76b3 100644 --- a/cli/src/main/java/bisq/cli/CliMain.java +++ b/cli/src/main/java/bisq/cli/CliMain.java @@ -31,6 +31,7 @@ import bisq.proto.grpc.GetPaymentAccountsRequest; import bisq.proto.grpc.GetPaymentMethodsRequest; import bisq.proto.grpc.GetTradeRequest; +import bisq.proto.grpc.GetTransactionRequest; import bisq.proto.grpc.GetTxFeeRateRequest; import bisq.proto.grpc.GetUnusedBsqAddressRequest; import bisq.proto.grpc.GetVersionRequest; @@ -116,6 +117,7 @@ private enum Method { gettxfeerate, settxfeerate, unsettxfeerate, + gettransaction, lockwallet, unlockwallet, removewalletpassword, @@ -275,7 +277,10 @@ public static void run(String[] args) { .build(); var reply = walletsService.sendBsq(request); TxInfo txInfo = reply.getTxInfo(); - out.printf("%s bsq sent to %s in tx %s%n", amount, address, txInfo.getId()); + out.printf("%s bsq sent to %s in tx %s%n", + amount, + address, + txInfo.getTxId()); return; } case sendbtc: { @@ -305,7 +310,10 @@ public static void run(String[] args) { .build(); var reply = walletsService.sendBtc(request); TxInfo txInfo = reply.getTxInfo(); - out.printf("%s btc sent to %s in tx %s%n", amount, address, txInfo.getId()); + out.printf("%s btc sent to %s in tx %s%n", + amount, + address, + txInfo.getTxId()); return; } case gettxfeerate: { @@ -332,6 +340,18 @@ public static void run(String[] args) { out.println(formatTxFeeRateInfo(reply.getTxFeeRateInfo())); return; } + case gettransaction: { + if (nonOptionArgs.size() < 2) + throw new IllegalArgumentException("no tx id specified"); + + var txId = nonOptionArgs.get(1); + var request = GetTransactionRequest.newBuilder() + .setTxId(txId) + .build(); + var reply = walletsService.getTransaction(request); + out.println(TransactionFormat.format(reply.getTxInfo())); + return; + } case createoffer: { if (nonOptionArgs.size() < 9) throw new IllegalArgumentException("incorrect parameter count," @@ -441,7 +461,7 @@ public static void run(String[] args) { return; } case gettrade: { - // TODO make short-id a valid argument + // TODO make short-id a valid argument? if (nonOptionArgs.size() < 2) throw new IllegalArgumentException("incorrect parameter count, " + " expecting trade id [,showcontract = true|false]"); @@ -720,6 +740,7 @@ private static void printHelp(OptionParser parser, PrintStream stream) { stream.format(rowFormat, "gettxfeerate", "", "Get current tx fee rate in sats/byte"); stream.format(rowFormat, "settxfeerate", "satoshis (per byte)", "Set custom tx fee rate in sats/byte"); stream.format(rowFormat, "unsettxfeerate", "", "Unset custom tx fee rate"); + stream.format(rowFormat, "gettransaction", "transaction id", "Get transaction with id"); stream.format(rowFormat, "createoffer", "payment acct id, buy | sell, currency code, \\", "Create and place an offer"); stream.format(rowFormat, "", "amount (btc), min amount, use mkt based price, \\", ""); stream.format(rowFormat, "", "fixed price (btc) | mkt price margin (%), security deposit (%) \\", ""); diff --git a/cli/src/main/java/bisq/cli/ColumnHeaderConstants.java b/cli/src/main/java/bisq/cli/ColumnHeaderConstants.java index 59b6230a2eb..e81e407d7b9 100644 --- a/cli/src/main/java/bisq/cli/ColumnHeaderConstants.java +++ b/cli/src/main/java/bisq/cli/ColumnHeaderConstants.java @@ -59,6 +59,16 @@ class ColumnHeaderConstants { static final String COL_HEADER_TRADE_SHORT_ID = "ID"; static final String COL_HEADER_TRADE_TX_FEE = "Tx Fee(%-3s)"; static final String COL_HEADER_TRADE_TAKER_FEE = "Taker Fee(%-3s)"; + static final String COL_HEADER_TRADE_WITHDRAWAL_TX_ID = "Withdrawal TX ID"; + + static final String COL_HEADER_TX_ID = "Tx ID"; + static final String COL_HEADER_TX_INPUT_SUM = "Tx Inputs (BTC)"; + static final String COL_HEADER_TX_OUTPUT_SUM = "Tx Outputs (BTC)"; + static final String COL_HEADER_TX_FEE = "Tx Fee (BTC)"; + static final String COL_HEADER_TX_SIZE = "Tx Size (Bytes)"; + static final String COL_HEADER_TX_IS_CONFIRMED = "Is Confirmed"; + static final String COL_HEADER_TX_MEMO = "Memo"; + static final String COL_HEADER_VOLUME = padEnd("%-3s(min - max)", 15, ' '); static final String COL_HEADER_UUID = padEnd("ID", 52, ' '); } diff --git a/cli/src/main/java/bisq/cli/TradeFormat.java b/cli/src/main/java/bisq/cli/TradeFormat.java index 2a28c1dccc4..0c4d4d29498 100644 --- a/cli/src/main/java/bisq/cli/TradeFormat.java +++ b/cli/src/main/java/bisq/cli/TradeFormat.java @@ -58,6 +58,7 @@ public static String format(TradeInfo tradeInfo) { + COL_HEADER_TRADE_FIAT_RECEIVED + COL_HEADER_DELIMITER + COL_HEADER_TRADE_PAYOUT_PUBLISHED + COL_HEADER_DELIMITER + COL_HEADER_TRADE_WITHDRAWN + COL_HEADER_DELIMITER + + (tradeInfo.getIsWithdrawn() ? COL_HEADER_TRADE_WITHDRAWAL_TX_ID + COL_HEADER_DELIMITER : "") + "%n"; String counterCurrencyCode = tradeInfo.getOffer().getCounterCurrencyCode(); @@ -66,18 +67,20 @@ public static String format(TradeInfo tradeInfo) { ? String.format(headersFormat, counterCurrencyCode, baseCurrencyCode, baseCurrencyCode, baseCurrencyCode) : String.format(headersFormat, counterCurrencyCode, baseCurrencyCode, baseCurrencyCode); - String colDataFormat = "%-" + shortIdColWidth + "s" // left justify - + " %-" + (roleColWidth + COL_HEADER_DELIMITER.length()) + "s" // left justify - + "%" + (COL_HEADER_PRICE.length() - 1) + "s" // right justify - + "%" + (COL_HEADER_TRADE_AMOUNT.length() + 1) + "s" // right justify - + "%" + (COL_HEADER_TRADE_TX_FEE.length() + 1) + "s" // right justify - + takerFeeHeader.get() // right justify - + " %-" + COL_HEADER_TRADE_DEPOSIT_PUBLISHED.length() + "s" // left justify - + " %-" + COL_HEADER_TRADE_DEPOSIT_CONFIRMED.length() + "s" // left justify - + " %-" + COL_HEADER_TRADE_FIAT_SENT.length() + "s" // left justify - + " %-" + COL_HEADER_TRADE_FIAT_RECEIVED.length() + "s" // left justify - + " %-" + COL_HEADER_TRADE_PAYOUT_PUBLISHED.length() + "s" // left justify - + " %-" + COL_HEADER_TRADE_WITHDRAWN.length() + "s"; // left justify + + String colDataFormat = "%-" + shortIdColWidth + "s" // lt justify + + " %-" + (roleColWidth + COL_HEADER_DELIMITER.length()) + "s" // left + + "%" + (COL_HEADER_PRICE.length() - 1) + "s" // rt justify + + "%" + (COL_HEADER_TRADE_AMOUNT.length() + 1) + "s" // rt justify + + "%" + (COL_HEADER_TRADE_TX_FEE.length() + 1) + "s" // rt justify + + takerFeeHeader.get() // rt justify + + " %-" + COL_HEADER_TRADE_DEPOSIT_PUBLISHED.length() + "s" // lt justify + + " %-" + COL_HEADER_TRADE_DEPOSIT_CONFIRMED.length() + "s" // lt justify + + " %-" + COL_HEADER_TRADE_FIAT_SENT.length() + "s" // lt justify + + " %-" + COL_HEADER_TRADE_FIAT_RECEIVED.length() + "s" // lt justify + + " %-" + COL_HEADER_TRADE_PAYOUT_PUBLISHED.length() + "s" // lt justify + + " %-" + COL_HEADER_TRADE_WITHDRAWN.length() + "s" // lt justify + + " %-" + COL_HEADER_TRADE_WITHDRAWAL_TX_ID.length() + "s"; // left return headerLine + (isTaker @@ -97,7 +100,8 @@ private static String formatTradeForMaker(String format, TradeInfo tradeInfo) { tradeInfo.getIsFiatSent() ? "YES" : "NO", tradeInfo.getIsFiatReceived() ? "YES" : "NO", tradeInfo.getIsPayoutPublished() ? "YES" : "NO", - tradeInfo.getIsWithdrawn() ? "YES" : "NO"); + tradeInfo.getIsWithdrawn() ? "YES" : "NO", + tradeInfo.getIsWithdrawn() ? tradeInfo.getWithdrawalTxId() : ""); } private static String formatTradeForTaker(String format, TradeInfo tradeInfo) { @@ -113,6 +117,7 @@ private static String formatTradeForTaker(String format, TradeInfo tradeInfo) { tradeInfo.getIsFiatSent() ? "YES" : "NO", tradeInfo.getIsFiatReceived() ? "YES" : "NO", tradeInfo.getIsPayoutPublished() ? "YES" : "NO", - tradeInfo.getIsWithdrawn() ? "YES" : "NO"); + tradeInfo.getIsWithdrawn() ? "YES" : "NO", + tradeInfo.getIsWithdrawn() ? tradeInfo.getWithdrawalTxId() : ""); } } diff --git a/cli/src/main/java/bisq/cli/TransactionFormat.java b/cli/src/main/java/bisq/cli/TransactionFormat.java new file mode 100644 index 00000000000..608c2fcb71f --- /dev/null +++ b/cli/src/main/java/bisq/cli/TransactionFormat.java @@ -0,0 +1,59 @@ +/* + * 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.cli; + +import bisq.proto.grpc.TxInfo; + +import com.google.common.annotations.VisibleForTesting; + +import static bisq.cli.ColumnHeaderConstants.*; +import static bisq.cli.CurrencyFormat.formatSatoshis; +import static com.google.common.base.Strings.padEnd; + +@VisibleForTesting +public class TransactionFormat { + + public static String format(TxInfo txInfo) { + String headerLine = padEnd(COL_HEADER_TX_ID, txInfo.getTxId().length(), ' ') + COL_HEADER_DELIMITER + + COL_HEADER_TX_IS_CONFIRMED + COL_HEADER_DELIMITER + + COL_HEADER_TX_INPUT_SUM + COL_HEADER_DELIMITER + + COL_HEADER_TX_OUTPUT_SUM + COL_HEADER_DELIMITER + + COL_HEADER_TX_FEE + COL_HEADER_DELIMITER + + COL_HEADER_TX_SIZE + COL_HEADER_DELIMITER + + (txInfo.getMemo().isEmpty() ? "" : COL_HEADER_TX_MEMO + COL_HEADER_DELIMITER) + + "\n"; + + String colDataFormat = "%-" + txInfo.getTxId().length() + "s" + + " %" + COL_HEADER_TX_IS_CONFIRMED.length() + "s" + + " %" + COL_HEADER_TX_INPUT_SUM.length() + "s" + + " %" + COL_HEADER_TX_OUTPUT_SUM.length() + "s" + + " %" + COL_HEADER_TX_FEE.length() + "s" + + " %" + COL_HEADER_TX_SIZE.length() + "s" + + " %s"; + + return headerLine + + String.format(colDataFormat, + txInfo.getTxId(), + txInfo.getIsPending() ? "NO" : "YES", // pending=true means not confirmed + formatSatoshis(txInfo.getInputSum()), + formatSatoshis(txInfo.getOutputSum()), + formatSatoshis(txInfo.getFee()), + txInfo.getSize(), + txInfo.getMemo().isEmpty() ? "" : txInfo.getMemo()); + } +} diff --git a/core/src/main/java/bisq/core/api/CoreApi.java b/core/src/main/java/bisq/core/api/CoreApi.java index dbba310f5dd..6709bf42ff1 100644 --- a/core/src/main/java/bisq/core/api/CoreApi.java +++ b/core/src/main/java/bisq/core/api/CoreApi.java @@ -279,6 +279,10 @@ public TxFeeRateInfo getMostRecentTxFeeRateInfo() { return walletsService.getMostRecentTxFeeRateInfo(); } + public Transaction getTransaction(String txId) { + return walletsService.getTransaction(txId); + } + public void setWalletPassword(String password, String newPassword) { walletsService.setWalletPassword(password, newPassword); } diff --git a/core/src/main/java/bisq/core/api/CoreWalletsService.java b/core/src/main/java/bisq/core/api/CoreWalletsService.java index 2bbd585f8dd..c107259ff98 100644 --- a/core/src/main/java/bisq/core/api/CoreWalletsService.java +++ b/core/src/main/java/bisq/core/api/CoreWalletsService.java @@ -331,6 +331,26 @@ TxFeeRateInfo getMostRecentTxFeeRateInfo() { feeService.getLastRequest()); } + Transaction getTransaction(String txId) { + if (txId.length() != 64) + throw new IllegalArgumentException(format("%s is not a transaction id", txId)); + + try { + Transaction tx = btcWalletService.getTransaction(txId); + if (tx == null) + throw new IllegalArgumentException(format("tx with id %s not found", txId)); + else + return tx; + + } catch (IllegalArgumentException ex) { + log.error("", ex); + throw new IllegalArgumentException( + format("could not get transaction with id %s%ncause: %s", + txId, + ex.getMessage().toLowerCase())); + } + } + int getNumConfirmationsForMostRecentTransaction(String addressString) { Address address = getAddressEntry(addressString).getAddress(); TransactionConfidence confidence = btcWalletService.getConfidenceForAddress(address); diff --git a/core/src/main/java/bisq/core/api/model/TradeInfo.java b/core/src/main/java/bisq/core/api/model/TradeInfo.java index 1a717a7672e..ec9880d5c69 100644 --- a/core/src/main/java/bisq/core/api/model/TradeInfo.java +++ b/core/src/main/java/bisq/core/api/model/TradeInfo.java @@ -47,6 +47,7 @@ public class TradeInfo implements Payload { private final String takerFeeTxId; private final String depositTxId; private final String payoutTxId; + private final String withdrawalTxId; private final long tradeAmountAsLong; private final long tradePrice; private final String tradingPeerNodeAddress; @@ -73,6 +74,7 @@ public TradeInfo(TradeInfoBuilder builder) { this.takerFeeTxId = builder.takerFeeTxId; this.depositTxId = builder.depositTxId; this.payoutTxId = builder.payoutTxId; + this.withdrawalTxId = builder.withdrawalTxId; this.tradeAmountAsLong = builder.tradeAmountAsLong; this.tradePrice = builder.tradePrice; this.tradingPeerNodeAddress = builder.tradingPeerNodeAddress; @@ -106,6 +108,7 @@ public static TradeInfo toTradeInfo(Trade trade, String role) { .withTakerFeeTxId(trade.getTakerFeeTxId()) .withDepositTxId(trade.getDepositTxId()) .withPayoutTxId(trade.getPayoutTxId()) + .withWithdrawalTxId(trade.getWithdrawalTxId()) .withTradeAmountAsLong(trade.getTradeAmountAsLong()) .withTradePrice(trade.getTradePrice().getValue()) .withTradingPeerNodeAddress(Objects.requireNonNull( @@ -141,6 +144,7 @@ public bisq.proto.grpc.TradeInfo toProtoMessage() { .setTakerFeeTxId(takerFeeTxId == null ? "" : takerFeeTxId) .setDepositTxId(depositTxId == null ? "" : depositTxId) .setPayoutTxId(payoutTxId == null ? "" : payoutTxId) + .setWithdrawalTxId(withdrawalTxId == null ? "" : withdrawalTxId) .setTradeAmountAsLong(tradeAmountAsLong) .setTradePrice(tradePrice) .setTradingPeerNodeAddress(tradingPeerNodeAddress) @@ -180,6 +184,7 @@ public static class TradeInfoBuilder { private String takerFeeTxId; private String depositTxId; private String payoutTxId; + private String withdrawalTxId; private long tradeAmountAsLong; private long tradePrice; private String tradingPeerNodeAddress; @@ -249,6 +254,11 @@ public TradeInfoBuilder withPayoutTxId(String payoutTxId) { return this; } + public TradeInfoBuilder withWithdrawalTxId(String withdrawalTxId) { + this.withdrawalTxId = withdrawalTxId; + return this; + } + public TradeInfoBuilder withTradeAmountAsLong(long tradeAmountAsLong) { this.tradeAmountAsLong = tradeAmountAsLong; return this; @@ -332,6 +342,7 @@ public String toString() { ", takerFeeTxId='" + takerFeeTxId + '\'' + "\n" + ", depositTxId='" + depositTxId + '\'' + "\n" + ", payoutTxId='" + payoutTxId + '\'' + "\n" + + ", withdrawalTxId='" + withdrawalTxId + '\'' + "\n" + ", tradeAmountAsLong='" + tradeAmountAsLong + '\'' + "\n" + ", tradePrice='" + tradePrice + '\'' + "\n" + ", tradingPeerNodeAddress='" + tradingPeerNodeAddress + '\'' + "\n" + diff --git a/core/src/main/java/bisq/core/api/model/TxInfo.java b/core/src/main/java/bisq/core/api/model/TxInfo.java index f1b24f6b0fa..16d8f5fc108 100644 --- a/core/src/main/java/bisq/core/api/model/TxInfo.java +++ b/core/src/main/java/bisq/core/api/model/TxInfo.java @@ -28,26 +28,42 @@ @Getter public class TxInfo implements Payload { - private final String id; + // The client cannot see an instance of an org.bitcoinj.core.Transaction. We use the + // lighter weight TxInfo proto wrapper instead, containing just enough fields to + // view some transaction details. A block explorer or bitcoin-core client can be + // used to see more detail. + + private final String txId; + private final long inputSum; private final long outputSum; private final long fee; private final int size; + private final boolean isPending; + private final String memo; - public TxInfo(String id, long outputSum, long fee, int size) { - this.id = id; - this.outputSum = outputSum; - this.fee = fee; - this.size = size; + public TxInfo(TxInfo.TxInfoBuilder builder) { + this.txId = builder.txId; + this.inputSum = builder.inputSum; + this.outputSum = builder.outputSum; + this.fee = builder.fee; + this.size = builder.size; + this.isPending = builder.isPending; + this.memo = builder.memo; } public static TxInfo toTxInfo(Transaction transaction) { if (transaction == null) throw new IllegalStateException("server created a null transaction"); - return new TxInfo(transaction.getTxId().toString(), - transaction.getOutputSum().value, - transaction.getFee().value, - transaction.getMessageSize()); + return new TxInfo.TxInfoBuilder() + .withTxId(transaction.getTxId().toString()) + .withInputSum(transaction.getInputSum().value) + .withOutputSum(transaction.getOutputSum().value) + .withFee(transaction.getFee().value) + .withSize(transaction.getMessageSize()) + .withIsPending(transaction.isPending()) + .withMemo(transaction.getMemo()) + .build(); } ////////////////////////////////////////////////////////////////////////////////////// @@ -57,28 +73,88 @@ public static TxInfo toTxInfo(Transaction transaction) { @Override public bisq.proto.grpc.TxInfo toProtoMessage() { return bisq.proto.grpc.TxInfo.newBuilder() - .setId(id) + .setTxId(txId) + .setInputSum(inputSum) .setOutputSum(outputSum) .setFee(fee) .setSize(size) + .setIsPending(isPending) + .setMemo(memo == null ? "" : memo) .build(); } @SuppressWarnings("unused") public static TxInfo fromProto(bisq.proto.grpc.TxInfo proto) { - return new TxInfo(proto.getId(), - proto.getOutputSum(), - proto.getFee(), - proto.getSize()); + return new TxInfo.TxInfoBuilder() + .withTxId(proto.getTxId()) + .withInputSum(proto.getInputSum()) + .withOutputSum(proto.getOutputSum()) + .withFee(proto.getFee()) + .withSize(proto.getSize()) + .withIsPending(proto.getIsPending()) + .withMemo(proto.getMemo()) + .build(); + } + + public static class TxInfoBuilder { + private String txId; + private long inputSum; + private long outputSum; + private long fee; + private int size; + private boolean isPending; + private String memo; + + public TxInfo.TxInfoBuilder withTxId(String txId) { + this.txId = txId; + return this; + } + + public TxInfo.TxInfoBuilder withInputSum(long inputSum) { + this.inputSum = inputSum; + return this; + } + + public TxInfo.TxInfoBuilder withOutputSum(long outputSum) { + this.outputSum = outputSum; + return this; + } + + public TxInfo.TxInfoBuilder withFee(long fee) { + this.fee = fee; + return this; + } + + public TxInfo.TxInfoBuilder withSize(int size) { + this.size = size; + return this; + } + + public TxInfo.TxInfoBuilder withIsPending(boolean isPending) { + this.isPending = isPending; + return this; + } + + public TxInfo.TxInfoBuilder withMemo(String memo) { + this.memo = memo; + return this; + } + + public TxInfo build() { + return new TxInfo(this); + } } @Override public String toString() { return "TxInfo{" + "\n" + - " id='" + id + '\'' + "\n" + - ", outputSum=" + outputSum + " sats" + "\n" + - ", fee=" + fee + " sats" + "\n" + - ", size=" + size + " bytes" + "\n" + + " txId='" + txId + '\'' + "\n" + + ", inputSum=" + inputSum + "\n" + + ", outputSum=" + outputSum + "\n" + + ", fee=" + fee + "\n" + + ", size=" + size + "\n" + + ", isPending=" + isPending + "\n" + + ", memo='" + memo + '\'' + "\n" + '}'; } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java index c7bbb4623da..51b30cb1871 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java @@ -29,6 +29,8 @@ import bisq.proto.grpc.GetBalancesRequest; import bisq.proto.grpc.GetFundingAddressesReply; import bisq.proto.grpc.GetFundingAddressesRequest; +import bisq.proto.grpc.GetTransactionReply; +import bisq.proto.grpc.GetTransactionRequest; import bisq.proto.grpc.GetTxFeeRateReply; import bisq.proto.grpc.GetTxFeeRateRequest; import bisq.proto.grpc.GetUnusedBsqAddressReply; @@ -280,6 +282,23 @@ public void unsetTxFeeRatePreference(UnsetTxFeeRatePreferenceRequest req, } } + @Override + public void getTransaction(GetTransactionRequest req, + StreamObserver responseObserver) { + try { + Transaction tx = coreApi.getTransaction(req.getTxId()); + var reply = GetTransactionReply.newBuilder() + .setTxInfo(toTxInfo(tx).toProtoMessage()) + .build(); + responseObserver.onNext(reply); + responseObserver.onCompleted(); + } catch (IllegalStateException | IllegalArgumentException cause) { + var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); + responseObserver.onError(ex); + throw ex; + } + } + @Override public void setWalletPassword(SetWalletPasswordRequest req, StreamObserver responseObserver) { diff --git a/proto/src/main/proto/grpc.proto b/proto/src/main/proto/grpc.proto index a6e30cdb473..81795112e55 100644 --- a/proto/src/main/proto/grpc.proto +++ b/proto/src/main/proto/grpc.proto @@ -273,19 +273,20 @@ message TradeInfo { string takerFeeTxId = 9; string depositTxId = 10; string payoutTxId = 11; - uint64 tradeAmountAsLong = 12; - uint64 tradePrice = 13; - string tradingPeerNodeAddress = 14; - string state = 15; - string phase = 16; - string tradePeriodState = 17; - bool isDepositPublished = 18; - bool isDepositConfirmed = 19; - bool isFiatSent = 20; - bool isFiatReceived = 21; - bool isPayoutPublished = 22; - bool isWithdrawn = 23; - string contractAsJson = 24; + string withdrawalTxId = 12; + uint64 tradeAmountAsLong = 13; + uint64 tradePrice = 14; + string tradingPeerNodeAddress = 15; + string state = 16; + string phase = 17; + string tradePeriodState = 18; + bool isDepositPublished = 19; + bool isDepositConfirmed = 20; + bool isFiatSent = 21; + bool isFiatReceived = 22; + bool isPayoutPublished = 23; + bool isWithdrawn = 24; + string contractAsJson = 25; } /////////////////////////////////////////////////////////////////////////////////////////// @@ -300,10 +301,13 @@ message TxFeeRateInfo { } message TxInfo { - string id = 1; - uint64 outputSum = 2; - uint64 fee = 3; - int32 size = 4; + string txId = 1; + uint64 inputSum = 2; + uint64 outputSum = 3; + uint64 fee = 4; + int32 size = 5; + bool isPending = 6; + string memo = 7; } /////////////////////////////////////////////////////////////////////////////////////////// @@ -325,7 +329,9 @@ service Wallets { } rpc SetTxFeeRatePreference (SetTxFeeRatePreferenceRequest) returns (SetTxFeeRatePreferenceReply) { } - rpc unsetTxFeeRatePreference (UnsetTxFeeRatePreferenceRequest) returns (UnsetTxFeeRatePreferenceReply) { + rpc UnsetTxFeeRatePreference (UnsetTxFeeRatePreferenceRequest) returns (UnsetTxFeeRatePreferenceReply) { + } + rpc GetTransaction (GetTransactionRequest) returns (GetTransactionReply) { } rpc GetFundingAddresses (GetFundingAddressesRequest) returns (GetFundingAddressesReply) { } @@ -405,6 +411,14 @@ message UnsetTxFeeRatePreferenceReply { TxFeeRateInfo txFeeRateInfo = 1; } +message GetTransactionRequest { + string txId = 1; +} + +message GetTransactionReply { + TxInfo txInfo = 1; +} + message GetFundingAddressesRequest { } From 0384642f32c8ffbdb8ec68c93cbc067e1de73008 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 14 Dec 2020 15:03:35 -0300 Subject: [PATCH 09/20] Adjust create TransferwiseAccount test As per commit 88f26f93241af698ae689bf081205d0f9dc929fa, do not autofill all currencies by default but keep all unselected. --- .../bisq/apitest/method/payment/CreatePaymentAccountTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java index ac66cc7993e..2fe5ed22abd 100644 --- a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java @@ -750,8 +750,7 @@ public void testCreateTransferwiseAccount(TestInfo testInfo) { // As per commit 88f26f93241af698ae689bf081205d0f9dc929fa // Do not autofill all currencies by default but keep all unselected. // verifyAccountTradeCurrencies(getAllTransferwiseCurrencies(), paymentAccount); - // TODO uncomment after master/merge - // assertEquals(0, paymentAccount.getTradeCurrencies().size()); + assertEquals(0, paymentAccount.getTradeCurrencies().size()); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount); From 4be87a6281961242dcb9689db869bc4641840b40 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 14 Dec 2020 15:12:39 -0300 Subject: [PATCH 10/20] Disable method test to avoid repetition --- .../java/bisq/apitest/method/trade/TakeSellBTCOfferTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 ce5bceff7a1..cd500cdd091 100644 --- a/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBTCOfferTest.java +++ b/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBTCOfferTest.java @@ -23,6 +23,7 @@ import lombok.extern.slf4j.Slf4j; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; @@ -42,7 +43,7 @@ import static protobuf.Offer.State.OFFER_FEE_PAID; import static protobuf.OpenOffer.State.AVAILABLE; -// @Disabled +@Disabled @Slf4j @TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class TakeSellBTCOfferTest extends AbstractTradeTest { From e6c6d3b8d3a5095690ea498980b71d19b7eb9252 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Wed, 16 Dec 2020 13:34:21 -0300 Subject: [PATCH 11/20] Add new CoreApiExceptionHandler to gRPC services This change reduces gRPC service error handling duplication by moving it into a @Singleton encapsulating everything needed to wrap an expected or unexpected core api exception into a gRPC StatusRuntimeException before sending it to the client. It also fixes some boilerpate classes were gRPC error handling was missing. --- .../daemon/grpc/CoreApiExceptionHandler.java | 93 +++++++++++++++++++ .../daemon/grpc/GrpcDisputeAgentsService.java | 16 +--- .../grpc/GrpcGetTradeStatisticsService.java | 23 +++-- .../bisq/daemon/grpc/GrpcOffersService.java | 48 +++++----- .../grpc/GrpcPaymentAccountsService.java | 46 +++------ .../bisq/daemon/grpc/GrpcPriceService.java | 12 +-- .../bisq/daemon/grpc/GrpcTradesService.java | 42 +++------ .../bisq/daemon/grpc/GrpcVersionService.java | 14 ++- .../bisq/daemon/grpc/GrpcWalletsService.java | 90 +++++++----------- 9 files changed, 207 insertions(+), 177 deletions(-) create mode 100644 daemon/src/main/java/bisq/daemon/grpc/CoreApiExceptionHandler.java diff --git a/daemon/src/main/java/bisq/daemon/grpc/CoreApiExceptionHandler.java b/daemon/src/main/java/bisq/daemon/grpc/CoreApiExceptionHandler.java new file mode 100644 index 00000000000..03cde748abd --- /dev/null +++ b/daemon/src/main/java/bisq/daemon/grpc/CoreApiExceptionHandler.java @@ -0,0 +1,93 @@ +/* + * 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.daemon.grpc; + +import io.grpc.Status; +import io.grpc.StatusRuntimeException; +import io.grpc.stub.StreamObserver; + +import javax.inject.Inject; +import javax.inject.Singleton; + +import java.util.function.Predicate; + +import lombok.extern.slf4j.Slf4j; + +import static io.grpc.Status.INVALID_ARGUMENT; +import static io.grpc.Status.UNKNOWN; + +/** + * The singleton instance of this class wraps a RuntimeException from the core api + * in a gRPC StatusRuntimeException before putting it in the server's response, + * then throwing it. + */ +@Singleton +@Slf4j +class CoreApiExceptionHandler { + + private final Predicate isExpectedException = (t) -> + t instanceof IllegalStateException || t instanceof IllegalArgumentException; + + @Inject + public CoreApiExceptionHandler() { + } + + public void handleException(Throwable t, StreamObserver responseObserver) { + // Log the core api error (this is last chance to do that), wrap it in a new + // gRPC StatusRuntimeException, then send it to the client in the gRPC response. + log.error("", t); + var grpcStatusRuntimeException = wrapException(t); + responseObserver.onError(grpcStatusRuntimeException); + throw grpcStatusRuntimeException; + } + + private StatusRuntimeException wrapException(Throwable t) { + // We want to be careful about what kinds of exception messages we send to the + // client. Expected core exceptions should be wrapped in an IllegalStateException + // or IllegalArgumentException, with a consistently styled and worded error + // message. But only a small number of the expected error types are currently + // handled this way; there is much work to do to handle the variety of errors + // that can occur in the api. In the meantime, we take care to not pass full, + // unexpected error messages to the client. If the exception type is unexpected, + // we omit details from the gRPC exception sent to the client. + if (isExpectedException.test(t)) { + if (t.getCause() != null) + return new StatusRuntimeException(mapGrpcErrorStatus(t.getCause(), t.getCause().getMessage())); + else + return new StatusRuntimeException(mapGrpcErrorStatus(t, t.getMessage())); + } else { + return new StatusRuntimeException(mapGrpcErrorStatus(t, "unexpected error on server")); + } + } + + private Status mapGrpcErrorStatus(Throwable t, String description) { + // We default to the UNKNOWN status, except were the mapping of a core api + // exception to a gRPC Status is obvious. If we ever use a gRPC reverse-proxy + // to support RESTful clients, we will need to have more specific mappings + // to support correct HTTP 1.1. status codes. + //noinspection SwitchStatementWithTooFewBranches + switch (t.getClass().getSimpleName()) { + // We go ahead and use a switch statement instead of if, in anticipation + // of more, specific exception mappings. + case "IllegalArgumentException": + return INVALID_ARGUMENT.withDescription(description); + default: + return UNKNOWN.withDescription(description); + } + } +} diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java index 24fd192fea8..1b5268bb81d 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java @@ -6,8 +6,6 @@ import bisq.proto.grpc.RegisterDisputeAgentReply; import bisq.proto.grpc.RegisterDisputeAgentRequest; -import io.grpc.Status; -import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import javax.inject.Inject; @@ -18,10 +16,12 @@ class GrpcDisputeAgentsService extends DisputeAgentsGrpc.DisputeAgentsImplBase { private final CoreApi coreApi; + private final CoreApiExceptionHandler exceptionHandler; @Inject - public GrpcDisputeAgentsService(CoreApi coreApi) { + public GrpcDisputeAgentsService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { this.coreApi = coreApi; + this.exceptionHandler = exceptionHandler; } @Override @@ -32,14 +32,8 @@ public void registerDisputeAgent(RegisterDisputeAgentRequest req, var reply = RegisterDisputeAgentReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.INVALID_ARGUMENT.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java index 4c98e939af3..d50d7eb477a 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java @@ -16,22 +16,27 @@ class GrpcGetTradeStatisticsService extends GetTradeStatisticsGrpc.GetTradeStatisticsImplBase { private final CoreApi coreApi; + private final CoreApiExceptionHandler exceptionHandler; @Inject - public GrpcGetTradeStatisticsService(CoreApi coreApi) { + public GrpcGetTradeStatisticsService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { this.coreApi = coreApi; + this.exceptionHandler = exceptionHandler; } @Override public void getTradeStatistics(GetTradeStatisticsRequest req, StreamObserver responseObserver) { - - var tradeStatistics = coreApi.getTradeStatistics().stream() - .map(TradeStatistics3::toProtoTradeStatistics3) - .collect(Collectors.toList()); - - var reply = GetTradeStatisticsReply.newBuilder().addAllTradeStatistics(tradeStatistics).build(); - responseObserver.onNext(reply); - responseObserver.onCompleted(); + try { + var tradeStatistics = coreApi.getTradeStatistics().stream() + .map(TradeStatistics3::toProtoTradeStatistics3) + .collect(Collectors.toList()); + + var reply = GetTradeStatisticsReply.newBuilder().addAllTradeStatistics(tradeStatistics).build(); + responseObserver.onNext(reply); + responseObserver.onCompleted(); + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); + } } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java index 1efed108ca6..2bc99ee4039 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java @@ -31,8 +31,6 @@ import bisq.proto.grpc.GetOffersRequest; import bisq.proto.grpc.OffersGrpc; -import io.grpc.Status; -import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import javax.inject.Inject; @@ -48,10 +46,12 @@ class GrpcOffersService extends OffersGrpc.OffersImplBase { private final CoreApi coreApi; + private final CoreApiExceptionHandler exceptionHandler; @Inject - public GrpcOffersService(CoreApi coreApi) { + public GrpcOffersService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { this.coreApi = coreApi; + this.exceptionHandler = exceptionHandler; } @Override @@ -64,26 +64,28 @@ public void getOffer(GetOfferRequest req, .build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException | IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @Override public void getOffers(GetOffersRequest req, StreamObserver responseObserver) { - List result = coreApi.getOffers(req.getDirection(), req.getCurrencyCode()) - .stream().map(OfferInfo::toOfferInfo) - .collect(Collectors.toList()); - var reply = GetOffersReply.newBuilder() - .addAllOffers(result.stream() - .map(OfferInfo::toProtoMessage) - .collect(Collectors.toList())) - .build(); - responseObserver.onNext(reply); - responseObserver.onCompleted(); + try { + List result = coreApi.getOffers(req.getDirection(), req.getCurrencyCode()) + .stream().map(OfferInfo::toOfferInfo) + .collect(Collectors.toList()); + var reply = GetOffersReply.newBuilder() + .addAllOffers(result.stream() + .map(OfferInfo::toProtoMessage) + .collect(Collectors.toList())) + .build(); + responseObserver.onNext(reply); + responseObserver.onCompleted(); + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); + } } @Override @@ -111,10 +113,8 @@ public void createOffer(CreateOfferRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); }); - } catch (IllegalStateException | IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -126,10 +126,8 @@ public void cancelOffer(CancelOfferRequest req, var reply = CancelOfferReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException | IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java index 0438d33655d..864ecc55e16 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java @@ -31,8 +31,6 @@ import bisq.proto.grpc.GetPaymentMethodsRequest; import bisq.proto.grpc.PaymentAccountsGrpc; -import io.grpc.Status; -import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import javax.inject.Inject; @@ -43,10 +41,12 @@ class GrpcPaymentAccountsService extends PaymentAccountsGrpc.PaymentAccountsImplBase { private final CoreApi coreApi; + private final CoreApiExceptionHandler exceptionHandler; @Inject - public GrpcPaymentAccountsService(CoreApi coreApi) { + public GrpcPaymentAccountsService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { this.coreApi = coreApi; + this.exceptionHandler = exceptionHandler; } @Override @@ -59,14 +59,8 @@ public void createPaymentAccount(CreatePaymentAccountRequest req, .build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.INVALID_ARGUMENT.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -81,14 +75,8 @@ public void getPaymentAccounts(GetPaymentAccountsRequest req, .addAllPaymentAccounts(paymentAccounts).build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.INVALID_ARGUMENT.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -103,14 +91,8 @@ public void getPaymentMethods(GetPaymentMethodsRequest req, .addAllPaymentMethods(paymentMethods).build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.INVALID_ARGUMENT.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -124,14 +106,8 @@ public void getPaymentAccountForm(GetPaymentAccountFormRequest req, .build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.INVALID_ARGUMENT.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java index 11410140fdc..5687aa554c3 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java @@ -23,8 +23,6 @@ import bisq.proto.grpc.MarketPriceRequest; import bisq.proto.grpc.PriceGrpc; -import io.grpc.Status; -import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import javax.inject.Inject; @@ -35,10 +33,12 @@ class GrpcPriceService extends PriceGrpc.PriceImplBase { private final CoreApi coreApi; + private final CoreApiExceptionHandler exceptionHandler; @Inject - public GrpcPriceService(CoreApi coreApi) { + public GrpcPriceService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { this.coreApi = coreApi; + this.exceptionHandler = exceptionHandler; } @Override @@ -49,10 +49,8 @@ public void getMarketPrice(MarketPriceRequest req, var reply = MarketPriceReply.newBuilder().setPrice(price).build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java index 1028c0cf9d7..3bf2b183cdd 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java @@ -35,8 +35,6 @@ import bisq.proto.grpc.WithdrawFundsReply; import bisq.proto.grpc.WithdrawFundsRequest; -import io.grpc.Status; -import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import javax.inject.Inject; @@ -49,10 +47,12 @@ class GrpcTradesService extends TradesGrpc.TradesImplBase { private final CoreApi coreApi; + private final CoreApiExceptionHandler exceptionHandler; @Inject - public GrpcTradesService(CoreApi coreApi) { + public GrpcTradesService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { this.coreApi = coreApi; + this.exceptionHandler = exceptionHandler; } @Override @@ -66,10 +66,8 @@ public void getTrade(GetTradeRequest req, .build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException | IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -88,10 +86,8 @@ public void takeOffer(TakeOfferRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); }); - } catch (IllegalStateException | IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -103,10 +99,8 @@ public void confirmPaymentStarted(ConfirmPaymentStartedRequest req, var reply = ConfirmPaymentStartedReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException | IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -118,10 +112,8 @@ public void confirmPaymentReceived(ConfirmPaymentReceivedRequest req, var reply = ConfirmPaymentReceivedReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException | IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -133,10 +125,8 @@ public void keepFunds(KeepFundsRequest req, var reply = KeepFundsReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException | IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -148,10 +138,8 @@ public void withdrawFunds(WithdrawFundsRequest req, var reply = WithdrawFundsReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException | IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java index 658ef10e27f..6414a6ceb44 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java @@ -13,16 +13,22 @@ class GrpcVersionService extends GetVersionGrpc.GetVersionImplBase { private final CoreApi coreApi; + private final CoreApiExceptionHandler exceptionHandler; @Inject - public GrpcVersionService(CoreApi coreApi) { + public GrpcVersionService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { this.coreApi = coreApi; + this.exceptionHandler = exceptionHandler; } @Override public void getVersion(GetVersionRequest req, StreamObserver responseObserver) { - var reply = GetVersionReply.newBuilder().setVersion(coreApi.getVersion()).build(); - responseObserver.onNext(reply); - responseObserver.onCompleted(); + try { + var reply = GetVersionReply.newBuilder().setVersion(coreApi.getVersion()).build(); + responseObserver.onNext(reply); + responseObserver.onCompleted(); + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); + } } } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java index 51b30cb1871..5cf74d38dab 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java @@ -53,8 +53,6 @@ import bisq.proto.grpc.UnsetTxFeeRatePreferenceRequest; import bisq.proto.grpc.WalletsGrpc; -import io.grpc.Status; -import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import org.bitcoinj.core.Transaction; @@ -76,10 +74,12 @@ class GrpcWalletsService extends WalletsGrpc.WalletsImplBase { private final CoreApi coreApi; + private final CoreApiExceptionHandler exceptionHandler; @Inject - public GrpcWalletsService(CoreApi coreApi) { + public GrpcWalletsService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { this.coreApi = coreApi; + this.exceptionHandler = exceptionHandler; } @Override @@ -91,10 +91,8 @@ public void getBalances(GetBalancesRequest req, StreamObserver .build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -107,10 +105,8 @@ public void getAddressBalance(GetAddressBalanceRequest req, .setAddressBalanceInfo(balanceInfo.toProtoMessage()).build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -127,10 +123,8 @@ public void getFundingAddresses(GetFundingAddressesRequest req, .build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -144,10 +138,8 @@ public void getUnusedBsqAddress(GetUnusedBsqAddressRequest req, .build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -178,10 +170,8 @@ public void onFailure(TxBroadcastException ex) { throw new IllegalStateException(ex); } }); - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -218,10 +208,8 @@ public void onFailure(@NotNull Throwable t) { throw new IllegalStateException(t); } }); - } catch (IllegalStateException | IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -237,10 +225,8 @@ public void getTxFeeRate(GetTxFeeRateRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); }); - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -256,10 +242,8 @@ public void setTxFeeRatePreference(SetTxFeeRatePreferenceRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); }); - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -275,10 +259,8 @@ public void unsetTxFeeRatePreference(UnsetTxFeeRatePreferenceRequest req, responseObserver.onNext(reply); responseObserver.onCompleted(); }); - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -292,10 +274,8 @@ public void getTransaction(GetTransactionRequest req, .build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException | IllegalArgumentException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -307,10 +287,8 @@ public void setWalletPassword(SetWalletPasswordRequest req, var reply = SetWalletPasswordReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -322,10 +300,8 @@ public void removeWalletPassword(RemoveWalletPasswordRequest req, var reply = RemoveWalletPasswordReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -337,10 +313,8 @@ public void lockWallet(LockWalletRequest req, var reply = LockWalletReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } @@ -352,10 +326,8 @@ public void unlockWallet(UnlockWalletRequest req, var reply = UnlockWalletReply.newBuilder().build(); responseObserver.onNext(reply); responseObserver.onCompleted(); - } catch (IllegalStateException cause) { - var ex = new StatusRuntimeException(Status.UNKNOWN.withDescription(cause.getMessage())); - responseObserver.onError(ex); - throw ex; + } catch (Throwable cause) { + exceptionHandler.handleException(cause, responseObserver); } } } From c60605f75c98e2632c2c80e30f5a824b9eb1c0ad Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Wed, 16 Dec 2020 14:26:12 -0300 Subject: [PATCH 12/20] Fix class level comment --- .../main/java/bisq/daemon/grpc/CoreApiExceptionHandler.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/daemon/src/main/java/bisq/daemon/grpc/CoreApiExceptionHandler.java b/daemon/src/main/java/bisq/daemon/grpc/CoreApiExceptionHandler.java index 03cde748abd..8f9a54a40b3 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/CoreApiExceptionHandler.java +++ b/daemon/src/main/java/bisq/daemon/grpc/CoreApiExceptionHandler.java @@ -32,9 +32,9 @@ import static io.grpc.Status.UNKNOWN; /** - * The singleton instance of this class wraps a RuntimeException from the core api - * in a gRPC StatusRuntimeException before putting it in the server's response, - * then throwing it. + * The singleton instance of this class handles any expected core api Throwable by + * wrapping its message in a gRPC StatusRuntimeException and sending it to the client. + * An unexpected Throwable's message will be replaced with an 'unexpected' error message. */ @Singleton @Slf4j From f7c1103848f33d1fada78a030c221f0f5fb4b01a Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Wed, 16 Dec 2020 14:42:23 -0300 Subject: [PATCH 13/20] Rename gRPC exception handler class --- .../main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java | 4 ++-- ...CoreApiExceptionHandler.java => GrpcExceptionHandler.java} | 4 ++-- .../java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java | 4 ++-- daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java | 4 ++-- .../java/bisq/daemon/grpc/GrpcPaymentAccountsService.java | 4 ++-- daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java | 4 ++-- daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java | 4 ++-- daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java | 4 ++-- daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java | 4 ++-- 9 files changed, 18 insertions(+), 18 deletions(-) rename daemon/src/main/java/bisq/daemon/grpc/{CoreApiExceptionHandler.java => GrpcExceptionHandler.java} (98%) diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java index 1b5268bb81d..cd336da436a 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcDisputeAgentsService.java @@ -16,10 +16,10 @@ class GrpcDisputeAgentsService extends DisputeAgentsGrpc.DisputeAgentsImplBase { private final CoreApi coreApi; - private final CoreApiExceptionHandler exceptionHandler; + private final GrpcExceptionHandler exceptionHandler; @Inject - public GrpcDisputeAgentsService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { + public GrpcDisputeAgentsService(CoreApi coreApi, GrpcExceptionHandler exceptionHandler) { this.coreApi = coreApi; this.exceptionHandler = exceptionHandler; } diff --git a/daemon/src/main/java/bisq/daemon/grpc/CoreApiExceptionHandler.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java similarity index 98% rename from daemon/src/main/java/bisq/daemon/grpc/CoreApiExceptionHandler.java rename to daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java index 8f9a54a40b3..8a1c4c2836e 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/CoreApiExceptionHandler.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcExceptionHandler.java @@ -38,13 +38,13 @@ */ @Singleton @Slf4j -class CoreApiExceptionHandler { +class GrpcExceptionHandler { private final Predicate isExpectedException = (t) -> t instanceof IllegalStateException || t instanceof IllegalArgumentException; @Inject - public CoreApiExceptionHandler() { + public GrpcExceptionHandler() { } public void handleException(Throwable t, StreamObserver responseObserver) { diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java index d50d7eb477a..18cd5ef1c7c 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcGetTradeStatisticsService.java @@ -16,10 +16,10 @@ class GrpcGetTradeStatisticsService extends GetTradeStatisticsGrpc.GetTradeStatisticsImplBase { private final CoreApi coreApi; - private final CoreApiExceptionHandler exceptionHandler; + private final GrpcExceptionHandler exceptionHandler; @Inject - public GrpcGetTradeStatisticsService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { + public GrpcGetTradeStatisticsService(CoreApi coreApi, GrpcExceptionHandler exceptionHandler) { this.coreApi = coreApi; this.exceptionHandler = exceptionHandler; } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java index 2bc99ee4039..9b6690864f2 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcOffersService.java @@ -46,10 +46,10 @@ class GrpcOffersService extends OffersGrpc.OffersImplBase { private final CoreApi coreApi; - private final CoreApiExceptionHandler exceptionHandler; + private final GrpcExceptionHandler exceptionHandler; @Inject - public GrpcOffersService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { + public GrpcOffersService(CoreApi coreApi, GrpcExceptionHandler exceptionHandler) { this.coreApi = coreApi; this.exceptionHandler = exceptionHandler; } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java index 864ecc55e16..4decb5b1bae 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcPaymentAccountsService.java @@ -41,10 +41,10 @@ class GrpcPaymentAccountsService extends PaymentAccountsGrpc.PaymentAccountsImplBase { private final CoreApi coreApi; - private final CoreApiExceptionHandler exceptionHandler; + private final GrpcExceptionHandler exceptionHandler; @Inject - public GrpcPaymentAccountsService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { + public GrpcPaymentAccountsService(CoreApi coreApi, GrpcExceptionHandler exceptionHandler) { this.coreApi = coreApi; this.exceptionHandler = exceptionHandler; } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java index 5687aa554c3..c8d20fcf29b 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcPriceService.java @@ -33,10 +33,10 @@ class GrpcPriceService extends PriceGrpc.PriceImplBase { private final CoreApi coreApi; - private final CoreApiExceptionHandler exceptionHandler; + private final GrpcExceptionHandler exceptionHandler; @Inject - public GrpcPriceService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { + public GrpcPriceService(CoreApi coreApi, GrpcExceptionHandler exceptionHandler) { this.coreApi = coreApi; this.exceptionHandler = exceptionHandler; } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java index 3bf2b183cdd..b862e04ac95 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcTradesService.java @@ -47,10 +47,10 @@ class GrpcTradesService extends TradesGrpc.TradesImplBase { private final CoreApi coreApi; - private final CoreApiExceptionHandler exceptionHandler; + private final GrpcExceptionHandler exceptionHandler; @Inject - public GrpcTradesService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { + public GrpcTradesService(CoreApi coreApi, GrpcExceptionHandler exceptionHandler) { this.coreApi = coreApi; this.exceptionHandler = exceptionHandler; } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java index 6414a6ceb44..48c4d8a6b12 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcVersionService.java @@ -13,10 +13,10 @@ class GrpcVersionService extends GetVersionGrpc.GetVersionImplBase { private final CoreApi coreApi; - private final CoreApiExceptionHandler exceptionHandler; + private final GrpcExceptionHandler exceptionHandler; @Inject - public GrpcVersionService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { + public GrpcVersionService(CoreApi coreApi, GrpcExceptionHandler exceptionHandler) { this.coreApi = coreApi; this.exceptionHandler = exceptionHandler; } diff --git a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java index 5cf74d38dab..f8b06758070 100644 --- a/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java +++ b/daemon/src/main/java/bisq/daemon/grpc/GrpcWalletsService.java @@ -74,10 +74,10 @@ class GrpcWalletsService extends WalletsGrpc.WalletsImplBase { private final CoreApi coreApi; - private final CoreApiExceptionHandler exceptionHandler; + private final GrpcExceptionHandler exceptionHandler; @Inject - public GrpcWalletsService(CoreApi coreApi, CoreApiExceptionHandler exceptionHandler) { + public GrpcWalletsService(CoreApi coreApi, GrpcExceptionHandler exceptionHandler) { this.coreApi = coreApi; this.exceptionHandler = exceptionHandler; } From 672eb79f95147fcb1dcae83148c9a081e717ac2a Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 21 Dec 2020 15:05:54 -0300 Subject: [PATCH 14/20] Revert "Append nullable withdrawalTxId field to Trade proto message" This reverts commit 6aa385e494eb8fa870257c76e078108607503d03. --- core/src/main/java/bisq/core/trade/Trade.java | 7 ------- core/src/main/java/bisq/core/trade/TradeManager.java | 1 - proto/src/main/proto/pb.proto | 1 - 3 files changed, 9 deletions(-) diff --git a/core/src/main/java/bisq/core/trade/Trade.java b/core/src/main/java/bisq/core/trade/Trade.java index b25587eba97..de5c1949bb7 100644 --- a/core/src/main/java/bisq/core/trade/Trade.java +++ b/core/src/main/java/bisq/core/trade/Trade.java @@ -317,10 +317,6 @@ public static protobuf.Trade.TradePeriodState toProtoMessage(Trade.TradePeriodSt @Getter @Setter private String payoutTxId; - @Nullable - @Getter - @Setter - private String withdrawalTxId; @Getter @Setter private long tradeAmountAsLong; @@ -558,7 +554,6 @@ public Message toProtoMessage() { Optional.ofNullable(takerFeeTxId).ifPresent(builder::setTakerFeeTxId); Optional.ofNullable(depositTxId).ifPresent(builder::setDepositTxId); Optional.ofNullable(payoutTxId).ifPresent(builder::setPayoutTxId); - Optional.ofNullable(withdrawalTxId).ifPresent(builder::setWithdrawalTxId); Optional.ofNullable(tradingPeerNodeAddress).ifPresent(e -> builder.setTradingPeerNodeAddress(tradingPeerNodeAddress.toProtoMessage())); Optional.ofNullable(contract).ifPresent(e -> builder.setContract(contract.toProtoMessage())); Optional.ofNullable(contractAsJson).ifPresent(builder::setContractAsJson); @@ -592,7 +587,6 @@ public static Trade fromProto(Trade trade, protobuf.Trade proto, CoreProtoResolv trade.setTakerFeeTxId(ProtoUtil.stringOrNullFromProto(proto.getTakerFeeTxId())); trade.setDepositTxId(ProtoUtil.stringOrNullFromProto(proto.getDepositTxId())); trade.setPayoutTxId(ProtoUtil.stringOrNullFromProto(proto.getPayoutTxId())); - trade.setWithdrawalTxId(ProtoUtil.stringOrNullFromProto(proto.getWithdrawalTxId())); trade.setContract(proto.hasContract() ? Contract.fromProto(proto.getContract(), coreProtoResolver) : null); trade.setContractAsJson(ProtoUtil.stringOrNullFromProto(proto.getContractAsJson())); trade.setContractHash(ProtoUtil.byteArrayOrNullFromProto(proto.getContractHash())); @@ -1130,7 +1124,6 @@ public String toString() { ",\n takerFeeTxId='" + takerFeeTxId + '\'' + ",\n depositTxId='" + depositTxId + '\'' + ",\n payoutTxId='" + payoutTxId + '\'' + - ",\n withdrawalTxId='" + withdrawalTxId + '\'' + ",\n tradeAmountAsLong=" + tradeAmountAsLong + ",\n tradePrice=" + tradePrice + ",\n tradingPeerNodeAddress=" + tradingPeerNodeAddress + diff --git a/core/src/main/java/bisq/core/trade/TradeManager.java b/core/src/main/java/bisq/core/trade/TradeManager.java index d6c99e28e3e..dc87d4a98cc 100644 --- a/core/src/main/java/bisq/core/trade/TradeManager.java +++ b/core/src/main/java/bisq/core/trade/TradeManager.java @@ -497,7 +497,6 @@ public void onSuccess(@javax.annotation.Nullable Transaction transaction) { onTradeCompleted(trade); trade.setState(Trade.State.WITHDRAW_COMPLETED); getTradeProtocol(trade).onWithdrawCompleted(); - trade.setWithdrawalTxId(transaction.getTxId().toString()); requestPersistence(); resultHandler.handleResult(); } diff --git a/proto/src/main/proto/pb.proto b/proto/src/main/proto/pb.proto index a4579016f0c..22d81d40de9 100644 --- a/proto/src/main/proto/pb.proto +++ b/proto/src/main/proto/pb.proto @@ -1450,7 +1450,6 @@ message Trade { string counter_currency_extra_data = 37; string asset_tx_proof_result = 38; // name of AssetTxProofResult enum string uid = 39; - string withdrawal_tx_id = 40; } message BuyerAsMakerTrade { From bdde24a46322e4c55561ef46fb8bce295d960c35 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 21 Dec 2020 15:10:43 -0300 Subject: [PATCH 15/20] Ajust TradeInfo to reverting 6aa385e494eb8fa870257c76e078108607503d03 --- core/src/main/java/bisq/core/api/model/TradeInfo.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/core/src/main/java/bisq/core/api/model/TradeInfo.java b/core/src/main/java/bisq/core/api/model/TradeInfo.java index ec9880d5c69..1a717a7672e 100644 --- a/core/src/main/java/bisq/core/api/model/TradeInfo.java +++ b/core/src/main/java/bisq/core/api/model/TradeInfo.java @@ -47,7 +47,6 @@ public class TradeInfo implements Payload { private final String takerFeeTxId; private final String depositTxId; private final String payoutTxId; - private final String withdrawalTxId; private final long tradeAmountAsLong; private final long tradePrice; private final String tradingPeerNodeAddress; @@ -74,7 +73,6 @@ public TradeInfo(TradeInfoBuilder builder) { this.takerFeeTxId = builder.takerFeeTxId; this.depositTxId = builder.depositTxId; this.payoutTxId = builder.payoutTxId; - this.withdrawalTxId = builder.withdrawalTxId; this.tradeAmountAsLong = builder.tradeAmountAsLong; this.tradePrice = builder.tradePrice; this.tradingPeerNodeAddress = builder.tradingPeerNodeAddress; @@ -108,7 +106,6 @@ public static TradeInfo toTradeInfo(Trade trade, String role) { .withTakerFeeTxId(trade.getTakerFeeTxId()) .withDepositTxId(trade.getDepositTxId()) .withPayoutTxId(trade.getPayoutTxId()) - .withWithdrawalTxId(trade.getWithdrawalTxId()) .withTradeAmountAsLong(trade.getTradeAmountAsLong()) .withTradePrice(trade.getTradePrice().getValue()) .withTradingPeerNodeAddress(Objects.requireNonNull( @@ -144,7 +141,6 @@ public bisq.proto.grpc.TradeInfo toProtoMessage() { .setTakerFeeTxId(takerFeeTxId == null ? "" : takerFeeTxId) .setDepositTxId(depositTxId == null ? "" : depositTxId) .setPayoutTxId(payoutTxId == null ? "" : payoutTxId) - .setWithdrawalTxId(withdrawalTxId == null ? "" : withdrawalTxId) .setTradeAmountAsLong(tradeAmountAsLong) .setTradePrice(tradePrice) .setTradingPeerNodeAddress(tradingPeerNodeAddress) @@ -184,7 +180,6 @@ public static class TradeInfoBuilder { private String takerFeeTxId; private String depositTxId; private String payoutTxId; - private String withdrawalTxId; private long tradeAmountAsLong; private long tradePrice; private String tradingPeerNodeAddress; @@ -254,11 +249,6 @@ public TradeInfoBuilder withPayoutTxId(String payoutTxId) { return this; } - public TradeInfoBuilder withWithdrawalTxId(String withdrawalTxId) { - this.withdrawalTxId = withdrawalTxId; - return this; - } - public TradeInfoBuilder withTradeAmountAsLong(long tradeAmountAsLong) { this.tradeAmountAsLong = tradeAmountAsLong; return this; @@ -342,7 +332,6 @@ public String toString() { ", takerFeeTxId='" + takerFeeTxId + '\'' + "\n" + ", depositTxId='" + depositTxId + '\'' + "\n" + ", payoutTxId='" + payoutTxId + '\'' + "\n" + - ", withdrawalTxId='" + withdrawalTxId + '\'' + "\n" + ", tradeAmountAsLong='" + tradeAmountAsLong + '\'' + "\n" + ", tradePrice='" + tradePrice + '\'' + "\n" + ", tradingPeerNodeAddress='" + tradingPeerNodeAddress + '\'' + "\n" + From 64c2ac51699461c4ce5af65b4a93b62a06bc4b2d Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 21 Dec 2020 15:17:52 -0300 Subject: [PATCH 16/20] Adjust grpc.proto to reverting 6aa385e494eb8fa870257c76e078108607503d03 --- proto/src/main/proto/grpc.proto | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/proto/src/main/proto/grpc.proto b/proto/src/main/proto/grpc.proto index 81795112e55..df37f08fd82 100644 --- a/proto/src/main/proto/grpc.proto +++ b/proto/src/main/proto/grpc.proto @@ -273,20 +273,19 @@ message TradeInfo { string takerFeeTxId = 9; string depositTxId = 10; string payoutTxId = 11; - string withdrawalTxId = 12; - uint64 tradeAmountAsLong = 13; - uint64 tradePrice = 14; - string tradingPeerNodeAddress = 15; - string state = 16; - string phase = 17; - string tradePeriodState = 18; - bool isDepositPublished = 19; - bool isDepositConfirmed = 20; - bool isFiatSent = 21; - bool isFiatReceived = 22; - bool isPayoutPublished = 23; - bool isWithdrawn = 24; - string contractAsJson = 25; + uint64 tradeAmountAsLong = 12; + uint64 tradePrice = 13; + string tradingPeerNodeAddress = 14; + string state = 15; + string phase = 16; + string tradePeriodState = 17; + bool isDepositPublished = 18; + bool isDepositConfirmed = 19; + bool isFiatSent = 20; + bool isFiatReceived = 21; + bool isPayoutPublished = 22; + bool isWithdrawn = 23; + string contractAsJson = 24; } /////////////////////////////////////////////////////////////////////////////////////////// From 97dcac2a2d94ce768933c433c5ccfe8d966871f2 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 21 Dec 2020 15:19:10 -0300 Subject: [PATCH 17/20] Adjust TradeFormat to reverting 6aa385e494eb8fa870257c76e078108607503d03 --- cli/src/main/java/bisq/cli/TradeFormat.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/cli/src/main/java/bisq/cli/TradeFormat.java b/cli/src/main/java/bisq/cli/TradeFormat.java index 0c4d4d29498..1c286f03772 100644 --- a/cli/src/main/java/bisq/cli/TradeFormat.java +++ b/cli/src/main/java/bisq/cli/TradeFormat.java @@ -58,7 +58,6 @@ public static String format(TradeInfo tradeInfo) { + COL_HEADER_TRADE_FIAT_RECEIVED + COL_HEADER_DELIMITER + COL_HEADER_TRADE_PAYOUT_PUBLISHED + COL_HEADER_DELIMITER + COL_HEADER_TRADE_WITHDRAWN + COL_HEADER_DELIMITER - + (tradeInfo.getIsWithdrawn() ? COL_HEADER_TRADE_WITHDRAWAL_TX_ID + COL_HEADER_DELIMITER : "") + "%n"; String counterCurrencyCode = tradeInfo.getOffer().getCounterCurrencyCode(); @@ -79,8 +78,7 @@ public static String format(TradeInfo tradeInfo) { + " %-" + COL_HEADER_TRADE_FIAT_SENT.length() + "s" // lt justify + " %-" + COL_HEADER_TRADE_FIAT_RECEIVED.length() + "s" // lt justify + " %-" + COL_HEADER_TRADE_PAYOUT_PUBLISHED.length() + "s" // lt justify - + " %-" + COL_HEADER_TRADE_WITHDRAWN.length() + "s" // lt justify - + " %-" + COL_HEADER_TRADE_WITHDRAWAL_TX_ID.length() + "s"; // left + + " %-" + COL_HEADER_TRADE_WITHDRAWN.length() + "s"; // lt justify return headerLine + (isTaker @@ -100,8 +98,7 @@ private static String formatTradeForMaker(String format, TradeInfo tradeInfo) { tradeInfo.getIsFiatSent() ? "YES" : "NO", tradeInfo.getIsFiatReceived() ? "YES" : "NO", tradeInfo.getIsPayoutPublished() ? "YES" : "NO", - tradeInfo.getIsWithdrawn() ? "YES" : "NO", - tradeInfo.getIsWithdrawn() ? tradeInfo.getWithdrawalTxId() : ""); + tradeInfo.getIsWithdrawn() ? "YES" : "NO"); } private static String formatTradeForTaker(String format, TradeInfo tradeInfo) { @@ -117,7 +114,6 @@ private static String formatTradeForTaker(String format, TradeInfo tradeInfo) { tradeInfo.getIsFiatSent() ? "YES" : "NO", tradeInfo.getIsFiatReceived() ? "YES" : "NO", tradeInfo.getIsPayoutPublished() ? "YES" : "NO", - tradeInfo.getIsWithdrawn() ? "YES" : "NO", - tradeInfo.getIsWithdrawn() ? tradeInfo.getWithdrawalTxId() : ""); + tradeInfo.getIsWithdrawn() ? "YES" : "NO"); } } From 3a770f4bc0ba94297ba03712c06a0e0206f59f19 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 21 Dec 2020 15:22:05 -0300 Subject: [PATCH 18/20] Adjust TakeSellBTCOfferTest to reverting 6aa385e494eb8fa870257c76e078108607503d03 --- .../method/trade/TakeSellBTCOfferTest.java | 16 ---------------- 1 file changed, 16 deletions(-) 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 cd500cdd091..673792c4f55 100644 --- a/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBTCOfferTest.java +++ b/apitest/src/test/java/bisq/apitest/method/trade/TakeSellBTCOfferTest.java @@ -33,7 +33,6 @@ import static bisq.apitest.config.BisqAppConfig.alicedaemon; import static bisq.apitest.config.BisqAppConfig.bobdaemon; import static bisq.cli.CurrencyFormat.formatSatoshis; -import static bisq.cli.TransactionFormat.format; import static bisq.core.trade.Trade.Phase.*; import static bisq.core.trade.Trade.State.*; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -165,19 +164,4 @@ public void testBobsBtcWithdrawalToExternalAddress(final TestInfo testInfo) { testName(testInfo), formatSatoshis(currentBalance.getAvailableBalance())); } - - @Test - @Order(5) - public void testGetTradeWithdrawalTx(final TestInfo testInfo) { - var trade = getTrade(bobdaemon, tradeId); - var withdrawalTxId = trade.getWithdrawalTxId(); - assertNotNull(withdrawalTxId); - - var txInfo = getTransaction(bobdaemon, withdrawalTxId); - assertEquals(WITHDRAWAL_TX_MEMO, txInfo.getMemo()); - - log.debug("{} Trade withdrawal Tx:\n{}", - testName(testInfo), - format(txInfo)); - } } From 4aa4270ed925e59a09608d4ef411408246c0d406 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 21 Dec 2020 15:23:23 -0300 Subject: [PATCH 19/20] Adjust TradeTest to reverting 6aa385e494eb8fa870257c76e078108607503d03 --- apitest/src/test/java/bisq/apitest/scenario/TradeTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/apitest/src/test/java/bisq/apitest/scenario/TradeTest.java b/apitest/src/test/java/bisq/apitest/scenario/TradeTest.java index 410b72ca6cb..4c07452abc6 100644 --- a/apitest/src/test/java/bisq/apitest/scenario/TradeTest.java +++ b/apitest/src/test/java/bisq/apitest/scenario/TradeTest.java @@ -60,6 +60,5 @@ public void testTakeSellBTCOffer(final TestInfo testInfo) { test.testBobsConfirmPaymentStarted(testInfo); test.testAlicesConfirmPaymentReceived(testInfo); test.testBobsBtcWithdrawalToExternalAddress(testInfo); - test.testGetTradeWithdrawalTx(testInfo); } } From 1507a2c791294f3e8efaf39f12a62af8f5724f4c Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 21 Dec 2020 15:31:07 -0300 Subject: [PATCH 20/20] Resolve file conflict w/ master --- .../apitest/method/payment/CreatePaymentAccountTest.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java index 2fe5ed22abd..fe9daf27df0 100644 --- a/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java +++ b/apitest/src/test/java/bisq/apitest/method/payment/CreatePaymentAccountTest.java @@ -65,9 +65,12 @@ import static bisq.apitest.Scaffold.BitcoinCoreApp.bitcoind; import static bisq.apitest.config.BisqAppConfig.alicedaemon; -import static bisq.core.locale.CurrencyUtil.*; +import static bisq.core.locale.CurrencyUtil.getAllAdvancedCashCurrencies; +import static bisq.core.locale.CurrencyUtil.getAllMoneyGramCurrencies; +import static bisq.core.locale.CurrencyUtil.getAllRevolutCurrencies; +import static bisq.core.locale.CurrencyUtil.getAllUpholdCurrencies; import static bisq.core.payment.payload.PaymentMethod.*; -import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.MethodOrderer.OrderAnnotation; @@ -746,7 +749,6 @@ public void testCreateTransferwiseAccount(TestInfo testInfo) { String jsonString = getCompletedFormAsJsonString(); TransferwiseAccount paymentAccount = (TransferwiseAccount) createPaymentAccount(alicedaemon, jsonString); verifyUserPayloadHasPaymentAccountWithId(paymentAccount.getId()); - verifyUserPayloadHasPaymentAccountWithId(paymentAccount.getId()); // As per commit 88f26f93241af698ae689bf081205d0f9dc929fa // Do not autofill all currencies by default but keep all unselected. // verifyAccountTradeCurrencies(getAllTransferwiseCurrencies(), paymentAccount);