From a8e4b5170907c207f234b607fb7dafcce747311b Mon Sep 17 00:00:00 2001 From: sqrrm Date: Mon, 6 Jul 2020 18:36:30 +0200 Subject: [PATCH] Fix taker use all BSQ for fee payment Issue: if a taker used exactly all BSQ from the BSQ inputs to pay the trading fee, there was no BSQ change in the takeOfferFeeTx. It was assumed that the second output was the reservedForTrade output, but in the case of missing BSQ change it was the first output. Fix: added a check to make sure the value of the inputs to the deposit tx match the expected inputAmount. Added a check that if there is no BSQ outputs in the bsqTradingFeeTx a change output is added of value 1 satoshi more than the BSQ input value. This ensures that the second output is always the reservedForTrade output. It also ensures that the BSQ is burnt, even in the very unlikely case that the amount of BSQ burnt is larger than the reservedForTrade amount. --- .../core/btc/wallet/BsqWalletService.java | 1 + .../core/btc/wallet/TradeWalletService.java | 21 +++++++++++++------ .../BuyerAsTakerCreatesDepositTxInputs.java | 9 ++------ 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java index 22cdb9ce6b..930a19e270 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java @@ -694,6 +694,7 @@ public class BsqWalletService extends WalletService implements DaoStateListener coinSelection.gathered.forEach(tx::addInput); try { Coin change = bsqCoinSelector.getChange(fee, coinSelection); + // Change can be ZERO, then no change output is created so don't rely on a BSQ change output if (change.isPositive()) { checkArgument(Restrictions.isAboveDust(change), "The change output of " + change.value / 100d + " BSQ is below the min. dust value of " diff --git a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java index 7f2692d1e4..07671fe687 100644 --- a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java @@ -58,6 +58,7 @@ import org.spongycastle.crypto.params.KeyParameter; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.stream.Collectors; import org.slf4j.Logger; @@ -219,13 +220,24 @@ public class TradeWalletService { // outputs [0-1] BTC change output // mining fee: BTC mining fee + burned BSQ fee - // In case of txs for burned BSQ fees we have no receiver output and it might be that there are no change outputs + // In case all BSQ were burnt as fees we have no receiver output and it might be that there are no change outputs // We need to guarantee that min. 1 valid output is added (OP_RETURN does not count). So we use a higher input // for BTC to force an additional change output. final int preparedBsqTxInputsSize = preparedBsqTx.getInputs().size(); + final boolean hasBsqOutputs = !preparedBsqTx.getOutputs().isEmpty(); + // If there are no BSQ change outputs an output larger than the burnt BSQ amount has to be added as the first + // output to make sure the reserved funds are in output 1, deposit tx input creation depends on the reserve + // being output 1. The amount has to be larger than the BSQ input to make sure the inputs get burnt. + // The BTC changeAddress is used, so it might get used for both output 0 and output 2. + if (!hasBsqOutputs) { + var bsqInputValue = preparedBsqTx.getInputs().stream() + .map(TransactionInput::getValue) + .reduce(Coin.valueOf(0), Coin::add); + preparedBsqTx.addOutput(bsqInputValue.add(Coin.valueOf(1)), changeAddress); + } // the reserved amount we need for the trade we send to our trade reservedForTradeAddress preparedBsqTx.addOutput(reservedFundsForOffer, reservedForTradeAddress); @@ -233,11 +245,6 @@ public class TradeWalletService { // wait for 1 confirmation) // In case of double spend we will detect later in the trade process and use a ban score to penalize bad behaviour (not impl. yet) - // If BSQ trade fee > reservedFundsForOffer we would create a BSQ output instead of a BTC output. - // As the min. reservedFundsForOffer is 0.001 BTC which is 1000 BSQ this is an unrealistic scenario and not - // handled atm (if BTC price is 1M USD and BSQ price is 0.1 USD, then fee would be 10% which still is unrealistic). - - // WalletService.printTx("preparedBsqTx", preparedBsqTx); SendRequest sendRequest = SendRequest.forTx(preparedBsqTx); sendRequest.shuffleOutputs = false; sendRequest.aesKey = aesKey; @@ -335,6 +342,8 @@ public class TradeWalletService { // We created the take offer fee tx in the structure that the second output is for the funds for the deposit tx. TransactionOutput reservedForTradeOutput = takeOfferFeeTx.getOutputs().get(1); + checkArgument(reservedForTradeOutput.getValue().equals(inputAmount), + "Reserve amount does not equal input amount"); dummyTX.addInput(reservedForTradeOutput); WalletService.removeSignatures(dummyTX); diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer_as_taker/BuyerAsTakerCreatesDepositTxInputs.java b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer_as_taker/BuyerAsTakerCreatesDepositTxInputs.java index ad0efc70bf..39af203b69 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/buyer_as_taker/BuyerAsTakerCreatesDepositTxInputs.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/buyer_as_taker/BuyerAsTakerCreatesDepositTxInputs.java @@ -42,19 +42,14 @@ public class BuyerAsTakerCreatesDepositTxInputs extends TradeTask { try { runInterceptHook(); - // In case we pay the taker fee in bsq we reduce tx fee by that as the burned bsq satoshis goes to miners. - Coin bsqTakerFee = trade.isCurrencyForTakerFeeBtc() ? Coin.ZERO : trade.getTakerFee(); - Coin txFee = trade.getTxFee(); Coin takerInputAmount = checkNotNull(trade.getOffer()).getBuyerSecurityDeposit() .add(txFee) - .add(txFee) - .subtract(bsqTakerFee); - Coin fee = txFee.subtract(bsqTakerFee); + .add(txFee); InputsAndChangeOutput result = processModel.getTradeWalletService().takerCreatesDepositTxInputs( processModel.getTakeOfferFeeTx(), takerInputAmount, - fee); + txFee); processModel.setRawTransactionInputs(result.rawTransactionInputs); processModel.setChangeOutputValue(result.changeOutputValue); processModel.setChangeOutputAddress(result.changeOutputAddress);