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

fix(x/gov): migrations from 2 to 3 (min proposer deposit) (#298) (#302) #9

Merged
merged 5 commits into from
Aug 3, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/core/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -5049,6 +5049,7 @@ DepositParams defines the params for deposits on governance proposals.
| ----- | ---- | ----- | ----------- |
| `min_deposit` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) | repeated | Minimum deposit for a proposal to enter voting period. |
| `max_deposit_period` | [google.protobuf.Duration](#google.protobuf.Duration) | | Maximum period for Atom holders to deposit on a proposal. Initial value: 2 months. |
| `min_initial_deposit_ratio` | [string](#string) | | The ratio representing the proportion of the deposit value that must be paid at proposal submission. |



Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ require (
github.com/gogo/protobuf v1.3.3
github.com/golang/mock v1.6.0
github.com/golang/protobuf v1.5.2
github.com/google/go-cmp v0.5.6 // indirect
github.com/gorilla/handlers v1.5.1
github.com/gorilla/mux v1.8.0
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
Expand Down
6 changes: 6 additions & 0 deletions proto/cosmos/gov/v1beta1/gov.proto
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ message DepositParams {
(gogoproto.jsontag) = "max_deposit_period,omitempty",
(gogoproto.moretags) = "yaml:\"max_deposit_period\""
];
// The ratio representing the proportion of the deposit value that must be paid at proposal submission.
string min_initial_deposit_ratio = 3 [
(gogoproto.nullable) = false,
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
(gogoproto.jsontag) = "min_initial_deposit_ratio,omitempty"
];
}

// VotingParams defines the params for voting on governance proposals.
Expand Down
5 changes: 3 additions & 2 deletions x/gov/client/testutil/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (s *IntegrationTestSuite) TestCmdParams() {
{
"json output",
[]string{fmt.Sprintf("--%s=json", tmcli.OutputFlag)},
`{"voting_params":{"voting_period":"172800000000000"},"tally_params":{"quorum":"0.334000000000000000","threshold":"0.500000000000000000","veto_threshold":"0.334000000000000000"},"deposit_params":{"min_deposit":[{"denom":"stake","amount":"10000000"}],"max_deposit_period":"172800000000000"}}`,
`{"voting_params":{"voting_period":"172800000000000"},"tally_params":{"quorum":"0.334000000000000000","threshold":"0.500000000000000000","veto_threshold":"0.334000000000000000"},"deposit_params":{"min_deposit":[{"denom":"stake","amount":"10000000"}],"max_deposit_period":"172800000000000","min_initial_deposit_ratio":"0.000000000000000000"}}`,
},
{
"text output",
Expand All @@ -99,6 +99,7 @@ deposit_params:
min_deposit:
- amount: "10000000"
denom: stake
min_initial_deposit_ratio: "0.000000000000000000"
tally_params:
quorum: "0.334000000000000000"
threshold: "0.500000000000000000"
Expand Down Expand Up @@ -153,7 +154,7 @@ func (s *IntegrationTestSuite) TestCmdParam() {
"deposit",
fmt.Sprintf("--%s=json", tmcli.OutputFlag),
},
`{"min_deposit":[{"denom":"stake","amount":"10000000"}],"max_deposit_period":"172800000000000"}`,
`{"min_deposit":[{"denom":"stake","amount":"10000000"}],"max_deposit_period":"172800000000000","min_initial_deposit_ratio":"0.000000000000000000"}`,
},
}

Expand Down
18 changes: 18 additions & 0 deletions x/gov/keeper/deposit.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,21 @@ func (keeper Keeper) RefundDeposits(ctx sdk.Context, proposalID uint64) {
return false
})
}

