From 7071416b2e3fb2b36737c6dedda7b271cdbaa839 Mon Sep 17 00:00:00 2001 From: p0p3yee Date: Thu, 24 Oct 2024 10:24:11 -0400 Subject: [PATCH 1/3] Fix idle validator slashing logic --- x/keyshare/module/module.go | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/x/keyshare/module/module.go b/x/keyshare/module/module.go index 916ced6d..121019d8 100644 --- a/x/keyshare/module/module.go +++ b/x/keyshare/module/module.go @@ -238,6 +238,12 @@ func (am AppModule) BeginBlock(cctx context.Context) error { if foundQc { am.keeper.SetActiveCommitments(ctx, qc) } + // When switching the active public key, + // Reset all validators last submitted height, + // So they won't get slashed if they miss the first block of the new active pubkey keyshare. + for _, v := range qk.EncryptedKeyshares { + am.keeper.SetLastSubmittedHeight(ctx, v.Validator, strconv.FormatInt(ctx.BlockHeight(), 10)) + } } am.keeper.DeleteQueuedPubkey(ctx) am.pepKeeper.DeleteQueuedPubkey(ctx) @@ -256,6 +262,13 @@ func (am AppModule) EndBlock(cctx context.Context) error { validators := am.keeper.GetAllValidatorSet(ctx) params := am.keeper.GetParams(ctx) + pubKey, found := am.keeper.GetActivePubkey(ctx) + // If no active public key / no validator in the current public key + // Then no Idling check needed + if !found || len(pubKey.PublicKey) == 0 || len(pubKey.EncryptedKeyshares) == 0 { + return nil + } + for _, eachValidator := range validators { lastSubmittedHeight := am.keeper.GetLastSubmittedHeight(ctx, eachValidator.Validator) am.keeper.Logger().Info(fmt.Sprintf("Last submitted: %s: %d", eachValidator.Validator, lastSubmittedHeight)) @@ -279,10 +292,19 @@ func (am AppModule) EndBlock(cctx context.Context) error { continue } - if val, found := am.keeper.GetActivePubkey(ctx); !found || len(val.PublicKey) == 0 { - // Not slashing validator if there is no active public key + inCurrentEpoch := false + + for _, k := range pubKey.EncryptedKeyshares { + if k.Validator == eachValidator.Validator { + inCurrentEpoch = true + break + } + } + + if !inCurrentEpoch { + am.keeper.Logger().Info(fmt.Sprintf("Validator: %s not in the current epoch, updating last submitted height to current block height.", eachValidator.Validator)) am.keeper.SetLastSubmittedHeight(ctx, eachValidator.Validator, strconv.FormatInt(ctx.BlockHeight(), 10)) - continue + return nil } am.keeper.SlashingKeeper().Slash( From 1af6c205a7a9dbcf7e8bcd380c14677f3ec7f083 Mon Sep 17 00:00:00 2001 From: p0p3yee Date: Thu, 24 Oct 2024 11:56:52 -0400 Subject: [PATCH 2/3] Fix set last submitted height for validator instead of authorized address --- x/keyshare/keeper/msg_send_keyshare.go | 4 +++- x/keyshare/keeper/msg_submit_encrypted_keyshare.go | 7 ++++--- x/keyshare/keeper/msg_submit_general_keyshare.go | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/x/keyshare/keeper/msg_send_keyshare.go b/x/keyshare/keeper/msg_send_keyshare.go index c02b3420..9a9aa903 100644 --- a/x/keyshare/keeper/msg_send_keyshare.go +++ b/x/keyshare/keeper/msg_send_keyshare.go @@ -116,7 +116,9 @@ func (k msgServer) SendKeyshare(goCtx context.Context, msg *types.MsgSendKeyshar // Save the new keyshare to state k.SetKeyshare(ctx, keyshare) - k.SetLastSubmittedHeight(ctx, msg.Creator, strconv.FormatUint(msg.BlockHeight, 10)) + // It should set the validator last submitted height instead of the `msg.creator`, + // which might be the authorized address + k.SetLastSubmittedHeight(ctx, validatorInfo.Validator, strconv.FormatUint(msg.BlockHeight, 10)) validatorList := k.GetAllValidatorSet(ctx) diff --git a/x/keyshare/keeper/msg_submit_encrypted_keyshare.go b/x/keyshare/keeper/msg_submit_encrypted_keyshare.go index 4c3ef151..4c37c629 100644 --- a/x/keyshare/keeper/msg_submit_encrypted_keyshare.go +++ b/x/keyshare/keeper/msg_submit_encrypted_keyshare.go @@ -17,7 +17,7 @@ func (k msgServer) SubmitEncryptedKeyshare(goCtx context.Context, msg *types.Msg ctx := sdk.UnwrapSDKContext(goCtx) // check if validator is registered - _, found := k.GetValidatorSet(ctx, msg.Creator) + validatorInfo, found := k.GetValidatorSet(ctx, msg.Creator) if !found { authorizedAddrInfo, found := k.GetAuthorizedAddress(ctx, msg.Creator) @@ -25,10 +25,11 @@ func (k msgServer) SubmitEncryptedKeyshare(goCtx context.Context, msg *types.Msg return nil, types.ErrAddrIsNotValidatorOrAuthorized.Wrap(msg.Creator) } - _, found = k.GetValidatorSet(ctx, authorizedAddrInfo.AuthorizedBy) + authorizedByValInfo, found := k.GetValidatorSet(ctx, authorizedAddrInfo.AuthorizedBy) if !found { return nil, types.ErrAuthorizerIsNotValidator.Wrap(authorizedAddrInfo.AuthorizedBy) } + validatorInfo = authorizedByValInfo // If the sender is in the validator set & authorized another address to submit key share } else if count := k.GetAuthorizedCount(ctx, msg.Creator); count != 0 { @@ -62,7 +63,7 @@ func (k msgServer) SubmitEncryptedKeyshare(goCtx context.Context, msg *types.Msg // Save the new private keyshare to state k.SetPrivateKeyshare(ctx, valEncKeyshare) - k.SetLastSubmittedHeight(ctx, msg.Creator, strconv.FormatInt(ctx.BlockHeight(), 10)) + k.SetLastSubmittedHeight(ctx, validatorInfo.Validator, strconv.FormatInt(ctx.BlockHeight(), 10)) validatorList := k.GetAllValidatorSet(ctx) diff --git a/x/keyshare/keeper/msg_submit_general_keyshare.go b/x/keyshare/keeper/msg_submit_general_keyshare.go index 4f181f57..3058ee68 100644 --- a/x/keyshare/keeper/msg_submit_general_keyshare.go +++ b/x/keyshare/keeper/msg_submit_general_keyshare.go @@ -144,7 +144,7 @@ func (k msgServer) SubmitGeneralKeyshare( // Save the new general key share to state k.SetGeneralKeyshare(ctx, generalKeyshare) - k.SetLastSubmittedHeight(ctx, msg.Creator, strconv.FormatInt(ctx.BlockHeight(), 10)) + k.SetLastSubmittedHeight(ctx, validatorInfo.Validator, strconv.FormatInt(ctx.BlockHeight(), 10)) validatorList := k.GetAllValidatorSet(ctx) From 35d19c0cd6535a3d8d1e5860fd7c321cf1c42788 Mon Sep 17 00:00:00 2001 From: p0p3yee Date: Thu, 24 Oct 2024 12:38:49 -0400 Subject: [PATCH 3/3] Add comments --- x/keyshare/keeper/msg_override_pubkey.go | 1 + x/keyshare/module/module.go | 3 +++ 2 files changed, 4 insertions(+) diff --git a/x/keyshare/keeper/msg_override_pubkey.go b/x/keyshare/keeper/msg_override_pubkey.go index c9cff44f..11c25e63 100644 --- a/x/keyshare/keeper/msg_override_pubkey.go +++ b/x/keyshare/keeper/msg_override_pubkey.go @@ -51,6 +51,7 @@ func (k msgServer) OverrideLatestPubkey( encSharesExistsValidators[encShare.Validator] = true } + // Remove all validators in the set that not in the current epoch for _, v := range allValidatorSet { if _, exists := encSharesExistsValidators[v.Validator]; !exists { k.RemoveValidatorSet(ctx, v.Validator) diff --git a/x/keyshare/module/module.go b/x/keyshare/module/module.go index 121019d8..0e45d07f 100644 --- a/x/keyshare/module/module.go +++ b/x/keyshare/module/module.go @@ -244,6 +244,9 @@ func (am AppModule) BeginBlock(cctx context.Context) error { for _, v := range qk.EncryptedKeyshares { am.keeper.SetLastSubmittedHeight(ctx, v.Validator, strconv.FormatInt(ctx.BlockHeight(), 10)) } + + // We don't need to remove the validators that not in the new round from the set + // They won't be slashed because of the new slashing idle validators logic } am.keeper.DeleteQueuedPubkey(ctx) am.pepKeeper.DeleteQueuedPubkey(ctx)