refactor(x/gov): Audit gov changes

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 {