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

spec: update roundup #482

Merged
merged 4 commits into from
Jun 16, 2020
Merged

spec: update roundup #482

merged 4 commits into from
Jun 16, 2020

Conversation

buck54321
Copy link
Member

@buck54321 buck54321 commented Jun 15, 2020

  1. Describe new trade suspension messaging
  2. Describe administrative API
  3. Rewrite intro section. Combine "Abstract" and "Introduction". Shortened and more focused.
  4. Full table of contents in the spec readme
  5. Various typos and drift corrections.

View at https://github.com/buck54321/dcrdex/tree/spec-roundup/spec

Resolves #280

Copy link
Member

@chappjc chappjc left a 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.

spec/orders.mediawiki Show resolved Hide resolved
spec/orders.mediawiki Outdated Show resolved Hide resolved
spec/README.mediawiki Outdated Show resolved Hide resolved
spec/README.mediawiki Outdated Show resolved Hide resolved
| -
| /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
Copy link
Member

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

Copy link
Member Author

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?

Copy link
Member

@chappjc chappjc Jun 15, 2020

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.

Copy link
Member

@chappjc chappjc Jun 15, 2020

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.

Copy link
Member Author

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.

Copy link
Member

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.

Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member

@chappjc chappjc Jun 16, 2020

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.

spec/comm.mediawiki Outdated Show resolved Hide resolved
spec/README.mediawiki Outdated Show resolved Hide resolved
spec/admin.mediawiki Show resolved Hide resolved
@chappjc chappjc merged commit f482b89 into decred:master Jun 16, 2020
@chappjc chappjc added the spec label Oct 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spec: update trade suspension message.
3 participants