// validateInitialDeposit validates if initial deposit is greater than or equal to the minimum
// required at the time of proposal submission. This threshold amount is determined by
// the deposit parameters. Returns nil on success, error otherwise.
func (keeper Keeper) validateInitialDeposit(ctx sdk.Context, initialDeposit sdk.Coins) error {
depositParams := keeper.GetDepositParams(ctx)
if depositParams.MinInitialDepositRatio.IsNil() || depositParams.MinInitialDepositRatio.IsZero() {
return nil
}
minDepositCoins := depositParams.MinDeposit
for i := range minDepositCoins {
minDepositCoins[i].Amount = minDepositCoins[i].Amount.ToDec().Mul(depositParams.MinInitialDepositRatio).RoundInt()
}
if !initialDeposit.IsAllGTE(minDepositCoins) {
return sdkerrors.Wrapf(types.ErrMinDepositTooSmall, "was (%s), need (%s)", initialDeposit, minDepositCoins)
}
return nil
}
105 changes: 105 additions & 0 deletions x/gov/keeper/deposit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ import (

"github.com/cosmos/cosmos-sdk/simapp"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/gov/types"
)

const (
baseDepositTestAmount = 100
baseDepositTestPercent = 25
)

func TestDeposits(t *testing.T) {
Expand Down Expand Up @@ -108,3 +114,102 @@ func TestDeposits(t *testing.T) {
deposits = app.GovKeeper.GetDeposits(ctx, proposalID)
require.Len(t, deposits, 0)
}

func TestValidateInitialDeposit(t *testing.T) {
testcases := map[string]struct {
minDeposit sdk.Coins
minInitialDepositPercent int64
initialDeposit sdk.Coins

expectError bool
}{
"min deposit * initial percent == initial deposit: success": {
minDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount))),
minInitialDepositPercent: baseDepositTestPercent,
initialDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount*baseDepositTestPercent/100))),
},
"min deposit * initial percent < initial deposit: success": {
minDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount))),
minInitialDepositPercent: baseDepositTestPercent,
initialDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount*baseDepositTestPercent/100+1))),
},
"min deposit * initial percent > initial deposit: error": {
minDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount))),
minInitialDepositPercent: baseDepositTestPercent,
initialDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount*baseDepositTestPercent/100-1))),

expectError: true,
},
"min deposit * initial percent == initial deposit (non-base values and denom): success": {
minDeposit: sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(56912))),
minInitialDepositPercent: 50,
initialDeposit: sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(56912/2+10))),
},
"min deposit * initial percent == initial deposit but different denoms: error": {
minDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount))),
minInitialDepositPercent: baseDepositTestPercent,
initialDeposit: sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(baseDepositTestAmount*baseDepositTestPercent/100))),

expectError: true,
},
"min deposit * initial percent == initial deposit (multiple coins): success": {
minDeposit: sdk.NewCoins(
sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount)),
sdk.NewCoin("uosmo", sdk.NewInt(baseDepositTestAmount*2))),
minInitialDepositPercent: baseDepositTestPercent,
initialDeposit: sdk.NewCoins(
sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount*baseDepositTestPercent/100)),
sdk.NewCoin("uosmo", sdk.NewInt(baseDepositTestAmount*2*baseDepositTestPercent/100)),
),
},
"min deposit * initial percent > initial deposit (multiple coins): error": {
minDeposit: sdk.NewCoins(
sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount)),
sdk.NewCoin("uosmo", sdk.NewInt(baseDepositTestAmount*2))),
minInitialDepositPercent: baseDepositTestPercent,
initialDeposit: sdk.NewCoins(
sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount*baseDepositTestPercent/100)),
sdk.NewCoin("uosmo", sdk.NewInt(baseDepositTestAmount*2*baseDepositTestPercent/100-1)),
),

expectError: true,
},
"min deposit * initial percent < initial deposit (multiple coins - coin not required by min deposit): success": {
minDeposit: sdk.NewCoins(
sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount))),
minInitialDepositPercent: baseDepositTestPercent,
initialDeposit: sdk.NewCoins(
sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount*baseDepositTestPercent/100)),
sdk.NewCoin("uosmo", sdk.NewInt(baseDepositTestAmount*baseDepositTestPercent/100-1)),
),
},
"0 initial percent: success": {
minDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount))),
minInitialDepositPercent: 0,
initialDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount*baseDepositTestPercent/100))),
},
}

