Skip to content

Commit

Permalink
Polish LocalBitcoinNode method names and visibility
Browse files Browse the repository at this point in the history
 - Rename #willUse => #shouldBeUsed
 - Rename #willIgnore => #shouldBeIgnored
 - Reduce visibility of methods where appropriate
 - Edit Javadoc typos and use @link syntax where appropriate
  • Loading branch information
cbeams committed Feb 27, 2020
1 parent b93ca2b commit c1a99cc
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 40 deletions.
7 changes: 3 additions & 4 deletions core/src/main/java/bisq/core/app/BisqSetup.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
import bisq.common.UserThread;
import bisq.common.app.DevEnv;
import bisq.common.app.Log;
import bisq.common.config.BaseCurrencyNetwork;
import bisq.common.config.Config;
import bisq.common.crypto.CryptoException;
import bisq.common.crypto.KeyRing;
Expand Down Expand Up @@ -483,7 +482,7 @@ private void maybeShowTac() {
}

private void maybeCheckLocalBitcoinNode(Runnable nextStep) {
if (localBitcoinNode.willIgnore()) {
if (localBitcoinNode.shouldBeIgnored()) {
nextStep.run();
return;
}
Expand Down Expand Up @@ -570,7 +569,7 @@ else if (displayTorNetworkSettingsHandler != null)
// We only init wallet service here if not using Tor for bitcoinj.
// When using Tor, wallet init must be deferred until Tor is ready.
// TODO encapsulate below conditional inside getUseTorForBitcoinJ
if (!preferences.getUseTorForBitcoinJ() || localBitcoinNode.willUse()) {
if (!preferences.getUseTorForBitcoinJ() || localBitcoinNode.shouldBeUsed()) {
initWallet();
}

Expand Down Expand Up @@ -873,7 +872,7 @@ private void maybeShowSecurityRecommendation() {
}

private void maybeShowLocalhostRunningInfo() {
maybeTriggerDisplayHandler("bitcoinLocalhostNode", displayLocalhostHandler, localBitcoinNode.willUse());
maybeTriggerDisplayHandler("bitcoinLocalhostNode", displayLocalhostHandler, localBitcoinNode.shouldBeUsed());
}

private void maybeShowAccountSigningStateInfo() {
Expand Down
42 changes: 20 additions & 22 deletions core/src/main/java/bisq/core/btc/nodes/LocalBitcoinNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
* Detects whether a Bitcoin node is running on localhost and whether it is well
* configured (meaning it's not pruning and has bloom filters enabled). The public
* methods automatically trigger detection and (if detected) configuration checks,
* and cache the results, and consequent queries to `LocalBitcoinNode` will always
* and cache the results, and subsequent queries to {@link LocalBitcoinNode} will always
* return the cached results.
* @see bisq.common.config.Config#ignoreLocalBtcNode
*/
Expand All @@ -66,44 +66,43 @@ public LocalBitcoinNode(Config config, @Named(LOCAL_BITCOIN_NODE_PORT) int port)
}

/**
* Returns whether Bisq will use a local Bitcoin node. Meaning a usable node was found
* and conditions under which we would ignore it are not met. If we're ignoring the
* local node, a call to this method will not trigger an unnecessary detection
* attempt.
* Returns whether Bisq should use a local Bitcoin node, meaning that a usable node
* was detected and conditions under which it should be ignored have not been met. If
* the local node should be ignored, a call to this method will not trigger an
* unnecessary detection attempt.
*/
public boolean willUse() {
return !willIgnore() && isUsable();
public boolean shouldBeUsed() {
return !shouldBeIgnored() && isUsable();
}

/**
* Returns whether Bisq will ignore a local Bitcoin node even if it is usable.
*/
public boolean willIgnore() {
public boolean shouldBeIgnored() {
BaseCurrencyNetwork baseCurrencyNetwork = config.baseCurrencyNetwork;

// For dao testnet (server side regtest) we disable the use of local bitcoin node to
// avoid confusion if local btc node is not synced with our dao testnet master node.
// Note: above comment was previously in WalletConfig::createPeerGroup.
// For dao testnet (server side regtest) we disable the use of local bitcoin node
// to avoid confusion if local btc node is not synced with our dao testnet master
// node. Note: above comment was previously in WalletConfig::createPeerGroup.
return config.ignoreLocalBtcNode ||
baseCurrencyNetwork.isDaoRegTest() ||
baseCurrencyNetwork.isDaoTestNet();
}

/**
* Returns whether or not a local Bitcion node was detected and was well configured
* Returns whether or not a local Bitcoin node was detected and was well-configured
* at the time the checks were performed. All checks are triggered in case they have
* not been performed.
*/
public boolean isUsable() {
private boolean isUsable() {
// If a node is found to be well configured, it implies that it was also detected,
// so this is query is enough to show if the relevant checks were performed and if
// their results are positive.
return isWellConfigured();
}

/**
* Returns whether the local node was detected, but misconfigured. Combination of
* methods isDetected and isWellConfigured.
* Returns whether a local node was detected but misconfigured.
*/
public boolean isDetectedButMisconfigured() {
return isDetected() && !isWellConfigured();
Expand All @@ -116,7 +115,7 @@ public boolean isDetectedButMisconfigured() {
* value. See {@code MainViewModel#setupBtcNumPeersWatcher} to understand how
* disconnection and reconnection of the local Bitcoin node is actually handled.
*/
public boolean isDetected() {
private boolean isDetected() {
if (detected == null) {
performChecks();
}
Expand All @@ -128,7 +127,7 @@ public boolean isDetected() {
* they were performed. All checks are triggered in case they have not been performed.
* We check if the local node is not pruning and has bloom filters enabled.
*/
public boolean isWellConfigured() {
private boolean isWellConfigured() {
if (wellConfigured == null) {
performChecks();
}
Expand All @@ -144,7 +143,7 @@ private void performChecks() {

/**
* Initiates detection and configuration checks. The results are cached so that the
* public methods isUsable, isDetected, etc. don't trigger a recheck.
* {@link #isUsable()}, {@link #isDetected()} et al don't trigger a recheck.
*/
private void checkUsable() {
var optionalVersionMessage = attemptHandshakeForVersionMessage();
Expand Down Expand Up @@ -183,12 +182,11 @@ private static boolean checkWellConfigured(VersionMessage versionMessage) {

/**
* Method backported from upstream bitcoinj: at the time of writing, our version is
* not BIP111-aware.
* Source routines and data can be found in Bitcoinj under:
* not BIP111-aware. Source routines and data can be found in bitcoinj under:
* core/src/main/java/org/bitcoinj/core/VersionMessage.java
* and
* core/src/main/java/org/bitcoinj/core/NetworkParameters.java
* and core/src/main/java/org/bitcoinj/core/NetworkParameters.java
*/
@SuppressWarnings("UnnecessaryLocalVariable")
private static boolean isBloomFilteringSupportedAndEnabled(VersionMessage versionMessage) {
// A service bit that denotes whether the peer supports BIP37 bloom filters or
// not. The service bit is defined in BIP111.
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/bisq/core/btc/setup/WalletConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ private PeerGroup createPeerGroup() {
peerGroup.setConnectTimeoutMillis(TOR_VERSION_EXCHANGE_TIMEOUT);
}

if (!localBitcoinNode.willUse())
if (!localBitcoinNode.shouldBeUsed())
peerGroup.setUseLocalhostPeerWhenPossible(false);

return peerGroup;
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/bisq/core/btc/setup/WalletsSetup.java
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ protected void onSetupCompleted() {
return;
}
}
} else if (localBitcoinNode.willUse()) {
} else if (localBitcoinNode.shouldBeUsed()) {
walletConfig.setMinBroadcastConnections(1);
walletConfig.setPeerNodesForLocalHost();
} else {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/bisq/core/user/Preferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ public boolean getUseTorForBitcoinJ() {
// nodes and we don't provide tor nodes.

if ((!Config.baseCurrencyNetwork().isMainnet()
|| localBitcoinNode.willUse())
|| localBitcoinNode.shouldBeUsed())
&& !config.useTorForBtcOptionSetExplicitly)
return false;
else
Expand Down
2 changes: 1 addition & 1 deletion desktop/src/main/java/bisq/desktop/main/MainViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ private void setupBtcNumPeersWatcher() {
checkNumberOfBtcPeersTimer = UserThread.runAfter(() -> {
// check again numPeers
if (walletsSetup.numPeersProperty().get() == 0) {
if (localBitcoinNode.willUse())
if (localBitcoinNode.shouldBeUsed())
getWalletServiceErrorMsg().set(
Res.get("mainView.networkWarning.localhostBitcoinLost",
Res.getBaseCurrencyName().toLowerCase()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ public void initialize() {
bitcoinPeerSubVersionColumn.setGraphic(new AutoTooltipLabel(Res.get("settings.net.subVersionColumn")));
bitcoinPeerHeightColumn.setGraphic(new AutoTooltipLabel(Res.get("settings.net.heightColumn")));
localhostBtcNodeInfoLabel.setText(Res.get("settings.net.localhostBtcNodeInfo"));
if (!localBitcoinNode.willUse()) {
if (localBitcoinNode.shouldBeIgnored()) {
localhostBtcNodeInfoLabel.setVisible(false);
}
useProvidedNodesRadio.setText(Res.get("settings.net.useProvidedNodesRadio"));
Expand Down Expand Up @@ -380,14 +380,14 @@ private void showShutDownPopup() {
}

private void onBitcoinPeersToggleSelected(boolean calledFromUser) {
boolean bitcoinLocalhostNodeBeingUsed = localBitcoinNode.willUse();
useTorForBtcJCheckBox.setDisable(bitcoinLocalhostNodeBeingUsed);
bitcoinNodesLabel.setDisable(bitcoinLocalhostNodeBeingUsed);
btcNodesLabel.setDisable(bitcoinLocalhostNodeBeingUsed);
btcNodesInputTextField.setDisable(bitcoinLocalhostNodeBeingUsed);
useProvidedNodesRadio.setDisable(bitcoinLocalhostNodeBeingUsed || !btcNodes.useProvidedBtcNodes());
useCustomNodesRadio.setDisable(bitcoinLocalhostNodeBeingUsed);
usePublicNodesRadio.setDisable(bitcoinLocalhostNodeBeingUsed || isPreventPublicBtcNetwork());
boolean localBitcoinNodeShouldBeUsed = localBitcoinNode.shouldBeUsed();
useTorForBtcJCheckBox.setDisable(localBitcoinNodeShouldBeUsed);
bitcoinNodesLabel.setDisable(localBitcoinNodeShouldBeUsed);
btcNodesLabel.setDisable(localBitcoinNodeShouldBeUsed);
btcNodesInputTextField.setDisable(localBitcoinNodeShouldBeUsed);
useProvidedNodesRadio.setDisable(localBitcoinNodeShouldBeUsed || !btcNodes.useProvidedBtcNodes());
useCustomNodesRadio.setDisable(localBitcoinNodeShouldBeUsed);
usePublicNodesRadio.setDisable(localBitcoinNodeShouldBeUsed || isPreventPublicBtcNetwork());

BtcNodes.BitcoinNodesOption currentBitcoinNodesOption = BtcNodes.BitcoinNodesOption.values()[preferences.getBitcoinNodesOptionOrdinal()];

Expand Down Expand Up @@ -454,7 +454,7 @@ private void onBitcoinPeersToggleSelected(boolean calledFromUser) {

private void applyPreventPublicBtcNetwork() {
final boolean preventPublicBtcNetwork = isPreventPublicBtcNetwork();
usePublicNodesRadio.setDisable(localBitcoinNode.willUse() || preventPublicBtcNetwork);
usePublicNodesRadio.setDisable(localBitcoinNode.shouldBeUsed() || preventPublicBtcNetwork);
if (preventPublicBtcNetwork && selectedBitcoinNodesOption == BtcNodes.BitcoinNodesOption.PUBLIC) {
selectedBitcoinNodesOption = BtcNodes.BitcoinNodesOption.PROVIDED;
preferences.setBitcoinNodesOptionOrdinal(selectedBitcoinNodesOption.ordinal());
Expand Down

0 comments on commit c1a99cc

Please sign in to comment.