cosmos / interchain-security

Interchain Security is an open sourced IBC application which allows cosmos blockchains to lease their proof-of-stake security to one another.
https://cosmos.github.io/interchain-security/
Other
156 stars 126 forks source link

Provider should never slash unbonded validator #197

Closed danwt closed 2 years ago

danwt commented 2 years ago

The provider will panic if it receives a Slash action for an unbonded validator

func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeight int64, power int64, slashFactor sdk.Dec) math.Int {
    logger := k.Logger(ctx)

    if slashFactor.IsNegative() {
        panic(fmt.Errorf("attempted to slash with a negative slash factor: %v", slashFactor))
    }

    // Amount of slashing = slash slashFactor * power at time of infraction
    amount := k.TokensFromConsensusPower(ctx, power)
    slashAmountDec := sdk.NewDecFromInt(amount).Mul(slashFactor)
    slashAmount := slashAmountDec.TruncateInt()

    // ref https://github.com/cosmos/cosmos-sdk/issues/1348

    validator, found := k.GetValidatorByConsAddr(ctx, consAddr)
    if !found {
        // If not found, the validator must have been overslashed and removed - so we don't need to do anything
        // NOTE:  Correctness dependent on invariant that unbonding delegations / redelegations must also have been completely
        //        slashed in this case - which we don't explicitly check, but should be true.
        // Log the slash attempt for future reference (maybe we should tag it too)
        logger.Error(
            "WARNING: ignored attempt to slash a nonexistent validator; we recommend you investigate immediately",
            "validator", consAddr.String(),
        )
        return sdk.NewInt(0)
    }

    // should not be slashing an unbonded validator
    if validator.IsUnbonded() {
        panic(fmt.Sprintf("should not be slashing unbonded validator: %s", validator.GetOperator()))

(https://github.com/cosmos/cosmos-sdk/blob/0cc82cf25da7720cd3bd6ea658d6edf1ee038de1/x/staking/keeper/slash.go#L26-L55)

This is guarded in cosmos-sdk/x/evidence/keeper/infraction.go::HandleEquivocationEvidence

https://github.com/cosmos/cosmos-sdk/blob/0cc82cf25da7720cd3bd6ea658d6edf1ee038de1/x/evidence/keeper/infraction.go#L67-L70

but we must protect against it in interchain security.

I think this is already protected implicitly if the consumer only sends slash requests for validators bonded on his side. If a slash request reaches the provider then the validator is BONDED or UNBONDING, otherwise a maturation arrived before the slash request. This is enforced by the channel being ordered.

Still, I wonder if we can clarify this in the spec somehow. We already have Slashable Consumer Misbehavior but this is a subtly different (liveness) property.

I will also think how to test it.

danwt commented 2 years ago

Closed as probably a bit overkill, and mostly addressed by

https://github.com/cosmos/interchain-security/blob/fbb81998f97720e0b22a100ddbff50c6494c2918/x/ccv/provider/keeper/relay.go#L182