Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(distribution)!: use collections for params state #16211

Merged
merged 8 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,12 @@ Ref: https://keepachangelog.com/en/1.0.0/
* Remove: errors unused errors

* (sims) [#16155](https://github.com/cosmos/cosmos-sdk/pull/16155)

* `simulation.NewOperationMsg` now marshals the operation msg as proto bytes instead of legacy amino JSON bytes.
* `simulation.NewOperationMsg` is now 2-arity instead of 3-arity with the obsolete argument `codec.ProtoCodec` removed.
* The field `OperationMsg.Msg` is now of type `[]byte` instead of `json.RawMessage`.
* (cli) [#16209](https://github.com/cosmos/cosmos-sdk/pull/16209) Add API `StartCmdWithOptions` to create customized start command.

* (x/distribution) [#16211](https://github.com/cosmos/cosmos-sdk/pull/16211) Use collections for params state management.

### Client Breaking Changes

Expand Down
6 changes: 3 additions & 3 deletions tests/integration/distribution/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func TestGRPCParams(t *testing.T) {
t.Parallel()
f := initFixture(t)

f.distrKeeper.SetParams(f.sdkCtx, types.DefaultParams())
f.distrKeeper.Params.Set(f.sdkCtx, types.DefaultParams())

qr := f.app.QueryHelper()
queryClient := types.NewQueryClient(qr)
Expand Down Expand Up @@ -51,7 +51,7 @@ func TestGRPCParams(t *testing.T) {
WithdrawAddrEnabled: true,
}

assert.NilError(t, f.distrKeeper.SetParams(f.sdkCtx, params))
assert.NilError(t, f.distrKeeper.Params.Set(f.sdkCtx, params))
expParams = params
},
msg: &types.QueryParamsRequest{},
Expand Down Expand Up @@ -369,7 +369,7 @@ func TestGRPCDelegatorWithdrawAddress(t *testing.T) {
t.Parallel()
f := initFixture(t)

f.distrKeeper.SetParams(f.sdkCtx, types.DefaultParams())
f.distrKeeper.Params.Set(f.sdkCtx, types.DefaultParams())

qr := f.app.QueryHelper()
queryClient := types.NewQueryClient(qr)
Expand Down
34 changes: 17 additions & 17 deletions tests/integration/distribution/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func TestMsgWithdrawDelegatorReward(t *testing.T) {
f.distrKeeper.SetFeePool(f.sdkCtx, distrtypes.FeePool{
CommunityPool: sdk.NewDecCoins(sdk.DecCoin{Denom: "stake", Amount: math.LegacyNewDec(10000)}),
})
f.distrKeeper.SetParams(f.sdkCtx, distrtypes.DefaultParams())
f.distrKeeper.Params.Set(f.sdkCtx, distrtypes.DefaultParams())
initFeePool, err := f.distrKeeper.GetFeePool(f.sdkCtx)
assert.NilError(t, err)

Expand Down Expand Up @@ -316,7 +316,7 @@ func TestMsgSetWithdrawAddress(t *testing.T) {
t.Parallel()
f := initFixture(t)

f.distrKeeper.SetParams(f.sdkCtx, distrtypes.DefaultParams())
f.distrKeeper.Params.Set(f.sdkCtx, distrtypes.DefaultParams())

delAddr := sdk.AccAddress(PKS[0].Address())
withdrawAddr := sdk.AccAddress(PKS[1].Address())
Expand All @@ -331,9 +331,9 @@ func TestMsgSetWithdrawAddress(t *testing.T) {
{
name: "empty delegator address",
preRun: func() {
params, _ := f.distrKeeper.GetParams(f.sdkCtx)
params, _ := f.distrKeeper.Params.Get(f.sdkCtx)
params.WithdrawAddrEnabled = true
assert.NilError(t, f.distrKeeper.SetParams(f.sdkCtx, params))
assert.NilError(t, f.distrKeeper.Params.Set(f.sdkCtx, params))
},
msg: &distrtypes.MsgSetWithdrawAddress{
DelegatorAddress: emptyDelAddr.String(),
Expand All @@ -345,9 +345,9 @@ func TestMsgSetWithdrawAddress(t *testing.T) {
{
name: "empty withdraw address",
preRun: func() {
params, _ := f.distrKeeper.GetParams(f.sdkCtx)
params, _ := f.distrKeeper.Params.Get(f.sdkCtx)
params.WithdrawAddrEnabled = true
assert.NilError(t, f.distrKeeper.SetParams(f.sdkCtx, params))
assert.NilError(t, f.distrKeeper.Params.Set(f.sdkCtx, params))
},
msg: &distrtypes.MsgSetWithdrawAddress{
DelegatorAddress: delAddr.String(),
Expand All @@ -359,9 +359,9 @@ func TestMsgSetWithdrawAddress(t *testing.T) {
{
name: "both empty addresses",
preRun: func() {
params, _ := f.distrKeeper.GetParams(f.sdkCtx)
params, _ := f.distrKeeper.Params.Get(f.sdkCtx)
params.WithdrawAddrEnabled = true
assert.NilError(t, f.distrKeeper.SetParams(f.sdkCtx, params))
assert.NilError(t, f.distrKeeper.Params.Set(f.sdkCtx, params))
},
msg: &distrtypes.MsgSetWithdrawAddress{
DelegatorAddress: emptyDelAddr.String(),
Expand All @@ -373,9 +373,9 @@ func TestMsgSetWithdrawAddress(t *testing.T) {
{
name: "withdraw address disabled",
preRun: func() {
params, _ := f.distrKeeper.GetParams(f.sdkCtx)
params, _ := f.distrKeeper.Params.Get(f.sdkCtx)
params.WithdrawAddrEnabled = false
assert.NilError(t, f.distrKeeper.SetParams(f.sdkCtx, params))
assert.NilError(t, f.distrKeeper.Params.Set(f.sdkCtx, params))
},
msg: &distrtypes.MsgSetWithdrawAddress{
DelegatorAddress: delAddr.String(),
Expand All @@ -387,9 +387,9 @@ func TestMsgSetWithdrawAddress(t *testing.T) {
{
name: "valid msg with same delegator and withdraw address",
preRun: func() {
params, _ := f.distrKeeper.GetParams(f.sdkCtx)
params, _ := f.distrKeeper.Params.Get(f.sdkCtx)
params.WithdrawAddrEnabled = true
assert.NilError(t, f.distrKeeper.SetParams(f.sdkCtx, params))
assert.NilError(t, f.distrKeeper.Params.Set(f.sdkCtx, params))
},
msg: &distrtypes.MsgSetWithdrawAddress{
DelegatorAddress: delAddr.String(),
Expand All @@ -400,9 +400,9 @@ func TestMsgSetWithdrawAddress(t *testing.T) {
{
name: "valid msg",
preRun: func() {
params, _ := f.distrKeeper.GetParams(f.sdkCtx)
params, _ := f.distrKeeper.Params.Get(f.sdkCtx)
params.WithdrawAddrEnabled = true
assert.NilError(t, f.distrKeeper.SetParams(f.sdkCtx, params))
assert.NilError(t, f.distrKeeper.Params.Set(f.sdkCtx, params))
},
msg: &distrtypes.MsgSetWithdrawAddress{
DelegatorAddress: delAddr.String(),
Expand Down Expand Up @@ -754,7 +754,7 @@ func TestMsgUpdateParams(t *testing.T) {
assert.NilError(t, err)

// query the params and verify it has been updated
params, _ := f.distrKeeper.GetParams(f.sdkCtx)
params, _ := f.distrKeeper.Params.Get(f.sdkCtx)
assert.DeepEqual(t, distrtypes.DefaultParams(), params)
}
})
Expand All @@ -765,7 +765,7 @@ func TestMsgCommunityPoolSpend(t *testing.T) {
t.Parallel()
f := initFixture(t)

f.distrKeeper.SetParams(f.sdkCtx, distrtypes.DefaultParams())
f.distrKeeper.Params.Set(f.sdkCtx, distrtypes.DefaultParams())
f.distrKeeper.SetFeePool(f.sdkCtx, distrtypes.FeePool{
CommunityPool: sdk.NewDecCoins(sdk.DecCoin{Denom: "stake", Amount: math.LegacyNewDec(10000)}),
})
Expand Down Expand Up @@ -845,7 +845,7 @@ func TestMsgDepositValidatorRewardsPool(t *testing.T) {
t.Parallel()
f := initFixture(t)

f.distrKeeper.SetParams(f.sdkCtx, distrtypes.DefaultParams())
f.distrKeeper.Params.Set(f.sdkCtx, distrtypes.DefaultParams())
f.distrKeeper.SetFeePool(f.sdkCtx, distrtypes.FeePool{
CommunityPool: sdk.NewDecCoins(sdk.DecCoin{Denom: "stake", Amount: math.LegacyNewDec(100)}),
})
Expand Down
4 changes: 2 additions & 2 deletions x/distribution/keeper/allocation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func TestAllocateTokensToManyValidators(t *testing.T) {
)

// reset fee pool & set params
distrKeeper.SetParams(ctx, disttypes.DefaultParams())
distrKeeper.Params.Set(ctx, disttypes.DefaultParams())
distrKeeper.SetFeePool(ctx, disttypes.InitialFeePool())

// create validator with 50% commission
Expand Down Expand Up @@ -237,7 +237,7 @@ func TestAllocateTokensTruncation(t *testing.T) {

// reset fee pool
distrKeeper.SetFeePool(ctx, disttypes.InitialFeePool())
distrKeeper.SetParams(ctx, disttypes.DefaultParams())
distrKeeper.Params.Set(ctx, disttypes.DefaultParams())

// create validator with 10% commission
valAddr0 := sdk.ValAddress(valConsAddr0)
Expand Down
18 changes: 9 additions & 9 deletions x/distribution/keeper/delegation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestCalculateRewardsBasic(t *testing.T) {

// reset fee pool
distrKeeper.SetFeePool(ctx, disttypes.InitialFeePool())
distrKeeper.SetParams(ctx, disttypes.DefaultParams())
distrKeeper.Params.Set(ctx, disttypes.DefaultParams())

// create validator with 50% commission
valAddr := sdk.ValAddress(valConsAddr0)
Expand Down Expand Up @@ -131,7 +131,7 @@ func TestCalculateRewardsAfterSlash(t *testing.T) {

// reset fee pool
distrKeeper.SetFeePool(ctx, disttypes.InitialFeePool())
distrKeeper.SetParams(ctx, disttypes.DefaultParams())
distrKeeper.Params.Set(ctx, disttypes.DefaultParams())

// create validator with 50% commission
valAddr := sdk.ValAddress(valConsAddr0)
Expand Down Expand Up @@ -231,7 +231,7 @@ func TestCalculateRewardsAfterManySlashes(t *testing.T) {

// reset fee pool
distrKeeper.SetFeePool(ctx, disttypes.InitialFeePool())
distrKeeper.SetParams(ctx, disttypes.DefaultParams())
distrKeeper.Params.Set(ctx, disttypes.DefaultParams())

// create validator with 50% commission
valAddr := sdk.ValAddress(valConsAddr0)
Expand Down Expand Up @@ -351,7 +351,7 @@ func TestCalculateRewardsMultiDelegator(t *testing.T) {

// reset fee pool
distrKeeper.SetFeePool(ctx, disttypes.InitialFeePool())
distrKeeper.SetParams(ctx, disttypes.DefaultParams())
distrKeeper.Params.Set(ctx, disttypes.DefaultParams())

// create validator with 50% commission
valAddr := sdk.ValAddress(valConsAddr0)
Expand Down Expand Up @@ -446,7 +446,7 @@ func TestWithdrawDelegationRewardsBasic(t *testing.T) {

// reset fee pool
distrKeeper.SetFeePool(ctx, disttypes.InitialFeePool())
distrKeeper.SetParams(ctx, disttypes.DefaultParams())
distrKeeper.Params.Set(ctx, disttypes.DefaultParams())

// create validator with 50% commission
valAddr := sdk.ValAddress(valConsAddr0)
Expand Down Expand Up @@ -519,7 +519,7 @@ func TestCalculateRewardsAfterManySlashesInSameBlock(t *testing.T) {

// reset fee pool
distrKeeper.SetFeePool(ctx, disttypes.InitialFeePool())
distrKeeper.SetParams(ctx, disttypes.DefaultParams())
distrKeeper.Params.Set(ctx, disttypes.DefaultParams())

// create validator with 50% commission
valAddr := sdk.ValAddress(valConsAddr0)
Expand Down Expand Up @@ -631,7 +631,7 @@ func TestCalculateRewardsMultiDelegatorMultiSlash(t *testing.T) {

// reset fee pool
distrKeeper.SetFeePool(ctx, disttypes.InitialFeePool())
distrKeeper.SetParams(ctx, disttypes.DefaultParams())
distrKeeper.Params.Set(ctx, disttypes.DefaultParams())

valPower := int64(100)

Expand Down Expand Up @@ -763,7 +763,7 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) {

// reset fee pool
distrKeeper.SetFeePool(ctx, disttypes.InitialFeePool())
distrKeeper.SetParams(ctx, disttypes.DefaultParams())
distrKeeper.Params.Set(ctx, disttypes.DefaultParams())

// create validator with 50% commission
valAddr := sdk.ValAddress(valConsAddr0)
Expand Down Expand Up @@ -959,7 +959,7 @@ func Test100PercentCommissionReward(t *testing.T) {

// reset fee pool
distrKeeper.SetFeePool(ctx, disttypes.InitialFeePool())
distrKeeper.SetParams(ctx, disttypes.DefaultParams())
distrKeeper.Params.Set(ctx, disttypes.DefaultParams())

// create validator with 50% commission
valAddr := sdk.ValAddress(valConsAddr0)
Expand Down
4 changes: 2 additions & 2 deletions x/distribution/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, data types.GenesisState) {

k.SetFeePool(ctx, data.FeePool)

if err := k.SetParams(ctx, data.Params); err != nil {
if err := k.Params.Set(ctx, data.Params); err != nil {
panic(err)
}

Expand Down Expand Up @@ -114,7 +114,7 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
panic(err)
}

params, err := k.GetParams(ctx)
params, err := k.Params.Get(ctx)
if err != nil {
panic(err)
}
Expand Down
2 changes: 1 addition & 1 deletion x/distribution/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func NewQuerier(keeper Keeper) Querier {

// Params queries params of distribution module
func (k Querier) Params(c context.Context, req *types.QueryParamsRequest) (*types.QueryParamsResponse, error) {
params, err := k.GetParams(c)
params, err := k.Keeper.Params.Get(c)
if err != nil {
return nil, err
}
Expand Down
15 changes: 14 additions & 1 deletion x/distribution/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"

"cosmossdk.io/collections"
"cosmossdk.io/core/store"
errorsmod "cosmossdk.io/errors"
"cosmossdk.io/log"
Expand All @@ -25,6 +26,9 @@ type Keeper struct {
// should be the x/gov module account.
authority string

Schema collections.Schema
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
Params collections.Item[types.Params]

feeCollectorName string // name of the FeeCollector ModuleAccount
}

Expand All @@ -39,15 +43,24 @@ func NewKeeper(
panic(fmt.Sprintf("%s module account has not been set", types.ModuleName))
}

return Keeper{
sb := collections.NewSchemaBuilder(storeService)
k := Keeper{
storeService: storeService,
cdc: cdc,
authKeeper: ak,
bankKeeper: bk,
stakingKeeper: sk,
feeCollectorName: feeCollectorName,
authority: authority,
Params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[types.Params](cdc)),
}

schema, err := sb.Build()
if err != nil {
panic(err)
}
k.Schema = schema
return k
}

// GetAuthority returns the x/distribution module's authority.
Expand Down
4 changes: 2 additions & 2 deletions x/distribution/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ func TestSetWithdrawAddr(t *testing.T) {

params := types.DefaultParams()
params.WithdrawAddrEnabled = false
require.NoError(t, distrKeeper.SetParams(ctx, params))
require.NoError(t, distrKeeper.Params.Set(ctx, params))

err := distrKeeper.SetWithdrawAddr(ctx, delegatorAddr, withdrawAddr)
require.NotNil(t, err)

params.WithdrawAddrEnabled = true
require.NoError(t, distrKeeper.SetParams(ctx, params))
require.NoError(t, distrKeeper.Params.Set(ctx, params))

err = distrKeeper.SetWithdrawAddr(ctx, delegatorAddr, withdrawAddr)
require.Nil(t, err)
Expand Down
2 changes: 1 addition & 1 deletion x/distribution/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func (k msgServer) UpdateParams(ctx context.Context, msg *types.MsgUpdateParams)
return nil, err
}

if err := k.SetParams(ctx, msg.Params); err != nil {
if err := k.Params.Set(ctx, msg.Params); err != nil {
return nil, err
}

Expand Down
29 changes: 2 additions & 27 deletions x/distribution/keeper/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,11 @@ import (
"context"

"cosmossdk.io/math"

"github.com/cosmos/cosmos-sdk/x/distribution/types"
)

// GetParams returns the total set of distribution parameters.
func (k Keeper) GetParams(ctx context.Context) (params types.Params, err error) {
store := k.storeService.OpenKVStore(ctx)
bz, err := store.Get(types.ParamsKey)
if bz == nil || err != nil {
return params, err
}

err = k.cdc.Unmarshal(bz, &params)
return params, err
}

// SetParams sets the distribution parameters.
// CONTRACT: This method performs no validation of the parameters.
func (k Keeper) SetParams(ctx context.Context, params types.Params) error {
store := k.storeService.OpenKVStore(ctx)
bz, err := k.cdc.Marshal(&params)
if err != nil {
return err
}
return store.Set(types.ParamsKey, bz)
}

// GetCommunityTax returns the current distribution community tax.
func (k Keeper) GetCommunityTax(ctx context.Context) (math.LegacyDec, error) {
params, err := k.GetParams(ctx)
params, err := k.Params.Get(ctx)
if err != nil {
return math.LegacyDec{}, err
}
Expand All @@ -44,7 +19,7 @@ func (k Keeper) GetCommunityTax(ctx context.Context) (math.LegacyDec, error) {
// GetWithdrawAddrEnabled returns the current distribution withdraw address
// enabled parameter.
func (k Keeper) GetWithdrawAddrEnabled(ctx context.Context) (enabled bool, err error) {
params, err := k.GetParams(ctx)
params, err := k.Params.Get(ctx)
if err != nil {
return false, err
}
Expand Down
Loading