There have been several long delays as well a wrong order of the
shutdown process (wallet got shutdown after network shutdown.
Shutdown is now pretty fast, but depends on open offers and connections.
Previous version had several performance issues:
- We kept tradeStatistics in a lookup map in memory (about 40 MB)
- We built up the StatefulAssets even if it was not used/needed.
Building the StatefulAssets was an expensive call.
- AssetTradeActivityCheck was only used for printing out statistics and
called on startup causing performance cost.
Optimized version:
- We call build the StatefulAssets only on demand (user activates
view). Method call takes now 22 ms.
- We create a optimized lookup map with a tuple of the data we need
and keep it as local variable.
- We removed AssetTradeActivityCheck.
Thanks to the profiling reporting from:
https://github.com/bisq-network/proposals/issues/199
Converted sound file to a 10 second mono noise with lower volume.
Tested with max possible sound volume and it was hardly audible.
Tested a binary built with it and I could not observe the network
degradation issues.
File is not 800 kb instead of 42 MB.
As part of the Tor V3 upgrade, this PR adds 2 new V3 seednodes, and will
start the process to retire older V2 seednodes.
The V2 seednodes will continue operating for 2-3 months during the
retirement phase-out period, after which time we can filter it out from
the network for clients who have not upgraded if necessary.
Pending merge of #4315 and rough consensus to proceed with migration plan in bisq-network/projects#35
wizpriceje6q5tdrxkyiazsgu7irquiqjy2dptezqhrtu7l2qelqktid (@wiz)
emzypricpidesmyqg2hc6dkwitqzaxrqnpkdg3ae2wef5znncu2ambqd (@Emzy)
devinpndvdwll4wiqcyq5e7itezmarg7rzicrvf6brzkwxdm374kmmyd (@devinbileck)
aprcndeiwdrkbf4fq7iozxbd27dl72oeo76n7zmjwdi4z34agdrnheyd (@mrosseel)
ro7nv73awqs3ga2qtqeqawrjpbxwarsazznszvr6whv7tes5ehffopid (@alexej996)
Rename timestamp field which implied it represents an epoch value in
seconds, but the way it was used to build a Date object showed that it
actually expected a millis value.
The lastRequest timestamp is changed to show the last request to a
pricenode.
The previous approach of using the "last provider request timestamp"
does not make sense in the new setup. Each currency rate is based on
rates from several providers, each with their own "request timestamps".
In addition, the pricenode returns the timestamp each rate was
calculated. On top of that comes the timestamp when the Bisq node
queries the pricenode.
Since what is most relevant for the Bisq node is the "freshness" of a
specific rate, the timestamp most indicative of that is the moment when
the pricenode is queried.
Update the displayed text, as well as the tooltip, of the price box in
the top right bar. It now indicates that the price data is provided by
Bisq pricenodes (for for fiat, as well as for alts).
When an offer is made using BSQ for trade fee, the BSQ amount
is burnt by doing a send-to-self. However if the BSQ change
is below the bitcoin dust limit this causes an error. We
fix this by maintaining a floor amount of 5.46 BSQ.
Fixes#4372
Display a mandatory warning when the seed backup/restore
screen is shown. The warning informs the user that a data
directory backup is necessary to restore the full state of
bisq, not just the wallet seed words.
Fixes#4375
Main part missing is the XMR proof service request processing. I did not
get the service compiled yet, so could not test response data and error
conditions.
Further it is missing a "news badge" and popup to guide the user to the
new feature.
Only basic dev tested so far.
Anyone welcome to pick the project up from here as I might not have
time soon to continue.
If the local Bitcoin full node is bound to only listen on the loopback interface (127.0.0.1), attempting to open a socket to `InetAddress.getLocalHost()` - the return of which is variable but usually NOT 127.0.0.1 - will not work. Changing to `InetAddress.getLoopbackAddress()` resolves this.
Issue: if a taker used exactly all BSQ from the BSQ inputs to pay the
trading fee, there was no BSQ change in the takeOfferFeeTx. It was
assumed that the second output was the reservedForTrade output, but in
the case of missing BSQ change it was the first output.
Fix: added a check to make sure the value of the inputs to the deposit
tx match the expected inputAmount.
Added a check that if there is no BSQ outputs in the bsqTradingFeeTx a
change output is added of value 1 satoshi more than the BSQ input value.
This ensures that the second output is always the reservedForTrade
output. It also ensures that the BSQ is burnt, even in the very unlikely
case that the amount of BSQ burnt is larger than the reservedForTrade
amount.
This fixes an issue whereby updates from the fee API were causing
Insufficient Funds exception. Since the wallet is funded with a
specific amount (deposit+trade fee+txFee), that same amount has
to be used when the user confirms offer creation (which could be
some time later).
Fixes#4227Fixes#4278Fixes#4336Fixes#4327
Respect the direction parmeter; do not give it meaning it does not
have. If the user passes a 'buy' parameter, return buy offers. Do
not misinterpret the param's intent. The direction parameter's value
does not imply "buy=I'm a buyer, show me sell offers" or
"sell=I'm a seller, show me buy offers".
I got mixed up by looking at the UI. If I want to sell BTC, I click
the SELL tab to view buy offers (maker as buyer). If I want to buy
BTC, I click the BUY tab to view sell offers (maker as seller).
This change also fixes an offer list sorting bug.
The commit is in response to a requested changes in PR 4329:
https://github.com/bisq-network/bisq/pull/4329#pullrequestreview-436033502
This change adds a new 'verifyWalletsAreAvailable' method to the client,
which eliminates this duplicated statement:
throw new IllegalStateException("wallet is not yet available");
The commit is in response to a requested change in PR 4312:
https://github.com/bisq-network/bisq/pull/4312#pullrequestreview-435659314
This commit is for a change requested in PR 4308:
https://github.com/bisq-network/bisq/pull/4308#pullrequestreview-435055483
".toUpperCase() seems misplaced here. It would soon get repetive.
Whether the underlying logic differentiates between capitalizations
is a low-level implementation detail and would do better at the
lowest practical level."
The new method returns current buy or sell offers for a fiat ccy.
These changes need refactoring and polishing before merging, but they're
committed in this state to be safe (don't lose work). Changes include:
* New core.grpc classes
CoreOffersService
GrpcOffersService
model.OfferInfo
* CoreApi -- The new CoreOffersService is injected into CoreApi and
the old getOffers() and placeOffer() impls were moved into the
new CoreOffersService. The getOffers implementation was re-done.
Other changes are just rearranging location of core method calls.
* GrpcServer -- The new GrpcOffersService replaced the old
GetOffersService and PlaceOfferService.
* grpc.proto -- The old GetOffers and PlaceOffer services were combined
into a single Offers service, and the PlaceOffer rpc was renamed
as CreateOffer. These are the only substantive changes; the rest
is just rearranging location of the service defs in the file.
Also created a lighterweight OfferInfo proto message wrapper to
be passed between server & client (client has no access to core's
Offer and OfferPayload).
* OfferInfo -- A new wrapper around the OfferInfo proto message.
* CliMain -- The new GetOffers service stub was added.
Some (maybe too much) number and ccy formatting logic was
copied & modified from core. Some tedius string formatting
was added too (needs to be tidied up).
* License comments were also copied to several classes, and I
made a mistake in reverting changes to the wrong file.
TODO add unit tests
Remove the recently added gRPC StatusRuntimeException wrapping
logic because we want unexpected Exceptions to bubble up for now,
until CorePaymentAccountsService.java throws specific
IllegalStateExceptions with user friendly error messages.
(See CoreWalletsService.java for example.)
This reverts commit bfcc693f69.
This change was reverted because we want unexpected Exceptions
to bubble up for now, until CorePaymentAccountsService.java
throws specific IllegalStateExceptions with user friendly
error messages. (See CoreWalletsService.java for example.)
The 'getaddressbalance' and 'getfundingaddresses' methods now send
new AddressBalanceInfo proto messages instead of a formatted String
to the client. The AddressBalanceInfo message contains addressString,
balance, and # of confirmations (transaction confidence) fields.
Changes include:
* A new AddressBalanceInfo proto message
* A wrapper class for the new AddressBalanceInfo proto
* New 'getaddressbalance' and 'getfundingaddresses' signatures in server
* AddressBalanceInfo display logic in client
* Removal of balance formatting logic in server
* Refactoring of balance formatting logic in client
This change is a refactoring of the gRPC Wallets service
for the purpose of making CoreApi the entry point to
all core implementations. These changes should have been
made in PR 4295.
See https://github.com/bisq-network/bisq/pull/4295
The gRPC Wallet proto def name was changed to Wallets because
this service manages BSQ and BTC wallets, and GrpcWalletService
was changed to GrpcWalletsService for the same reason.
This PR should be reviewed/merged after PR 4308.
See https://github.com/bisq-network/bisq/pull/4308
This PR's branch was created from the PR 4308 branch.
This addresses task 4 in issue 4257.
https://github.com/bisq-network/bisq/issues/4257
This PR should be reviewed/merged after PR 4304.
https://github.com/bisq-network/bisq/pull/4304
This new gRPC PaymentAccounts service method creates a dummy
PerfectMoney payment account for the given name, number and fiat
currency code, as part of the required "simplest possible trading
API" (for demo). An implementation supporting all payment
methods is not in the scope.
Changes specific to the new rpc method implementation follow:
* New createpaymentacct method + help text was added to CliMain.
Help text formatting was also changed to make room for larger
method names and argument lists.
* The PaymentAccount proto service def was renamed PaymentAccounts
to avoid a name collision, and the new rpc CreatePaymentAccount
was made part of the newly named PaymentAccounts service def.
* New GrpcPaymentAccountsService (gRPC boilerplate) and
CorePaymentAccountsService (method implementations) classes were
added.
* The gRPC GetPaymentAccountsService stub was moved from GrpcServer
to the new GrpcPaymentAccountsService class, and
GrpcPaymentAccountsService is injected into GrpcServer.
* A new createpaymentacct unit test was added to the bats test
suite (checks for successful return status code).
Maybe bit out of scope, some small changes were made towards making
sure the entire API is defined in CoreApi, which is used as a
pass-through object to the new CorePaymentAccountsService. In the
next PR, similar refactoring will be done to make CoreApi the
pass-through object for all of the existing CoreWalletsService
methods. (CoreWalletsService will be injected into CoreApi.)
In the future, all Grpc*Service implementations will call core
services through CoreApi, for the sake of consistency.
If the user has an account that can sign, it signs other accounts with
the same holder name when they are added, or on startup if one of
several accounts with the same name got signer rights.
This addresses task 2 in issue 4257
https://github.com/bisq-network/bisq/issues/4257
This new gRPC Wallet service method displays the balance and number
of confimirmations of the most recent transaction for the given BTC
wallet address.
The new method required the usual boilerplate changes to grpc.proto,
CliMain, and GrpcWalletService.
Two unit tests to check error msgs was added to cli/test.sh.
As part of the Tor V3 upgrade, and the effort to increase the number of
Bisq seednodes from 8 to 12, this PR adds 2 new V3 seednodes, and will
start the process to retire the old V2 seednode ef5qnzx6znifo3df.
The ef5qnzx6znifo3df seednode will continue operating for 2-3 months
during the retirement phase-out period, after which time we can filter
it out from the network for clients who have not upgraded if necessary.
This addresses task #1 in issue https://github.com/bisq-network/bisq/issues/4257.
This new gRPC WalletService method displays the BTC wallet's list of
receiving addresses. The balance and number of confirmations
for the most recent transaction is displayed to the right of each
address. Instead of returning a gRPC data structure to the client,
the service method returns a formatted String.
If the BTC wallet has no unused addresses, one will be created and
included in the returned list, and it can be used to fund the wallet.
The new method required injection of the BtcWalletService into CoreWalletsService,
and the usual boilerplate changes to grpc.proto, CliMain, and GrpcWalletService.
Some of the next PRs (for #4257) will require some common functionality within
CoreWalletsService, so these additional changes were included:
* a private, class level formatSatoshis function
* a public getNumConfirmationsForMostRecentTransaction method
* a public getAddressBalance method
* a private getAddressEntry method
A unit test that verifies a successful return status was added to cli/test.sh.
When the user restores a BIP39 wallet from seed words there is
an optional 'wallet date' field. If the user does not enter that
field the chain scan will be a lengthy process.
Issue a popup message to the user in this case, informing them
that the synch will take a long time and can be reduced by either
specifying the wallet creation date or the date when Bisq introduced
BIP39 seed words.
Fixes#3766
- "Memo" field is modeled as property of the new object Transaction which is stored in persitant storage.
- Transaction object is modeled in a way that allows extension in the furure for more persisted attributes.
The routine `cleanUpAddressEntries` in TradeManager was prematurely
releasing funds associated with trades when they reach the last
step of the process (Step 4, Withdraw to External Wallet).
Usually, with a single trade active this would not be an issue
because `cleanUpAddressEntries` is called after the withdrawal,
but if you have more than one trade at Step 4 then the first
withdrawal would go though, but all the others would fail with
an error box "Missing x.xxx BTC" (the trade proceeds or deposit
amount). Alternatively, if you restart the bisq app with a
trade in Step 4, the same cleanup will occur and the attempt
to withdraw to external wallet will fail.
The change here considers any trade held by the TradeManager to
be in use and therefore will not have their associated address
entries freed up. After Step 4 has passed, the trade is no
longer held by the TradeManager, and so cleanUpAddressEntries
will at that point free up the address.
The CoreWalletService should never be running more than one wallet lock
TimerTask at any given time, and the wallet should lock at the correct time
after a user overrides a prior timeout value.
The CoreWalletService now cancels any existing lock TimerTask thread
before creating a new one, to prevent the wallet from re-locking at
unexpected times.
This change prevents error conditions created in scenarios similar to
the following:
(1) User unlocks wallet for 60s
(2) User overrides unlock timeout by calling unlockwallet "pwd" 600s
(3) After 60s, the 1st, uncanceled lock timeout expires, and wallet is
locked 4 minutes too soon.
When exporting trade history to CSV, users would expect to see
their own transaction and trade fees listed. Maker fees are
stored on the Offer, Taker fees are stored on the Trade object.
The code was not properly checking whether the user was the
Offer Maker or Taker, and as such the report was always displaying
the Taker information for Tx and Trade fee. Therefore any trades
where the user was Maker showed the wrong fees.
The column title `Maker Fee` is incorrect and misleading since
the user doing the export is not necessarily the Maker. We
rename this column to `Trade Fee` to indicate that it is the fee
the user paid for trading regardless of Maker/Taker status.
Fixes#4207
Manfred [0] pointed out that COMPENSATION_REQUEST_MIN_AMOUNT and
REIMBURSEMENT_MIN_AMOUNT should be checked for not exceeding 200k BSQ
too.
[0] 4ec6bac658..fb76fd6548 (r420249962)
Without the break statement, the execution would continue through the subsequent case clauses until it encountered a break, executing `checkArgument` calls meant for `REIMBURSEMENT_MAX_AMOUNT`. More specifically, the bug would cause a failed check in the case where `inputValueAsCoin.value <= 200000000` is false.
There is no need to decrypt and re-encrypt wallet files in the unlock
and lock wallet methods. The temporary aesKey saved in the unlock
method will used for operations on an encrypted wallet.
There is also no need to cache a tempKeyCrypterScrypt; this class
level variable was removed.
Cache the aesKey instead of the password in unlock wallet method,
avoiding extra overhead of deriving it from the temp password in the
lock method.
The KeyCrypterScrypt used in the unlock method must also be cached for
use in the manual lock method. Creating a new KeyCrypterScrypt instance
in the manual lock method would have a different random salt value,
invalidating the password used in the unlock method.
Without bugfix, method would try to encrypt the wallet using using the
old password argument immediately after encrypting the wallet with
a new password.
Like the change in commits 163061a and 9164579, removeWalletPassword
now throws an IllegalStateException with an appropriate message if
the wallet password cannot be removed.
Also deletes unused StatusApi enums.
Like the change in commit 163061a, setWalletPassword now throws an
IllegalStateException with an appropriate message if the wallet password
cannot be set.
Also deletes unused StatusApi enums.
And throw an IllegalStateException with an appropriate message if the
wallet is not available or still locked.
This change also eliminates the NullPointerException that would
sometimes be thrown when calling #getAvailableBalance after the wallet
has become available but before the balance has become available.
Previously, each wallet-related method was implemented with its own grpc
service. There is no need to do this, as a grpc service may declare
multiple rpc methods. This commit refactors everything wallet-related
into a single GrpcWalletService and also extracts a CoreWalletService
from CoreApi in order to avoid the latter becoming overly large.
Ideally, there would be no need for an abstraction in bisq.grpc called
CoreWalletService; we would ideally use such a service implemented in
bisq.core. The closest we have is WalletsManager, but it is not designed
to be used the way we are using it here in the grpc context. Rather than
making changes directly to core (which can be very risky), we will
rather make them here in this layer, designing exactly the "core wallet
service" we need, and can then later see about folding it into the
actual core.
This change removes non-idiomatic gRPC *Reply proto message fields.
The client should not receive success/fail values from server methods
with a void return type, nor an optional error_message from any server
method. This change improves error handling by wrapping an appropriate
gRPC Status with a meaningful error description in a StatusRuntimeException,
and placing it in the server's response StreamObserver. User error
messages are mapped to general purpose gRPC Status codes in a new ApiStatus
enum class. (Maybe ApiStatus should be renamed to CoreApiStatus.)
Add a missing bsqWalletService.isEncrypted() check and optional AES key
argument to the signMessage(..) call in ProofOfBurnService, analogous to
the BitcoinJ call to sign with an EC key in MyBlindVoteListService.
This fixes#3836.
Implemented lockwallet, unlockwallet, removewalletpassword, and
setwalletpassword methods with basic error handling.
Also added basic error handling to existing getbalance method,
and removed unused BalancePresentation from CoreAPI.
TODO: update help text
Without the break statement, the execution would continue through the subsequent case clauses until it encountered a break, executing `checkArgument` calls meant for `REIMBURSEMENT_MAX_AMOUNT`. More specifically, the bug would cause a failed check in the case where `inputValueAsCoin.value <= 200000000` is false.
This omission is currently not disruptive, since the next case clause only contains a break. Also converted a few `return false` to breaks, to align with the rest of the switch statement.
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.
There is no actual requirement for this method, so removing it. It also
helps improve the implementation by removing the need for the static
'instance' field.
This is a newly deployed instance intended on replacing one of emzy's
nodes in an attempt to reduce the number of nodes under the control of a
single contributor.
There is actually no use case for both username and password forming the
authentication token. This change simplifies the arrangement such that a
single token is passed.
--rpcUser and --rpcPassword are no longer used and are replaced by
--apiToken on both cli and daemon sides.
Final fields should be favored wherever possible to advertise and
enforce immutability. Also as a matter of style, favor declaring fields
one per line instead of multiple on the same line. This style guideline
is not consistent throughout the codebase, but is favored because it
makes diffs / patches cleaner and is the more widely accepted style
througout most modern Java codebases.
* Renamed CliCommand to RpcCommand, differentiating it from cmd
string token(s)
* Added new CliConfig, based on :common Config
* Injected Config into CoreApi to make bisq.properties and cmd line
args available to server
* Added cleartext username:password BisqCallCredentials to :cli, and
an AuthenticationInterceptor to :core (server)
* Moved :daemon resources folder to expected location under src/main
* Duplicated CompositeOptionSet, ConfigException and BisqException
in :cli because they are not visible from :common. (TODO refactor?)
CompositeOptionSet will be used to let command line parameters
override params set in config file.
* Removed outdated references to :cli in a couple of comments in
:common Config
* gRPC parameters & command names are lowercase to mimic bitcoind/rpc
TBD
* Decide what rpc auth param names to use, to differentiate them from
bitcoind rpc user/passord param names
Fixes#3709. The issuance log message displayed the issued BSQ
with 2 extra zeros (i.e. sats value). This was because BSQ is
represented internally within the application as sats, so it has to
be converted by `MathUtils.scaleDownByPowerOf10(amount, 2)`
whenever displayed to the user.
Test AccountAgeWitnessServiceTest > testArbitratorSignWitness failed
in full gradle build, but passed when run as a single test:
./gradlew :core:cleanTest :core:test \
--tests "bisq.core.account.witness.AccountAgeWitnessServiceTest"
This test also passed when run in the IDE.
Solved by not passing a mocked PaymentMethod.SEPA argument into the
test's service.getTraderPaymentAccounts() method, where mock paymentId
field was null when running full build's test suites.
Fix for #4158
Test AccountAgeWitnessServiceTest > testArbitratorSignWitness failed
in full gradle build, but passed when run as a single test:
./gradlew :core:cleanTest :core:test \
--tests "bisq.core.account.witness.AccountAgeWitnessServiceTest"
This test also passed when run in the IDE.
Solved by not passing a mocked PaymentMethod.SEPA argument into the
test's service.getTraderPaymentAccounts() method, where mock paymentId
field was null when running full build's test suites.
Fix for #4158
The former class is dead code, together with its store service, as they
were only referenced from CorePersistenceProtoResolver::fromProto, the
binding logic and from AppendOnlyDataStoreService by orphaned migration
code. However, migration from the old persisted data was completed long
ago and the store file is no longer being read or written from anywhere
in the codebase.
Also remove the associated PersistableEnvelope proto message type, along
with the TradeStatisticsList message type. The latter is long deprecated
and has no corresponding Java class implementing PersistableEnvelope, so
removing it won't change behaviour (outside the exception message thrown
when attempting to resolve it).
* Add signed witness filter
- Add a filter to pubkeys used in AccountAgeWitness signing
- Fix inverted arbitrator signing of initial account age witnesses from
disputes
- Add test to verify that signed witness filter works
- Add test to verify that the arbitrator signing was fixed
* Fix codacy complaints
* Prevent NullPointerException during toggle group initialization
* Add scrollbar to filter window
* Format test class
Co-authored-by: Christoph Atteneder <christoph.atteneder@gmail.com>
The moment we are validating the tx is not committed and therefore the
connected tx output is not wired up and thus null.
Refactor and consolidate the validation code to the static class and
improve error handling.
The moment we are validating the tx is not committed and therefore the
connected tx output is not wired up and thus null.
Refactor and consolidate the validation code to the static class and
improve error handling.
Buyer was not verifying if the donation address is either the default
DAO param address or the recent one changed by DAO voting. We do not
support past DAO param addresses to avoid that past addresses receive
funds.
Buyer was not verifying if the donation address is either the default
DAO param address or the recent one changed by DAO voting. We do not
support past DAO param addresses to avoid that past addresses receive
funds.
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.
Showing an `Open Trader Chat` button until the trade period is over will
reduce the number of unnecessary support tickets and encourage
trader-to-trader dialogue. If the trade timer expires without
completing the button changes (as before) to open a mediation ticket.
Implementation of this feature requires the button in TradeStepView to
notify its parent TradeSubView which in turn notify its parent
PendingTradesView that trader chat is to be opened. Basically a
callback interface is passed two levels down the GUI hierarchy.
Fixes#3801
* Report HS version to pricenode
In order to evaluate progress on https://github.com/bisq-network/projects/issues/23,
the Bisq app reports its hiddenservice version.
This change is going to be undone as soon as we do not need the
info anymore.
* Added hsversion scraper script
* Added installer/uninstaller
* Cleanup
* Fix unit name
