perf!: Make slashing not write sign info every block (SDK: cosmos#19458

) (#543)

* perf!: Make slashing not write sign info every block (SDK: cosmos#19458)

cosmos#19458

* change NewSignInfo API

* Bring back String() change

* Fix test

* Another test fix that didn't get committed

* merge conflict

---------

Co-authored-by: Adam Tucker <adam@osmosis.team>
Co-authored-by: Adam Tucker <adamleetucker@outlook.com>

CHANGELOG.md

@@ -37,6 +37,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
+* (slashing) [#19458](https://github.com/cosmos/cosmos-sdk/pull/19458) Avoid writing SignInfo's for validator's who did not miss a block. (Every BeginBlock)
 * (slashing) [#18959](https://github.com/cosmos/cosmos-sdk/pull/18959) Slight speedup to Slashing Beginblock logic
 
 ## [v0.47.5](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.5) - 2023-09-01

proto/cosmos/slashing/v1beta1/slashing.proto

@@ -19,10 +19,10 @@ message ValidatorSigningInfo {
   string address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
   // Height at which validator was first a candidate OR was unjailed
   int64 start_height = 2;
-  // Index which is incremented each time the validator was a bonded
-  // in a block and may have signed a precommit or not. This in conjunction with the
-  // `SignedBlocksWindow` param determines the index in the `MissedBlocksBitArray`.
-  int64 index_offset = 3;
+  // DEPRECATED: Index which is incremented every time a validator is bonded in a block and
+  // _may_ have signed a pre-commit or not. This in conjunction with the
+  // signed_blocks_window param determines the index in the missed block bitmap.
+  int64 index_offset = 3 [deprecated = true];
   // Timestamp until which the validator is jailed due to liveness downtime.
   google.protobuf.Timestamp jailed_until = 4
       [(gogoproto.stdtime) = true, (gogoproto.nullable) = false, (amino.dont_omitempty) = true];

tests/integration/slashing/keeper/keeper_test.go

@@ -58,9 +58,9 @@ func (s *KeeperTestSuite) SetupTest() {
 	s.slashingKeeper.SetParams(ctx, testutil.TestParams())
 	addrDels := simtestutil.AddTestAddrsIncremental(s.bankKeeper, s.stakingKeeper, ctx, 5, s.stakingKeeper.TokensFromConsensusPower(ctx, 200))
 
-	info1 := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addrDels[0]), int64(4), int64(3),
+	info1 := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addrDels[0]), int64(4),
 		time.Unix(2, 0), false, int64(10))
-	info2 := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addrDels[1]), int64(5), int64(4),
+	info2 := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addrDels[1]), int64(5),
 		time.Unix(2, 0), false, int64(10))
 
 	s.slashingKeeper.SetValidatorSigningInfo(ctx, sdk.ConsAddress(addrDels[0]), info1)
