-
Notifications
You must be signed in to change notification settings - Fork 14
Conversation
Also fix bug with incorrect delegation field used.
Remove validator staked balance in favor of only delegations. Compute commission.
…ards instead of when unbonding delegation.
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.
LGTM!
I did not go through the state transition pseudo-code in detail though, as this part of the spec isn't immediately implementation critical, as we we will use the staking / distribution modules without any modifications at first.
validator.commissionRate = tx.commissionRate | ||
validator.delegatedCount = 0 | ||
validator.votingPower = tx.amount | ||
validator.votingPower = 0 |
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.
What is the benefit here?
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.
This is part of the refactor of removing staked balance for validators:
Remove
stakedBalance
for validators. Validators now have no balance and need to delegate to themselves. This can be abstracted away at the UI layer.
(Note that this does introduce a divide-by-zero case when computing entries, but I'll take care of that in a future PR, issue in #115.)
@@ -585,7 +586,7 @@ Apply the following to the state: | |||
state.accounts[sender].nonce += 1 | |||
state.accounts[sender].balance -= totalCost(tx.amount, bytesPaid) | |||
|
|||
state.activeValidatorSet[block.header.proposerAddress].pendingRewards += tipCost(bytesPaid) | |||
state.activeValidatorSet.proposerBlockReward += tipCost(bytesPaid) | |||
``` | |||
|
|||
#### Begin Block |
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.
This spec assumes these computations will be done every block. Why not epoch 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 guess this is because this is how it works in the SDK currently. While we want epochs on the long run I'm against putting more effort into this now. It is "just" an optimization (yeah, a really cool one but I think there are higher prio ones).
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.
cosmos/cosmos-sdk#8328 😉 Should land in the sdk before launch. May be worth leaving a note that epochs are preferred.
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.
Nice! Thanks for bringing this to our attention.
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.
The only real benefit to epochs for our usecase is reducing light client overhead since fewer block headers need to be downloaded. It'll probably be important long-term, but it's just an optimization and we can worry about those later.
Fixes #95, #104.
Major changes:
stakedBalance
for validators. Validators now have no balance and need to delegate to themselves. This can be abstracted away at the UI layer.Currently validator commission rewards simply sit there until the validator unbonds. We should support delegating this reward #117.