Add a new method to DisplayUtils to restore the old rounding behaviour
of formatVolumeWithCode whenever a fractional volume is required. This
fixes a regression caused by #3926 to remove unnecessarily displayed
decimals for fiat volumes - it appears that in every case but the avg.
dollar price on the BSQ dashboard a whole number should be shown.
Also add a relevant test.
* Add signing debug logs
Fix warning about possible nullpointer exceptions. Better to have an
explicit check for debug purposes.
Smaller cleanups
* Fix codacy comment
* Remove decimals for displayed fiat volume amounts
* Reset rounded for privacy info when switching between payment methods
Co-authored-by: Christoph Atteneder <christoph.atteneder@gmail.com>
* Add keyboard shortcut for trade process refresh, fix#3905
Trades have been getting stuck in the `Wait for payment` state, perhaps
due to lost mailbox messages, but it's hard to know for sure. There is
currently no way to get out of this state except going to mediation.
With ctrl+R the seller can ask the buyer to refresh the current trade
state and the buyer will resend the
`CounterCurrencyTransferStartedMessage` if they are in the phase
FIAT_SENT.
* Disallow more than one trade refresh per day
* Add refresh button for seller step 2, fix#3905
A seller can ask to refresh the trade process once every 24 hours. This
step has been a problem causing a lot of mediation lately so this is a
way to ask the buyer to resend the CounterCurrencyTransferStartedMessage
This fixes the problem when a mailbox message was lost. To test the
seller need to not get the first CounterCurrencyTransferStartedMessage
sent by the buyer, for example by letting the seednode drop it instead
of sending to the seller. When button is pressed
- a RefreshTradeStateRequest is sent from seller to buyer
- the buyer receives the RefreshTradeStateRequest and
- ignores it if it's not in FIAT_SENT phase
- responds with a CounterCurrencyTransferStartedMessage if in FIAT_SENT
phase and has already sent a CounterCurrencyTransferStartedMessage
* Fix codacy remarks
Move incoming message handling method to the right section
* Add refresh button info text
Hide refresh section when not available rather than graying out
Added info text:
Sometimes P2P network messages acknowledging payment are not delivered,
causing trades to get stuck. Hit the button below to make your peer
resend the last message.
* Fix codacy issues
* 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>
* 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.
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.
In previous commits, BisqEnvironment functionality has been fully ported
to the new, simpler and more type-safe Config class. This change removes
BisqEnvironment and all dependencies on the Spring Framework Environment
interface that it implements.
The one exception is the pricenode module, which is separate and apart
from the rest of the codebase in that it is a standalone, Spring-based
HTTP service.
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.
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'.
* Use macOS app Info.plist setting for automatic light/dark title bar
* Update desktop/package/macosx/Info.plist
Co-Authored-By: Christoph Atteneder <christoph.atteneder@gmail.com>
Co-authored-by: Christoph Atteneder <christoph.atteneder@gmail.com>
When a user tries to take an offer which is above his limit, the old popup is still being shown.
It says that the user limits will be raised when his account is 2 months old.
But that is not true anymore, now we have account signing etc...
This fix uses the string that is already used for offer creation, which is good since preserves translations.
But I think we would need a better message, which I am afraid I cannot get done, showing the sign state and age.
I am submitting this PR as is now since I think this is a big issue.
New users might try to use Bisq to accept an offer, get this message and then decide to wait 2 months, in vain.
So, it partially fixes#3885.
If the user entered an invalid hostname for a custom BTC node, such as a
V3 onion address, after restarting Bisq they would be presented with an
error due to the node being unreachable and unable to continue nor
correct the config.
So now a warning message will be shown in this situation informing the
user of an invalid config and once they restart their client they will
connect to the provided BTC nodes.
Fixes#3137
