Merge PR #4403: Paramchange proposal skips omitempty fields

This commit is contained in:
Joon 2019-05-28 02:47:12 +02:00 committed by Alexander Bezobchuk
parent 8fecc7724b
commit 91e75cb74a
16 changed files with 225 additions and 91 deletions

View File

@ -0,0 +1,2 @@
#4403 Allow for parameter change proposals to supply only desired fields to be updated
in objects instead of the entire object (only applies to values that are objects).

View File

@ -11,26 +11,12 @@ import (
"github.com/tendermint/tendermint/libs/log"
)
// Parameter store key
var (
ParamStoreKeyDepositParams = []byte("depositparams")
ParamStoreKeyVotingParams = []byte("votingparams")
ParamStoreKeyTallyParams = []byte("tallyparams")
// TODO: Find another way to implement this without using accounts, or find a cleaner way to implement it using accounts.
DepositedCoinsAccAddr = sdk.AccAddress(crypto.AddressHash([]byte("govDepositedCoins")))
BurnedDepositCoinsAccAddr = sdk.AccAddress(crypto.AddressHash([]byte("govBurnedDepositCoins")))
)
// Key declaration for parameters
func ParamKeyTable() params.KeyTable {
return params.NewKeyTable(
ParamStoreKeyDepositParams, DepositParams{},
ParamStoreKeyVotingParams, VotingParams{},
ParamStoreKeyTallyParams, TallyParams{},
)
}
// Governance Keeper
type Keeper struct {
// The reference to the Param Keeper to get and set Global Params

View File

@ -5,12 +5,29 @@ import (
"time"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/params"
)
// Parameter store key
var (
ParamStoreKeyDepositParams = []byte("depositparams")
ParamStoreKeyVotingParams = []byte("votingparams")
ParamStoreKeyTallyParams = []byte("tallyparams")
)
// Key declaration for parameters
func ParamKeyTable() params.KeyTable {
return params.NewKeyTable(
ParamStoreKeyDepositParams, DepositParams{},
ParamStoreKeyVotingParams, VotingParams{},
ParamStoreKeyTallyParams, TallyParams{},
)
}
// Param around deposits for governance
type DepositParams struct {
MinDeposit sdk.Coins `json:"min_deposit"` // Minimum deposit for a proposal to enter voting period.
MaxDepositPeriod time.Duration `json:"max_deposit_period"` // Maximum period for Atom holders to deposit on a proposal. Initial value: 2 months
MinDeposit sdk.Coins `json:"min_deposit,omitempty"` // Minimum deposit for a proposal to enter voting period.
MaxDepositPeriod time.Duration `json:"max_deposit_period,omitempty"` // Maximum period for Atom holders to deposit on a proposal. Initial value: 2 months
}
// NewDepositParams creates a new DepositParams object
@ -34,9 +51,9 @@ func (dp DepositParams) Equal(dp2 DepositParams) bool {
// Param around Tallying votes in governance
type TallyParams struct {
Quorum sdk.Dec `json:"quorum"` // Minimum percentage of total stake needed to vote for a result to be considered valid
Threshold sdk.Dec `json:"threshold"` // Minimum proportion of Yes votes for proposal to pass. Initial value: 0.5
Veto sdk.Dec `json:"veto"` // Minimum value of Veto votes to Total votes ratio for proposal to be vetoed. Initial value: 1/3
Quorum sdk.Dec `json:"quorum,omitempty"` // Minimum percentage of total stake needed to vote for a result to be considered valid
Threshold sdk.Dec `json:"threshold,omitempty"` // Minimum proportion of Yes votes for proposal to pass. Initial value: 0.5
Veto sdk.Dec `json:"veto,omitempty"` // Minimum value of Veto votes to Total votes ratio for proposal to be vetoed. Initial value: 1/3
}
// NewTallyParams creates a new TallyParams object
@ -58,7 +75,7 @@ func (tp TallyParams) String() string {
// Param around Voting in governance
type VotingParams struct {
VotingPeriod time.Duration `json:"voting_period"` // Length of the voting period.
VotingPeriod time.Duration `json:"voting_period,omitempty"` // Length of the voting period.
}
// NewVotingParams creates a new VotingParams object

View File

@ -40,9 +40,11 @@ func getMockApp(t *testing.T, numGenAccs int, genState GenesisState, genAccs []a
tKeyStaking := sdk.NewTransientStoreKey(staking.TStoreKey)
keyGov := sdk.NewKVStoreKey(StoreKey)
rtr := NewRouter().AddRoute(RouterKey, ProposalHandler)
pk := mApp.ParamsKeeper
rtr := NewRouter().
AddRoute(RouterKey, ProposalHandler)
ck := bank.NewBaseKeeper(mApp.AccountKeeper, mApp.ParamsKeeper.Subspace(bank.DefaultParamspace), bank.DefaultCodespace)
sk := staking.NewKeeper(mApp.Cdc, keyStaking, tKeyStaking, ck, pk.Subspace(staking.DefaultParamspace), staking.DefaultCodespace)
keeper := NewKeeper(mApp.Cdc, keyGov, pk, pk.Subspace("testgov"), ck, sk, DefaultCodespace, rtr)

View File

@ -37,6 +37,7 @@ var (
ErrEmptyValue = types.ErrEmptyValue
NewParameterChangeProposal = types.NewParameterChangeProposal
NewParamChange = types.NewParamChange
NewParamChangeWithSubkey = types.NewParamChangeWithSubkey
ValidateChanges = types.ValidateChanges
)

View File

@ -26,7 +26,8 @@ func GetCmdSubmitProposal(cdc *codec.Codec) *cobra.Command {
Short: "Submit a parameter change proposal",
Long: strings.TrimSpace(
fmt.Sprintf(`Submit a parameter proposal along with an initial deposit.
The proposal details must be supplied via a JSON file.
The proposal details must be supplied via a JSON file. For values that contains
objects, only non-empty fields will be updated.
IMPORTANT: Currently parameter changes are evaluated but not validated, so it is
very important that any "value" change is valid (ie. correct type and within bounds)
@ -34,7 +35,7 @@ for its respective parameter, eg. "MaxValidators" should be an integer and not a
Proper vetting of a parameter change proposal should prevent this from happening
(no deposits should occur during the governance process), but it should be noted
regardless.
regardless.
Example:
$ %s tx gov submit-proposal param-change <path/to/proposal.json> --from=<key_or_address>

View File

@ -51,7 +51,7 @@ func NewParamChangeJSON(subspace, key, subkey string, value json.RawMessage) Par
// ToParamChange converts a ParamChangeJSON object to ParamChange.
func (pcj ParamChangeJSON) ToParamChange() params.ParamChange {
return params.NewParamChange(pcj.Subspace, pcj.Key, pcj.Subkey, string(pcj.Value))
return params.NewParamChangeWithSubkey(pcj.Subspace, pcj.Key, pcj.Subkey, string(pcj.Value))
}
// ToParamChanges converts a slice of ParamChangeJSON objects to a slice of

View File

@ -6,40 +6,10 @@ import (
"github.com/stretchr/testify/require"
abci "github.com/tendermint/tendermint/abci/types"
dbm "github.com/tendermint/tendermint/libs/db"
"github.com/tendermint/tendermint/libs/log"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/store"
"github.com/cosmos/cosmos-sdk/store/prefix"
sdk "github.com/cosmos/cosmos-sdk/types"
)
func defaultContext(key sdk.StoreKey, tkey sdk.StoreKey) sdk.Context {
db := dbm.NewMemDB()
cms := store.NewCommitMultiStore(db)
cms.MountStoreWithDB(key, sdk.StoreTypeIAVL, db)
cms.MountStoreWithDB(tkey, sdk.StoreTypeTransient, db)
cms.LoadLatestVersion()
ctx := sdk.NewContext(cms, abci.Header{}, false, log.NewNopLogger())
return ctx
}
type invalid struct{}
type s struct {
I int
}
func createTestCodec() *codec.Codec {
cdc := codec.New()
sdk.RegisterCodec(cdc)
cdc.RegisterConcrete(s{}, "test/s", nil)
cdc.RegisterConcrete(invalid{}, "test/invalid", nil)
return cdc
}
func TestKeeper(t *testing.T) {
kvs := []struct {
key string
@ -66,11 +36,8 @@ func TestKeeper(t *testing.T) {
[]byte("extra2"), string(""),
)
cdc := codec.New()
skey := sdk.NewKVStoreKey("test")
tkey := sdk.NewTransientStoreKey("transient_test")
ctx := defaultContext(skey, tkey)
keeper := NewKeeper(cdc, skey, tkey, DefaultCodespace)
cdc, ctx, skey, _, keeper := testComponents()
store := prefix.NewStore(ctx.KVStore(skey), []byte("test/"))
space := keeper.Subspace("test").WithKeyTable(table)
@ -137,11 +104,7 @@ func indirect(ptr interface{}) interface{} {
}
func TestSubspace(t *testing.T) {
cdc := createTestCodec()
key := sdk.NewKVStoreKey("test")
tkey := sdk.NewTransientStoreKey("transient_test")
ctx := defaultContext(key, tkey)
keeper := NewKeeper(cdc, key, tkey, DefaultCodespace)
cdc, ctx, key, _, keeper := testComponents()
kvs := []struct {
key string
@ -216,3 +179,34 @@ func TestSubspace(t *testing.T) {
require.Equal(t, kv.param, indirect(kv.ptr), "stored param not equal, tc #%d", i)
}
}
type paramJSON struct {
Param1 int64 `json:"param1,omitempty"`
Param2 string `json:"param2,omitempty"`
}
func TestJSONUpdate(t *testing.T) {
_, ctx, _, _, keeper := testComponents()
key := []byte("key")
space := keeper.Subspace("test").WithKeyTable(NewKeyTable(key, paramJSON{}))
var param paramJSON
space.Update(ctx, key, []byte(`{"param1": "10241024"}`))
space.Get(ctx, key, &param)
require.Equal(t, paramJSON{10241024, ""}, param)
space.Update(ctx, key, []byte(`{"param2": "helloworld"}`))
space.Get(ctx, key, &param)
require.Equal(t, paramJSON{10241024, "helloworld"}, param)
space.Update(ctx, key, []byte(`{"param1": "20482048"}`))
space.Get(ctx, key, &param)
require.Equal(t, paramJSON{20482048, "helloworld"}, param)
space.Update(ctx, key, []byte(`{"param1": "40964096", "param2": "goodbyeworld"}`))
space.Get(ctx, key, &param)
require.Equal(t, paramJSON{40964096, "goodbyeworld"}, param)
}

View File

@ -32,12 +32,13 @@ func handleParameterChangeProposal(ctx sdk.Context, k Keeper, p ParameterChangeP
k.Logger(ctx).Info(
fmt.Sprintf("setting new parameter; key: %s, value: %s", c.Key, c.Value),
)
err = ss.SetRaw(ctx, []byte(c.Key), []byte(c.Value))
err = ss.Update(ctx, []byte(c.Key), []byte(c.Value))
} else {
k.Logger(ctx).Info(
fmt.Sprintf("setting new parameter; key: %s, subkey: %s, value: %s", c.Key, c.Subspace, c.Value),
)
err = ss.SetRawWithSubkey(ctx, []byte(c.Key), []byte(c.Subkey), []byte(c.Value))
err = ss.UpdateWithSubkey(ctx, []byte(c.Key), []byte(c.Subkey), []byte(c.Value))
}
if err != nil {

View File

@ -27,16 +27,24 @@ var (
_ subspace.ParamSet = (*testParams)(nil)
keyMaxValidators = "MaxValidators"
keySlashingRate = "SlashingRate"
testSubspace = "TestSubspace"
)
type testParamsSlashingRate struct {
DoubleSign uint16 `json:"double_sign,omitempty"`
Downtime uint16 `json:"downtime,omitempty"`
}
type testParams struct {
MaxValidators uint16 `json:"max_validators"` // maximum number of validators (max uint16 = 65535)
MaxValidators uint16 `json:"max_validators"` // maximum number of validators (max uint16 = 65535)
SlashingRate testParamsSlashingRate `json:"slashing_rate"`
}
func (tp *testParams) ParamSetPairs() subspace.ParamSetPairs {
return subspace.ParamSetPairs{
{[]byte(keyMaxValidators), &tp.MaxValidators},
{[]byte(keySlashingRate), &tp.SlashingRate},
}
}
@ -76,7 +84,7 @@ func TestProposalHandlerPassed(t *testing.T) {
params.NewKeyTable().RegisterParamSet(&testParams{}),
)
tp := testProposal(params.NewParamChange(testSubspace, keyMaxValidators, "", "1"))
tp := testProposal(params.NewParamChange(testSubspace, keyMaxValidators, "1"))
hdlr := params.NewParamChangeProposalHandler(input.keeper)
require.NoError(t, hdlr(input.ctx, tp))
@ -91,9 +99,31 @@ func TestProposalHandlerFailed(t *testing.T) {
params.NewKeyTable().RegisterParamSet(&testParams{}),
)
tp := testProposal(params.NewParamChange(testSubspace, keyMaxValidators, "", "invalidType"))
tp := testProposal(params.NewParamChange(testSubspace, keyMaxValidators, "invalidType"))
hdlr := params.NewParamChangeProposalHandler(input.keeper)
require.Error(t, hdlr(input.ctx, tp))
require.False(t, ss.Has(input.ctx, []byte(keyMaxValidators)))
}
func TestProposalHandlerUpdateOmitempty(t *testing.T) {
input := newTestInput(t)
ss := input.keeper.Subspace(testSubspace).WithKeyTable(
params.NewKeyTable().RegisterParamSet(&testParams{}),
)
hdlr := params.NewParamChangeProposalHandler(input.keeper)
var param testParamsSlashingRate
tp := testProposal(params.NewParamChange(testSubspace, keySlashingRate, `{"downtime": 7}`))
require.NoError(t, hdlr(input.ctx, tp))
ss.Get(input.ctx, []byte(keySlashingRate), &param)
require.Equal(t, testParamsSlashingRate{0, 7}, param)
tp = testProposal(params.NewParamChange(testSubspace, keySlashingRate, `{"double_sign": 10}`))
require.NoError(t, hdlr(input.ctx, tp))
ss.Get(input.ctx, []byte(keySlashingRate), &param)
require.Equal(t, testParamsSlashingRate{10, 7}, param)
}

View File

@ -1,6 +1,7 @@
package simulation
import (
"encoding/json"
"fmt"
"math/rand"
"time"
@ -26,8 +27,6 @@ func (spc simParamChange) compKey() string {
// paramChangePool defines a static slice of possible simulated parameter changes
// where each simParamChange corresponds to a ParamChange with a simValue
// function to generate a simulated new value.
//
// TODO: governance parameters (blocked on an upgrade to go-amino)
var paramChangePool = []simParamChange{
// staking parameters
{
@ -80,6 +79,55 @@ var paramChangePool = []simParamChange{
return fmt.Sprintf("\"%s\"", simulation.ModuleParamSimulator["InflationRateChange"](r).(sdk.Dec))
},
},
// gov parameters
{
"gov",
"votingparams",
"",
func(r *rand.Rand) string {
return fmt.Sprintf(`{"voting_period": "%d"}`, simulation.ModuleParamSimulator["VotingParams/VotingPeriod"](r).(time.Duration))
},
},
{
"gov",
"depositparams",
"",
func(r *rand.Rand) string {
return fmt.Sprintf(`{"max_deposit_period": "%d"}`, simulation.ModuleParamSimulator["VotingParams/VotingPeriod"](r).(time.Duration))
},
},
{
"gov",
"tallyparams",
"",
func(r *rand.Rand) string {
changes := []struct {
key string
value sdk.Dec
}{
{"quorum", simulation.ModuleParamSimulator["TallyParams/Quorum"](r).(sdk.Dec)},
{"threshold", simulation.ModuleParamSimulator["TallyParams/Threshold"](r).(sdk.Dec)},
{"veto", simulation.ModuleParamSimulator["TallyParams/Veto"](r).(sdk.Dec)},
}
pc := make(map[string]string)
numChanges := simulation.RandIntBetween(r, 1, len(changes))
for i := 0; i < numChanges; i++ {
c := changes[r.Intn(len(changes))]
_, ok := pc[c.key]
for ok {
c := changes[r.Intn(len(changes))]
_, ok = pc[c.key]
}
pc[c.key] = c.value.String()
}
bz, _ := json.Marshal(pc)
return string(bz)
},
},
// auth parameters
{
"auth",
@ -126,7 +174,7 @@ func SimulateParamChangeProposalContent(r *rand.Rand, _ *baseapp.BaseApp, _ sdk.
}
paramChangesKeys[spc.compKey()] = struct{}{}
paramChanges[i] = params.NewParamChange(spc.subspace, spc.key, spc.subkey, spc.simValue(r))
paramChanges[i] = params.NewParamChangeWithSubkey(spc.subspace, spc.key, spc.subkey, spc.simValue(r))
}
return params.NewParameterChangeProposal(

View File

@ -177,10 +177,10 @@ func (s Subspace) Set(ctx sdk.Context, key []byte, param interface{}) {
}
// SetRaw stores raw parameter bytes. It returns error if the stored parameter
// Update stores raw parameter bytes. It returns error if the stored parameter
// has a different type from the input. It also sets to the transient store to
// record change.
func (s Subspace) SetRaw(ctx sdk.Context, key []byte, param []byte) error {
func (s Subspace) Update(ctx sdk.Context, key []byte, param []byte) error {
attr, ok := s.table.m[string(key)]
if !ok {
panic("Parameter not registered")
@ -188,13 +188,13 @@ func (s Subspace) SetRaw(ctx sdk.Context, key []byte, param []byte) error {
ty := attr.ty
dest := reflect.New(ty).Interface()
s.GetIfExists(ctx, key, dest)
err := s.cdc.UnmarshalJSON(param, dest)
if err != nil {
return err
}
store := s.kvStore(ctx)
store.Set(key, param)
s.Set(ctx, key, dest)
tStore := s.transientStore(ctx)
tStore.Set(key, []byte{})
@ -220,9 +220,9 @@ func (s Subspace) SetWithSubkey(ctx sdk.Context, key []byte, subkey []byte, para
tstore.Set(newkey, []byte{})
}
// SetRawWithSubkey stores raw parameter bytes with a key and subkey. It checks
// UpdateWithSubkey stores raw parameter bytes with a key and subkey. It checks
// the parameter type only over the key.
func (s Subspace) SetRawWithSubkey(ctx sdk.Context, key []byte, subkey []byte, param []byte) error {
func (s Subspace) UpdateWithSubkey(ctx sdk.Context, key []byte, subkey []byte, param []byte) error {
concatkey := concatKeys(key, subkey)
attr, ok := s.table.m[string(concatkey)]
@ -232,13 +232,13 @@ func (s Subspace) SetRawWithSubkey(ctx sdk.Context, key []byte, subkey []byte, p
ty := attr.ty
dest := reflect.New(ty).Interface()
err := s.cdc.UnmarshalJSON(param, &dest)
s.GetWithSubkeyIfExists(ctx, key, subkey, dest)
err := s.cdc.UnmarshalJSON(param, dest)
if err != nil {
return err
}
store := s.kvStore(ctx)
store.Set(concatkey, param)
s.SetWithSubkey(ctx, key, subkey, dest)
tStore := s.transientStore(ctx)
tStore.Set(concatkey, []byte{})

48
x/params/test_common.go Normal file
View File

@ -0,0 +1,48 @@
package params
import (
abci "github.com/tendermint/tendermint/abci/types"
dbm "github.com/tendermint/tendermint/libs/db"
"github.com/tendermint/tendermint/libs/log"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/store"
sdk "github.com/cosmos/cosmos-sdk/types"
)
type invalid struct{}
type s struct {
I int
}
func createTestCodec() *codec.Codec {
cdc := codec.New()
sdk.RegisterCodec(cdc)
cdc.RegisterConcrete(s{}, "test/s", nil)
cdc.RegisterConcrete(invalid{}, "test/invalid", nil)
return cdc
}
func defaultContext(key sdk.StoreKey, tkey sdk.StoreKey) sdk.Context {
db := dbm.NewMemDB()
cms := store.NewCommitMultiStore(db)
cms.MountStoreWithDB(key, sdk.StoreTypeIAVL, db)
cms.MountStoreWithDB(tkey, sdk.StoreTypeTransient, db)
err := cms.LoadLatestVersion()
if err != nil {
panic(err)
}
ctx := sdk.NewContext(cms, abci.Header{}, false, log.NewNopLogger())
return ctx
}
func testComponents() (*codec.Codec, sdk.Context, sdk.StoreKey, sdk.StoreKey, Keeper) {
cdc := createTestCodec()
mkey := sdk.NewKVStoreKey("test")
tkey := sdk.NewTransientStoreKey("transient_test")
ctx := defaultContext(mkey, tkey)
keeper := NewKeeper(cdc, mkey, tkey, DefaultCodespace)
return cdc, ctx, mkey, tkey, keeper
}

View File

@ -85,7 +85,11 @@ type ParamChange struct {
Value string `json:"value"`
}
func NewParamChange(subspace, key, subkey, value string) ParamChange {
func NewParamChange(subspace, key, value string) ParamChange {
return ParamChange{subspace, key, "", value}
}
func NewParamChangeWithSubkey(subspace, key, subkey, value string) ParamChange {
return ParamChange{subspace, key, subkey, value}
}

View File

@ -7,8 +7,8 @@ import (
)
func TestParameterChangeProposal(t *testing.T) {
pc1 := NewParamChange("sub", "foo", "", "baz")
pc2 := NewParamChange("sub", "bar", "cat", "dog")
pc1 := NewParamChange("sub", "foo", "baz")
pc2 := NewParamChangeWithSubkey("sub", "bar", "cat", "dog")
pcp := NewParameterChangeProposal("test title", "test description", []ParamChange{pc1, pc2})
require.Equal(t, "test title", pcp.GetTitle())
@ -17,15 +17,15 @@ func TestParameterChangeProposal(t *testing.T) {
require.Equal(t, ProposalTypeChange, pcp.ProposalType())
require.Nil(t, pcp.ValidateBasic())
pc3 := NewParamChange("", "bar", "cat", "dog")
pc3 := NewParamChangeWithSubkey("", "bar", "cat", "dog")
pcp = NewParameterChangeProposal("test title", "test description", []ParamChange{pc3})
require.Error(t, pcp.ValidateBasic())
pc4 := NewParamChange("sub", "", "cat", "dog")
pc4 := NewParamChangeWithSubkey("sub", "", "cat", "dog")
pcp = NewParameterChangeProposal("test title", "test description", []ParamChange{pc4})
require.Error(t, pcp.ValidateBasic())
pc5 := NewParamChange("sub", "foo", "cat", "")
pc5 := NewParamChangeWithSubkey("sub", "foo", "cat", "")
pcp = NewParameterChangeProposal("test title", "test description", []ParamChange{pc5})
require.Error(t, pcp.ValidateBasic())
}

View File

@ -59,13 +59,13 @@ var (
return time.Duration(RandIntBetween(r, 1, 2*60*60*24*2)) * time.Second
},
"TallyParams/Quorum": func(r *rand.Rand) interface{} {
return sdk.NewDecWithPrec(334, 3)
return sdk.NewDecWithPrec(int64(RandIntBetween(r, 334, 500)), 3)
},
"TallyParams/Threshold": func(r *rand.Rand) interface{} {
return sdk.NewDecWithPrec(5, 1)
return sdk.NewDecWithPrec(int64(RandIntBetween(r, 450, 550)), 3)
},
"TallyParams/Veto": func(r *rand.Rand) interface{} {
return sdk.NewDecWithPrec(334, 3)
return sdk.NewDecWithPrec(int64(RandIntBetween(r, 250, 334)), 3)
},
"UnbondingTime": func(r *rand.Rand) interface{} {
return time.Duration(RandIntBetween(r, 60, 60*60*24*3*2)) * time.Second