feat: unfork cosmos sdk #17
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| package staking | ||
|
|
||
| import ( | ||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" | ||
| stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" | ||
| ) | ||
|
|
||
| const ( | ||
| ColumbusChainID = "columbus-5" | ||
| ) | ||
|
|
||
| var _ stakingtypes.StakingHooks = &TerraStakingHooks{} | ||
|
|
||
| // TerraStakingHooks implements staking hooks to enforce validator power limit | ||
| type TerraStakingHooks struct { | ||
| sk stakingkeeper.Keeper | ||
| } | ||
|
|
||
| func NewTerraStakingHooks(sk stakingkeeper.Keeper) *TerraStakingHooks { | ||
| return &TerraStakingHooks{sk: sk} | ||
| } | ||
|
|
||
| // Implement required staking hooks interface methods | ||
| func (h TerraStakingHooks) BeforeDelegationCreated(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) error { | ||
| return nil | ||
| } | ||
|
|
||
| func (h TerraStakingHooks) BeforeDelegationSharesModified(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) error { | ||
| return nil | ||
| } | ||
|
|
||
| // Other required hook methods with empty implementations | ||
| func (h TerraStakingHooks) AfterDelegationModified(ctx sdk.Context, _ sdk.AccAddress, valAddr sdk.ValAddress) error { | ||
|
There was a problem hiding this comment. Would it make sense to modify and put the logic into the "BeforeDelegationCreated" and "BeforeDelegationSharesModified" functions using a helper function? This would allow an early exit like the current fork change does. Or is there a good reason to rather do it after? There was a problem hiding this comment. The reason why i don't use those hooks is because the hooks only provide us with this context |
||
| if ctx.ChainID() != ColumbusChainID { | ||
| return nil | ||
| } | ||
|
|
||
| validator, found := h.sk.GetValidator(ctx, valAddr) | ||
| if !found { | ||
| return nil | ||
| } | ||
|
|
||
| // Get validator's current power (after delegation modified) | ||
| validatorPower := sdk.TokensToConsensusPower(validator.Tokens, h.sk.PowerReduction(ctx)) | ||
|
|
||
| // Get the total power of the validator set | ||
| totalPower := h.sk.GetLastTotalPower(ctx) | ||
|
|
||
| // Get validator delegation percent | ||
| validatorDelegationPercent := sdk.NewDec(validatorPower).QuoInt64(totalPower.Int64()) | ||
|
|
||
| if validatorDelegationPercent.GT(sdk.NewDecWithPrec(20, 2)) { | ||
| panic("validator power is over the allowed limit") | ||
|
There was a problem hiding this comment. Would it be possible to use a normal error instead of a panic here? I am not 100% sure why a panic was used in the original code, maybe you can clarify on that although you didn't write it. There was a problem hiding this comment. if subtractAccount {
if tokenSrc == types.Bonded {
panic("delegation token source cannot be bonded")
}
var sendName string
switch {
case validator.IsBonded():
sendName = types.BondedPoolName
case validator.IsUnbonding(), validator.IsUnbonded():
sendName = types.NotBondedPoolName
default:
panic("invalid validator status")
}i think it align with the current checks logic. I think we should keep it as it is, but lets me find out if it is better to return err here There was a problem hiding this comment. Do you also need to handle the event of creating a new validator? There was a problem hiding this comment. @StrathCole the hook |
||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (h TerraStakingHooks) BeforeValidatorSlashed(_ sdk.Context, _ sdk.ValAddress, _ sdk.Dec) error { | ||
| return nil | ||
| } | ||
|
|
||
| func (h TerraStakingHooks) BeforeValidatorModified(_ sdk.Context, _ sdk.ValAddress) error { | ||
| return nil | ||
| } | ||
|
|
||
| func (h TerraStakingHooks) AfterValidatorBonded(_ sdk.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error { | ||
| return nil | ||
| } | ||
|
|
||
| func (h TerraStakingHooks) AfterValidatorBeginUnbonding(_ sdk.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error { | ||
| return nil | ||
| } | ||
|
|
||
| func (h TerraStakingHooks) AfterValidatorRemoved(_ sdk.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error { | ||
| return nil | ||
| } | ||
|
|
||
| func (h TerraStakingHooks) AfterUnbondingInitiated(_ sdk.Context, _ uint64) error { | ||
| return nil | ||
| } | ||
|
|
||
| // Add this method to TerraStakingHooks | ||
| func (h TerraStakingHooks) AfterValidatorCreated(_ sdk.Context, _ sdk.ValAddress) error { | ||
| return nil | ||
| } | ||
|
|
||
| // Add the missing method | ||
| func (h TerraStakingHooks) BeforeDelegationRemoved(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) error { | ||
| return nil | ||
| } | ||
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 am aware it is "clean" to create a full hook file. Given that only one function is used, would it make sense to create a simple hook file that inherits the distribution hooks file and in the function you alter first calls the inherited hook and then the additional code? This would remove one hook from the multihook list in the stakingkeeper.
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 meant like this:
And in the app when setting up the hooks something like:
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.
the distribution hooks is used for supporting rewards operation stuff, and the hook that i used already have the implementation. Imo, i think keeping it as the separate file though make the code looks "long", but will be cleaner and easier to maintain in the future
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.
My only thought was to remove the small overhead of calling all the hook functions that aren't used anyway. But it's nothing I would insist in.