refactor(x/slashing): audit x/slashing changes (backport #21270) (#21279)

Co-authored-by: Julián Toledano <JulianToledano@users.noreply.github.com>
This commit is contained in:
mergify[bot] 2024-08-13 22:14:05 +02:00 committed by GitHub
parent 0230cc50b9
commit 1bc7ff528c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 31 additions and 40 deletions

View File

@ -29,7 +29,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Improvements
* [#19458](https://github.com/cosmos/cosmos-sdk/pull/19458) Avoid writing SignInfo's for validator's who did not miss a block. (Every BeginBlock)
* [#19458](https://github.com/cosmos/cosmos-sdk/pull/19458) Avoid writing SignInfo's for validators who did not miss a block. (Every BeginBlock)
* [#18959](https://github.com/cosmos/cosmos-sdk/pull/18959) Avoid deserialization of parameters with every validator lookup
* [#18636](https://github.com/cosmos/cosmos-sdk/pull/18636) `JailUntil` and `Tombstone` methods no longer panics if the signing info does not exist for the validator but instead returns error.

View File

@ -15,9 +15,9 @@ import (
func BeginBlocker(ctx context.Context, k keeper.Keeper, cometService comet.Service) error {
defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyBeginBlocker)
// Iterate over all the validators which *should* have signed this block
// store whether or not they have actually signed it and slash/unbond any
// which have missed too many blocks in a row (downtime slashing)
// Retrieve CometBFT info, then iterate through all validator votes
// from the last commit. For each vote, handle the validator's signature, potentially
// slashing or unbonding validators who have missed too many blocks.
params, err := k.Params.Get(ctx)
if err != nil {
return err

View File

@ -34,7 +34,7 @@ func (k Querier) Params(ctx context.Context, req *types.QueryParamsRequest) (*ty
}
// SigningInfo returns signing-info of a specific validator.
func (k Keeper) SigningInfo(ctx context.Context, req *types.QuerySigningInfoRequest) (*types.QuerySigningInfoResponse, error) {
func (k Querier) SigningInfo(ctx context.Context, req *types.QuerySigningInfoRequest) (*types.QuerySigningInfoResponse, error) {
if req == nil {
return nil, status.Errorf(codes.InvalidArgument, "empty request")
}
@ -57,7 +57,7 @@ func (k Keeper) SigningInfo(ctx context.Context, req *types.QuerySigningInfoRequ
}
// SigningInfos returns signing-infos of all validators.
func (k Keeper) SigningInfos(ctx context.Context, req *types.QuerySigningInfosRequest) (*types.QuerySigningInfosResponse, error) {
func (k Querier) SigningInfos(ctx context.Context, req *types.QuerySigningInfosRequest) (*types.QuerySigningInfosResponse, error) {
if req == nil {
return nil, status.Errorf(codes.InvalidArgument, "empty request")
}

View File

@ -45,10 +45,8 @@ func (s *KeeperTestSuite) TestGRPCSigningInfo() {
info, err := keeper.ValidatorSigningInfo.Get(ctx, consAddr)
require.NoError(err)
consAddrStr, err := s.stakingKeeper.ConsensusAddressCodec().BytesToString(consAddr)
require.NoError(err)
infoResp, err = queryClient.SigningInfo(gocontext.Background(),
&slashingtypes.QuerySigningInfoRequest{ConsAddress: consAddrStr})
&slashingtypes.QuerySigningInfoRequest{ConsAddress: consStr})
require.NoError(err)
require.Equal(info, infoResp.ValSigningInfo)
}
@ -58,10 +56,10 @@ func (s *KeeperTestSuite) TestGRPCSigningInfos() {
require := s.Require()
// set two validator signing information
consAddr1 := sdk.ConsAddress(sdk.AccAddress([]byte("addr1_______________")))
consAddr1 := sdk.ConsAddress("addr1_______________")
consStr1, err := s.stakingKeeper.ConsensusAddressCodec().BytesToString(consAddr1)
require.NoError(err)
consAddr2 := sdk.ConsAddress(sdk.AccAddress([]byte("addr2_______________")))
consAddr2 := sdk.ConsAddress("addr2_______________")
signingInfo := slashingtypes.NewValidatorSigningInfo(
consStr1,
0,

View File

@ -28,12 +28,12 @@ func (k Keeper) HandleValidatorSignatureWithParams(ctx context.Context, params t
// fetch the validator public key
consAddr := sdk.ConsAddress(addr)
// don't update missed blocks when validator's jailed
val, err := k.sk.ValidatorByConsAddr(ctx, consAddr)
if err != nil {
return err
}
// don't update missed blocks when validator's jailed
if val.IsJailed() {
return nil
}
@ -142,11 +142,11 @@ func (k Keeper) HandleValidatorSignatureWithParams(ctx context.Context, params t
}
if validator != nil && !validator.IsJailed() {
// Downtime confirmed: slash and jail the validator
// We need to retrieve the stake distribution which signed the block, so we subtract ValidatorUpdateDelay from the evidence height,
// We need to retrieve the stake distribution that signed the block. To do this, we subtract ValidatorUpdateDelay from the evidence height,
// and subtract an additional 1 since this is the LastCommit.
// Note that this *can* result in a negative "distributionHeight" up to -ValidatorUpdateDelay-1,
// Note that this *can* result in a negative "distributionHeight" of up to -ValidatorUpdateDelay-1,
// i.e. at the end of the pre-genesis block (none) = at the beginning of the genesis block.
// That's fine since this is just used to filter unbonding delegations & redelegations.
// This is acceptable since it's only used to filter unbonding delegations & redelegations.
distributionHeight := height - sdk.ValidatorUpdateDelay - 1
slashFractionDowntime, err := k.SlashFractionDowntime(ctx)

View File

@ -90,13 +90,13 @@ func (k Keeper) GetPubkey(ctx context.Context, a cryptotypes.Address) (cryptotyp
}
// Slash attempts to slash a validator. The slash is delegated to the staking
// module to make the necessary validator changes. It specifies no intraction reason.
// module to make the necessary validator changes. It specifies no infraction reason.
func (k Keeper) Slash(ctx context.Context, consAddr sdk.ConsAddress, fraction sdkmath.LegacyDec, power, distributionHeight int64) error {
return k.SlashWithInfractionReason(ctx, consAddr, fraction, power, distributionHeight, st.Infraction_INFRACTION_UNSPECIFIED)
}
// SlashWithInfractionReason attempts to slash a validator. The slash is delegated to the staking
// module to make the necessary validator changes. It specifies an intraction reason.
// module to make the necessary validator changes. It specifies an infraction reason.
func (k Keeper) SlashWithInfractionReason(ctx context.Context, consAddr sdk.ConsAddress, fraction sdkmath.LegacyDec, power, distributionHeight int64, infraction st.Infraction) error {
coinsBurned, err := k.sk.SlashWithInfractionReason(ctx, consAddr, distributionHeight, power, fraction, infraction)
if err != nil {
@ -137,11 +137,7 @@ func (k Keeper) Jail(ctx context.Context, consAddr sdk.ConsAddress) error {
return err
}
if err := k.EventService.EventManager(ctx).EmitKV(
return k.EventService.EventManager(ctx).EmitKV(
types.EventTypeSlash,
event.NewAttribute(types.AttributeKeyJailed, consStr),
); err != nil {
return err
}
return nil
event.NewAttribute(types.AttributeKeyJailed, consStr))
}

View File

@ -29,7 +29,7 @@ import (
moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
)
var consAddr = sdk.ConsAddress(sdk.AccAddress([]byte("addr1_______________")))
var consAddr = sdk.ConsAddress("addr1_______________")
type KeeperTestSuite struct {
suite.Suite
@ -153,7 +153,7 @@ func validatorMissedBlockBitmapKey(v sdk.ConsAddress, chunkIndex int64) []byte {
func (s *KeeperTestSuite) TestValidatorMissedBlockBMMigrationToColls() {
s.SetupTest()
consAddr := sdk.ConsAddress(sdk.AccAddress([]byte("addr1_______________")))
consAddr := sdk.ConsAddress("addr1_______________")
index := int64(0)
err := sdktestutil.DiffCollectionsMigration(
s.ctx,

View File

@ -23,8 +23,7 @@ func (k Keeper) HasValidatorSigningInfo(ctx context.Context, consAddr sdk.ConsAd
return err == nil && has
}
// JailUntil attempts to set a validator's JailedUntil attribute in its signing
// info.
// JailUntil attempts to set a validator's JailedUntil attribute in its signing info.
func (k Keeper) JailUntil(ctx context.Context, consAddr sdk.ConsAddress, jailTime time.Time) error {
signInfo, err := k.ValidatorSigningInfo.Get(ctx, consAddr)
if err != nil {
@ -32,7 +31,7 @@ func (k Keeper) JailUntil(ctx context.Context, consAddr sdk.ConsAddress, jailTim
if err != nil {
return types.ErrNoSigningInfoFound.Wrapf("could not convert consensus address to string. Error: %s", err.Error())
}
return errorsmod.Wrap(err, fmt.Sprintf("cannot jail validator with consensus address %s that does not have any signing information", addr))
return types.ErrNoSigningInfoFound.Wrapf(fmt.Sprintf("cannot jail validator with consensus address %s that does not have any signing information", addr))
}
signInfo.JailedUntil = jailTime

View File

@ -102,7 +102,7 @@ func (s *KeeperTestSuite) TestValidatorMissedBlockBitmap_SmallWindow() {
require.Len(missedBlocks, int(params.SignedBlocksWindow)-1)
// if the validator rotated it's key there will be different consKeys and a mapping will be added in the state.
consAddr1 := sdk.ConsAddress(sdk.AccAddress([]byte("addr1_______________")))
consAddr1 := sdk.ConsAddress("addr1_______________")
s.stakingKeeper.EXPECT().ValidatorIdentifier(gomock.Any(), consAddr1).Return(consAddr, nil).AnyTimes()
missedBlocks, err = keeper.GetValidatorMissedBlocks(ctx, consAddr1)
@ -121,11 +121,11 @@ func (s *KeeperTestSuite) TestPerformConsensusPubKeyUpdate() {
oldConsAddr := sdk.ConsAddress(pks[0].Address())
newConsAddr := sdk.ConsAddress(pks[1].Address())
consAddr, err := s.stakingKeeper.ConsensusAddressCodec().BytesToString(newConsAddr)
consStrAddr, err := s.stakingKeeper.ConsensusAddressCodec().BytesToString(newConsAddr)
s.Require().NoError(err)
newInfo := slashingtypes.NewValidatorSigningInfo(
consAddr,
consStrAddr,
int64(4),
time.Unix(2, 0).UTC(),
false,

View File

@ -63,7 +63,7 @@ func Migrate(ctx context.Context, cdc codec.BinaryCodec, store storetypes.KVStor
}
func iterateValidatorSigningInfos(
ctx context.Context,
_ context.Context,
cdc codec.BinaryCodec,
store storetypes.KVStore,
cb func(address sdk.ConsAddress, info types.ValidatorSigningInfo) (stop bool),
@ -72,18 +72,18 @@ func iterateValidatorSigningInfos(
defer iter.Close()
for ; iter.Valid(); iter.Next() {
address := ValidatorSigningInfoAddress(iter.Key())
addr := ValidatorSigningInfoAddress(iter.Key())
var info types.ValidatorSigningInfo
cdc.MustUnmarshal(iter.Value(), &info)
if cb(address, info) {
if cb(addr, info) {
break
}
}
}
func iterateValidatorMissedBlockBitArray(
ctx context.Context,
_ context.Context,
cdc codec.BinaryCodec,
store storetypes.KVStore,
addr sdk.ConsAddress,
@ -120,7 +120,7 @@ func GetValidatorMissedBlocks(
return missedBlocks
}
func deleteValidatorMissedBlockBitArray(ctx context.Context, store storetypes.KVStore, addr sdk.ConsAddress) {
func deleteValidatorMissedBlockBitArray(_ context.Context, store storetypes.KVStore, addr sdk.ConsAddress) {
iter := storetypes.KVStorePrefixIterator(store, validatorMissedBlockBitArrayPrefixKey(addr))
defer iter.Close()
@ -129,7 +129,7 @@ func deleteValidatorMissedBlockBitArray(ctx context.Context, store storetypes.KV
}
}
func setMissedBlockBitmapValue(ctx context.Context, store storetypes.KVStore, addr sdk.ConsAddress, index int64, missed bool) error {
func setMissedBlockBitmapValue(_ context.Context, store storetypes.KVStore, addr sdk.ConsAddress, index int64, missed bool) error {
// get the chunk or "word" in the logical bitmap
chunkIndex := index / MissedBlockBitmapChunkSize
key := ValidatorMissedBlockBitmapKey(addr, chunkIndex)

View File

@ -19,7 +19,7 @@ import (
moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
)
var consAddr = sdk.ConsAddress(sdk.AccAddress([]byte("addr1_______________")))
var consAddr = sdk.ConsAddress("addr1_______________")
func TestMigrate(t *testing.T) {
cdc := moduletestutil.MakeTestEncodingConfig(codectestutil.CodecOptions{}, slashing.AppModule{}).Codec

View File

@ -83,8 +83,6 @@ func TestRandomizedGenState1(t *testing.T) {
}
for _, tt := range tests {
tt := tt
require.Panicsf(t, func() { simulation.RandomizedGenState(&tt.simState) }, tt.panicMsg)
}
}