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.

Sign up for GitHub

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(x/distribution)!: remove Accounts.String() #19868

Merged
merged 8 commits into from
Mar 27, 2024
Merged

refactor(x/distribution)!: remove Accounts.String() #19868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b832358
del: remove AccAccount String
JulianToledano Mar 26, 2024
5c18394
update: remove ConsAddress String
JulianToledano Mar 26, 2024
e7fb825
update: remove ValAddress String
JulianToledano Mar 26, 2024
ca5569e
update: remove AddressFromBech32
JulianToledano Mar 26, 2024
f0ee986
lint
JulianToledano Mar 26, 2024
d28cd9a
fix: tests
JulianToledano Mar 26, 2024
83f3835
fix
JulianToledano Mar 26, 2024
e574b91
Merge branch 'main' into julian/distribution-accString-removal
sontrinh16 Mar 27, 2024
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: 3 additions & 0 deletions x/distribution/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* [#19868](https://github.com/cosmos/cosmos-sdk/pull/19868) Removes Accounts String method
* `NewMsgSetWithdrawAddress` now takes strings as argument instead of `sdk.AccAddress`.
* `NewGenesisState` now takes a string as argument instead of `sdk.ConsAddress`.
Comment on lines +34 to +36
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a space between sentences for better readability. Also, ensure that sentences start with an uppercase letter for consistency.

-    * `NewMsgSetWithdrawAddress` now takes strings as argument instead of `sdk.AccAddress`.
+    * `NewMsgSetWithdrawAddress` now takes strings as argument instead of `sdk.AccAddress`. 
+    * `NewGenesisState` now takes a string as argument instead of `sdk.ConsAddress`.
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
* [#19868](https://github.com/cosmos/cosmos-sdk/pull/19868) Removes Accounts String method
* `NewMsgSetWithdrawAddress` now takes strings as argument instead of `sdk.AccAddress`.
* `NewGenesisState` now takes a string as argument instead of `sdk.ConsAddress`.
* [#19868](https://github.com/cosmos/cosmos-sdk/pull/19868) Removes Accounts String method
* `NewMsgSetWithdrawAddress` now takes strings as argument instead of `sdk.AccAddress`.
* `NewGenesisState` now takes a string as argument instead of `sdk.ConsAddress`.

* [#19445](https://github.com/cosmos/cosmos-sdk/pull/19445) `appmodule.Environment` is received on the Keeper to get access to different application services
* [#17115](https://github.com/cosmos/cosmos-sdk/pull/17115) Use collections for `PreviousProposer` and `ValidatorSlashEvents`:
* remove from `Keeper`: `GetPreviousProposerConsAddr`, `SetPreviousProposerConsAddr`, `GetValidatorHistoricalReferenceCount`, `GetValidatorSlashEvent`, `SetValidatorSlashEvent`.
Expand Down
5 changes: 2 additions & 3 deletions x/distribution/client/common/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,17 @@ import (
"cosmossdk.io/x/distribution/types"

"github.com/cosmos/cosmos-sdk/client"
sdk "github.com/cosmos/cosmos-sdk/types"
)

// QueryDelegationRewards queries a delegation rewards between a delegator and a
// validator.
func QueryDelegationRewards(clientCtx client.Context, delAddr, valAddr string) ([]byte, int64, error) {
delegatorAddr, err := sdk.AccAddressFromBech32(delAddr)
delegatorAddr, err := clientCtx.AddressCodec.StringToBytes(delAddr)
if err != nil {
return nil, 0, err
}

validatorAddr, err := sdk.ValAddressFromBech32(valAddr)
validatorAddr, err := clientCtx.ValidatorAddressCodec.StringToBytes(valAddr)
if err != nil {
return nil, 0, err
}
Expand Down
7 changes: 6 additions & 1 deletion x/distribution/client/common/common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,15 @@ import (

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec/legacy"
codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil"
)

func TestQueryDelegationRewardsAddrValidation(t *testing.T) {
clientCtx := client.Context{}.WithLegacyAmino(legacy.Cdc)
cdcOpts := codectestutil.CodecOptions{}
clientCtx := client.Context{}.
WithLegacyAmino(legacy.Cdc).
WithAddressCodec(cdcOpts.GetAddressCodec()).
WithValidatorAddressCodec(cdcOpts.GetValidatorCodec())

type args struct {
delAddr string
Expand Down
7 changes: 6 additions & 1 deletion x/distribution/depinject.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ func ProvideModule(in ModuleInputs) ModuleOutputs {
authority = authtypes.NewModuleAddressOrBech32Address(in.Config.Authority)
}

authorityAddr, err := in.AccountKeeper.AddressCodec().BytesToString(authority)
if err != nil {
panic(err)
}

k := keeper.NewKeeper(
in.Cdc,
in.Environment,
Expand All @@ -65,7 +70,7 @@ func ProvideModule(in ModuleInputs) ModuleOutputs {
in.StakingKeeper,
in.PoolKeeper,
feeCollectorName,
authority.String(),
authorityAddr,
)

m := NewAppModule(in.Cdc, k, in.AccountKeeper, in.BankKeeper, in.StakingKeeper, in.PoolKeeper)
Expand Down
48 changes: 36 additions & 12 deletions x/distribution/keeper/allocation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ func TestAllocateTokensToValidatorWithCommission(t *testing.T) {
ctrl := gomock.NewController(t)
key := storetypes.NewKVStoreKey(disttypes.StoreKey)
testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test"))
encCfg := moduletestutil.MakeTestEncodingConfig(codectestutil.CodecOptions{}, distribution.AppModule{})
cdcOpts := codectestutil.CodecOptions{}
encCfg := moduletestutil.MakeTestEncodingConfig(cdcOpts, distribution.AppModule{})
ctx := testCtx.Ctx.WithHeaderInfo(header.Info{Time: time.Now()})

bankKeeper := distrtestutil.NewMockBankKeeper(ctrl)
Expand All @@ -47,6 +48,9 @@ func TestAllocateTokensToValidatorWithCommission(t *testing.T) {
accountKeeper.EXPECT().GetModuleAddress("distribution").Return(distrAcc.GetAddress())
stakingKeeper.EXPECT().ValidatorAddressCodec().Return(valCodec).AnyTimes()

authorityAddr, err := cdcOpts.GetAddressCodec().BytesToString(authtypes.NewModuleAddress("gov"))
require.NoError(t, err)

distrKeeper := keeper.NewKeeper(
encCfg.Codec,
env,
Expand All @@ -55,11 +59,13 @@ func TestAllocateTokensToValidatorWithCommission(t *testing.T) {
stakingKeeper,
poolKeeper,
"fee_collector",
authtypes.NewModuleAddress("gov").String(),
authorityAddr,
)

// create validator with 50% commission
val, err := distrtestutil.CreateValidator(valConsPk0, math.NewInt(100))
operatorAddr, err := stakingKeeper.ValidatorAddressCodec().BytesToString(valConsPk0.Address())
require.NoError(t, err)
val, err := distrtestutil.CreateValidator(valConsPk0, operatorAddr, math.NewInt(100))
require.NoError(t, err)
val.Commission = stakingtypes.NewCommission(math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDec(0))
stakingKeeper.EXPECT().ValidatorByConsAddr(gomock.Any(), sdk.GetConsAddress(valConsPk0)).Return(val, nil).AnyTimes()
Expand Down Expand Up @@ -92,7 +98,8 @@ func TestAllocateTokensToManyValidators(t *testing.T) {
ctrl := gomock.NewController(t)
key := storetypes.NewKVStoreKey(disttypes.StoreKey)
testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test"))
encCfg := moduletestutil.MakeTestEncodingConfig(codectestutil.CodecOptions{}, distribution.AppModule{})
cdcOpts := codectestutil.CodecOptions{}
encCfg := moduletestutil.MakeTestEncodingConfig(cdcOpts, distribution.AppModule{})
ctx := testCtx.Ctx.WithHeaderInfo(header.Info{Time: time.Now()})

bankKeeper := distrtestutil.NewMockBankKeeper(ctrl)
Expand All @@ -107,6 +114,9 @@ func TestAllocateTokensToManyValidators(t *testing.T) {

env := runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger())

authorityAddr, err := cdcOpts.GetAddressCodec().BytesToString(authtypes.NewModuleAddress("gov"))
require.NoError(t, err)

distrKeeper := keeper.NewKeeper(
encCfg.Codec,
env,
Expand All @@ -115,7 +125,7 @@ func TestAllocateTokensToManyValidators(t *testing.T) {
stakingKeeper,
poolKeeper,
"fee_collector",
authtypes.NewModuleAddress("gov").String(),
authorityAddr,
)

// reset fee pool & set params
Expand All @@ -124,14 +134,18 @@ func TestAllocateTokensToManyValidators(t *testing.T) {

// create validator with 50% commission
valAddr0 := sdk.ValAddress(valConsAddr0)
val0, err := distrtestutil.CreateValidator(valConsPk0, math.NewInt(100))
operatorAddr, err := stakingKeeper.ValidatorAddressCodec().BytesToString(valConsPk0.Address())
require.NoError(t, err)
val0, err := distrtestutil.CreateValidator(valConsPk0, operatorAddr, math.NewInt(100))
require.NoError(t, err)
val0.Commission = stakingtypes.NewCommission(math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDecWithPrec(5, 1), math.LegacyNewDec(0))
stakingKeeper.EXPECT().ValidatorByConsAddr(gomock.Any(), sdk.GetConsAddress(valConsPk0)).Return(val0, nil).AnyTimes()

// create second validator with 0% commission
valAddr1 := sdk.ValAddress(valConsAddr1)
val1, err := distrtestutil.CreateValidator(valConsPk1, math.NewInt(100))
operatorAddr, err = stakingKeeper.ValidatorAddressCodec().BytesToString(valConsPk1.Address())
require.NoError(t, err)
val1, err := distrtestutil.CreateValidator(valConsPk1, operatorAddr, math.NewInt(100))
require.NoError(t, err)
val1.Commission = stakingtypes.NewCommission(math.LegacyNewDec(0), math.LegacyNewDec(0), math.LegacyNewDec(0))
stakingKeeper.EXPECT().ValidatorByConsAddr(gomock.Any(), sdk.GetConsAddress(valConsPk1)).Return(val1, nil).AnyTimes()
Expand Down Expand Up @@ -224,7 +238,8 @@ func TestAllocateTokensTruncation(t *testing.T) {
ctrl := gomock.NewController(t)
key := storetypes.NewKVStoreKey(disttypes.StoreKey)
testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test"))
encCfg := moduletestutil.MakeTestEncodingConfig(codectestutil.CodecOptions{}, distribution.AppModule{})
cdcOpts := codectestutil.CodecOptions{}
encCfg := moduletestutil.MakeTestEncodingConfig(cdcOpts, distribution.AppModule{})
ctx := testCtx.Ctx.WithHeaderInfo(header.Info{Time: time.Now()})

bankKeeper := distrtestutil.NewMockBankKeeper(ctrl)
Expand All @@ -239,6 +254,9 @@ func TestAllocateTokensTruncation(t *testing.T) {

env := runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger())

authorityAddr, err := cdcOpts.GetAddressCodec().BytesToString(authtypes.NewModuleAddress("gov"))
require.NoError(t, err)

distrKeeper := keeper.NewKeeper(
encCfg.Codec,
env,
Expand All @@ -247,7 +265,7 @@ func TestAllocateTokensTruncation(t *testing.T) {
stakingKeeper,
poolKeeper,
"fee_collector",
authtypes.NewModuleAddress("gov").String(),
authorityAddr,
)

// reset fee pool
Expand All @@ -256,21 +274,27 @@ func TestAllocateTokensTruncation(t *testing.T) {

// create validator with 10% commission
valAddr0 := sdk.ValAddress(valConsAddr0)
val0, err := distrtestutil.CreateValidator(valConsPk0, math.NewInt(100))
operatorAddr, err := stakingKeeper.ValidatorAddressCodec().BytesToString(valConsPk0.Address())
require.NoError(t, err)
val0, err := distrtestutil.CreateValidator(valConsPk0, operatorAddr, math.NewInt(100))
require.NoError(t, err)
val0.Commission = stakingtypes.NewCommission(math.LegacyNewDecWithPrec(1, 1), math.LegacyNewDecWithPrec(1, 1), math.LegacyNewDec(0))
stakingKeeper.EXPECT().ValidatorByConsAddr(gomock.Any(), sdk.GetConsAddress(valConsPk0)).Return(val0, nil).AnyTimes()

// create second validator with 10% commission
valAddr1 := sdk.ValAddress(valConsAddr1)
val1, err := distrtestutil.CreateValidator(valConsPk1, math.NewInt(100))
operatorAddr, err = stakingKeeper.ValidatorAddressCodec().BytesToString(valConsPk1.Address())
require.NoError(t, err)
val1, err := distrtestutil.CreateValidator(valConsPk1, operatorAddr, math.NewInt(100))
require.NoError(t, err)
val1.Commission = stakingtypes.NewCommission(math.LegacyNewDecWithPrec(1, 1), math.LegacyNewDecWithPrec(1, 1), math.LegacyNewDec(0))
stakingKeeper.EXPECT().ValidatorByConsAddr(gomock.Any(), sdk.GetConsAddress(valConsPk1)).Return(val1, nil).AnyTimes()

// create third validator with 10% commission
valAddr2 := sdk.ValAddress(valConsAddr2)
val2, err := stakingtypes.NewValidator(sdk.ValAddress(valConsAddr2).String(), valConsPk1, stakingtypes.Description{})
valAddr2Str, err := cdcOpts.GetValidatorCodec().BytesToString(valAddr2)
require.NoError(t, err)
val2, err := stakingtypes.NewValidator(valAddr2Str, valConsPk1, stakingtypes.Description{})
require.NoError(t, err)
val2.Commission = stakingtypes.NewCommission(math.LegacyNewDecWithPrec(1, 1), math.LegacyNewDecWithPrec(1, 1), math.LegacyNewDec(0))
stakingKeeper.EXPECT().ValidatorByConsAddr(gomock.Any(), sdk.GetConsAddress(valConsPk2)).Return(val2, nil).AnyTimes()
Expand Down
Loading
Loading