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

Deposit improvements #4347

Merged
merged 11 commits into from
Jul 3, 2020
Merged

Conversation

sqrrm
Copy link
Member

@sqrrm sqrrm commented Jun 30, 2020

Implements suggestion in bisq-network/proposals#233 (comment)

Increased visibility of security deposit in offer overview.

Set suggested deposit to double of trade range over the trade period.

Both traders have the same security deposit.

@wiz
Copy link
Member

wiz commented Jul 1, 2020

Are you sure it’s safe to trust the trade statistics data? AFAIK it’s not reliable and can be easily manipulated

@sqrrm
Copy link
Member Author

sqrrm commented Jul 1, 2020

I think it's safe enough to trust it to give a suggested deposit value. This value can still be changed if the trader doesn't like it, and it's still in the 15%-50% range so I don't really see a problem there.

@Conza88
Copy link

Conza88 commented Jul 1, 2020

Yes to making it more prominent in the offer book.
Again - why are we not allowing for the market / choice? Why can I not choose to have a 100% security desposit?!

Why can I not have a high security deposit as a BTC BUYER / offer maker, and set a low one for the BTC seller?

@sqrrm
Copy link
Member Author

sqrrm commented Jul 1, 2020

There seems to be support for symmetric deposits in bisq-network/proposals#233. It's still easy to just change the constant USE_SYMMETRIC_SECURITY_DEPOSIT for those that want to run with non symmetric deposits. Adding more UI elements is quite a bit of work and it makes the UX worse for most users so it's usually good to have reasonable defaults.

@wiz
Copy link
Member

wiz commented Jul 1, 2020

Yeah let's talk about this algorithm. First of all, 50% is wayyy too high - why is it being suggested for a simple 0.1 BTC trade for BSQ ? It's a bit ironic that after I NACK'd your proposal to increase deposit to 30% you responded with a PR that increases it to 50% lol

Screen Shot 2020-07-01 at 20 43 28

@Conza88
Copy link

Conza88 commented Jul 1, 2020

I'm probably not helping. And these are probably dumb questions. But why does there even need to be an algorithm?
Value is subjective - some will see 50% as too high, some as too low, some just right... I mean arguing over 30%, 50% is all missing the point imo.

Choice please. Flexibility. That is the only real answer imo.

Why not indicate the things to consider: i.e. consequences of a low % security deposit? (Less capital tied up for maker, and taker?, more risk for taker? [thus less likely to accept?])

Consequences of a high % deposit? (more capital tied up, but more reassurance for taker?)

shrugs those are all poorly formulated, but hopefully the gist is clear... maker sets security deposit %'s (10%-100%), and let the market (individual) decide whether its acceptable or not.

@sqrrm
Copy link
Member Author

sqrrm commented Jul 1, 2020

@wiz the previous PR raised the minimum to 30%, this leaves it at 15%. It's just suggesting a deposit that the user can change.

@sqrrm
Copy link
Member Author

sqrrm commented Jul 1, 2020

@Conza88 this is basically what's happening now, except the range is 15% - 50%. Widening the range might be problematic during deployment due to the trade protocol, but can be done of course.

There is a problem that the cost of mediation and arbitration is not easily priced for traders so we're basically trying to suggest a reasonable deposit that includes that risk. I suspect many Bisq users don't understand the implication of the deposit as the price of an option that not only depend on the price movement of the underlying but also on the arbitration counterparty risk.

@wiz
Copy link
Member

wiz commented Jul 1, 2020

As a mediator, I have very few BSQ trades that go into mediation. There’s no need to suggest users to more than triple the security deposits, why would you do that?

Please only suggest users to use higher security deposits for the markets and payment methods that need them. BSQ and JPY don’t need higher deposits, those can be left alone. The markets that always go into mediation like XMR and USD with Zelle are probably the ones that could benefit from higher deposit. Didn’t we discuss this in the proposal already

sqrrm added 2 commits July 1, 2020 22:52
Don't include outliers (20% deviation from moving average) in moving
average calculation. It's quite likely that low liquidity markets or
markets with large spreads can't calculate deposit suggestion and will
then suggest deposit from preferences.

Added test for moving average class
@sqrrm
Copy link
Member Author

sqrrm commented Jul 2, 2020

The security deposit needs to be higher for volatile markets, that's what it's pricing, not the safety of the payment method. During very volatile times even 50% might be low so leaving that as max for now is quite reasonable.

The latest changes improves the moving average calculation by excluding prices that deviate more than 20% (not sure if too high, 10%?) from the the moving average. This might cause a failure to calculate a suggestion for illiquid markets and suggest the default which seems reasonable. Issues in new or illiquid markets are likely not from suggesting a low deposit.

@wiz
Copy link
Member

wiz commented Jul 3, 2020

So if someone wants to manipulate your security deposit suggestion algorithm into always suggesting 50% max deposit for all markets, they just need to make 1 self-trade that's way off the current market price? How often do they need to do this?

@sqrrm
Copy link
Member Author

sqrrm commented Jul 3, 2020

They would need to continuously do this at price levels that are less than 20% off the moving average to manipulate the suggested price. Not saying it can't happen, but it seems unlikely as there is little benefit to doing so for the tape painter, and any user can change the suggested deposit if they don't like it.

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 it on Regtest and Mainnet and the values seem reasonable (e.g before in EUR/BTC market 15% vs 17%). Of course long living offers might not have a good adapted deposit amount, but as the deposit can be adapted by the maker and is shown in the offerbook its fine IMO.

@ripcurlx ripcurlx merged commit cfc3252 into bisq-network:master Jul 3, 2020
@ripcurlx ripcurlx added this to the v1.3.6 milestone Jul 6, 2020
@wiz
Copy link
Member

wiz commented Jul 7, 2020

NACK - I just went to create an L-BTC offer as part of the pre-release testing and it suggests 50% deposit - how can that be a reasonable suggestion when L-BTC margins and volatility are razor thin?

Screen Shot 2020-07-07 at 21 32 13

@Conza88
Copy link

Conza88 commented Jul 7, 2020

Why is there even a need for suggested deposits algorithm %?!

Explain the consequences of low and high security deposit to maker & taker.
Clearly indicate in offer books.
Highlight what it means when making/taking.
Sort by default to indicate whatever is more favourable?

@sqrrm
Copy link
Member Author

sqrrm commented Jul 8, 2020

@wiz are you testing this live or regtest? When I create an offer live I get the expected 15% since there aren't enough trades in the near past to get an estimate
image

Also make sure you didn't set the deposit to 50% at some point which saves that value to settings as your preferred minimum deposit.

@wiz
Copy link
Member

wiz commented Jul 8, 2020

This was mainnet testing of the release branch

@sqrrm
Copy link
Member Author

sqrrm commented Jul 8, 2020

Do you still get this when you have preferences set to 15%?

It's not obvious that this is set to preferences, but that's how it's apparently always been, if you set the deposit manually, that value will be saved as your preferred value.

@ripcurlx
Copy link
Contributor

ripcurlx commented Jul 8, 2020

This was mainnet testing of the release branch

@wiz As @sqrrm already mentioned I'm quite sure that this is a stored deposit percentage from your preferences.
Bildschirmfoto 2020-07-08 um 13 23 54

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.

4 participants