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

Modular gov tally #64

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
5 changes: 1 addition & 4 deletions codec/types/any.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions proto/cosmos/gov/v1beta1/gov.proto
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ message TextProposal {

option (gogoproto.equal) = true;

string title = 1;
string description = 2;
string title = 1;
string description = 2;
string tallystrategy = 3;
Copy link

@antstalepresh antstalepresh Feb 5, 2021

Choose a reason for hiding this comment

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

Do we need to make all text proposals to have dynamic tally strategy? Or add some configuration for tally strategy for a specific type of proposal to be restricted to few listings?

}

// Deposit defines an amount deposited by an account address to an active
Expand Down
10 changes: 9 additions & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import (
genutiltypes "github.com/cosmos/cosmos-sdk/x/genutil/types"
"github.com/cosmos/cosmos-sdk/x/gov"
govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper"
"github.com/cosmos/cosmos-sdk/x/tallystrategies/stakingtally"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
transfer "github.com/cosmos/cosmos-sdk/x/ibc/applications/transfer"
ibctransferkeeper "github.com/cosmos/cosmos-sdk/x/ibc/applications/transfer/keeper"
Expand Down Expand Up @@ -289,11 +290,18 @@ func NewSimApp(
AddRoute(distrtypes.RouterKey, distr.NewCommunityPoolSpendProposalHandler(app.DistrKeeper)).
AddRoute(upgradetypes.RouterKey, upgrade.NewSoftwareUpgradeProposalHandler(app.UpgradeKeeper)).
AddRoute(ibchost.RouterKey, ibcclient.NewClientUpdateProposalHandler(app.IBCKeeper.ClientKeeper))

// routes are added to tallyrouter after creation of govkeeper, so the govkeeper can be passed into tally routes
tallyRouter := govtypes.NewTallyRouter()

app.GovKeeper = govkeeper.NewKeeper(
appCodec, keys[govtypes.StoreKey], app.GetSubspace(govtypes.ModuleName), app.AccountKeeper, app.BankKeeper,
&stakingKeeper, govRouter,
&stakingKeeper, govRouter, tallyRouter,
)

tallyRouter.AddRoute(govtypes.RootTallyRoute, stakingtally.NewStakingTallyHandler(app.GovKeeper, app.StakingKeeper))
antstalepresh marked this conversation as resolved.
Show resolved Hide resolved
tallyRouter.AddRoute(stakingtally.TallyRoute, stakingtally.NewStakingTallyHandler(app.GovKeeper, app.StakingKeeper))

// Create Transfer Keepers
app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec, keys[ibctransfertypes.StoreKey], app.GetSubspace(ibctransfertypes.ModuleName),
Expand Down
1 change: 1 addition & 0 deletions types/simulation/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type Content interface {
GetDescription() string
ProposalRoute() string
ProposalType() string
TallyRoute() string
ValidateBasic() error
String() string
}
Expand Down
4 changes: 4 additions & 0 deletions x/distribution/types/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ func (csp *CommunityPoolSpendProposal) ProposalRoute() string { return RouterKey
// ProposalType returns the type of a community pool spend proposal.
func (csp *CommunityPoolSpendProposal) ProposalType() string { return ProposalTypeCommunityPoolSpend }

// TallyRoute returns the tally route of the strategy that should weight votes
// on a community pool spend proposal.
func (csp *CommunityPoolSpendProposal) TallyRoute() string { return govtypes.RootTallyRoute }

// ValidateBasic runs basic stateless validity checks
func (csp *CommunityPoolSpendProposal) ValidateBasic() error {
err := govtypes.ValidateAbstract(csp)
Expand Down
8 changes: 4 additions & 4 deletions x/gov/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestTickExpiredDepositPeriod(t *testing.T) {
inactiveQueue.Close()

newProposalMsg, err := types.NewMsgSubmitProposal(
types.ContentFromProposalType("test", "test", types.ProposalTypeText),
types.ContentFromProposalType("test", "test", types.ProposalTypeText, types.RootTallyRoute),
sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 5)},
addrs[0],
)
Expand Down Expand Up @@ -83,7 +83,7 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) {
inactiveQueue.Close()

newProposalMsg, err := types.NewMsgSubmitProposal(
types.ContentFromProposalType("test", "test", types.ProposalTypeText),
types.ContentFromProposalType("test", "test", types.ProposalTypeText, types.RootTallyRoute),
sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 5)},
addrs[0],
)
Expand All @@ -106,7 +106,7 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) {
inactiveQueue.Close()

newProposalMsg2, err := types.NewMsgSubmitProposal(
types.ContentFromProposalType("test2", "test2", types.ProposalTypeText),
types.ContentFromProposalType("test2", "test2", types.ProposalTypeText, types.RootTallyRoute),
sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 5)},
addrs[0],
)
Expand Down Expand Up @@ -163,7 +163,7 @@ func TestTickPassedDepositPeriod(t *testing.T) {
activeQueue.Close()

newProposalMsg, err := types.NewMsgSubmitProposal(
types.ContentFromProposalType("test2", "test2", types.ProposalTypeText),
types.ContentFromProposalType("test2", "test2", types.ProposalTypeText, types.RootTallyRoute),
sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 5)},
addrs[0],
)
Expand Down
4 changes: 2 additions & 2 deletions x/gov/client/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (s *IntegrationTestSuite) SetupSuite() {

// create a proposal with deposit
_, err = govtestutil.MsgSubmitProposal(val.ClientCtx, val.Address.String(),
"Text Proposal 1", "Where is the title!?", types.ProposalTypeText,
"Text Proposal 1", "Where is the title!?", types.ProposalTypeText, types.RootTallyRoute,
fmt.Sprintf("--%s=%s", cli.FlagDeposit, sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens).String()))
s.Require().NoError(err)
_, err = s.network.WaitForHeight(1)
Expand All @@ -57,7 +57,7 @@ func (s *IntegrationTestSuite) SetupSuite() {

// create a proposal without deposit
_, err = govtestutil.MsgSubmitProposal(val.ClientCtx, val.Address.String(),
"Text Proposal 2", "Where is the title!?", types.ProposalTypeText)
"Text Proposal 2", "Where is the title!?", types.ProposalTypeText, types.RootTallyRoute)
s.Require().NoError(err)
_, err = s.network.WaitForHeight(1)
s.Require().NoError(err)
Expand Down
1 change: 1 addition & 0 deletions x/gov/client/cli/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func parseSubmitProposalFlags(fs *pflag.FlagSet) (*proposal, error) {
proposal.Description, _ = fs.GetString(FlagDescription)
proposal.Type = govutils.NormalizeProposalType(proposalType)
proposal.Deposit, _ = fs.GetString(FlagDeposit)
proposal.TallyRoute, _ = fs.GetString(FlagTallyRoute)
return proposal, nil
}

