This is causing erroneous PRICE_OUT_OF_TOLERANCE errors when trying
to take offers having (fixed) price!=0, and isUsingMktPriceMargin=true
in the payload.
The API daemon editoffer's treatment of (fixed) price and
isUsingMktPriceMargin flag in the API has been inconsistent with the UI.
With this change: when isUsingMktPriceMargin=true, (fixed) price is
set to 0 on the server. API clients, however, still must show the
calculated price when isUsingMktPriceMargin=true, making this fix hard
to test in the client. The server will now throw an exception if
(fixed) price and isUsingMktPriceMargin flag in the API are not
properly set in the API server.
This fix is intended to prevent issues such as
https://github.com/bisq-network/bisq/issues/6170 from happening for
this reason. The offer maker edited offers with API, creating
inconsistent state described above. It is hoped the user's offers
can be fixed by editing them in the UI.
Based on `master`.
Price change events were being fired too frequently and for ccys
which had not changed.
This caused UI freezing issues most noticably in the DAO dashboard.
Non-CLI clients need a better way of accessing payment details
than a contract json string.
- Add grpc.proto PaymentAccountPayloadInfo field: payment_details.
- Adjust proto wrapper PaymentAccountPayloadInfo to new field.
- Add test asserts to verify payment details are sent to client.
- Fix a test name: testKeepFunds -> testCloseTrade.
Based on branch `master` @ Sat 12 Mar 2022 01:42 PM -03 ,
commit c6293b5273
This recently added field is automatically set inside PaymentAccount.
It should not be included in the payment acct json forms used by API clients.
Based on branch `master`.
I think this bug was introduced when deprecating GrpcOffersService
.getMyOffer(id), in favor of using only getOffer(id) for 'my'
and 'available' offers. This change explicitly sets the proto's
isActivated flag in the OfferInfo factory methods, and adds checks
to api offer test cases.
Based on branch `2-improve-grpc-exception-status-code-mapping`,
PR https://github.com/bisq-network/bisq/pull/6088
The resync popup was being shown when flag isInConflictWithSeedNode
was set, but there is also a different relevant flag
isDaoStateBlockChainNotConnecting which can happen in certain
situations. For that case we also need to show the popup.
Exceptions thrown by the core.api services for the daemon Grpc*Services
have to be converted into gRPC StatusRuntimeExceptions before being sent to
gRPC clients. Most of these gRPC StatusRuntimeExceptions had a gRPC
Status.Code.UNKNOWN, which not helpful to client error handlers.
This change partially resolves the issue by sending more meaningful
io.grpc.Status.Codes to clients, where possible. But it is not as
comprehensive as it an be for a webapp because HTTP has so many more
possible response status codes than the gRPC library (sixteen). See:
https://github.com/grpc/grpc-java/blob/master/api/src/main/java/io/grpc/Status.java
There are three types of changes:
- Create custom exceptions in bisq.core.api.exception.
- Map any custom bisq.core.api.exception to a meaningful
io.grpc.Status.Code within daemon Grpc*Service classes.
- Adjust apitest cases to new grpc status codes.
Based on branch `move-cli-crypto-offer-filter-to-server`, PR https://github.com/bisq-network/bisq/pull/6086
The resync popup was being shown when flag isInConflictWithSeedNode
was set, but there is also a different relevant flag
isDaoStateBlockChainNotConnecting which can happen in certain
situations. For that case we also need to show the popup.
Some API reference & Python bot examples exposed an API bug the Java CLI
has been hiding. Due due this Bisq v1 Offer entity design:
- In fiat offers, the baseCurrencyCode=BTC, counterCurrencyCode=FiatCode
- In altcoin offers, baseCurrencyCode=AltcoinCode, counterCurrencyCode=BTC,
new API examples were not doing what the CLI has been doing for several
months, which is get (cryptocurrency) all BTC offers from the server,
and filter on the altcoin code. The CLI side filtering should have been
done on the server, as it is in this commit.
A lot of dead gRPC client side offer filtering code is removed as well.
Based on `master`.
The client may have passed an empty string for the price parameter,
if only enabling or disabling the offer. If so, validate with new
price = old price.
The CLI and apitest cases always pass "0", but java & python clients
might pass an empty string. This change avoids number formatting
& scaling problems when clients pass an empty string in the price
parameter.
This protobuf definition and service stub has been in place since
the start of work on the API, but was never fully implmented, nor
intended to be included in the API beta & v1 releases.
Its presence added a useless section to the gRPC API Reference doc.
https://ghubstan.github.io/slate
Based on branch `7-more-grpcproto-comments`,
PR https://github.com/bisq-network/bisq/pull/6068.
Was getting an NPE with closed trades older than v1.7.0.
makerPaymentMethodId was new at that point (null for older trades).
makerPaymentAccountPayload can be null for trades where we have
removed account info for privacy reasons (see #6001).
The payment method ID can always be obtained from offerPayload.
Was getting an NPE with closed trades older than v1.7.0.
makerPaymentMethodId was new at that point (null for older trades).
makerPaymentAccountPayload can be null for trades where we have
removed account info for privacy reasons (see #6001).
The payment method ID can always be obtained from offerPayload.
The BisqDefaultCoinSelector is invoked multiple times when a
transaction is being built. Problem was that the coin selection
was being reset within, ultimately leading to coin selection
being completely ignored. Solution is to reset the selection
after the transaction has been built.
The UI for selecting UTXO inputs was being resized to the number
of UTXO available, which caused problems when there is a large
UTXO set. Solution is to size the UI to accomodate 3 to 15 rows,
which provides a reasonable screen display and a scrollbar is
provided for the case of a large UTXO set.
Except `dao.bond.bondedRoleType.ROCKET_CHAT_ADMIN` which refers to an
unused role, every other instance of `keybase` has been contextually
updated to Matrix (bisq.chat); functions to display moderator name
and profile address on keybase have been updated to work with Matrix
usernames and urls.
Kept translated resources in original state as per jmacxx suggestion.
The new version 2.3.0 of mempool.space supports BSQ exporer only on a separate domain.
update URL to http://bisq.mempool.emzy.de/....
There is a redirect from the old location to the new one.
So nothing is broken and the PR is low prio.
Nequi payment method did not include that there might be a penalty for exceeding the transfer limits. I have used the same wording for every payment method.
Prerequisite for next PR: Add API method 'gettrades'
The `gettrades` method will show 'open', 'closed', and 'failed' trades.
Users already needed to be able to fail and unfail trades for the
same reasons they do in the UI. API test cases will need to be able to
fail and unfail trades to check correct behavior of 'gettrades' method.
Based on branch `rename-keepfunds2closetrade`.
Trade proceeds and deposits have already been transfered to Bisq wallets
before the `keepfunds` command is (was) executed; `keepfunds` merely moves
open trades to closed trades lists and persistence files. Renaming `keepfunds`
as `closetrade` makes its purpose clear to API users.
The commit modifies only method names and comments in api server+cli classes,
apitest cases, and api trade simulation scripts.
Based on `master`
A test has been added to validate the UTXO issue above.
Other tests have been updated as a result of the changes in PR 5826
where fee parameter is now supplied from a filter; this results
in more accurate (less lenient) fee checking.
The method 'getoffer' should support looking up a user's open-offers, and other users' available offers.
- Adjust api-beta-test-guide.md to use only 'getoffer'.
- Adjust trade simulation scripts to use only 'getoffer'.
- Adjust api testcases to use 'getoffer' in place of 'getmyoffer'.
- Mark appropriate methods and protobuf msgs as deprecated.
is expected as we shut down the executor immediately.
If not thrown at shutdown or exception is not a RejectedExecutionException
we log a warning and re-throw the exception (to avoid change of behaviour
of current version). The exception is likely not handled by callers and goes up to
the uncaught exception handler.
Prevent a "URI is not hierarchical" IllegalArgumentException from the
expression, 'new File(dirUrl.toURI())', which occurs on Linux & Windows
when listing the resource directory of BSQ blocks. Adapt a solution from
StackOverflow which uses two separate code paths depending on the
environment, into the new method 'FileUtil::listResourceDirectory'.
The issue is caused by the resource URL taking one of the two forms:
file:/Users/[USER]/Java/bisq/bisq/p2p/out/production/resources/BsqBlocks_BTC_MAINNET
jar:file:...p2p.jar!/BsqBlocks_BTC_MAINNET
depending on whether the system is OSX or not.
Move timeout before shutdown sequence starts and use a Timer thread instead of
UserThread to avoid that in case the UserThread gets blocked that the timeout
would not get triggered.
Reduce timeout from 20 sec. to 10 sec.
Move timeout before shutdown sequence starts and use a Timer thread instead of
UserThread to avoid that in case the UserThread gets blocked that the timeout
would not get triggered.
Reduce timeout from 20 sec. to 10 sec.
is expected as we shut down the executor immediately.
If not thrown at shutdown or exception is not a RejectedExecutionException
we log a warning and re-throw the exception (to avoid change of behaviour
of current version). The exception is likely not handled by callers and goes up to
the uncaught exception handler.
Prevent a "URI is not hierarchical" IllegalArgumentException from the
expression, 'new File(dirUrl.toURI())', which occurs on Linux & Windows
when listing the resource directory of BSQ blocks. Adapt a solution from
StackOverflow which uses two separate code paths depending on the
environment, into the new method 'FileUtil::listResourceDirectory'.
The issue is caused by the resource URL taking one of the two forms:
file:/Users/[USER]/Java/bisq/bisq/p2p/out/production/resources/BsqBlocks_BTC_MAINNET
jar:file:...p2p.jar!/BsqBlocks_BTC_MAINNET
depending on whether the system is OSX or not.
1. Reorder the PoW fields in the 'Filter' proto by field index, instead
of contextually.
2. Deduplicate expression for 'pow' & replace if-block with boolean op
to simplify 'FilterManager::isProofOfWorkValid'.
3. Avoid slightly confusing use of null char as a separator to prevent
hashing collisions in 'EquihashProofOfWorkService::getChallenge'. Use
comma separator and escape the 'itemId' & 'ownerId' arguments instead.
(based on PR #5858 review comments)
Remove all the 'challengeValidation', 'difficultyValidation' and
'testDifficulty' BiPredicate method params from 'HashCashService' &
'ProofOfWorkService', to simplify the API. These were originally
included to aid testing, but turned out to be unnecessary.
Patches committed on behalf of @chimp1984.
Change the type of the 'difficulty' field in the Filter & ProofOfWork
proto objects from int32/bytes to double and make it use a linear scale,
in place of the original logarithmic scale which counts the (effective)
number of required zeros.
This allows fine-grained difficulty control for Equihash, though for
Hashcash it simply rounds up to the nearest power of 2 internally.
NOTE: This is a breaking change to PoW & filter serialisation (unlike
the earlier PR commits), as the proto field version nums aren't updated.
Add an abstract base class, 'ProofOfWorkService', for the existing PoW
implementation 'HashCashService' and a new 'EquihashProofOfWorkService'
PoW implementation based on Equihash-90-5 (which has 72 byte solutions &
5-10 MB peak memory usage). Since the current 'ProofOfWork' protobuf
object only provides a 64-bit counter field to hold the puzzle solution
(as that is all Hashcash requires), repurpose the 'payload' field to
hold the Equihash puzzle solution bytes, with the 'challenge' field
equal to the puzzle seed: the SHA256 hash of the offerId & makerAddress.
Use a difficulty scale factor of 3e-5 (derived from benchmarking) to try
to make the average Hashcash & Equihash puzzle solution times roughly
equal for any given log-difficulty/numLeadingZeros integer chosen in the
filter.
NOTE: An empty enabled-version-list in the filter defaults to Hashcash
(= version 0) only. The new Equihash-90-5 PoW scheme is version 1.
Add a numeric version field to the 'ProofOfWork' protobuf object, along
with a list of allowed version numbers, 'enabled_pow_versions', to the
filter. The versions are taken to be in order of preference from most to
least preferred when creating a PoW, with an empty list signifying use
of the default algorithm only (that is, version 0: Hashcash).
An explicit list is used instead of an upper & lower version bound, in
case a new PoW algorithm (or changed algorithm params) turns out to
provide worse resistance than an earlier version.
(The fields are unused for now, to be enabled in a later commit.)
Replace 'BiFunction<T, U, Boolean>' with the primitive specialisation
'BiPredicate<T, U>' in HashCashService & FilterManager.
As part of this, replace similar predicate constructs found elsewhere.
NOTE: This touches the DAO packages (trivially @ VoteResultService).
1. Reorder the PoW fields in the 'Filter' proto by field index, instead
of contextually.
2. Deduplicate expression for 'pow' & replace if-block with boolean op
to simplify 'FilterManager::isProofOfWorkValid'.
3. Avoid slightly confusing use of null char as a separator to prevent
hashing collisions in 'EquihashProofOfWorkService::getChallenge'. Use
comma separator and escape the 'itemId' & 'ownerId' arguments instead.
(based on PR #5858 review comments)
When doing a resync from genesis the number of blocks is limited to 6000
so that requires lots of requests and with that increases risk of broken
connections. Giving more tolerance for retries avoids that the user has
to restart the app.
When syncing from genesis the number of blocks are limited so we get the
`onParseBlockCompleteAfterBatchProcessing` called each time when the received
blocks are processed, and as we are not at wallet height we repeat requesting
blocks. But the new check for the BTC recipient triggers a resync from resource call.
We add now a check that we do this check only once the wallet is synced and our
block height from dao state matches wallet blockheight.
- At genesis we use the genesis height for request (not height+1)
- If wallet is not synced yet we do not call onParseBlockChainComplete (as it was before)
We added 1 as with the lite monitor mode we persist the most recent block,
thus we request with the start height for the next block.
But that cause a problem at a DAO full mode which has lite monitor mode set
as then the block parsing would not be triggered.
We refactor it so that we take the chainHeight from the dao state
directly and add 1 at the requests.
We add a check if we are at chain tip, and if so we skip requests
and call the onParseBlockChainComplete directly.
Add a check of 'scriptTypeId' field, against the output of the spending
tx, to the 'RawTransactionInput::validate' method. Also make the seller
as well as the buyer validate each raw BSQ/BTC input received from the
peer. This prevents either peer from claiming that any of their
non-segwit inputs are segwit in order to underpay the tx fee.
Prevent the seller from stealing the combined tx fee as change by lying
about the value of one or more of his BTC inputs, which are passed to
the buyer as raw inputs in the 'BsqSwapFinalizeTxRequest' message.
To this end, add a 'RawTransactionInput::validate' method to check the
'value' field against the output value of the respective spending tx and
run it on every seller input in 'ProcessBsqSwapFinalizeTxRequest', so
that the buyer is no longer just trusting those numbers.
Additionally, check that the spending txIds from the raw BTC inputs
supplied by the seller actually match those of his signed inputs in the
accompanying partially signed tx, thus tying the raw input values to the
seller's tx.
When doing a resync from genesis the number of blocks is limited to 6000
so that requires lots of requests and with that increases risk of broken
connections. Giving more tolerance for retries avoids that the user has
to restart the app.
When syncing from genesis the number of blocks are limited so we get the
`onParseBlockCompleteAfterBatchProcessing` called each time when the received
blocks are processed, and as we are not at wallet height we repeat requesting
blocks. But the new check for the BTC recipient triggers a resync from resource call.
We add now a check that we do this check only once the wallet is synced and our
block height from dao state matches wallet blockheight.
- At genesis we use the genesis height for request (not height+1)
- If wallet is not synced yet we do not call onParseBlockChainComplete (as it was before)
Add a check of 'scriptTypeId' field, against the output of the spending
tx, to the 'RawTransactionInput::validate' method. Also make the seller
as well as the buyer validate each raw BSQ/BTC input received from the
peer. This prevents either peer from claiming that any of their
non-segwit inputs are segwit in order to underpay the tx fee.
The editoffer validation bug fixes:
- A trigger-price edit forced offer.price-margin=0.00.
This needs to be checked in new apitest case asserts.
- An activate state (only) edit forced offer.isUseMarketBasedPrice=true.
The CLI does not have the offer instance, and cannot know the correct
value of the isUseMarketBasedPrice param sent in the editoffer request.
The daemon has to figure this out. If the editType parameter value
sent to daemon is ACTIVATION_STATE_ONLY, use the current offer.isUseMarketBasedPrice.
The refactoring includes more useful and readable information in core's EditOfferValidator
and MutableOfferPayloadFields toString methods, for debugging with the daemon log. And some
adjustments for allowing edits to XMR offers.
Prevent the seller from stealing the combined tx fee as change by lying
about the value of one or more of his BTC inputs, which are passed to
the buyer as raw inputs in the 'BsqSwapFinalizeTxRequest' message.
To this end, add a 'RawTransactionInput::validate' method to check the
'value' field against the output value of the respective spending tx and
run it on every seller input in 'ProcessBsqSwapFinalizeTxRequest', so
that the buyer is no longer just trusting those numbers.
Additionally, check that the spending txIds from the raw BTC inputs
supplied by the seller actually match those of his signed inputs in the
accompanying partially signed tx, thus tying the raw input values to the
seller's tx.
**Unconfirmed** BSQ swap seems like something failed. **Processing** is used by some wallets for unconfirmed transactions and has no negative implications.
We added 1 as with the lite monitor mode we persist the most recent block,
thus we request with the start height for the next block.
But that cause a problem at a DAO full mode which has lite monitor mode set
as then the block parsing would not be triggered.
We refactor it so that we take the chainHeight from the dao state
directly and add 1 at the requests.
We add a check if we are at chain tip, and if so we skip requests
and call the onParseBlockChainComplete directly.
There are some use cases where the CLI needs to know what kind of offer
is being acted on before the request is made, For example:
There are differences between a BsqSwap 'takeoffer'request, and a v1
'takeoffer' request.
A BsqSwap offer cannot be edited by an 'editoffer' request, and an
attempt should be blocked by the CLI.
- Append isMyOffer GetOfferCategoryRequest rpc msg def.
- Adjust daemon.grpc services for new boolean GetOfferCategoryRequest param.
- Adjust core.api for new boolean GetOfferCategoryRequest param.
- Add validation check in core.api EditOfferValidator to block attempt to
edit a BsqSwap offer.
- Refactor CoreOffersService get*offer(id) methods to optionally throw
excpetions.
Remove all the 'challengeValidation', 'difficultyValidation' and
'testDifficulty' BiPredicate method params from 'HashCashService' &
'ProofOfWorkService', to simplify the API. These were originally
included to aid testing, but turned out to be unnecessary.
Patches committed on behalf of @chimp1984.
Change the type of the 'difficulty' field in the Filter & ProofOfWork
proto objects from int32/bytes to double and make it use a linear scale,
in place of the original logarithmic scale which counts the (effective)
number of required zeros.
This allows fine-grained difficulty control for Equihash, though for
Hashcash it simply rounds up to the nearest power of 2 internally.
NOTE: This is a breaking change to PoW & filter serialisation (unlike
the earlier PR commits), as the proto field version nums aren't updated.
Add an abstract base class, 'ProofOfWorkService', for the existing PoW
implementation 'HashCashService' and a new 'EquihashProofOfWorkService'
PoW implementation based on Equihash-90-5 (which has 72 byte solutions &
5-10 MB peak memory usage). Since the current 'ProofOfWork' protobuf
object only provides a 64-bit counter field to hold the puzzle solution
(as that is all Hashcash requires), repurpose the 'payload' field to
hold the Equihash puzzle solution bytes, with the 'challenge' field
equal to the puzzle seed: the SHA256 hash of the offerId & makerAddress.
Use a difficulty scale factor of 3e-5 (derived from benchmarking) to try
to make the average Hashcash & Equihash puzzle solution times roughly
equal for any given log-difficulty/numLeadingZeros integer chosen in the
filter.
NOTE: An empty enabled-version-list in the filter defaults to Hashcash
(= version 0) only. The new Equihash-90-5 PoW scheme is version 1.
Add a numeric version field to the 'ProofOfWork' protobuf object, along
with a list of allowed version numbers, 'enabled_pow_versions', to the
filter. The versions are taken to be in order of preference from most to
least preferred when creating a PoW, with an empty list signifying use
of the default algorithm only (that is, version 0: Hashcash).
An explicit list is used instead of an upper & lower version bound, in
case a new PoW algorithm (or changed algorithm params) turns out to
provide worse resistance than an earlier version.
(The fields are unused for now, to be enabled in a later commit.)
PaymentMethod use an instance of TradeLimits and expect that it
has been injected, which is the case for desktop but not for
headless apps, so we enforce injection in the app base classes
used for headless apps.
The validation of trade statistics use a method in PaymentMethod
where that dependency is required.
Tha hack how the PaymentMethod use TradeLimits is not nice, but
would require more effort for refactoring.
We had historically higher trade limits and assets which are not in the
currency list anymore, so we apply the filter only for data after
Nov 1st 2021.
We had historically higher trade limits and assets which are not in the
currency list anymore, so we apply the filter only for data after
Nov 1st 2021.
PaymentMethod use an instance of TradeLimits and expect that it
has been injected, which is the case for desktop but not for
headless apps, so we enforce injection in the app base classes
used for headless apps.
The validation of trade statistics use a method in PaymentMethod
where that dependency is required.
Tha hack how the PaymentMethod use TradeLimits is not nice, but
would require more effort for refactoring.
found in the active list. Otherwise the 2 BTC default is used.
We get TradeStatistics3 objects from old retired PaymentMethods
which are not found in the active list.
found in the active list. Otherwise the 2 BTC default is used.
We get TradeStatistics3 objects from old retired PaymentMethods
which are not found in the active list.
