We add a second button (displayed as first) to the preferences UI for
resync from latest resources. We add a warning to the resync from
genesis as it takes very long time and causes heavy network load for
seeds.
The resync button at the stateMonitor does now a resync from resources,
not from genesis.
We add handling of the UnconfirmedBsqChangeOutputList file as well.
In Filemanager we add a check if file exists.
Protobuf definition files were moved from common and core to a new
protodefinition subproject.
The two main reasons for doing this are to speed up builds by not
having to regenerate common and core protobuf classes
every time a change is made in those subprojects, and to remove
the grpc cli's direct dependency on core, and the transitive dependency
on common.
In order to accomplish this, cli's BisqCliMain was stripped of
its dependencies on common and core. Cli can only get the version
and balance now.
gRPC stub boilerplate was moved from BisqCliMain to a CliCommand
class to avoid some of the bloat that is going to happen as the
read-response loop supports more rpc commands.
Provide UserThreadMappedPersistableList subclass for persistable lists
which need to implement UserThreadMappedPersistableEnvelope, instead of
putting the interface on the base class.
Make the (non-storage) classes MeritList and VoteWithProposalTxIdList
keep the original PersistableList superclass, deriving the remaining
subclasses of PersistableList from the new class instead. In this way,
further persistence-related changes are less likely to inadvertently
alter the behaviour of those two consensus-critical classes.
Removing the superfluous PersistableEnvelope interface from the two
classes (via the base class) will be done in a separate PR.
Make the default toPersistableMessage() method of PersistableEnvelope
simply delegate to Proto.toProtoMessage for speed, so that stores can
explicitly implement (Threaded|UserThreadMapped)PersistableEnvelope if
they actually need concurrency control.
As part of this, make PeerList implement PersistableEnvelope directly
instead of extending PersistableList, as it is non-critical & cloned on
the user thread prior to storage anyway, so doesn't need be thread-safe.
In this way, only PaymentAccountList & small DAO-related stores extend
PersistableList, so they can all be made user-thread-mapped.
After this change, the only concrete store classes not implementing
(Threaded|UserThreadMapped)PersistableEnvelope are:
AccountAgeWitness, BlindVotePayload, ProposalPayload, SignedWitness,
TradeStatistics2, NavigationPath & PeerList
The first five appear to erroneously implement PersistableEnvelope and
can be cleaned up in a separate commit. The last two are non-critical.
(Make NavigationPath.path an immutable list, for slightly better thread
safety anyway - that way it will never be observed half-constructed.)
There are circumstances where input via --appDataDir
will lead to the hiddenservice-directory to be deleted.
Successfully reproduced using
--appDataDir=~/foo
Although the "~" does not get interpreted correctly on
my linux system, it does manage to throw off the
mechanics of sparing the hiddenservice-directory from
being deletd.
Add toProtoMessageSynchronized() default method to PersistableEnvelope,
which performs (blocking) protobuf serialisation in the user thread,
regardless of the calling thread. This should prevent data races like
the ConcurrentModificationException observed in #3752, under the
reasonable assumption that shared persistable objects are only mutated
in the user thread.
Also add a ThreadedPersistableEnvelope sub-interface overriding the
default method above, to let objects which are expensive to serialise
(like DaoStateStore) be selectively serialised in the 'save-file-task-X'
thread as before, but directly synchronised with each mutating op. As
most objects are cheap to serialise, this avoids a noticeable perf drop
without having to track down every mutating method for each store.
In all cases but one, classes implementing ThreadedPersistableEnvelope
are stores like TradeStatistic2Store, with a single ConcurrentHashMap
field. These require no further serialisation, since the map entries are
immutable, so the only mutating operations are map.put(..) calls which
are already synchronised with map reads. (Even if map.values().stream()
sees updates @ different keys happen out-of-order, it should be benign.)
The remaining case is DaoStateStore, which is only ever reset or
modified via a single persist(..) call with a cloned DaoState instance
and hash chain from DaoStateSnapshotService, so there is no aliasing
risk from the various DAO state mutations done in DaoStateService and
elsewhere.
This should fix#3752.
Minor change for consistency: narrow the signature of some remaining
such methods, which have return type 'PersistableEnvelope'.
(This excludes some other cases with return type 'NetworkEnvelope'.)
This change fixes#3998 by avoiding the attempt to create a user's
appDataDir if it already exists. Normally this attempt is not a problem,
as Files#createDirectories does nothing if the target is a directory
that already exists, but in the case that the target is a symbolic link,
Files#createDirectories throws a FileAlreadyExistsException. This change
prevents the call to Files#createDirectories if the target already
exists, and if in fact the target is a symbolic link pointing to an
existing directory, this is not a problem and everything proceeds as per
usual. This mimics the behavior in Bisq v1.2.5 and prior, where
File#mkdirs was used instead of Files#createDirectories. File#mkdirs
does not throw an exception when the target directory is a symlink, it
just returns false.
This is a pure refactoring that renames and inlines variables to tighten
up and make more consistent the implementation of these two methods. It
is done in preparation for a subsequent substantive change that fixes a
bug.
Previously, Config#mkdir and #mkAppDir threw ConfigException if any
underlying IOException was thrown from Files#createDirectory or
Files#createDirectories respectively. This resulted in only a simple
error message being reported at the command line and the details of the
underlying exception's stack trace were lost. This change wraps and
rethrows the underlying IOException as an UncheckedIOException such that
it gets caught by the catch(Throwable) clause in BisqExecutable, which
does print a stacktrace to stderr. This is the way it always should have
worked; throwing ConfigException in these cases was an oversight in the
original implementation. This change also removes the ConfigException
constructor that allows for passing a nested exception to be wrapped as
there is actually no use case for this now that these two anomalies have
been corrected.
This change was made in the process of attempting to reproduce #3998.
Prior to this commit, Bisq's new `Config` infrastructure would throw an
exception on encountering an unrecognized option in the
`bisq.properties` config file and the Bisq application would handle this
by reporting an error at the command line indicating that the given
option was not recognized.
For example, given a bisq.properties file that reads as follows:
$ cat $APP_DATA_DIR/bisq.properties
bogus=42
running Bisq would exit with the following error message:
$ java -jar bisq.jar
error: problem parsing option 'bogus': bogus is not a recognized option.
This was reasonable enough behavior, but it had two problems. The first
is that the error message did not indicate to the user that the
unrecognized option was found in the config file as opposed to the
command line. Users unfamiliar with the config file might not know to
look there to eliminate or fix the offending option. The second problem
surfaced when testing the Windows executable build for the v1.2.6
release. When such an error was encountered, the executable would just
fail silently, reporting nothing to the user.
Both of these problems should be addressed on their own, i.e. option
parsing errors should report to the user whether the offending option
was at the command line or in the config file, and our packaged
executables should never just fail silently if we can avoid it.
However, this change does not address either of these problems directly.
It rather avoids the problems completely by relaxing config file
parsing to silently allow unrecognized options. This behavior mimics
prior (pre-`Config`) Bisq versions, and it also follows suit with
Bitcoin Core's own config file processing.
This fixes#3966, which surfaced this problem. In that particular issue,
an option line had somehow been added to the user's bisq.properties
config file that consisted of an option key containing 452 octal null
characters (\u0000) followed by an equals sign and an empty option
value. It is unknown how this bogus option was ever added to the user's
config file in the first place, but on the chance that previous Bisq
versions somehow added this, the changes in this commit ensure that such
entries will not cause affected users' Bisq applications to silently
fail in versions 1.2.6 and beyond.
Prior to this commit, Bisq's new `Config` infrastructure would throw an
exception on encountering an unrecognized option in the
`bisq.properties` config file and the Bisq application would handle this
by reporting an error at the command line indicating that the given
option was not recognized.
For example, given a bisq.properties file that reads as follows:
$ cat $APP_DATA_DIR/bisq.properties
bogus=42
running Bisq would exit with the following error message:
$ java -jar bisq.jar
error: problem parsing option 'bogus': bogus is not a recognized option.
This was reasonable enough behavior, but it had two problems. The first
is that the error message did not indicate to the user that the
unrecognized option was found in the config file as opposed to the
command line. Users unfamiliar with the config file might not know to
look there to eliminate or fix the offending option. The second problem
surfaced when testing the Windows executable build for the v1.2.6
release. When such an error was encountered, the executable would just
fail silently, reporting nothing to the user.
Both of these problems should be addressed on their own, i.e. option
parsing errors should report to the user whether the offending option
was at the command line or in the config file, and our packaged
executables should never just fail silently if we can avoid it.
However, this change does not address either of these problems directly.
It rather avoids the problems completely by relaxing config file
parsing to silently allow unrecognized options. This behavior mimics
prior (pre-`Config`) Bisq versions, and it also follows suit with
Bitcoin Core's own config file processing.
This fixes#3966, which surfaced this problem. In that particular issue,
an option line had somehow been added to the user's bisq.properties
config file that consisted of an option key containing 452 octal null
characters (\u0000) followed by an equals sign and an empty option
value. It is unknown how this bogus option was ever added to the user's
config file in the first place, but on the chance that previous Bisq
versions somehow added this, the changes in this commit ensure that such
entries will not cause affected users' Bisq applications to silently
fail in versions 1.2.6 and beyond.
* Close stream writer prior to deleting temp file
On Windows, instantiating the stream writer appears to lock the file,
preventing the file from being deleted. This was causing the assertion
that the file is deleted to fail.
* Use proper file path on Windows
On Windows, the exception message contained backwards slashes causing
the test to fail.
* Resolve codacy issues
* 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
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.
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.
