From fde751eb4acdf212490e6d5af6069b6033085148 Mon Sep 17 00:00:00 2001 From: insumity Date: Thu, 18 Jul 2024 21:08:06 +0300 Subject: [PATCH] fix!: change UX in key assignment (#1998) * init * added nit changes * added CHANGELOGs * fixed E2E tests error message * brought one more test from #1732 * fixed logging * fixed event key * fixed CHANGELOGs * rebase --- .../1998-change-ux-in-key-assignment.md | 3 + .../1998-change-ux-in-key-assignment.md | 3 + tests/e2e/steps_compatibility.go | 12 ++++ tests/e2e/steps_start_chains.go | 18 ++---- .../consumer-double-sign.json | 30 ++-------- .../e2e/tracehandler_testdata/democracy.json | 30 ++-------- .../democracyRewardsSteps.json | 30 ++-------- .../e2e/tracehandler_testdata/happyPath.json | 30 ++-------- .../multipleConsumers.json | 60 +++---------------- .../e2e/tracehandler_testdata/shorthappy.json | 30 ++-------- .../tracehandler_testdata/slashThrottle.json | 30 ++-------- tests/integration/key_assignment.go | 8 +-- x/ccv/provider/handler_test.go | 30 +++++++++- x/ccv/provider/keeper/key_assignment.go | 25 +++----- x/ccv/provider/keeper/msg_server.go | 4 +- 15 files changed, 96 insertions(+), 247 deletions(-) create mode 100644 .changelog/unreleased/api-breaking/provider/1998-change-ux-in-key-assignment.md create mode 100644 .changelog/unreleased/state-breaking/provider/1998-change-ux-in-key-assignment.md diff --git a/.changelog/unreleased/api-breaking/provider/1998-change-ux-in-key-assignment.md b/.changelog/unreleased/api-breaking/provider/1998-change-ux-in-key-assignment.md new file mode 100644 index 0000000000..094583d465 --- /dev/null +++ b/.changelog/unreleased/api-breaking/provider/1998-change-ux-in-key-assignment.md @@ -0,0 +1,3 @@ +- Change the UX in key assignment by returning an error if a validator tries to + reuse the same consumer key. +([\#1998](https://github.com/cosmos/interchain-security/pull/1998)) diff --git a/.changelog/unreleased/state-breaking/provider/1998-change-ux-in-key-assignment.md b/.changelog/unreleased/state-breaking/provider/1998-change-ux-in-key-assignment.md new file mode 100644 index 0000000000..094583d465 --- /dev/null +++ b/.changelog/unreleased/state-breaking/provider/1998-change-ux-in-key-assignment.md @@ -0,0 +1,3 @@ +- Change the UX in key assignment by returning an error if a validator tries to + reuse the same consumer key. +([\#1998](https://github.com/cosmos/interchain-security/pull/1998)) diff --git a/tests/e2e/steps_compatibility.go b/tests/e2e/steps_compatibility.go index dfcf14cd58..2cf0513596 100644 --- a/tests/e2e/steps_compatibility.go +++ b/tests/e2e/steps_compatibility.go @@ -89,6 +89,18 @@ func compstepsStartConsumerChain(consumerName string, proposalIndex, chainIndex }, }, }, + { + // op should fail - key already assigned by the same validator + Action: AssignConsumerPubKeyAction{ + Chain: ChainID(consumerName), + Validator: ValidatorID("carol"), + ConsumerPubkey: getDefaultValidators()[ValidatorID("carol")].ConsumerValPubKey, + ReconfigureNode: false, + ExpectError: true, + ExpectedError: "a validator has assigned the consumer key already: consumer key is already in use by a validator", + }, + State: State{}, + }, { // op should fail - key already assigned by another validator Action: AssignConsumerPubKeyAction{ diff --git a/tests/e2e/steps_start_chains.go b/tests/e2e/steps_start_chains.go index 898f562dc0..c8cfdaf212 100644 --- a/tests/e2e/steps_start_chains.go +++ b/tests/e2e/steps_start_chains.go @@ -86,24 +86,16 @@ func stepsStartConsumerChain(consumerName string, proposalIndex, chainIndex uint }, }, { - // op should be a noop - key already assigned, but by the same validator + // op should fail - key already assigned by the same validator Action: AssignConsumerPubKeyAction{ Chain: ChainID(consumerName), Validator: ValidatorID("carol"), ConsumerPubkey: getDefaultValidators()[ValidatorID("carol")].ConsumerValPubKey, ReconfigureNode: false, - ExpectError: false, - }, - State: State{ - ChainID(consumerName): ChainState{ - AssignedKeys: &map[ValidatorID]string{ - ValidatorID("carol"): getDefaultValidators()[ValidatorID("carol")].ConsumerValconsAddressOnProvider, - }, - ProviderKeys: &map[ValidatorID]string{ - ValidatorID("carol"): getDefaultValidators()[ValidatorID("carol")].ValconsAddress, - }, - }, + ExpectError: true, + ExpectedError: "a validator has or had assigned this consumer key already", }, + State: State{}, }, { // op should fail - key already assigned by another validator @@ -114,7 +106,7 @@ func stepsStartConsumerChain(consumerName string, proposalIndex, chainIndex uint ConsumerPubkey: getDefaultValidators()[ValidatorID("carol")].ConsumerValPubKey, ReconfigureNode: false, ExpectError: true, - ExpectedError: "a validator has assigned the consumer key already: consumer key is already in use by a validator", + ExpectedError: "a validator has or had assigned this consumer key already", }, State: State{ ChainID(consumerName): ChainState{ diff --git a/tests/e2e/tracehandler_testdata/consumer-double-sign.json b/tests/e2e/tracehandler_testdata/consumer-double-sign.json index f2696cb70c..a12b429736 100644 --- a/tests/e2e/tracehandler_testdata/consumer-double-sign.json +++ b/tests/e2e/tracehandler_testdata/consumer-double-sign.json @@ -145,32 +145,10 @@ "Validator": "carol", "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, - "ExpectError": false, - "ExpectedError": "" + "ExpectError": true, + "ExpectedError": "a validator has or had assigned this consumer key already" }, - "State": { - "consu": { - "ValBalances": null, - "ProposedConsumerChains": null, - "ValPowers": null, - "StakedTokens": null, - "IBCTransferParams": null, - "Params": null, - "Rewards": null, - "ConsumerChains": null, - "AssignedKeys": { - "carol": "cosmosvalcons1kswr5sq599365kcjmhgufevfps9njf43e4lwdk" - }, - "ProviderKeys": { - "carol": "cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6" - }, - "ConsumerPendingPacketQueueSize": null, - "RegisteredConsumerRewardDenoms": null, - "ClientsFrozenHeights": null, - "HasToValidate": null, - "Proposals": null - } - } + "State": {} }, { "ActionType": "main.AssignConsumerPubKeyAction", @@ -180,7 +158,7 @@ "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, "ExpectError": true, - "ExpectedError": "a validator has assigned the consumer key already: consumer key is already in use by a validator" + "ExpectedError": "a validator has or had assigned this consumer key already" }, "State": { "consu": { diff --git a/tests/e2e/tracehandler_testdata/democracy.json b/tests/e2e/tracehandler_testdata/democracy.json index b1431d0ee8..0a2c8bfd00 100644 --- a/tests/e2e/tracehandler_testdata/democracy.json +++ b/tests/e2e/tracehandler_testdata/democracy.json @@ -145,32 +145,10 @@ "Validator": "carol", "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, - "ExpectError": false, - "ExpectedError": "" + "ExpectError": true, + "ExpectedError": "a validator has or had assigned this consumer key already" }, - "State": { - "democ": { - "ValBalances": null, - "ProposedConsumerChains": null, - "ValPowers": null, - "StakedTokens": null, - "IBCTransferParams": null, - "Params": null, - "Rewards": null, - "ConsumerChains": null, - "AssignedKeys": { - "carol": "cosmosvalcons1kswr5sq599365kcjmhgufevfps9njf43e4lwdk" - }, - "ProviderKeys": { - "carol": "cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6" - }, - "ConsumerPendingPacketQueueSize": null, - "RegisteredConsumerRewardDenoms": null, - "ClientsFrozenHeights": null, - "HasToValidate": null, - "Proposals": null - } - } + "State": {} }, { "ActionType": "main.AssignConsumerPubKeyAction", @@ -180,7 +158,7 @@ "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, "ExpectError": true, - "ExpectedError": "a validator has assigned the consumer key already: consumer key is already in use by a validator" + "ExpectedError": "a validator has or had assigned this consumer key already" }, "State": { "democ": { diff --git a/tests/e2e/tracehandler_testdata/democracyRewardsSteps.json b/tests/e2e/tracehandler_testdata/democracyRewardsSteps.json index 0c96ba328e..6ed185c046 100644 --- a/tests/e2e/tracehandler_testdata/democracyRewardsSteps.json +++ b/tests/e2e/tracehandler_testdata/democracyRewardsSteps.json @@ -145,32 +145,10 @@ "Validator": "carol", "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, - "ExpectError": false, - "ExpectedError": "" + "ExpectError": true, + "ExpectedError": "a validator has or had assigned this consumer key already" }, - "State": { - "democ": { - "ValBalances": null, - "ProposedConsumerChains": null, - "ValPowers": null, - "StakedTokens": null, - "IBCTransferParams": null, - "Params": null, - "Rewards": null, - "ConsumerChains": null, - "AssignedKeys": { - "carol": "cosmosvalcons1kswr5sq599365kcjmhgufevfps9njf43e4lwdk" - }, - "ProviderKeys": { - "carol": "cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6" - }, - "ConsumerPendingPacketQueueSize": null, - "RegisteredConsumerRewardDenoms": null, - "ClientsFrozenHeights": null, - "HasToValidate": null, - "Proposals": null - } - } + "State": {} }, { "ActionType": "main.AssignConsumerPubKeyAction", @@ -180,7 +158,7 @@ "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, "ExpectError": true, - "ExpectedError": "a validator has assigned the consumer key already: consumer key is already in use by a validator" + "ExpectedError": "a validator has or had assigned this consumer key already" }, "State": { "democ": { diff --git a/tests/e2e/tracehandler_testdata/happyPath.json b/tests/e2e/tracehandler_testdata/happyPath.json index 566ebbfe24..dee231b5cb 100644 --- a/tests/e2e/tracehandler_testdata/happyPath.json +++ b/tests/e2e/tracehandler_testdata/happyPath.json @@ -145,32 +145,10 @@ "Validator": "carol", "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, - "ExpectError": false, - "ExpectedError": "" + "ExpectError": true, + "ExpectedError": "a validator has or had assigned this consumer key already" }, - "State": { - "consu": { - "ValBalances": null, - "ProposedConsumerChains": null, - "ValPowers": null, - "StakedTokens": null, - "IBCTransferParams": null, - "Params": null, - "Rewards": null, - "ConsumerChains": null, - "AssignedKeys": { - "carol": "cosmosvalcons1kswr5sq599365kcjmhgufevfps9njf43e4lwdk" - }, - "ProviderKeys": { - "carol": "cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6" - }, - "ConsumerPendingPacketQueueSize": null, - "RegisteredConsumerRewardDenoms": null, - "ClientsFrozenHeights": null, - "HasToValidate": null, - "Proposals": null - } - } + "State": {} }, { "ActionType": "main.AssignConsumerPubKeyAction", @@ -180,7 +158,7 @@ "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, "ExpectError": true, - "ExpectedError": "a validator has assigned the consumer key already: consumer key is already in use by a validator" + "ExpectedError": "a validator has or had assigned this consumer key already" }, "State": { "consu": { diff --git a/tests/e2e/tracehandler_testdata/multipleConsumers.json b/tests/e2e/tracehandler_testdata/multipleConsumers.json index 1e4bbc3ed3..5ae656f392 100644 --- a/tests/e2e/tracehandler_testdata/multipleConsumers.json +++ b/tests/e2e/tracehandler_testdata/multipleConsumers.json @@ -145,32 +145,10 @@ "Validator": "carol", "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, - "ExpectError": false, - "ExpectedError": "" + "ExpectError": true, + "ExpectedError": "a validator has or had assigned this consumer key already" }, - "State": { - "consu": { - "ValBalances": null, - "ProposedConsumerChains": null, - "ValPowers": null, - "StakedTokens": null, - "IBCTransferParams": null, - "Params": null, - "Rewards": null, - "ConsumerChains": null, - "AssignedKeys": { - "carol": "cosmosvalcons1kswr5sq599365kcjmhgufevfps9njf43e4lwdk" - }, - "ProviderKeys": { - "carol": "cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6" - }, - "ConsumerPendingPacketQueueSize": null, - "RegisteredConsumerRewardDenoms": null, - "ClientsFrozenHeights": null, - "HasToValidate": null, - "Proposals": null - } - } + "State": {} }, { "ActionType": "main.AssignConsumerPubKeyAction", @@ -180,7 +158,7 @@ "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, "ExpectError": true, - "ExpectedError": "a validator has assigned the consumer key already: consumer key is already in use by a validator" + "ExpectedError": "a validator has or had assigned this consumer key already" }, "State": { "consu": { @@ -449,32 +427,10 @@ "Validator": "carol", "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, - "ExpectError": false, - "ExpectedError": "" + "ExpectError": true, + "ExpectedError": "a validator has or had assigned this consumer key already" }, - "State": { - "densu": { - "ValBalances": null, - "ProposedConsumerChains": null, - "ValPowers": null, - "StakedTokens": null, - "IBCTransferParams": null, - "Params": null, - "Rewards": null, - "ConsumerChains": null, - "AssignedKeys": { - "carol": "cosmosvalcons1kswr5sq599365kcjmhgufevfps9njf43e4lwdk" - }, - "ProviderKeys": { - "carol": "cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6" - }, - "ConsumerPendingPacketQueueSize": null, - "RegisteredConsumerRewardDenoms": null, - "ClientsFrozenHeights": null, - "HasToValidate": null, - "Proposals": null - } - } + "State": {} }, { "ActionType": "main.AssignConsumerPubKeyAction", @@ -484,7 +440,7 @@ "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, "ExpectError": true, - "ExpectedError": "a validator has assigned the consumer key already: consumer key is already in use by a validator" + "ExpectedError": "a validator has or had assigned this consumer key already" }, "State": { "densu": { diff --git a/tests/e2e/tracehandler_testdata/shorthappy.json b/tests/e2e/tracehandler_testdata/shorthappy.json index d4cfe8c0c5..424564f54f 100644 --- a/tests/e2e/tracehandler_testdata/shorthappy.json +++ b/tests/e2e/tracehandler_testdata/shorthappy.json @@ -145,32 +145,10 @@ "Validator": "carol", "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, - "ExpectError": false, - "ExpectedError": "" + "ExpectError": true, + "ExpectedError": "a validator has or had assigned this consumer key already" }, - "State": { - "consu": { - "ValBalances": null, - "ProposedConsumerChains": null, - "ValPowers": null, - "StakedTokens": null, - "IBCTransferParams": null, - "Params": null, - "Rewards": null, - "ConsumerChains": null, - "AssignedKeys": { - "carol": "cosmosvalcons1kswr5sq599365kcjmhgufevfps9njf43e4lwdk" - }, - "ProviderKeys": { - "carol": "cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6" - }, - "ConsumerPendingPacketQueueSize": null, - "RegisteredConsumerRewardDenoms": null, - "ClientsFrozenHeights": null, - "HasToValidate": null, - "Proposals": null - } - } + "State": {} }, { "ActionType": "main.AssignConsumerPubKeyAction", @@ -180,7 +158,7 @@ "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, "ExpectError": true, - "ExpectedError": "a validator has assigned the consumer key already: consumer key is already in use by a validator" + "ExpectedError": "a validator has or had assigned this consumer key already" }, "State": { "consu": { diff --git a/tests/e2e/tracehandler_testdata/slashThrottle.json b/tests/e2e/tracehandler_testdata/slashThrottle.json index 513d16118a..f4495c5840 100644 --- a/tests/e2e/tracehandler_testdata/slashThrottle.json +++ b/tests/e2e/tracehandler_testdata/slashThrottle.json @@ -145,32 +145,10 @@ "Validator": "carol", "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, - "ExpectError": false, - "ExpectedError": "" + "ExpectError": true, + "ExpectedError": "a validator has or had assigned this consumer key already" }, - "State": { - "consu": { - "ValBalances": null, - "ProposedConsumerChains": null, - "ValPowers": null, - "StakedTokens": null, - "IBCTransferParams": null, - "Params": null, - "Rewards": null, - "ConsumerChains": null, - "AssignedKeys": { - "carol": "cosmosvalcons1kswr5sq599365kcjmhgufevfps9njf43e4lwdk" - }, - "ProviderKeys": { - "carol": "cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6" - }, - "ConsumerPendingPacketQueueSize": null, - "RegisteredConsumerRewardDenoms": null, - "ClientsFrozenHeights": null, - "HasToValidate": null, - "Proposals": null - } - } + "State": {} }, { "ActionType": "main.AssignConsumerPubKeyAction", @@ -180,7 +158,7 @@ "ConsumerPubkey": "{\"@type\":\"/cosmos.crypto.ed25519.PubKey\",\"key\":\"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is=\"}", "ReconfigureNode": false, "ExpectError": true, - "ExpectedError": "a validator has assigned the consumer key already: consumer key is already in use by a validator" + "ExpectedError": "a validator has or had assigned this consumer key already" }, "State": { "consu": { diff --git a/tests/integration/key_assignment.go b/tests/integration/key_assignment.go index 8d8826daf9..c6478d98ba 100644 --- a/tests/integration/key_assignment.go +++ b/tests/integration/key_assignment.go @@ -121,10 +121,10 @@ func (s *CCVTestSuite) TestKeyAssignment() { s.nextEpoch() return nil - }, false, 2, + }, true, 2, }, { - "double key assignment in same block", func(pk *providerkeeper.Keeper) error { + "double key assignment in same block by same val", func(pk *providerkeeper.Keeper) error { // establish CCV channel s.SetupCCVChannel(s.path) @@ -191,10 +191,10 @@ func (s *CCVTestSuite) TestKeyAssignment() { s.nextEpoch() return nil - }, false, 2, + }, true, 2, }, { - "double key assignment in different blocks", func(pk *providerkeeper.Keeper) error { + "double key assignment in different blocks by same val", func(pk *providerkeeper.Keeper) error { // establish CCV channel s.SetupCCVChannel(s.path) diff --git a/x/ccv/provider/handler_test.go b/x/ccv/provider/handler_test.go index ffb3614e17..9c14efe91b 100644 --- a/x/ccv/provider/handler_test.go +++ b/x/ccv/provider/handler_test.go @@ -128,7 +128,7 @@ func TestAssignConsensusKeyMsgHandling(t *testing.T) { chainID: "chainid", }, { - name: "success: consumer key in use, but by the same validator", + name: "fail: consumer key in use by the same validator", setup: func(ctx sdk.Context, k keeper.Keeper, mocks testkeeper.MockedKeepers, ) { @@ -147,7 +147,33 @@ func TestAssignConsensusKeyMsgHandling(t *testing.T) { ).Return(stakingtypes.Validator{}, stakingtypes.ErrNoValidatorFound), ) }, - expError: false, + expError: true, + chainID: "chainid", + }, + { + name: "fail: consumer key in use by other validator", + setup: func(ctx sdk.Context, + k keeper.Keeper, mocks testkeeper.MockedKeepers, + ) { + k.SetPendingConsumerAdditionProp(ctx, &providertypes.ConsumerAdditionProposal{ + ChainId: "chainid", + }) + + // Use the consumer key already used by some other validator + k.SetValidatorByConsumerAddr(ctx, "chainid", consumerConsAddr, providerConsAddr2) + + gomock.InOrder( + mocks.MockStakingKeeper.EXPECT().GetValidator( + ctx, providerCryptoId.SDKValOpAddress(), + // validator should not be missing + ).Return(providerCryptoId.SDKStakingValidator(), nil).Times(1), + mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(ctx, + consumerConsAddr.ToSdkConsAddr(), + // return false - no other validator uses the consumer key to validate *on the provider* + ).Return(stakingtypes.Validator{}, stakingtypes.ErrNoValidatorFound), + ) + }, + expError: true, chainID: "chainid", }, } diff --git a/x/ccv/provider/keeper/key_assignment.go b/x/ccv/provider/keeper/key_assignment.go index 596c3be606..93bad8eb97 100644 --- a/x/ccv/provider/keeper/key_assignment.go +++ b/x/ccv/provider/keeper/key_assignment.go @@ -372,24 +372,13 @@ func (k Keeper) AssignConsumerKey( } } - if existingProviderAddr, found := k.GetValidatorByConsumerAddr(ctx, chainID, consumerAddr); found { - // consumer key is already in use - if providerAddr.Address.Equals(existingProviderAddr.Address) { - // the validator itself is the one already using the consumer key, - // just do a noop - k.Logger(ctx).Info("tried to assign a consumer key that is already assigned to the validator", - "consumer chainID", chainID, - "validator", providerAddr.String(), - "consumer consensus addr", consumerAddr.String(), - ) - return nil - } else { - // the validators are different -> throw an error to - // prevent multiple validators from assigning the same key - return errorsmod.Wrapf( - types.ErrConsumerKeyInUse, "a validator has assigned the consumer key already", - ) - } + if _, found := k.GetValidatorByConsumerAddr(ctx, chainID, consumerAddr); found { + // This consumer key is already in use, or it is to be pruned. With this check we prevent another validator + // from assigning the same consumer key as some other validator. Additionally, we prevent a validator from + // reusing a consumer key that it used in the past and is now to be pruned. + return errorsmod.Wrapf( + types.ErrConsumerKeyInUse, "a validator has or had assigned this consumer key already", + ) } // get the previous key assigned for this validator on this consumer chain diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go index 08429c55a8..50320e8196 100644 --- a/x/ccv/provider/keeper/msg_server.go +++ b/x/ccv/provider/keeper/msg_server.go @@ -73,14 +73,14 @@ func (k msgServer) AssignConsumerKey(goCtx context.Context, msg *types.MsgAssign k.Logger(ctx).Info("assigned consumer key", "consumer chainID", msg.ChainId, "validator operator addr", msg.ProviderAddr, - "consumer tm pubkey", consumerTMPublicKey.String(), + "consumer public key", msg.ConsumerKey, ) ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( types.EventTypeAssignConsumerKey, sdk.NewAttribute(types.AttributeProviderValidatorAddress, msg.ProviderAddr), - sdk.NewAttribute(types.AttributeConsumerConsensusPubKey, consumerTMPublicKey.String()), + sdk.NewAttribute(types.AttributeConsumerConsensusPubKey, msg.ConsumerKey), ), })