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

Display the reason for auto-disabling an open offer. #6952

Merged

Conversation

ghost
Copy link

@ghost ghost commented Nov 28, 2023

Fixes #6127. Users have complained that when fee validation flags & disables their offer, they have no idea why.
This PR adds a status which is shown to the UI when an offer has been disabled.
The disabled reason is shown in Open Offers via the shield icon tooltip and globally via a notification.

Reasons an offer can be flagged:

  • BTC fee tx not found
  • BSQ fee tx not found
  • Maker Fee underpaid
  • Taker Fee underpaid
  • Fee receiver not a recognised address
  • JSON parsing failure

An offer can also become disabled if the user chooses a Trigger price #4998. This reason is also shown on the shield icon and notification.

Key features:

  • Adds an enum of potential fee validation reasons.
  • Shows the reason to Maker in Open Offers if there was an issue with the maker fee. Previously would silently disable the offer.
  • Shows the reason to Taker in Take Offer, if there was an issue with the maker fee. Previously would say maker validation failed.
  • Shows the reason to Maker & Taker in Trade Step 2, if there was an issue with the taker fee. Previously would say taker validation failed.

Bug Fix:

In prior releases, sometimes there would be a false flagging of the offer when checked at startup because the BSQ tx was not yet available. This PR includes a fix to postpone the initial check until a minute after the boostrapping.

Screenshots:

Bisq_2023-11-24_11-13-12

 

Bisq_2023-11-28_10-13-07

 

Bisq_2023-11-28_13-18-41


Fixes #6959

@ghost
Copy link
Author

ghost commented Dec 7, 2023

Added 265ca17 Fixes #6959

@ghost ghost changed the title Fee validation refactor, adds reason code & notifications. Display the reason for auto-disabling an open offer. Dec 19, 2023
@ghost
Copy link
Author

ghost commented Dec 19, 2023

I created this PR as a result of numerous requests during the year, both via #6127 and Matrix chat.
Maintainers, would you prefer the two included bugfixes moved into separate PRs?

@pazza83
Copy link

pazza83 commented Dec 20, 2023

Looks like a great addition.

I think the trade amount being higher than a user's Bisq's 'trade limit' settings is also a common reason why offers are not shown.

@ghost
Copy link
Author

ghost commented Dec 20, 2023

Not true @pazza83. A user's offer is de-activated (a) while a user edits it. (b) if the trigger price is breached (c) if the fee validation fails.

Indicating the reason for an auto-disable is a nice-to-have, but this PR has included two bug fixes which are higher priority, i.e. they fix issues that can cause failed trades. It would be disappointing if the bug fixes missed the 1.9.15 release due to this larger feature being left on the shelf. Hence my question above, should the bug fixes be separated out so they can be reviewed with higher priority?

@pazza83
Copy link

pazza83 commented Dec 20, 2023

Ok, thanks for clarifying. I was confused about how the trade limit setting works

@ghost ghost mentioned this pull request Dec 26, 2023
Copy link
Contributor

@alvasw alvasw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@alejandrogarcia83
Copy link
Contributor

@alvasw Thank you for reviewing this PR! :-)

Copy link
Contributor

@alejandrogarcia83 alejandrogarcia83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@alejandrogarcia83 alejandrogarcia83 merged commit e5ed806 into bisq-network:master Dec 31, 2023
3 checks passed
@alejandrogarcia83 alejandrogarcia83 added this to the v1.9.15 milestone Dec 31, 2023
@ghost ghost mentioned this pull request Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants