The runAfter* methods delegate to UITimer::run(Later|Periodically) in
the case of the desktop application. These use the JavaFX TimeLine API
(via bisq.common.reactfx.FXTimer) to schedule future events. However,
this API isn't thread safe and isn't meant to be called outside the FX
application thread. This causes occasional misfirings and out-of-order
scheduling when UserThread::runAfter is called outside the user thread.
Make the UITimer::run* methods safe to call from any thread by checking
we are in the application thread and delegating to UserThread::execute
otherwise. This also improves consistency between the contracts of the
runAfter* and execute methods. As the former has many call sites, this
is safer than trying to track down all the non-thread-safe uses.
(The Timer used in the headless app already appears to be thread-safe.)
This fixes#4055 (Bisq sometimes fails to prompt user for password to
unlock wallet), caused by out-of-order scheduling of the execute and
runAfter tasks in the WalletConfig.onSetupCompleted anonymous class
method in bisq.core.btc.setup.WalletsSetup.initialize.
Also prevent an exception caused by non-thread-safe calls into JavaFX
during the shutdown of OpenOfferManager, which was uncovered by the
above, by adding a missing UserThread::execute call.
- added a comment describing the `removeDust` method and its effects.
- use more descriptive variable names.
- made the logging more verbose to help log readers.
- use a constant for the dust limit
- add a notice to the user when dust is padded to the fee
Before, the graceful shutdown procedures have been executed in the
user thread. However, the sync mechanics for connections/offers
caused a lockup, as some little parts of the code do execute on the
user thread as well.
Display the account number on the same row as the sort code in the trade
step view, to prevent scrolling with the extra name field (as suggested
in the code review).
(This also affects the layout of old accounts without the extra field.)
Also apply the suggested popup text simplifications.
Open an info popup in the take/create offer view, upon choosing to take
or make a new offer, to instruct the user to recreate their old Faster
Payments account with an owner full name (and preserved salt). Also show
the popup upon manual selection of any old (i.e. missing full name)
Faster Payments account from the trading account combo box, analogously
to the ClearXchange (Zelle) warning popup logic.
(Also eliminate slight differences between the private
'maybeShow[ClearXchange|FasterPayments]Warning' methods in TakeOfferView
and MutableOfferView, to make the code easier to deduplicate in future.)
This change fixes an issue whereby dust change outputs are
inadvertently created when users make withdrawals from their
wallets. (Funds -> Send Funds)
The solution taken here is to detect a dust TXO during the withdrawal
fee estimation process and add that amount to the fee thus eliminating
the dust output.
For example if the user has 1 BTC and goes to withdraw 0.99999900 BTC
it will detect a change TXO of 100 sats which is below the dust limit,
increase the fee by 100 sats and therefore withdraw 1 BTC.
This fix only applies to user withdrawals from their wallet. Other
use cases such as P2P trading, deposits and fees will be handled
separately.
Related to #4039
This change fixes an issue with sorting the offer list when the amount
is shown as a range. In OfferBookView::activate() we add a listener
for the sortTypeProperty on amountColumn and volumeColumn. When the
sortType is changed we set the comparator to be the approprate property
of the Offer; either getAmount/getMinAmount; getVolume/getMinVolume.
Fixes#3818
Currently the offer book tables are only being sized when the window
proportions change. However if the window is made bigger before the
offer book is opened, then the tables are not resized from their
default.
Other windows (for example Market -> Trades) solve this by make a
sizing call in the activate method. In order to do this the sizing
code is separated into a layout method where it can be called from
both activate and the existing heightProperty listener. The same
approach is taken here.
Fixes#4030
Testing showed that the new mechanic for checking a local BTC node's
configuration is unstable. This commit reverts to just checking if the
relevant port is open. The recent refactoring and centralization of
logic is still in place.
Testing showed that the new mechanic for checking a local BTC node's
configuration is unstable. This commit reverts to just checking if the
relevant port is open. The recent refactoring and centralization of
logic is still in place.
This change substitutes use of memory efficient, OS supplied sleep/suspend
inhibitor utilties for the silent audio file player on Linux, merges
OSXStandbyModeDisabler functionality into AvoidStandyModeService, and
removes that class.
This change also stops Bisq from running the audio file on OSX; it is
currently running both caffeinate and the silent audio file, and the avoid
standby mode button in the the OSX settings view is currently toggling
the audio player on and off, but not caffeinate. (The only way to shut
down caffeinate is by shutting down Bisq.)
The OSX avoid standby mode button button has not been hidden so a cached
'do not avoid standby mode' preference does not leave the user stuck without
a caffeinate service the next time they start Bisq. (They can use it to
turn caffeinate on, but it can't be used to turn it off without shutting
down Bisq too.)
The avoid standy mode button is now displayed on Linux because the native
inhibitor can be toggled on and off.
There is no change to the avoid shutdown service on Windows and Unix.
Use excludeFromJsonDataMap to add an optional owner full name property
to Faster Payments (GBP) accounts, without affecting the trade JSON
contract (for backwards compatibility). This is to fix#3976, that some
banks have started checking the recipient name.
Make the name a non-optional field for _new_ accounts, but display the
details for old Faster Payments accounts in exactly the same way as
before, only showing the extra field in the info popup, account and
trade step views when it is present.
Also update the now misleading payment instructions in the buyer popup.
TODO: Update translations (which probably aren't needed anyway).
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.)
Avoid a bottleneck computing the cycle index & calling 'Res.get(..)' for
every block since genesis in the DAO state monitor view, when building
the DaoStateBlockListItem objects, by making the 'height' field lazy. To
do this, pass the cycle index into the constructor using an IntSupplier
and make the height a memoised 'Supplier<String>' with a custom getter.
Also add a unit test to check that the auto-generated equals & hashCode
methods still work as expected, as it isn't totally clear what Lombok
would do when a field type differs from its getter return type.
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.
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#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).
As mentioned in the issue #3984, Have created lot of buy/sell offers
with different currencies spanning across around 30+ offers
so that we need to use scroll bar in my open offers and able to
recreate the issue.
