Stub out support for OpenOffer's triggerPrice in api #5076
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
- Fix bug locating index of method name in String[] args. - Make createoffer method's security-deposit param format consistent with UI's. When creating an offer, the CLI should take "15.0", not "0.15" as a 15% security deposit. This is consistent with the UI, and the CLI's mkt-price-margin input format.
- Injected OfferFilter into CoreOffersService and CoreTradesService. The filter constrains 'getoffer(s)' results to offers an api user can take, as the first part of ongoing protection tools / api integration. - Created a new CoreContext singleton. Initially, lets Core*Services know if the user is using the api -- isApiUser=true if the current thread's name is "BisqDaemonMain" or the name contains "grpc". We do this anticipating future :desktop dependencies on the core api, and we don't pass hardcoded isApiUser=true params to lower level domain objects. We cannot check BisqDaemonMain.class.getSimpleName() because :core cannot have a circular dependency on :daemon, but there is probably a better way to do this than depending on the thread name set in BisqDaemonMain#configUserThread(). - Added @singleton annotation to all Core*Service classes.
This is a feature that will not be included in api v1, but partial support is added in this change to the server. CLI will pass a default triggerPrice of 0 (unused) with the createoffer command. When fully implemented, an optional trigger-price param will be added to the CLI's createoffer method, and the value will only be visible to offer owners. New enableoffer and disableoffer methods will also need to be added.
ghubstan
changed the title
|
force travis build (download jar) |
The createoffer trigger price was defined as the 0 default value in the gRPC request object (causing the file conflict with the main branch). This line can be removed because a protobuf long's default value is 0.
sqrrm
reviewed
Jan 13, 2021
There was a problem hiding this comment.
Triggerprice changes look ok.
The reason I don't feel it's an important feature for the api is that the api user could listen to the price feed and disable offers programmatically rather than relying on the built in feature. At least once enable/disable offer is implemented.
Do not set isApiUser=true based on hardcoded thread name ;-( For requested change bisq-network#5065 (review)
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.
This is a feature that will not be included in api v1, but partial support is added with this change to the server. CLI will pass a default
triggerPriceof 0 (default, unused) with thecreateoffercommand.When fully implemented, an optional trigger-price param will be added to the
createoffermethod, and its value will only be visible to the offer owner. Newenableofferanddisableoffermethods will also need to be added at that time.This is the 3rd in a chain of PRs, starting with #5063.
#5065 should be reviewed & merged before this one.