Add local Bitcoin node configuration detection

Refactors LocalBitcoinNode and adds detection for local Bitcoin node's
configuration, namely, whether it is pruning and whether it has bloom
filter queries enabled.

The local node's configuration (and its presence) is retrieved by
performing a Bitcoin protocol handshake, which includes the local
Bitcoin node sending us its version message (VersionMessage in
BitcoinJ), which contains the information we're interested in.

Due to some quirky BitcoinJ logic, sometimes the handshake is
interrupted, even though we have received the local node's version
message. That contributes to the handshake handling in LocalBitcoinNode
being a bit complicated.

Refactoring consists of two principle changes: the public interface is
split into methods that trigger checks and methods that retrieve the
cached results. The methods that trigger checks have names starting
with "check", and methods that retrieve the cached results have names
that start with "is".

The other major refactor is the use of Optional<Boolean> instead of
boolean for storing and returning the results, an empty Optional
signifying that the relevant check was not yet performed. Switching to
Optionals has caused other code that queries LocalBitcoinNode to throw
an exception in case the query is made before the checks are. Before,
the results were instantiated to "false" and that would be returned
in case the query was made before the checks completed. This change has
revealed one occasion (Preferences class) where this happens.
This commit is contained in:
Dominykas Mostauskis 2020-02-13 21:14:41 +02:00
parent a3f4f7cfab
commit f895da416f
No known key found for this signature in database
GPG Key ID: B5DA7DD87C5D5FB0
8 changed files with 348 additions and 44 deletions

View File

