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/2] 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 e2e485d05fbf50c04cf6b32a36925e4afb907f13 Mon Sep 17 00:00:00 2001 From: ghubstan <36207203+ghubstan@users.noreply.github.com> Date: Mon, 14 Dec 2020 11:46:50 -0300 Subject: [PATCH 2/2] 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);