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

Validator Minimum Self-Delegation #3495

Closed
rigelrozanski opened this issue Feb 4, 2019 · 15 comments · Fixed by #3546
Closed

Validator Minimum Self-Delegation #3495

rigelrozanski opened this issue Feb 4, 2019 · 15 comments · Fixed by #3546
Assignees

Comments

@rigelrozanski
Copy link
Contributor

A long time ago there was discussion about allowing validators to set a %-self-delegation guarantee parameter for delegators. for Instance "I will always have a self-bond ratio of at least 10%". The idea was that if this threshold was crossed the validator would start unbonding for safety.

currently we do in a way have this, validators who unbond their self delegation and are left with 0% self-delegation currently trigger a validator unbonding

The obvious constraint of allowing for this new parameter is that a malicious delegator could trigger a validator to unbond, OR if we prevented this transaction, validators could use the mechanism to intentionally limit their size (is this latter point a problem?)

Anyways, just soliciting discussion because I couldn't find any on github... but I know we've talked about it.

CC @cwgoes @sunnya97 @jaekwon

@sunnya97
Copy link
Member

sunnya97 commented Feb 5, 2019

Easy way to solve this for now is just allow them to define their min self-delegation as an amount of Atoms, rather than percentage of total bond.

@sunnya97 sunnya97 added this to the v0.31.0 (Launch RC) milestone Feb 6, 2019
@sunnya97 sunnya97 added this to To Do in v0.31.0 (Launch RC) via automation Feb 6, 2019
@alexanderbez alexanderbez moved this from To Do to In Progress in v0.31.0 (Launch RC) Feb 6, 2019
@alexanderbez alexanderbez moved this from In Progress to To Do in v0.31.0 (Launch RC) Feb 6, 2019
@awrelll
Copy link

awrelll commented Feb 6, 2019

What's maximum ?

@gamarin2
Copy link
Contributor

gamarin2 commented Feb 6, 2019

I don't think this is needed prelaunch. Before transfers are enabled, only GoS winners + fundraiser participants can have self-bond. Moreover, even if you participated in fundraiser, you probably don't want to use an account derived from your fundraiser mnemonic as operator account for security reasons.

I think we should discuss that later and that it should not block Launch RC.

@alexanderbez
Copy link
Contributor

@gamarin2 it is a relatively simple adjustment to add to the validator. My thinking is something like this should be added pre-launch otherwise it might make it difficult to include once we already have bonded validators at launch.

@gamarin2
Copy link
Contributor

gamarin2 commented Feb 6, 2019

But please consider this. If we have this minimum, then it shouldn't be changeable, just like max-commission and max-change-rate (otherwise what guarantee does it offer?).

This would mean that validators could only put a non-zero parameter if they have a genesis allocation through GoS (again, I don't think it's secure in most cases to use a fundraiser account as validator operator account). And even then, it would be a pretty low amount, so not much of a guarantee...

@alexanderbez
Copy link
Contributor

Yes, it is changeable. It must be non-zero and can never be changed below the initial value.

@gamarin2
Copy link
Contributor

gamarin2 commented Feb 6, 2019

Yes, it is changeable. It must be non-zero and can never be changed below the initial value.

Ok, this makes sense to me. However I still don't see it as a hard requirement for launch, except if there are strong technical reasons regarding difficulty of update.

@cwgoes
Copy link
Contributor

cwgoes commented Feb 6, 2019

@gamarin2 The primary concern is that a malicious validator could convince delegators to delegate, unbond all but one micro-Atom of their self bond, commit an equivocation and get their delegators slashed without incurring any punishment themselves.

I don't think implementation is too challenging; I believe @sunnya97 is working on it at the moment.

@alexanderbez alexanderbez moved this from To Do to In Progress in v0.31.0 (Launch RC) Feb 6, 2019
@rigelrozanski
Copy link
Contributor Author

rigelrozanski commented Feb 7, 2019

For documentation purposes the conclusion that @sunnya97 and I came to is there needs to be two parameters:

  • Self-Delegation Minimum (default 0)
    • This parameter may be arbitrarily increased at no limit (favourable for delegators)
    • This parameter may be decreased at a maximum daily rate (next bullet)
  • Maximum Self-Delegation Reduction Per Day (default very large)
    • This parameter can only ever be decreased (arbitrarily by the validator)

This mechanism works quite similarly to the commission change mechanism to rate limit changes unfavourable to delegators, but at a rate which the validator imposes on itself. The protocol simply forces the validator into the unbonding period if these conditions are broken.

@mdyring
Copy link

mdyring commented Feb 10, 2019

Is there a reason the --min-self-delegation flag does not take a coin denom?

Since denom is user everywhere else, I think it would be nice to introduce this. Both for consistency but also to check denom is same as the one in amount.

@alexanderbez
Copy link
Contributor

I imagine because we only allow for a single staking coin and hence no need to provide user input of denom as it's implicit. Correct me if I'm wrong @rigelrozanski or @sunnya97.

@mdyring
Copy link

mdyring commented Feb 10, 2019

It is a fairly important parameter to get right from the start. So I'd like to advocate for consistency and clarity here, to avoid any mistakes.

@alexanderbez
Copy link
Contributor

@mdyring most certainly! But what unclarity is there since you can only bond in one token -- the staking token?

@mdyring
Copy link

mdyring commented Feb 10, 2019

From a user (well, validator) perspective it is confusing when there are different conventions depending on which command is invoked, or even between flags of a single command.

So I'd like to think that whatever arguments were used to add denoms to all other staking related operations would apply here as well. :-)

@rigelrozanski
Copy link
Contributor Author

@mdyring I agree - it would make sense to require consistency - AFAICT, we ought to require the denom to be included in the CLI/GUI but msg types should not require use of denom - opening a spin off issue: #3594

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants