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(x/evidence): accept address codec in constructor #21859

Merged
merged 11 commits into from
Sep 25, 2024
13 changes: 10 additions & 3 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,8 @@ func NewSimApp(
cometService,
)

app.MintKeeper = mintkeeper.NewKeeper(appCodec, runtime.NewEnvironment(runtime.NewKVStoreService(keys[minttypes.StoreKey]), logger.With(log.ModuleKey, "x/mint")), app.StakingKeeper, app.AuthKeeper, app.BankKeeper, authtypes.FeeCollectorName, govModuleAddr)
app.MintKeeper = mintkeeper.NewKeeper(appCodec, runtime.NewEnvironment(runtime.NewKVStoreService(keys[minttypes.StoreKey]), logger.With(log.ModuleKey, "x/mint")), app.AuthKeeper, app.BankKeeper, authtypes.FeeCollectorName, govModuleAddr)
app.MintKeeper.SetMintFn(mintkeeper.DefaultMintFn(minttypes.DefaultInflationCalculationFn, app.StakingKeeper, app.MintKeeper))

app.PoolKeeper = poolkeeper.NewKeeper(appCodec, runtime.NewEnvironment(runtime.NewKVStoreService(keys[pooltypes.StoreKey]), logger.With(log.ModuleKey, "x/protocolpool")), app.AuthKeeper, app.BankKeeper, app.StakingKeeper, govModuleAddr)

