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

[BUGFIX] Account validation comparing user trade limit with MAX amount of offer #3625

Merged
merged 1 commit into from Nov 18, 2019

Conversation

@rafaelpac
Copy link
Contributor

rafaelpac commented Nov 17, 2019

Fixes #3601 and fixes #3537.
The bug only happens when the offer has MIN-MAX value range.
The new isAmountValidForOffer in PaymentAccountUtil.java considers the MAX amount of the offer, but the button to take it is ungrayed considering the MIN amount, which causes the bug.
For example, if a user has a trade limit of 0.01BTC and an offer has a range of 0.005BTC to 0.1BTC, this user might want to accept it up to his limit, and the button will be available to him.
But the bug will kick in, which freezes any other offer taking until Bisq is restarted.
After getting to next screen, other methods of preventing the user to go above his limit are already implemented in TakeOfferDataModel.java, like:

void initWithData(Offer offer) {
        [...]
        this.amount.set(Coin.valueOf(Math.min(offer.getAmount().value, getMaxTradeLimit())));
public void onPaymentAccountSelected(PaymentAccount paymentAccount) {
        [...]
        this.amount.set(Coin.valueOf(Math.max(offer.getMinAmount().value, Math.min(amount.get().value, myLimit))));
void applyAmount(Coin amount) {
        this.amount.set(Coin.valueOf(Math.min(amount.value, getMaxTradeLimit())));
…mount of offer

Fixes issues #3601 and #3537.
The bug only happens when the offer has MIN-MAX value range.
The new isAmountValidForOffer in PaymentAccountUtil.java consider only the MAX amount, but the button to take the offer is ungrayed considering the MIN amount, which causes the bug.
Copy link
Member

freimair left a comment

utAck

@freimair freimair mentioned this pull request Nov 18, 2019
Copy link
Member

ripcurlx left a comment

utACK - Thanks for catching and fixing this bug! This indeed doesn't work for range offers in that scenario.

@ripcurlx ripcurlx merged commit 7c83230 into bisq-network:master Nov 18, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rafaelpac rafaelpac mentioned this pull request Nov 19, 2019
@rafaelpac

This comment has been minimized.

Copy link
Contributor Author

rafaelpac commented Dec 13, 2019

Fixes #3663

@ripcurlx ripcurlx mentioned this pull request Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.