Review

README.md

@@ -3,7 +3,7 @@
 
 # Budget Module
 
-A budget module is a Cosmos SDK module that implements budget functionality. It is an independent module from other SDK modules and core functionality is to enable anyone to create a budget plan through governance param change proposal. Once it is agreed within the community, voted, and passed, it uses the budget source address to distribute amount of coins by the rate defined in the plan to the collection address. Collecting all budgets and distribution take place every epoch blocks that can be modified by a governance proposal.
+The budget module is a Cosmos SDK module that implements budget functionality. It is an independent module (not sure about this because it uses the bank and auth modules for instance) from other SDK modules and core functionality is to enable anyone to create a budget plan through governance param change proposal. Once it is agreed within the community, voted, and passed, it uses the budget source address to distribute amount of coins by the rate defined in the plan to the collection address. Collecting all budgets and distribution take place every epoch blocks that can be modified by a governance proposal.
 
 One use case is for Gravity DEX farming plan. Budget module can be used to create a budget plan that defines Cosmos Hub's FeeCollector module account where transaction gas fees and part of ATOM inflation are collected as budget source address and uses custom module account (created by budget creator) as collection address. Read [spec docs](./x/budget/spec/01_concepts.md) to get to know more about the module.
 

proto/tendermint/budget/v1beta1/budget.proto

@@ -43,6 +43,9 @@ message Budget {
   // collection_address defines the bech32-encoded address of the budget pool to distribute
   string collection_address = 4 [(gogoproto.moretags) = "yaml:\"collection_address\""];
 
+  // - budget_source_address can be changed to source_address, since this is already in the context of Budget
+  // - Consider changing collection_address -> budget_destination_address so it matches the source address
+
   // start_time specifies the start time of the budget
   google.protobuf.Timestamp start_time = 5
       [(gogoproto.stdtime) = true, (gogoproto.nullable) = false, (gogoproto.moretags) = "yaml:\"start_time\""];

x/budget/keeper/budget.go

@@ -74,7 +74,11 @@ func (k Keeper) CollectibleBudgets(ctx sdk.Context) (budgets []types.Budget) {
 	if params.EpochBlocks > 0 && ctx.BlockHeight()%int64(params.EpochBlocks) == 0 {
 		for _, budget := range params.Budgets {
 			err := budget.Validate()
+			// ^ We can avoid having to validate all budgets at every BeginBlock since
+			// we already know that the budgets are valid from the validation of params
 			if err == nil && !budget.Expired(ctx.BlockTime()) {
+				// ^ We can consider automatically deleting expired budgets, instead of
+				// expecting this to be done through governance proposals.
 				budgets = append(budgets, budget)
 			}
 		}

x/budget/spec/01_concepts.md

@@ -4,9 +4,9 @@
 
 ## Budget Module
 
-`x/budget` is a simple Cosmos SDK module that implements budget functionality. It is an independent module from other SDK modules and core functionality is to enable anyone to create a budget plan through parameter change governance proposal. Once it is agreed within the community, voted, and passed, it uses `BudgetSourceAddress` to distribute amount of coins relative to the rate defined in the plan to the `CollectionAddress`. At each `BeginBlock`, collecting all budgets and distribution take place every `EpochBlocks`. `EpochBlocks` is a global parameter that can be modified by a governance proposal.
+`x/budget` is a simple Cosmos SDK module that implements budget functionality. It is an independent module (not sure about this because it uses the bank and auth modules for instance) from other SDK modules and core functionality is to enable anyone to create a budget plan through parameter change governance proposal. Once it is agreed within the community, voted, and passed, it uses `BudgetSourceAddress` to distribute amount of coins relative to the rate defined in the plan to the `CollectionAddress`. At each `BeginBlock`, collecting all budgets and distribution take place every `EpochBlocks`. `EpochBlocks` is a global parameter that can be modified by a governance proposal.
 
-A primary use case is for Gravity DEX farming plan. A budget module can be used to create a budget plan that has `BudgetSourceAddress` for Cosmos Hub's [FeeCollector](https://github.com/cosmos/cosmos-sdk/blob/v0.44.0/x/auth/types/keys.go#L15) module account which collects transaction gas fees and part of ATOM inflation. Then, `BudgetSourceAddress` plans to distribute some amount of coins to `CollectionAddress` for farming plan. 
+A primary use case is for Gravity DEX farming plan. The budget module can be used to create a budget plan that has `BudgetSourceAddress` for Cosmos Hub's [FeeCollector](https://github.com/cosmos/cosmos-sdk/blob/v0.44.0/x/auth/types/keys.go#L15) module account which collects transaction gas fees and part of ATOM inflation. Then, `BudgetSourceAddress` plans to distribute some amount of coins to `CollectionAddress` for farming plan. 
 
 ### Budget Plan for ATOM Inflation Use Case
 
@@ -46,7 +46,7 @@ Cosmos SDK's current reward workflow
 
 Implementation with Budget Module
 
-  - A budget module is 100% independent from other Cosmos SDK's existing modules
+  - The budget module is 100% independent from other Cosmos SDK's existing modules
 
   - BeginBlock processing order is the following order
 

x/budget/spec/04_begin_block.md

@@ -12,7 +12,7 @@ At each `BeginBlock`, get all budgets registered in `params.Budgets` and select
 
 2. Create a map by `BudgetSourceAddress` to handle the budgets for the same `BudgetSourceAddress` together based on the same balance when calculating rates for the same `BudgetSourceAddress`.
 
-3. Collect budgets from `BudgetSourceAddress` and send amount of coins to `CollectionAddress` relative to the rate of each budget`.
+3. Collect budgets from `BudgetSourceAddress` and send amount of coins to `CollectionAddress` relative to the rate of each budget.
 
 4. Cumulate `TotalCollectedCoins` and emit events about the successful budget collection for each budget.
 

x/budget/types/expected_keepers.go

@@ -17,6 +17,9 @@ type BankKeeper interface {
 	GetSupply(ctx sdk.Context, denom string) sdk.Coin
 	SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
 	SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
+
+	// Note that only InputOutputCoins and GetAllBalances are used from this interface.
+	// Consider limiting BankKeeper expected functions to only these used functions.
 }
 
 // AccountKeeper defines the expected account keeper
@@ -25,4 +28,7 @@ type AccountKeeper interface {
 	GetModuleAddress(name string) sdk.AccAddress
 	GetModuleAccount(ctx sdk.Context, moduleName string) authtypes.ModuleAccountI
 	SetModuleAccount(sdk.Context, authtypes.ModuleAccountI)
+
+	// Note that GetAccount function is not used anywhere in the project.
+	// Consider limiting AccountKeeper expected functions to only these used functions.
 }