Expand Down
8 changes: 6 additions & 2 deletions x/gov/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ const (
flagDepositor = "depositor"
flagStatus = "status"
FlagProposal = "proposal"
FlagTallyRoute = "tallyroute"
)

type proposal struct {
Title string
Description string
Type string
Deposit string
TallyRoute string
}

// ProposalFlags defines the core required fields of a proposal. It is used to
Expand All @@ -43,6 +45,7 @@ var ProposalFlags = []string{
FlagDescription,
FlagProposalType,
FlagDeposit,
FlagTallyRoute,
}

// NewTxCmd returns the transaction commands for this module
Expand Down Expand Up @@ -97,7 +100,7 @@ Where proposal.json contains:

Which is equivalent to:

$ %s tx gov submit-proposal --title="Test Proposal" --description="My awesome proposal" --type="Text" --deposit="10test" --from mykey
$ %s tx gov submit-proposal --title="Test Proposal" --description="My awesome proposal" --type="Text" --deposit="10test" --tallyroute="staking" --from mykey
`,
version.AppName, version.AppName,
),
Expand All @@ -118,7 +121,7 @@ $ %s tx gov submit-proposal --title="Test Proposal" --description="My awesome pr
return err
}

content := types.ContentFromProposalType(proposal.Title, proposal.Description, proposal.Type)
content := types.ContentFromProposalType(proposal.Title, proposal.Description, proposal.Type, proposal.TallyRoute)

msg, err := types.NewMsgSubmitProposal(content, amount, clientCtx.GetFromAddress())
if err != nil {
Expand All @@ -138,6 +141,7 @@ $ %s tx gov submit-proposal --title="Test Proposal" --description="My awesome pr
cmd.Flags().String(FlagProposalType, "", "The proposal Type")
cmd.Flags().String(FlagDeposit, "", "The proposal deposit")
cmd.Flags().String(FlagProposal, "", "Proposal file path (if this path is given, other proposal flags are ignored)")
cmd.Flags().String(FlagTallyRoute, "", "Tally route for requested tally strategy")
flags.AddTxFlagsToCmd(cmd)

return cmd
Expand Down
4 changes: 2 additions & 2 deletions x/gov/client/rest/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (s *IntegrationTestSuite) SetupSuite() {

// create a proposal with deposit
_, err = govtestutil.MsgSubmitProposal(val.ClientCtx, val.Address.String(),
"Text Proposal 1", "Where is the title!?", types.ProposalTypeText,
"Text Proposal 1", "Where is the title!?", types.ProposalTypeText, types.RootTallyRoute,
fmt.Sprintf("--%s=%s", cli.FlagDeposit, sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens).String()))
s.Require().NoError(err)
_, err = s.network.WaitForHeight(1)
Expand All @@ -53,7 +53,7 @@ func (s *IntegrationTestSuite) SetupSuite() {

// create a proposal without deposit
_, err = govtestutil.MsgSubmitProposal(val.ClientCtx, val.Address.String(),
"Text Proposal 2", "Where is the title!?", types.ProposalTypeText)
"Text Proposal 2", "Where is the title!?", types.ProposalTypeText, types.RootTallyRoute)
s.Require().NoError(err)
_, err = s.network.WaitForHeight(1)
s.Require().NoError(err)
Expand Down
1 change: 1 addition & 0 deletions x/gov/client/rest/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type PostProposalReq struct {
ProposalType string `json:"proposal_type" yaml:"proposal_type"` // Type of proposal. Initial set {PlainTextProposal }
Proposer sdk.AccAddress `json:"proposer" yaml:"proposer"` // Address of the proposer
InitialDeposit sdk.Coins `json:"initial_deposit" yaml:"initial_deposit"` // Coins to add to the proposal's deposit
TallyRoute string `json:"tally_route" yaml:"tally_route"` // Route for Tally Strategy to use
}

// DepositReq defines the properties of a deposit request's body.
Expand Down
2 changes: 1 addition & 1 deletion x/gov/client/rest/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func newPostProposalHandlerFn(clientCtx client.Context) http.HandlerFunc {
}

proposalType := gcutils.NormalizeProposalType(req.ProposalType)
content := types.ContentFromProposalType(req.Title, req.Description, proposalType)
content := types.ContentFromProposalType(req.Title, req.Description, proposalType, req.TallyRoute)

msg, err := types.NewMsgSubmitProposal(content, req.InitialDeposit, req.Proposer)
if rest.CheckBadRequestError(w, err) {
Expand Down
3 changes: 2 additions & 1 deletion x/gov/client/testutil/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ var commonArgs = []string{
}

// MsgSubmitProposal creates a tx for submit proposal
func MsgSubmitProposal(clientCtx client.Context, from, title, description, proposalType string, extraArgs ...string) (testutil.BufferWriter, error) {
func MsgSubmitProposal(clientCtx client.Context, from, title, description, proposalType, tallyRoute string, extraArgs ...string) (testutil.BufferWriter, error) {
args := append([]string{
fmt.Sprintf("--%s=%s", govcli.FlagTitle, title),
fmt.Sprintf("--%s=%s", govcli.FlagDescription, description),
fmt.Sprintf("--%s=%s", govcli.FlagProposalType, proposalType),
fmt.Sprintf("--%s=%s", govcli.FlagTallyRoute, tallyRoute),
fmt.Sprintf("--%s=%s", flags.FlagFrom, from),
}, commonArgs...)

Expand Down
2 changes: 1 addition & 1 deletion x/gov/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

var (
valTokens = sdk.TokensFromConsensusPower(42)
TestProposal = types.NewTextProposal("Test", "description")
TestProposal = types.NewTextProposal("Test", "description", "staking")
TestDescription = stakingtypes.NewDescription("T", "E", "S", "T", "Z")
TestCommissionRates = stakingtypes.NewCommissionRates(sdk.ZeroDec(), sdk.ZeroDec(), sdk.ZeroDec())
)
Expand Down
4 changes: 3 additions & 1 deletion x/gov/keeper/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import (
)

var (
TestProposal = types.NewTextProposal("Test", "description")
TestProposal = types.NewTextProposal("Test", "description", "staking")
TestProposalNoRoute = types.NewTextProposal("Test", "description", "")
TestProposalRandRoute = types.NewTextProposal("Test", "description", "Rand")
)

func createValidators(t *testing.T, ctx sdk.Context, app *simapp.SimApp, powers []int64) ([]sdk.AccAddress, []sdk.ValAddress) {
Expand Down
4 changes: 2 additions & 2 deletions x/gov/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (suite *KeeperTestSuite) TestGRPCQueryProposal() {
"valid request",
func() {
req = &types.QueryProposalRequest{ProposalId: 1}
testProposal := types.NewTextProposal("Proposal", "testing proposal")
testProposal := types.NewTextProposal("Proposal", "testing proposal", "staking")
submittedProposal, err := app.GovKeeper.SubmitProposal(ctx, testProposal)
suite.Require().NoError(err)
suite.Require().NotEmpty(submittedProposal)
Expand Down Expand Up @@ -105,7 +105,7 @@ func (suite *KeeperTestSuite) TestGRPCQueryProposals() {
// create 5 test proposals
for i := 0; i < 5; i++ {
num := strconv.Itoa(i + 1)
testProposal := types.NewTextProposal("Proposal"+num, "testing proposal "+num)
testProposal := types.NewTextProposal("Proposal"+num, "testing proposal "+num, "staking")
proposal, err := app.GovKeeper.SubmitProposal(ctx, testProposal)
suite.Require().NotEmpty(proposal)
suite.Require().NoError(err)
Expand Down
25 changes: 17 additions & 8 deletions x/gov/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ type Keeper struct {

// Proposal router
router types.Router

// Tally router
tallyrouter types.TallyRouter
}

// NewKeeper returns a governance keeper. It handles:
Expand All @@ -42,7 +45,7 @@ type Keeper struct {
// CONTRACT: the parameter Subspace must have the param key table already initialized
func NewKeeper(
cdc codec.BinaryMarshaler, key sdk.StoreKey, paramSpace types.ParamSubspace,
authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, sk types.StakingKeeper, rtr types.Router,
authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, sk types.StakingKeeper, rtr types.Router, tallyrtr types.TallyRouter,
) Keeper {

// ensure governance module account is set
Expand All @@ -56,13 +59,14 @@ func NewKeeper(
rtr.Seal()

return Keeper{
storeKey: key,
paramSpace: paramSpace,
authKeeper: authKeeper,
bankKeeper: bankKeeper,
sk: sk,
cdc: cdc,
router: rtr,
storeKey: key,
paramSpace: paramSpace,
authKeeper: authKeeper,
bankKeeper: bankKeeper,
sk: sk,
cdc: cdc,
router: rtr,
tallyrouter: tallyrtr,
}
}

Expand All @@ -76,6 +80,11 @@ func (keeper Keeper) Router() types.Router {
return keeper.router
}

// TallyRouter returns the gov Keeper's Router
func (keeper Keeper) TallyRouter() types.TallyRouter {
return keeper.tallyrouter
}

// GetGovernanceAccount returns the governance ModuleAccount
func (keeper Keeper) GetGovernanceAccount(ctx sdk.Context) authtypes.ModuleAccountI {
return keeper.authKeeper.GetModuleAccount(ctx, types.ModuleName)
Expand Down
4 changes: 4 additions & 0 deletions x/gov/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ func (keeper Keeper) SubmitProposal(ctx sdk.Context, content types.Content) (typ
return types.Proposal{}, sdkerrors.Wrap(types.ErrNoProposalHandlerExists, content.ProposalRoute())
}

if keeper.TallyRouter().GetRoute(content.TallyRoute()) == nil {
return types.Proposal{}, sdkerrors.Wrap(types.ErrInvalidTallyRoute, content.TallyRoute())
}

// Execute the proposal content in a new context branch (with branched store)
// to validate the actual parameter changes before the proposal proceeds
// through the governance process. State is not persisted.
Expand Down
11 changes: 6 additions & 5 deletions x/gov/keeper/proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,15 @@ func TestSubmitProposal(t *testing.T) {
content types.Content
expectedErr error
}{
{&types.TextProposal{Title: "title", Description: "description"}, nil},
{&types.TextProposal{Title: "title", Description: "description", Tallystrategy: "root"}, nil},
// Keeper does not check the validity of title and description, no error
{&types.TextProposal{Title: "", Description: "description"}, nil},
{&types.TextProposal{Title: strings.Repeat("1234567890", 100), Description: "description"}, nil},
{&types.TextProposal{Title: "title", Description: ""}, nil},
{&types.TextProposal{Title: "title", Description: strings.Repeat("1234567890", 1000)}, nil},
{&types.TextProposal{Title: "", Description: "description", Tallystrategy: "root"}, nil},
{&types.TextProposal{Title: strings.Repeat("1234567890", 100), Description: "description", Tallystrategy: "root"}, nil},
{&types.TextProposal{Title: "title", Description: "", Tallystrategy: "root"}, nil},
{&types.TextProposal{Title: "title", Description: strings.Repeat("1234567890", 1000), Tallystrategy: "root"}, nil},
// error only when invalid route
{&invalidProposalRoute{}, types.ErrNoProposalHandlerExists},
{&types.TextProposal{Title: "title", Description: "description", Tallystrategy: ""}, types.ErrInvalidTallyRoute},
}

for i, tc := range testCases {
Expand Down
Loading