@ -193,7 +193,7 @@ public class BisqSetup {
@Setter
@Nullable
private Consumer<Runnable> displayTacHandler;
private Consumer<Runnable> displayTacHandler, displayLocalNodeMisconfigurationHandler;
@Setter
@Nullable
private Consumer<String> cryptoSetupFailedHandler, chainFileLockedExceptionHandler,
@ -348,7 +348,7 @@ public class BisqSetup {
}
private void step2() {
detectLocalBitcoinNode(this::step3);
maybeCheckLocalBitcoinNode(this::step3);
}
private void step3() {
@ -482,14 +482,31 @@ public class BisqSetup {
}
}
private void detectLocalBitcoinNode(Runnable nextStep) {
private void maybeCheckLocalBitcoinNode(Runnable nextStep) {
BaseCurrencyNetwork baseCurrencyNetwork = config.baseCurrencyNetwork;
if (config.ignoreLocalBtcNode || baseCurrencyNetwork.isDaoRegTest() || baseCurrencyNetwork.isDaoTestNet()) {
var shouldIgnoreLocalNode =
config.ignoreLocalBtcNode
|| baseCurrencyNetwork.isDaoRegTest()
|| baseCurrencyNetwork.isDaoTestNet();
if (shouldIgnoreLocalNode) {
nextStep.run();
return;
}
localBitcoinNode.detectAndRun(nextStep);
// Results of the check don't have to be passed to nextStep,
// because they're cached in LocalBitcoinNode and dependent routines query it themselves.
localBitcoinNode.checkUsable();
// Here we only want to provide the user with a choice (in a popup) in case a local node is
// detected, but badly configured.
var detectedButMisconfigured = localBitcoinNode.isDetectedButMisconfigured().get();
if (detectedButMisconfigured) {
displayLocalNodeMisconfigurationHandler.accept(nextStep);
return;
}
nextStep.run();
}
private void readMapsFromResources(Runnable nextStep) {
@ -559,7 +576,7 @@ public class BisqSetup {
// We only init wallet service here if not using Tor for bitcoinj.
// When using Tor, wallet init must be deferred until Tor is ready.
if (!preferences.getUseTorForBitcoinJ() || localBitcoinNode.isDetected()) {
if (!preferences.getUseTorForBitcoinJ()) {
initWallet();
}
@ -862,7 +879,7 @@ public class BisqSetup {
}
private void maybeShowLocalhostRunningInfo() {
maybeTriggerDisplayHandler("bitcoinLocalhostNode", displayLocalhostHandler, localBitcoinNode.isDetected());
maybeTriggerDisplayHandler("bitcoinLocalhostNode", displayLocalhostHandler, localBitcoinNode.isUsable().get());
}
private void maybeShowAccountSigningStateInfo() {

View File

@ -1,19 +1,43 @@
package bisq.core.btc.nodes;
import org.bitcoinj.core.AbstractBlockChain;
import org.bitcoinj.core.Context;
import org.bitcoinj.core.Peer;
import org.bitcoinj.core.PeerAddress;
import org.bitcoinj.core.VersionMessage;
import org.bitcoinj.core.listeners.PeerDisconnectedEventListener;
import org.bitcoinj.net.NioClient;
import org.bitcoinj.params.MainNetParams;
import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton;
import com.google.common.util.concurrent.FutureCallback;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.SettableFuture;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.Socket;
import java.net.UnknownHostException;
import java.io.IOException;
import java.util.Optional;
import java.util.concurrent.CancellationException;
import java.util.concurrent.ExecutionException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* Detects whether a Bitcoin node is running on localhost.
* 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 class is split into
* methods that perform relevant checks and cache the result (methods that start with "check"),
* and methods that query that cache (methods that start with "is"). The querying methods return
* an Optional<Boolean>, whose emptiness shows if the relevant check is pending, and whose
* contents show the result of the check.
* @see bisq.common.config.Config#ignoreLocalBtcNode
*/
@Singleton
@ -25,7 +49,8 @@ public class LocalBitcoinNode {
private static final int CONNECTION_TIMEOUT = 5000;
private final int port;
private boolean detected = false;
private Optional<Boolean> detected = Optional.empty();
private Optional<Boolean> wellConfigured = Optional.empty();
@Inject
public LocalBitcoinNode(@Named(LOCAL_BITCOIN_NODE_PORT) int port) {
@ -33,31 +58,263 @@ public class LocalBitcoinNode {
}
/**
* Detect whether a Bitcoin node is running on localhost by attempting to connect
* to the node's port and run the given callback regardless of whether the connection
* was successful. If the connection is successful, subsequent calls to
* {@link #isDetected()} will return {@code true}.
* @param callback code to run after detecting whether the node is running
* Creates an NioClient that is expected to only be used to coerce a VersionMessage out of a
* local Bitcoin node and be closed right after.
*/
public void detectAndRun(Runnable callback) {
try (Socket socket = new Socket()) {
socket.connect(new InetSocketAddress("127.0.0.1", port), CONNECTION_TIMEOUT);
log.info("Local Bitcoin node detected on port {}", port);
detected = true;
} catch (IOException ex) {
log.info("No local Bitcoin node detected on port {}.", port);
}
callback.run();
private static NioClient createClient(
Peer peer, int port, int connectionTimeout
) throws IOException {
InetSocketAddress serverAddress =
new InetSocketAddress(InetAddress.getLocalHost(), port);
// This initiates the handshake procedure, which, if successful, will complete
// the peerVersionMessageFuture, or be cancelled, in case of failure.
NioClient client = new NioClient(serverAddress, peer, connectionTimeout);
return client;
}
/**
* Returns whether or not a Bitcoin node was running on localhost at the time
* {@link #detectAndRun(Runnable)} was called. No further monitoring is performed, so
* if the node goes up or down in the meantime, this method will continue to return
* its original value. See {@code MainViewModel#setupBtcNumPeersWatcher} to understand
* how disconnection and reconnection of the local Bitcoin node is actually handled.
* Creates a Peer that is expected to only be used to coerce a VersionMessage out of a
* local Bitcoin node and be closed right after.
*/
public boolean isDetected() {
private static Peer createLocalPeer(
int port
) throws UnknownHostException {
// TODO: what's the effect of NetworkParameters on handshake?
// i.e. is it fine to just always use MainNetParams?
var networkParameters = new MainNetParams();
var context = new Context(networkParameters);
var ourVersionMessage = new VersionMessage(networkParameters, 0);
var localPeerAddress = new PeerAddress(InetAddress.getLocalHost(), port);
AbstractBlockChain blockchain = null;
var peer = new Peer(networkParameters, ourVersionMessage, localPeerAddress, blockchain);
return peer;
}
private static boolean checkWellConfigured(VersionMessage versionMessage) {
var notPruning = versionMessage.hasBlockChain();
var supportsAndAllowsBloomFilters =
isBloomFilteringSupportedAndEnabled(versionMessage);
return notPruning && supportsAndAllowsBloomFilters;
}
/**
* 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:
* core/src/main/java/org/bitcoinj/core/VersionMessage.java
* and
* core/src/main/java/org/bitcoinj/core/NetworkParameters.java
*/
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.
int NODE_BLOOM = 1 << 2;
int BLOOM_FILTERS_BIP37_PROTOCOL_VERSION = 70000;
var whenBloomFiltersWereIntroduced = BLOOM_FILTERS_BIP37_PROTOCOL_VERSION;
int BLOOM_FILTERS_BIP111_PROTOCOL_VERSION = 70011;
var whenBloomFiltersWereDisabledByDefault = BLOOM_FILTERS_BIP111_PROTOCOL_VERSION;
int clientVersion = versionMessage.clientVersion;
long localServices = versionMessage.localServices;
if (clientVersion >= whenBloomFiltersWereIntroduced
&& clientVersion < whenBloomFiltersWereDisabledByDefault)
return true;
return (localServices & NODE_BLOOM) == NODE_BLOOM;
}
/**
* Initiates detection and configuration checks. The results are cached so that the public
* methods isUsable, isDetected, isWellConfigured don't trigger a recheck.
*/
public boolean checkUsable() {
var optionalVersionMessage = attemptHandshakeForVersionMessage();
handleHandshakeAttempt(optionalVersionMessage);
// We know that the Optional/s will be populated by the end of the checks.
return isUsable().get();
}
private void handleHandshakeAttempt(Optional<VersionMessage> optionalVersionMessage) {
if (optionalVersionMessage.isEmpty()) {
detected = Optional.of(false);
wellConfigured = Optional.of(false);
log.info("No local Bitcoin node detected on port {},"
+ " or the connection was prematurely closed"
+ " (before a version messages could be coerced)",
port);
} else {
detected = Optional.of(true);
log.info("Local Bitcoin node detected on port {}", port);
var versionMessage = optionalVersionMessage.get();
var configurationCheckResult = checkWellConfigured(versionMessage);
if (configurationCheckResult) {
wellConfigured = Optional.of(true);
log.info("Local Bitcoin node found to be well configured"
+ " (not pruning and allows bloom filters)");
} else {
wellConfigured = Optional.of(false);
log.info("Local Bitcoin node badly configured"
+ " (it is pruning and/or bloom filters are disabled)");
}
}
}
/** Performs a blocking Bitcoin protocol handshake, which includes exchanging version messages and acks.
* Its purpose is to check if a local Bitcoin node is running, and, if it is, check its advertised
* configuration. The returned Optional is empty, if a local peer wasn't found, or if handshake failed
* for some reason. This method could be noticably simplified, by turning connection failure callback
* into a future and using a first-future-to-complete type of construct, but I couldn't find a
* ready-made implementation.
*/
private Optional<VersionMessage> attemptHandshakeForVersionMessage() {
Peer peer;
try {
peer = createLocalPeer(port);
} catch (UnknownHostException ex) {
log.error("Local bitcoin node handshake attempt unexpectedly threw: {}", ex);
return Optional.empty();
}
NioClient client;
try {
log.info("Initiating attempt to connect to and handshake with a local "
+ "Bitcoin node (which may or may not be running) on port {}.",
port);
client = createClient(peer, port, CONNECTION_TIMEOUT);
} catch (IOException ex) {
log.error("Local bitcoin node handshake attempt unexpectedly threw: {}", ex);
return Optional.empty();
}
ListenableFuture<VersionMessage> peerVersionMessageFuture = getVersionMessage(peer);
Optional<VersionMessage> optionalPeerVersionMessage;
// block for VersionMessage or cancellation (in case of connection failure)
try {
var peerVersionMessage = peerVersionMessageFuture.get();
optionalPeerVersionMessage = Optional.of(peerVersionMessage);
} catch (ExecutionException | InterruptedException | CancellationException ex) {
optionalPeerVersionMessage = Optional.empty();
}
peer.close();
client.closeConnection();
return optionalPeerVersionMessage;
}
private ListenableFuture<VersionMessage> getVersionMessage(Peer peer) {
SettableFuture<VersionMessage> peerVersionMessageFuture = SettableFuture.create();
var versionHandshakeDone = peer.getVersionHandshakeFuture();
FutureCallback<Peer> fetchPeerVersionMessage = new FutureCallback<Peer>() {
public void onSuccess(Peer peer) {
peerVersionMessageFuture.set(peer.getPeerVersionMessage());
}
public void onFailure(Throwable thr) {
}
};
Futures.addCallback(
versionHandshakeDone,
fetchPeerVersionMessage
);
PeerDisconnectedEventListener cancelIfConnectionFails =
new PeerDisconnectedEventListener() {
public void onPeerDisconnected(Peer peer, int peerCount) {
var peerVersionMessageAlreadyReceived =
peerVersionMessageFuture.isDone();
if (peerVersionMessageAlreadyReceived) {
// This method is called whether or not the handshake was successful.
// In case it was successful, we don't want to do anything here.
return;
}
// In some cases Peer will self-disconnect after receiving
// node's VersionMessage, but before completing the handshake.
// In such a case, we want to retrieve the VersionMessage.
var peerVersionMessage = peer.getPeerVersionMessage();
if (peerVersionMessage != null) {
log.info("Handshake attempt was interrupted;"
+ " however, the local node's version message was coerced.");
peerVersionMessageFuture.set(peerVersionMessage);
} else {
log.info("Handshake attempt did not result in a version message exchange.");
var mayInterruptWhileRunning = true;
peerVersionMessageFuture.cancel(mayInterruptWhileRunning);
}
}
};
// Cancel peerVersionMessageFuture if connection failed
peer.addDisconnectedEventListener(cancelIfConnectionFails);
return peerVersionMessageFuture;
}
/**
* Returns an optional that, in case it is not empty, shows whether or not the
* local node was fit for usage at the time the checks were performed called,
* meaning it's been detected and its configuration satisfied our checks; or, in
* the case that it is empty, it signifies that the checks have not yet completed.
*/
public Optional<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 an Optional<Boolean> that, when not empty, tells you whether the local node
* was detected, but misconfigured.
*/
public Optional<Boolean> isDetectedButMisconfigured() {
return isDetected().flatMap(goodDetect ->
isWellConfigured().map(goodConfig ->
goodDetect && !goodConfig
));
}
/**
* Returns an optional, which is empty in case detection has not yet completed, or
* which contains a Boolean, in case detection has been performed, which signifies
* whether or not a Bitcoin node was running on localhost at the time the checks were
* performed. No further monitoring is performed, so if the node goes up or down in the
* meantime, this method will continue to return its original value. See
* {@code MainViewModel#setupBtcNumPeersWatcher} to understand how disconnection and
* reconnection of the local Bitcoin node is actually handled.
*/
public Optional<Boolean> isDetected() {
return detected;
}
/**
* Returns an optional whose emptiness signifies whether or not configuration checks
* have been performed, and its Boolean contents whether the local node's configuration
* satisfied our checks at the time they were performed. We check if the local node is
* not pruning and has bloom filters enabled.
*/
public Optional<Boolean> isWellConfigured() {
return wellConfigured;
}
}

View File

@ -239,11 +239,12 @@ public class WalletConfig extends AbstractIdleService {
peerGroup.setConnectTimeoutMillis(TOR_VERSION_EXCHANGE_TIMEOUT);
}
// For dao testnet (server side regtest) we prevent to connect to a localhost node to avoid confusion
// if local btc node is not synced with our dao testnet master node.
// 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.
// It is also disabled if the local node was not found or was found to be misconfigured.
if (Config.baseCurrencyNetwork().isDaoRegTest() ||
Config.baseCurrencyNetwork().isDaoTestNet() ||
!localBitcoinNode.isDetected())
!localBitcoinNode.isUsable().get())
peerGroup.setUseLocalhostPeerWhenPossible(false);
return peerGroup;

View File

@ -278,7 +278,7 @@ public class WalletsSetup {
return;
}
}
} else if (localBitcoinNode.isDetected()) {
} else if (localBitcoinNode.isUsable().get()) {
walletConfig.setMinBroadcastConnections(1);
walletConfig.setPeerNodesForLocalHost();
} else {

View File

@ -736,11 +736,24 @@ public final class Preferences implements PersistedDataHost, BridgeAddressProvid
}
public boolean getUseTorForBitcoinJ() {
// We override the useTorForBitcoinJ and set it to false if we detected a localhost node or if we are not on mainnet,
// We override the useTorForBitcoinJ and set it to false if we found a usable localhost node or if we are not on mainnet,
// unless the useTorForBtc parameter is explicitly provided.
// On testnet there are very few Bitcoin tor nodes and we don't provide tor nodes.
// TODO bug. Non-critical, apparently.
// Sometimes this method, which queries LocalBitcoinNode for whether or not there's a
// usable local Bitcoin node, is called before LocalBitcoinNode has performed its
// checks. This was noticed when LocalBitcoinNode was refactored to return
// Optional<Boolean> istead of boolean, an empty Optional signifying that the relevant
// check has not yet been performed.
//
// To keep the method's behaviour unchanged, until a fix is implemented, we use
// Optional.orElse(false). Here 'false' normally means that the checks were performed
// and a suitable local Bitcoin node wasn't found.
var usableLocalNodePresent = localBitcoinNode.isUsable().orElse(false);
if ((!Config.baseCurrencyNetwork().isMainnet()
|| localBitcoinNode.isDetected())
|| usableLocalNodePresent)
&& !config.useTorForBtcOptionSetExplicitly)
return false;
else

View File

@ -2659,9 +2659,13 @@ popup.privateNotification.headline=Important private notification!
popup.securityRecommendation.headline=Important security recommendation
popup.securityRecommendation.msg=We would like to remind you to consider using password protection for your wallet if you have not already enabled that.\n\nIt is also highly recommended to write down the wallet seed words. Those seed words are like a master password for recovering your Bitcoin wallet.\nAt the \"Wallet Seed\" section you find more information.\n\nAdditionally you should backup the complete application data folder at the \"Backup\" section.
popup.warning.localNodeMisconfigured.explanation=Bisq detected a locally running Bitcoin Core node (at localhost); however, its configuration is incompatible with Bisq.\n\n\
For Bisq to use a local Bitcoin node, the node has to have pruning disabled and bloom filters enabled. Starting with Bitcoin Core v0.19 you have to manually enable bloom filters by setting peerbloomfilters=1 in your bitcoin.conf configuration file.
popup.warning.localNodeMisconfigured.continueWithoutLocalNode=Continue without using local node
popup.bitcoinLocalhostNode.msg=Bisq detected a locally running Bitcoin Core node (at localhost).\n\
Please make sure that this node is fully synced before you start Bisq and that it is not running in pruned mode.
popup.bitcoinLocalhostNode.additionalRequirements=\n\nStarting with Bitcoin Core v0.19 you have to manually enable bloom filters by setting peerbloomfilters=1 in your bitcoin.conf configuration file.
Please make sure that this node is fully synced before you start Bisq.
popup.bitcoinLocalhostNode.additionalRequirements=\n\nThe node was found to be well configured. For reference, the requirements are for the node to have pruning disabled and bloom filters enabled.
popup.shutDownInProgress.headline=Shut down in progress
popup.shutDownInProgress.msg=Shutting down application can take a few seconds.\nPlease don't interrupt this process.

View File

@ -305,6 +305,16 @@ public class MainViewModel implements ViewModel, BisqSetup.BisqSetupListener {
else
torNetworkSettingsWindow.hide();
});
bisqSetup.setDisplayLocalNodeMisconfigurationHandler(
(Runnable continueWithoutLocalNode) ->
new Popup()
.hideCloseButton()
.warning(Res.get("popup.warning.localNodeMisconfigured.explanation"))
.useShutDownButton()
.secondaryActionButtonText(Res.get("popup.warning.localNodeMisconfigured.continueWithoutLocalNode"))
.onSecondaryAction(continueWithoutLocalNode)
.show()
);
bisqSetup.setSpvFileCorruptedHandler(msg -> new Popup().warning(msg)
.actionButtonText(Res.get("settings.net.reSyncSPVChainButton"))
.onAction(() -> GUIUtil.reSyncSPVChain(preferences))
@ -441,10 +451,12 @@ public class MainViewModel implements ViewModel, BisqSetup.BisqSetupListener {
checkNumberOfBtcPeersTimer = UserThread.runAfter(() -> {
// check again numPeers
if (walletsSetup.numPeersProperty().get() == 0) {
if (localBitcoinNode.isDetected())
getWalletServiceErrorMsg().set(Res.get("mainView.networkWarning.localhostBitcoinLost", Res.getBaseCurrencyName().toLowerCase()));
if (localBitcoinNode.isUsable().get())
getWalletServiceErrorMsg().set(
Res.get("mainView.networkWarning.localhostBitcoinLost", Res.getBaseCurrencyName().toLowerCase()));
else
getWalletServiceErrorMsg().set(Res.get("mainView.networkWarning.allConnectionsLost", Res.getBaseCurrencyName().toLowerCase()));
getWalletServiceErrorMsg().set(
Res.get("mainView.networkWarning.allConnectionsLost", Res.getBaseCurrencyName().toLowerCase()));
} else {
getWalletServiceErrorMsg().set(null);
}

View File

@ -165,7 +165,7 @@ public class NetworkSettingsView extends ActivatableView<GridPane, Void> {
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.isDetected()) {
if (!localBitcoinNode.isUsable().get()) {
localhostBtcNodeInfoLabel.setVisible(false);
}
useProvidedNodesRadio.setText(Res.get("settings.net.useProvidedNodesRadio"));
@ -380,7 +380,7 @@ public class NetworkSettingsView extends ActivatableView<GridPane, Void> {
}
private void onBitcoinPeersToggleSelected(boolean calledFromUser) {
boolean bitcoinLocalhostNodeRunning = localBitcoinNode.isDetected();
boolean bitcoinLocalhostNodeRunning = localBitcoinNode.isUsable().get();
useTorForBtcJCheckBox.setDisable(bitcoinLocalhostNodeRunning);
bitcoinNodesLabel.setDisable(bitcoinLocalhostNodeRunning);
btcNodesLabel.setDisable(bitcoinLocalhostNodeRunning);
@ -454,7 +454,7 @@ public class NetworkSettingsView extends ActivatableView<GridPane, Void> {
private void applyPreventPublicBtcNetwork() {
final boolean preventPublicBtcNetwork = isPreventPublicBtcNetwork();
usePublicNodesRadio.setDisable(localBitcoinNode.isDetected() || preventPublicBtcNetwork);
usePublicNodesRadio.setDisable(localBitcoinNode.isUsable().get() || preventPublicBtcNetwork);
if (preventPublicBtcNetwork && selectedBitcoinNodesOption == BtcNodes.BitcoinNodesOption.PUBLIC) {
selectedBitcoinNodesOption = BtcNodes.BitcoinNodesOption.PROVIDED;
preferences.setBitcoinNodesOptionOrdinal(selectedBitcoinNodesOption.ordinal());