Merge pull request #62 from dongsam/61-refactoring-collectbudgets

feat: refactor collectbudget
tendermint · Oct 15, 2021 · 6bae189 · 6bae189
2 parents 2ffbd46 + 71211ee
commit 6bae189
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 68 deletions.
diff --git a/x/budget/keeper/budget.go b/x/budget/keeper/budget.go
@@ -15,9 +15,6 @@ func (k Keeper) CollectBudgets(ctx sdk.Context) error {
 		return nil
 	}
 
-	var inputs []banktypes.Input
-	var outputs []banktypes.Output
-
 	// Get a map GetBudgetsBySourceMap that has a list of budgets and their total rate, which
 	// contain the same BudgetSourceAddress
 	budgetsBySourceMap := types.GetBudgetsBySourceMap(budgets)
@@ -30,56 +27,43 @@ func (k Keeper) CollectBudgets(ctx sdk.Context) error {
 		if budgetSourceBalances.IsZero() {
 			continue
 		}
-		expectedCollectionCoins, _ := budgetSourceBalances.MulDecTruncate(budgetsBySource.TotalRate).TruncateDecimal()
-		var validatedExpectedCollectionCoins sdk.Coins
-		for _, coin := range expectedCollectionCoins {
-			if coin.IsValid() && coin.IsZero() {
-				validatedExpectedCollectionCoins = append(validatedExpectedCollectionCoins, coin)
-			}
-		}
-		expectedDiffCoins := expectedCollectionCoins.Sub(validatedExpectedCollectionCoins)
 
-		var totalCollectionCoins sdk.Coins
-		var totalChangeCoins sdk.DecCoins
-		for _, budget := range budgetsBySource.Budgets {
+		var inputs []banktypes.Input
+		var outputs []banktypes.Output
+		budgetsBySource.CollectionCoins = make([]sdk.Coins, len(budgetsBySource.Budgets))
+		for i, budget := range budgetsBySource.Budgets {
 			collectionAcc, err := sdk.AccAddressFromBech32(budget.CollectionAddress)
 			if err != nil {
 				continue
 			}
-			collectionCoins, changeCoins := budgetSourceBalances.MulDecTruncate(budget.Rate).TruncateDecimal()
-			totalCollectionCoins = totalCollectionCoins.Add(collectionCoins...)
-			totalChangeCoins = totalChangeCoins.Add(changeCoins...)
 
-			if !collectionCoins.Empty() && collectionCoins.IsValid() {
-				inputs = append(inputs, banktypes.NewInput(budgetSourceAcc, collectionCoins))
-				outputs = append(outputs, banktypes.NewOutput(collectionAcc, collectionCoins))
+			collectionCoins, _ := budgetSourceBalances.MulDecTruncate(budget.Rate).TruncateDecimal()
+			if collectionCoins.Empty() || !collectionCoins.IsValid() {
+				continue
 			}
 
-			k.AddTotalCollectedCoins(ctx, budget.Name, collectionCoins)
+			inputs = append(inputs, banktypes.NewInput(budgetSourceAcc, collectionCoins))
+			outputs = append(outputs, banktypes.NewOutput(collectionAcc, collectionCoins))
+			budgetsBySource.CollectionCoins[i] = collectionCoins
+		}
 
+		if err := k.bankKeeper.InputOutputCoins(ctx, inputs, outputs); err != nil {
+			continue
+		}
+		for i, budget := range budgetsBySource.Budgets {
+			k.AddTotalCollectedCoins(ctx, budget.Name, budgetsBySource.CollectionCoins[i])
 			ctx.EventManager().EmitEvents(sdk.Events{
 				sdk.NewEvent(
 					types.EventTypeBudgetCollected,
 					sdk.NewAttribute(types.AttributeValueName, budget.Name),
 					sdk.NewAttribute(types.AttributeValueCollectionAddress, budget.CollectionAddress),
 					sdk.NewAttribute(types.AttributeValueBudgetSourceAddress, budget.BudgetSourceAddress),
 					sdk.NewAttribute(types.AttributeValueRate, budget.Rate.String()),
-					sdk.NewAttribute(types.AttributeValueAmount, collectionCoins.String()),
+					sdk.NewAttribute(types.AttributeValueAmount, budgetsBySource.CollectionCoins[i].String()),
 				),
 			})
 		}
-		// temporary validation logic
-		if totalCollectionCoins.IsAnyGT(validatedExpectedCollectionCoins) {
-			panic("totalCollectionCoins.IsAnyGT(expectedCollectionCoins)")
-		}
-		if _, neg := sdk.NewDecCoinsFromCoins(expectedDiffCoins...).SafeSub(totalChangeCoins); neg {
-			panic("expectedChangeCoins.Sub(totalChangeCoins).IsAnyNegative()")
-		}
-	}
-	if err := k.bankKeeper.InputOutputCoins(ctx, inputs, outputs); err != nil {
-		return err
 	}
-	// TODO: add metric
 	return nil
 }
 

diff --git a/x/budget/keeper/invariants.go b/x/budget/keeper/invariants.go
diff --git a/x/budget/module.go b/x/budget/module.go
@@ -113,9 +113,7 @@ func (AppModule) Name() string {
 }
 
 // RegisterInvariants registers the budget module invariants.
-func (am AppModule) RegisterInvariants(ir sdk.InvariantRegistry) {
-	keeper.RegisterInvariants(ir, am.keeper)
-}
+func (AppModule) RegisterInvariants(_ sdk.InvariantRegistry) {}
 
 // Route returns the message routing key for the budget module.
 func (am AppModule) Route() sdk.Route {

diff --git a/x/budget/types/budget.go b/x/budget/types/budget.go
@@ -75,14 +75,15 @@ func ValidateName(name string) error {
 
 // BudgetsBySource defines the total rate of budget lists.
 type BudgetsBySource struct {
-	Budgets   []Budget
-	TotalRate sdk.Dec
+	Budgets         []Budget
+	CollectionCoins []sdk.Coins
+	TotalRate       sdk.Dec
 }
 
 type BudgetsBySourceMap map[string]BudgetsBySource
 
 // GetBudgetsBySourceMap returns BudgetsBySourceMap that has a list of budgets and their total rate
-// which contain the same BudgetSourceAddress. It can be used to track of what budgets are avilable with BudgetSourceAddress
+// which contain the same BudgetSourceAddress. It can be used to track of what budgets are available with BudgetSourceAddress
 // and validate their total rate.
 func GetBudgetsBySourceMap(budgets []Budget) BudgetsBySourceMap {
 	budgetsMap := make(BudgetsBySourceMap)