Skip to content

Commit

Permalink
Move 'numConnectionsForBtc' option handling to Config
Browse files Browse the repository at this point in the history
Note that this change makes the user-facing change of renaming
the 'numConnectionForBtc' (singular 'Connection') to
'numConnectionsForBtc' (plural 'Connections'). It is presumed that not
many users are relying on this option for day-to-day operations, and the
singular version was pretty clearly a typo / oversight.
  • Loading branch information
cbeams committed Jan 10, 2020
1 parent 3434247 commit 799eb12
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 24 deletions.
14 changes: 14 additions & 0 deletions common/src/main/java/bisq/common/config/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,14 @@ public class Config {
public static final String SOCKS5_DISCOVER_MODE = "socks5DiscoverMode";
public static final String USE_ALL_PROVIDED_NODES = "useAllProvidedNodes";
public static final String USER_AGENT = "userAgent";
public static final String NUM_CONNECTIONS_FOR_BTC = "numConnectionsForBtc";

private static final Logger log = LoggerFactory.getLogger(Config.class);

public static final int DEFAULT_INT = Integer.MIN_VALUE;
static final String DEFAULT_CONFIG_FILE_NAME = "bisq.properties";
public static final String DEFAULT_REGTEST_HOST = "localhost";
public static final int DEFAULT_NUM_CONNECTIONS_FOR_BTC = 9; // down from BitcoinJ default of 12

public static File CURRENT_APP_DATA_DIR;

Expand Down Expand Up @@ -122,6 +124,7 @@ public class Config {
private final String socks5DiscoverMode;
private final boolean useAllProvidedNodes;
private final String userAgent;
private final int numConnectionsForBtc;

// properties derived from cli options, but not exposed as cli options themselves
private boolean localBitcoinNodeIsRunning = false; // FIXME: eliminate mutable state
Expand Down Expand Up @@ -430,6 +433,12 @@ public Config(String defaultAppName, String[] args) throws HelpRequested {
.withRequiredArg()
.defaultsTo("Bisq");

ArgumentAcceptingOptionSpec<Integer> numConnectionsForBtcOpt =
parser.accepts(NUM_CONNECTIONS_FOR_BTC, "Number of connections to the Bitcoin network")
.withRequiredArg()
.ofType(int.class)
.defaultsTo(DEFAULT_NUM_CONNECTIONS_FOR_BTC);

try {
OptionSet cliOpts = parser.parse(args);

Expand Down Expand Up @@ -516,6 +525,7 @@ public Config(String defaultAppName, String[] args) throws HelpRequested {
this.socks5DiscoverMode = options.valueOf(socks5DiscoverModeOpt);
this.useAllProvidedNodes = options.valueOf(useAllProvidedNodesOpt);
this.userAgent = options.valueOf(userAgentOpt);
this.numConnectionsForBtc = options.valueOf(numConnectionsForBtcOpt);
} catch (OptionException ex) {
throw new ConfigException(format("problem parsing option '%s': %s",
ex.options().get(0),
Expand Down Expand Up @@ -775,4 +785,8 @@ public boolean isUseAllProvidedNodes() {
public String getUserAgent() {
return userAgent;
}

public int getNumConnectionsForBtc() {
return numConnectionsForBtc;
}
}
6 changes: 1 addition & 5 deletions core/src/main/java/bisq/core/app/BisqEnvironment.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package bisq.core.app;

import bisq.core.btc.BtcOptionKeys;
import bisq.core.dao.DaoOptionKeys;

import bisq.common.BisqException;
Expand Down Expand Up @@ -59,7 +58,7 @@ public class BisqEnvironment extends StandardEnvironment {

protected final String rpcUser, rpcPassword,
rpcHost, rpcPort, rpcBlockNotificationPort, rpcBlockNotificationHost, dumpBlockchainData, fullDaoNode,
numConnectionForBtc, genesisTxId, genesisBlockHeight, genesisTotalSupply,
genesisTxId, genesisBlockHeight, genesisTotalSupply,
daoActivated;

public BisqEnvironment(OptionSet options) {
Expand All @@ -84,7 +83,6 @@ public BisqEnvironment(PropertySource commandLineProperties) {
daoActivated = getProperty(commandLineProperties, DaoOptionKeys.DAO_ACTIVATED, "true");

//BtcOptionKeys
numConnectionForBtc = getProperty(commandLineProperties, BtcOptionKeys.NUM_CONNECTIONS_FOR_BTC, "9");

MutablePropertySources propertySources = getPropertySources();
propertySources.addFirst(commandLineProperties);
Expand Down Expand Up @@ -114,8 +112,6 @@ private PropertySource<?> defaultProperties() {
setProperty(DaoOptionKeys.GENESIS_BLOCK_HEIGHT, genesisBlockHeight);
setProperty(DaoOptionKeys.GENESIS_TOTAL_SUPPLY, genesisTotalSupply);
setProperty(DaoOptionKeys.DAO_ACTIVATED, daoActivated);

setProperty(BtcOptionKeys.NUM_CONNECTIONS_FOR_BTC, numConnectionForBtc);
}
});
}
Expand Down
4 changes: 0 additions & 4 deletions core/src/main/java/bisq/core/app/BisqExecutable.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package bisq.core.app;

import bisq.core.btc.BtcOptionKeys;
import bisq.core.btc.setup.WalletsSetup;
import bisq.core.btc.wallet.BsqWalletService;
import bisq.core.btc.wallet.BtcWalletService;
Expand Down Expand Up @@ -286,9 +285,6 @@ protected void customizeOptionParsing(OptionParser parser) {
.ofType(boolean.class);

//BtcOptionKeys
parser.accepts(BtcOptionKeys.NUM_CONNECTIONS_FOR_BTC,
format("Number of connections to the Bitcoin network (default: %s)", "9"))
.withRequiredArg();

//RpcOptionKeys
parser.accepts(DaoOptionKeys.RPC_USER,
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/bisq/core/btc/BitcoinModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ protected void configure() {

bindConstant().annotatedWith(named(Config.BTC_NODES)).to(config.getBtcNodes());
bindConstant().annotatedWith(named(Config.USER_AGENT)).to(config.getUserAgent());
bindConstant().annotatedWith(named(BtcOptionKeys.NUM_CONNECTIONS_FOR_BTC)).to(environment.getRequiredProperty(BtcOptionKeys.NUM_CONNECTIONS_FOR_BTC));
bindConstant().annotatedWith(named(Config.NUM_CONNECTIONS_FOR_BTC)).to(config.getNumConnectionsForBtc());
bindConstant().annotatedWith(named(Config.USE_ALL_PROVIDED_NODES)).to(config.isUseAllProvidedNodes());
bindConstant().annotatedWith(named(Config.IGNORE_LOCAL_BTC_NODE)).to(config.isIgnoreLocalBtcNode());
bindConstant().annotatedWith(Names.named(Config.SOCKS5_DISCOVER_MODE)).to(config.getSocks5DiscoverMode());
Expand Down
1 change: 0 additions & 1 deletion core/src/main/java/bisq/core/btc/BtcOptionKeys.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,4 @@
package bisq.core.btc;

public class BtcOptionKeys {
public static final String NUM_CONNECTIONS_FOR_BTC = "numConnectionForBtc";
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@

package bisq.core.btc.nodes;

import bisq.core.btc.setup.WalletsSetup;
import bisq.core.user.Preferences;

import bisq.common.config.Config;
import bisq.common.util.Utilities;

import java.util.Collections;
Expand Down Expand Up @@ -83,7 +83,7 @@ public int calculateMinBroadcastConnections(List<BtcNodes.BtcNode> nodes) {
break;
case PUBLIC:
// We keep the empty nodes
result = (int) Math.floor(WalletsSetup.DEFAULT_CONNECTIONS * 0.8);
result = (int) Math.floor(Config.DEFAULT_NUM_CONNECTIONS_FOR_BTC * 0.8);
break;
case PROVIDED:
default:
Expand Down
8 changes: 4 additions & 4 deletions core/src/main/java/bisq/core/btc/setup/WalletConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public interface BisqWalletFactory extends WalletProtobufSerializer.WalletFactor
private final BisqWalletFactory walletFactory;
private final Config config;
private final String userAgent;
private int numConnectionForBtc;
private int numConnectionsForBtc;

private volatile Wallet vBtcWallet;
@Nullable
Expand Down Expand Up @@ -153,13 +153,13 @@ public WalletConfig(NetworkParameters params,
File directory,
Config config,
String userAgent,
int numConnectionForBtc,
int numConnectionsForBtc,
@SuppressWarnings("SameParameterValue") String btcWalletFileName,
@SuppressWarnings("SameParameterValue") String bsqWalletFileName,
@SuppressWarnings("SameParameterValue") String spvChainFileName) {
this.config = config;
this.userAgent = userAgent;
this.numConnectionForBtc = numConnectionForBtc;
this.numConnectionsForBtc = numConnectionsForBtc;
this.context = new Context(params);
this.params = checkNotNull(context.getParams());
this.directory = checkNotNull(directory);
Expand Down Expand Up @@ -438,7 +438,7 @@ protected void startUp() throws Exception {
// before we're actually connected the broadcast waits for an appropriate number of connections.
if (peerAddresses != null) {
for (PeerAddress addr : peerAddresses) vPeerGroup.addAddress(addr);
int maxConnections = Math.min(numConnectionForBtc, peerAddresses.length);
int maxConnections = Math.min(numConnectionsForBtc, peerAddresses.length);
log.info("We try to connect to {} btc nodes", maxConnections);
vPeerGroup.setMaxConnections(maxConnections);
vPeerGroup.setAddPeersFromAddressMessage(false);
Expand Down
11 changes: 4 additions & 7 deletions core/src/main/java/bisq/core/btc/setup/WalletsSetup.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package bisq.core.btc.setup;

import bisq.core.btc.BtcOptionKeys;
import bisq.core.btc.exceptions.RejectedTxException;
import bisq.core.btc.model.AddressEntry;
import bisq.core.btc.model.AddressEntryList;
Expand Down Expand Up @@ -105,8 +104,6 @@
// merge WalletsSetup with WalletConfig to one class.
@Slf4j
public class WalletsSetup {
// We reduce defaultConnections from 12 (PeerGroup.DEFAULT_CONNECTIONS) to 9 nodes
public static final int DEFAULT_CONNECTIONS = 9;

private static final long STARTUP_TIMEOUT = 180;
private static final String BSQ_WALLET_FILE_NAME = "bisq_BSQ.wallet";
Expand All @@ -119,7 +116,7 @@ public class WalletsSetup {
private final Config config;
private final BtcNodes btcNodes;
private final String btcWalletFileName;
private final int numConnectionForBtc;
private final int numConnectionsForBtc;
private final String userAgent;
private final NetworkParameters params;
private final File walletDir;
Expand Down Expand Up @@ -148,15 +145,15 @@ public WalletsSetup(RegTestHost regTestHost,
@Named(Config.USER_AGENT) String userAgent,
@Named(Config.WALLET_DIR) File appDir,
@Named(Config.USE_ALL_PROVIDED_NODES) boolean useAllProvidedNodes,
@Named(BtcOptionKeys.NUM_CONNECTIONS_FOR_BTC) String numConnectionForBtc,
@Named(Config.NUM_CONNECTIONS_FOR_BTC) int numConnectionsForBtc,
@Named(Config.SOCKS5_DISCOVER_MODE) String socks5DiscoverModeString) {
this.regTestHost = regTestHost;
this.addressEntryList = addressEntryList;
this.preferences = preferences;
this.socks5ProxyProvider = socks5ProxyProvider;
this.config = config;
this.btcNodes = btcNodes;
this.numConnectionForBtc = numConnectionForBtc != null ? Integer.parseInt(numConnectionForBtc) : DEFAULT_CONNECTIONS;
this.numConnectionsForBtc = numConnectionsForBtc;
this.useAllProvidedNodes = useAllProvidedNodes;
this.userAgent = userAgent;

Expand Down Expand Up @@ -197,7 +194,7 @@ public void initialize(@Nullable DeterministicSeed seed,
walletDir,
config,
userAgent,
numConnectionForBtc,
numConnectionsForBtc,
btcWalletFileName,
BSQ_WALLET_FILE_NAME,
SPV_CHAIN_FILE_NAME) {
Expand Down

0 comments on commit 799eb12

Please sign in to comment.