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

Further examine necessity of clipping on withdrawals #3041

Closed
cwgoes opened this issue Dec 7, 2018 · 9 comments
Closed

Further examine necessity of clipping on withdrawals #3041

cwgoes opened this issue Dec 7, 2018 · 9 comments

Comments

@cwgoes
Copy link
Contributor

cwgoes commented Dec 7, 2018

Ref https://github.com/cosmos/cosmos-sdk/pull/3033/files#r239978938

@cwgoes
Copy link
Contributor Author

cwgoes commented Dec 7, 2018

Possibly this is addressed by #2958.

@rigelrozanski
Copy link
Contributor

Sure we ought to specify this better in new docs. In the Dec=>Int PR we always ensure that the validator will be emptied aka we do not burn tokens ever.

@cwgoes
Copy link
Contributor Author

cwgoes commented Dec 8, 2018

My concern is as to why this was happening in the first place - seems like some delegator must have previously received more tokens than they ought to have (which is very concerning).

@rigelrozanski
Copy link
Contributor

What's happening in Jae's PR is not completely obvious to me, however in the Dec=>Int PR there is a clear situation which gives rise to a delegator share being "worth" an uneven amount of tokens THUS when they withdraw, the remaining tokens should be left in the validator, making all the other delegator shares now worth slightly more

@jackzampolin
Copy link
Member

Is this closed by F1?

@cwgoes
Copy link
Contributor Author

cwgoes commented Jan 10, 2019

Is this closed by F1?

No, this has nothing to do with fee distribution, the "withdrawal" referred to is withdrawal of tokens from a validator (in the staking module).

@rigelrozanski
Copy link
Contributor

Do we even clip any more? - during the Dec=>Int this has been consolidated to 1 or 2 straight forward places

@jackzampolin
Copy link
Member

@rigelrozanski thats a great question!

@rigelrozanski
Copy link
Contributor

Just investigated this... within staking clipping is isolated to the (validator) RemoveDelShares
function

if remainingShares.IsZero() {
// last delegation share gets any trimmings
issuedTokens = v.Tokens
v.Tokens = sdk.ZeroInt()
} else {
// leave excess tokens in the validator
// however fully use all the delegator shares
issuedTokens = v.DelegatorShareExRate().Mul(delShares).TruncateInt()
v.Tokens = v.Tokens.Sub(issuedTokens)
if v.Tokens.IsNegative() {
panic("attempting to remove more tokens than available in validator")
}
}

I think it's pretty straightforward why this needs to happen, under the situation that slashing caused delegator's shares to be worth an uneven amount of tokens. Again we are rounding at the nano-atom level.

I'm confident that this is correct and no reason for concern.

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

No branches or pull requests

3 participants