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

[WIP] Change api method params to posix-style opts #5079

Closed

Conversation

ghubstan
Copy link
Contributor

@ghubstan ghubstan commented Jan 13, 2021

Posix-style method opts replace the formerly ambiguous, positional method params in the CLI. The opts parsing lib used also adds some much needed client side method param validation.

This change also adds new api CoreHelpService for serving method specific help from the server. Help text is defined on the server to avoid duplicating it when the api starts serving RESTful clients.

To see takeoffer method help, run: ./bisq-cli --password=xyz --port=9998 takeoffer -help
The output will probably change, but here is what it looks like now:

takeoffer

Takes an existing offer for a matching payment method.  The Bisq trade fee can be paid in BSQ or BTC.

Usage:  takeoffer -o=offer-id -p=payment-acct-id [-c=taker-fee-currency-code = bsq|btc]

Parameters:
-o=offer-id                   (required)                  The ID of the offer being taken.
-i=payment-acct-id            (required)                  The ID of the payment account to be used in the trade.
-c=taker-fee-currency-code    (optional, default=BTC)     The Bisq trade taker fee currency (BSQ or BTC).

Example:
To take an offer with ID f2930107-7e67-4006-b92c-4f2cebef9376 using matching payment account with ID f0f02d74-6a9a-4dca-80c9-7216b2c4d8bc, paying the Bisq trading fee in BSQ:
./bisq-cli --password=xyz --port=9998 -m=takeoffer  -o=f2930107-7e67-4006-b92c-4f2cebef9376  -p=f0f02d74-6a9a-4dca-80c9-7216b2c4d8bc  -c=bsq

This is the 4th in a chain of PRs, starting with #5063.
#5076 should be reviewed & merged before this one.

- 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.
Helps reduce size of CliMain class file.
For serving method level help to CLI (and later, RESTful clients).
Method opt names are not finalized yet.
All api methods that take one or more parameters now use posix-style params
instead of positional params.  This makes method params less ambiguous,
and provides some client side validation that was absent before this change.

This is ongoing work, as the CLI level help text needs to be adjusted for
the new parameter style, and server side method help needs to be written.
This is no longer needed because joptsimple correctly handles negative
posix style parm values.  (With positional params, joptsimple was treating
negative param values as separate opts.)
We do not need to associate an order number posix-style param descriptions.
@ghubstan
Copy link
Contributor Author

I made a mistake by making the CLI method name a posix-style opt. I will change it to be the only positional parameter in a command.

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.
@ghubstan
Copy link
Contributor Author

Closing, will re-create later, but right now there are too many changes, making it a messy PR.

@ghubstan ghubstan closed this Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants