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 1/8] 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 2/8] 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 3/8] 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 4/8] 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 5/8] 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 6/8] 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 7/8] 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 4a09275111cd0428b2d376188f70445eab1af040 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 14 Dec 2020 11:31:40 -0300 Subject: [PATCH 8/8] Adjust create TransferwiseAccount test As per commit 88f26f93241af698ae689bf081205d0f9dc929fa, do not autofill all currencies by default but keep all unselected. --- .../apitest/method/payment/CreatePaymentAccountTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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..2ef2809eb0b 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,10 @@ public void testCreateTransferwiseAccount(TestInfo testInfo) { String jsonString = getCompletedFormAsJsonString(); TransferwiseAccount paymentAccount = (TransferwiseAccount) createPaymentAccount(alicedaemon, jsonString); verifyUserPayloadHasPaymentAccountWithId(paymentAccount.getId()); - verifyAccountTradeCurrencies(getAllTransferwiseCurrencies(), paymentAccount); + // As per commit 88f26f93241af698ae689bf081205d0f9dc929fa + // Do not autofill all currencies by default but keep all unselected. + // verifyAccountTradeCurrencies(getAllTransferwiseCurrencies(), paymentAccount); + assertEquals(0, paymentAccount.getTradeCurrencies().size()); verifyCommonFormEntries(paymentAccount); assertEquals(COMPLETED_FORM_MAP.get(PROPERTY_NAME_EMAIL), paymentAccount.getEmail()); log.debug("Deserialized {}: {}", paymentAccount.getClass().getSimpleName(), paymentAccount);