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

Add custom withdrawal transaction fee options on Send funds (BTC) #5312

Merged
merged 4 commits into from
Apr 20, 2021
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
9 changes: 6 additions & 3 deletions core/src/main/resources/i18n/displayStrings.properties
Original file line number Diff line number Diff line change
Expand Up @@ -1029,6 +1029,11 @@ funds.withdrawal.fillDestAddress=Fill in your destination address
funds.withdrawal.warn.noSourceAddressSelected=You need to select a source address in the table above.
funds.withdrawal.warn.amountExceeds=You don't have sufficient funds available from the selected address.\n\
Consider to select multiple addresses in the table above or change the fee toggle to include the miner fee.
funds.withdrawal.txFee=Withdrawal transaction fee (satoshis/vbyte)
funds.withdrawal.useCustomFeeValueInfo=Insert a custom transaction fee value
funds.withdrawal.useCustomFeeValue=Use custom value
funds.withdrawal.txFeeMin=Transaction fee must be at least {0} satoshis/vbyte
funds.withdrawal.txFeeTooLarge=Your input is above any reasonable value (>5000 satoshis/vbyte). Transaction fee is usually in the range of 50-400 satoshis/vbyte.

funds.reserved.noFunds=No funds are reserved in open offers
funds.reserved.reserved=Reserved in local wallet for offer with ID: {0}
Expand Down Expand Up @@ -1199,10 +1204,8 @@ setting.preferences.autoConfirmRequiredConfirmations=Required confirmations
setting.preferences.autoConfirmMaxTradeSize=Max. trade amount (BTC)
setting.preferences.autoConfirmServiceAddresses=Monero Explorer URLs (uses Tor, except for localhost, LAN IP addresses, and *.local hostnames)
setting.preferences.deviationToLarge=Values higher than {0}% are not allowed.
setting.preferences.txFee=Withdrawal transaction fee (satoshis/vbyte)
setting.preferences.txFee=BSQ Withdrawal transaction fee (satoshis/vbyte)
setting.preferences.useCustomValue=Use custom value
setting.preferences.txFeeMin=Transaction fee must be at least {0} satoshis/vbyte
setting.preferences.txFeeTooLarge=Your input is above any reasonable value (>5000 satoshis/vbyte). Transaction fee is usually in the range of 50-400 satoshis/vbyte.
setting.preferences.ignorePeers=Ignored peers [onion address:port]
setting.preferences.ignoreDustThreshold=Min. non-dust output value
setting.preferences.currenciesInList=Currencies in market price feed list
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import bisq.desktop.components.AutoTooltipLabel;
import bisq.desktop.components.ExternalHyperlink;
import bisq.desktop.components.HyperlinkWithIcon;
import bisq.desktop.components.InputTextField;
import bisq.desktop.components.TitledGroupBg;
import bisq.desktop.main.overlays.popups.Popup;
import bisq.desktop.main.overlays.windows.TxDetails;
Expand All @@ -38,14 +39,14 @@
import bisq.core.btc.wallet.BtcWalletService;
import bisq.core.btc.wallet.Restrictions;
import bisq.core.locale.Res;
import bisq.core.provider.fee.FeeService;
import bisq.core.trade.Trade;
import bisq.core.trade.TradeManager;
import bisq.core.user.DontShowAgainLookup;
import bisq.core.user.Preferences;
import bisq.core.util.FormattingUtils;
import bisq.core.util.ParsingUtils;
import bisq.core.util.coin.CoinFormatter;
import bisq.core.util.coin.CoinUtil;
import bisq.core.util.validation.BtcAddressValidator;

