Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix taker use all BSQ for fee payment #4354

Merged
merged 1 commit into from
Jul 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,7 @@ private void addInputsAndChangeOutputForTx(Transaction tx,
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 "
Expand Down
25 changes: 17 additions & 8 deletions core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;

import org.slf4j.Logger;
Expand Down Expand Up @@ -219,25 +220,31 @@ public Transaction completeBsqTradingFeeTx(Transaction preparedBsqTx,
// 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);

// we allow spending of unconfirmed tx (double spend risk is low and usability would suffer if we need to
// 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;
Expand Down Expand Up @@ -335,6 +342,8 @@ OUT[0] dummyOutputAmount (inputAmount - tx fee)

// 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,14 @@ protected void run() {
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);
Expand Down