-
Notifications
You must be signed in to change notification settings - Fork 102
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
spec: update roundup #482
spec: update roundup #482
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great updates overall. Just a few suggestions.
| - | ||
| /market/{marketID}/suspend || schedule a market suspension at the end of the current epoch | ||
| - | ||
| /unban/{accountID} || clear an account's penalties and re-enable trading |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feelings about the /ban API endpoint? This relates to the previously discussed penalization notification, which should probably be signed by the server and include details of the infraction(s).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was gonna chat with you and @JoeGruffins about that. I see the question is still sort of in the air at #469 (comment). My gut says that we shouldn't even have the option. Is there a good reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unforeseen mischief, including hack attempts, etc. There are already a myriad of ways an operator can DoS a user, so making this seamless just makes sense to me. If the issue is sorted out, the unban function exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, the ban/unban will probably need to be renamed to suspend/reinstate or similar (eventually, not now), or at least the ban function will need a duration, maybe even option to disallow connection, when we figure out more advanced scoring and temporary suspension mechanics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. Will just add /ban
for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think with the current rules, ban is not so useful. Maybe only to re-ban someone you accidentally unbanned. But new rules may be added in the future, that can't be done programmatically, or that need to be retroactively imposed.
However, it could be abused by the operator. A normal exchange works on trading fees, so banning isn't in the best interest of the operator. But since the DEX is based on an initial fee, unless a user is vocal about it, banning has no impact on the operator. I don't know if this is an actual problem. I think we will find out when the DEX goes into production. Of course, even without this endpoint, an operator can just switch a value in the db and have the same effect. One possible solution is to try to make ban data public, if it is indeed a problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
banning has no impact on the operator
This may not be true. I guess having low volume would not get you new users. So there may be enough incentive for this to not be abused. If I were banned for nothing, I certainly wouldn't pay the fee again, I'd find another DEX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with all of that. My sort of half-baked philosophy on this is that if we need to manually ban someone, then our software isn't doing it's job. But I also recognize that there are bugs, and a learning curve, and that the ability to ban someone who skirts the rules might be critical at some point. Right now, I'm feeling like we should leave it, and just make it one of our goals that nobody ever has to use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should have it in the code and document it. It could be hacked into the code anyway, the DB could be fiddled with, and firewalls employed for banning, so it should be a supported function. I don't think it make senses to neuter the operator on principle, especially since there will be legit reasons to close an account like this. Consider that a programmatic ban according to the rules also potentially creates a burden for the operator to demonstrate the broken rules. Little changes with a ban function IMO. If the operator closes accounts, their exchange is going to be on everyone's shit list.
View at https://github.com/buck54321/dcrdex/tree/spec-roundup/spec
Resolves #280