From 512f1a5972f828bf25322c84a7685d441f9e98b7 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Tue, 1 Sep 2020 16:00:02 -0500 Subject: [PATCH] Refactor AutoConfirmSettings handling in preferences - Do not use immutability for AutoConfirmSettings as it makes setting values cumbersome. - Add btc validator for trade limit - Make AutoConfirmSettings an optional and add find method by currency code to be better prepared when used for other coins. - Add static getDefaultForXmr to AutoConfirmSettings - Move listener creation to init method --- .../bisq/core/user/AutoConfirmSettings.java | 38 +++-- .../main/java/bisq/core/user/Preferences.java | 58 ++++---- .../util/validation/IntegerValidator.java | 5 + .../settings/preferences/PreferencesView.java | 131 +++++++++--------- .../main/java/bisq/desktop/util/GUIUtil.java | 4 +- 5 files changed, 130 insertions(+), 106 deletions(-) diff --git a/core/src/main/java/bisq/core/user/AutoConfirmSettings.java b/core/src/main/java/bisq/core/user/AutoConfirmSettings.java index e83acb7c6d..06d16f846a 100644 --- a/core/src/main/java/bisq/core/user/AutoConfirmSettings.java +++ b/core/src/main/java/bisq/core/user/AutoConfirmSettings.java @@ -21,21 +21,37 @@ import bisq.common.proto.persistable.PersistablePayload; import com.google.protobuf.Message; +import org.bitcoinj.core.Coin; + import java.util.ArrayList; import java.util.List; -public final class AutoConfirmSettings implements PersistablePayload { - public final boolean enabled; - public final int requiredConfirmations; - public final long tradeLimit; - public final List serviceAddresses; - public final String currencyCode; +import lombok.Getter; +import lombok.Setter; - public AutoConfirmSettings(boolean enabled, - int requiredConfirmations, - long tradeLimit, - List serviceAddresses, - String currencyCode) { +@Getter +@Setter +public final class AutoConfirmSettings implements PersistablePayload { + private boolean enabled; + private int requiredConfirmations; + private long tradeLimit; + private List serviceAddresses; + private String currencyCode; + + public static AutoConfirmSettings getDefaultForXmr(List serviceAddresses) { + return new AutoConfirmSettings( + false, + 5, + Coin.COIN.value, + serviceAddresses, + "XMR"); + } + + AutoConfirmSettings(boolean enabled, + int requiredConfirmations, + long tradeLimit, + List serviceAddresses, + String currencyCode) { this.enabled = enabled; this.requiredConfirmations = requiredConfirmations; this.tradeLimit = tradeLimit; diff --git a/core/src/main/java/bisq/core/user/Preferences.java b/core/src/main/java/bisq/core/user/Preferences.java index 1493e779c6..597e0c95e6 100644 --- a/core/src/main/java/bisq/core/user/Preferences.java +++ b/core/src/main/java/bisq/core/user/Preferences.java @@ -40,8 +40,6 @@ import bisq.common.proto.persistable.PersistedDataHost; import bisq.common.storage.Storage; import bisq.common.util.Utilities; -import org.bitcoinj.core.Coin; - import javax.inject.Inject; import javax.inject.Named; import javax.inject.Singleton; @@ -62,6 +60,7 @@ import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.Random; import java.util.stream.Collectors; @@ -321,6 +320,10 @@ public final class Preferences implements PersistedDataHost, BridgeAddressProvid setUsePriceNotifications(true); } + if (prefPayload.getAutoConfirmSettingsList().isEmpty()) { + prefPayload.getAutoConfirmSettingsList().add(AutoConfirmSettings.getDefaultForXmr(getDefaultXmrProofProviders())); + } + // We set the capability in CoreNetworkCapabilities if the program argument is set. // If we have set it in the preferences view we handle it here. CoreNetworkCapabilities.maybeApplyDaoFullMode(config); @@ -410,33 +413,38 @@ public final class Preferences implements PersistedDataHost, BridgeAddressProvid persist(); } - // AutoConfirmSettings is currently only used for XMR. Although it could - // potentially in the future be used for others too. In the interest of flexibility - // we store it as a list in the proto definition, but in practical terms the - // application is not coded to handle more than one entry. For now this API - // to get/set AutoConfirmSettings is the gatekeeper. If in the future we adapt - // the application to manage more than one altcoin AutoConfirmSettings then - // this API will need to incorporate lookup by coin. - public AutoConfirmSettings getAutoConfirmSettings() { - if (prefPayload.getAutoConfirmSettingsList().size() == 0) { - // default values for AutoConfirmSettings when persisted payload is empty: - prefPayload.getAutoConfirmSettingsList().add(new AutoConfirmSettings( - false, 5, Coin.COIN.value, getDefaultXmrProofProviders(), "XMR")); - } - return prefPayload.getAutoConfirmSettingsList().get(0); + public Optional findAutoConfirmSettings(String currencyCode) { + return prefPayload.getAutoConfirmSettingsList().stream() + .filter(e -> e.getCurrencyCode().equals(currencyCode)) + .findAny(); } - public void setAutoConfirmSettings(AutoConfirmSettings autoConfirmSettings) { - // see above comment regarding only one entry in this list currently - prefPayload.getAutoConfirmSettingsList().clear(); - prefPayload.getAutoConfirmSettingsList().add(autoConfirmSettings); - persist(); + public void setAutoConfServiceAddresses(String currencyCode, List serviceAddresses) { + findAutoConfirmSettings(currencyCode).ifPresent(e -> { + e.setServiceAddresses(serviceAddresses); + persist(); + }); } - public void setAutoConfServiceAddresses(List serviceAddresses) { - AutoConfirmSettings x = this.getAutoConfirmSettings(); - this.setAutoConfirmSettings(new AutoConfirmSettings( - x.enabled, x.requiredConfirmations, x.tradeLimit, serviceAddresses, x.currencyCode)); + public void setAutoConfEnabled(String currencyCode, boolean enabled) { + findAutoConfirmSettings(currencyCode).ifPresent(e -> { + e.setEnabled(enabled); + persist(); + }); + } + + public void setAutoConfRequiredConfirmations(String currencyCode, int requiredConfirmations) { + findAutoConfirmSettings(currencyCode).ifPresent(e -> { + e.setRequiredConfirmations(requiredConfirmations); + persist(); + }); + } + + public void setAutoConfTradeLimit(String currencyCode, long tradeLimit) { + findAutoConfirmSettings(currencyCode).ifPresent(e -> { + e.setTradeLimit(tradeLimit); + persist(); + }); } private void persist() { diff --git a/core/src/main/java/bisq/core/util/validation/IntegerValidator.java b/core/src/main/java/bisq/core/util/validation/IntegerValidator.java index d2392e142e..d904eb497a 100644 --- a/core/src/main/java/bisq/core/util/validation/IntegerValidator.java +++ b/core/src/main/java/bisq/core/util/validation/IntegerValidator.java @@ -32,6 +32,11 @@ public class IntegerValidator extends InputValidator { public IntegerValidator() { } + public IntegerValidator(int minValue, int maxValue) { + this.minValue = minValue; + this.maxValue = maxValue; + } + public ValidationResult validate(String input) { ValidationResult validationResult = super.validate(input); if (!validationResult.isValid) diff --git a/desktop/src/main/java/bisq/desktop/main/settings/preferences/PreferencesView.java b/desktop/src/main/java/bisq/desktop/main/settings/preferences/PreferencesView.java index 7f80fdc27f..58a39ad650 100644 --- a/desktop/src/main/java/bisq/desktop/main/settings/preferences/PreferencesView.java +++ b/desktop/src/main/java/bisq/desktop/main/settings/preferences/PreferencesView.java @@ -29,6 +29,7 @@ import bisq.desktop.main.overlays.popups.Popup; import bisq.desktop.util.GUIUtil; import bisq.desktop.util.ImageUtil; import bisq.desktop.util.Layout; +import bisq.desktop.util.validation.BtcValidator; import bisq.desktop.util.validation.RegexValidator; import bisq.core.btc.wallet.Restrictions; @@ -44,7 +45,6 @@ import bisq.core.locale.LanguageUtil; import bisq.core.locale.Res; import bisq.core.locale.TradeCurrency; import bisq.core.provider.fee.FeeService; -import bisq.core.user.AutoConfirmSettings; import bisq.core.user.BlockChainExplorer; import bisq.core.user.Preferences; import bisq.core.util.FormattingUtils; @@ -111,12 +111,12 @@ public class PreferencesView extends ActivatableViewAndModel preferredTradeCurrencyComboBox; private ToggleButton showOwnOffersInOfferBook, useAnimations, useDarkMode, sortMarketCurrenciesNumerically, - avoidStandbyMode, useCustomFee, autoConfirmXmr; + avoidStandbyMode, useCustomFee, autoConfirmXmrToggle; private int gridRow = 0; private int displayCurrenciesGridRowIndex = 0; private InputTextField transactionFeeInputTextField, ignoreTradersListInputTextField, ignoreDustThresholdInputTextField, - autoConfRequiredConfirmations, autoConfServiceAddress, autoConfTradeLimit, /*referralIdInputTextField,*/ - rpcUserTextField, blockNotifyPortTextField; + autoConfRequiredConfirmationsTf, autoConfServiceAddressTf, autoConfTradeLimitTf, /*referralIdInputTextField,*/ + rpcUserTextField, blockNotifyPortTextField; private ToggleButton isDaoFullNodeToggleButton; private PasswordTextField rpcPwTextField; private TitledGroupBg daoOptionsTitledGroupBg; @@ -656,19 +656,52 @@ public class PreferencesView extends ActivatableViewAndModel { + if (!newValue.equals(oldValue) && autoConfServiceAddressTf.getValidator().validate(newValue).isValid) { + List serviceAddresses = Arrays.asList(StringUtils.deleteWhitespace(newValue).split(",")); + // revert to default service providers when user empties the list + if (serviceAddresses.size() == 1 && serviceAddresses.get(0).isEmpty()) { + serviceAddresses = Preferences.getDefaultXmrProofProviders(); + } + preferences.setAutoConfServiceAddresses("XMR", serviceAddresses); + } + }; + + autoConfRequiredConfirmationsListener = (observable, oldValue, newValue) -> { + if (!newValue.equals(oldValue) && autoConfRequiredConfirmationsTf.getValidator().validate(newValue).isValid) { + int requiredConfirmations = Integer.parseInt(newValue); + preferences.setAutoConfRequiredConfirmations("XMR", requiredConfirmations); + } + }; + autoConfTradeLimitListener = (observable, oldValue, newValue) -> { + if (!newValue.equals(oldValue) && autoConfTradeLimitTf.getValidator().validate(newValue).isValid) { + Coin amountAsCoin = ParsingUtils.parseToCoin(newValue, formatter); + preferences.setAutoConfTradeLimit("XMR", amountAsCoin.value); + } + }; + autoConfFocusOutListener = (observable, oldValue, newValue) -> { if (oldValue && !newValue) { log.info("Service address focus out, check and re-display default option"); - if (autoConfServiceAddress.getText().length() == 0) { - autoConfServiceAddress.setText(String.join(", ", - preferences.getAutoConfirmSettings().serviceAddresses)); + if (autoConfServiceAddressTf.getText().isEmpty()) { + preferences.findAutoConfirmSettings("XMR").ifPresent(autoConfirmSettings -> { + List serviceAddresses = autoConfirmSettings.getServiceAddresses(); + autoConfServiceAddressTf.setText(String.join(", ", serviceAddresses)); + }); } } }; @@ -926,59 +959,21 @@ public class PreferencesView extends ActivatableViewAndModel { + autoConfirmXmrToggle.setSelected(autoConfirmSettings.isEnabled()); + autoConfRequiredConfirmationsTf.setText(String.valueOf(autoConfirmSettings.getRequiredConfirmations())); + autoConfTradeLimitTf.setText(formatter.formatCoin(Coin.valueOf(autoConfirmSettings.getTradeLimit()))); + autoConfServiceAddressTf.setText(String.join(", ", autoConfirmSettings.getServiceAddresses())); - autoConfirmXmr.setOnAction(e -> { - boolean enabled = autoConfirmXmr.isSelected(); - AutoConfirmSettings x = preferences.getAutoConfirmSettings(); - preferences.setAutoConfirmSettings( - new AutoConfirmSettings(enabled, x.requiredConfirmations, x.tradeLimit, x.serviceAddresses, x.currencyCode)); + autoConfRequiredConfirmationsTf.textProperty().addListener(autoConfRequiredConfirmationsListener); + autoConfTradeLimitTf.textProperty().addListener(autoConfTradeLimitListener); + autoConfServiceAddressTf.textProperty().addListener(autoConfServiceAddressListener); + autoConfServiceAddressTf.focusedProperty().addListener(autoConfFocusOutListener); + + autoConfirmXmrToggle.setOnAction(e -> { + preferences.setAutoConfEnabled(autoConfirmSettings.getCurrencyCode(), autoConfirmXmrToggle.isSelected()); + }); }); - - autoConfServiceAddress.setValidator(GUIUtil.addressRegexValidator()); - autoConfServiceAddress.setErrorMessage(Res.get("validation.invalidAddressList")); - autoConfServiceAddressListener = (observable, oldValue, newValue) -> { - if (GUIUtil.addressRegexValidator().validate(newValue).isValid && !newValue.equals(oldValue)) { - List serviceAddresses = Arrays.asList(StringUtils.deleteWhitespace(newValue).split(",")); - // revert to default service providers when user empties the list - if (serviceAddresses.size() == 1 && serviceAddresses.get(0).length() == 0) - serviceAddresses = Preferences.getDefaultXmrProofProviders(); - preferences.setAutoConfServiceAddresses(serviceAddresses); - } - }; - - IntegerValidator validator = new IntegerValidator(); - validator.setMinValue(1); validator.setMaxValue(10000); - autoConfRequiredConfirmations.setValidator(validator); - autoConfRequiredConfirmationsListener = (observable, oldValue, newValue) -> { - try { - int value = Integer.parseInt(newValue); - if (!newValue.equals(oldValue)) { - AutoConfirmSettings x = preferences.getAutoConfirmSettings(); - preferences.setAutoConfirmSettings( - new AutoConfirmSettings(x.enabled, value, x.tradeLimit, x.serviceAddresses, x.currencyCode)); - } - } catch (Throwable ignore) { - } - }; - autoConfTradeLimitListener = (observable, oldValue, newValue) -> { - try { - Coin amountAsCoin = ParsingUtils.parseToCoin(newValue, formatter); - AutoConfirmSettings x = preferences.getAutoConfirmSettings(); - preferences.setAutoConfirmSettings( - new AutoConfirmSettings(x.enabled, x.requiredConfirmations, amountAsCoin.value, x.serviceAddresses, x.currencyCode)); - } catch (Throwable ignore) { - } - }; - - autoConfRequiredConfirmations.textProperty().addListener(autoConfRequiredConfirmationsListener); - autoConfTradeLimit.textProperty().addListener(autoConfTradeLimitListener); - autoConfServiceAddress.textProperty().addListener(autoConfServiceAddressListener); - autoConfServiceAddress.focusedProperty().addListener(autoConfFocusOutListener); } private void updateDaoFields() { @@ -1051,10 +1046,10 @@ public class PreferencesView extends ActivatableViewAndModel