for name, tc := range testcases {
t.Run(name, func(t *testing.T) {
app := simapp.Setup(false)
ctx := app.BaseApp.NewContext(false, tmproto.Header{})

govKeeper := app.GovKeeper

params := types.DefaultDepositParams()
params.MinDeposit = tc.minDeposit
params.MinInitialDepositRatio = sdk.NewDec(tc.minInitialDepositPercent).Quo(sdk.NewDec(100))

govKeeper.SetDepositParams(ctx, params)

err := govKeeper.ValidateInitialDeposit(ctx, tc.initialDeposit)

if tc.expectError {
require.Error(t, err)
return
}
require.NoError(t, err)
})
}
}
7 changes: 7 additions & 0 deletions x/gov/keeper/export_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package keeper

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

func (k Keeper) ValidateInitialDeposit(ctx sdk.Context, initialDeposit sdk.Coins) error {
return k.validateInitialDeposit(ctx, initialDeposit)
}
8 changes: 5 additions & 3 deletions x/gov/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,8 +470,9 @@ func (suite *KeeperTestSuite) TestGRPCQueryParams() {
func() {
req = &types.QueryParamsRequest{ParamsType: types.ParamVoting}
expRes = &types.QueryParamsResponse{
VotingParams: types.DefaultVotingParams(),
TallyParams: types.NewTallyParams(sdk.NewDec(0), sdk.NewDec(0), sdk.NewDec(0)),
VotingParams: types.DefaultVotingParams(),
DepositParams: types.DepositParams{MinInitialDepositRatio: sdk.ZeroDec()},
TallyParams: types.NewTallyParams(sdk.NewDec(0), sdk.NewDec(0), sdk.NewDec(0)),
}
},
true,
Expand All @@ -481,7 +482,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryParams() {
func() {
req = &types.QueryParamsRequest{ParamsType: types.ParamTallying}
expRes = &types.QueryParamsResponse{
TallyParams: types.DefaultTallyParams(),
DepositParams: types.DepositParams{MinInitialDepositRatio: sdk.ZeroDec()},
TallyParams: types.DefaultTallyParams(),
}
},
true,
Expand Down
6 changes: 6 additions & 0 deletions x/gov/keeper/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package keeper
import (
sdk "github.com/cosmos/cosmos-sdk/types"
v043 "github.com/cosmos/cosmos-sdk/x/gov/legacy/v043"
v3 "github.com/cosmos/cosmos-sdk/x/gov/legacy/v3"
)

// Migrator is a struct for handling in-place store migrations.
Expand All @@ -19,3 +20,8 @@ func NewMigrator(keeper Keeper) Migrator {
func (m Migrator) Migrate1to2(ctx sdk.Context) error {
return v043.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc)
}

// Migrate2to3 migrates from version 2 to 3.
func (m Migrator) Migrate2to3(ctx sdk.Context) error {
return v3.MigrateStore(ctx, m.keeper.paramSpace)
}
6 changes: 6 additions & 0 deletions x/gov/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ var _ types.MsgServer = msgServer{}

func (k msgServer) SubmitProposal(goCtx context.Context, msg *types.MsgSubmitProposal) (*types.MsgSubmitProposalResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)
initialDeposit := msg.GetInitialDeposit()

if err := k.validateInitialDeposit(ctx, initialDeposit); err != nil {
return nil, err
}

proposal, err := k.Keeper.SubmitProposal(ctx, msg.GetContent())
if err != nil {
return nil, err
Expand Down
88 changes: 88 additions & 0 deletions x/gov/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package keeper_test

import (
"testing"

"github.com/cosmos/cosmos-sdk/simapp"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/gov/keeper"
"github.com/cosmos/cosmos-sdk/x/gov/types"
"github.com/stretchr/testify/require"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
)

func TestSubmitProposal_InitialDeposit(t *testing.T) {
const meetsDepositValue = baseDepositTestAmount * baseDepositTestPercent / 100
var baseDepositRatioDec = sdk.NewDec(baseDepositTestPercent).Quo(sdk.NewDec(100))

testcases := map[string]struct {
minDeposit sdk.Coins
minInitialDepositRatio sdk.Dec
initialDeposit sdk.Coins
accountBalance sdk.Coins

expectError bool
}{
"meets initial deposit, enough balance - success": {
minDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount))),
minInitialDepositRatio: baseDepositRatioDec,
initialDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(meetsDepositValue))),
accountBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(meetsDepositValue))),
},
"does not meet initial deposit, enough balance - error": {
minDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount))),
minInitialDepositRatio: baseDepositRatioDec,
initialDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(meetsDepositValue-1))),
accountBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(meetsDepositValue))),

expectError: true,
},
"meets initial deposit, not enough balance - error": {
minDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount))),
minInitialDepositRatio: baseDepositRatioDec,
initialDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(meetsDepositValue))),
accountBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(meetsDepositValue-1))),

expectError: true,
},
"does not meet initial deposit and not enough balance - error": {
minDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(baseDepositTestAmount))),
minInitialDepositRatio: baseDepositRatioDec,
initialDeposit: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(meetsDepositValue-1))),
accountBalance: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(meetsDepositValue-1))),

expectError: true,
},
}

for name, tc := range testcases {
t.Run(name, func(t *testing.T) {
// Setup
app := simapp.Setup(false)
ctx := app.BaseApp.NewContext(false, tmproto.Header{})
govKeeper := app.GovKeeper
msgServer := keeper.NewMsgServerImpl(govKeeper)

params := types.DefaultDepositParams()
params.MinDeposit = tc.minDeposit
params.MinInitialDepositRatio = tc.minInitialDepositRatio
govKeeper.SetDepositParams(ctx, params)

address := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(0))[0]
simapp.FundAccount(app.BankKeeper, ctx, address, tc.accountBalance)

msg, err := types.NewMsgSubmitProposal(TestProposal, tc.initialDeposit, address)
require.NoError(t, err)

// System under test
_, err = msgServer.SubmitProposal(sdk.WrapSDKContext(ctx), msg)

// Assertions
if tc.expectError {
require.Error(t, err)
return
}
require.NoError(t, err)
})
}
}
3 changes: 2 additions & 1 deletion x/gov/legacy/v040/migrate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ func TestMigrate(t *testing.T) {
expected := `{
"deposit_params": {
"max_deposit_period": "0s",
"min_deposit": []
"min_deposit": [],
"min_initial_deposit_ratio": "0"
},
"deposits": [],
"proposals": [
Expand Down
3 changes: 2 additions & 1 deletion x/gov/legacy/v043/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ func TestMigrateJSON(t *testing.T) {
expected := `{
"deposit_params": {
"max_deposit_period": "0s",
"min_deposit": []
"min_deposit": [],
"min_initial_deposit_ratio": "0"
},
"deposits": [],
"proposals": [],
Expand Down
3 changes: 3 additions & 0 deletions x/gov/legacy/v3/export_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package v3

var MinInitialDepositRatio = minInitialDepositRatio
28 changes: 28 additions & 0 deletions x/gov/legacy/v3/store.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package v3

import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/gov/types"
)

var minInitialDepositRatio = sdk.NewDec(25).Quo(sdk.NewDec(100))

// MigrateStore performs in-place store migrations for consensus version 3
// in the gov module.
// Please note that this is the first version that switches from using
// SDK versioning (v043 etc) for package names to consensus versioning
// of the gov module.
// The migration includes:
//
// - Setting the minimum deposit param in the paramstore.
func MigrateStore(ctx sdk.Context, paramstore types.ParamSubspace) error {
migrateParamsStore(ctx, paramstore)
return nil
}

func migrateParamsStore(ctx sdk.Context, paramstore types.ParamSubspace) {
var depositParams types.DepositParams
paramstore.Get(ctx, types.ParamStoreKeyDepositParams, &depositParams)
depositParams.MinInitialDepositRatio = minInitialDepositRatio
paramstore.Set(ctx, types.ParamStoreKeyDepositParams, depositParams)
}