import bisq.network.p2p.P2PService;
Expand Down Expand Up @@ -79,6 +80,7 @@
import javafx.scene.control.TableView;
import javafx.scene.control.TextField;
import javafx.scene.control.Toggle;
import javafx.scene.control.ToggleButton;
import javafx.scene.control.ToggleGroup;
import javafx.scene.control.Tooltip;
import javafx.scene.layout.GridPane;
Expand Down Expand Up @@ -124,7 +126,7 @@ public class WithdrawalView extends ActivatableView<VBox, Void> {

private RadioButton useAllInputsRadioButton, useCustomInputsRadioButton, feeExcludedRadioButton, feeIncludedRadioButton;
private Label amountLabel;
private TextField amountTextField, withdrawFromTextField, withdrawToTextField, withdrawMemoTextField;
private TextField amountTextField, withdrawFromTextField, withdrawToTextField, withdrawMemoTextField, transactionFeeInputTextField;

private final BtcWalletService btcWalletService;
private final TradeManager tradeManager;
Expand All @@ -143,12 +145,15 @@ public class WithdrawalView extends ActivatableView<VBox, Void> {
private Coin amountAsCoin = Coin.ZERO;
private Coin sendersAmount = Coin.ZERO;
private ChangeListener<String> amountListener;
private ChangeListener<Boolean> amountFocusListener;
private ChangeListener<Boolean> amountFocusListener, useCustomFeeCheckboxListener, transactionFeeFocusedListener;
private ChangeListener<Toggle> feeToggleGroupListener, inputsToggleGroupListener;
private ChangeListener<Number> transactionFeeChangeListener;
private ToggleGroup feeToggleGroup, inputsToggleGroup;
private ToggleButton useCustomFee;
private final BooleanProperty useAllInputs = new SimpleBooleanProperty(true);
private boolean feeExcluded;
private int rowIndex = 0;
private final FeeService feeService;


///////////////////////////////////////////////////////////////////////////////////////////
Expand All @@ -163,7 +168,8 @@ private WithdrawalView(BtcWalletService btcWalletService,
@Named(FormattingUtils.BTC_FORMATTER_KEY) CoinFormatter formatter,
Preferences preferences,
BtcAddressValidator btcAddressValidator,
WalletPasswordWindow walletPasswordWindow) {
WalletPasswordWindow walletPasswordWindow,
FeeService feeService) {
this.btcWalletService = btcWalletService;
this.tradeManager = tradeManager;
this.p2PService = p2PService;
Expand All @@ -172,6 +178,7 @@ private WithdrawalView(BtcWalletService btcWalletService,
this.preferences = preferences;
this.btcAddressValidator = btcAddressValidator;
this.walletPasswordWindow = walletPasswordWindow;
this.feeService = feeService;
}

@Override
Expand Down Expand Up @@ -221,6 +228,52 @@ public void initialize() {
withdrawMemoTextField = addTopLabelInputTextField(gridPane, ++rowIndex,
Res.get("funds.withdrawal.memoLabel", Res.getBaseCurrencyCode())).second;

Tuple3<Label, InputTextField, ToggleButton> customFeeTuple = addTopLabelInputTextFieldSlideToggleButton(gridPane, ++rowIndex,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're reorganising 40 lines of code(more than 7lines i.e. readable). Where are the unit tests and/or end-to-end tests for this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That way we know it works all the time, and we also know when it gets accidentally broken by some other feature.

Copy link
Contributor Author

@BtcContributor BtcContributor Mar 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am just reusing most of the code that was already written for Settings.
Unit tests were not available and I would prefer that someone more experienced than me could take that, if needed.

Res.get("funds.withdrawal.txFee"), Res.get("funds.withdrawal.useCustomFeeValue"));
transactionFeeInputTextField = customFeeTuple.second;
useCustomFee = customFeeTuple.third;

useCustomFeeCheckboxListener = (observable, oldValue, newValue) -> {
transactionFeeInputTextField.setEditable(newValue);
if (!newValue) {
try {
transactionFeeInputTextField.setText(String.valueOf(feeService.getTxFeePerVbyte().value));
} catch (Exception e) {
e.printStackTrace();
}
}
};

transactionFeeFocusedListener = (o, oldValue, newValue) -> {
if (oldValue && !newValue) {
String estimatedFee = String.valueOf(feeService.getTxFeePerVbyte().value);
try {
int withdrawalTxFeePerVbyte = Integer.parseInt(transactionFeeInputTextField.getText());
final long minFeePerVbyte = feeService.getMinFeePerVByte();
if (withdrawalTxFeePerVbyte < minFeePerVbyte) {
new Popup().warning(Res.get("funds.withdrawal.txFeeMin", minFeePerVbyte)).show();
transactionFeeInputTextField.setText(estimatedFee);
} else if (withdrawalTxFeePerVbyte > 5000) {
new Popup().warning(Res.get("funds.withdrawal.txFeeTooLarge")).show();
transactionFeeInputTextField.setText(estimatedFee);
} else {
preferences.setWithdrawalTxFeeInVbytes(withdrawalTxFeePerVbyte);
}
} catch (NumberFormatException t) {
log.error(t.toString());
t.printStackTrace();
new Popup().warning(Res.get("validation.integerOnly")).show();
transactionFeeInputTextField.setText(estimatedFee);
} catch (Throwable t) {
log.error(t.toString());
t.printStackTrace();
new Popup().warning(Res.get("validation.inputError", t.getMessage())).show();
transactionFeeInputTextField.setText(estimatedFee);
}
}
};
transactionFeeChangeListener = (observable, oldValue, newValue) -> transactionFeeInputTextField.setText(String.valueOf(feeService.getTxFeePerVbyte().value));

final Button withdrawButton = addButton(gridPane, ++rowIndex, Res.get("funds.withdrawal.withdrawButton"), 15);

withdrawButton.setOnAction(event -> onWithdraw());
Expand Down Expand Up @@ -304,6 +357,13 @@ protected void activate() {
if (inputsToggleGroup.getSelectedToggle() == null)
inputsToggleGroup.selectToggle(useAllInputsRadioButton);

useCustomFee.setSelected(false);
transactionFeeInputTextField.setEditable(false);
transactionFeeInputTextField.setText(String.valueOf(feeService.getTxFeePerVbyte().value));
feeService.feeUpdateCounterProperty().addListener(transactionFeeChangeListener);
useCustomFee.selectedProperty().addListener(useCustomFeeCheckboxListener);
transactionFeeInputTextField.focusedProperty().addListener(transactionFeeFocusedListener);

updateInputSelection();
GUIUtil.requestFocus(withdrawToTextField);
}
Expand All @@ -317,6 +377,10 @@ protected void deactivate() {
amountTextField.focusedProperty().removeListener(amountFocusListener);
feeToggleGroup.selectedToggleProperty().removeListener(feeToggleGroupListener);
inputsToggleGroup.selectedToggleProperty().removeListener(inputsToggleGroupListener);
transactionFeeInputTextField.focusedProperty().removeListener(transactionFeeFocusedListener);
if (transactionFeeChangeListener != null)
feeService.feeUpdateCounterProperty().removeListener(transactionFeeChangeListener);
useCustomFee.selectedProperty().removeListener(useCustomFeeCheckboxListener);
}


Expand Down Expand Up @@ -361,15 +425,14 @@ private void onWithdraw() {
log.info("Fee for tx with size {}: {} " + Res.getBaseCurrencyCode() + "", txVsize, fee.toPlainString());

if (receiverAmount.isPositive()) {
double feePerVbyte = CoinUtil.getFeePerVbyte(fee, txVsize);
double vkb = txVsize / 1000d;

String messageText = Res.get("shared.sendFundsDetailsWithFee",
formatter.formatCoinWithCode(sendersAmount),
withdrawFromTextField.getText(),
withdrawToTextField.getText(),
formatter.formatCoinWithCode(fee),
feePerVbyte,
Double.parseDouble(transactionFeeInputTextField.getText()),
vkb,
formatter.formatCoinWithCode(receiverAmount));
if (dust.isPositive()) {
Expand Down Expand Up @@ -550,6 +613,9 @@ private void reset() {
withdrawMemoTextField.setText("");
withdrawMemoTextField.setPromptText(Res.get("funds.withdrawal.memo"));

transactionFeeInputTextField.setText("");
transactionFeeInputTextField.setPromptText(Res.get("funds.withdrawal.useCustomFeeValueInfo"));

selectedItems.clear();
tableView.getSelectionModel().clearSelection();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,7 @@ public class PreferencesView extends ActivatableViewAndModel<GridPane, Preferenc
private PasswordTextField rpcPwTextField;
private TitledGroupBg daoOptionsTitledGroupBg;

private ChangeListener<Boolean> transactionFeeFocusedListener;
private ChangeListener<Boolean> autoConfServiceAddressFocusOutListener, autoConfRequiredConfirmationsFocusOutListener;
private ChangeListener<Boolean> transactionFeeFocusedListener, autoConfServiceAddressFocusOutListener, autoConfRequiredConfirmationsFocusOutListener;
private final Preferences preferences;
private final FeeService feeService;
//private final ReferralIdService referralIdService;
Expand Down Expand Up @@ -808,7 +807,6 @@ private void activateGeneralOptions() {
transactionFeeInputTextField.setText(String.valueOf(feeService.getTxFeePerVbyte().value));
feeService.feeUpdateCounterProperty().addListener(transactionFeeChangeListener);
}

transactionFeeInputTextField.setText(String.valueOf(getTxFeeForWithdrawalPerVbyte()));
ignoreTradersListInputTextField.setText(String.join(", ", preferences.getIgnoreTradersList()));
/* referralIdService.getOptionalReferralId().ifPresent(referralId -> referralIdInputTextField.setText(referralId));
Expand Down