WIP: IS changes in sdk #12830
WIP: IS changes in sdk #12830
Conversation
Create a slashing hook that allows external modules to register an execution when a validator has downtime. Implementation details: * Call hook in HandleValidatorSignature (x/slashing/keeper/infractions.go) which updates validator SignInfo data * Defer hook execution in case it also wants to update validator SignInfo data * Add methods to update SignInfo to slashing keeper interface(/x/slashing/types/expected_keepers.go)
* update: Remove slashing module hooks Hooks are not required anymore to implement the slashing for downtime in CCV. The logic is now using the staking keeper interface definition from the slashing module. The SDK changes are the following: - /x/slashing/keeper/infractions.go - remove hook calls and don't update validators missed blocks when jailed - /x/slashing/types/expected_keepers.go - remove `AfterValidatorDowntime` hook interface and add `IsJailed()` method to staking interface definition - /x/staking/keeper/validator.go - implement `IsJailed()` method * fix last details
* allow stopping and completing of redelegations * refactor to remove BeforeUnbondingDelegationEntryComplete hook and notes for validator unbonding * WIP rough draft of validator unbonding hooks * add many of marius's suggested changes * More review changes * unbonding delegation tests pass * WIP adding redelegation tests * WIP redelegation tests work * unbondingDelegation and redelegation tests pass and cleanup * WIP validator unbonding tests almost pass * tests for all new functionality pass * fix index deleting logic * clean up TODOs * fix small logic bug * fix slashing tests * Rename statements containing 'UnbondingOp' to 'Unbond' in code, docs and proto files Co-authored-by: Simon <[email protected]>
* remove leak for UBDEs and REDEs * remove leak for val unbondings
* updating staking spec * clarify code * fix typo
Co-authored-by: Simon Noetzlin <[email protected]>
Co-authored-by: Simon Noetzlin <[email protected]>
| } | ||
|
|
||
| // ValidatorUpdates defines an array of abci.ValidatorUpdate objects. | ||
| message ValidatorUpdates { |
There was a problem hiding this comment.
nit: this might make more sense in cosmos/tendermint. Ill pick this up when I rebase
|
At what stage should I review this? |
| @@ -332,3 +351,20 @@ message Pool { | |||
| (gogoproto.moretags) = "yaml:\"bonded_tokens\"" | |||
| ]; | |||
| } | |||
|
|
|||
| // InfractionType indicates the infraction type a validator commited. | |||
| enum InfractionType { | |||
There was a problem hiding this comment.
how come this is in staking instead of evidence? I think the evidence module should handle this instead of staking
There was a problem hiding this comment.
I think it was a decision made by @sainoe after a discussion with @alexanderbez (if I'm not mistaken).
There was a problem hiding this comment.
@alexanderbez do you remember the outcome? Right now it feels like we are conflating evidence with staking when evidence module exists for this purpose
There was a problem hiding this comment.
this isn't needed here imo. Its only needed for the ibc packet and to know the slash fraction before calling slash.
| @@ -109,6 +113,7 @@ func (k Keeper) HandleEquivocationEvidence(ctx sdk.Context, evidence *types.Equi | |||
| consAddr, | |||
| k.slashingKeeper.SlashFractionDoubleSign(ctx), | |||
| evidence.GetValidatorPower(), distributionHeight, | |||
| stakingtypes.DoubleSign, | |||
There was a problem hiding this comment.
this doesn't seem needed? the slash amount signals the slash type. I might be missing some context?
There was a problem hiding this comment.
going through the paths this isn't needed to slash validators, I think we can remove it. Its only needed here https://github.com/cosmos/interchain-security/blob/main/x/ccv/provider/keeper/relay.go#L244
|
don't need this to be open anymore |
Description
This PR is meant to help review the changes before rebasing onto main for a merge and release later on