feat!: throttle with retries provider changes (#1230)

* wip, tests not fixed yet * rm packet query on provider * rm unneeded UTs * rm tests from relay_test * rm query and more tests * rm more tests * builds again and rm debug tests * lint * fix handling of slash packet and integration test * Fix TestMultiConsumerSlashPacketThrottling * fix two more slashing integration tests * Update TestSlashRetries, cleanup neededc * cleaned up TestSlashRetries * UT for TestOnRecvDowntimeSlashPacket * cleans * use helper in throttle test * lintz * Revert "rm packet query on provider" This reverts commit a10a239. * cmd file too * fully restore query * Revert "Merge branch 'main' into shawn/throttle-with-retries-provider-changes" This reverts commit 73db33b, reversing changes made to 78a8269. * make e2e test pass, with todos * clean * Revert "Revert "Merge branch 'main' into shawn/throttle-with-retries-provider-changes"" This reverts commit 5bfccc3. * lint * Update CHANGELOG.md * slightly longer buffer * build(deps): bump actions/checkout from 3 to 4 (#1257) Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v3...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps)!: bump github.com/cosmos/ibc-go/v7 from 7.2.0 to 7.3.0 (#1258) * build(deps): bump github.com/cosmos/ibc-go/v7 from 7.2.0 to 7.3.0 Bumps [github.com/cosmos/ibc-go/v7](https://github.com/cosmos/ibc-go) from 7.2.0 to 7.3.0. - [Release notes](https://github.com/cosmos/ibc-go/releases) - [Changelog](https://github.com/cosmos/ibc-go/blob/main/CHANGELOG.md) - [Commits](cosmos/ibc-go@v7.2.0...v7.3.0) --- updated-dependencies: - dependency-name: github.com/cosmos/ibc-go/v7 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * add changelog entries --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: mpoke <marius.poke@posteo.de> * build(deps): bump github.com/cosmos/cosmos-sdk from 0.47.4 to 0.47.5 (#1259) * build(deps): bump github.com/cosmos/cosmos-sdk from 0.47.3 to 0.47.5 Bumps [github.com/cosmos/cosmos-sdk](https://github.com/cosmos/cosmos-sdk) from 0.47.3 to 0.47.5. - [Release notes](https://github.com/cosmos/cosmos-sdk/releases) - [Changelog](https://github.com/cosmos/cosmos-sdk/blob/main/CHANGELOG.md) - [Commits](cosmos/cosmos-sdk@v0.47.3...v0.47.5) --- updated-dependencies: - dependency-name: github.com/cosmos/cosmos-sdk dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * add changelog entries --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: mpoke <marius.poke@posteo.de> * chore: Separate semver (#1217) separate semver * docs: cleanup changelog (#1260) fix changelog * fix!: validate MsgTransfer before calling Transfer() (#1244) * validate MsgTransfer * add TestSendRewardsToProvider * update DefaultConsumerUnbondingPeriod to 14 days * update changelog * fix linter * fix test * apply review suggestions * update changelog * docs: Create adr-012-separate-releasing.md (#1229) * Create adr-011-separate-releasing.md * Update docs/docs/adrs/adr-011-separate-releasing.md Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> * adr 12 not 11 * correct that we use postfix not prefix * explanation on example release flow --------- Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> * fix: remove addr validation for provider fee pool addr param (#1262) * fix: remove validation for provider chain address since we cannot validate it properly on consumer * add changelog entry * Revert "Merge branch 'main' into shawn/throttle-with-retries-provider-changes" This reverts commit 6bdfff9, reversing changes made to d8f5690. * fmt * Update steps_downtime.go * Update tests/e2e/steps_downtime.go Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> * waittime instead of sleep * Update x/ccv/provider/client/cli/query.go Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> * Update x/ccv/provider/client/cli/query.go Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> * rm throttled packets from query * rm provider query * whoopsies forgot to rm some boilerplate --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: mpoke <marius.poke@posteo.de> Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> Co-authored-by: Dmitry Kolupaev <dmitry.klpv@gmail.com>
cosmos · Sep 14, 2023 · 727e731 · 727e731
1 parent 0b1d429
commit 727e731
Show file tree

Hide file tree

Showing 24 changed files with 456 additions and 3,742 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 Add an entry to the unreleased provider section whenever merging a PR to main that is not targeted at a specific release. These entries will eventually be included in a provider release.
 
+* (feat!) [#1230](https://github.com/cosmos/interchain-security/pull/1230) Throttle with retries provider changes.
 * (feature!) [#1244](https://github.com/cosmos/interchain-security/pull/1244) Update the default consumer unbonding period to 2 weeks.
 * (deps) [#1259](https://github.com/cosmos/interchain-security/pull/1259) Bump [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) to [v0.47.5](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.5).
 * (deps!) [#1258](https://github.com/cosmos/interchain-security/pull/1258) Bump [ibc-go](https://github.com/cosmos/ibc-go) to [v7.3.0](https://github.com/cosmos/ibc-go/releases/tag/v7.3.0).

diff --git a/proto/interchain_security/ccv/provider/v1/query.proto b/proto/interchain_security/ccv/provider/v1/query.proto
@@ -65,14 +65,6 @@ service Query {
         "/interchain_security/ccv/provider/throttle_state";
   }
 
-  // QueryThrottledConsumerPacketData returns a list of pending packet data
-  // instances (slash packet and vsc matured) for a single consumer chain
-  rpc QueryThrottledConsumerPacketData(QueryThrottledConsumerPacketDataRequest)
-      returns (QueryThrottledConsumerPacketDataResponse) {
-    option (google.api.http).get =
-        "/interchain_security/ccv/provider/pending_consumer_packets";
-  }
-
   // QueryRegisteredConsumerRewardDenoms returns a list of consumer reward
   // denoms that are registered
   rpc QueryRegisteredConsumerRewardDenoms(
@@ -151,35 +143,6 @@ message QueryThrottleStateResponse {
   // full
   google.protobuf.Timestamp next_replenish_candidate = 3
       [ (gogoproto.stdtime) = true, (gogoproto.nullable) = false ];
-  // data relevant to currently throttled slash packets
-  repeated ThrottledSlashPacket packets = 4;
-}
-
-message QueryThrottledConsumerPacketDataRequest { string chain_id = 1; }
-
-message QueryThrottledConsumerPacketDataResponse {
-  string chain_id = 1;
-  uint64 size = 2;
-  repeated ThrottledPacketDataWrapper packetDataInstances = 3
-      [ (gogoproto.nullable) = false ];
-}
-
-// A query wrapper type for the global entry and data relevant to a throttled
-// slash packet.
-message ThrottledSlashPacket {
-  interchain_security.ccv.provider.v1.GlobalSlashEntry global_entry = 1
-      [ (gogoproto.nullable) = false ];
-  interchain_security.ccv.v1.SlashPacketData data = 2
-      [ (gogoproto.nullable) = false ];
-}
-
-// ThrottledPacketDataWrapper contains either SlashPacketData or
-// VSCMaturedPacketData
-message ThrottledPacketDataWrapper {
-  oneof data {
-    interchain_security.ccv.v1.SlashPacketData slash_packet = 1;
-    interchain_security.ccv.v1.VSCMaturedPacketData vsc_matured_packet = 2;
-  }
 }
 
 message QueryRegisteredConsumerRewardDenomsRequest {}

diff --git a/tests/e2e/actions.go b/tests/e2e/actions.go
@@ -1937,50 +1937,41 @@ func (tr TestRun) assignConsumerPubKey(action assignConsumerPubKeyAction, verbos
 	tr.waitBlocks(chainID("provi"), 2, 30*time.Second)
 }
 
-// slashThrottleDequeue polls slash queue sizes until nextQueueSize is achieved
-type slashThrottleDequeue struct {
-	chain            chainID
-	currentQueueSize int
-	nextQueueSize    int
+// slashMeterReplenishmentAction polls the slash meter on provider until value is achieved
+type slashMeterReplenishmentAction struct {
+	targetValue int64
 	// panic if timeout is exceeded
 	timeout time.Duration
 }
 
-func (tr TestRun) waitForSlashThrottleDequeue(
-	action slashThrottleDequeue,
+func (tr TestRun) waitForSlashMeterReplenishment(
+	action slashMeterReplenishmentAction,
 	verbose bool,
 ) {
 	timeout := time.Now().Add(action.timeout)
-	initialGlobalQueueSize := int(tr.getGlobalSlashQueueSize())
+	initialSlashMeter := tr.getSlashMeter()
 
-	if initialGlobalQueueSize != action.currentQueueSize {
-		panic(fmt.Sprintf("wrong initial queue size: %d - expected global queue: %d\n", initialGlobalQueueSize, action.currentQueueSize))
+	if initialSlashMeter >= 0 {
+		panic(fmt.Sprintf("No need to wait for slash meter replenishment, current value: %d", initialSlashMeter))
 	}
+
 	for {
-		globalQueueSize := int(tr.getGlobalSlashQueueSize())
-		chainQueueSize := int(tr.getConsumerChainPacketQueueSize(action.chain))
+		slashMeter := tr.getSlashMeter()
 		if verbose {
-			fmt.Printf("waiting for packed queue size to reach: %d - current: %d\n", action.nextQueueSize, globalQueueSize)
+			fmt.Printf("waiting for slash meter to be replenished, current value: %d\n", slashMeter)
 		}
 
-		// check if global queue size is equal to chain queue size
-		if globalQueueSize == chainQueueSize && globalQueueSize == action.nextQueueSize { //nolint:gocritic // this is the comparison that we want here.
+		// check if meter has reached target value
+		if slashMeter >= action.targetValue {
 			break
 		}
 
 		if time.Now().After(timeout) {
-			panic(fmt.Sprintf("\n\n\nwaitForSlashThrottleDequeuemethod has timed out after: %s\n\n", action.timeout))
+			panic(fmt.Sprintf("\n\nwaitForSlashMeterReplenishment has timed out after: %s\n\n", action.timeout))
 		}
 
-		time.Sleep(500 * time.Millisecond)
+		tr.WaitTime(5 * time.Second)
 	}
-	// wair for 2 blocks to be created
-	// allowing the jailing to be incorporated into voting power
-	tr.waitBlocks(action.chain, 2, time.Minute)
-}
-
-func uintPointer(i uint) *uint {
-	return &i
 }
 
 // GetPathNameForGorelayer returns the name of the path between two given chains used by Gorelayer.

diff --git a/tests/e2e/main.go b/tests/e2e/main.go
@@ -242,8 +242,8 @@ func (tr *TestRun) runStep(step Step, verbose bool) {
 		tr.registerRepresentative(action, verbose)
 	case assignConsumerPubKeyAction:
 		tr.assignConsumerPubKey(action, verbose)
-	case slashThrottleDequeue:
-		tr.waitForSlashThrottleDequeue(action, verbose)
+	case slashMeterReplenishmentAction:
+		tr.waitForSlashMeterReplenishment(action, verbose)
 	case startRelayerAction:
 		tr.startRelayer(action, verbose)
 	case registerConsumerRewardDenomAction:

diff --git a/tests/e2e/state.go b/tests/e2e/state.go
@@ -168,19 +168,6 @@ func (tr TestRun) getChainState(chain chainID, modelState ChainState) ChainState
 		chainState.ProviderKeys = &providerKeys
 	}
 
-	if modelState.GlobalSlashQueueSize != nil {
-		globalQueueSize := tr.getGlobalSlashQueueSize()
-		chainState.GlobalSlashQueueSize = &globalQueueSize
-	}
-
-	if modelState.ConsumerChainQueueSizes != nil {
-		consumerChainQueueSizes := map[chainID]uint{}
-		for c := range *modelState.ConsumerChainQueueSizes {
-			consumerChainQueueSizes[c] = tr.getConsumerChainPacketQueueSize(c)
-		}
-		chainState.ConsumerChainQueueSizes = &consumerChainQueueSizes
-	}
-
 	if modelState.RegisteredConsumerRewardDenoms != nil {
 		registeredConsumerRewardDenoms := tr.getRegisteredConsumerRewardDenoms(chain)
 		chainState.RegisteredConsumerRewardDenoms = &registeredConsumerRewardDenoms
@@ -667,9 +654,10 @@ func (tr TestRun) getProviderAddressFromConsumer(consumerChain chainID, validato
 	return addr
 }
 
-func (tr TestRun) getGlobalSlashQueueSize() uint {
+func (tr TestRun) getSlashMeter() int64 {
 	//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
-	cmd := exec.Command("docker", "exec", tr.containerConfig.instanceName, tr.chainConfigs[chainID("provi")].binaryName,
+	cmd := exec.Command("docker", "exec",
+		tr.containerConfig.instanceName, tr.chainConfigs[chainID("provi")].binaryName,
 
 		"query", "provider", "throttle-state",
 		`--node`, tr.getQueryNode(chainID("provi")),
@@ -680,26 +668,8 @@ func (tr TestRun) getGlobalSlashQueueSize() uint {
 		log.Fatal(err, "\n", string(bz))
 	}
 
-	packets := gjson.Get(string(bz), "packets").Array()
-	return uint(len(packets))
-}
-
-func (tr TestRun) getConsumerChainPacketQueueSize(consumerChain chainID) uint {
-	//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
-	cmd := exec.Command("docker", "exec", tr.containerConfig.instanceName, tr.chainConfigs[chainID("provi")].binaryName,
-
-		"query", "provider", "throttled-consumer-packet-data",
-		string(consumerChain),
-		`--node`, tr.getQueryNode(chainID("provi")),
-		`-o`, `json`,
-	)
-	bz, err := cmd.CombinedOutput()
-	if err != nil {
-		log.Fatal(err, "\n", string(bz))
-	}
-
-	size := gjson.Get(string(bz), "size").Uint()
-	return uint(size)
+	slashMeter := gjson.Get(string(bz), "slash_meter")
+	return slashMeter.Int()
 }
 
 func (tr TestRun) getRegisteredConsumerRewardDenoms(chain chainID) []string {

diff --git a/tests/e2e/steps_downtime.go b/tests/e2e/steps_downtime.go
@@ -56,7 +56,8 @@ func stepsDowntime(consumerName string) []Step {
 				chainID(consumerName): ChainState{
 					ValPowers: &map[validatorID]uint{
 						validatorID("alice"): 509,
-						validatorID("bob"):   500,
+						// Bob's stake may or may not be slashed at this point depending on comet vs cometmock
+						// See https://github.com/cosmos/interchain-security/issues/1304
 						validatorID("carol"): 501,
 					},
 				},
@@ -278,7 +279,7 @@ func stepsThrottledDowntime(consumerName string) []Step {
 				validator: validatorID("bob"),
 			},
 			state: State{
-				// slash packet queued on consumer, but powers not affected on either chain yet
+				// slash packet queued for bob on consumer, but powers not affected on either chain yet
 				chainID("provi"): ChainState{
 					ValPowers: &map[validatorID]uint{
 						validatorID("alice"): 511,
@@ -312,11 +313,6 @@ func stepsThrottledDowntime(consumerName string) []Step {
 						validatorID("bob"):   0, // bob is jailed
 						validatorID("carol"): 500,
 					},
-					// no provider throttling engaged yet
-					GlobalSlashQueueSize: uintPointer(0),
-					ConsumerChainQueueSizes: &map[chainID]uint{
-						chainID(consumerName): uint(0),
-					},
 				},
 				chainID(consumerName): ChainState{
 					// VSC packet applying jailing is not yet relayed to consumer
@@ -328,13 +324,13 @@ func stepsThrottledDowntime(consumerName string) []Step {
 				},
 			},
 		},
+		// Invoke carol downtime slash on consumer
 		{
 			action: downtimeSlashAction{
 				chain:     chainID(consumerName),
 				validator: validatorID("carol"),
 			},
 			state: State{
-				// powers not affected on either chain yet
 				chainID("provi"): ChainState{
 					ValPowers: &map[validatorID]uint{
 						validatorID("alice"): 511,
@@ -343,10 +339,9 @@ func stepsThrottledDowntime(consumerName string) []Step {
 					},
 				},
 				chainID(consumerName): ChainState{
-					// VSC packet applying jailing is not yet relayed to consumer
 					ValPowers: &map[validatorID]uint{
 						validatorID("alice"): 511,
-						validatorID("bob"):   500,
+						validatorID("bob"):   500, // VSC packet jailing bob is not yet relayed to consumer
 						validatorID("carol"): 500,
 					},
 				},
@@ -364,42 +359,35 @@ func stepsThrottledDowntime(consumerName string) []Step {
 					ValPowers: &map[validatorID]uint{
 						validatorID("alice"): 511,
 						validatorID("bob"):   0,
-						validatorID("carol"): 500, // not slashed due to throttling
-					},
-					GlobalSlashQueueSize: uintPointer(1), // carol's slash request is throttled
-					ConsumerChainQueueSizes: &map[chainID]uint{
-						chainID(consumerName): uint(1),
+						validatorID("carol"): 500, // slash packet for carol recv by provider, carol not slashed due to throttling
 					},
 				},
 				chainID(consumerName): ChainState{
 					ValPowers: &map[validatorID]uint{
 						validatorID("alice"): 511,
-						validatorID("bob"):   0,
+						validatorID("bob"):   0, // VSC packet applying bob jailing is also relayed and recv by consumer
 						validatorID("carol"): 500,
 					},
 				},
 			},
 		},
+		// TODO(Shawn): Improve this test to have the consumer retry it's downtime slash, and to assert queue size on consumer.
+		// See https://github.com/cosmos/interchain-security/issues/1103 and https://github.com/cosmos/interchain-security/issues/1233
 		{
-			action: slashThrottleDequeue{
-				chain:            chainID(consumerName),
-				currentQueueSize: 1,
-				nextQueueSize:    0,
+			action: slashMeterReplenishmentAction{
+				targetValue: 0, // We just want slash meter to be non-negative
+
 				// Slash meter replenish fraction is set to 10%, replenish period is 20 seconds, see config.go
 				// Meter is initially at 10%, decremented to -23% from bob being jailed. It'll then take three replenishments
-				// for meter to become positive again. 3*20 = 60 seconds + buffer = 80 seconds
-				timeout: 80 * time.Second,
+				// for meter to become positive again. 3*20 = 60 seconds + buffer = 100 seconds
+				timeout: 100 * time.Second,
 			},
 			state: State{
 				chainID("provi"): ChainState{
 					ValPowers: &map[validatorID]uint{
 						validatorID("alice"): 511,
 						validatorID("bob"):   0,
-						validatorID("carol"): 0, // Carol is jailed upon packet being handled on provider
-					},
-					GlobalSlashQueueSize: uintPointer(0), // slash packets dequeued
-					ConsumerChainQueueSizes: &map[chainID]uint{
-						chainID(consumerName): 0,
+						validatorID("carol"): 500, // Carol still not slashed, packet must be retried
 					},
 				},
 				chainID(consumerName): ChainState{
@@ -412,36 +400,5 @@ func stepsThrottledDowntime(consumerName string) []Step {
 				},
 			},
 		},
-		// A block is incremented each action, hence why VSC is committed on provider,
-		// and can now be relayed as packet to consumer
-		{
-			action: relayPacketsAction{
-				chainA:  chainID("provi"),
-				chainB:  chainID(consumerName),
-				port:    "provider",
-				channel: 0,
-			},
-			state: State{
-				chainID("provi"): ChainState{
-					ValPowers: &map[validatorID]uint{
-						validatorID("alice"): 511,
-						validatorID("bob"):   0,
-						validatorID("carol"): 0,
-					},
-					GlobalSlashQueueSize: uintPointer(0),
-					ConsumerChainQueueSizes: &map[chainID]uint{
-						chainID(consumerName): 0,
-					},
-				},
-				chainID(consumerName): ChainState{
-					ValPowers: &map[validatorID]uint{
-						validatorID("alice"): 511,
-						// throttled update gets to consumer
-						validatorID("bob"):   0,
-						validatorID("carol"): 0,
-					},
-				},
-			},
-		},
 	}
 }