This was originally added with the intention that the local Bitcoin node
port could be customized, but in fact it never could be, because Guice
configuration always hard-wired the value to the default port for the
CurrentBaseNetwork's Parameters (eg. 8333 for BTC_MAINNET).
This change removes the constant, removes any Guice wiring and injection
and localizes the hard-coded assignment to the LocalBitcoinNode
constructor to simplify and make things explicit.
If it is desired to allow users to specify a custom port for their local
Bitcoin node, a proper option shoud be added to Config. In the meantime,
users may work around this by using `--btcNodes=localhost:4242` where
4242 is the custom port. Note however, that the pruning and bloom filter
checks will not occur in this case as the provided node address will not
being treated as a LocalBitcoinNode.
Introduces LocalBitcoinNode::willUse and ::willIgnore to move logic that
was previously littered throughout the codebase into one place. Also,
changes the usages of LocalBitcoinNode to be more precise, specifying
which of these questions are being asked:
- "is there a local BTC node" (isDetected);
- "is it well configured" (isWellConfigured and isUsable);
- "will we ignore a local BTC node even if we found a usable one"
(willIgnore);
- "is there a usable local BTC node and will we use it" (willUse).
These changes make related logic much easier to maintain and to read.
It's quite amazing how obvious this was, yet I missed it for such a long
time. Simplifies usage of LocalBitcoinNode and its internals even more
so. Fixes#4005. The way we structured LocalBitcoinNode was as if the
detection checks were expensive, but they're not. Previously, in some
cases we would notice that a local BTC node wouldn't be used even if it
was detected, so we would skip these checks. This optimization now
doesn't happen. It might be reimplemented in a coming change where more
local BTC node logic is moved into LocalBitcoinNode, but, even if it's
not, this check is fairly cheap. A notable exception is if the local BTC
node is not responding, which would cause us to wait for a timeout, but
if that is the case the mentioned optimization wouldn't help (most of
the time).
As mentioned in the issue #3984, Have created lot of buy/sell offers
with different currencies spanning across around 30+ offers
so that we need to use scroll bar in my open offers and able to
recreate the issue.
This change fixes#3998 by avoiding the attempt to create a user's
appDataDir if it already exists. Normally this attempt is not a problem,
as Files#createDirectories does nothing if the target is a directory
that already exists, but in the case that the target is a symbolic link,
Files#createDirectories throws a FileAlreadyExistsException. This change
prevents the call to Files#createDirectories if the target already
exists, and if in fact the target is a symbolic link pointing to an
existing directory, this is not a problem and everything proceeds as per
usual. This mimics the behavior in Bisq v1.2.5 and prior, where
File#mkdirs was used instead of Files#createDirectories. File#mkdirs
does not throw an exception when the target directory is a symlink, it
just returns false.
This is a pure refactoring that renames and inlines variables to tighten
up and make more consistent the implementation of these two methods. It
is done in preparation for a subsequent substantive change that fixes a
bug.
Previously, Config#mkdir and #mkAppDir threw ConfigException if any
underlying IOException was thrown from Files#createDirectory or
Files#createDirectories respectively. This resulted in only a simple
error message being reported at the command line and the details of the
underlying exception's stack trace were lost. This change wraps and
rethrows the underlying IOException as an UncheckedIOException such that
it gets caught by the catch(Throwable) clause in BisqExecutable, which
does print a stacktrace to stderr. This is the way it always should have
worked; throwing ConfigException in these cases was an oversight in the
original implementation. This change also removes the ConfigException
constructor that allows for passing a nested exception to be wrapped as
there is actually no use case for this now that these two anomalies have
been corrected.
This change was made in the process of attempting to reproduce #3998.
The workings of LocalBitcoinNode significantly changed, especially how
detection works. Before, we were only checking if a port was open, but
now we're actually performing a Bitcoin protocol handshake, which is
difficult to stub. For these reasons the old tests are irrelevant and
replacement tests were not written.
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.
