* Add new BSQ issued v. burnt chart
Adds a new two-line chart that plots the month-bucketed BSQ issued and
burnt series. Until now there wasn't a direct visual means of
examining BSQ issue and burn together. This chart aims to fix that.
* Change wording from 15 days to 15-day moving average
Co-Authored-By: Steve Jain <mfiver@gmail.com>
* Make chart's title more clear
"BSQ issued v. burnt" > "BSQ issued v. BSQ burnt"
Co-Authored-By: Steve Jain <mfiver@gmail.com>
* Fix spacing between chart and title
* Reduce title-chart spacing from 20 to 10
Co-authored-by: Steve Jain <mfiver@gmail.com>
This prevents unnecessary load on the seednodes from old clients
loading all trade statistic objects on startup as they interpret
the null value as empty string
The code didn't handle before the use case of new trade statistic objects
created by two old clients. This change make it independent of the cut off date
and allows us at a later point to update all trade statistics objects with
depositTxId value of null.
The check for an address entry before finalizing the multisig payout
might prevent a valid payout when the initiation of the trade process
was interrupted. It's better to allow the completion of the trade
and just log a warning.
Use a separate file, delayed_payout_txs, to write all txs and their
delayed payout tx hashes as json. This will enable users to publish
locked txs when their data dir has been corrupted, as long as the trade
data is there.
* Split seednode systemd service ExecStart command into multiple lines
* Add setting in seednode configuration to specify btcnode host/port
* Tweak seednode torrc configuration options to improve P2P connectivity
* Require bitcoin.service from bisq-seednode.service via systemd binding
* Make seednode installer run from master and build bisq from release tag
* Seednode must be shutdown using `kill -9` until #3884 is fixed
* Fix seednode uninstall script to use correct service names
* Disable CircuitBuildTimeout in seednode Tor configuration
* Disable seednode torrc advanced configuration options for now
* Refactoring of and function in ValidationResult
This function didnt have terminating short circuit which is important
for and operator functions. Also this function gets called multiple
times since it's validation function. hence improving needed here.
After verifying usages, left out following two classes
1. PercentageNumberValidator - since it's using only two validation
2. PhoneNumberValidator - have variable inputs and this is only place.
* Fix code format issue
Co-authored-by: Christoph Atteneder <christoph.atteneder@gmail.com>
Ensure that the superclass methods XYChart.removeDataItemFromDisplay &
XYChart.removeSeriesFromDisplay are always called from the implemented
dataItemRemoved & seriesRemoved methods respectively, as specified by
the API javadoc.
This prevents a leak of old Candle & VolumeBar objects every time the
trades charts view is updated. The former is quite substantial, as each
Candle object has a retained size of about 70kB and there are up to 90
candlesticks / volume bars leaked per chart update.
Replace tail recursion of the play() method with an ordinary loop, to
prevent a new open JAR resource InputStream + sound file OutputStream
(which were created every 4 minute playback) from accumulating on the
stack, closing them inside the loop instead. (This also prevents
eventual stack overflow.)
Also tidy up FileUtil.resourceToFile and put the JAR URL InputStream in
a try-with-resources block, to ensure that it doesn't leak either.
Previously, Travis CI was failing non-deterministically due to a race
condition in which a thread was started in order to call the blocking
ServerSocket.accept() method, and sometimes the subsequent attempt by
LocalBitcoinNode.detectAndRun() to connect to that socket's port would
occur before the thread had actually called the accept() method.
This commit simplifies the approach by removing the thread entirely. As
it turns out, calling accept() is not necessary; simply constructing a
new ServerSocket() binds to and listens on the given port, such that a
subsequent attempt to connect() will succeed.
There is currently no explicit rule for how option-related elements are
ordered in the code, but it is important to understand that the order in
which options are registered via parser.accept() calls is the order in
which they appear in --help output.
It would be nice to group options together into sections and separate
them in the --help output with section headers similar to the way that
Bitcoin Core's help output does it, but this is not a built-in option
with the JOpt Simple library, and not worth trying to hack into place at
the moment.
Previously ConfigTests constructed Config instances with string-based
options, e.g.:
Config config = new Config("--appName=My-Bisq");
The advantage here is clarity, but the downside is repetition of the
option names without any reference to their corresponding Config.*
constants.
One solution to the problem would be to format the option strings using
constants declared in the Config class, e.g.:
Config config = new Config(format("--%s=My-Bisq", APP_NAME));
but this is verbose, cumbersome to read and write and requires repeating
he '--' and '=' option syntax.
This commit introduces the Opt class and the opt() and configWithOpts()
methods to ConfigTests to make testing easier while using constant
references and preserving readability. e.g.:
Config config = configWithOpts(opt(APP_NAME, "My-Bisq"));
In the process of making these changes a bug was discovered in the
monitor submodule's P2PNetworkLoad class and that has been fixed here as
well.
This change also required introducing several option name constants that
had not previously been extracted in order to be referenced within
ConfigTests. For consistency and completeness, all additional option
names that did not previously have a contstant now have one.
Previously (as of the prior commit), a warning was issued if a
non-default config file path was specified at the command line, and then
the default config file path was used as a fallback. On review, however,
it would be better to halt execution immediately if the config file does
not exist. There is no risk of breaking backward compatibility by doing
this as Bisq never had a --configFile option before the recent commits
that introduce it. Furthermore, there is no clear benefit to the
fallback approach. If the user specifies a given config file and it does
not exist, they may not see the warning message in the log, and they may
be left with the impression that they are running against their custom
config file when in fact they are running against the default (which may
be empty or non-existent itself). Thus throwing an exception as is now
done in this commit should make everything more explicit and clear.
This behavior had already been implemented prior to this commit, but has
now been tested and improved with refactoring and logging messages.
Note that this approach emulates Bitcoin Core's own behavior. When
running, for example, `bitcoind -conf=rel/path/to/bitcoin.conf`, the
relative path is prefixed / fully qualified by the value of the
`datadir` option. So if `datadir` equals `~/Library/Application
Support/Bitcoin`, then the `conf` option value above would be fully
qualified as
~/Library/Application Support/Bitcoin/rel/path/to/bitcoin.conf
If the argument to `-conf` is an absolute path, e.g.
`/tmp/bitcoin.conf`, then that absolute path is used without further
modification or qualification. It is assumed that the rationale for this
behavior is to avoid accidentally running against the wrong conf file
because `bitcoind` was invoked in a different directory than usual or
because a malicious actor replaced the relative conf file with their own
version.
Bisq's new `--configFile` option works (and is now tested to work) in
the same way: relative paths get prefixed by the value of
Config.getAppDataDir(), and absolute paths are processed unmodified.
This new class encapsulates all functionality related to detecting a
local Bitcoin node and reporting whether or not it was detected.
Previously this functionality was spread across the Config class
(formerly BisqEnvironment) with its mutable static
isLocalBitcoinNodeRunning property and the BisqSetup class with its
checkIfLocalHostNodeIsRunning method. All of this functionality now
lives within the LocalBitcoinNode class, an instance of which is wired
up via Guice and injected wherever necessary.
Note that the code for detecting whether the node is running has been
simplified, in that it is no longer wrapped in its own dedicated Thread.
There appears to be no performance benefit from doing so, and leaving it
in place would have made testing more difficult than necessary.
Several methods in BisqSetup have also been refactored to accept
callbacks indicating which step should be run next. This has the effect
of clarifying when the step2()-step5() methods will be called.
Previously this static property had been managed within
BaseCurrencyNetwork itself and was accessed directly by callers. Now it
is managed within Config, made private and accessed only via the
new and well-documented baseCurrencyNetwork() method. The same goes for
baseCurrencyNetworkParameters().
It is unfortunate that we must retain these mutable static fields and
accessors, but after trying to eliminate them entirely, this approach is
the lesser of two evils; attempting to use a Config instance and
instance methods only ends up being quite cumbersome to implement,
requiring Config to be injected into many more classes than it currently
is. Getting access to the BaseCurrencyNetwork is basically a special
case, and treating it specially as a static field is in the end the most
pragmatic approach.
Prior to this commit, the way that the appDataDir and its subdirectories
were created was a haphazard process that worked but in a fragile and
non-obvious way. When Config was instantiated, an attempt to call
btcNetworkDir.mkdir() was made, but if appDataDir did not already exist,
this call would always fail because mkdir() does not create parent
directories. This problem was never detected, though, because the
KeyStorage class happened to call mkdirs() on its 'keys' subdirectory,
which, because of the plural mkdirs() call ended up creating the whole
${appDataDir}/${btcNetworkDir}/keys hierarchy. Other btcNetworkDir
subdirectories such as tor/ and db/ then benefited from the hierarchy
already existing when they attempted to call mkdir() for their own dirs.
So the whole arrangement worked only because KeyStorage happened to make
a mkdirs() call and because that code in KeyStorage happened to get
invoked before the code that managed the other subdirectories.
This change ensures that appDataDir and all its subdirectories are
created up front, such that they are guaranteed to exist by the time
they are injected into Storage, KeyStorage, WalletsSetup and TorSetup.
The hierarchy is unchanged, structured as it always has been:
${appDataDir}
└── btc_mainnet
├── db
├── keys
├── wallet
└── tor
Note that the tor/ subdirectory actually gets deleted and re-created
within the TorSetup infrastructure regardless of whether the directory
exists beforehand.
