Consumer sends invalid validator updates to tendermint, resulting in tm.verifyRemovals fail

[danwt]

This tendermint code checks if the validator set updates returned in EndBlock are valid

https://github.com/tendermint/tendermint/blob/85870def7b628effad73af942e638bbddf2ba8fd/types/validator_set.go#L538-L544

func verifyRemovals(deletes []*Validator, vals *ValidatorSet) (votingPower int64, err error) {
	removedVotingPower := int64(0)
	for _, valUpdate := range deletes {
		address := valUpdate.Address
		_, val := vals.GetByAddress(address)
		if val == nil {
			return removedVotingPower, fmt.Errorf("failed to find validator %X to remove", address)

The check can fail on consumer because the passed updates are not filtered correctly.

interchain-security/x/ccv/consumer/module.go

Lines 187 to 195 in 657aaea

	data, ok := am.keeper.GetPendingChanges(ctx)
	if !ok {
	return []abci.ValidatorUpdate{}
	}
	// apply changes to cross-chain validator set
	am.keeper.ApplyCCValidatorChanges(ctx, data.ValidatorUpdates)
	am.keeper.DeletePendingChanges(ctx)
	return data.ValidatorUpdates

The data needs to be modified in here

interchain-security/x/ccv/consumer/keeper/validators.go

Lines 16 to 52 in 657aaea

	func (k Keeper) ApplyCCValidatorChanges(ctx sdk.Context, changes []abci.ValidatorUpdate) {
	for _, change := range changes {
	addr := utils.GetChangePubKeyAddress(change)
	val, found := k.GetCCValidator(ctx, addr)
	// set new validator bonded
	if !found {
	if 0 < change.Power {
	consAddr := sdk.ConsAddress(addr)
	// convert validator pubkey from TM proto to SDK crytpo type
	pubkey, err := cryptocodec.FromTmProtoPublicKey(change.GetPubKey())
	if err != nil {
	panic(err)
	}
	ccVal, err := types.NewCCValidator(addr, change.Power, pubkey)
	if err != nil {
	panic(err)
	}
	k.SetCCValidator(ctx, ccVal)
	k.AfterValidatorBonded(ctx, consAddr, nil)
	}
	continue
	}
	// remove unbonding existing-validators
	if change.Power < 1 {
	k.DeleteCCValidator(ctx, addr)
	continue
	}
	// update existing validators power
	val.Power = change.Power
	k.SetCCValidator(ctx, val)
	}
	}

See the very closely related spec issue that was fixed already

• ICS28: system aborted due to update aggregation logic ibc#743

I can make a PR for this soon.

Found with diff testing.

[sainoe]

The check can fail on consumer because the passed updates are not filtered correctly.

So Tendermint errors when ValUpdates contains a change that has an unexisting validator in the consumer valset and a voting power equals to 0 ?

[danwt]

Yes. Tendermint interprets this as trying to delete a non-existing validator.

The pattern is:

A. Send valUpdate(val=val0, power=0) to tendermint (consumer side)
B. Send valUpdate(val=val0, power=1) to consumer (but consumer does not process it)
C. Send valUpdate(val=val0, power=0) to consumer (but consumer does not process it)
D. Consumer process updates from B,C. They are aggregated to valUpdate(val=val0, power=0)
E. Consumer sends update from D to tendermint