-
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
Adjust API 'editoffer' to PR 5651 (include extraData field when editing offer) #5666
Merged
sqrrm
merged 65 commits into
bisq-network:master
from
ghubstan:08-handle-extradata-in-editoffer
Aug 30, 2021
Merged
Adjust API 'editoffer' to PR 5651 (include extraData field when editing offer) #5666
sqrrm
merged 65 commits into
bisq-network:master
from
ghubstan:08-handle-extradata-in-editoffer
Aug 30, 2021
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
- Define set of editable offer payload fields in MutableOfferPayloadFields. - Move bulk of EditOfferDataModel#onPublishOffer logic to OfferUtil.
- Add editOffer to GrpcOffersService, CoreApi, CoreOffersService. - Set editOffer call rate meter to 1 / minute. - Use new EditOfferValidator to verify editOffer params OK. - Adust getMyOffer(s) rpc impl and OfferInfo model to use OpenOffer for accessing activation state and trigger price.
Reduces size of GrpcClient while allowing for additional bot-friendly variations of the new grpc editOffer method.
Support for editing BSQ offers is in place, but will be added in another PR.
Optionally displaying an ENABLED column in CLI side getoffer output depends on the value of offer.isMyOffer, which is passed via new boolean arguments to the trade & offer pojo builders.
BSQ offers are fixed-price only. This change blocks an attempt to change an altcoin offer to a margin price based offer, or set a trigger price.
And log CLI's getoffer output to see getoffer formatting -- after adding new ENABLED and TRIGGER-PRICE columns.
This change is a refactoring for handling the removal of a peer UI's offer item when it is deactivated and edited in the same CLI `editoffer` command. On the API side, an `editoffer --price=N --enable=false` command results in the edited offer not being re-published. On a UI peer's side, the edited offer is not added to the peer's storage, and the peer's onRemoved(offer) listener event does not find a storage entry with matching payload-hash. This fix assumes an offer that is not in the local store should be removed from the UI's view list -- when the onRemoved method's hashOfPayload does not match the UI's view list item's hashOfPayload.
A newly created offer has no OpenOffer+State (AVAILABLE || DEACTIVATED) when displayed in the CLI's console. This change adds a 'bool isMyPendingOffer' to the OfferInfo proto + wrapper, and the CLI's console offer output formatter uses it to determine if it should display a new offer's Enabled column value as PENDING, instead of an ambiguous NO value.
Resolve file conflicts resulting from merging PRs bisq-network#5577 bisq-network#5651
…ffer-with-pending-status
…xtradata-in-editoffer
This was referenced Aug 16, 2021
Hash of protectedStorageEntry (should be offerPayload) was sometimes resulting in incorrect hash being sent to OfferBook listener methods onAdded(offer, hashOfPayload, sequenceNumber), and onRemoved(offer, hashOfPayload, sequenceNumber). Hash of OfferPayload is correctly passed to listener with this change. Sending the correct hash allows removal of a dubious code block that removed a book view list item when hash compare failed, and no matching offer existed in the OfferBookService. See bisq-network#5659 (comment)
Checking offer payload hashes in OfferBook's onAdded and onRemove methods is sufficient to prevent incorrect removal of offer list items from the UI OfferBook view (where api 'editoffer' causes onRemoved to be called after onAdded on peers).
sqrrm
reviewed
Aug 20, 2021
Any and all view list items with a matching offerId should be removed from view just before adding a new list item.
sqrrm
reviewed
Aug 25, 2021
desktop/src/main/java/bisq/desktop/main/offer/offerbook/OfferBook.java
Outdated
Show resolved
Hide resolved
desktop/src/main/java/bisq/desktop/main/offer/offerbook/OfferBook.java
Outdated
Show resolved
Hide resolved
The new debug log statements included in this PR help trace add/remove list item actions if problems are seen in the UI's OfferBook, after the API 'editoffer' method is released. They can and should be removed in a future PR if the released API feature proves it did not introduce bugs into the UI.
sqrrm
approved these changes
Aug 30, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
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 #5651 ensures payment acct changes are saved when the UI edits an offer.
Although the API
editoffer
command does not yet support editing payment account details, this series of PRs must adjust to 5651 (merged today) to avoid file conflicts.This PR is based on #5663, which should be reviewed and merged before this one.