@@ -169,7 +169,6 @@ func (s *KeeperTestSuite) TestHandleNewValidator() {
 	info, found := s.slashingKeeper.GetValidatorSigningInfo(ctx, sdk.ConsAddress(val.Address()))
 	s.Require().True(found)
 	s.Require().Equal(s.slashingKeeper.SignedBlocksWindow(ctx)+1, info.StartHeight)
-	s.Require().Equal(int64(2), info.IndexOffset)
 	s.Require().Equal(int64(1), info.MissedBlocksCounter)
 	s.Require().Equal(time.Unix(0, 0).UTC(), info.JailedUntil)
 
@@ -308,7 +307,6 @@ func (s *KeeperTestSuite) TestValidatorDippingInAndOut() {
 	s.Require().True(found)
 	s.Require().Equal(int64(700), signInfo.StartHeight)
 	s.Require().Equal(int64(0), signInfo.MissedBlocksCounter)
-	s.Require().Equal(int64(0), signInfo.IndexOffset)
 
 	// some blocks pass
 	height = int64(5000)

x/slashing/README.md

@@ -212,8 +212,8 @@ provisions and rewards.
 At the beginning of each block, we update the `ValidatorSigningInfo` for each
 validator and check if they've crossed below the liveness threshold over a
 sliding window. This sliding window is defined by `SignedBlocksWindow` and the
-index in this window is determined by `IndexOffset` found in the validator's
-`ValidatorSigningInfo`. For each block processed, the `IndexOffset` is incremented
+index in this window is determined by (`IndexOffset = height - SignInfo.StartHeight`).
+Note that in each block processed, the `IndexOffset` is incremented
 regardless if the validator signed or not. Once the index is determined, the
 `MissedBlocksBitArray` and `MissedBlocksCounter` are updated accordingly.
 
@@ -235,10 +235,8 @@ for vote in block.LastCommitInfo.Votes {
   signInfo := GetValidatorSigningInfo(vote.Validator.Address)
 
   // This is a relative index, so we counts blocks the validator SHOULD have
-  // signed. We use the 0-value default signing info if not present, except for
-  // start height.
-  index := signInfo.IndexOffset % SignedBlocksWindow()
-  signInfo.IndexOffset++
+  // signed. We use the 0-value default signing info if not present.
+  index := (height - signInfo.StartHeight) % SignedBlocksWindow()
 
   // Update MissedBlocksBitArray and MissedBlocksCounter. The MissedBlocksCounter
   // just tracks the sum of MissedBlocksBitArray. That way we avoid needing to

x/slashing/abci_test.go

@@ -73,7 +73,6 @@ func TestBeginBlocker(t *testing.T) {
 	info, found := slashingKeeper.GetValidatorSigningInfo(ctx, sdk.ConsAddress(pk.Address()))
 	require.True(t, found)
 	require.Equal(t, ctx.BlockHeight(), info.StartHeight)
-	require.Equal(t, int64(1), info.IndexOffset)
 	require.Equal(t, time.Unix(0, 0).UTC(), info.JailedUntil)
 	require.Equal(t, int64(0), info.MissedBlocksCounter)
 

x/slashing/keeper/genesis_test.go

@@ -19,9 +19,9 @@ func (s *KeeperTestSuite) TestExportAndInitGenesis() {
 	consAddr1 := sdk.ConsAddress(sdk.AccAddress([]byte("addr1_______________")))
 	consAddr2 := sdk.ConsAddress(sdk.AccAddress([]byte("addr2_______________")))
 
-	info1 := types.NewValidatorSigningInfo(consAddr1, int64(4), int64(3),
+	info1 := types.NewValidatorSigningInfo(consAddr1, int64(4),
 		time.Now().UTC().Add(100000000000), false, int64(10))
-	info2 := types.NewValidatorSigningInfo(consAddr2, int64(5), int64(4),
+	info2 := types.NewValidatorSigningInfo(consAddr2, int64(5),
 		time.Now().UTC().Add(10000000000), false, int64(10))
 
 	keeper.SetValidatorSigningInfo(ctx, consAddr1, info1)

x/slashing/keeper/grpc_query_test.go

@@ -31,7 +31,6 @@ func (s *KeeperTestSuite) TestGRPCSigningInfo() {
 	signingInfo := slashingtypes.NewValidatorSigningInfo(
 		consAddr,
 		0,
-		int64(0),
 		time.Unix(2, 0),
 		false,
 		int64(0),
@@ -57,7 +56,6 @@ func (s *KeeperTestSuite) TestGRPCSigningInfos() {
 	signingInfo := slashingtypes.NewValidatorSigningInfo(
 		consAddr1,
 		0,
-		int64(0),
 		time.Unix(2, 0),
 		false,
 		int64(0),

x/slashing/keeper/hooks.go

@@ -30,7 +30,6 @@ func (h Hooks) AfterValidatorBonded(ctx sdk.Context, consAddr sdk.ConsAddress, v
 		signingInfo = types.NewValidatorSigningInfo(
 			consAddr,
 			ctx.BlockHeight(),
-			0,
 			time.Unix(0, 0),
 			false,
 			0,

x/slashing/keeper/infractions.go

@@ -36,24 +36,33 @@ func (k Keeper) HandleValidatorSignatureWithParams(ctx sdk.Context, params types
 	signedBlocksWindow := params.SignedBlocksWindow
 
 	// Compute the relative index, so we count the blocks the validator *should*
-	// have signed. We will use the 0-value default signing info if not present,
-	// except for start height. The index is in the range [0, SignedBlocksWindow)
+	// have signed. We will also use the 0-value default signing info if not present.
+	// The index is in the range [0, SignedBlocksWindow)
 	// and is used to see if a validator signed a block at the given height, which
 	// is represented by a bit in the bitmap.
-	index := signInfo.IndexOffset % signedBlocksWindow
-	signInfo.IndexOffset++
-
+	// The validator start height should get mapped to index 0, so we computed index as:
+	// (height - startHeight) % signedBlocksWindow
+	//
+	// NOTE: There is subtle different behavior between genesis validators and non-genesis validators.
+	// A genesis validator will start at index 0, whereas a non-genesis validator's startHeight will be the block
+	// they bonded on, but the first block they vote on will be one later. (And thus their first vote is at index 1)
+	index := (height - signInfo.StartHeight) % signedBlocksWindow
+	if signInfo.StartHeight > height {
+		panic(fmt.Errorf("invalid state, the validator %v has start height %d , which is greater than the current height %d (as parsed from the header)",
+			signInfo.Address, signInfo.StartHeight, height))
+	}
 	// Update signed block bit array & counter
 	// This counter just tracks the sum of the bit array
 	// That way we avoid needing to read/write the whole array each time
 	previous, err := k.GetMissedBlockBitmapValue(ctx, consAddr, index)
 	if err != nil {
 		panic(fmt.Sprintf("Expected to get missed block bitmap value for validator %s at index %d but not found, error: %v", consAddr, index, err))
 	}
-
+	modifiedSignInfo := false
 	missed := !signed
 	switch {
 	case !previous && missed:
+		modifiedSignInfo = true
 		// Bitmap value has changed from not missed to missed, so we flip the bit
 		// and increment the counter.
 		if err := k.SetMissedBlockBitmapValue(ctx, consAddr, index, true); err != nil {
@@ -63,6 +72,7 @@ func (k Keeper) HandleValidatorSignatureWithParams(ctx sdk.Context, params types
 		signInfo.MissedBlocksCounter++
 
 	case previous && !missed:
+		modifiedSignInfo = true
 		// Bitmap value has changed from missed to not missed, so we flip the bit
 		// and decrement the counter.
 		if err := k.SetMissedBlockBitmapValue(ctx, consAddr, index, false); err != nil {
@@ -101,6 +111,7 @@ func (k Keeper) HandleValidatorSignatureWithParams(ctx sdk.Context, params types
 
 	// if we are past the minimum height and the validator has missed too many blocks, punish them
 	if height > minHeight && signInfo.MissedBlocksCounter > maxMissed {
+		modifiedSignInfo = true
 		validator := k.sk.ValidatorByConsAddr(ctx, consAddr)
 		if validator != nil && !validator.IsJailed() {
 			// Downtime confirmed: slash and jail the validator
@@ -127,8 +138,9 @@ func (k Keeper) HandleValidatorSignatureWithParams(ctx sdk.Context, params types
 			signInfo.JailedUntil = ctx.BlockHeader().Time.Add(k.DowntimeJailDuration(ctx))
 
 			// We need to reset the counter & array so that the validator won't be immediately slashed for downtime upon rebonding.
+			// We don't set the start height as this will get correctly set
+			// once they bond again in the AfterValidatorBonded hook!
 			signInfo.MissedBlocksCounter = 0
-			signInfo.IndexOffset = 0
 			err = k.DeleteMissedBlockBitmap(ctx, consAddr)
 			if err != nil {
 				panic(fmt.Sprintf("Expected to delete missed block bitmap for validator %s but not found, error: %v", consAddr, err))
@@ -153,5 +165,7 @@ func (k Keeper) HandleValidatorSignatureWithParams(ctx sdk.Context, params types
 	}
 
 	// Set the updated signing info
-	k.SetValidatorSigningInfo(ctx, consAddr, signInfo)
+	if modifiedSignInfo {
+		k.SetValidatorSigningInfo(ctx, consAddr, signInfo)
+	}
 }

x/slashing/keeper/msg_server_test.go

@@ -196,7 +196,7 @@ func (s *KeeperTestSuite) TestUnjail() {
 
 				s.Require().NoError(err)
 
-				info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addr), int64(4), int64(3),
+				info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addr), int64(4),
 					time.Unix(2, 0), false, int64(10))
 
 				s.slashingKeeper.SetValidatorSigningInfo(s.ctx, sdk.ConsAddress(addr), info)
@@ -226,7 +226,7 @@ func (s *KeeperTestSuite) TestUnjail() {
 
 				s.Require().NoError(err)
 
-				info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addr), int64(4), int64(3),
+				info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addr), int64(4),
 					time.Unix(2, 0), true, int64(10))
 
 				s.slashingKeeper.SetValidatorSigningInfo(s.ctx, sdk.ConsAddress(addr), info)
@@ -256,7 +256,7 @@ func (s *KeeperTestSuite) TestUnjail() {
 
 				s.Require().NoError(err)
 
-				info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addr), int64(4), int64(3),
+				info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addr), int64(4),
 					s.ctx.BlockTime().AddDate(0, 0, 1), false, int64(10))
 
 				s.slashingKeeper.SetValidatorSigningInfo(s.ctx, sdk.ConsAddress(addr), info)
@@ -286,7 +286,7 @@ func (s *KeeperTestSuite) TestUnjail() {
 				val.Jailed = true
 				s.Require().NoError(err)
 
-				info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addr), int64(4), int64(3),
+				info := slashingtypes.NewValidatorSigningInfo(sdk.ConsAddress(addr), int64(4),
 					time.Unix(2, 0), false, int64(10))
 
 				s.slashingKeeper.SetValidatorSigningInfo(s.ctx, sdk.ConsAddress(addr), info)

x/slashing/keeper/signing_info_test.go

@@ -16,7 +16,6 @@ func (s *KeeperTestSuite) TestValidatorSigningInfo() {
 	signingInfo := slashingtypes.NewValidatorSigningInfo(
 		consAddr,
 		ctx.BlockHeight(),
-		int64(3),
 		time.Unix(2, 0),
 		false,
 		int64(10),
@@ -29,7 +28,7 @@ func (s *KeeperTestSuite) TestValidatorSigningInfo() {
 	info, found := keeper.GetValidatorSigningInfo(ctx, consAddr)
 	require.True(found)
 	require.Equal(info.StartHeight, ctx.BlockHeight())
-	require.Equal(info.IndexOffset, int64(3))
+	require.Equal(info.IndexOffset, int64(0))
 	require.Equal(info.JailedUntil, time.Unix(2, 0).UTC())
 	require.Equal(info.MissedBlocksCounter, int64(10))
 

x/slashing/simulation/decoder_test.go

@@ -29,7 +29,7 @@ func TestDecodeStore(t *testing.T) {
 	depinject.Inject(testutil.AppConfig, &cdc)
 	dec := simulation.NewDecodeStore(cdc)
 
-	info := types.NewValidatorSigningInfo(consAddr1, 0, 1, time.Now().UTC(), false, 0)
+	info := types.NewValidatorSigningInfo(consAddr1, 0, time.Now().UTC(), false, 0)
 	missed := []byte{1} // we want to display the bytes for simulation diffs
 	bz, err := cdc.MarshalInterface(delPk1)
 	require.NoError(t, err)

x/slashing/simulation/operations_test.go

@@ -153,7 +153,7 @@ func (suite *SimTestSuite) TestSimulateMsgUnjail() {
 	suite.stakingKeeper.SetValidatorByConsAddr(ctx, validator0)
 	val0ConsAddress, err := validator0.GetConsAddr()
 	suite.Require().NoError(err)
-	info := types.NewValidatorSigningInfo(val0ConsAddress, int64(4), int64(3),
+	info := types.NewValidatorSigningInfo(val0ConsAddress, int64(4),
 		time.Unix(2, 0), false, int64(10))
 	suite.slashingKeeper.SetValidatorSigningInfo(ctx, val0ConsAddress, info)
 

x/slashing/types/signing_info.go

@@ -12,13 +12,12 @@ import (
 //
 //nolint:interfacer
 func NewValidatorSigningInfo(
-	consAddr sdk.ConsAddress, startHeight, indexOffset int64,
+	consAddr sdk.ConsAddress, startHeight int64,
 	jailedUntil time.Time, tombstoned bool, missedBlocksCounter int64,
 ) ValidatorSigningInfo {
 	return ValidatorSigningInfo{
 		Address:             consAddr.String(),
 		StartHeight:         startHeight,
-		IndexOffset:         indexOffset,
 		JailedUntil:         jailedUntil,
 		Tombstoned:          tombstoned,
 		MissedBlocksCounter: missedBlocksCounter,