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

Editing an offer should not allow the BTC amount to be changed #4182

Merged
merged 1 commit into from May 11, 2020
Merged

Editing an offer should not allow the BTC amount to be changed #4182

merged 1 commit into from May 11, 2020

Conversation

ghost
Copy link

@ghost ghost commented Apr 19, 2020

When editing an offer to adjust the price, the fiat amount is recalculated to reflect the new price. The BTC amount should not change. Due to a bug, sometimes the BTC amounts were changed due to a recalculation from the fiat amount.

This PR Fixes #2798 by disabling a call to recalculate BTC and min BTC amounts when editing.

When editing an offer to adjust the price the fiat amount is
recalculated to reflect the new price.  The BTC amount should
not change.  Due to a bug, sometimes the BTC amounts were
changed due to a recalculation from the fiat amount.
This PR Fixes 2798 by disabling a call to recalculate BTC and
min BTC amounts when editing.
@ghost
Copy link
Author

ghost commented Apr 19, 2020

TEST - Editing an offer should not change the BTC amount due to rounding.

Setup - Using bisq prior to the fix.

  • Create and save an offer to sell 0.0625 BTC for a price of 7500 (469 USD)
  • Go to Open Offers and click on the edit icon
  • Verify that the edit dialog shows 0.0625 BTC and price of 7500 (469 USD)
  • Change the price to 7400 and press tab. The amount of BTC changes to 0.0626 (463 USD)
  • Change the price to 1111 and press tab. The amount of BTC changes to 0.0630 (70 USD)
  • Change the price to 1024 and press tab. The amount of BTC changes to 0.0635 (65 USD)
  • Change the price to 7400 and press tab. The amount of BTC remains at 0.0635 (470 USD)
  • Save the offer.
  • It is now a sell of 0.0635 at 7400 (470 USD).
  • The BTC amount has changed from 0.0625 to 0.0635 when you only wanted to edit the price of the offer. Bug.

Setup - Using bisq incorporating this fix.

  • Create and save an offer to sell 0.0625 BTC for a price of 7500 (469 USD)
  • Go to Open Offers and click on the edit icon
  • Verify that the edit dialog shows 0.0625 BTC and price of 7500 (469 USD)
  • Change the price to 7400 and press tab. The amount of BTC remains 0.0625 (463 USD)
  • Change the price to 1111 and press tab. The amount of BTC remains 0.0625 (69 USD)
  • Change the price to 1024 and press tab. The amount of BTC remains 0.0625 (64 USD)
  • Change the price to 7400 and press tab. The amount of BTC remains 0.0625 (463 USD)
  • Save the offer.
  • It is now a sell of 0.0625 at 7400 (463 USD).
  • The BTC amount has remained the same, you've changed the price and the resultant USD amount. This is good.

These examples use USD but it works the same using EUR or other currencies.

Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

ACK

I have tested it and it this bug is no longer apparent.

@ripcurlx I worry this might affect some other part of the offer screen update at some point, but I feel it would be smaller issue than the bug so I will merge this.

@sqrrm sqrrm merged commit 6d108ea into bisq-network:master May 11, 2020
@ghost ghost mentioned this pull request May 12, 2020
@ghost ghost mentioned this pull request May 28, 2020
@ripcurlx
Copy link
Contributor

@jmacxx Yes, I fear this might effect some use-cases in the create offer step. Unfortunately this view grew quite big over the years and is time intensive to test. I'll move your fix so it will only effect the edit offer view so we are on the save side for this release. See #4291

@ghost ghost deleted the fix_issue_2798 branch June 29, 2020 20:44
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.

Cannot edit offer: after a few edits of the "sell BTC" offer, the BTC amount has increased
2 participants