feat(community): add switchover param #1704
Conversation
rhuairahrighairidh
changed the title
| import "time" | ||
|
|
||
| var ( | ||
| DefaultUpgradeTimeDisableInflation = time.Time{} |
There was a problem hiding this comment.
Should we use a non-zero/default value for the default time and enforce it in Validate()?
In case we're doing Time.After() or Time.Before() checks when using the value on switchover, empty value might cause issues since it's considered earlier
There was a problem hiding this comment.
The inflation upgrade logic will currently handle unset times by upgrading if unset to ensure vanilla kava chain always have new inflation logic enabled. https://github.com/Kava-Labs/kava/pull/1706/files#diff-15d449f4515711b278439f4600052557555689d56ea115a58619aa3061738906R23
There was a problem hiding this comment.
Agree with @DracoLi here, new chains should default to new logic to match mainnet behavior after the upgrade time
I updated protonet genesis, going off the assumption that protonet follows master, and internal testnet follows latest release. |
ci/env/kava-protonet/KAVA.VERSION
Outdated
| 47416b3d1af6dfd31bab479d78b24cdbb677be31 | ||
| master |
There was a problem hiding this comment.
@evgeniy-scherbina I saw you pinned this commit recently, is it ok to change back to master?
| @@ -0,0 +1,20 @@ | |||
| package types | |||
There was a problem hiding this comment.
though super simple, i think these public methods should have tests.
|
|
||
| // Validate checks the params are valid | ||
| func (p Params) Validate() error { | ||
| return nil |
There was a problem hiding this comment.
i'd add a comment here noting why p.UpgradeTimeDisableInflation.IsZero() is still a valid state (zero time means disable inflation on the block 1)
There was a problem hiding this comment.
added some doc comments to the proto files where the param is defined
* Add RewardsPerSecond param to community * Update rewards per second param to int * Add rewards_per_second to protonet genesis * Use default rewards per second of 744191 * Include value if negative in Validate error * Rename RewardsPerSecond param to StakingRewardsPerSecond * Add changelog entry * Add param migration, update consensus version to 2 * Update proto docs
Co-authored-by: Robert Pirtle <Astropirtle@gmail.com>
rhuairahrighairidh
deleted the
ro-incentive-restructuring-add-switchover-param
branch
Add a timestamp to the communiity params for when the incentive changes take effect.
Description
This uses the new style params instead of the params module. It makes it slightly more inconvenient to do a gov proposal, but saves re-writing the params when we eventually upgrade the sdk.
Checklist
Migrations writtenLeaving migrations out to unblock @drklee3 on AddRewardsPerSecondparam tox/communitymodule #1707in migration files added directly to master branch, write module migrations generically so both mainnet and testnet can use them. if the change is consensus breaking to a module, the module’s consensus version is incremented and the migration is registered to the module’s services.migrations are fully unit tested to ensure state changes are made correctlyrecord any release branch migrations needed in the feature tech spec document, to be added when the release branch is created