Throttle bug fixes + req refactors

tests/e2e/throttle.go

@@ -5,8 +5,10 @@ import (
 
 	sdktypes "github.com/cosmos/cosmos-sdk/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
+	channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
 	icstestingutils "github.com/cosmos/interchain-security/testutil/ibc_testing"
 	providertypes "github.com/cosmos/interchain-security/x/ccv/provider/types"
+	ccvtypes "github.com/cosmos/interchain-security/x/ccv/types"
 	tmtypes "github.com/tendermint/tendermint/types"
 )
 
@@ -339,6 +341,57 @@ func (s *CCVTestSuite) TestSlashMeterAllowanceChanges() {
 
 }
 
+// TestSlashSameValidator tests the edge case that that the total slashed validator power
+// queued up for a single block exceeds the slash meter allowance,
+// but some of the slash packets are for the same validator, and should all be handled.
+func (s *CCVTestSuite) TestSlashSameValidator() {
+
+	s.SetupAllCCVChannels()
+
+	// Setup 4 validators with 25% of the total power each.
+	s.setupValidatorPowers()
+
+	providerKeeper := s.providerApp.GetProviderKeeper()
+
+	// Set replenish fraction to 1.0 so that all sent packets should handled immediately (no throttling)
+	params := providerKeeper.GetParams(s.providerCtx())
+	params.SlashMeterReplenishFraction = "1.0"
+	providerKeeper.SetParams(s.providerCtx(), params)
+	providerKeeper.InitializeSlashMeter(s.providerCtx())
+
+	// Send a downtime and double-sign slash packet for 3/4 validators
+	// This will have a total slashing power of 150% total power.
+	tmval1 := s.providerChain.Vals.Validators[1]
+	tmval2 := s.providerChain.Vals.Validators[2]
+	tmval3 := s.providerChain.Vals.Validators[3]
+	s.setDefaultValSigningInfo(*tmval1)
+	s.setDefaultValSigningInfo(*tmval2)
+	s.setDefaultValSigningInfo(*tmval3)
+
+	packets := []channeltypes.Packet{
+		s.constructSlashPacketFromConsumer(s.getFirstBundle(), *tmval1, stakingtypes.Downtime, 1),
+		s.constructSlashPacketFromConsumer(s.getFirstBundle(), *tmval2, stakingtypes.Downtime, 2),
+		s.constructSlashPacketFromConsumer(s.getFirstBundle(), *tmval3, stakingtypes.Downtime, 3),
+		s.constructSlashPacketFromConsumer(s.getFirstBundle(), *tmval1, stakingtypes.DoubleSign, 4),
+		s.constructSlashPacketFromConsumer(s.getFirstBundle(), *tmval2, stakingtypes.DoubleSign, 5),
+		s.constructSlashPacketFromConsumer(s.getFirstBundle(), *tmval3, stakingtypes.DoubleSign, 6),
+	}
+
+	// Recv and queue all slash packets.
+	for _, packet := range packets {
+		providerKeeper.OnRecvSlashPacket(s.providerCtx(), packet, ccvtypes.MustUnmarshalJsonBzToSlashPacketData(packet.GetData()))
+	}
+
+	// We should have 6 pending slash packet entries queued.
+	s.Require().Len(providerKeeper.GetAllPendingSlashPacketEntries(s.providerCtx()), 6)
+
+	// Call next block to process all pending slash packets in end blocker.
+	s.providerChain.NextBlock()
+
+	// All slash packets should have been handled immediately, even though they totaled to 150% of total power.
+	s.Require().Len(providerKeeper.GetAllPendingSlashPacketEntries(s.providerCtx()), 0)
+}
+
 func (s *CCVTestSuite) confirmValidatorJailed(tmVal tmtypes.Validator) {
 	sdkVal, found := s.providerApp.GetE2eStakingKeeper().GetValidator(
 		s.providerCtx(), sdktypes.ValAddress(tmVal.Address))

tests/integration/config.go

@@ -300,7 +300,8 @@ func MultiConsumerTestRun() TestRun {
 					".app_state.slashing.params.signed_blocks_window = \"2\" | " +
 					".app_state.slashing.params.min_signed_per_window = \"0.500000000000000000\" | " +
 					".app_state.slashing.params.downtime_jail_duration = \"2s\" | " +
-					".app_state.slashing.params.slash_fraction_downtime = \"0.010000000000000000\"",
+					".app_state.slashing.params.slash_fraction_downtime = \"0.010000000000000000\" | " +
+					".app_state.provider.params.slash_meter_replenish_fraction = \"1.0\"",
 			},
 			chainID("consu"): {
 				chainId:        chainID("consu"),

tests/integration/steps_double_sign.go

@@ -3,8 +3,13 @@ package main
 // simulates double signing on provider and vsc propagation to consumer chains
 // steps continue from downtime tests state.
 //
-// Note: These steps are not affected by slash packet throttling since
-// only one consumer initiated slash is implemented.
+// Note: These steps ARE affected by slash packet throttling, since the
+// consumer-initiated slash steps are executed after consumer-initiated downtime
+// slashes have already occurred. However slash packet throttling is
+// psuedo-disabled in this test by setting the slash meter replenish
+// fraction to 1.0 in the config file.
+//
+// TODO: test throttling logic directly, https://github.com/cosmos/interchain-security/issues/509
 func stepsDoubleSign(consumer1, consumer2 string) []Step {
 	return []Step{
 		{

testutil/e2e/debug_test.go

@@ -168,6 +168,10 @@ func TestSlashMeterAllowanceChanges(t *testing.T) {
 	runCCVTestByName(t, "TestSlashMeterAllowanceChanges")
 }
 
+func TestSlashSameValidator(t *testing.T) {
+	runCCVTestByName(t, "TestSlashSameValidator")
+}
+
 //
 // Unbonding tests
 //

x/ccv/provider/keeper/throttle.go

@@ -29,9 +29,25 @@ func (k Keeper) HandlePendingSlashPackets(ctx sdktypes.Context) {
 	// Iterate through ordered (by received time) slash packet entries from any consumer chain
 	k.IteratePendingSlashPacketEntries(ctx, func(entry providertypes.SlashPacketEntry) (stop bool) {
 
+		// Obtain validator from consensus address.
+		// The slash packet validator address may be known only on the consumer chain
+		// in this case, it must be mapped back to the consensus address on the provider chain
+		consumerConsAddr := sdktypes.ConsAddress(entry.ValAddr)
+		providerConsAddr := k.GetProviderAddrFromConsumerAddr(ctx, entry.ConsumerChainID, consumerConsAddr)
+
+		// Note: if validator is not found or unbonded, this will be handled appropriately in HandleSlashPacket
+		val, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, providerConsAddr)
+
 		// Obtain the validator power relevant to the slash packet that's about to be handled
 		// (this power will be removed via jailing or tombstoning)
-		valPower := k.stakingKeeper.GetLastValidatorPower(ctx, entry.ValAddr)
+		var valPower int64
+		if val.IsJailed() || !found {
+			// If validator is jailed or not found, it's power is 0. This path is explicitly defined since the
+			// staking keeper's LastValidatorPower values are not updated till the staking keeper's endblocker.
+			valPower = 0
+		} else {
+			valPower = k.stakingKeeper.GetLastValidatorPower(ctx, val.GetOperator())
+		}
 
 		// Subtract this power from the slash meter
 		meter = meter.Sub(sdktypes.NewInt(valPower))

x/ccv/types/ccv.go

@@ -76,3 +76,10 @@ func (vdt SlashPacketData) GetBytes() []byte {
 	valDowntimeBytes := ModuleCdc.MustMarshalJSON(&vdt)
 	return valDowntimeBytes
 }
+
+// Unmarshals json bytes to SlashPacketData or panics if the bytes are invalid.
+func MustUnmarshalJsonBzToSlashPacketData(bz []byte) SlashPacketData {
+	data := SlashPacketData{}
+	ModuleCdc.MustUnmarshalJSON(bz, &data)
+	return data
+}