PSS: Error when opting in with your already-assigned key

[p-offtermatt]

Problem

Thanks to @dasanchez for surfacing!

If validator V first assigns a consumer key for a chain C (but is not opted in),
and then V opts into C, there are two natural ways:

1. opt in specifying the already assigned key again
2. opt in without specifying a consumer key (and using the already assigned key implicitly)

Right now, 2) works, but 1) throws an error:

Error: rpc error: code = Unknown desc = rpc error: code = Unknown desc = failed to execute message; message index: 0: a validator has assigned the consumer key already: consumer key is already in use by a validator [/home/runner/work/cosmos-builds/cosmos-builds/interchain-security/x/ccv/provider/keeper/key_assignment.go:377] With gas wanted: '18446744073709551615' and gas used: '38919' : unknown request

I think 1) should not throw an error, and just behave like 2).

Closing criteria

We either take care to document that 1) is not intended, or we make 1) behave like 2).

Problem details

The problem is here:

Optin in assigns a consumer key

interchain-security/x/ccv/provider/keeper/partial_set_security.go

Line 34 in 8ed60ee

err = k.AssignConsumerKey(ctx, chainID, validator, consumerTMPublicKey)

Assignment checks whether the consumer key is in use by any validator:

interchain-security/x/ccv/provider/keeper/key_assignment.go

Lines 374 to 380 in 8ed60ee

	if _, found := k.GetValidatorByConsumerAddr(ctx, chainID, consumerAddr); found {
	// consumer key is already in use
	// prevent multiple validators from assigning the same key
	return errorsmod.Wrapf(
	types.ErrConsumerKeyInUse, "a validator has assigned the consumer key already",
	)
	}

My preferred solution would be to adjust

interchain-security/x/ccv/provider/keeper/key_assignment.go

Lines 374 to 380 in 8ed60ee

	if _, found := k.GetValidatorByConsumerAddr(ctx, chainID, consumerAddr); found {
	// consumer key is already in use
	// prevent multiple validators from assigning the same key
	return errorsmod.Wrapf(
	types.ErrConsumerKeyInUse, "a validator has assigned the consumer key already",
	)
	}

to only throw an error if the validator that has already assigned the consumer key is not the same as the one now trying to assign the key, and have it act as a noop otherwise. I think it is slightly weird to have the pure key assignment tx sometimes be a noop without failing, but I think it shouldn't be an issue with gas or anything