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
cosmos · Jul 18, 2024 · fde751e · fde751e
1 parent 360a0b0
commit fde751e
Show file tree

Hide file tree

Showing 15 changed files with 96 additions and 247 deletions.
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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": {