Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add local Bitcoin node configuration detection #3982

Merged
merged 20 commits into from
Feb 27, 2020

Commits on Feb 17, 2020

  1. 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.
    dmos62 committed Feb 17, 2020
    Configuration menu
    Copy the full SHA
    f895da4 View commit details
    Browse the repository at this point in the history

Commits on Feb 18, 2020

  1. Downgrade Optional usage to Java 10

    I was erroniously targeting Java 11, when Travis requires Java 10. `Optional.isEmpty` shows up only in Java 11.
    dmos62 committed Feb 18, 2020
    Configuration menu
    Copy the full SHA
    18478d9 View commit details
    Browse the repository at this point in the history
  2. Remove defunct test suite

    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.
    dmos62 committed Feb 18, 2020
    Configuration menu
    Copy the full SHA
    74c946a View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    daa1b0b View commit details
    Browse the repository at this point in the history
  4. Improve marking that method is empty

    Co-Authored-By: sqrrm <sqrrm@users.noreply.github.com>
    dmos62 and sqrrm committed Feb 18, 2020
    Configuration menu
    Copy the full SHA
    e6dea3d View commit details
    Browse the repository at this point in the history

Commits on Feb 21, 2020

  1. Configuration menu
    Copy the full SHA
    65177fc View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    08cd31b View commit details
    Browse the repository at this point in the history
  3. Formating changes

    dmos62 committed Feb 21, 2020
    Configuration menu
    Copy the full SHA
    7848836 View commit details
    Browse the repository at this point in the history

Commits on Feb 25, 2020

  1. Perform checks automatically on first query

    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 bisq-network#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).
    dmos62 committed Feb 25, 2020
    Configuration menu
    Copy the full SHA
    0bbbe8c View commit details
    Browse the repository at this point in the history
  2. Reorder methods

    dmos62 committed Feb 25, 2020
    Configuration menu
    Copy the full SHA
    aceb608 View commit details
    Browse the repository at this point in the history
  3. Centralize some of local BTC node logic

    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.
    dmos62 committed Feb 25, 2020
    Configuration menu
    Copy the full SHA
    6b4878a View commit details
    Browse the repository at this point in the history
  4. Fix failing test

    dmos62 committed Feb 25, 2020
    Configuration menu
    Copy the full SHA
    2a57ecd View commit details
    Browse the repository at this point in the history

Commits on Feb 26, 2020

  1. Minor requested changes (github batch)

    Co-Authored-By: Christoph Atteneder <christoph.atteneder@gmail.com>
    dmos62 and ripcurlx committed Feb 26, 2020
    Configuration menu
    Copy the full SHA
    6dec780 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    a92b6ad View commit details
    Browse the repository at this point in the history
  3. Have detection work on other network modes

    This makes detection work on other BTC network modes, not only mainnet.
    dmos62 committed Feb 26, 2020
    Configuration menu
    Copy the full SHA
    30578bf View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    fdaced4 View commit details
    Browse the repository at this point in the history
  5. Polish formatting

    cbeams committed Feb 26, 2020
    Configuration menu
    Copy the full SHA
    b93ca2b View commit details
    Browse the repository at this point in the history

Commits on Feb 27, 2020

  1. Polish LocalBitcoinNode method names and visibility

     - Rename #willUse => #shouldBeUsed
     - Rename #willIgnore => #shouldBeIgnored
     - Reduce visibility of methods where appropriate
     - Edit Javadoc typos and use @link syntax where appropriate
    cbeams committed Feb 27, 2020
    Configuration menu
    Copy the full SHA
    c1a99cc View commit details
    Browse the repository at this point in the history
  2. Remove unnecessary LOCAL_BITCOIN_NODE_PORT constant

    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.
    cbeams committed Feb 27, 2020
    Configuration menu
    Copy the full SHA
    57b7041 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    85e4515 View commit details
    Browse the repository at this point in the history