Skip to content

Commit

Permalink
refactor(x/protocolpool): audit QA (#21337)
Browse files Browse the repository at this point in the history
  • Loading branch information
lucaslopezf authored Aug 19, 2024
1 parent 0a0250a commit 0add6d5
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 18 deletions.
22 changes: 11 additions & 11 deletions x/protocolpool/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ If you know the protocolpool module account address, you can directly use bank `
:::

```protobuf reference
https://github.com/cosmos/cosmos-sdk/blob/97724493d792517ba2be8969078b5f92ad04d79c/proto/cosmos/protocolpool/v1/tx.proto#L32-L42
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/protocolpool/proto/cosmos/protocolpool/v1/tx.proto#L43-L53
```

* The msg will fail if the amount cannot be transferred from the sender to the protocolpool module account.
Expand All @@ -101,7 +101,7 @@ func (k Keeper) FundCommunityPool(ctx context.Context, amount sdk.Coins, sender
This message distributes funds from the protocolpool module account to the recipient using `DistributeFromCommunityPool` keeper method.

```protobuf reference
https://github.com/cosmos/cosmos-sdk/blob/97724493d792517ba2be8969078b5f92ad04d79c/proto/cosmos/protocolpool/v1/tx.proto#L47-L58
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/protocolpool/proto/cosmos/protocolpool/v1/tx.proto#L58-L69
```

The message will fail under the following conditions:
Expand All @@ -120,7 +120,7 @@ func (k Keeper) DistributeFromCommunityPool(ctx context.Context, amount sdk.Coin
This message is used to submit a budget proposal to allocate funds for a specific recipient. The proposed funds will be distributed periodically over a specified time frame.

```protobuf reference
https://github.com/cosmos/cosmos-sdk/blob/97724493d792517ba2be8969078b5f92ad04d79c/proto/cosmos/protocolpool/v1/tx.proto#L64-L82
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/protocolpool/proto/cosmos/protocolpool/v1/tx.proto#L75-L94
```

The message will fail under the following conditions:
Expand All @@ -136,15 +136,15 @@ If two budgets to the same address are created, the budget would be updated with
:::

```go reference
https://github.com/cosmos/cosmos-sdk/blob/97724493d792517ba2be8969078b5f92ad04d79c/x/protocolpool/keeper/msg_server.go#L39-l61
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/protocolpool/keeper/msg_server.go#L37-L59
```

### MsgClaimBudget

This message is used to claim funds from a previously submitted budget proposal. When a budget proposal is passed and funds are allocated, recipients can use this message to claim their share of the budget.

```protobuf reference
https://github.com/cosmos/cosmos-sdk/blob/97724493d792517ba2be8969078b5f92ad04d79c/proto/cosmos/protocolpool/v1/tx.proto#L88-L92
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/protocolpool/proto/cosmos/protocolpool/v1/tx.proto#L100-L104
```

The message will fail under the following conditions:
Expand All @@ -155,15 +155,15 @@ The message will fail under the following conditions:
- The budget proposal's distribution period has not passed yet.

```go reference
https://github.com/cosmos/cosmos-sdk/blob/97724493d792517ba2be8969078b5f92ad04d79c/x/protocolpool/keeper/msg_server.go#L25-L37
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/protocolpool/keeper/msg_server.go#L28-L35
```

### MsgCreateContinuousFund

This message is used to create a continuous fund for a specific recipient. The proposed percentage of funds will be distributed only on withdraw request for the recipient. This fund distribution continues until expiry time is reached or continuous fund request is canceled.

```protobuf reference
https://github.com/cosmos/cosmos-sdk/blob/44985ec56557e2d5b763c8676fabbed971f157ba/proto/cosmos/protocolpool/v1/tx.proto#L111-L130
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/protocolpool/proto/cosmos/protocolpool/v1/tx.proto#L114-L130
```

The message will fail under the following conditions:
Expand All @@ -177,15 +177,15 @@ If two continuous fund proposals to the same address are created, the previous C
:::

```go reference
https://github.com/cosmos/cosmos-sdk/blob/44985ec56557e2d5b763c8676fabbed971f157ba/x/protocolpool/keeper/msg_server.go#L109-L140
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/protocolpool/keeper/msg_server.go#L103-L166
```

### MsgCancelContinuousFund

This message is used to cancel an existing continuous fund proposal for a specific recipient. Once canceled, the continuous fund will no longer distribute funds at each end block, and the state object will be removed. Users should be cautious when canceling continuous funds, as it may affect the planned distribution for the recipient.

```protobuf reference
https://github.com/cosmos/cosmos-sdk/blob/44985ec56557e2d5b763c8676fabbed971f157ba/proto/cosmos/protocolpool/v1/tx.proto#L136-L144
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/protocolpool/proto/cosmos/protocolpool/v1/tx.proto#L136-L161
```

The message will fail under the following conditions:
Expand All @@ -194,13 +194,13 @@ The message will fail under the following conditions:
- The ContinuousFund for the recipient does not exist.

```go reference
https://github.com/cosmos/cosmos-sdk/blob/44985ec56557e2d5b763c8676fabbed971f157ba/x/protocolpool/keeper/msg_server.go#L142-L174
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/protocolpool/keeper/msg_server.go#L188-L226
```

## Client

It takes the advantage of `AutoCLI`

```go reference
https://github.com/cosmos/cosmos-sdk/blob/9dd34510e27376005e7e7ff3628eab9dbc8ad6dc/x/protocolpool/autocli.go
https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/protocolpool/autocli.go
```
2 changes: 1 addition & 1 deletion x/protocolpool/depinject.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type ModuleOutputs struct {

func ProvideModule(in ModuleInputs) ModuleOutputs {
// default to governance authority if not provided
authority := authtypes.NewModuleAddress("gov")
authority := authtypes.NewModuleAddress(types.GovModuleName)
if in.Config.Authority != "" {
authority = authtypes.NewModuleAddressOrBech32Address(in.Config.Authority)
}
Expand Down
2 changes: 1 addition & 1 deletion x/protocolpool/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func NewQuerier(keeper Keeper) Querier {
}

// CommunityPool queries the community pool coins
func (k Querier) CommunityPool(ctx context.Context, req *types.QueryCommunityPoolRequest) (*types.QueryCommunityPoolResponse, error) {
func (k Querier) CommunityPool(ctx context.Context, _ *types.QueryCommunityPoolRequest) (*types.QueryCommunityPoolResponse, error) {
amount, err := k.Keeper.GetCommunityPool(ctx)
if err != nil {
return nil, err
Expand Down
10 changes: 7 additions & 3 deletions x/protocolpool/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,23 @@ type Keeper struct {
LastBalance collections.Item[math.Int]
}

const (
errModuleAccountNotSet = "%s module account has not been set"
)

func NewKeeper(cdc codec.BinaryCodec, env appmodule.Environment, ak types.AccountKeeper, bk types.BankKeeper, sk types.StakingKeeper, authority string,
) Keeper {
// ensure pool module account is set
if addr := ak.GetModuleAddress(types.ModuleName); addr == nil {
panic(fmt.Sprintf("%s module account has not been set", types.ModuleName))
panic(fmt.Sprintf(errModuleAccountNotSet, types.ModuleName))
}
// ensure stream account is set
if addr := ak.GetModuleAddress(types.StreamAccount); addr == nil {
panic(fmt.Sprintf("%s module account has not been set", types.StreamAccount))
panic(fmt.Sprintf(errModuleAccountNotSet, types.StreamAccount))
}
// ensure protocol pool distribution account is set
if addr := ak.GetModuleAddress(types.ProtocolPoolDistrAccount); addr == nil {
panic(fmt.Sprintf("%s module account has not been set", types.ProtocolPoolDistrAccount))
panic(fmt.Sprintf(errModuleAccountNotSet, types.ProtocolPoolDistrAccount))
}

sb := collections.NewSchemaBuilder(env.KVStoreService)
Expand Down
59 changes: 59 additions & 0 deletions x/protocolpool/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,62 @@ func (s *KeeperTestSuite) TestIterateAndUpdateFundsDistribution() {
})
s.Require().NoError(err)
}

func (suite *KeeperTestSuite) TestGetCommunityPool() {
suite.SetupTest()

expectedBalance := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(1000000)))
suite.authKeeper.EXPECT().GetModuleAccount(suite.ctx, types.ModuleName).Return(poolAcc).Times(1)
suite.bankKeeper.EXPECT().GetAllBalances(suite.ctx, poolAcc.GetAddress()).Return(expectedBalance).Times(1)

balance, err := suite.poolKeeper.GetCommunityPool(suite.ctx)
suite.Require().NoError(err)
suite.Require().Equal(expectedBalance, balance)

// Test error case when module account doesn't exist
suite.authKeeper.EXPECT().GetModuleAccount(suite.ctx, types.ModuleName).Return(nil).Times(1)
_, err = suite.poolKeeper.GetCommunityPool(suite.ctx)
suite.Require().Error(err)
suite.Require().Contains(err.Error(), "module account protocolpool does not exist")
}

func (suite *KeeperTestSuite) TestSetToDistribute() {
suite.SetupTest()

suite.authKeeper.EXPECT().GetModuleAccount(suite.ctx, types.ProtocolPoolDistrAccount).Return(poolDistrAcc).AnyTimes()
distrBal := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(1000000)))
suite.bankKeeper.EXPECT().GetAllBalances(suite.ctx, poolDistrAcc.GetAddress()).Return(distrBal).AnyTimes()

err := suite.poolKeeper.SetToDistribute(suite.ctx)
suite.Require().NoError(err)

// Verify that LastBalance was set correctly
lastBalance, err := suite.poolKeeper.LastBalance.Get(suite.ctx)
suite.Require().NoError(err)
suite.Require().Equal(math.NewInt(1000000), lastBalance)

// Verify that a distribution was set
var distribution math.Int
err = suite.poolKeeper.Distributions.Walk(suite.ctx, nil, func(key time.Time, value math.Int) (bool, error) {
distribution = value
return true, nil
})
suite.Require().NoError(err)
suite.Require().Equal(math.NewInt(1000000), distribution)

// Test case when balance is zero
zeroBal := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.ZeroInt()))
suite.bankKeeper.EXPECT().GetAllBalances(suite.ctx, poolDistrAcc.GetAddress()).Return(zeroBal).AnyTimes()

err = suite.poolKeeper.SetToDistribute(suite.ctx)
suite.Require().NoError(err)

// Verify that no new distribution was set
count := 0
err = suite.poolKeeper.Distributions.Walk(suite.ctx, nil, func(key time.Time, value math.Int) (bool, error) {
count++
return false, nil
})
suite.Require().NoError(err)
suite.Require().Equal(1, count) // Only the previous distribution should exist
}
4 changes: 2 additions & 2 deletions x/protocolpool/simulation/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const (
// WeightedOperations returns all the operations from the module with their respective weights
func WeightedOperations(
appParams simtypes.AppParams,
cdc codec.JSONCodec,
_ codec.JSONCodec,
txConfig client.TxConfig,
ak types.AccountKeeper,
bk types.BankKeeper,
Expand All @@ -44,7 +44,7 @@ func WeightedOperations(

// SimulateMsgFundCommunityPool simulates MsgFundCommunityPool execution where
// a random account sends a random amount of its funds to the community pool.
func SimulateMsgFundCommunityPool(txConfig client.TxConfig, ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper) simtypes.Operation {
func SimulateMsgFundCommunityPool(txConfig client.TxConfig, ak types.AccountKeeper, bk types.BankKeeper, _ keeper.Keeper) simtypes.Operation {
return func(
r *rand.Rand, app simtypes.AppEntrypoint, ctx sdk.Context, accs []simtypes.Account, chainID string,
) (simtypes.OperationMsg, []simtypes.FutureOperation, error) {
Expand Down

0 comments on commit 0add6d5

Please sign in to comment.