-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add 'placeoffer' method #4574
Closed
Closed
Add 'placeoffer' method #4574
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
Bats version check tests now use a bash script for parsing the value from the Bisq class file, and these test cases no longer need to be manually updated.
The string constants deleted from the test case are re-defined as enums, but the test harness still passes around strings (enum.name()) because the handling of invalid dispute agent type string args needs to be tested. (Reminder: CLI does not accept any enum arguments.)
And make test dispute agent registration work from static fixture setup methods.
The converter is needed for CLI, which has no access to core or common utils.
Don't convert parameter case in CLI.
And list the args in CLI --help outout.
The CreateOfferRequest's price field type was changed from long to string, so a CLI param like 10000.0055 can be passed to the server as is, to be scaled and converted there. In general, we want to keep validation logic on the CLI as simple as possible, and use existing core logic to validate args and convert arg types when needed.
And move createAndPlaceOffer to bottom of class file.
A gRPC price service was added, and api create-offer tests can check mkt based price margin calculations.
The CLI needs to be able to register a REFUND_AGENT using the 'refund_agent' or 'refundagent' parameter value (in any case), so an alt-name mapping was added to the enum def.
This is an ugly, temporary fix. Need to refactor again.
…oblem" This reverts commit 50d4b9f. Back out codacy fix; it did not work.
Separates offer placement from offer creation to fix tx result handling problem in GrpcOffersService, and readies the core api for a new CLI 'placeoffer' implementation. Offer placement still happens in the api's 'createoffer', but we may want to change it to show the created offer to a CLI user for review, prior to manual placement via a new 'placeoffer offer-id' (of 'confirmoffer offer-id'?) api method.
This needs to be carefully reviewed to be sure it does not break the create/place offer error messaging in the UI, and should be reverted if offer validation will be refactored to work for both UI and gRPC CLI.
New offer placement is moved out of 'createoffer' into a separate method, to give users a chance to review new offer details before broadcasting it. When createoffer is called, the new offer is cached in CoreOffersService for 5 minutes and details are displayed in the CLI console. The user can broadcast the cached offer by calling 'placeoffer offer-id security-deposit'. The security-deposit parameter is a % as double, e.g., 0.15.
CLI example: Start daemons:
Get the dummy payment account id needed for create offer:
Create and cache a new offer, then examine it:
Place the offer with offer-id param, and a security deposit param as a % double value:
Look at the sell/usd offer just placed:
|
But show original error msgs in server log. - TaskRunner: Prepend "Error at taskRunner:" to logged server error, but pass only the throwable.message to the error msg handler. - Task: Prepend "An error occurred at task:" to logged server error, but pass only the throwable.message to the error msg handler. This change will show a CLI user cleaner error msgs. For example, a CLI user would now see: "Amount is larger than 1.00 BTC", not "Error at taskRunner: An error occurred at task ValidateOffer: Amount is larger than 1.00 BTC" Minor re-formatting was made in Task and TaskRunner.
Closed, as per #4559 (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.
New offer placement is moved out of
createoffer
into a separate method, to give users a chance to review new offer details before broadcasting it.When
createoffer
is called, the new offer is cached inCoreOffersService
for 5 minutes, and details are displayed in the CLI console. The user can broadcast the cached offer by callingplaceoffer offer-id security-deposit
. The security-deposit parameter is a % as double, e.g., 0.15.PRs #4554 and #4559 need to be review/merged before this one.