-
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
[WIP] Workaround API editoffer event rcv order problem in peer UI #5627
Closed
ghubstan
wants to merge
44
commits into
bisq-network:master
from
ghubstan:06-add-remove-evt-order-workaround
Closed
[WIP] Workaround API editoffer event rcv order problem in peer UI #5627
ghubstan
wants to merge
44
commits into
bisq-network:master
from
ghubstan:06-add-remove-evt-order-workaround
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.
And make sure function is not duplicated CLI side logic.
Avoid inconsistent CLI output decimal formats across different systems' default locales.
When the API is used to edit an offer, the daemon's P2PDataStorage will send onRemoved(offer) and onAdded(offer) events 1-3 ms apart, and those events may arrive at a peer's UI in the wrong order: onAdded, onRemoved. When the order is backwards, the edited offer disappears from the UI's OfferBook view. Reloading the offerBookListItems works around the problem. Reloading 100 offers usually takes less then 1 ms. I would prefer to solve this in 2+ other classes: - P2PDataStorage, where these events are fired in the correct order, but down to 1ms apart. - OfferBookService constructor, where the peer UI's HashMapChangedListener methods 'onRemoved' and 'onAdded' are causing API edited offers to dissappear if received in backwards order: 'onAdded', then 'onRemoved'. Trying to insert a delay between remove an offer for edit, then republishing the offer, does not work. The events can still arrive in the wrong order. I am going to need some help from @chimp1984 to solve it.
ghubstan
changed the title
[WIP] Workaround API editoffer event rcv order problem in peer UI
Workaround API editoffer event rcv order problem in peer UI
Jul 18, 2021
ghubstan
changed the title
Workaround API editoffer event rcv order problem in peer UI
[WIP] Workaround API editoffer event rcv order problem in peer UI
Jul 18, 2021
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.
When the API is used to edit an offer, the daemon's
P2PDataStorage
will sendonRemoved(offer)
andonAdded(offer)
events 1-3 ms apart, and those events may arrive at a peer's UI in the wrong order:onAdded
, thenonRemoved
. When the event receive order is backwards, the edited offer disappears from the UI's OfferBook view.Reloading the
OfferBook#offerBookListItems
works around the problem. Reloading 100 offers usually takes ~ 1-2 ms.I said "less than 1ms" in the commit message, but I am still testing the load time while more offers are added.
The proper solution is probably in or near 2+ other classes:
P2PDataStorage
, where these events are fired in the correct order, but down to 1ms apart.OfferBookService
, where a peer UI'sHashMapChangedListener
methods 'onRemoved' and 'onAdded' are sometimes called in backwards order, causing API edited offers to disappear.Trying to insert a delay between remove an offer for edit then republishing the offer does not work. The events can still arrive in the wrong order.
I might need some help from @chimp1984 to solve this.
This is the 6th in a series of PRs, beginning with #5570.
#5577 should be reviewed and merged before this one.
Note: All commits prior to 1d21cb9 (near the end of this page's commit list) have been reviewed.