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.

Sign up for GitHub

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

Ignore dust utxo #2621

Merged
merged 2 commits into from
Apr 17, 2019
Merged

Ignore dust utxo #2621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
21c5eec
Remove referralIdInputTextField
ManfredKarrer Apr 3, 2019
fcbc3b6
Ignore utxo below a user defined threshold to avoid dust attacks
ManfredKarrer Apr 3, 2019
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
1 change: 1 addition & 0 deletions common/src/main/proto/pb.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1351,6 +1351,7 @@ message PreferencesPayload {
string rpc_pw = 48;
string take_offer_selected_payment_account_id = 49;
double buyer_security_deposit_as_percent = 50;
int32 ignore_dust_threshold = 51;
}

///////////////////////////////////////////////////////////////////////////////////////////
Expand Down
26 changes: 15 additions & 11 deletions core/src/main/java/bisq/core/btc/wallet/BisqDefaultCoinSelector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,24 +75,28 @@ public CoinSelection select(Coin target, List<TransactionOutput> candidates) {
long total = 0;
long targetValue = target.value;
for (TransactionOutput output : sortedOutputs) {
if (total >= targetValue) {
long change = total - targetValue;
if (change == 0 || change >= Restrictions.getMinNonDustOutput().value)
break;
}

if (output.getParentTransaction() != null &&
isTxSpendable(output.getParentTransaction()) &&
isTxOutputSpendable(output)) {
selected.add(output);
total += output.getValue().value;
if (!isDustAttackUtxo(output)) {
if (total >= targetValue) {
long change = total - targetValue;
if (change == 0 || change >= Restrictions.getMinNonDustOutput().value)
break;
}

if (output.getParentTransaction() != null &&
isTxSpendable(output.getParentTransaction()) &&
isTxOutputSpendable(output)) {
selected.add(output);
total += output.getValue().value;
}
}
}
// Total may be lower than target here, if the given candidates were insufficient to create to requested
// transaction.
return new CoinSelection(Coin.valueOf(total), selected);
}

protected abstract boolean isDustAttackUtxo(TransactionOutput output);

public Coin getChange(Coin target, CoinSelection coinSelection) throws InsufficientMoneyException {
long value = target.value;
long available = coinSelection.valueGathered.value;
Expand Down
7 changes: 7 additions & 0 deletions core/src/main/java/bisq/core/btc/wallet/BsqCoinSelector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,11 @@ protected boolean isTxOutputSpendable(TransactionOutput output) {
// check if it is an own change output.
return unconfirmedBsqChangeOutputListService.hasTransactionOutput(output);
}

// For BSQ we do not check for dust attack utxos as they are 5.46 BSQ and a considerable value.
// The default 546 sat dust limit is handled in the BitcoinJ side anyway.
@Override
protected boolean isDustAttackUtxo(TransactionOutput output) {
return false;
}
}
7 changes: 7 additions & 0 deletions core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -669,4 +669,11 @@ public Address getUnusedAddress() {
public String getUnusedBsqAddressAsString() {
return "B" + getUnusedAddress().toBase58();
}

// For BSQ we do not check for dust attack utxos as they are 5.46 BSQ and a considerable value.
// The default 546 sat dust limit is handled in the BitcoinJ side anyway.
@Override
protected boolean isDustAttackUtxo(TransactionOutput output) {
return false;
}
}
30 changes: 19 additions & 11 deletions core/src/main/java/bisq/core/btc/wallet/BtcCoinSelector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,38 +24,38 @@

import java.util.Set;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import lombok.extern.slf4j.Slf4j;

/**
* We use a specialized version of the CoinSelector based on the DefaultCoinSelector implementation.
* We lookup for spendable outputs which matches any of our addresses.
*/
@Slf4j
class BtcCoinSelector extends BisqDefaultCoinSelector {
private static final Logger log = LoggerFactory.getLogger(BtcCoinSelector.class);

private final Set<Address> addresses;
private final long ignoreDustThreshold;


///////////////////////////////////////////////////////////////////////////////////////////
// Constructor
///////////////////////////////////////////////////////////////////////////////////////////

BtcCoinSelector(Set<Address> addresses, boolean permitForeignPendingTx) {
private BtcCoinSelector(Set<Address> addresses, long ignoreDustThreshold, boolean permitForeignPendingTx) {
super(permitForeignPendingTx);
this.addresses = addresses;
this.ignoreDustThreshold = ignoreDustThreshold;
}

BtcCoinSelector(Set<Address> addresses) {
this(addresses, true);
BtcCoinSelector(Set<Address> addresses, long ignoreDustThreshold) {
this(addresses, ignoreDustThreshold, true);
}

BtcCoinSelector(Address address, @SuppressWarnings("SameParameterValue") boolean permitForeignPendingTx) {
this(Sets.newHashSet(address), permitForeignPendingTx);
BtcCoinSelector(Address address, long ignoreDustThreshold, @SuppressWarnings("SameParameterValue") boolean permitForeignPendingTx) {
this(Sets.newHashSet(address), ignoreDustThreshold, permitForeignPendingTx);
}

BtcCoinSelector(Address address) {
this(Sets.newHashSet(address), true);
BtcCoinSelector(Address address, long ignoreDustThreshold) {
this(Sets.newHashSet(address), ignoreDustThreshold, true);
}

@Override
Expand All @@ -68,4 +68,12 @@ protected boolean isTxOutputSpendable(TransactionOutput output) {
return false;
}
}

// We ignore utxos which are considered dust attacks for spying on users wallets.
// The ignoreDustThreshold value is set in the preferences. If not set we use default non dust
// value of 546 sat.
@Override
protected boolean isDustAttackUtxo(TransactionOutput output) {
return output.getValue().value < ignoreDustThreshold;
}
}
56 changes: 35 additions & 21 deletions core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,17 +202,18 @@ private Transaction completePreparedProposalTx(Transaction feeTx, byte[] opRetur
// safety check counter to avoid endless loops
int counter = 0;
// estimated size of input sig
final int sigSizePerInput = 106;
int sigSizePerInput = 106;
// typical size for a tx with 3 inputs
int txSizeWithUnsignedInputs = 300;
final Coin txFeePerByte = feeService.getTxFeePerByte();
Coin txFeePerByte = feeService.getTxFeePerByte();

Address changeAddress = getFreshAddressEntry().getAddress();
checkNotNull(changeAddress, "changeAddress must not be null");

final BtcCoinSelector coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE));
final List<TransactionInput> preparedBsqTxInputs = preparedTx.getInputs();
final List<TransactionOutput> preparedBsqTxOutputs = preparedTx.getOutputs();
BtcCoinSelector coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE),
preferences.getIgnoreDustThreshold());
List<TransactionInput> preparedBsqTxInputs = preparedTx.getInputs();
List<TransactionOutput> preparedBsqTxOutputs = preparedTx.getOutputs();
int numInputs = preparedBsqTxInputs.size();
Transaction resultTx = null;
boolean isFeeOutsideTolerance;
Expand Down Expand Up @@ -309,17 +310,18 @@ private Transaction addInputsForMinerFee(Transaction preparedTx, byte[] opReturn
// safety check counter to avoid endless loops
int counter = 0;
// estimated size of input sig
final int sigSizePerInput = 106;
int sigSizePerInput = 106;
// typical size for a tx with 3 inputs
int txSizeWithUnsignedInputs = 300;
final Coin txFeePerByte = feeService.getTxFeePerByte();
Coin txFeePerByte = feeService.getTxFeePerByte();

Address changeAddress = getFreshAddressEntry().getAddress();
checkNotNull(changeAddress, "changeAddress must not be null");

final BtcCoinSelector coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE));
final List<TransactionInput> preparedBsqTxInputs = preparedTx.getInputs();
final List<TransactionOutput> preparedBsqTxOutputs = preparedTx.getOutputs();
BtcCoinSelector coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE),
preferences.getIgnoreDustThreshold());
List<TransactionInput> preparedBsqTxInputs = preparedTx.getInputs();
List<TransactionOutput> preparedBsqTxOutputs = preparedTx.getOutputs();
int numInputs = preparedBsqTxInputs.size();
Transaction resultTx = null;
boolean isFeeOutsideTolerance;
Expand Down Expand Up @@ -444,20 +446,21 @@ public Transaction completePreparedBsqTx(Transaction preparedBsqTx, boolean useC
// safety check counter to avoid endless loops
int counter = 0;
// estimated size of input sig
final int sigSizePerInput = 106;
int sigSizePerInput = 106;
// typical size for a tx with 2 inputs
int txSizeWithUnsignedInputs = 203;
// If useCustomTxFee we allow overriding the estimated fee from preferences
final Coin txFeePerByte = useCustomTxFee ? getTxFeeForWithdrawalPerByte() : feeService.getTxFeePerByte();
Coin txFeePerByte = useCustomTxFee ? getTxFeeForWithdrawalPerByte() : feeService.getTxFeePerByte();
// In case there are no change outputs we force a change by adding min dust to the BTC input
Coin forcedChangeValue = Coin.ZERO;

Address changeAddress = getFreshAddressEntry().getAddress();
checkNotNull(changeAddress, "changeAddress must not be null");

final BtcCoinSelector coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE));
final List<TransactionInput> preparedBsqTxInputs = preparedBsqTx.getInputs();
final List<TransactionOutput> preparedBsqTxOutputs = preparedBsqTx.getOutputs();
BtcCoinSelector coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE),
preferences.getIgnoreDustThreshold());
List<TransactionInput> preparedBsqTxInputs = preparedBsqTx.getInputs();
List<TransactionOutput> preparedBsqTxOutputs = preparedBsqTx.getOutputs();
int numInputs = preparedBsqTxInputs.size() + 1; // We add 1 for the BTC fee input
Transaction resultTx = null;
boolean isFeeOutsideTolerance;
Expand Down Expand Up @@ -781,7 +784,7 @@ public void doubleSpendTransaction(String txId, Runnable resultHandler, ErrorMes
sendRequest.feePerKb = Coin.ZERO;
sendRequest.ensureMinRequiredFee = false;
sendRequest.aesKey = aesKey;
sendRequest.coinSelector = new BtcCoinSelector(toAddress);
sendRequest.coinSelector = new BtcCoinSelector(toAddress, preferences.getIgnoreDustThreshold());
sendRequest.changeAddress = toAddress;
wallet.completeTx(sendRequest);
tx = sendRequest.tx;
Expand All @@ -802,7 +805,7 @@ public void doubleSpendTransaction(String txId, Runnable resultHandler, ErrorMes
sendRequest.feePerKb = Coin.ZERO;
sendRequest.ensureMinRequiredFee = false;
sendRequest.aesKey = aesKey;
sendRequest.coinSelector = new BtcCoinSelector(toAddress);
sendRequest.coinSelector = new BtcCoinSelector(toAddress, preferences.getIgnoreDustThreshold());
sendRequest.changeAddress = toAddress;
sendResult = wallet.sendCoins(sendRequest);
} catch (InsufficientMoneyException e) {
Expand All @@ -818,7 +821,8 @@ public void doubleSpendTransaction(String txId, Runnable resultHandler, ErrorMes
sendRequest.feePerKb = Coin.ZERO;
sendRequest.ensureMinRequiredFee = false;
sendRequest.aesKey = aesKey;
sendRequest.coinSelector = new BtcCoinSelector(toAddress, false);
sendRequest.coinSelector = new BtcCoinSelector(toAddress,
preferences.getIgnoreDustThreshold(), false);
sendRequest.changeAddress = toAddress;

try {
Expand Down Expand Up @@ -972,7 +976,8 @@ public int getEstimatedFeeTxSize(List<Coin> outputValues, Coin txFee)
SendRequest sendRequest = SendRequest.forTx(transaction);
sendRequest.shuffleOutputs = false;
sendRequest.aesKey = aesKey;
sendRequest.coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE));
sendRequest.coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE),
preferences.getIgnoreDustThreshold());
sendRequest.fee = txFee;
sendRequest.feePerKb = Coin.ZERO;
sendRequest.ensureMinRequiredFee = false;
Expand Down Expand Up @@ -1044,7 +1049,7 @@ private SendRequest getSendRequest(String fromAddress,

checkNotNull(addressEntry.get(), "addressEntry.get() must not be null");
checkNotNull(addressEntry.get().getAddress(), "addressEntry.get().getAddress() must not be null");
sendRequest.coinSelector = new BtcCoinSelector(addressEntry.get().getAddress());
sendRequest.coinSelector = new BtcCoinSelector(addressEntry.get().getAddress(), preferences.getIgnoreDustThreshold());
sendRequest.changeAddress = addressEntry.get().getAddress();
return sendRequest;
}
Expand Down Expand Up @@ -1089,7 +1094,8 @@ private SendRequest getSendRequestForMultipleAddresses(Set<String> fromAddresses
if (addressEntries.isEmpty())
throw new AddressEntryException("No Addresses for withdraw found in our wallet");

sendRequest.coinSelector = new BtcCoinSelector(walletsSetup.getAddressesFromAddressEntries(addressEntries));
sendRequest.coinSelector = new BtcCoinSelector(walletsSetup.getAddressesFromAddressEntries(addressEntries),
preferences.getIgnoreDustThreshold());
Optional<AddressEntry> addressEntryOptional = Optional.<AddressEntry>empty();
AddressEntry changeAddressAddressEntry = null;
if (changeAddress != null)
Expand All @@ -1100,4 +1106,12 @@ private SendRequest getSendRequestForMultipleAddresses(Set<String> fromAddresses
sendRequest.changeAddress = changeAddressAddressEntry.getAddress();
return sendRequest;
}

// We ignore utxos which are considered dust attacks for spying on users wallets.
// The ignoreDustThreshold value is set in the preferences. If not set we use default non dust
// value of 546 sat.
@Override
protected boolean isDustAttackUtxo(TransactionOutput output) {
return output.getValue().value < preferences.getIgnoreDustThreshold();
}
}
6 changes: 6 additions & 0 deletions core/src/main/java/bisq/core/btc/wallet/NonBsqCoinSelector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,10 @@ protected boolean isTxOutputSpendable(TransactionOutput output) {
// So we consider any txOutput which is not in the state as BTC output.
return !daoStateService.existsTxOutput(key) || daoStateService.isRejectedIssuanceOutput(key);
}

