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

Update DefaultHistoricalEntries to 100 #6059

Merged
merged 6 commits into from
Apr 23, 2020
Merged
Show file tree
Hide file tree
Changes from 3 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ functionality that requires an online connection.
* (x/evidence) [\#5952](https://github.com/cosmos/cosmos-sdk/pull/5952) Tendermint Consensus parameters can now be changed via parameter change proposals through `x/gov`.
* (x/auth/ante) [\#6040](https://github.com/cosmos/cosmos-sdk/pull/6040) `AccountKeeper` interface used for `NewAnteHandler` and handler's decorators to add support of using custom `AccountKeeper` implementations.
* (simulation) [\#6002](https://github.com/cosmos/cosmos-sdk/pull/6002) Add randomized consensus params into simulation.
* (x/staking) [\#6059](https://github.com/cosmos/cosmos-sdk/pull/6059) Updated `HistoricalEntries` parameter to 1000.

## [v0.38.3] - 2020-04-09

Expand Down
1 change: 1 addition & 0 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ func NewSimApp(
// During begin block slashing happens after distr.BeginBlocker so that
// there is nothing left over in the validator fee pool, so as to keep the
// CanWithdrawInvariant invariant.
// NOTE: staking module is required if HistoricalEntries param > 0
app.mm.SetOrderBeginBlockers(
upgrade.ModuleName, mint.ModuleName, distr.ModuleName, slashing.ModuleName,
evidence.ModuleName, staking.ModuleName, ibc.ModuleName,
Expand Down
26 changes: 19 additions & 7 deletions x/staking/simulation/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ import (

// Simulation parameter constants
const (
UnbondingTime = "unbonding_time"
MaxValidators = "max_validators"
unbondingTime = "unbonding_time"
maxValidators = "max_validators"
historicalEntries = "historical_entries"
)

// GenUnbondingTime randomized UnbondingTime
Expand All @@ -31,26 +32,37 @@ func GenMaxValidators(r *rand.Rand) (maxValidators uint32) {
return uint32(r.Intn(250) + 1)
}

// GetHistEntries randomized HistoricalEntries
func GetHistEntries(r *rand.Rand) uint32 {
return uint32(r.Intn(int(types.DefaultHistoricalEntries)) + 1)
}

// RandomizedGenState generates a random GenesisState for staking
func RandomizedGenState(simState *module.SimulationState) {
// params
var unbondTime time.Duration
simState.AppParams.GetOrGenerate(
simState.Cdc, UnbondingTime, &unbondTime, simState.Rand,
simState.Cdc, unbondingTime, &unbondTime, simState.Rand,
func(r *rand.Rand) { unbondTime = GenUnbondingTime(r) },
)

var maxValidators uint32
var maxVals uint32
simState.AppParams.GetOrGenerate(
simState.Cdc, maxValidators, &maxVals, simState.Rand,
func(r *rand.Rand) { maxVals = GenMaxValidators(r) },
)

var histEntries uint32
simState.AppParams.GetOrGenerate(
simState.Cdc, MaxValidators, &maxValidators, simState.Rand,
func(r *rand.Rand) { maxValidators = GenMaxValidators(r) },
simState.Cdc, historicalEntries, &histEntries, simState.Rand,
func(r *rand.Rand) { histEntries = GetHistEntries(r) },
)

// NOTE: the slashing module need to be defined after the staking module on the
// NewSimulationManager constructor for this to work
simState.UnbondTime = unbondTime

params := types.NewParams(simState.UnbondTime, maxValidators, 7, 3, sdk.DefaultBondDenom)
params := types.NewParams(simState.UnbondTime, maxVals, 7, histEntries, sdk.DefaultBondDenom)

// validators & delegations
var (
Expand Down
14 changes: 7 additions & 7 deletions x/staking/simulation/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,24 @@ import (
"github.com/cosmos/cosmos-sdk/x/staking/types"
)

const (
keyMaxValidators = "MaxValidators"
keyUnbondingTime = "UnbondingTime"
)

// ParamChanges defines the parameters that can be modified by param change proposals
// on the simulation
func ParamChanges(r *rand.Rand) []simtypes.ParamChange {
return []simtypes.ParamChange{
simulation.NewSimParamChange(types.ModuleName, keyMaxValidators,
simulation.NewSimParamChange(types.ModuleName, string(types.KeyMaxValidators),
func(r *rand.Rand) string {
return fmt.Sprintf("%d", GenMaxValidators(r))
},
),
simulation.NewSimParamChange(types.ModuleName, keyUnbondingTime,
simulation.NewSimParamChange(types.ModuleName, string(types.KeyUnbondingTime),
func(r *rand.Rand) string {
return fmt.Sprintf("\"%d\"", GenUnbondingTime(r))
},
),
simulation.NewSimParamChange(types.ModuleName, string(types.KeyHistoricalEntries),
func(r *rand.Rand) string {
return fmt.Sprintf("\"%d\"", GetHistEntries(r))
},
),
}
}
7 changes: 4 additions & 3 deletions x/staking/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ const (
// Default maximum entries in a UBD/RED pair
DefaultMaxEntries uint32 = 7

// DefaultHistorical entries is 0 since it must only be non-zero for
// IBC connected chains
DefaultHistoricalEntries uint32 = 0
// DefaultHistorical entries is 1000. Apps that don't use IBC can ignore this
// value by not adding the staking module to the application module manager's
// SetOrderBeginBlockers.
DefaultHistoricalEntries uint32 = 1000
Copy link
Contributor

Choose a reason for hiding this comment

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

Any rationale behind this value?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

This will govern how low-latency IBC handshakes must be in order to complete in time. I think 1000 is fine, it's probably even fine to use something much lower, like 100.

Copy link
Contributor

@alexanderbez alexanderbez Apr 23, 2020

Choose a reason for hiding this comment

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

I see. Are historical entries persisted in state? If so, we should opt for a lower bound instead (nitpick).

Copy link
Collaborator

@fedekunze fedekunze Apr 23, 2020

Choose a reason for hiding this comment

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

after block 100 the new ones start overriding the old ones so there are always at most 100

)

// nolint - Keys for parameter access
Expand Down