[bisq-network/bisq] WalletAppSetup's constructor has an unfulfilled dependency (#4005)

[dmos62]

### Description

`WalletAppSetup` is instantiated on Bisq app's startup by Guice. Its constructor calls `Preferences::getUseTorForBitcoinJ`, which in turn queries the singleton `LocalBitcoinNode` for whether or not a local Bitcoin node will be used.

The problem is that `LocalBitcoinNode` does not actually check for a local Bitcoin node until it is told to during one of the later stages of setup (in `BisqSetup::step2`).

The reason why LocalBitcoinNode does not throw an exception when it is queried too early is that the queries return a cached value (which is populated by the check triggered in `BisqSetup::step2`), but the cache is initialized to `false` (signifying that a locally running Bitcoin node is not found), so when LocalBitcoinNode is queried too early it just returns `false`.

That's how this erronious usage went unnoticed, and there could be other usages like this.

[This PR](https://github.com/bisq-network/bisq/pull/3982) affects this a bit. It switches LocalBitcoinNode from using plain `boolean` to using `Optional<Boolean>`, the rationale being that a variable that describes the result of a detection attempt in this case has 3 possible states: detected; undetected; not yet attempted detection (represented by `Optional.empty()`). As the PR is now, in case `LocalBitcoinNode` is queried before it has performed its checks, it reverts to the old behaviour (returning `false`), but it also logs an error message with a stacktrace. That's how this bug was discovered.

### Steps to reproduce

You could use the mentioned PR, which makes `LocalBitcoinNode` aware of whether or not the checks have been performed, or, you could log the `LocalBitcoinNode` query inside `Preferences::getUseTorForBitcoinJ` and run a local Bitcoin node (you would see that `LocalBitcoinNode` sometimes says that there isn't a local Bitcoin node).

### Expected behaviour

`LocalBitcoinNode` to return accurate information ("undetected" is not the same as "detection has not been attempted"), and one of the following:
a) assure that `LocalBitcoinNode` is never queried when its checks were not performed beforehand;
b) or, assure that it is impossible to do that.

### Possible solution

Restructure `LocalBitcoinNode` to trigger a detection whenever it is queried, but the detection is not yet performed.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/issues/4005
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20200224/18e2291f/attachment.html>