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

revert: Turn staking power reduction into an on-chain param #9495

Merged
merged 4 commits into from
Jun 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ if input key is empty, or input data contains empty key.
* (x/gov) [\#8473](https://github.com/cosmos/cosmos-sdk/pull/8473) On genesis init, if the gov module account balance, coming from bank module state, does not match the one in gov module state, the initialization will panic.
* (x/distribution) [\#8473](https://github.com/cosmos/cosmos-sdk/pull/8473) On genesis init, if the distribution module account balance, coming from bank module state, does not match the one in distribution module state, the initialization will panic.
* (client/keys) [\#8500](https://github.com/cosmos/cosmos-sdk/pull/8500) `InfoImporter` interface is removed from legacy keybase.
* (x/staking) [\#8505](https://github.com/cosmos/cosmos-sdk/pull/8505) `sdk.PowerReduction` has been renamed to `sdk.DefaultPowerReduction`, and most staking functions relying on power reduction take a new function argument, instead of relying on that global variable.
* [\#8629](https://github.com/cosmos/cosmos-sdk/pull/8629) Deprecated `SetFullFundraiserPath` from `Config` in favor of `SetPurpose` and `SetCoinType`.
* (x/upgrade) [\#8673](https://github.com/cosmos/cosmos-sdk/pull/8673) Remove IBC logic from x/upgrade. Deprecates IBC fields in an Upgrade Plan. IBC upgrade logic moved to 02-client and an IBC UpgradeProposal is added.
* (x/bank) [\#8517](https://github.com/cosmos/cosmos-sdk/pull/8517) `SupplyI` interface and `Supply` are removed and uses `sdk.Coins` for supply tracking
Expand Down Expand Up @@ -126,7 +127,6 @@ if input key is empty, or input data contains empty key.
* (x/bank) [\#8656](https://github.com/cosmos/cosmos-sdk/pull/8656) balance and supply are now correctly tracked via `coin_spent`, `coin_received`, `coinbase` and `burn` events.
* (x/bank) [\#8517](https://github.com/cosmos/cosmos-sdk/pull/8517) Supply is now stored and tracked as `sdk.Coins`
* (store) [\#8790](https://github.com/cosmos/cosmos-sdk/pull/8790) Reduce gas costs by 10x for transient store operations.
* (x/staking) [\#8505](https://github.com/cosmos/cosmos-sdk/pull/8505) Convert staking power reduction into an on-chain parameter rather than a hardcoded in-code variable.
* (x/bank) [\#9051](https://github.com/cosmos/cosmos-sdk/pull/9051) Supply value is stored as `sdk.Int` rather than `string`.

### Improvements
Expand Down
1 change: 0 additions & 1 deletion docs/core/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -6394,7 +6394,6 @@ Params defines the parameters for the staking module.
| `max_entries` | [uint32](#uint32) | | max_entries is the max entries for either unbonding delegation or redelegation (per pair/trio). |
| `historical_entries` | [uint32](#uint32) | | historical_entries is the number of historical entries to persist. |
| `bond_denom` | [string](#string) | | bond_denom defines the bondable coin denomination. |
| `power_reduction` | [string](#string) | | power_reduction is the amount of staking tokens required for 1 unit of consensus-engine power |



Expand Down
6 changes: 0 additions & 6 deletions proto/cosmos/staking/v1beta1/staking.proto
Original file line number Diff line number Diff line change
Expand Up @@ -282,12 +282,6 @@ message Params {
uint32 historical_entries = 4 [(gogoproto.moretags) = "yaml:\"historical_entries\""];
// bond_denom defines the bondable coin denomination.
string bond_denom = 5 [(gogoproto.moretags) = "yaml:\"bond_denom\""];
// power_reduction is the amount of staking tokens required for 1 unit of consensus-engine power
string power_reduction = 6 [
(gogoproto.moretags) = "yaml:\"power_reduction\"",
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
(gogoproto.nullable) = false
];
}

// DelegationResponse is equivalent to Delegation except that it contains a
Expand Down
3 changes: 1 addition & 2 deletions x/staking/client/testutil/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -867,13 +867,12 @@ func (s *IntegrationTestSuite) TestGetCmdQueryParams() {
historical_entries: 10000
max_entries: 7
max_validators: 100
power_reduction: "1000000"
unbonding_time: 1814400s`,
},
{
"with json output",
[]string{fmt.Sprintf("--%s=json", tmcli.OutputFlag)},
`{"unbonding_time":"1814400s","max_validators":100,"max_entries":7,"historical_entries":10000,"bond_denom":"stake","power_reduction":"1000000"}`,
`{"unbonding_time":"1814400s","max_validators":100,"max_entries":7,"historical_entries":10000,"bond_denom":"stake"}`,
},
}
for _, tc := range testCases {
Expand Down
45 changes: 0 additions & 45 deletions x/staking/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,51 +42,6 @@ func bootstrapHandlerGenesisTest(t *testing.T, power int64, numAddrs int, accAmo
return app, ctx, addrDels, addrVals
}

func TestPowerReductionChangeValidatorSetUpdates(t *testing.T) {
initPower := int64(1000000)
app, ctx, _, valAddrs := bootstrapHandlerGenesisTest(t, initPower, 10, sdk.TokensFromConsensusPower(initPower, sdk.DefaultPowerReduction))
validatorAddr, validatorAddr3 := valAddrs[0], valAddrs[1]
tstaking := teststaking.NewHelper(t, ctx, app.StakingKeeper)

// create validator
tstaking.CreateValidatorWithValPower(validatorAddr, PKs[0], initPower, true)

// must end-block
updates, err := app.StakingKeeper.ApplyAndReturnValidatorSetUpdates(ctx)
require.NoError(t, err)
require.Equal(t, 1, len(updates))

// create a second validator keep it bonded
tstaking.CreateValidatorWithValPower(validatorAddr3, PKs[2], initPower, true)

// must end-block
updates, err = app.StakingKeeper.ApplyAndReturnValidatorSetUpdates(ctx)
require.NoError(t, err)
require.Equal(t, 1, len(updates))

// modify power reduction to 10 times bigger one
params := app.StakingKeeper.GetParams(ctx)
params.PowerReduction = sdk.DefaultPowerReduction.Mul(sdk.NewInt(10))
app.StakingKeeper.SetParams(ctx, params)

// validator updates for tendermint power
updates, err = app.StakingKeeper.ApplyAndReturnValidatorSetUpdates(ctx)
require.NoError(t, err)
require.Equal(t, 2, len(updates))
require.Equal(t, updates[0].Power, initPower/10)
require.Equal(t, updates[1].Power, initPower/10)

// power reduction back to default
params = app.StakingKeeper.GetParams(ctx)
params.PowerReduction = sdk.DefaultPowerReduction
app.StakingKeeper.SetParams(ctx, params)

// validator updates for tendermint power
updates, err = app.StakingKeeper.ApplyAndReturnValidatorSetUpdates(ctx)
require.NoError(t, err)
require.Equal(t, 2, len(updates))
}

func TestValidatorByPowerIndex(t *testing.T) {
initPower := int64(1000000)
app, ctx, _, valAddrs := bootstrapHandlerGenesisTest(t, initPower, 10, sdk.TokensFromConsensusPower(initPower, sdk.DefaultPowerReduction))
Expand Down
2 changes: 1 addition & 1 deletion x/staking/keeper/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ func NewMigrator(keeper Keeper) Migrator {

// Migrate1to2 migrates from version 1 to 2.
func (m Migrator) Migrate1to2(ctx sdk.Context) error {
return v043.MigrateStore(ctx, m.keeper.storeKey, m.keeper.paramstore)
return v043.MigrateStore(ctx, m.keeper.storeKey)
}
12 changes: 6 additions & 6 deletions x/staking/keeper/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ func (k Keeper) BondDenom(ctx sdk.Context) (res string) {
return
}

// PowerReduction - is the amount of staking tokens required for 1 unit of consensus-engine power
// governance can update it on a running chain
func (k Keeper) PowerReduction(ctx sdk.Context) (res sdk.Int) {
k.paramstore.Get(ctx, types.KeyPowerReduction, &res)
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
return
// PowerReduction - is the amount of staking tokens required for 1 unit of consensus-engine power.
// Currently, this returns a global variable that the app developer can tweak.
// TODO: we might turn this into an on-chain param:
// https://github.com/cosmos/cosmos-sdk/issues/8365
func (k Keeper) PowerReduction(ctx sdk.Context) sdk.Int {
return sdk.DefaultPowerReduction
}

// Get all parameteras as types.Params
Expand All @@ -54,7 +55,6 @@ func (k Keeper) GetParams(ctx sdk.Context) types.Params {
k.MaxEntries(ctx),
k.HistoricalEntries(ctx),
k.BondDenom(ctx),
k.PowerReduction(ctx),
)
}

Expand Down
13 changes: 0 additions & 13 deletions x/staking/keeper/power_reduction_test.go
Original file line number Diff line number Diff line change
@@ -1,22 +1,9 @@
package keeper_test

import (
"math/big"

sdk "github.com/cosmos/cosmos-sdk/types"
)

func (suite *KeeperTestSuite) TestPowerReductionChange() {
// modify power reduction
newPowerReduction := sdk.NewIntFromBigInt(new(big.Int).Exp(big.NewInt(10), big.NewInt(12), nil))
params := suite.app.StakingKeeper.GetParams(suite.ctx)
params.PowerReduction = newPowerReduction
suite.app.StakingKeeper.SetParams(suite.ctx, params)

// check power reduction change
suite.Require().Equal(newPowerReduction, suite.app.StakingKeeper.PowerReduction(suite.ctx))
}

func (suite *KeeperTestSuite) TestTokensToConsensusPower() {
suite.Require().Equal(int64(0), suite.app.StakingKeeper.TokensToConsensusPower(suite.ctx, sdk.DefaultPowerReduction.Sub(sdk.NewInt(1))))
suite.Require().Equal(int64(1), suite.app.StakingKeeper.TokensToConsensusPower(suite.ctx, sdk.DefaultPowerReduction))
Expand Down
2 changes: 1 addition & 1 deletion x/staking/keeper/val_state_change.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (k Keeper) BlockValidatorUpdates(ctx sdk.Context) []abci.ValidatorUpdate {
func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx sdk.Context) (updates []abci.ValidatorUpdate, err error) {
params := k.GetParams(ctx)
maxValidators := params.MaxValidators
powerReduction := params.PowerReduction
powerReduction := k.PowerReduction(ctx)
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

totalPower := sdk.ZeroInt()
amtFromBondedToNotBonded, amtFromNotBondedToBonded := sdk.ZeroInt(), sdk.ZeroInt()

Expand Down
138 changes: 0 additions & 138 deletions x/staking/legacy/v043/json.go

This file was deleted.

67 changes: 0 additions & 67 deletions x/staking/legacy/v043/json_test.go

This file was deleted.

Loading