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

Fix fat finger protection in Offer Entry screen #5914

Merged
merged 1 commit into from Jan 3, 2022

Conversation

jmacxx
Copy link
Contributor

@jmacxx jmacxx commented Dec 13, 2021

Normally, for currencies that have a market price feed, Bisq prevents the user from entering offers that are more than 50% away from spot. It recently came to light that this fat finger protection in Bisq has a flaw.

A certain sequence of focus operations in the enter offer screen causes fat finger protection to turn off, and then the user is no longer protected from making offers that are significantly out of the market.

To reproduce the issue:

  1. Go to BUY (or SELL) BTC for an market priced asset, e.g. ETH
  2. Click Create New Offer
  3. Enter 0.25 BTC
  4. Tab to the next field.
  5. Click the up/down arrow icon to make sure the fixed price edit box is selected at the top. The price and deviation gets auto-populated to market price and 0% deviation.
  6. Enter a fixed price that is 10x higher than market value.
  7. Click "Next" and the fat finger protection warning DOES NOT pop up.
  8. This is because the deviation still shows 0%.

Video example:

fat_finger


The two lines of code removed by this PR seem to be a relic from < 2018 so its hard to locate the original intent. Regardless, it will require a good regression test to check that this fix does not affect any other path in the offer UI.

@ripcurlx
Copy link
Member

@jmacxx Can you re-produce this all the time? I tried to, but failed so far (see recording)

Bildschirmaufnahme.2021-12-20.um.13.05.14.mov

@jmacxx
Copy link
Contributor Author

jmacxx commented Dec 20, 2021

It is reproducible consistently, whenever the % field is by default shown at the top when the offer dialog is opened. If it is not at the top, make it so, then close the dialog and reopen to reproduce the issue.

@ripcurlx
Copy link
Member

It is reproducible consistently, whenever the % field is by default shown at the top when the offer dialog is opened. If it is not at the top, make it so, then close the dialog and reopen to reproduce the issue.

Now I was able to re-produce it as well. I've to check why I've added this bit in 2018 🤔

@ripcurlx
Copy link
Member

Luckily there was a test for it otherwise I wouldn't know it anymore

Bildschirmaufnahme.2021-12-21.um.13.24.37.mov

This change would break the behavior above, so we need to solve it somehow else.

Normally, for currencies that have a market price feed, Bisq provents
the user from entering offers that are more than 50% away from spot.
It recently came to light via a mediation case that this fat finger
protection in Bisq has a flaw.
A certain sequence of focus operations in the enter offer screen
causes fat finger protection to turn off, and then the user is no
longer protected from making offers that are significantly out of
the market.

To reproduce the issue:

Go to BUY (or SELL) BTC for an market priced asset, e.g. ETH
Click Create New Offer
Click the up/down arrow icon to make sure the % from market price
edit box is selected at the top.
Close the dialog, this saves the selected price format.
Click Create New Offer
Enter 0.25 BTC
Tab to the next field.
Click the up/down arrow icon to make sure the fixed price edit box
is selected at the top.  The price and deviation gets auto-populated
to market price and 0% deviation.
Enter a fixed price that is 10x higher than market value.
Click "Next" and the fat finger protection warning DOES NOT pop up.
This is because the deviation still shows 0%.
@jmacxx
Copy link
Contributor Author

jmacxx commented Dec 28, 2021

Here is an alternative fix that does not break the test for #5914 (comment)

Copy link
Member

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK - code change looks fine to me now 👍

@ripcurlx ripcurlx added this to the v1.8.1 milestone Jan 3, 2022
@ripcurlx ripcurlx merged commit b5fab64 into bisq-network:master Jan 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants