Skip to content

Commit

Permalink
feat!: initial deposit requirement for proposals (#296)
Browse files Browse the repository at this point in the history
* feat!: initial deposit for proposals

* fix TestValidateInitialDeposit

* fix migration tests

* integrate new param into simulator

* increase consensus version and migration tests

* fix app state determinism tests

* fix gov cli

* fix grpc query tests

* more fixes

* restore build tags

* fix genesis and proposal handler tests

* Adam's comment

* revert some test changes

* fix TestSimulateMsgSubmitProposal

* more fixes

* err check

* uncomment test

* fixTestProposalHandler

* convert uint32 to dec

* revert NewDepositParams API, use decorator instead

* fix proto comment

* fix more tests

* go mod

* remove copied proto field

* typo

* fix tests

* fix x/gov/client/testutil tests

* add json tags
  • Loading branch information
p0mvn authored Jul 27, 2022
1 parent 0aca1df commit 8417fe8
Show file tree
Hide file tree
Showing 23 changed files with 538 additions and 137 deletions.
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_expedited_deposit` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) | repeated | Minimum expedited deposit for a proposal to enter voting period. |
| `min_initial_deposit_ratio` | [string](#string) | | The ratio representing the proportion of the deposit value that must be paid at proposal submission. |



Expand Down
7 changes: 7 additions & 0 deletions proto/cosmos/gov/v1beta1/gov.proto
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,13 @@ message DepositParams {
(gogoproto.moretags) = "yaml:\"min_expedited_deposit\"",
(gogoproto.jsontag) = "min_expedited_deposit,omitempty"
];

// The ratio representing the proportion of the deposit value that must be paid at proposal submission.
string min_initial_deposit_ratio = 4 [
(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
1 change: 1 addition & 0 deletions x/feegrant/client/testutil/cli_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build norace
// +build norace

package testutil
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","proposal_voting_periods":null,"expedited_voting_period":"86400000000000"},"tally_params":{"quorum":"0.334000000000000000","threshold":"0.500000000000000000","veto_threshold":"0.334000000000000000","expedited_threshold":"0.667000000000000000"},"deposit_params":{"min_deposit":[{"denom":"stake","amount":"10000000"}],"max_deposit_period":"172800000000000","min_expedited_deposit":[{"denom":"stake","amount":"50000000"}]}}`,
`{"voting_params":{"voting_period":"172800000000000","proposal_voting_periods":null,"expedited_voting_period":"86400000000000"},"tally_params":{"quorum":"0.334000000000000000","threshold":"0.500000000000000000","veto_threshold":"0.334000000000000000","expedited_threshold":"0.667000000000000000"},"deposit_params":{"min_deposit":[{"denom":"stake","amount":"10000000"}],"max_deposit_period":"172800000000000","min_expedited_deposit":[{"denom":"stake","amount":"50000000"}],"min_initial_deposit_ratio":"0.000000000000000000"}}`,
},
{
"text output",
Expand All @@ -102,6 +102,7 @@ deposit_params:
min_expedited_deposit:
- amount: "50000000"
denom: stake
min_initial_deposit_ratio: "0.000000000000000000"
tally_params:
expedited_threshold: "0.667000000000000000"
quorum: "0.334000000000000000"
Expand Down Expand Up @@ -159,7 +160,7 @@ func (s *IntegrationTestSuite) TestCmdParam() {
"deposit",
fmt.Sprintf("--%s=json", tmcli.OutputFlag),
},
`{"min_deposit":[{"denom":"stake","amount":"10000000"}],"max_deposit_period":"172800000000000","min_expedited_deposit":[{"denom":"stake","amount":"50000000"}]}`,
`{"min_deposit":[{"denom":"stake","amount":"10000000"}],"max_deposit_period":"172800000000000","min_expedited_deposit":[{"denom":"stake","amount":"50000000"}],"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 @@ -186,3 +186,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
}
104 changes: 104 additions & 0 deletions x/gov/keeper/deposit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ import (
"github.com/cosmos/cosmos-sdk/x/gov/types"
)

const (
baseDepositTestAmount = 100
baseDepositTestPercent = 25
)

func TestDeposits(t *testing.T) {
testcases := map[string]struct {
isExpedited bool
Expand Down Expand Up @@ -130,3 +135,102 @@ func TestDeposits(t *testing.T) {
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)
}
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(), msg.IsExpedited)
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 @@ -98,7 +98,8 @@ func TestMigrate(t *testing.T) {
"deposit_params": {
"max_deposit_period": "0s",
"min_deposit": [],
"min_expedited_deposit": []
"min_expedited_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 @@ -51,7 +51,8 @@ func TestMigrateJSON(t *testing.T) {
"deposit_params": {
"max_deposit_period": "0s",
"min_deposit": [],
"min_expedited_deposit": []
"min_expedited_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
29 changes: 29 additions & 0 deletions x/gov/legacy/v3/store.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package v3

import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/gov/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/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 paramtypes.Subspace) error {
migrateParamsStore(ctx, paramstore)
return nil
}

func migrateParamsStore(ctx sdk.Context, paramstore paramtypes.Subspace) {
var depositParams types.DepositParams
paramstore.Get(ctx, types.ParamStoreKeyDepositParams, &depositParams)
depositParams.MinInitialDepositRatio = minInitialDepositRatio
paramstore.Set(ctx, types.ParamStoreKeyDepositParams, depositParams)
}
38 changes: 38 additions & 0 deletions x/gov/legacy/v3/store_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package v3_test

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/cosmos/cosmos-sdk/simapp"
"github.com/cosmos/cosmos-sdk/testutil"
sdk "github.com/cosmos/cosmos-sdk/types"
v3 "github.com/cosmos/cosmos-sdk/x/gov/legacy/v3"
"github.com/cosmos/cosmos-sdk/x/gov/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
)

func TestGovStoreMigrationToV3ConsensusVersion(t *testing.T) {
encCfg := simapp.MakeTestEncodingConfig()
govKey := sdk.NewKVStoreKey("gov")
transientTestKey := sdk.NewTransientStoreKey("transient_test")
ctx := testutil.DefaultContext(govKey, transientTestKey)
paramstore := paramtypes.NewSubspace(encCfg.Marshaler, encCfg.Amino, govKey, transientTestKey, "gov")

paramstore = paramstore.WithKeyTable(types.ParamKeyTable())

// We assume that all deposit params are set besdides the MinInitialDepositRatio
originalDepositParams := types.DefaultDepositParams()
originalDepositParams.MinInitialDepositRatio = sdk.ZeroDec()
paramstore.Set(ctx, types.ParamStoreKeyDepositParams, originalDepositParams)

// Run migrations.
err := v3.MigrateStore(ctx, paramstore)
require.NoError(t, err)

// Make sure the new param is set.
var depositParams types.DepositParams
paramstore.Get(ctx, types.ParamStoreKeyDepositParams, &depositParams)
require.Equal(t, v3.MinInitialDepositRatio, depositParams.MinInitialDepositRatio)
}
Loading

0 comments on commit 8417fe8

Please sign in to comment.