chg: address gov PR comments (#15)

* chg: address PR comments

* chg: rename MaxVoteOptionsLen

tests/e2e/gov/tx.go

@@ -545,8 +545,7 @@ func (s *E2ETestSuite) TestNewCmdWeightedVote() {
 	for _, tc := range testCases {
 		tc := tc
 		s.Run(tc.name, func() {
-			// TODO HV2: changed from NewCmdWeightedVote to NewCmdVote. Fix tests accordingly.
-			cmd := cli.NewCmdVote()
+			cmd := cli.NewCmdWeightedVote()
 			clientCtx := val.ClientCtx
 			var txResp sdk.TxResponse
 

x/feegrant/client/cli/tx_test.go

@@ -694,8 +694,7 @@ func (s *CLITestSuite) msgVote(clientCtx client.Context, from, id, vote string,
 	}, commonArgs...)
 
 	args = append(args, extraArgs...)
-	// TODO HV2: changed from NewCmdWeightedVote to NewCmdVote. Fix tests accordingly.
-	cmd := govcli.NewCmdVote()
+	cmd := govcli.NewCmdWeightedVote()
 
 	out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
 

x/gov/keeper/deposit.go

@@ -36,6 +36,8 @@ func (keeper Keeper) GetDeposits(ctx context.Context, proposalID uint64) (deposi
 
 // DeleteAndBurnDeposits deletes and burns all the deposits on a specific proposal.
 func (keeper Keeper) DeleteAndBurnDeposits(ctx context.Context, proposalID uint64) error {
+	// HV2: no support in heimdall for burn deposits
+	panic(errors.ErrPanic)
 	coinsToBurn := sdk.NewCoins()
 	err := keeper.IterateDeposits(ctx, proposalID, func(key collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) (stop bool, err error) {
 		coinsToBurn = coinsToBurn.Add(deposit.Amount...)

x/gov/keeper/keeper.go

@@ -90,9 +90,9 @@ func NewKeeper(
 		config.MaxMetadataLen = types.DefaultConfig().MaxMetadataLen
 	}
 
-	// Enforce MaxOptionsLen to be 1.
-	if config.MaxOptionsLen != 1 {
-		config.MaxOptionsLen = types.DefaultConfig().MaxOptionsLen
+	// Enforce MaxVoteOptionsLen to be 1.
+	if config.MaxVoteOptionsLen != 1 {
+		config.MaxVoteOptionsLen = types.DefaultConfig().MaxVoteOptionsLen
 	}
 
 	sb := collections.NewSchemaBuilder(storeService)
@@ -189,11 +189,11 @@ func (k Keeper) assertMetadataLength(metadata string) error {
 	return nil
 }
 
-// assertOptionsLength returns an error if given options length
-// is greater than a pre-defined MaxOptionsLen.
-func (k Keeper) assertOptionsLength(options v1.WeightedVoteOptions) error {
-	if uint64(len(options)) > k.config.MaxOptionsLen {
-		return types.ErrTooManyVoteOptions.Wrapf("got %d options, maximum allowed is %d", len(options), k.config.MaxOptionsLen)
+// assertVoteOptionsLength returns an error if given vote options length
+// is greater than a pre-defined MaxVoteOptionsLen.
+func (k Keeper) assertVoteOptionsLength(options v1.WeightedVoteOptions) error {
+	if uint64(len(options)) > k.config.MaxVoteOptionsLen {
+		return types.ErrTooManyVoteOptions.Wrapf("got %d options, maximum allowed is %d", len(options), k.config.MaxVoteOptionsLen)
 	}
 	return nil
 }

x/gov/keeper/tally.go

@@ -114,17 +114,17 @@ func (keeper Keeper) Tally(ctx context.Context, proposal v1.Proposal) (passes, b
 		*/
 
 		// HV2: check on the length of the options.
-		// This should never fail as all votes must have only 1 option
-		err := keeper.assertOptionsLength(val.Vote)
+		// This should never fail as all votes must have only 1 option, hence we panic if something goes wrong
+		err := keeper.assertVoteOptionsLength(val.Vote)
 		if err != nil {
-			return false, false, tallyResults, err
+			panic(err)
 		}
 
 		for _, option := range val.Vote {
 			// HV2: check on the weight of the option.
-			// This should never fail as the only option should have weight=1
+			// This should never fail as the only option should have weight=1, hence we panic if something goes wrong
 			if !v1.ValidWeightedVoteOption(*option) {
-				return false, false, tallyResults, err
+				panic(err)
 			}
 			weight, _ := math.LegacyNewDecFromStr(option.Weight)
 			subPower := votingPower.Mul(weight)

x/gov/keeper/vote.go

@@ -31,9 +31,10 @@ func (keeper Keeper) AddVote(ctx context.Context, proposalID uint64, voterAddr s
 
 	// HV2: check on the length of the options. This should never fail as it is only used by `MsgServer.Vote`.
 	// There, `v1.NewNonSplitVoteOption` is used to enforce the constraints (only 1 option with weight=1)
-	err = keeper.assertOptionsLength(options)
+	// Hence, we panic if something goes wrong
+	err = keeper.assertVoteOptionsLength(options)
 	if err != nil {
-		return err
+		panic(err)
 	}
 
 	// HV2: check on the weight of the option. This should never fail as it is only used by `MsgServer.Vote`.

x/gov/types/config.go

@@ -4,14 +4,14 @@ package types
 type Config struct {
 	// MaxMetadataLen defines the maximum proposal metadata length.
 	MaxMetadataLen uint64
-	// MaxOptionsLen defines the maximum number of options a proposal can have.
-	MaxOptionsLen uint64
+	// MaxVoteOptionsLen defines the maximum number of options a proposal can have.
+	MaxVoteOptionsLen uint64
 }
 
 // DefaultConfig returns the default config for gov.
 func DefaultConfig() Config {
 	return Config{
-		MaxMetadataLen: 255,
-		MaxOptionsLen:  1,
+		MaxMetadataLen:    255,
+		MaxVoteOptionsLen: 1,
 	}
 }

x/upgrade/types/proposal.go

@@ -5,8 +5,11 @@ import (
 )
 
 const (
-	// TODO HV2: ProposalTypeSoftwareUpgrade was removed. What to do with it?
-	//  ProposalTypeCancelSoftwareUpgrade was not even there in heimdall's gov/types/proposal.go. What to do with it?
+	/* TODO HV2: ProposalTypeSoftwareUpgrade was removed. What to do with it?
+	   ProposalTypeCancelSoftwareUpgrade was not even there in heimdall's gov/types/proposal.go.
+	   Also, ProposalTypeCancelSoftwareUpgrade depends on ProposalTypeSoftwareUpgrade semantically,
+	   so whatever we do with one, we should do with the other.
+	*/
 
 	ProposalTypeSoftwareUpgrade       string = "SoftwareUpgrade"
 	ProposalTypeCancelSoftwareUpgrade string = "CancelSoftwareUpgrade"