epoching: copy/paste necessary code from staking/evidence/slashing and make modifications #28
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -371,8 +371,7 @@ func NewBabylonApp( | |
| // NOTE: staking module is required if HistoricalEntries param > 0 | ||
| // NOTE: capability module's beginblocker must come before any modules using capabilities (e.g. IBC) | ||
| app.mm.SetOrderBeginBlockers( | ||
| upgradetypes.ModuleName, capabilitytypes.ModuleName, minttypes.ModuleName, distrtypes.ModuleName, stakingtypes.ModuleName, | ||
| authtypes.ModuleName, banktypes.ModuleName, govtypes.ModuleName, crisistypes.ModuleName, genutiltypes.ModuleName, | ||
| authz.ModuleName, feegrant.ModuleName, | ||
| paramstypes.ModuleName, vestingtypes.ModuleName, | ||
|
|
@@ -382,15 +381,12 @@ func NewBabylonApp( | |
| checkpointingtypes.ModuleName, | ||
| ) | ||
| app.mm.SetOrderEndBlockers( | ||
| crisistypes.ModuleName, govtypes.ModuleName, | ||
|
There was a problem hiding this comment. I suspect this will not work, because SetOrderEndBlockers will panic if you forget to include a module that is otherwise registered in the app. The way I see it is that you can call this first to get the benefit of this check, then remove the staking module from There was a problem hiding this comment. Good point! Didn't know they have an assert there... will fix |
||
| capabilitytypes.ModuleName, authtypes.ModuleName, banktypes.ModuleName, distrtypes.ModuleName, | ||
| slashingtypes.ModuleName, minttypes.ModuleName, | ||
| genutiltypes.ModuleName, evidencetypes.ModuleName, authz.ModuleName, | ||
| feegrant.ModuleName, | ||
| paramstypes.ModuleName, upgradetypes.ModuleName, vestingtypes.ModuleName, | ||
| epochingtypes.ModuleName, | ||
| btclightclienttypes.ModuleName, | ||
| btccheckpointtypes.ModuleName, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package epoching | ||
|
|
||
| import ( | ||
| "time" | ||
|
|
||
| "github.com/babylonchain/babylon/x/epoching/keeper" | ||
| "github.com/babylonchain/babylon/x/epoching/types" | ||
|
|
||
| evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types" | ||
| slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" | ||
| stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" | ||
|
|
||
| "github.com/cosmos/cosmos-sdk/telemetry" | ||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| abci "github.com/tendermint/tendermint/abci/types" | ||
| ) | ||
|
|
||
| func BeginBlocker(ctx sdk.Context, k keeper.Keeper, req abci.RequestBeginBlock) { | ||
| defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) | ||
| defer telemetry.ModuleMeasureSince(evidencetypes.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) | ||
| defer telemetry.ModuleMeasureSince(slashingtypes.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) | ||
|
|
||
| // TODO: unimplemented: | ||
| // - slashing equivocating/unlive validators without jailing them | ||
| } | ||
|
|
||
| // Called every block, update validator set | ||
| func EndBlocker(ctx sdk.Context, k keeper.Keeper) []abci.ValidatorUpdate { | ||
| defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker) | ||
| defer telemetry.ModuleMeasureSince(stakingtypes.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker) | ||
|
|
||
| // TODO: unimplemented: | ||
| // - if an epoch is newly checkpointed, make unbonding validators/delegations in this epoch unbonded | ||
|
There was a problem hiding this comment. I [put this in the diagram to happen when the BTC checkpoint is confirmed, mostly because this is the easiest in terms of book-keeping: I can clearly see when a checkpoint is going confirmed the first time. To do it at the end would be more difficult, need to capture before and after. I also thought it's not actually the epoch's job to know about checkpointing, and that release is related to checkpointing, so it should be in the checkpointing module. There was a problem hiding this comment. I was meaning that the epoching module gets the epoch's chechpoint status from the checkpointing module, rather than judging the status by itself. Is my understanding correct and consistent with yours? There was a problem hiding this comment. Yes, but I mean I'm not sure that the epoching module even needs a notion of status. To put it differently, it doesn't have to know that it's being checkpointed. It knows that it has ended, and it can tell other modules what the last block was, but those dependant modules can figure out on their own what they need to do. It just so happens that the checkpointing module needs the last commit hash for its own purposes, and it will use the checkpointing status as a requirement to release unbonding tokens. But the epoching can be oblivious to that. There was a problem hiding this comment.
Checkpoint-assisted unbonding is a part of the epoching module rather than checkpointing, right? One of the main reasons that checkpoint-assisted unbonding fits the epoching module better is that, the checkpoint-assisted unbonding depends on the staking module (especially the "mature" queues), maintained by the epoching module. There was a problem hiding this comment. I think we agreed with @fishermanymc that the release of unbonding tokens doesn't need to be tied to the epoch. You can imagine using just epochs, with 21 days unbonding, it's fine. But because we are doing checkpointing, we can release tokens earlier, as soon as the checkpoints are confirmed. Therefore it's associated with checkpoints, not epochs. It happens in the middle of the next epoch, depends on BTC, not the next epoch boundary. Epochs manage the delayed staking indeed, but they only manage the inputs. The maturing queues are managed by the There was a problem hiding this comment. How about we do the following?
This way each module does its own thing, and we only have a dependency from checkpointing to epoching (instead of being the other way where epoching hooks to the There was a problem hiding this comment. Thanks @SebastianElvis and @aakoshh . The BTC/Babylon-specific bond release condition is that all the epochs up to the epoch where the unbonding request is submitted are confirmed. The bond release does not have to happen at the epoch boundary unless there are implementation level limitations preventing us from doing this. |
||
| // - if reaching an epoch boundary, execute validator-related msgs (bonded -> unbonding) | ||
| return nil | ||
|
There was a problem hiding this comment. Doesn't matter too much, but you could return an empty array like the rest of the modules. I say doesn't matter because if we never call the staking module at all the app won't work anyway. Might as well panic, to make sure this doesn't go unnoticed. There was a problem hiding this comment.
Default operations in slashing/evidence handling include both slashing and jailing. We want the slashing part but not the jailing part, right? There was a problem hiding this comment. Oh, I don't know much about jailing. What would happen if we allow it to go through? There was a problem hiding this comment. When a validator is jailed, then:
If we allow jailing, then 1 will change the validator set within an epoch, and There was a problem hiding this comment. Okay, so jailing does remove the power and does change the validator set, but only temporarily, without slashing. I don't know if this has a big impact on our epochs. I'm a bit lost, I thought we can let slashing happen immediately, which changes some of the power, and it sounds like jailing has a similar effect, so as long as it's done in moderation, the system should keep working. Because you can't replicate the whole behaviour with copy-pasting, I would not include that part in this PR. What we are doing is not functionally equivalent to what the SDK is doing, therefore it is unacceptable IMO. We can't ignore certain bits on the basis that we may or may not be able to fix it later, when we don't even understand what we are breaking. I would first restrict ourselves to dealing with the delayed staking, and have another go at consulting with Frojdi about whether we can just let jailing happen in the middle of the epoch. @fishermanymc wdyt? There was a problem hiding this comment.
A potential issue is as follows. Assuming the system slashes only a part of its stake (say, 50%) of a validator, then:
Agree with this one. At the moment we have limited understanding on this part and the modification might break the system. At least the vanilla evidence/slashing modules will still work when only few validators are slashed. There was a problem hiding this comment. Yes you are right, if we don't slash/jail then the validator set won't change from what it was at the beginning of the epoch and everyone in the bitmap is still okay to present a BLS signature. To our advantage in this is that
Is that true? If we slash +1/3 then these dishonest validators can produce a BLS signature on their own, but in order to convince anyone's Tendermint node to go to an adversarial fork, you would need +2/3 to be dishonest. What the slashed validators can do is to submit what looks like a "bogus" checkpoint, with their valid signatures, and send the application down this path. Looks like we have to be careful in that situation not to over react and kill Tendermint. My understanding so far has been that:
If we took the snapshot for I suppose we can always discard BLS signatures from slashed validators during collection, and thus reject checkpoints signed by them as well, so we would know that an honest Tendermint validator set would not produce such a checkpoint. Ultimately, in each epoch we assume that +2/3rd are honest, and if we have to slash +1/3rd we have just proven that assumption to be wrong. Not sure if at that point if the +1/3rd dishonest people need to produce a real alternative fork, or just pretend to have signed a hidden fork, for us to conclude that Babylon failed. @fishermanymc wdyt? There was a problem hiding this comment. This is a complicated topic. Option-1: If slashing is 100%, then BLS signatures from the slashed validators are useless and should be ignored during signature aggregation. This is because even if they create an attacking QC and get caught later, they have nothing more to be slashed. These are the lunatic malicious validators here: https://babylon-chain.atlassian.net/wiki/spaces/BABYLON/pages/15597569/How+Many+Validators+Can+Babylon+Slash+During+an+Epoch+WIP#3.-Tendermint%E2%80%99s-Native-Slashing-Capacity-in-One-Block Option-2: If slashing is less than 100%, then the slashed nodes still have some stake in the system and thus are motivated to sign genuine BLS to get unbonded. There seems also theoretical tradeoff between how many to slash and how many BLS signatures should be collected, which affects the security - liveness tradeoff. David and Nusret are working on this. As our first pass, let's 1) not delay any slashing, and 2) ask the checkpointing module to only accept BLS signatures from unslashed validators from the original validator set at the beginning of the epoch. The aggregated power should be min(1/3, 1-X), where X is the slashed power. @gitferry , you are also a stakeholder. There was a problem hiding this comment. Thanks for the insightful discussions Akosh and Fisher! I also have a feeling that how many to slash and whether to jail seem to have connection with some fundamental trade-offs. Given that we are at the MVP phase and David and Nusret are working on related issues, at the moment let's follow the minimalist 2/3-secure approach proposed above, at least in this PR:
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| package keeper | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
|
|
||
| evidencekeeper "github.com/cosmos/cosmos-sdk/x/evidence/keeper" | ||
| evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types" | ||
| slashingkeeper "github.com/cosmos/cosmos-sdk/x/slashing/keeper" | ||
| stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" | ||
| ) | ||
|
|
||
| // HandleEquivocationEvidence implements an equivocation evidence handler. Assuming the | ||
| // evidence is valid, the validator committing the misbehavior will be slashed and tombstoned. | ||
| // Once tombstoned, the validator will not be able to recover. | ||
| // Note, the evidence contains the block time and height at the time of the equivocation. | ||
| // | ||
| // The evidence is considered invalid if: | ||
| // - the evidence is too old | ||
| // - the validator is unbonded or does not exist | ||
| // - the signing info does not exist (will panic) | ||
| // - is already tombstoned | ||
| // | ||
| // TODO: Some of the invalid constraints listed above may need to be reconsidered | ||
| // in the case of a lunatic attack. | ||
| // (adapted from https://github.com/cosmos/cosmos-sdk/blob/v0.45.5/x/evidence/keeper/infraction.go#L11-L123) | ||
| func HandleEquivocationEvidence(ctx sdk.Context, ek *evidencekeeper.Keeper, slk *slashingkeeper.Keeper, stk *stakingkeeper.Keeper, evidence *evidencetypes.Equivocation) { | ||
| logger := ek.Logger(ctx) | ||
| consAddr := evidence.GetConsensusAddress() | ||
|
|
||
| if _, err := slk.GetPubkey(ctx, consAddr.Bytes()); err != nil { | ||
| // Ignore evidence that cannot be handled. | ||
| // | ||
| // NOTE: We used to panic with: | ||
| // `panic(fmt.Sprintf("Validator consensus-address %v not found", consAddr))`, | ||
| // but this couples the expectations of the app to both Tendermint and | ||
| // the simulator. Both are expected to provide the full range of | ||
| // allowable but none of the disallowed evidence types. Instead of | ||
| // getting this coordination right, it is easier to relax the | ||
| // constraints and ignore evidence that cannot be handled. | ||
| return | ||
| } | ||
|
|
||
| // calculate the age of the evidence | ||
| infractionHeight := evidence.GetHeight() | ||
| infractionTime := evidence.GetTime() | ||
| ageDuration := ctx.BlockHeader().Time.Sub(infractionTime) | ||
| ageBlocks := ctx.BlockHeader().Height - infractionHeight | ||
|
|
||
| // Reject evidence if the double-sign is too old. Evidence is considered stale | ||
| // if the difference in time and number of blocks is greater than the allowed | ||
| // parameters defined. | ||
| cp := ctx.ConsensusParams() | ||
| if cp != nil && cp.Evidence != nil { | ||
| if ageDuration > cp.Evidence.MaxAgeDuration && ageBlocks > cp.Evidence.MaxAgeNumBlocks { | ||
| logger.Info( | ||
| "ignored equivocation; evidence too old", | ||
| "validator", consAddr, | ||
| "infraction_height", infractionHeight, | ||
| "max_age_num_blocks", cp.Evidence.MaxAgeNumBlocks, | ||
| "infraction_time", infractionTime, | ||
| "max_age_duration", cp.Evidence.MaxAgeDuration, | ||
| ) | ||
| return | ||
| } | ||
| } | ||
|
|
||
| validator := stk.ValidatorByConsAddr(ctx, consAddr) | ||
| if validator == nil || validator.IsUnbonded() { | ||
| // Defensive: Simulation doesn't take unbonding periods into account, and | ||
| // Tendermint might break this assumption at some point. | ||
| return | ||
| } | ||
|
|
||
| if ok := slk.HasValidatorSigningInfo(ctx, consAddr); !ok { | ||
| panic(fmt.Sprintf("expected signing info for validator %s but not found", consAddr)) | ||
| } | ||
|
|
||
| // ignore if the validator is already tombstoned | ||
| if slk.IsTombstoned(ctx, consAddr) { | ||
| logger.Info( | ||
| "ignored equivocation; validator already tombstoned", | ||
| "validator", consAddr, | ||
| "infraction_height", infractionHeight, | ||
| "infraction_time", infractionTime, | ||
| ) | ||
| return | ||
| } | ||
|
|
||
| logger.Info( | ||
| "confirmed equivocation", | ||
| "validator", consAddr, | ||
| "infraction_height", infractionHeight, | ||
| "infraction_time", infractionTime, | ||
| ) | ||
|
|
||
| // We need to retrieve the stake distribution which signed the block, so we | ||
| // subtract ValidatorUpdateDelay from the evidence height. | ||
| // Note, that this *can* result in a negative "distributionHeight", up to | ||
| // -ValidatorUpdateDelay, 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. | ||
| distributionHeight := infractionHeight - sdk.ValidatorUpdateDelay | ||
|
|
||
| // Slash validator. The `power` is the int64 power of the validator as provided | ||
| // to/by Tendermint. This value is validator.Tokens as sent to Tendermint via | ||
| // ABCI, and now received as evidence. The fraction is passed in to separately | ||
| // to slash unbonding and rebonding delegations. | ||
| slk.Slash( | ||
| ctx, | ||
| consAddr, | ||
| slk.SlashFractionDoubleSign(ctx), | ||
| evidence.GetValidatorPower(), distributionHeight, | ||
| ) | ||
|
|
||
| // // Jail the validator if not already jailed. This will begin unbonding the | ||
| // // validator if not already unbonding (tombstoned). | ||
| // if !validator.IsJailed() { | ||
| // slk.Jail(ctx, consAddr) | ||
| // } | ||
|
|
||
| // slk.JailUntil(ctx, consAddr, evidencetypes.DoubleSignJailEndTime) | ||
| slk.Tombstone(ctx, consAddr) | ||
| ek.SetEvidence(ctx, evidence) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| package keeper | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" | ||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
|
|
||
| slashingkeeper "github.com/cosmos/cosmos-sdk/x/slashing/keeper" | ||
| slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" | ||
| stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" | ||
| ) | ||
|
|
||
| // HandleValidatorSignature handles a validator signature (for slashing equivocating validators) | ||
| // called once per validator per block. | ||
| // (adapted from https://github.com/cosmos/cosmos-sdk/blob/v0.45.5/x/slashing/keeper/infractions.go#L11-L126) | ||
| func HandleValidatorSignature(ctx sdk.Context, slk *slashingkeeper.Keeper, stk *stakingkeeper.Keeper, addr cryptotypes.Address, power int64, signed bool) { | ||
| logger := slk.Logger(ctx) | ||
| height := ctx.BlockHeight() | ||
|
|
||
| // fetch the validator public key | ||
| consAddr := sdk.ConsAddress(addr) | ||
| if _, err := slk.GetPubkey(ctx, addr); err != nil { | ||
| panic(fmt.Sprintf("Validator consensus-address %s not found", consAddr)) | ||
| } | ||
|
|
||
| // fetch signing info | ||
| signInfo, found := slk.GetValidatorSigningInfo(ctx, consAddr) | ||
| if !found { | ||
| panic(fmt.Sprintf("Expected signing info for validator %s but not found", consAddr)) | ||
| } | ||
|
|
||
| // this is a relative index, so it counts blocks the validator *should* have signed | ||
| // will use the 0-value default signing info if not present, except for start height | ||
| index := signInfo.IndexOffset % slk.SignedBlocksWindow(ctx) | ||
| signInfo.IndexOffset++ | ||
|
|
||
| // 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 := slk.GetValidatorMissedBlockBitArray(ctx, consAddr, index) | ||
| missed := !signed | ||
| switch { | ||
| case !previous && missed: | ||
| // Array value has changed from not missed to missed, increment counter | ||
| slk.SetValidatorMissedBlockBitArray(ctx, consAddr, index, true) | ||
| signInfo.MissedBlocksCounter++ | ||
| case previous && !missed: | ||
| // Array value has changed from missed to not missed, decrement counter | ||
| slk.SetValidatorMissedBlockBitArray(ctx, consAddr, index, false) | ||
| signInfo.MissedBlocksCounter-- | ||
| default: | ||
| // Array value at this index has not changed, no need to update counter | ||
| } | ||
|
|
||
| minSignedPerWindow := slk.MinSignedPerWindow(ctx) | ||
|
|
||
| if missed { | ||
| ctx.EventManager().EmitEvent( | ||
| sdk.NewEvent( | ||
| slashingtypes.EventTypeLiveness, | ||
| sdk.NewAttribute(slashingtypes.AttributeKeyAddress, consAddr.String()), | ||
| sdk.NewAttribute(slashingtypes.AttributeKeyMissedBlocks, fmt.Sprintf("%d", signInfo.MissedBlocksCounter)), | ||
| sdk.NewAttribute(slashingtypes.AttributeKeyHeight, fmt.Sprintf("%d", height)), | ||
| ), | ||
| ) | ||
|
|
||
| logger.Debug( | ||
| "absent validator", | ||
| "height", height, | ||
| "validator", consAddr.String(), | ||
| "missed", signInfo.MissedBlocksCounter, | ||
| "threshold", minSignedPerWindow, | ||
| ) | ||
| } | ||
|
|
||
| minHeight := signInfo.StartHeight + slk.SignedBlocksWindow(ctx) | ||
| maxMissed := slk.SignedBlocksWindow(ctx) - minSignedPerWindow | ||
|
|
||
| // if we are past the minimum height and the validator has missed too many blocks, punish them | ||
| if height > minHeight && signInfo.MissedBlocksCounter > maxMissed { | ||
| validator := stk.ValidatorByConsAddr(ctx, consAddr) | ||
| if validator != nil && !validator.IsJailed() { | ||
| // Downtime confirmed: slash the validator | ||
| // We need to retrieve the stake distribution which signed the block, so 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, | ||
| // 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. | ||
| distributionHeight := height - sdk.ValidatorUpdateDelay - 1 | ||
|
|
||
| ctx.EventManager().EmitEvent( | ||
| sdk.NewEvent( | ||
| slashingtypes.EventTypeSlash, | ||
| sdk.NewAttribute(slashingtypes.AttributeKeyAddress, consAddr.String()), | ||
| sdk.NewAttribute(slashingtypes.AttributeKeyPower, fmt.Sprintf("%d", power)), | ||
| sdk.NewAttribute(slashingtypes.AttributeKeyReason, slashingtypes.AttributeValueMissingSignature), | ||
| // sdk.NewAttribute(slashingtypes.AttributeKeyJailed, consAddr.String()), | ||
| ), | ||
| ) | ||
| stk.Slash(ctx, consAddr, distributionHeight, power, slk.SlashFractionDowntime(ctx)) | ||
| // stk.Jail(ctx, consAddr) | ||
|
|
||
| // signInfo.JailedUntil = ctx.BlockHeader().Time.Add(slk.DowntimeJailDuration(ctx)) | ||
|
|
||
| // We need to reset the counter & array so that the validator won't be immediately slashed for downtime upon rebonding. | ||
| signInfo.MissedBlocksCounter = 0 | ||
| signInfo.IndexOffset = 0 | ||
| // TODO: commented the line below as it's a private function. Find a way to work around this. | ||
| // slk.clearValidatorMissedBlockBitArray(ctx, consAddr) | ||
|
There was a problem hiding this comment. @vitsalis this seems to contradict the concept of being able to copy paste. It's also easy to miss this comment (maybe instead of a I didn't expect we have to copy paste slashing, though, only staking. |
||
|
|
||
| logger.Info( | ||
| // "slashing and jailing validator due to liveness fault", | ||
| "slashing validator due to liveness fault", | ||
| "height", height, | ||
| "validator", consAddr.String(), | ||
| "min_height", minHeight, | ||
| "threshold", minSignedPerWindow, | ||
| "slashed", slk.SlashFractionDowntime(ctx).String(), | ||
| // "jailed_until", signInfo.JailedUntil, | ||
| ) | ||
| } else { | ||
| // validator was (a) not found or (b) already jailed so we do not slash | ||
| logger.Info( | ||
| "validator would have been slashed for downtime, but was either not found in store or already jailed", | ||
| "validator", consAddr.String(), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| // Set the updated signing info | ||
| slk.SetValidatorSigningInfo(ctx, consAddr, signInfo) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both evidence and slashing are removed, but staking is kept? Can you explain what's the idea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Staking's BeginBlock processing only includes
TrackHistoricalInfo(see https://github.com/cosmos/cosmos-sdk/blob/v0.45.5/x/staking/abci.go), which basically does some cleanup job. Keeping it seems to be not harmful.