Note that the default value of 600 advertised in BisqExecutable's option
handling was incorrect. The actual value had since become 1200 MB. This
correct default is now reflected in Config's option handling.
This was added way back in 8f8866da and has since fallen out of use
entirely.
Removing it now because it depends on BisqEnvironment.appName, which
will be moved to Config in the next commit.
NOTE: This removes entirely the old BisqExecutable.appDataDir method
implemented for the v0.5.3 hotfix that renames the data dir from 'bisq'
to 'Bisq'. See a7f3d68cb for details.
Set OptionParser.allowsUnrecognizedOptions(true) to make sure
BisqEnvironment doesn't fail while options are still being transferred
one-by-one to Config.
Prior to this commit, BisqExecutable has been responsible for parsing
command line and config file options and BisqEnvironment has been
responsible for assigning default values to those options and providing
access to option values to callers throughout the codebase.
This approach has worked, but at considerable costs in complexity,
verbosity, and lack of any type-safety in option values. BisqEnvironment
is based on the Spring Framework's Environment abstraction, which
provides a great deal of flexibility in handling command line options,
environment variables, and more, but also operates on the assumption
that such inputs have String-based values.
After having this infrastructure in place for years now, it has become
evident that using Spring's Environment abstraction was both overkill
for what we needed and limited us from getting the kind of concision and
type saftey that we want. The Environment abstraction is by default
actually too flexible. For example, Bisq does not want or need to have
environment variables potentially overriding configuration file values,
as this increases our attack surface and makes our threat model more
complex. This is why we explicitly removed support for handling
environment variables quite some time ago.
The BisqEnvironment class has also organically evolved toward becoming a
kind of "God object", responsible for more than just option handling. It
is also, for example, responsible for tracking the status of the user's
local Bitcoin node, if any. It is also responsible for writing values to
the bisq.properties config file when certain ban filters arrive via the
p2p network. In the commits that follow, these unrelated functions will
be factored out appropriately in order to separate concerns.
As a solution to these problems, this commit begins the process of
eliminating BisqEnvironment in favor of a new, bespoke Config class
custom-tailored to Bisq's needs. Config removes the responsibility for
option parsing from BisqExecutable, and in the end provides "one-stop
shopping" for all option parsing and access needs.
The changes included in this commit represent a proof of concept for the
Config class, where handling of a number of options has been moved from
BisqEnvironment and BisqExecutable over to Config. Because the migration
is only partial, both Config and BisqEnvironment are injected
side-by-side into calling code that needs access to options. As the
migration is completed, BisqEnvironment will be removed entirely, and
only the Config object will remain.
An additional benefit of the elimination of BisqEnvironment is that it
will allow us to remove our dependency on the Spring Framework (with the
exception of the standalone pricenode application, which is Spring-based
by design).
Note that while this change and those that follow it are principally a
refactoring effort, certain functional changes have been introduced. For
example, Bisq now supports a `--configFile` argument at the command line
that functions very similarly to Bitcoin Core's `-conf` option.
Problem: a stack trace was being thrown during daemon startup from
BisqDaemonMain.onSetupComplete when it attempted to start a
second BisqGrpcServer and failed to bind to the already-bound port.
The first BisqGrpcServer is started in
BisqDaemonMain.onApplicationStarted much earlier in the startup process.
Solution: remove the second attempt to start the server by removing
BisqDaemonMain's implementation of onSetupComplete, and in turn remove
the now-obsolete bisqGrpcServer field as well.
This change also eliminates the BisqGrpcServer.blockUntilShutdown
method, which in turn called the underlying grpc server's
awaitTermination method. As the comment there explained, this was
thought to be necessary because grpc "does not use daemon threads by
default", but this is actually incorrect. According to the grpc Javadoc
at [1], "Grpc uses non-daemon Threads by default and thus a Server will
continue to run even after the main thread has terminated."
[1]: https://git.io/JePjn
- Rename package bisq.grpc => bisq.daemon.app
- Rename BisqGrpcApp => BisqDaemon
- Rename BisqGrpcServerMain => BisqDaemonMain
The script `bisq-grpc` has been renamed to `bisq-daemon` accordingly
(and will later be customized to `bisqd`). To see everything in action,
issue the following commands:
$ gradle build
$ ./bisq-daemon --appName=Bisq-throwaway --daoActivated=false
$ echo getVersion | ./bisq-cli # in a second terminal
1.2.3
The :grpc module will soon be renamed to :daemon. These two modules
represent two separate and equal modes of running bisq, either as a
desktop GUI or as a daemon. They are both applications, and one should
not depend on the other as it would be illogical and confusing to model
things that way. The reason for the current dependency from :desktop to
:grpc is because :grpc is the home of BisqGrpcServer. This change moves
this class up to :core, in a new bisq.core.grpc package, such that both
the :desktop and :daemon applications can pull it in cleanly.
The CoreApi 'facade' that BisqGrpcServer uses to abstract away bisq
internals has been moved from bisq.core to bisq.core.grpc as well and
for the same reasons detailed in 8b30c22d6.
This change also renames the Java package for generated grpc types from
bisq.grpc.protobuf to bisq.core.grpc (the same new package that
BisqGrpcServer and CoreApi now live in). Again, this is for reasons of
cohesion: BisqGrpcServer is the only user of these grpc-generated types,
and they should logically live in the same package (even if they
physically live in separate source dirs at the build level).
This change:
- Removes several superfluous dependencies not required for our
purposes with gRPC
- Cleans up the way Gradle source sets are managed for generated gRPC
sources and classes
- Makes use of Gradle's new `implementation`, `compileOnly` and
`runtimeOnly` dependency configurations where changes were otherwise
being made. See https://stackoverflow.com/a/47365147 for details.
Remaining uses of the now-deprecated `compile` and `runtime`
configurations should be eliminated in a refactoring separate and
apart from the present gRPC API work.
- Upgrades several existing dependencies to align with newer versions
of the same dependencies introduced transitively by grpc-* 1.25.0
libraries, including:
- protoc from 3.9.1 => 3.10.0
- gson from 2.7 => 2.8.5
Note that a number of the grpc-* libraries depend on Guava v28, and our
existing dependency on Guava v20 has *not* been upgraded to this newer
version because it is incompatible with the way we have used Guava's
Futures API. It appears that the grpc-* libraries function correctly
against this older version of Guava, and more investigation would be
required see whether upgrading our uses to the new Guava API is feasible
/ worth it. The way we are preventing this upgrade is with the use of
`exclude(module: "guava")` directives on grpc-* dependencies.
This change replaces the previous "Hello, World!" :cli main stub with
the contents of the BisqGrpcClient as originally implemented in :grpc,
such that the following now works:
$ gradle build
$ ./bisq-daemon # in first terminal
$ ./bisq-cli # in second terminal
getVersion # user input to stdin
1.2.3 # rpc response
getBalance # user input to stdin
0.00 # rpc response
...
Or for a user experience closer to where things are headed:
$ echo getVersion | ./bisq-cli
1.2.3
Note that processing the above command is currently far too slow:
$ time echo getVersion | ./bisq-cli
1.2.3
real 0m1.634s # ouch
user 0m1.746s
sys 0m0.180s
Subsequent commits will work to bring this time down to a minimum.
Note: Includes the code quality changes originally made to
BisqGrpcClient in commit 7595387.
This change stubs out the `bisq-cli` utility with a placeholder main
method, such that the following now works:
$ gradle :cli:build
$ ./bisq-cli
Hello, World!
There are two structural / organizational reasons for this move:
1. References from one package to another should always be upward or
lateral, never downward, as the latter causes package cycles (aka
'tangles') which damage the suppleness and understandability of a large
codebase. Prior to this change the high-level bisq.core.CoreModule
class imported many classes from child packages like
bisq.core.{btc,dao,user,util}, etc. By moving CoreModule down into the
'.app' package, it can reference all these other packages as siblings
instead of doing so as a parent.
2. the bisq.core.desktop and bisq.core.app packages are the only
locations that reference the CoreModule class. By moving the class
into bisq.core.app, greater cohesion is acheived, again making the
codebase that much easier to read and understand.
Relevant issue thread: #3753
Currently the daily burnt BSQ chart under 'DAO -> Facts and Figures' is
distorted by outliers. This introduces a 'Zoom to inliers' toggle (off
by default), which, when toggled on, effectively zooms the chart to
inliers, thus removing the distortion. Also, a moving average is
plotted, to further improve the chart's readibility.
The chart is also changed from an area chart to a line chart, on the
presumption that it was an area chart for cosmetic reasons, but now that
there are two series in it (the moving average was added) an area chart
makes less sense.
Another noteworthy change is that the other chart in the screen, monthly
issued BSQ, has its Y axis set to start at zero, so as to improve
readability. This might seem outside the scope of this commit, but the
other changes involved some refactoring, which involved cleaning up some
duplicated logic, which involved configuring both of these charts
together, which involved forcing zero to be on the axis.
This implementation mixes some plotting logic (responsible for zooming
in on inliers) into the view logic, because I opted to implement said
zooming as an external manipulation of a chart's axis. I chose this in
favor of implementing a new Chart, because it would have required
including multiple large classes (relevant JavaFX's classes can't be
ergonomically extended) to the code base. I presumed that my chosen
solution will be easier to maintain.
I am not entirely happy with this choice and can see myself introducing
some plotting-related classes to encapsulate creating charts like these,
thus unmixing plotting logic from view logic. In the meantime this is a
working solution, and I plan to continue working on these charts in the
near future.
Allows computing a simple moving average lazily from a Stream, to be
used in plotting noisy data. Will be used in the daily burnt BSQ chart
under 'DAO -> Facts and Figures'. Can be easily modified to compute
other moving averages, like exponential.
This utility provides ways to zoom in a chart (more specifically its
axes) on inlier data points. The principal use-case is to restore
readability to charts that were distorted by outliers. The first (and
maybe the last) chart to use this is the daily burnt BSQ chart under
'DAO -> Facts and Figures'.
