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

Use min. refund at mediated payout #3827

Merged

Conversation

chimp1984
Copy link
Contributor

At mediation we require a min. payout to the losing party to keep
incentive for the trader to accept the mediated payout. For Refund
agent cases we do not have that restriction.

There is a change in the adjustment behaviour for custom payout. The previous adjustment at keystroken does not work anymore as when one enters 0.1 the first 0 key stroke would trigger already the min refund value. I changed the event type to focus out, so the adjustment is only done at focus out. Clicking on the background does not trigger a focus out, only if you click on a control (text field, button, toggle,...).

I also added a check to not allow that both outputs are 0 at refund agent case. That was previously possible but caused an exception in the wallet class. Not the button will stay deactivated if both are 0 BTC.

Testing:
Run test case with mediation and refund agent and toggle between the 4 predefined payout distributions as well as a custom payout. Use diff. values including 0, negative and too high to see if it auto-adjusts after focus out. Note that mediation and refund agent have differnet policy here.

At mediation we require a min. payout to the losing party to keep
incentive for the trader to accept the mediated payout. For Refund
agent cases we do not have that restriction.
@ripcurlx
Copy link
Contributor

Just a heads up that I haven't planed to be available until 2nd of Jan, but I'll still try to do some reviews tomorrow.

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.

Overall good but please have a look at the comments.

Also, should we add a note to the users that the offer given during mediation includes some refund of deposit and that all might be lost if taken to refund agent?

Copy link
Contributor

@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.

ACK

Tested on Regtest mediation and refund agent payout and everything works as expected.

@ripcurlx
Copy link
Contributor

ripcurlx commented Jan 3, 2020

@sqrrm Could you please review the changes by @chimp1984 so I'm able to merge?

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

@sqrrm sqrrm merged commit ae2e06d into bisq-network:master Jan 3, 2020
@chimp1984 chimp1984 deleted the use-min-refund-at-mediated-payout branch January 23, 2020 23:56
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.

3 participants