Cosmos-sdk: Ensure that unbonding-delegations/redelegations for removed validators are still slashable

[cwgoes]

Presently, if a validator is removed from the store, we won't slash any associated unbonding delegations. I think this is incorrect, since a validator will be removed when the last delegate starts unbonding from it - but we want the unbonding delegations to be slashable for an unbonding period even if the validator no longer has any power.

[rigelrozanski]

Yup - if a validator is removed we still need to slash for those unbonding/redelgation

[ebuchman]

Should this be in current iteration? I think it's state-machine breaking no?

[cwgoes]

Should this be in current iteration? I think it's state-machine breaking no?

It is - a bug, not a new feature, but we could delay if we want to avoid any state-machine breaking changes.

[ebuchman]

Ok, removed from current iteration. Let's try to keep this iteration to non-breaking fixes so we can push them out to gaia-7000. We'll pick up the breaking ones for the next iteration

[alexanderbez]

@cwgoes / @rigelrozanski can I work on this independently of the staking refactor? I imagine a good chunk of work will be in the slashing module.

[rigelrozanski]

I think that sounds like a good idea if you think you have a solid grasp on the problem. This is certainly something which needs to be fixed

[cwgoes]

I imagine a good chunk of work will be in the slashing module.

I think most of the work is just in the Slash() function, which is in the staking module but independent of the validator set logic.

[cwgoes]

This is rather annoying. At minimum, we have to keep a map from consensus address to operator address around so we can lookup the unbonding delegations / redelegations by the operator address.

I wonder if instead we can instead remove zero-power validators in the unbonding-handling queue whenever the last unbonding delegation or redelegation to that validator has been completed (at which point the validator won't be slashable since the unbonding period will have passed).

cc @rigelrozanski @sunnya97

[jaekwon]

I wonder if instead we can instead remove zero-power validators in the unbonding-handling queue whenever the last unbonding delegation or redelegation to that validator has been completed (at which point the validator won't be slashable since the unbonding period will have passed).

This appears to me like a good workaround, if it's easy we can do it for game-of-stakes, if not then later?

[rigelrozanski]

I wonder if instead we can instead remove zero-power validators in the unbonding-handling queue whenever the last unbonding delegation or redelegation to that validator has been completed (at which point the validator won't be slashable since the unbonding period will have passed).

I like the concept, introduces some new stuff:

• now we would need to track the number of existing unbonding-delegation/redelegation objects = not too bad, can actually be lumped into one counter.
• There will need to be slight modifications to unbonding/redelegation logic in staking, would need to have the old method IsUnbonded again (that sucks but not too bad)
• now we need to verify that the validator slashing mechanisms aren't negatively affected even if the status is left as unbonding but really everything has already been unbonded (I think we're already doing this right?)
  I think that's it - as long as you can verify my last point is correct (@cwgoes) then this sounds like a dandy idea.

---

Edit: I think there is an alternative simple solution which eliminates bullet 2 (and maybe bullet 3?):

• When the validator leaves the unbonding queue, it is delete if it's unbondings-redelegations counter is at zero and there is no voting power left
• If this validator does not have this counter at zero or still has voting power, the validator still leaves the queue, but the validator record still exists with the status of unbonded
• every time this counter is reduced, the we check if the validator is zero-power, if that is the case the validator object is permanently deleted

[cwgoes]

I think solution 2 above will work, with the slight modification that validators are only permanently deleted at counter-zero if they're already unbonded - if they're not, we know that they'll hit the end of the unbonding queue and be deleted then.

[rigelrozanski]

totally 🤙

[sunnya97]

I don't understand why you need to check that the unbonding/redelegations counter is 0 when the validator hits the end of the unbonding queue. I think you just need to check that the DelegatorShares is 0. Once the validator has finished unbonding queue, they're now in the unbonded state and are not going to get slashed anymore. So, what's the point of keeping the validator in the state? It's not necessary for the CompleteUnbonding or CompleteRedelegation.

You should be able to delete the validator once the last delegator has started unbonding, not once the last delegator has finished unbonding.

[cwgoes]

You should be able to delete the validator once the last delegator has started unbonding, not once the last delegator has finished unbonding.

At present, we need to keep the validator in the state so that we can look it up if a delegator (who has started unbonding) needs to be slashed, because (at minimum) we need to lookup the operator address by the consensus address.

[rigelrozanski]

I think you just need to check that the DelegatorShares is 0

But when you redelegate or unbond - shares are removed from the validator - we need a reference that the validator needs to be slashed

So, what's the point of keeping the validator in the state?

So there is a reference point to look for undelegations/redelegations records to slash

[sunnya97]

But when a delegator starts unbonding from an unbonding validator, their ubd gets the same FinishTime as the valid ator's unbonding time. Once a validator has been completely unbonded, there's no reason for their delegators to still be in the unbonding period, and thus they're not slashable. Correct me if I'm wrong, but slashes to ubds/reds happen as they come in, not when they hit the end of the unbonding queue.

[cwgoes]

Slashes would happen when they are in the unbonding queue but have not yet been removed from it. While they are in the queue (and within the bond period) we need to be able to look up the validator to potentially slash the unbonding delegations or redelegations.

[sunnya97]

Seems this hasn't been 100% resolved. Finishing this in #2676

[cwgoes]

Closed by #2676 et al.