Expand Down Expand Up @@ -439,7 +440,13 @@ func NewSimApp(

// create evidence keeper with router
evidenceKeeper := evidencekeeper.NewKeeper(
appCodec, runtime.NewEnvironment(runtime.NewKVStoreService(keys[evidencetypes.StoreKey]), logger.With(log.ModuleKey, "x/evidence"), runtime.EnvWithMsgRouterService(app.MsgServiceRouter()), runtime.EnvWithQueryRouterService(app.GRPCQueryRouter())), app.StakingKeeper, app.SlashingKeeper, app.ConsensusParamsKeeper, app.AuthKeeper.AddressCodec(),
appCodec,
runtime.NewEnvironment(runtime.NewKVStoreService(keys[evidencetypes.StoreKey]), logger.With(log.ModuleKey, "x/evidence"), runtime.EnvWithMsgRouterService(app.MsgServiceRouter()), runtime.EnvWithQueryRouterService(app.GRPCQueryRouter())),
app.StakingKeeper,
app.SlashingKeeper,
app.ConsensusParamsKeeper,
app.AuthKeeper.AddressCodec(),
app.StakingKeeper.ConsensusAddressCodec(),
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
)
// If evidence needs to be handled for the app, set routes in router here and seal
app.EvidenceKeeper = *evidenceKeeper
Expand Down Expand Up @@ -467,7 +474,7 @@ func NewSimApp(
bank.NewAppModule(appCodec, app.BankKeeper, app.AuthKeeper),
feegrantmodule.NewAppModule(appCodec, app.FeeGrantKeeper, app.interfaceRegistry),
gov.NewAppModule(appCodec, &app.GovKeeper, app.AuthKeeper, app.BankKeeper, app.PoolKeeper),
mint.NewAppModule(appCodec, app.MintKeeper, app.AuthKeeper, nil),
mint.NewAppModule(appCodec, app.MintKeeper, app.AuthKeeper),
slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AuthKeeper, app.BankKeeper, app.StakingKeeper, app.interfaceRegistry, cometService),
distr.NewAppModule(appCodec, app.DistrKeeper, app.StakingKeeper),
staking.NewAppModule(appCodec, app.StakingKeeper),
Expand Down
11 changes: 6 additions & 5 deletions x/evidence/depinject.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ type ModuleInputs struct {
EvidenceHandlers []eviclient.EvidenceHandler `optional:"true"`
CometService comet.Service

StakingKeeper types.StakingKeeper
SlashingKeeper types.SlashingKeeper
ConsensusKeeper types.ConsensusKeeper
AddressCodec address.Codec
StakingKeeper types.StakingKeeper
SlashingKeeper types.SlashingKeeper
ConsensusKeeper types.ConsensusKeeper
AddressCodec address.Codec
ConsensusAddressCodec address.Codec
}

type ModuleOutputs struct {
Expand All @@ -47,7 +48,7 @@ type ModuleOutputs struct {
}

func ProvideModule(in ModuleInputs) ModuleOutputs {
k := keeper.NewKeeper(in.Cdc, in.Environment, in.StakingKeeper, in.SlashingKeeper, in.ConsensusKeeper, in.AddressCodec)
k := keeper.NewKeeper(in.Cdc, in.Environment, in.StakingKeeper, in.SlashingKeeper, in.ConsensusKeeper, in.AddressCodec, in.ConsensusAddressCodec)
m := NewAppModule(in.Cdc, *k, in.CometService, in.EvidenceHandlers...)

return ModuleOutputs{EvidenceKeeper: *k, Module: m}
Expand Down
2 changes: 1 addition & 1 deletion x/evidence/keeper/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
// It's still ongoing discussion how should we treat and slash attacks with
// premeditation. So for now we agree to treat them in the same way.
case comet.LightClientAttack, comet.DuplicateVote:
evidence := types.FromABCIEvidence(evidence, k.stakingKeeper.ConsensusAddressCodec())
evidence := types.FromABCIEvidence(evidence, k.consensusAddressCodec)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

path flow from Begin/EndBlock to a panic call
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
err := k.handleEquivocationEvidence(ctx, evidence)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion x/evidence/keeper/infraction.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
// TODO: Some of the invalid constraints listed above may need to be reconsidered
// in the case of a lunatic attack.
func (k Keeper) handleEquivocationEvidence(ctx context.Context, evidence *types.Equivocation) error {
consAddr := evidence.GetConsensusAddress(k.stakingKeeper.ConsensusAddressCodec())
consAddr := evidence.GetConsensusAddress(k.consensusAddressCodec)
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved

validator, err := k.stakingKeeper.ValidatorByConsAddr(ctx, consAddr)
if err != nil {
Expand Down
30 changes: 16 additions & 14 deletions x/evidence/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ import (
type Keeper struct {
appmodule.Environment

cdc codec.BinaryCodec
router types.Router
stakingKeeper types.StakingKeeper
slashingKeeper types.SlashingKeeper
consensusKeeper types.ConsensusKeeper
addressCodec address.Codec
cdc codec.BinaryCodec
router types.Router
stakingKeeper types.StakingKeeper
slashingKeeper types.SlashingKeeper
consensusKeeper types.ConsensusKeeper
addressCodec address.Codec
consensusAddressCodec address.Codec

Schema collections.Schema
// Evidences key: evidence hash bytes | value: Evidence
Expand All @@ -38,17 +39,18 @@ type Keeper struct {
// NewKeeper creates a new Keeper object.
func NewKeeper(
cdc codec.BinaryCodec, env appmodule.Environment, stakingKeeper types.StakingKeeper,
slashingKeeper types.SlashingKeeper, ck types.ConsensusKeeper, ac address.Codec,
slashingKeeper types.SlashingKeeper, ck types.ConsensusKeeper, ac address.Codec, consensusAddressCodec address.Codec,
) *Keeper {
sb := collections.NewSchemaBuilder(env.KVStoreService)
k := &Keeper{
Environment: env,
cdc: cdc,
stakingKeeper: stakingKeeper,
slashingKeeper: slashingKeeper,
consensusKeeper: ck,
addressCodec: ac,
Evidences: collections.NewMap(sb, types.KeyPrefixEvidence, "evidences", collections.BytesKey, codec.CollInterfaceValue[exported.Evidence](cdc)),
Environment: env,
cdc: cdc,
stakingKeeper: stakingKeeper,
slashingKeeper: slashingKeeper,
consensusKeeper: ck,
addressCodec: ac,
consensusAddressCodec: consensusAddressCodec,
Evidences: collections.NewMap(sb, types.KeyPrefixEvidence, "evidences", collections.BytesKey, codec.CollInterfaceValue[exported.Evidence](cdc)),
}
schema, err := sb.Build()
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions x/evidence/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ func (suite *KeeperTestSuite) SetupTest() {
slashingKeeper,
ck,
address.NewBech32Codec("cosmos"),
address.NewBech32Codec("cosmosvalcons"),
)

suite.stakingKeeper = stakingKeeper
Expand Down
15 changes: 0 additions & 15 deletions x/evidence/testutil/expected_keepers_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions x/evidence/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"time"

st "cosmossdk.io/api/cosmos/staking/v1beta1"
"cosmossdk.io/core/address"
"cosmossdk.io/math"

cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
Expand All @@ -15,7 +14,6 @@ import (
// StakingKeeper defines the staking module interface contract needed by the
// evidence module.
type StakingKeeper interface {
ConsensusAddressCodec() address.Codec
ValidatorByConsAddr(context.Context, sdk.ConsAddress) (sdk.ValidatorI, error)
}

Expand Down
21 changes: 6 additions & 15 deletions x/mint/depinject.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,31 +64,22 @@ func ProvideModule(in ModuleInputs) ModuleOutputs {
panic(err)
}

if in.MintFn == nil {
panic("mintFn cannot be nil")
}

k := keeper.NewKeeper(
in.Cdc,
in.Environment,
in.StakingKeeper,
in.AccountKeeper,
in.BankKeeper,
feeCollectorName,
as,
)

if in.MintFn != nil && in.InflationCalculationFn != nil {
panic("MintFn and InflationCalculationFn cannot both be set")
}

// if no mintFn is provided, use the default minting function
if in.MintFn == nil {
// if no inflationCalculationFn is provided, use the default inflation calculation function
if in.InflationCalculationFn == nil {
in.InflationCalculationFn = types.DefaultInflationCalculationFn
}

in.MintFn = k.DefaultMintFn(in.InflationCalculationFn)
}
k.SetMintFn(in.MintFn)

m := NewAppModule(in.Cdc, k, in.AccountKeeper, in.MintFn)
m := NewAppModule(in.Cdc, k, in.AccountKeeper)

return ModuleOutputs{MintKeeper: k, Module: m, EpochHooks: epochstypes.EpochHooksWrapper{EpochHooks: m}}
}
2 changes: 1 addition & 1 deletion x/mint/epoch_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func (am AppModule) BeforeEpochStart(ctx context.Context, epochIdentifier string

oldMinter := minter

err = am.mintFn(ctx, am.keeper.Environment, &minter, epochIdentifier, epochNumber)
err = am.keeper.MintFn(ctx, &minter, epochIdentifier, epochNumber)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions x/mint/keeper/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

// BeginBlocker mints new tokens for the previous block.
func (k Keeper) BeginBlocker(ctx context.Context, mintFn types.MintFn) error {
func (k Keeper) BeginBlocker(ctx context.Context) error {
defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyBeginBlocker)

// fetch stored minter & params
Expand All @@ -22,7 +22,7 @@ func (k Keeper) BeginBlocker(ctx context.Context, mintFn types.MintFn) error {

// we pass -1 as epoch number to indicate that this is not an epoch minting,
// but a regular block minting. Same with epoch id "block".
err = mintFn(ctx, k.Environment, &minter, "block", -1)
err = k.MintFn(ctx, &minter, "block", -1)
if err != nil {
return err
}
Expand Down
4 changes: 4 additions & 0 deletions x/mint/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ func (keeper Keeper) InitGenesis(ctx context.Context, ak types.AccountKeeper, da

ak.GetModuleAccount(ctx, types.ModuleName)

if keeper.mintFn == nil {
panic("mintFn cannot be nil")
}

return nil
}

Expand Down
9 changes: 7 additions & 2 deletions x/mint/keeper/genesis_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package keeper_test

import (
"context"
"testing"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/suite"

"cosmossdk.io/collections"
"cosmossdk.io/core/appmodule"
"cosmossdk.io/log"
"cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"
Expand Down Expand Up @@ -51,14 +53,17 @@ func (s *GenesisTestSuite) SetupTest() {
s.sdkCtx = testCtx.Ctx
s.key = key

stakingKeeper := minttestutil.NewMockStakingKeeper(ctrl)
accountKeeper := minttestutil.NewMockAccountKeeper(ctrl)
bankKeeper := minttestutil.NewMockBankKeeper(ctrl)
s.accountKeeper = accountKeeper
accountKeeper.EXPECT().GetModuleAddress(minterAcc.Name).Return(minterAcc.GetAddress())
accountKeeper.EXPECT().GetModuleAccount(s.sdkCtx, minterAcc.Name).Return(minterAcc)

s.keeper = keeper.NewKeeper(s.cdc, runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger()), stakingKeeper, accountKeeper, bankKeeper, "", "")
s.keeper = keeper.NewKeeper(s.cdc, runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger()), accountKeeper, bankKeeper, "", "")
err := s.keeper.SetMintFn(func(ctx context.Context, env appmodule.Environment, minter *types.Minter, epochId string, epochNumber int64) error {
return nil
})
s.NoError(err)
}

func (s *GenesisTestSuite) TestImportExportGenesis() {
Expand Down
2 changes: 0 additions & 2 deletions x/mint/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,12 @@ func (suite *MintTestSuite) SetupTest() {
ctrl := gomock.NewController(suite.T())
accountKeeper := minttestutil.NewMockAccountKeeper(ctrl)
bankKeeper := minttestutil.NewMockBankKeeper(ctrl)
stakingKeeper := minttestutil.NewMockStakingKeeper(ctrl)

accountKeeper.EXPECT().GetModuleAddress("mint").Return(sdk.AccAddress{})

suite.mintKeeper = keeper.NewKeeper(
encCfg.Codec,
env,
stakingKeeper,
accountKeeper,
bankKeeper,
authtypes.FeeCollectorName,
Expand Down
Loading
Loading