From 24cc6800e76f533423b81e38e66adb41210a9c10 Mon Sep 17 00:00:00 2001 From: Manfred Karrer Date: Thu, 28 Apr 2016 18:58:14 +0200 Subject: [PATCH] Bugfix with withdrawal, display exact bitcoin value without rounding --- .../java/io/bitsquare/btc/Restrictions.java | 2 +- .../io/bitsquare/btc/TradeWalletService.java | 2 +- .../java/io/bitsquare/btc/WalletService.java | 4 +- .../io/bitsquare/btc/RestrictionsTest.java | 12 +- .../java/io/bitsquare/app/BitsquareApp.java | 4 +- .../java/io/bitsquare/gui/main/MainView.java | 4 +- .../gui/main/funds/deposit/DepositView.java | 2 +- .../main/funds/withdrawal/WithdrawalView.java | 118 ++++++++++++------ .../steps/buyer/BuyerStep5View.java | 2 +- .../io/bitsquare/gui/util/BSFormatter.java | 4 +- 10 files changed, 99 insertions(+), 55 deletions(-) diff --git a/core/src/main/java/io/bitsquare/btc/Restrictions.java b/core/src/main/java/io/bitsquare/btc/Restrictions.java index 69fedb07e3..d3805d1066 100644 --- a/core/src/main/java/io/bitsquare/btc/Restrictions.java +++ b/core/src/main/java/io/bitsquare/btc/Restrictions.java @@ -24,7 +24,7 @@ public class Restrictions { public static final Coin MIN_TRADE_AMOUNT = Coin.parseCoin("0.0001"); // 4 cent @ 400 EUR/BTC - public static boolean isAboveFixedTxFeeAndDust(Coin amount) { + public static boolean isAboveFixedTxFeeForTradesAndDust(Coin amount) { return amount != null && amount.compareTo(FeePolicy.getFixedTxFeeForTrades().add(Transaction.MIN_NONDUST_OUTPUT)) > 0; } diff --git a/core/src/main/java/io/bitsquare/btc/TradeWalletService.java b/core/src/main/java/io/bitsquare/btc/TradeWalletService.java index e838f58ea3..1c48f983b3 100644 --- a/core/src/main/java/io/bitsquare/btc/TradeWalletService.java +++ b/core/src/main/java/io/bitsquare/btc/TradeWalletService.java @@ -145,7 +145,7 @@ public class TradeWalletService { boolean useSavingsWallet, Coin tradingFee, String feeReceiverAddresses) throws InsufficientMoneyException, AddressFormatException { Transaction tradingFeeTx = new Transaction(params); - Preconditions.checkArgument(Restrictions.isAboveFixedTxFeeAndDust(tradingFee), + Preconditions.checkArgument(Restrictions.isAboveFixedTxFeeForTradesAndDust(tradingFee), "You cannot send an amount which are smaller than the fee + dust output."); Coin outPutAmount = tradingFee.subtract(FeePolicy.getFixedTxFeeForTrades()); tradingFeeTx.addOutput(outPutAmount, new Address(params, feeReceiverAddresses)); diff --git a/core/src/main/java/io/bitsquare/btc/WalletService.java b/core/src/main/java/io/bitsquare/btc/WalletService.java index dc52402fc0..bdbbc1c406 100644 --- a/core/src/main/java/io/bitsquare/btc/WalletService.java +++ b/core/src/main/java/io/bitsquare/btc/WalletService.java @@ -717,7 +717,7 @@ public class WalletService { AddressEntryException, InsufficientMoneyException { Transaction tx = new Transaction(params); Preconditions.checkArgument(Restrictions.isAboveDust(amount), - "You cannot send an amount which are smaller than 546 satoshis."); + "The amount is too low (dust limit)."); tx.addOutput(amount, new Address(params, toAddress)); Wallet.SendRequest sendRequest = Wallet.SendRequest.forTx(tx); @@ -742,7 +742,7 @@ public class WalletService { AddressFormatException, AddressEntryException, InsufficientMoneyException { Transaction tx = new Transaction(params); Preconditions.checkArgument(Restrictions.isAboveDust(amount), - "You cannot send an amount which are smaller than 546 satoshis."); + "The amount is too low (dust limit)."); tx.addOutput(amount, new Address(params, toAddress)); Wallet.SendRequest sendRequest = Wallet.SendRequest.forTx(tx); diff --git a/core/src/test/java/io/bitsquare/btc/RestrictionsTest.java b/core/src/test/java/io/bitsquare/btc/RestrictionsTest.java index 7b42b4df24..6d77e06947 100644 --- a/core/src/test/java/io/bitsquare/btc/RestrictionsTest.java +++ b/core/src/test/java/io/bitsquare/btc/RestrictionsTest.java @@ -28,21 +28,21 @@ public class RestrictionsTest { @Test public void testIsMinSpendableAmount() { Coin amount = null; - assertFalse("tx unfunded, pending", Restrictions.isAboveFixedTxFeeAndDust(amount)); + assertFalse("tx unfunded, pending", Restrictions.isAboveFixedTxFeeForTradesAndDust(amount)); amount = Coin.ZERO; - assertFalse("tx unfunded, pending", Restrictions.isAboveFixedTxFeeAndDust(amount)); + assertFalse("tx unfunded, pending", Restrictions.isAboveFixedTxFeeForTradesAndDust(amount)); amount = FeePolicy.getFixedTxFeeForTrades(); - assertFalse("tx unfunded, pending", Restrictions.isAboveFixedTxFeeAndDust(amount)); + assertFalse("tx unfunded, pending", Restrictions.isAboveFixedTxFeeForTradesAndDust(amount)); amount = Transaction.MIN_NONDUST_OUTPUT; - assertFalse("tx unfunded, pending", Restrictions.isAboveFixedTxFeeAndDust(amount)); + assertFalse("tx unfunded, pending", Restrictions.isAboveFixedTxFeeForTradesAndDust(amount)); amount = FeePolicy.getFixedTxFeeForTrades().add(Transaction.MIN_NONDUST_OUTPUT); - assertFalse("tx unfunded, pending", Restrictions.isAboveFixedTxFeeAndDust(amount)); + assertFalse("tx unfunded, pending", Restrictions.isAboveFixedTxFeeForTradesAndDust(amount)); amount = FeePolicy.getFixedTxFeeForTrades().add(Transaction.MIN_NONDUST_OUTPUT).add(Coin.valueOf(1)); - assertTrue("tx unfunded, pending", Restrictions.isAboveFixedTxFeeAndDust(amount)); + assertTrue("tx unfunded, pending", Restrictions.isAboveFixedTxFeeForTradesAndDust(amount)); } } diff --git a/gui/src/main/java/io/bitsquare/app/BitsquareApp.java b/gui/src/main/java/io/bitsquare/app/BitsquareApp.java index 9d66890774..034867677d 100644 --- a/gui/src/main/java/io/bitsquare/app/BitsquareApp.java +++ b/gui/src/main/java/io/bitsquare/app/BitsquareApp.java @@ -161,7 +161,7 @@ public class BitsquareApp extends Application { mainView.setPersistedFilesCorrupted(corruptedDatabaseFiles); });*/ - scene = new Scene(mainView.getRoot(), 1150, 740); + scene = new Scene(mainView.getRoot(), 1190, 740); scene.getStylesheets().setAll( "/io/bitsquare/gui/bitsquare.css", "/io/bitsquare/gui/images.css"); @@ -193,7 +193,7 @@ public class BitsquareApp extends Application { // configure the primary stage primaryStage.setTitle(env.getRequiredProperty(APP_NAME_KEY)); primaryStage.setScene(scene); - primaryStage.setMinWidth(1130); + primaryStage.setMinWidth(1170); primaryStage.setMinHeight(620); // on windows the title icon is also used as task bar icon in a larger size diff --git a/gui/src/main/java/io/bitsquare/gui/main/MainView.java b/gui/src/main/java/io/bitsquare/gui/main/MainView.java index 0ce31b3a72..b7b2b0c043 100644 --- a/gui/src/main/java/io/bitsquare/gui/main/MainView.java +++ b/gui/src/main/java/io/bitsquare/gui/main/MainView.java @@ -152,7 +152,7 @@ public class MainView extends InitializableView { return type != null ? "Market price (" + type.name + ")" : ""; }, model.marketPriceCurrency, model.typeProperty)); - HBox.setMargin(marketPriceBox.third, new Insets(0, 20, 0, 0)); + HBox.setMargin(marketPriceBox.third, new Insets(0, 0, 0, 0)); Tuple2 availableBalanceBox = getBalanceBox("Available balance"); @@ -243,7 +243,7 @@ public class MainView extends InitializableView { private Tuple2 getBalanceBox(String text) { TextField textField = new TextField(); textField.setEditable(false); - textField.setPrefWidth(120); + textField.setPrefWidth(140); textField.setMouseTransparent(true); textField.setFocusTraversable(false); textField.setStyle("-fx-alignment: center; -fx-background-color: white;"); diff --git a/gui/src/main/java/io/bitsquare/gui/main/funds/deposit/DepositView.java b/gui/src/main/java/io/bitsquare/gui/main/funds/deposit/DepositView.java index ec818f1df1..6481d6e441 100644 --- a/gui/src/main/java/io/bitsquare/gui/main/funds/deposit/DepositView.java +++ b/gui/src/main/java/io/bitsquare/gui/main/funds/deposit/DepositView.java @@ -292,7 +292,7 @@ public class DepositView extends ActivatableView { private Coin getAmountAsCoin() { Coin senderAmount = formatter.parseToCoin(amountTextField.getText()); - if (!Restrictions.isAboveFixedTxFeeAndDust(senderAmount)) { + if (!Restrictions.isAboveFixedTxFeeForTradesAndDust(senderAmount)) { senderAmount = Coin.ZERO; /* new Popup() .warning("The amount is lower than the transaction fee and the min. possible tx value (dust).") diff --git a/gui/src/main/java/io/bitsquare/gui/main/funds/withdrawal/WithdrawalView.java b/gui/src/main/java/io/bitsquare/gui/main/funds/withdrawal/WithdrawalView.java index f83bdf928f..164429cba8 100644 --- a/gui/src/main/java/io/bitsquare/gui/main/funds/withdrawal/WithdrawalView.java +++ b/gui/src/main/java/io/bitsquare/gui/main/funds/withdrawal/WithdrawalView.java @@ -22,7 +22,6 @@ import de.jensd.fx.fontawesome.AwesomeIcon; import io.bitsquare.app.BitsquareApp; import io.bitsquare.btc.AddressEntry; import io.bitsquare.btc.AddressEntryException; -import io.bitsquare.btc.Restrictions; import io.bitsquare.btc.WalletService; import io.bitsquare.btc.listeners.BalanceListener; import io.bitsquare.common.UserThread; @@ -40,8 +39,10 @@ import io.bitsquare.trade.TradeManager; import io.bitsquare.trade.closed.ClosedTradableManager; import io.bitsquare.trade.failed.FailedTradesManager; import io.bitsquare.user.Preferences; -import javafx.beans.binding.Bindings; +import javafx.beans.property.ObjectProperty; import javafx.beans.property.ReadOnlyObjectWrapper; +import javafx.beans.property.SimpleObjectProperty; +import javafx.beans.value.ChangeListener; import javafx.collections.FXCollections; import javafx.collections.ObservableList; import javafx.collections.transformation.SortedList; @@ -50,10 +51,7 @@ import javafx.scene.control.*; import javafx.scene.layout.VBox; import javafx.util.Callback; import org.apache.commons.lang3.StringUtils; -import org.bitcoinj.core.AddressFormatException; -import org.bitcoinj.core.Coin; -import org.bitcoinj.core.InsufficientMoneyException; -import org.bitcoinj.core.Transaction; +import org.bitcoinj.core.*; import org.jetbrains.annotations.NotNull; import org.spongycastle.crypto.params.KeyParameter; @@ -87,7 +85,10 @@ public class WithdrawalView extends ActivatableView { private Set selectedItems = new HashSet<>(); private BalanceListener balanceListener; private Set fromAddresses; - private Coin amountOfSelectedItems; + private Coin amountOfSelectedItems = Coin.ZERO; + private ObjectProperty senderAmountAsCoinProperty = new SimpleObjectProperty<>(Coin.ZERO); + private ChangeListener amountListener; + private ChangeListener amountFocusListener; /////////////////////////////////////////////////////////////////////////////////////////// @@ -131,6 +132,23 @@ public class WithdrawalView extends ActivatableView { updateList(); } }; + amountListener = (observable, oldValue, newValue) -> { + if (amountTextField.focusedProperty().get()) { + try { + senderAmountAsCoinProperty.set(formatter.parseToCoin(amountTextField.getText())); + } catch (Throwable t) { + log.error("Error at amountTextField input. " + t.toString()); + } + } + }; + amountFocusListener = (observable, oldValue, newValue) -> { + if (oldValue && !newValue) { + if (senderAmountAsCoinProperty.get().isPositive()) + amountTextField.setText(formatter.formatCoin(senderAmountAsCoinProperty.get())); + else + amountTextField.setText(""); + } + }; } @Override @@ -141,17 +159,18 @@ public class WithdrawalView extends ActivatableView { reset(); + amountTextField.textProperty().addListener(amountListener); + amountTextField.focusedProperty().addListener(amountFocusListener); walletService.addBalanceListener(balanceListener); - withdrawButton.disableProperty().bind(Bindings.createBooleanBinding(() -> !areInputsValid(), - amountTextField.textProperty(), withdrawToTextField.textProperty())); } @Override protected void deactivate() { sortedList.comparatorProperty().unbind(); observableList.forEach(WithdrawalListItem::cleanup); - withdrawButton.disableProperty().unbind(); walletService.removeBalanceListener(balanceListener); + amountTextField.textProperty().removeListener(amountListener); + amountTextField.focusedProperty().removeListener(amountFocusListener); } @@ -161,8 +180,7 @@ public class WithdrawalView extends ActivatableView { @FXML public void onWithdraw() { - Coin senderAmount = formatter.parseToCoin(amountTextField.getText()); - if (Restrictions.isAboveFixedTxFeeAndDust(senderAmount)) { + if (areInputsValid()) { FutureCallback callback = new FutureCallback() { @Override public void onSuccess(@javax.annotation.Nullable Transaction transaction) { @@ -192,31 +210,33 @@ public class WithdrawalView extends ActivatableView { // TODO Get a proper fee calculation from BitcoinJ directly Coin requiredFee = walletService.getRequiredFeeForMultipleAddresses(fromAddresses, withdrawToTextField.getText(), amountOfSelectedItems); - Coin receiverAmount = senderAmount.subtract(requiredFee); - if (BitsquareApp.DEV_MODE) { - doWithdraw(receiverAmount, callback); - } else { - new Popup().headLine("Confirm withdrawal request") - .confirmation("Sending: " + formatter.formatCoinWithCode(senderAmount) + "\n" + - "From address: " + withdrawFromTextField.getText() + "\n" + - "To receiving address: " + withdrawToTextField.getText() + ".\n" + - "Required transaction fee is: " + formatter.formatCoinWithCode(requiredFee) + "\n\n" + - "The recipient will receive: " + formatter.formatCoinWithCode(receiverAmount) + "\n\n" + - "Are you sure you want to withdraw that amount?") - .actionButtonText("Yes") - .onAction(() -> doWithdraw(receiverAmount, callback)) - .closeButtonText("Cancel") - .show(); + Coin receiverAmount = senderAmountAsCoinProperty.get().subtract(requiredFee); + if (receiverAmount.isPositive()) { + if (BitsquareApp.DEV_MODE) { + doWithdraw(receiverAmount, callback); + } else { + new Popup().headLine("Confirm withdrawal request") + .confirmation("Sending: " + formatter.formatCoinWithCode(senderAmountAsCoinProperty.get()) + "\n" + + "From address: " + withdrawFromTextField.getText() + "\n" + + "To receiving address: " + withdrawToTextField.getText() + ".\n" + + "Required transaction fee is: " + formatter.formatCoinWithCode(requiredFee) + "\n\n" + + "The recipient will receive: " + formatter.formatCoinWithCode(receiverAmount) + "\n\n" + + "Are you sure you want to withdraw that amount?") + .actionButtonText("Yes") + .onAction(() -> doWithdraw(receiverAmount, callback)) + .closeButtonText("Cancel") + .show(); + } + } else { + new Popup().warning("The amount you would like to send is too low as the bitcoin transaction fee will be deducted.\n" + + "Please use a higher amount.").show(); } } catch (Throwable e) { e.printStackTrace(); log.error(e.getMessage()); - new Popup().error(e.getMessage()).show(); + new Popup().warning(e.getMessage()).show(); } - } else { - new Popup().warning("The amount to transfer is lower than the transaction fee and the min. possible tx value (dust).") - .show(); } } @@ -233,8 +253,11 @@ public class WithdrawalView extends ActivatableView { if (!selectedItems.isEmpty()) { amountOfSelectedItems = Coin.valueOf(selectedItems.stream().mapToLong(e -> e.getBalance().getValue()).sum()); if (amountOfSelectedItems.isPositive()) { + senderAmountAsCoinProperty.set(amountOfSelectedItems); amountTextField.setText(formatter.formatCoin(amountOfSelectedItems)); } else { + senderAmountAsCoinProperty.set(Coin.ZERO); + amountOfSelectedItems = Coin.ZERO; amountTextField.setText(""); withdrawFromTextField.setText(""); } @@ -302,11 +325,17 @@ public class WithdrawalView extends ActivatableView { updateList(); } catch (AddressFormatException e) { new Popup().warning("The address is not correct. Please check the address format.").show(); + } catch (Wallet.DustySendRequested e) { + new Popup().warning("The amount you would like to send is below the dust limit and would be rejected by the bitcoin network.\n" + + "Please use a higher amount.").show(); } catch (AddressEntryException e) { new Popup().error(e.getMessage()).show(); } catch (InsufficientMoneyException e) { log.warn(e.getMessage()); new Popup().warning("You don't have enough fund in your wallet.").show(); + } catch (Throwable e) { + log.warn(e.getMessage()); + new Popup().warning(e.getMessage()).show(); } } @@ -319,6 +348,8 @@ public class WithdrawalView extends ActivatableView { withdrawFromTextField.setPromptText("Select a source address from the table"); withdrawFromTextField.setTooltip(null); + amountOfSelectedItems = Coin.ZERO; + senderAmountAsCoinProperty.set(Coin.ZERO); amountTextField.setText(""); amountTextField.setPromptText("Set the amount to withdraw"); @@ -342,14 +373,27 @@ public class WithdrawalView extends ActivatableView { } private boolean areInputsValid() { - if (amountTextField.getText().length() > 0) { - Coin amount = formatter.parseToCoin(amountTextField.getText()); - return btcAddressValidator.validate(withdrawToTextField.getText()).isValid && - amount.compareTo(amountOfSelectedItems) <= 0 && - Restrictions.isAboveFixedTxFeeAndDust(amount); - } else { + if (!senderAmountAsCoinProperty.get().isPositive()) { + new Popup().warning("Please fill in a valid value for the amount to send (max. 8 decimal places).").show(); return false; } + + if (!btcAddressValidator.validate(withdrawToTextField.getText()).isValid) { + new Popup().warning("Please fill in a valid receiver bitcoin address.").show(); + return false; + } + if (!amountOfSelectedItems.isPositive()) { + new Popup().warning("You need to select a source address in the table above.").show(); + return false; + } + + if (senderAmountAsCoinProperty.get().compareTo(amountOfSelectedItems) > 0) { + new Popup().warning("Your amount exceeds the available amount for the selected address.\n" + + "Consider to select multiple addresses in the table above if you want to withdraw more.").show(); + return false; + } + + return true; } diff --git a/gui/src/main/java/io/bitsquare/gui/main/portfolio/pendingtrades/steps/buyer/BuyerStep5View.java b/gui/src/main/java/io/bitsquare/gui/main/portfolio/pendingtrades/steps/buyer/BuyerStep5View.java index 56501330e3..ac3b1f6019 100644 --- a/gui/src/main/java/io/bitsquare/gui/main/portfolio/pendingtrades/steps/buyer/BuyerStep5View.java +++ b/gui/src/main/java/io/bitsquare/gui/main/portfolio/pendingtrades/steps/buyer/BuyerStep5View.java @@ -183,7 +183,7 @@ public class BuyerStep5View extends TradeStepView { } else { if (toAddresses.isEmpty()) { validateWithdrawAddress(); - } else if (Restrictions.isAboveFixedTxFeeAndDust(senderAmount)) { + } else if (Restrictions.isAboveFixedTxFeeForTradesAndDust(senderAmount)) { if (BitsquareApp.DEV_MODE) { doWithdrawal(receiverAmount); diff --git a/gui/src/main/java/io/bitsquare/gui/util/BSFormatter.java b/gui/src/main/java/io/bitsquare/gui/util/BSFormatter.java index 465e2805b2..42d367a326 100644 --- a/gui/src/main/java/io/bitsquare/gui/util/BSFormatter.java +++ b/gui/src/main/java/io/bitsquare/gui/util/BSFormatter.java @@ -53,7 +53,7 @@ public class BSFormatter { // Input of a group separator (1,123,45) lead to an validation error. // Note: BtcFormat was intended to be used, but it lead to many problems (automatic format to mBit, // no way to remove grouping separator). It seems to be not optimal for user input formatting. - private MonetaryFormat coinFormat = MonetaryFormat.BTC.repeatOptionalDecimals(2, 2); + private MonetaryFormat coinFormat = MonetaryFormat.BTC.minDecimals(2).repeatOptionalDecimals(1, 6); // private String currencyCode = CurrencyUtil.getDefaultFiatCurrencyAsCode(); @@ -97,7 +97,7 @@ public class BSFormatter { if (useMilliBit) return MonetaryFormat.MBTC; else - return MonetaryFormat.BTC.repeatOptionalDecimals(2, 2); + return MonetaryFormat.BTC.minDecimals(2).repeatOptionalDecimals(1, 6); } /* public void setFiatCurrencyCode(String currencyCode) {