Skip to content
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

Merged
merged 52 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
e085b40
chore: merge main into feat/sdk-47-ibc-7 branch (#2625)
sainoe Jun 23, 2023
a8f0e87
resolve merge conflicts
mpoke Jun 29, 2023
ec7c870
feat!: SDK v0.47 & IBC v7 Base (#2541)
glnro Jul 4, 2023
f5fc4ba
feat: refactor base E2E tests (#2587)
sainoe Jul 12, 2023
0510ee7
feat: refactor global fee module for SDK v47 (#2660)
sainoe Jul 24, 2023
861ba39
deps: bump ics to v3.1; chore: update e2e (#2663)
MSalopek Jul 24, 2023
2e4d98d
chore: refactor remaining E2E tests for SDK v47 (#2744)
sainoe Sep 26, 2023
ff8acab
deps: bump go version to 1.21
MSalopek Dec 8, 2023
d0a672a
tests: update hermes images and setup (#2841)
MSalopek Dec 8, 2023
21a192a
deps: use lsm cosmos-sdk fork and ics 3.3.0-rc0-lsm (#2842)
MSalopek Dec 8, 2023
b4730f5
chore!: merge main into feat/sdk-47-ibc7 (#2851)
MSalopek Dec 12, 2023
925bfee
chore: merge main into feat/sdk-47-ibc-7; update broken tests (#2853)
MSalopek Dec 12, 2023
df6cb1a
Merge branch 'main' into feat/sdk-47-ibc-7
MSalopek Dec 12, 2023
a91f8cf
chore: post-merge cleanup
MSalopek Dec 12, 2023
fd63f6e
chore: post-merge cleanup
MSalopek Dec 12, 2023
ce86f31
chore: post-merge cleanup
MSalopek Dec 12, 2023
1b54845
deps: update migration to use latest cosmos-sdk/lsm
MSalopek Dec 13, 2023
c9b7817
chore: appease linter
MSalopek Dec 13, 2023
16a9c97
chore!: add min commission rates migration (prop 826)
MSalopek Dec 13, 2023
ed041ce
chore: add migration tests for v15
MSalopek Dec 14, 2023
ed08c80
chore: rm 3rd party
MSalopek Dec 15, 2023
304e982
migration: change MaxRate to match 0.05 if not provided
MSalopek Dec 18, 2023
89c5afe
nit: add back removed lines from historic v7 migration
MSalopek Dec 18, 2023
b5f8207
Merge branch 'feat/sdk-47-ibc-7' into masa/migrate-v15-validator-comm…
mpoke Jan 4, 2024
f9e494f
save
sainoe Jan 5, 2024
e75e9b6
lint
sainoe Jan 9, 2024
4a30eee
nit
sainoe Jan 10, 2024
d171594
draft version
sainoe Jan 12, 2024
846fd80
wip - refactor
sainoe Jan 12, 2024
2420752
refactor
sainoe Jan 15, 2024
1516387
refactoring
sainoe Jan 17, 2024
8a5d155
add CHANGELOG entry
sainoe Jan 18, 2024
8a0b312
nits
sainoe Jan 18, 2024
6b0f529
lint
sainoe Jan 18, 2024
18db793
nit
sainoe Jan 18, 2024
8a85583
refactor
sainoe Jan 18, 2024
9bb3ce7
add .changelog entry
sainoe Jan 18, 2024
2e9e16a
nits
sainoe Jan 18, 2024
726f35c
Merge branch 'masa/migrate-v15-validator-commission-rates' into saino…
sainoe Jan 18, 2024
cd9e764
lint
sainoe Jan 18, 2024
0aebe9b
doc
sainoe Jan 18, 2024
24f50f4
Merge branch 'sainoe/v15-si-upgrade' into sainoe/clawback-migration
sainoe Jan 18, 2024
3ed79a2
add .changelog entry
sainoe Jan 18, 2024
49a18b7
add tests
sainoe Jan 18, 2024
c336768
Merge branch 'main' into sainoe/clawback-migration
sainoe Jan 22, 2024
12af738
update test
sainoe Jan 22, 2024
919c237
lint
sainoe Jan 22, 2024
ea30b95
remove sdkerrors
sainoe Jan 22, 2024
4e0872e
Update app/upgrades/v15/upgrades.go
sainoe Jan 22, 2024
11eff7c
remove panics
sainoe Jan 22, 2024
7f50252
Update app/upgrades/v15/upgrades.go
sainoe Jan 22, 2024
6a22eee
refactor
sainoe Jan 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Migrate min commission rate migration rate staking parameter to 5%
([prop 826](https://www.mintscan.io/cosmos/proposals/826))
and updates the commission rate for all validators that have a commission
rate less than 5% ([\#2855](https://github.com/cosmos/gaia/pull/2855))



Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Migrate the signing infos of validators for which the consensus address
is missing, see https://github.com/cosmos/gaia/issues/1734
([\#2866](https://github.com/cosmos/gaia/pull/2866))



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))
261 changes: 260 additions & 1 deletion app/upgrades/v15/upgrades.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,36 @@
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,
Copy link
Contributor Author

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.

// 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,
Expand All @@ -21,7 +44,243 @@
return vm, err
}

ctx.Logger().Info("Upgrade complete")
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%
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This func is already merged into main.

// and updates the commission rate for all validators that have a commission rate less than 5%
func UpgradeMinCommissionRate(ctx sdk.Context, sk stakingkeeper.Keeper) {
params := sk.GetParams(ctx)
params.MinCommissionRate = sdk.NewDecWithPrec(5, 2)
err := sk.SetParams(ctx, params)
if err != nil {
panic(err)
Copy link
Contributor

Choose a reason for hiding this comment

The 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)
}
}
}

// UpgradeSigningInfos updates the signing infos of validators for which
// the consensus address is missing
func UpgradeSigningInfos(ctx sdk.Context, sk slashingkeeper.Keeper) {
signingInfos := []slashingtypes.ValidatorSigningInfo{}

// update consensus address in signing info
// using the store key of validators
sk.IterateValidatorSigningInfos(ctx, func(address sdk.ConsAddress, info slashingtypes.ValidatorSigningInfo) (stop bool) {
if info.Address == "" {
info.Address = address.String()
signingInfos = append(signingInfos, info)
}

return false
})

for _, si := range signingInfos {
addr, err := sdk.ConsAddressFromBech32(si.Address)
if err != nil {
panic(err)
}
sk.SetValidatorSigningInfo(ctx, addr, si)
}
}

// ClawbackVestingFunds transfers the unvested tokens from the given vesting account
// to the community pool
func ClawbackVestingFunds(ctx sdk.Context, address sdk.AccAddress, keepers *keepers.AppKeepers) {
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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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(
Copy link
Contributor

Choose a reason for hiding this comment

The 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 120 in app/upgrades/v15/upgrades.go

View workflow job for this annotation

GitHub Actions / golangci-lint

SA1019: sdkerrors.Wrap is deprecated: functionality of this package has been moved to it's own module: (staticcheck)

Check failure on line 120 in app/upgrades/v15/upgrades.go

View workflow job for this annotation

GitHub Actions / Analyze

SA1019: sdkerrors.Wrap is deprecated: functionality of this package has been moved to it's own module: (staticcheck)
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)
}

// transfers still vesting tokens of BondDenom to community pool
_, vestingCoins := vestAccount.GetVestingCoins(ctx.BlockTime()).Find(sk.BondDenom(ctx))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"GetVestingCoins returns the total number of vesting coins. If no coins are vesting, nil is returned." If by the time the upgrade happens there are no coints vesting, this line will fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)
Copy link
Contributor

Choose a reason for hiding this comment

The 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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why this needs to be vestAccount.BaseAccount and not just the vestAccount?

Copy link
Contributor Author

@sainoe sainoe Jan 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly bc vestAccount is a ContinuousVestingAccount type which isn't required after migration, since there will only remain unlocked and vested coins in the account.
Therefore, its embedded BaseAccount field that holds the basic account value is sufficient.

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto: avoid panicking

}
}

// forceUnbondAllDelegations unbonds all the delegations from the given account address,
// without 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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take it outside the if condition and also use it in UndelegateCoinsFromModuleToAccount.

err = bk.SendCoinsFromModuleToModule(ctx, stakingtypes.BondedPoolName, stakingtypes.NotBondedPoolName, coins)
if err != nil {
return err
}
}

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
}
}

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 218 in app/upgrades/v15/upgrades.go

View workflow job for this annotation

GitHub Actions / golangci-lint

SA1019: sdkerrors.Wrapf is deprecated: functionality of this package has been moved to it's own module: (staticcheck)

Check failure on line 218 in app/upgrades/v15/upgrades.go

View workflow job for this annotation

GitHub Actions / Analyze

SA1019: sdkerrors.Wrapf is deprecated: functionality of this package has been moved to it's own module: (staticcheck)
}

senderBal := bk.GetBalance(ctx, sender, amount.Denom)
if _, hasNeg := sdk.NewCoins(senderBal).SafeSub(amount); hasNeg {
return sdkerrors.Wrapf(

Check failure on line 223 in app/upgrades/v15/upgrades.go

View workflow job for this annotation

GitHub Actions / golangci-lint

SA1019: sdkerrors.Wrapf is deprecated: functionality of this package has been moved to it's own module: (staticcheck)

Check failure on line 223 in app/upgrades/v15/upgrades.go

View workflow job for this annotation

GitHub Actions / Analyze

SA1019: sdkerrors.Wrapf is deprecated: functionality of this package has been moved to it's own module: (staticcheck)
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
Copy link
Contributor

Choose a reason for hiding this comment

The 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 it follows the same logic of the sendBalance method in
// 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 259 in app/upgrades/v15/upgrades.go

View workflow job for this annotation

GitHub Actions / golangci-lint

SA1019: sdkerrors.Wrap is deprecated: functionality of this package has been moved to it's own module: (staticcheck)

Check failure on line 259 in app/upgrades/v15/upgrades.go

View workflow job for this annotation

GitHub Actions / Analyze

SA1019: sdkerrors.Wrap is deprecated: functionality of this package has been moved to it's own module: (staticcheck)
}

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
}
Loading
Loading