// BTC utxo in the BSQ wallet are usually from rejected comp request so we don't expect dust attack utxos here.
@Override
protected boolean isDustAttackUtxo(TransactionOutput output) {
return false;
}
}
27 changes: 17 additions & 10 deletions core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import bisq.core.btc.setup.WalletConfig;
import bisq.core.btc.setup.WalletsSetup;
import bisq.core.locale.Res;
import bisq.core.user.Preferences;

import org.bitcoinj.core.Address;
import org.bitcoinj.core.AddressFormatException;
Expand Down Expand Up @@ -115,6 +116,7 @@ public class TradeWalletService {
private static final Logger log = LoggerFactory.getLogger(TradeWalletService.class);

private final WalletsSetup walletsSetup;
private final Preferences preferences;
private final NetworkParameters params;

@Nullable
Expand All @@ -130,8 +132,9 @@ public class TradeWalletService {
///////////////////////////////////////////////////////////////////////////////////////////

@Inject
public TradeWalletService(WalletsSetup walletsSetup) {
public TradeWalletService(WalletsSetup walletsSetup, Preferences preferences) {
this.walletsSetup = walletsSetup;
this.preferences = preferences;
this.params = BisqEnvironment.getParameters();
walletsSetup.addSetupCompletedHandler(() -> {
walletConfig = walletsSetup.getWalletConfig();
Expand Down Expand Up @@ -198,10 +201,12 @@ public Transaction createBtcTradingFeeTx(Address fundingAddress,
sendRequest = SendRequest.forTx(tradingFeeTx);
sendRequest.shuffleOutputs = false;
sendRequest.aesKey = aesKey;
if (useSavingsWallet)
sendRequest.coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE));
else
sendRequest.coinSelector = new BtcCoinSelector(fundingAddress);
if (useSavingsWallet) {
sendRequest.coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE),
preferences.getIgnoreDustThreshold());
} else {
sendRequest.coinSelector = new BtcCoinSelector(fundingAddress, preferences.getIgnoreDustThreshold());
}
// We use a fixed fee

sendRequest.fee = txFee;
Expand Down Expand Up @@ -278,10 +283,12 @@ public Transaction completeBsqTradingFeeTx(Transaction preparedBsqTx,
SendRequest sendRequest = SendRequest.forTx(preparedBsqTx);
sendRequest.shuffleOutputs = false;
sendRequest.aesKey = aesKey;
if (useSavingsWallet)
sendRequest.coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE));
else
sendRequest.coinSelector = new BtcCoinSelector(fundingAddress);
if (useSavingsWallet) {
sendRequest.coinSelector = new BtcCoinSelector(walletsSetup.getAddressesByContext(AddressEntry.Context.AVAILABLE),
preferences.getIgnoreDustThreshold());
} else {
sendRequest.coinSelector = new BtcCoinSelector(fundingAddress, preferences.getIgnoreDustThreshold());
}
// We use a fixed fee
sendRequest.fee = txFee;
sendRequest.feePerKb = Coin.ZERO;
Expand Down Expand Up @@ -1196,7 +1203,7 @@ private void addAvailableInputsAndChangeOutputs(Transaction transaction, Address
sendRequest.feePerKb = Coin.ZERO;
sendRequest.ensureMinRequiredFee = false;
// we allow spending of unconfirmed tx (double spend risk is low and usability would suffer if we need to wait for 1 confirmation)
sendRequest.coinSelector = new BtcCoinSelector(address);
sendRequest.coinSelector = new BtcCoinSelector(address, preferences.getIgnoreDustThreshold());
// We use always the same address in a trade for all transactions
sendRequest.changeAddress = changeAddress;
// With the usage of completeTx() we get all the work done with fee calculation, validation and coin selection.
Expand Down
12 changes: 8 additions & 4 deletions core/src/main/java/bisq/core/btc/wallet/WalletService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,14 +423,18 @@ public Coin getBalanceForAddress(Address address) {
protected Coin getBalance(List<TransactionOutput> transactionOutputs, Address address) {
Coin balance = Coin.ZERO;
for (TransactionOutput output : transactionOutputs) {
if (isOutputScriptConvertibleToAddress(output) &&
address != null &&
address.equals(getAddressFromOutput(output)))
balance = balance.add(output.getValue());
if (!isDustAttackUtxo(output)) {
if (isOutputScriptConvertibleToAddress(output) &&
address != null &&
address.equals(getAddressFromOutput(output)))
balance = balance.add(output.getValue());
}
}
return balance;
}

protected abstract boolean isDustAttackUtxo(TransactionOutput output);

public Coin getBalance(TransactionOutput output) {
return getBalanceForAddress(getAddressFromOutput(output));
}
Expand Down
Loading