refactor(x/gov): Audit gov changes (#21454)

(cherry picked from commit 0c10dd0)

x/gov/README.md

@@ -2204,8 +2204,6 @@ Example Output:
 
 The `params` endpoint allows users to query all parameters for the `gov` module.
 
-<!-- TODO: #10197 Querying governance params outputs nil values -->
-
 Using legacy v1beta1:
 
 ```bash
@@ -2225,16 +2223,6 @@ Example Output:
  "voting_params": {
  "voting_period": "172800s"
  },
- "deposit_params": {
- "min_deposit": [
- ],
- "max_deposit_period": "0s"
- },
- "tally_params": {
- "quorum": "0.000000000000000000",
- "threshold": "0.000000000000000000",
- "veto_threshold": "0.000000000000000000"
- }
 }
 ```
 
@@ -2270,6 +2258,8 @@ Example Output:
 }
 ```
 
+Note: `params_type` are deprecated in v1 since all params are stored in Params.
+
 #### deposits
 
 The `deposits` endpoint allows users to query a deposit for a given proposal from a given depositor.

x/gov/client/utils/query_test.go

@@ -2,6 +2,9 @@ package utils_test
 
 import (
  "context"
+ "fmt"
+ "strconv"
+ "strings"
  "testing"
 
  "github.com/cometbft/cometbft/rpc/client/mock"
@@ -13,6 +16,7 @@ import (
  "cosmossdk.io/x/gov"
  "cosmossdk.io/x/gov/client/utils"
  v1 "cosmossdk.io/x/gov/types/v1"
+ "cosmossdk.io/x/gov/types/v1beta1"
 
  "github.com/cosmos/cosmos-sdk/client"
  codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil"
@@ -24,9 +28,12 @@ type TxSearchMock struct {
  txConfig client.TxConfig
  mock.Client
  txs []cmttypes.Tx
+
+ // use for filter tx with query conditions
+ msgsSet [][]sdk.Msg
 }
 
-func (mock TxSearchMock) TxSearch(ctx context.Context, query string, prove bool, page, perPage *int, orderBy string) (*coretypes.ResultTxSearch, error) {
+func (mock TxSearchMock) TxSearch(ctx context.Context, query string, _ bool, page, perPage *int, _ string) (*coretypes.ResultTxSearch, error) {
  if page == nil {
  *page = 0
  }
@@ -40,8 +47,11 @@ func (mock TxSearchMock) TxSearch(ctx context.Context, query string, prove bool,
  // nil result with nil error crashes utils.QueryTxsByEvents
  return &coretypes.ResultTxSearch{}, nil
  }
+ txs, err := mock.filterTxs(query, start, end)
+ if err != nil {
+ return nil, err
+ }
 
- txs := mock.txs[start:end]
  rst := &coretypes.ResultTxSearch{Txs: make([]*coretypes.ResultTx, len(txs)), TotalCount: len(txs)}
  for i := range txs {
  rst.Txs[i] = &coretypes.ResultTx{Tx: txs[i]}
@@ -54,6 +64,74 @@ func (mock TxSearchMock) Block(ctx context.Context, height *int64) (*coretypes.R
  return &coretypes.ResultBlock{Block: &cmttypes.Block{}}, nil
 }
 
+// mock applying the query string in TxSearch
+func (mock TxSearchMock) filterTxs(query string, start, end int) ([]cmttypes.Tx, error) {
+ filterTxs := []cmttypes.Tx{}
+ proposalIdStr, senderAddr := getQueryAttributes(query)
+ if senderAddr != "" {
+ proposalId, err := strconv.ParseUint(proposalIdStr, 10, 64)
+ if err != nil {
+ return nil, err
+ }
+
+ for i, msgs := range mock.msgsSet {
+ for _, msg := range msgs {
+ if voteMsg, ok := msg.(*v1beta1.MsgVote); ok {
+ if voteMsg.Voter == senderAddr && voteMsg.ProposalId == proposalId {
+ filterTxs = append(filterTxs, mock.txs[i])
+ continue
+ }
+ }
+
+ if voteMsg, ok := msg.(*v1.MsgVote); ok {
+ if voteMsg.Voter == senderAddr && voteMsg.ProposalId == proposalId {
+ filterTxs = append(filterTxs, mock.txs[i])
+ continue
+ }
+ }
+
+ if voteWeightedMsg, ok := msg.(*v1beta1.MsgVoteWeighted); ok {
+ if voteWeightedMsg.Voter == senderAddr && voteWeightedMsg.ProposalId == proposalId {
+ filterTxs = append(filterTxs, mock.txs[i])
+ continue
+ }
+ }
+
+ if voteWeightedMsg, ok := msg.(*v1.MsgVoteWeighted); ok {
+ if voteWeightedMsg.Voter == senderAddr && voteWeightedMsg.ProposalId == proposalId {
+ filterTxs = append(filterTxs, mock.txs[i])
+ continue
+ }
+ }
+ }
+ }
+ } else {
+ filterTxs = append(filterTxs, mock.txs...)
+ }
+
+ if len(filterTxs) < end {
+ return filterTxs, nil
+ }
+
+ return filterTxs[start:end], nil
+}
+
+// getQueryAttributes extracts value from query string
+func getQueryAttributes(q string) (proposalId, senderAddr string) {
+ splitStr := strings.Split(q, " OR ")
+ if len(splitStr) >= 2 {
+ keySender := strings.Trim(splitStr[1], ")")
+ senderAddr = strings.Trim(strings.Split(keySender, "=")[1], "'")
+
+ keyProposal := strings.Split(q, " AND ")[0]
+ proposalId = strings.Trim(strings.Split(keyProposal, "=")[1], "'")
+ } else {
+ proposalId = strings.Trim(strings.Split(splitStr[0], "=")[1], "'")
+ }
+
+ return proposalId, senderAddr
+}
+
 func TestGetPaginatedVotes(t *testing.T) {
  cdcOpts := codectestutil.CodecOptions{}
  encCfg := moduletestutil.MakeTestEncodingConfig(cdcOpts, gov.AppModule{})
@@ -178,3 +256,113 @@ func TestGetPaginatedVotes(t *testing.T) {
  })
  }
 }
+
+func TestGetSingleVote(t *testing.T) {
+ cdcOpts := codectestutil.CodecOptions{}
+ encCfg := moduletestutil.MakeTestEncodingConfig(cdcOpts, gov.AppModule{})
+
+ type testCase struct {
+ description string
+ msgs [][]sdk.Msg
+ votes []v1.Vote
+ expErr string
+ }
+ acc1 := make(sdk.AccAddress, 20)
+ acc1[0] = 1
+ acc1Str, err := cdcOpts.GetAddressCodec().BytesToString(acc1)
+ require.NoError(t, err)
+ acc2 := make(sdk.AccAddress, 20)
+ acc2[0] = 2
+ acc2Str, err := cdcOpts.GetAddressCodec().BytesToString(acc2)
+ require.NoError(t, err)
+ acc1Msgs := []sdk.Msg{
+ v1.NewMsgVote(acc1Str, 0, v1.OptionYes, ""),
+ v1.NewMsgVote(acc1Str, 0, v1.OptionYes, ""),
+ v1.NewMsgDeposit(acc1Str, 0, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10)))), // should be ignored
+ }
+ acc2Msgs := []sdk.Msg{
+ v1.NewMsgVote(acc2Str, 0, v1.OptionYes, ""),
+ v1.NewMsgVoteWeighted(acc2Str, 0, v1.NewNonSplitVoteOption(v1.OptionYes), ""),
+ v1beta1.NewMsgVoteWeighted(acc2Str, 0, v1beta1.NewNonSplitVoteOption(v1beta1.OptionYes)),
+ }
+ for _, tc := range []testCase{
+ {
+ description: "no vote found: no msgVote",
+ msgs: [][]sdk.Msg{
+ acc1Msgs[:1],
+ },
+ votes: []v1.Vote{},
+ expErr: "did not vote on proposalID",
+ },
+ {
+ description: "no vote found: wrong proposal ID",
+ msgs: [][]sdk.Msg{
+ acc1Msgs[:1],
+ },
+ votes: []v1.Vote{},
+ expErr: "did not vote on proposalID",
+ },
+ {
+ description: "query 2 voter vote",
+ msgs: [][]sdk.Msg{
+ acc1Msgs,
+ acc2Msgs[:1],
+ },
+ votes: []v1.Vote{
+ v1.NewVote(0, acc1Str, v1.NewNonSplitVoteOption(v1.OptionYes), ""),
+ v1.NewVote(0, acc2Str, v1.NewNonSplitVoteOption(v1.OptionYes), ""),
+ },
+ },
+ {
+ description: "query 2 voter vote with v1beta1",
+ msgs: [][]sdk.Msg{
+ acc1Msgs,
+ acc2Msgs[2:],
+ },
+ votes: []v1.Vote{
+ v1.NewVote(0, acc1Str, v1.NewNonSplitVoteOption(v1.OptionYes), ""),
+ v1.NewVote(0, acc2Str, v1.NewNonSplitVoteOption(v1.OptionYes), ""),
+ },
+ },
+ } {
+ tc := tc
+
+ t.Run(tc.description, func(t *testing.T) {
+ marshaled := make([]cmttypes.Tx, len(tc.msgs))
+ cli := TxSearchMock{txs: marshaled, txConfig: encCfg.TxConfig, msgsSet: tc.msgs}
+ clientCtx := client.Context{}.
+ WithLegacyAmino(encCfg.Amino).
+ WithClient(cli).
+ WithTxConfig(encCfg.TxConfig).
+ WithAddressCodec(cdcOpts.GetAddressCodec()).
+ WithCodec(encCfg.Codec)
+
+ for i := range tc.msgs {
+ txBuilder := clientCtx.TxConfig.NewTxBuilder()
+ err := txBuilder.SetMsgs(tc.msgs[i]...)
+ require.NoError(t, err)
+
+ tx, err := clientCtx.TxConfig.TxEncoder()(txBuilder.GetTx())
+ require.NoError(t, err)
+ marshaled[i] = tx
+ }
+
+ // testing query single vote
+ for i, v := range tc.votes {
+ accAddr, err := clientCtx.AddressCodec.StringToBytes(v.Voter)
+ require.NoError(t, err)
+ voteParams := utils.QueryVoteParams{ProposalID: 0, Voter: accAddr}
+ voteData, err := utils.QueryVoteByTxQuery(clientCtx, voteParams)
+ if tc.expErr != "" {
+ require.Error(t, err)
+ require.True(t, strings.Contains(err.Error(), tc.expErr))
+ continue
+ }
+ require.NoError(t, err)
+ vote := v1.Vote{}
+ require.NoError(t, clientCtx.Codec.UnmarshalJSON(voteData, &vote))
+ require.Equal(t, v, vote, fmt.Sprintf("vote should be equal at entry %v", i))
+ }
+ })
+ }
+}

x/gov/keeper/deposit.go

@@ -157,6 +157,11 @@ func (k Keeper) AddDeposit(ctx context.Context, proposalID uint64, depositorAddr
  activatedVotingPeriod = true
  }
 
+ addr, err := k.authKeeper.AddressCodec().BytesToString(depositorAddr)
+ if err != nil {
+ return false, err
+ }
+
  // Add or update deposit object
  deposit, err := k.Deposits.Get(ctx, collections.Join(proposalID, depositorAddr))
  switch {
@@ -165,10 +170,6 @@ func (k Keeper) AddDeposit(ctx context.Context, proposalID uint64, depositorAddr
  deposit.Amount = sdk.NewCoins(deposit.Amount...).Add(depositAmount...)
  case errors.IsOf(err, collections.ErrNotFound):
  // deposit doesn't exist
- addr, err := k.authKeeper.AddressCodec().BytesToString(depositorAddr)
- if err != nil {
- return false, err
- }
  deposit = v1.NewDeposit(proposalID, addr, depositAmount)
  default:
  // failed to get deposit
@@ -181,14 +182,9 @@ func (k Keeper) AddDeposit(ctx context.Context, proposalID uint64, depositorAddr
  return false, err
  }
 
- depositorStrAddr, err := k.authKeeper.AddressCodec().BytesToString(depositorAddr)
- if err != nil {
- return false, err
- }
-
  if err := k.EventService.EventManager(ctx).EmitKV(
  types.EventTypeProposalDeposit,
- event.NewAttribute(types.AttributeKeyDepositor, depositorStrAddr),
+ event.NewAttribute(types.AttributeKeyDepositor, addr),
  event.NewAttribute(sdk.AttributeKeyAmount, depositAmount.String()),
  event.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposalID)),
  ); err != nil {

x/gov/keeper/proposal.go

@@ -27,11 +27,15 @@ func (k Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, metadata
  return v1.Proposal{}, err
  }
 
+ proposerAddr, err := k.authKeeper.AddressCodec().BytesToString(proposer)
+ if err != nil {
+ return v1.Proposal{}, err
+ }
+
  // additional checks per proposal types
  switch proposalType {
  case v1.ProposalType_PROPOSAL_TYPE_OPTIMISTIC:
- proposerStr, _ := k.authKeeper.AddressCodec().BytesToString(proposer)
- if len(params.OptimisticAuthorizedAddresses) > 0 && !slices.Contains(params.OptimisticAuthorizedAddresses, proposerStr) {
+ if len(params.OptimisticAuthorizedAddresses) > 0 && !slices.Contains(params.OptimisticAuthorizedAddresses, proposerAddr) {
  return v1.Proposal{}, errorsmod.Wrap(types.ErrInvalidProposer, "proposer is not authorized to submit optimistic proposal")
  }
  case v1.ProposalType_PROPOSAL_TYPE_MULTIPLE_CHOICE:
@@ -43,20 +47,35 @@ func (k Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, metadata
  msgs := make([]string, 0, len(messages)) // will hold a string slice of all Msg type URLs.
 
  // Loop through all messages and confirm that each has a handler and the gov module account as the only signer
+ var currentMessagedBasedParams *v1.MessageBasedParams
  for _, msg := range messages {
  msgs = append(msgs, sdk.MsgTypeURL(msg))
 
  // check if any of the message has message based params
- hasMessagedBasedParams, err := k.MessageBasedParams.Has(ctx, sdk.MsgTypeURL(msg))
+ hasMessagedBasedParams := true
+ messagedBasedParams, err := k.MessageBasedParams.Get(ctx, sdk.MsgTypeURL(msg))
  if err != nil {
- return v1.Proposal{}, err
+ if !errorsmod.IsOf(err, collections.ErrNotFound) {
+ return v1.Proposal{}, err
+ }
+
+ hasMessagedBasedParams = false
  }
 
  if hasMessagedBasedParams {
- // TODO(@julienrbrt), in the future, we can check if all messages have the same params
- // and if so, we can allow the proposal.
- if len(messages) > 1 {
- return v1.Proposal{}, errorsmod.Wrap(types.ErrInvalidProposalMsg, "cannot submit multiple messages proposal with message based params")
+ // set initial value for currentMessagedBasedParams
+ if currentMessagedBasedParams == nil {
+ currentMessagedBasedParams = &messagedBasedParams
+ }
+
+ // check if newly fetched messagedBasedParams is different from the previous fetched params
+ isEqual, err := currentMessagedBasedParams.Equal(&messagedBasedParams)
+ if err != nil {
+ return v1.Proposal{}, err
+ }
+
+ if len(messages) > 1 && !isEqual {
+ return v1.Proposal{}, errorsmod.Wrap(types.ErrInvalidProposalMsg, "cannot submit multiple messages proposal with different message based params")
  }
 
  if proposalType != v1.ProposalType_PROPOSAL_TYPE_STANDARD {
@@ -131,10 +150,6 @@ func (k Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, metadata
  return v1.Proposal{}, err
  }
 
- proposerAddr, err := k.authKeeper.AddressCodec().BytesToString(proposer)
- if err != nil {
- return v1.Proposal{}, err
- }
  submitTime := k.HeaderService.HeaderInfo(ctx).Time
  proposal, err := v1.NewProposal(messages, proposalID, submitTime, submitTime.Add(*params.MaxDepositPeriod), metadata, title, summary, proposerAddr, proposalType)
  if err != nil {