Add 'takeoffer' API method #4673
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These refactoring changes are for reducing existing and potential duplication coming with the addition of new trading protocol support in the gRPC API. Some minor styling and logic simplification changes are also include. - Convert OfferUtil to injected singleton, and move various offer related utility methods into it. - Delete both MakerFeeProvider classes, which were wrappers around the same static old OfferUtil method. - Inject OfferUtil into CreateOfferDataModel, CreateOfferViewModel, TakeOfferDataModel, TakeOfferViewModel, MutableOfferDataModel, MutableOfferViewModel, OfferDataModel, EditOfferDataModel, EditOfferViewModel - Refactor TakeOfferViewModel Use OfferUtil, remove unused fields & methods. Made minor logic simplification, style and formatting changes. - MutableOfferDataModel Made minor logic simplification, style and formatting changes. - MutableOfferView uses new paymentAccount.isHalCashAccount(). - MutableOfferViewModel Refactored to use new VolumeUtil, CoinUtil, OfferUtil. Removed unused fields & accessors. Made minor style change. - Refactored OfferDataModel to use new OfferUtil - Refactor CreateOfferService Inject and use OfferUtil Move some utility methods to OfferUtil Remove unused fields - Offer Refactored to use new VolumeUtil for volume calculations. Made stateProperty and errorMessageProperty fields private. - PaymentAccount Moved isHalCashAccount type check to this class. Moved getTradeCurrency logic to this class. - Contract, radeStatistics2, TradeStatistics3 Refactored to use new VolumeUtil for volume calculations. - Trade Refactored to use new VolumeUtil for volume calculations. Made minor logic simplification, style and formatting changes. - CoinUtil Moved some coin utility methods into this class - CoinUtilTest Moved (coin related) tests from CoinCryptoUtilsTest and OfferUtilTest into CoinUtilTest, and deleted OfferUtilTest, CoinCryptoUtilsTest. - Adjust create and edit offer tests to model refactoring
- Add new core.offer.takeoffer.TakeOfferModel Would have been nice to move more logic from bisq.desktop.main.offer.takeoffer.TakeOfferDataModel, but it has JFX dependencies that cannot be use in :core. - Add grpc protos to support takeoffer, confirmpaymentsent, confirmpaymentreceived Only takeoffer is implemented in this commit. - Refactor OfferInfo grpc proto wrapper, and add offer state field - Add new TradeInfo grpc proto wrapper - Implement takeoffer on server and cli side - Refactor offer/trade tests, add test cases
This was referenced Oct 20, 2020
sqrrm
requested changes
Oct 22, 2020
apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBTCOfferTest.java
Show resolved
Hide resolved
apitest/src/test/java/bisq/apitest/method/trade/TakeBuyBTCOfferTest.java
Show resolved
Hide resolved
| private Volume volume; | ||
|
|
||
| @Inject | ||
| public TakeOfferModel(AccountAgeWitnessService accountAgeWitnessService, |
There was a problem hiding this comment.
There's a lot of code that would be nice to reuse from TakeOfferDataModel here. Can be done later though in some pure refactor commit.
There was a problem hiding this comment.
I mentioned that I wanted to move more from TakeOfferDataModel into core TakeOfferModel`, but I could not pass JFX property values into the new core model. If you or @chimp1984 know how, do tell.
There was a problem hiding this comment.
Don't think we want to have any jfx dependency in core, rather some stuff could probably be rewritten in TakeOfferDataModel to utilize the new class.
ghubstan
added a commit
to ghubstan/bisq
that referenced
this pull request
Oct 22, 2020
Resolves issue found during bisq-network#4673 review, and mentioned in comment bisq-network#4673 (comment)
ghubstan
added a commit
to ghubstan/bisq
that referenced
this pull request
Oct 22, 2020
Resolves issue found during bisq-network#4673 review, and suggested in comment bisq-network#4673 (comment)
ghubstan
added a commit
to ghubstan/bisq
that referenced
this pull request
Oct 22, 2020
Resolves issue found during bisq-network#4673 review, and suggested in comment bisq-network#4673 (comment) Also shortened comment lines to < 90 chars.
ghubstan
added a commit
to ghubstan/bisq
that referenced
this pull request
Oct 22, 2020
Resolves issue found during bisq-network#4673 review, and suggested in comment bisq-network#4673 (comment)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR #4672 has to be reviewed & merged before this PR.
Add new
core.offer.takeoffer.TakeOfferModelWould have been nice to move more logic from
bisq.desktop.main.offer.takeoffer.TakeOfferDataModel,but it has JFX dependencies that cannot be use in :core.
Add grpc protos to support
takeoffer,confirmpaymentsent,confirmpaymentreceivedOnly
takeofferis implemented in this commit.Refactor
OfferInfogrpc proto wrapper, and add offer state fieldAdd new
TradeInfogrpc proto wrapperImplement
takeofferon server and cli sideRefactor offer/trade tests, add test cases