fix: refine sentinel errors

also update comments and simplify logic

x/budget/keeper/budget.go

@@ -17,12 +17,6 @@ func (k Keeper) CollectBudgets(ctx sdk.Context) error {
 
 	var inputs []banktypes.Input
 	var outputs []banktypes.Output
-	sendCoins := func(from, to sdk.AccAddress, coins sdk.Coins) {
-		if !coins.Empty() && coins.IsValid() {
-			inputs = append(inputs, banktypes.NewInput(from, coins))
-			outputs = append(outputs, banktypes.NewOutput(to, coins))
-		}
-	}
 
 	// Get a map GetBudgetsBySourceMap that has a list of budgets and their total rate, which
 	// contain the same BudgetSourceAddress
@@ -56,8 +50,11 @@ func (k Keeper) CollectBudgets(ctx sdk.Context) error {
 			totalCollectionCoins = totalCollectionCoins.Add(collectionCoins...)
 			totalChangeCoins = totalChangeCoins.Add(changeCoins...)
 
-			// TODO: sendcoins after validation
-			sendCoins(budgetSourceAcc, collectionAcc, collectionCoins)
+			if !collectionCoins.Empty() && collectionCoins.IsValid() {
+				inputs = append(inputs, banktypes.NewInput(budgetSourceAcc, collectionCoins))
+				outputs = append(outputs, banktypes.NewOutput(collectionAcc, collectionCoins))
+			}
+
 			k.AddTotalCollectedCoins(ctx, budget.Name, collectionCoins)
 
 			ctx.EventManager().EmitEvents(sdk.Events{
@@ -86,7 +83,8 @@ func (k Keeper) CollectBudgets(ctx sdk.Context) error {
 	return nil
 }
 
-// Get all the Budgets registered in params.Budgets and return only the valid and not expired budgets
+// CollectibleBudgets returns scan through the budgets registered in params.Budgets
+// and returns only the valid and not expired budgets.
 func (k Keeper) CollectibleBudgets(ctx sdk.Context) (budgets []types.Budget) {
 	params := k.GetParams(ctx)
 	if params.EpochBlocks > 0 && ctx.BlockHeight()%int64(params.EpochBlocks) == 0 {

x/budget/types/budget.go

@@ -38,20 +38,23 @@ func (budget Budget) Validate() error {
 	}
 
 	if _, err := sdk.AccAddressFromBech32(budget.CollectionAddress); err != nil {
-		return err
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid collection address %s: %v", budget.CollectionAddress, err)
 	}
 
 	if _, err := sdk.AccAddressFromBech32(budget.BudgetSourceAddress); err != nil {
-		return err
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid budget source address %s: %v", budget.CollectionAddress, err)
 	}
 
 	if budget.EndTime.Before(budget.StartTime) {
 		return ErrInvalidStartEndTime
 	}
 
-	if budget.Rate == sdk.ZeroDec() {
-		return ErrZeroBudgetRate
+	if !budget.Rate.IsPositive() {
+		return sdkerrors.Wrapf(ErrInvalidBudgetRate, "budget rate must not be positive: %s", budget.Rate)
+	} else if budget.Rate.GT(sdk.OneDec()) {
+		return sdkerrors.Wrapf(ErrInvalidBudgetRate, "budget rate must not exceed 1: %s", budget.Rate)
 	}
+
 	return nil
 }
 

x/budget/types/errors.go

@@ -6,11 +6,9 @@ import (
 
 // Sentinel errors for the budget module.
 var (
-	ErrInvalidBudgetEndTime = sdkerrors.Register(ModuleName, 2, "invalid Budget end time")
-	ErrInvalidBudgetName    = sdkerrors.Register(ModuleName, 3, "budget name only allows letters, digits, and dash(-) without spaces and the maximum length is 50")
-	ErrInvalidNameOfAddr    = sdkerrors.Register(ModuleName, 4, "address must be generated in accordance with Rule adr-028 using the name and matched")
-	ErrInvalidStartEndTime  = sdkerrors.Register(ModuleName, 5, "budget endtime should not be earlier than budget starttime")
-	ErrZeroBudgetRate       = sdkerrors.Register(ModuleName, 6, "budget rate can't be zero")
-	ErrOverflowedBudgetRate = sdkerrors.Register(ModuleName, 7, "the total rate of budgets with the same budget source address value should not exceed 1")
-	ErrDuplicatedBudgetName = sdkerrors.Register(ModuleName, 8, "budget name cannot be duplicated")
+	ErrInvalidBudgetName      = sdkerrors.Register(ModuleName, 2, "budget name only allows letters, digits, and dash(-) without spaces and the maximum length is 50")
+	ErrInvalidStartEndTime    = sdkerrors.Register(ModuleName, 3, "budget end time should not be earlier than budget start time")
+	ErrInvalidBudgetRate      = sdkerrors.Register(ModuleName, 4, "invalid budget rate")
+	ErrInvalidTotalBudgetRate = sdkerrors.Register(ModuleName, 5, "invalid total rate of the budgets with the same budget source address")
+	ErrDuplicateBudgetName    = sdkerrors.Register(ModuleName, 6, "duplicate budget name")
 )

x/budget/types/genesis.go

@@ -1,5 +1,9 @@
 package types
 
+import (
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+)
+
 // NewGenesisState returns new GenesisState instance.
 func NewGenesisState(params Params, records []BudgetRecord) *GenesisState {
 	return &GenesisState{
@@ -23,7 +27,9 @@ func ValidateGenesis(data GenesisState) error {
 	}
 	for _, record := range data.BudgetRecords {
 		if err := record.TotalCollectedCoins.Validate(); err != nil {
-			return err
+			return sdkerrors.Wrapf(
+				sdkerrors.ErrInvalidCoins,
+				"invalid total collected coins %s: %v", record.TotalCollectedCoins, err)
 		}
 		if err := ValidateName(record.Name); err != nil {
 			return err

x/budget/types/genesis_test.go

@@ -58,10 +58,10 @@ func TestValidateGenesis(t *testing.T) {
 					},
 				}
 			},
-			"decoding bech32 failed: failed converting data to bytes: invalid character not part of charset: 105",
+			"invalid budget source address cosmos1xq6geq9rc9zgkqmn9kxz2rykwe52thp0w9vz2w: decoding bech32 failed: failed converting data to bytes: invalid character not part of charset: 105: invalid address",
 		},
 		{
-			"duplicated budget case",
+			"duplicate budget name",
 			func(genState *types.GenesisState) {
 				genState.Params = types.DefaultParams()
 				genState.Params.Budgets = []types.Budget{
@@ -83,7 +83,7 @@ func TestValidateGenesis(t *testing.T) {
 					},
 				}
 			},
-			"budget1: budget name cannot be duplicated",
+			"budget1: duplicate budget name",
 		},
 		{
 			"invalid budget name case",
@@ -109,7 +109,7 @@ func TestValidateGenesis(t *testing.T) {
 					},
 				}
 			},
-			"coin 0stake amount is not positive",
+			"invalid total collected coins 0stake: coin 0stake amount is not positive: invalid coins",
 		},
 	}
 	for _, tc := range testCases {

x/budget/types/params.go

@@ -81,15 +81,17 @@ func ValidateBudgets(i interface{}) error {
 			return err
 		}
 		if _, ok := names[budget.Name]; ok {
-			return sdkerrors.Wrap(ErrDuplicatedBudgetName, budget.Name)
+			return sdkerrors.Wrap(ErrDuplicateBudgetName, budget.Name)
 		}
 		names[budget.Name] = true
 	}
 
 	budgetsBySourceMap := GetBudgetsBySourceMap(budgets)
 	for addr, budgets := range budgetsBySourceMap {
-		if budgets.TotalRate.GT(sdk.NewDec(1)) {
-			return sdkerrors.Wrap(ErrOverflowedBudgetRate, addr)
+		if budgets.TotalRate.GT(sdk.OneDec()) {
+			return sdkerrors.Wrapf(
+				ErrInvalidTotalBudgetRate,
+				"total rate for budget source address %s must not exceed 1: %v", addr, budgets.TotalRate)
 		}
 	}
 	return nil

x/budget/types/params_test.go

@@ -67,10 +67,10 @@ func TestValidateBudgets(t *testing.T) {
 	require.NoError(t, err)
 
 	err = types.ValidateBudgets(budgets[:3])
-	require.Error(t, err, types.ErrOverflowedBudgetRate)
+	require.ErrorIs(t, err, types.ErrInvalidTotalBudgetRate)
 
 	err = types.ValidateBudgets(budgets)
-	require.Error(t, err, types.ErrDuplicatedBudgetName)
+	require.ErrorIs(t, err, types.ErrDuplicateBudgetName)
 }
 
 func TestValidateEpochBlocks(t *testing.T) {