Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API bug-fix for incorrect BsqSwap's OfferInfo.isActivated value [No. 3] #6094

Merged

Conversation

ghubstan
Copy link
Contributor

@ghubstan ghubstan commented Mar 7, 2022

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 #6088

ghubstan added 7 commits March 3, 2022 16:19
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`.
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 bisq-network#6086
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 bisq-network#6088
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@ripcurlx ripcurlx added this to the v1.8.5 milestone Mar 8, 2022
@ripcurlx ripcurlx merged commit b3b488c into bisq-network:master Mar 8, 2022
@ghubstan ghubstan deleted the 3-fix-is-activated-setting-bug branch March 8, 2022 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants