Skip to content

Commit 5cd741a

Browse files
authored
revert: Turn staking power reduction into an on-chain param (#9495)
<!-- The default pull request template is for types feat, fix, or refactor. For other templates, add one of the following parameters to the url: - template=docs.md - template=other.md --> ## Description Closes: #9447 This PR partially reverts #8505. Namely: - it removes PowerReduction as a staking on-chain param - however, it keeps #8505's API changes regarding adding a `powerReduction` function argument to staking functions. This allows us to rely less on global variables in said functions. <!-- Add a description of the changes that this PR introduces and the files that are the most critical to review. --> --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)
1 parent 179c819 commit 5cd741a

File tree

17 files changed

+740
-1091
lines changed

17 files changed

+740
-1091
lines changed

CHANGELOG.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ if input key is empty, or input data contains empty key.
8686
* (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.
8787
* (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.
8888
* (client/keys) [\#8500](https://github.com/cosmos/cosmos-sdk/pull/8500) `InfoImporter` interface is removed from legacy keybase.
89+
* (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.
8990
* [\#8629](https://github.com/cosmos/cosmos-sdk/pull/8629) Deprecated `SetFullFundraiserPath` from `Config` in favor of `SetPurpose` and `SetCoinType`.
9091
* (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.
9192
* (x/bank) [\#8517](https://github.com/cosmos/cosmos-sdk/pull/8517) `SupplyI` interface and `Supply` are removed and uses `sdk.Coins` for supply tracking
@@ -126,7 +127,6 @@ if input key is empty, or input data contains empty key.
126127
* (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.
127128
* (x/bank) [\#8517](https://github.com/cosmos/cosmos-sdk/pull/8517) Supply is now stored and tracked as `sdk.Coins`
128129
* (store) [\#8790](https://github.com/cosmos/cosmos-sdk/pull/8790) Reduce gas costs by 10x for transient store operations.
129-
* (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.
130130
* (x/bank) [\#9051](https://github.com/cosmos/cosmos-sdk/pull/9051) Supply value is stored as `sdk.Int` rather than `string`.
131131

132132
### Improvements

docs/core/proto-docs.md

-1
Original file line numberDiff line numberDiff line change
@@ -6394,7 +6394,6 @@ Params defines the parameters for the staking module.
63946394
| `max_entries` | [uint32](#uint32) | | max_entries is the max entries for either unbonding delegation or redelegation (per pair/trio). |
63956395
| `historical_entries` | [uint32](#uint32) | | historical_entries is the number of historical entries to persist. |
63966396
| `bond_denom` | [string](#string) | | bond_denom defines the bondable coin denomination. |
6397-
| `power_reduction` | [string](#string) | | power_reduction is the amount of staking tokens required for 1 unit of consensus-engine power |
63986397

63996398

64006399

proto/cosmos/staking/v1beta1/staking.proto

-6
Original file line numberDiff line numberDiff line change
@@ -282,12 +282,6 @@ message Params {
282282
uint32 historical_entries = 4 [(gogoproto.moretags) = "yaml:\"historical_entries\""];
283283
// bond_denom defines the bondable coin denomination.
284284
string bond_denom = 5 [(gogoproto.moretags) = "yaml:\"bond_denom\""];
285-
// power_reduction is the amount of staking tokens required for 1 unit of consensus-engine power
286-
string power_reduction = 6 [
287-
(gogoproto.moretags) = "yaml:\"power_reduction\"",
288-
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
289-
(gogoproto.nullable) = false
290-
];
291285
}
292286

293287
// DelegationResponse is equivalent to Delegation except that it contains a

x/staking/client/testutil/suite.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -867,13 +867,12 @@ func (s *IntegrationTestSuite) TestGetCmdQueryParams() {
867867
historical_entries: 10000
868868
max_entries: 7
869869
max_validators: 100
870-
power_reduction: "1000000"
871870
unbonding_time: 1814400s`,
872871
},
873872
{
874873
"with json output",
875874
[]string{fmt.Sprintf("--%s=json", tmcli.OutputFlag)},
876-
`{"unbonding_time":"1814400s","max_validators":100,"max_entries":7,"historical_entries":10000,"bond_denom":"stake","power_reduction":"1000000"}`,
875+
`{"unbonding_time":"1814400s","max_validators":100,"max_entries":7,"historical_entries":10000,"bond_denom":"stake"}`,
877876
},
878877
}
879878
for _, tc := range testCases {

x/staking/handler_test.go

-45
Original file line numberDiff line numberDiff line change
@@ -42,51 +42,6 @@ func bootstrapHandlerGenesisTest(t *testing.T, power int64, numAddrs int, accAmo
4242
return app, ctx, addrDels, addrVals
4343
}
4444

45-
func TestPowerReductionChangeValidatorSetUpdates(t *testing.T) {
46-
initPower := int64(1000000)
47-
app, ctx, _, valAddrs := bootstrapHandlerGenesisTest(t, initPower, 10, sdk.TokensFromConsensusPower(initPower, sdk.DefaultPowerReduction))
48-
validatorAddr, validatorAddr3 := valAddrs[0], valAddrs[1]
49-
tstaking := teststaking.NewHelper(t, ctx, app.StakingKeeper)
50-
51-
// create validator
52-
tstaking.CreateValidatorWithValPower(validatorAddr, PKs[0], initPower, true)
53-
54-
// must end-block
55-
updates, err := app.StakingKeeper.ApplyAndReturnValidatorSetUpdates(ctx)
56-
require.NoError(t, err)
57-
require.Equal(t, 1, len(updates))
58-
59-
// create a second validator keep it bonded
60-
tstaking.CreateValidatorWithValPower(validatorAddr3, PKs[2], initPower, true)
61-
62-
// must end-block
63-
updates, err = app.StakingKeeper.ApplyAndReturnValidatorSetUpdates(ctx)
64-
require.NoError(t, err)
65-
require.Equal(t, 1, len(updates))
66-
67-
// modify power reduction to 10 times bigger one
68-
params := app.StakingKeeper.GetParams(ctx)
69-
params.PowerReduction = sdk.DefaultPowerReduction.Mul(sdk.NewInt(10))
70-
app.StakingKeeper.SetParams(ctx, params)
71-
72-
// validator updates for tendermint power
73-
updates, err = app.StakingKeeper.ApplyAndReturnValidatorSetUpdates(ctx)
74-
require.NoError(t, err)
75-
require.Equal(t, 2, len(updates))
76-
require.Equal(t, updates[0].Power, initPower/10)
77-
require.Equal(t, updates[1].Power, initPower/10)
78-
79-
// power reduction back to default
80-
params = app.StakingKeeper.GetParams(ctx)
81-
params.PowerReduction = sdk.DefaultPowerReduction
82-
app.StakingKeeper.SetParams(ctx, params)
83-
84-
// validator updates for tendermint power
85-
updates, err = app.StakingKeeper.ApplyAndReturnValidatorSetUpdates(ctx)
86-
require.NoError(t, err)
87-
require.Equal(t, 2, len(updates))
88-
}
89-
9045
func TestValidatorByPowerIndex(t *testing.T) {
9146
initPower := int64(1000000)
9247
app, ctx, _, valAddrs := bootstrapHandlerGenesisTest(t, initPower, 10, sdk.TokensFromConsensusPower(initPower, sdk.DefaultPowerReduction))

x/staking/keeper/migrations.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,5 @@ func NewMigrator(keeper Keeper) Migrator {
1717

1818
// Migrate1to2 migrates from version 1 to 2.
1919
func (m Migrator) Migrate1to2(ctx sdk.Context) error {
20-
return v043.MigrateStore(ctx, m.keeper.storeKey, m.keeper.paramstore)
20+
return v043.MigrateStore(ctx, m.keeper.storeKey)
2121
}

x/staking/keeper/params.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,12 @@ func (k Keeper) BondDenom(ctx sdk.Context) (res string) {
3939
return
4040
}
4141

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

4950
// Get all parameteras as types.Params
@@ -54,7 +55,6 @@ func (k Keeper) GetParams(ctx sdk.Context) types.Params {
5455
k.MaxEntries(ctx),
5556
k.HistoricalEntries(ctx),
5657
k.BondDenom(ctx),
57-
k.PowerReduction(ctx),
5858
)
5959
}
6060

x/staking/keeper/power_reduction_test.go

-13
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,9 @@
11
package keeper_test
22

33
import (
4-
"math/big"
5-
64
sdk "github.com/cosmos/cosmos-sdk/types"
75
)
86

9-
func (suite *KeeperTestSuite) TestPowerReductionChange() {
10-
// modify power reduction
11-
newPowerReduction := sdk.NewIntFromBigInt(new(big.Int).Exp(big.NewInt(10), big.NewInt(12), nil))
12-
params := suite.app.StakingKeeper.GetParams(suite.ctx)
13-
params.PowerReduction = newPowerReduction
14-
suite.app.StakingKeeper.SetParams(suite.ctx, params)
15-
16-
// check power reduction change
17-
suite.Require().Equal(newPowerReduction, suite.app.StakingKeeper.PowerReduction(suite.ctx))
18-
}
19-
207
func (suite *KeeperTestSuite) TestTokensToConsensusPower() {
218
suite.Require().Equal(int64(0), suite.app.StakingKeeper.TokensToConsensusPower(suite.ctx, sdk.DefaultPowerReduction.Sub(sdk.NewInt(1))))
229
suite.Require().Equal(int64(1), suite.app.StakingKeeper.TokensToConsensusPower(suite.ctx, sdk.DefaultPowerReduction))

x/staking/keeper/val_state_change.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func (k Keeper) BlockValidatorUpdates(ctx sdk.Context) []abci.ValidatorUpdate {
112112
func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx sdk.Context) (updates []abci.ValidatorUpdate, err error) {
113113
params := k.GetParams(ctx)
114114
maxValidators := params.MaxValidators
115-
powerReduction := params.PowerReduction
115+
powerReduction := k.PowerReduction(ctx)
116116
totalPower := sdk.ZeroInt()
117117
amtFromBondedToNotBonded, amtFromNotBondedToBonded := sdk.ZeroInt(), sdk.ZeroInt()
118118

x/staking/legacy/v043/json.go

-138
This file was deleted.

x/staking/legacy/v043/json_test.go

-67
This file was deleted.

0 commit comments

Comments
 (0)