-
Notifications
You must be signed in to change notification settings - Fork 715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore! add clawback migration (prop 826) #2891
Changes from 45 commits
e085b40
a8f0e87
ec7c870
f5fc4ba
0510ee7
861ba39
2e4d98d
ff8acab
d0a672a
21a192a
b4730f5
925bfee
df6cb1a
a91f8cf
fd63f6e
ce86f31
1b54845
c9b7817
16a9c97
ed041ce
ed08c80
304e982
89c5afe
b5f8207
f9e494f
e75e9b6
4a30eee
d171594
846fd80
2420752
1516387
8a5d155
8a0b312
6b0f529
18db793
8a85583
9bb3ce7
2e9e16a
726f35c
cd9e764
0aebe9b
24f50f4
3ed79a2
49a18b7
c336768
12af738
919c237
ea30b95
4e0872e
11eff7c
7f50252
6a22eee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
Migrate vesting funds from "cosmos145hytrc49m0hn6fphp8d5h4xspwkawcuzmx498" | ||
to community pool according to signal prop [860](https://www.mintscan.io/cosmos/proposals/860) | ||
([\#2891](https://github.com/cosmos/gaia/pull/2891)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,35 @@ | ||
package v15 | ||
|
||
import ( | ||
"github.com/cosmos/cosmos-sdk/store/prefix" | ||
storetypes "github.com/cosmos/cosmos-sdk/store/types" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
"github.com/cosmos/cosmos-sdk/types/address" | ||
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | ||
"github.com/cosmos/cosmos-sdk/types/module" | ||
accountkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" | ||
vesting "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" | ||
bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper" | ||
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" | ||
distributionkeeper "github.com/cosmos/cosmos-sdk/x/distribution/keeper" | ||
distributiontypes "github.com/cosmos/cosmos-sdk/x/distribution/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" | ||
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" | ||
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" | ||
|
||
"github.com/cosmos/gaia/v15/app/keepers" | ||
) | ||
|
||
// CreateUpgradeHandler returns a upgrade handler for Gaia v15 | ||
// which executes the following migrations: | ||
// - adhere to prop 826 which sets the minimum commission rate to 5% for all validators, | ||
// see https://www.mintscan.io/cosmos/proposals/826 | ||
// - update the slashing module SigningInfos for which the consensus address is empty, | ||
// see https://github.com/cosmos/gaia/issues/1734. | ||
// - adhere to signal prop 860 which claws back vesting funds | ||
// see https://www.mintscan.io/cosmos/proposals/860 | ||
func CreateUpgradeHandler( | ||
mm *module.Manager, | ||
configurator module.Configurator, | ||
|
@@ -28,14 +43,45 @@ | |
return vm, err | ||
} | ||
|
||
UpgradeSigningInfos(ctx, keepers.SlashingKeeper) | ||
UpgradeMinCommissionRate(ctx, *keepers.StakingKeeper) | ||
UpgradeSigningInfos(ctx, keepers.SlashingKeeper) | ||
ClawbackVestingFunds(ctx, sdk.MustAccAddressFromBech32("cosmos145hytrc49m0hn6fphp8d5h4xspwkawcuzmx498"), keepers) | ||
|
||
ctx.Logger().Info("Upgrade v15 complete") | ||
return vm, err | ||
} | ||
} | ||
|
||
// UpgradeMinCommissionRate sets the minimum commission rate staking parameter to 5% | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This func is already merged into |
||
// and updates the commission rate for all validators that have a commission rate less than 5% | ||
// adhere to prop 826 which sets the minimum commission rate to 5% for all validators | ||
// https://www.mintscan.io/cosmos/proposals/826 | ||
func UpgradeMinCommissionRate(ctx sdk.Context, sk stakingkeeper.Keeper) { | ||
ctx.Logger().Info("Migrating min commission rate...") | ||
|
||
params := sk.GetParams(ctx) | ||
params.MinCommissionRate = sdk.NewDecWithPrec(5, 2) | ||
err := sk.SetParams(ctx, params) | ||
if err != nil { | ||
panic(err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto: avoid panicking |
||
} | ||
|
||
for _, val := range sk.GetAllValidators(ctx) { | ||
if val.Commission.CommissionRates.Rate.LT(sdk.NewDecWithPrec(5, 2)) { | ||
// set the commission rate to 5% | ||
val.Commission.CommissionRates.Rate = sdk.NewDecWithPrec(5, 2) | ||
// set the max rate to 5% if it is less than 5% | ||
if val.Commission.CommissionRates.MaxRate.LT(sdk.NewDecWithPrec(5, 2)) { | ||
val.Commission.CommissionRates.MaxRate = sdk.NewDecWithPrec(5, 2) | ||
} | ||
val.Commission.UpdateTime = ctx.BlockHeader().Time | ||
sk.SetValidator(ctx, val) | ||
} | ||
} | ||
|
||
ctx.Logger().Info("Finished migrating min commission rate") | ||
} | ||
|
||
// UpgradeSigningInfos updates the signing infos of validators for which | ||
// the consensus address is missing | ||
func UpgradeSigningInfos(ctx sdk.Context, sk slashingkeeper.Keeper) { | ||
|
@@ -66,31 +112,189 @@ | |
ctx.Logger().Info("Finished migrating signing infos") | ||
} | ||
|
||
// UpgradeMinCommissionRate sets the minimum commission rate staking parameter to 5% | ||
// and updates the commission rate for all validators that have a commission rate less than 5% | ||
// adhere to prop 826 which sets the minimum commission rate to 5% for all validators | ||
// https://www.mintscan.io/cosmos/proposals/826 | ||
func UpgradeMinCommissionRate(ctx sdk.Context, sk stakingkeeper.Keeper) { | ||
ctx.Logger().Info("Migrating min commission rate...") | ||
// ClawbackVestingFunds transfers the vesting tokens from the given vesting account | ||
// to the community pool | ||
func ClawbackVestingFunds(ctx sdk.Context, address sdk.AccAddress, keepers *keepers.AppKeepers) { | ||
ctx.Logger().Info("Migrating vesting funds...") | ||
|
||
params := sk.GetParams(ctx) | ||
params.MinCommissionRate = sdk.NewDecWithPrec(5, 2) | ||
err := sk.SetParams(ctx, params) | ||
ak := keepers.AccountKeeper | ||
bk := keepers.BankKeeper | ||
dk := keepers.DistrKeeper | ||
sk := *keepers.StakingKeeper | ||
|
||
// get target account | ||
account := ak.GetAccount(ctx, address) | ||
|
||
// verify that it's a vesting account type | ||
vestAccount, ok := account.(*vesting.ContinuousVestingAccount) | ||
if !ok { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not returning an error here shouldn't affect the states and prevent breaking the upgrade test of the git workflow. |
||
panic( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid panicking in the upgrade handler. Just log an error. Worst case scenario, we need to do another upgrade to do the migration again. But if the upgrade fails, it's much harder to deal with it. |
||
sdkerrors.Wrap( | ||
Check failure on line 132 in app/upgrades/v15/upgrades.go GitHub Actions / Analyze
|
||
sdkerrors.ErrInvalidType, | ||
"account with address %s isn't a vesting account", | ||
), | ||
) | ||
} | ||
|
||
// unbond all delegations from vesting account | ||
err := forceUnbondAllDelegations(sk, bk, ctx, address) | ||
if err != nil { | ||
panic(err) | ||
} | ||
|
||
for _, val := range sk.GetAllValidators(ctx) { | ||
if val.Commission.CommissionRates.Rate.LT(sdk.NewDecWithPrec(5, 2)) { | ||
// set the commission rate to 5% | ||
val.Commission.CommissionRates.Rate = sdk.NewDecWithPrec(5, 2) | ||
// set the max rate to 5% if it is less than 5% | ||
if val.Commission.CommissionRates.MaxRate.LT(sdk.NewDecWithPrec(5, 2)) { | ||
val.Commission.CommissionRates.MaxRate = sdk.NewDecWithPrec(5, 2) | ||
// transfers still vesting tokens of BondDenom to community pool | ||
_, vestingCoins := vestAccount.GetVestingCoins(ctx.BlockTime()).Find(sk.BondDenom(ctx)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. " There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch. |
||
err = forceFundCommunityPool( | ||
ak, | ||
dk, | ||
bk, | ||
ctx, | ||
vestingCoins, | ||
address, | ||
keepers.GetKey(banktypes.StoreKey), | ||
) | ||
if err != nil { | ||
panic(err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto: avoid panicking |
||
} | ||
|
||
// overwrite vesting account using its embedded base account | ||
ak.SetAccount(ctx, vestAccount.BaseAccount) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain why this needs to be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mainly bc The other option would have required updating the vesting account's original amount and end time, since it's lazy and calculates the vesting/vested coins according to the current block time rather than storing it. |
||
|
||
// validate account balance | ||
err = bk.ValidateBalance(ctx, address) | ||
if err != nil { | ||
panic(err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto: avoid panicking |
||
} | ||
|
||
ctx.Logger().Info("Finished migrating vesting funds") | ||
} | ||
|
||
// forceUnbondAllDelegations unbonds all the delegations from the given account address, | ||
// without waiting for an unbonding period | ||
func forceUnbondAllDelegations( | ||
sk stakingkeeper.Keeper, | ||
bk bankkeeper.Keeper, | ||
ctx sdk.Context, | ||
delegator sdk.AccAddress, | ||
) error { | ||
dels := sk.GetDelegatorDelegations(ctx, delegator, 100) | ||
|
||
for _, del := range dels { | ||
valAddr := del.GetValidatorAddr() | ||
|
||
validator, found := sk.GetValidator(ctx, valAddr) | ||
if !found { | ||
return stakingtypes.ErrNoValidatorFound | ||
} | ||
|
||
returnAmount, err := sk.Unbond(ctx, delegator, valAddr, del.GetShares()) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// transfer the validator tokens to the not bonded pool | ||
if validator.IsBonded() { | ||
// doing stakingKeeper.bondedTokensToNotBonded | ||
coins := sdk.NewCoins(sdk.NewCoin(sk.BondDenom(ctx), returnAmount)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Take it outside the if condition and also use it in |
||
err = bk.SendCoinsFromModuleToModule(ctx, stakingtypes.BondedPoolName, stakingtypes.NotBondedPoolName, coins) | ||
if err != nil { | ||
return err | ||
} | ||
val.Commission.UpdateTime = ctx.BlockHeader().Time | ||
sk.SetValidator(ctx, val) | ||
} | ||
|
||
bondDenom := sk.GetParams(ctx).BondDenom | ||
amt := sdk.NewCoin(bondDenom, returnAmount) | ||
|
||
err = bk.UndelegateCoinsFromModuleToAccount(ctx, stakingtypes.NotBondedPoolName, delegator, sdk.NewCoins(amt)) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
ctx.Logger().Info("Finished migrating min commission rate") | ||
|
||
return nil | ||
} | ||
|
||
// forceFundCommunityPool sends the given coin from the sender account to the community pool | ||
// even if the coin is locked. | ||
// Note that it partially follows the logic of the FundCommunityPool method in | ||
// https://github.com/cosmos/cosmos-sdk/blob/release%2Fv0.47.x/x/distribution/keeper/keeper.go#L155 | ||
func forceFundCommunityPool( | ||
mpoke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ak accountkeeper.AccountKeeper, | ||
dk distributionkeeper.Keeper, | ||
bk bankkeeper.Keeper, | ||
ctx sdk.Context, | ||
amount sdk.Coin, | ||
sender sdk.AccAddress, | ||
bs storetypes.StoreKey, | ||
) error { | ||
recipientAcc := ak.GetModuleAccount(ctx, distributiontypes.ModuleName) | ||
if recipientAcc == nil { | ||
return sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, distributiontypes.ModuleName) | ||
Check failure on line 232 in app/upgrades/v15/upgrades.go GitHub Actions / Analyze
|
||
} | ||
|
||
senderBal := bk.GetBalance(ctx, sender, amount.Denom) | ||
if _, hasNeg := sdk.NewCoins(senderBal).SafeSub(amount); hasNeg { | ||
return sdkerrors.Wrapf( | ||
Check failure on line 237 in app/upgrades/v15/upgrades.go GitHub Actions / Analyze
|
||
sdkerrors.ErrInsufficientFunds, | ||
"spendable balance %s is smaller than %s", | ||
senderBal, amount, | ||
) | ||
} | ||
if err := setBalance(ctx, sender, senderBal.Sub(amount), bs); err != nil { | ||
return err | ||
} | ||
recipientBal := bk.GetBalance(ctx, recipientAcc.GetAddress(), amount.Denom) | ||
if err := setBalance(ctx, recipientAcc.GetAddress(), recipientBal.Add(amount), bs); err != nil { | ||
return err | ||
} | ||
|
||
accExists := ak.HasAccount(ctx, recipientAcc.GetAddress()) | ||
if !accExists { | ||
ak.SetAccount(ctx, ak.NewAccountWithAddress(ctx, recipientAcc.GetAddress())) | ||
} | ||
Comment on lines
+276
to
+279
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIUC, this check that the CP account exists. Is that right? |
||
|
||
feePool := dk.GetFeePool(ctx) | ||
feePool.CommunityPool = feePool.CommunityPool.Add(sdk.NewDecCoinsFromCoins(amount)...) | ||
dk.SetFeePool(ctx, feePool) | ||
|
||
return nil | ||
} | ||
|
||
// setBalance sets the coin balance for an account by address. | ||
// Note that it follows the same logic of the sendBalance method in | ||
sainoe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// https://github.com/cosmos/cosmos-sdk/blob/release%2Fv0.47.x/x/bank/keeper/send.go#L337 | ||
sainoe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func setBalance( | ||
ctx sdk.Context, | ||
addr sdk.AccAddress, | ||
balance sdk.Coin, | ||
bs storetypes.StoreKey, | ||
) error { | ||
if !balance.IsValid() { | ||
return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, balance.String()) | ||
Check failure on line 273 in app/upgrades/v15/upgrades.go GitHub Actions / Analyze
|
||
} | ||
|
||
store := ctx.KVStore(bs) | ||
accountStore := prefix.NewStore(store, banktypes.CreateAccountBalancesPrefix(addr)) | ||
denomPrefixStore := prefix.NewStore(store, banktypes.CreateDenomAddressPrefix(balance.Denom)) | ||
|
||
if balance.IsZero() { | ||
accountStore.Delete([]byte(balance.Denom)) | ||
denomPrefixStore.Delete(address.MustLengthPrefix(addr)) | ||
} else { | ||
amount, err := balance.Amount.Marshal() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
accountStore.Set([]byte(balance.Denom), amount) | ||
|
||
// Store a reverse index from denomination to account address with a | ||
// sentinel value. | ||
denomAddrKey := address.MustLengthPrefix(addr) | ||
if !denomPrefixStore.Has(denomAddrKey) { | ||
denomPrefixStore.Set(denomAddrKey, []byte{0}) | ||
} | ||
} | ||
|
||
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.
That's already in
main
.