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 |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| package epoching | ||
|
|
||
| import ( | ||
| "time" | ||
|
|
||
| "github.com/babylonchain/babylon/x/epoching/keeper" | ||
| "github.com/babylonchain/babylon/x/epoching/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) | ||
|
|
||
| // TODO: unimplemented: | ||
| // - if the first block of an epoch, | ||
| // - increment epoch number | ||
| // - trigger hook and emit event | ||
| } | ||
|
|
||
| // 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,110 @@ | ||
| package keeper | ||
|
|
||
| import ( | ||
| abci "github.com/tendermint/tendermint/abci/types" | ||
|
|
||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| "github.com/cosmos/cosmos-sdk/x/staking/types" | ||
| tmproto "github.com/tendermint/tendermint/proto/tendermint/types" | ||
| ) | ||
|
|
||
| // ApplyMatureUnbonding | ||
| // - unbonds all mature validators/delegations, and | ||
| // - finishes all mature redelegations | ||
| // in the corresponding queues, where | ||
| // - an unbonding/redelegation becomes mature when its corresponding epoch and all previous epochs have been checkpointed. | ||
| // Triggered by the checkpointing module upon the above condition. | ||
| // (adapted from https://github.com/cosmos/cosmos-sdk/blob/v0.45.5/x/staking/keeper/val_state_change.go#L32-L91) | ||
| func (k *Keeper) ApplyMatureUnbonding(ctx sdk.Context, epochBoundaryHeader tmproto.Header) { | ||
| currHeader := ctx.BlockHeader() | ||
|
|
||
| // unbond all mature validators till the epoch boundary from the unbonding queue | ||
| ctx.WithBlockHeader(epochBoundaryHeader) | ||
| k.stk.UnbondAllMatureValidators(ctx) | ||
| ctx.WithBlockHeader(currHeader) | ||
|
|
||
| // get all mature unbonding delegations the epoch boundary from the ubd queue. | ||
| ctx.WithBlockHeader(epochBoundaryHeader) | ||
| matureUnbonds := k.stk.DequeueAllMatureUBDQueue(ctx, epochBoundaryHeader.Time) | ||
| ctx.WithBlockHeader(currHeader) | ||
| // unbond all mature delegations | ||
| for _, dvPair := range matureUnbonds { | ||
| addr, err := sdk.ValAddressFromBech32(dvPair.ValidatorAddress) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| delegatorAddress, err := sdk.AccAddressFromBech32(dvPair.DelegatorAddress) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| balances, err := k.stk.CompleteUnbonding(ctx, delegatorAddress, addr) | ||
| if err != nil { | ||
| continue | ||
| } | ||
|
|
||
| ctx.EventManager().EmitEvent( | ||
| sdk.NewEvent( | ||
| types.EventTypeCompleteUnbonding, | ||
| sdk.NewAttribute(sdk.AttributeKeyAmount, balances.String()), | ||
| sdk.NewAttribute(types.AttributeKeyValidator, dvPair.ValidatorAddress), | ||
| sdk.NewAttribute(types.AttributeKeyDelegator, dvPair.DelegatorAddress), | ||
| ), | ||
| ) | ||
| } | ||
|
|
||
| // get all mature redelegations till the epoch boundary from the red queue. | ||
| ctx.WithBlockHeader(epochBoundaryHeader) | ||
| matureRedelegations := k.stk.DequeueAllMatureRedelegationQueue(ctx, epochBoundaryHeader.Time) | ||
| ctx.WithBlockHeader(currHeader) | ||
| // finish all mature redelegations | ||
| for _, dvvTriplet := range matureRedelegations { | ||
| valSrcAddr, err := sdk.ValAddressFromBech32(dvvTriplet.ValidatorSrcAddress) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| valDstAddr, err := sdk.ValAddressFromBech32(dvvTriplet.ValidatorDstAddress) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| delegatorAddress, err := sdk.AccAddressFromBech32(dvvTriplet.DelegatorAddress) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| balances, err := k.stk.CompleteRedelegation( | ||
| ctx, | ||
| delegatorAddress, | ||
| valSrcAddr, | ||
| valDstAddr, | ||
| ) | ||
| if err != nil { | ||
| continue | ||
| } | ||
|
|
||
| ctx.EventManager().EmitEvent( | ||
| sdk.NewEvent( | ||
| types.EventTypeCompleteRedelegation, | ||
| sdk.NewAttribute(sdk.AttributeKeyAmount, balances.String()), | ||
| sdk.NewAttribute(types.AttributeKeyDelegator, dvvTriplet.DelegatorAddress), | ||
| sdk.NewAttribute(types.AttributeKeySrcValidator, dvvTriplet.ValidatorSrcAddress), | ||
| sdk.NewAttribute(types.AttributeKeyDstValidator, dvvTriplet.ValidatorDstAddress), | ||
| ), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| // ApplyAndReturnValidatorSetUpdates applies and return accumulated updates to the bonded validator set, including | ||
| // * Updates the active validator set as keyed by LastValidatorPowerKey. | ||
| // * Updates the total power as keyed by LastTotalPowerKey. | ||
| // * Updates validator status' according to updated powers. | ||
| // * Updates the fee pool bonded vs not-bonded tokens. | ||
| // * Updates relevant indices. | ||
| // Triggered upon every epoch. | ||
| // (adapted from https://github.com/cosmos/cosmos-sdk/blob/v0.45.5/x/staking/keeper/val_state_change.go#L18-L30) | ||
| func (k *Keeper) ApplyAndReturnValidatorSetUpdates(ctx sdk.Context) []abci.ValidatorUpdate { | ||
| validatorUpdates, err := k.stk.ApplyAndReturnValidatorSetUpdates(ctx) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| return validatorUpdates | ||
| } |
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.
I didn't think about that method! 😃