From d54f6fa95b4aa61efada936b83161d0302faccdd Mon Sep 17 00:00:00 2001 From: Marko Date: Wed, 25 Sep 2024 11:42:27 +0200 Subject: [PATCH] refactor(x/evidence): accept address codec in constructor (#21859) --- collections/naming_test.go | 3 ++ schema/indexer/start.go | 2 +- schema/indexer/start_test.go | 1 + simapp/app.go | 8 ++++- .../evidence/keeper/infraction_test.go | 2 +- tools/hubl/internal/remote.go | 1 - x/evidence/CHANGELOG.md | 1 + x/evidence/depinject.go | 11 +++---- x/evidence/keeper/abci.go | 2 +- x/evidence/keeper/infraction.go | 2 +- x/evidence/keeper/keeper.go | 30 ++++++++++--------- x/evidence/keeper/keeper_test.go | 1 + x/evidence/testutil/expected_keepers_mocks.go | 15 ---------- x/evidence/types/expected_keepers.go | 2 -- 14 files changed, 39 insertions(+), 42 deletions(-) diff --git a/collections/naming_test.go b/collections/naming_test.go index 1dc271391433..39b275d920a9 100644 --- a/collections/naming_test.go +++ b/collections/naming_test.go @@ -33,6 +33,7 @@ func TestNaming(t *testing.T) { } func expectKeyCodecName[T any](t *testing.T, name string, cdc codec.KeyCodec[T]) { + t.Helper() schema, err := codec.KeySchemaCodec(cdc) require.NoError(t, err) require.Equal(t, 1, len(schema.Fields)) @@ -40,6 +41,7 @@ func expectKeyCodecName[T any](t *testing.T, name string, cdc codec.KeyCodec[T]) } func expectValueCodecName[T any](t *testing.T, name string, cdc codec.ValueCodec[T]) { + t.Helper() schema, err := codec.ValueSchemaCodec(cdc) require.NoError(t, err) require.Equal(t, 1, len(schema.Fields)) @@ -47,6 +49,7 @@ func expectValueCodecName[T any](t *testing.T, name string, cdc codec.ValueCodec } func expectKeyCodecNames[T any](t *testing.T, cdc codec.KeyCodec[T], names ...string) { + t.Helper() schema, err := codec.KeySchemaCodec(cdc) require.NoError(t, err) require.Equal(t, len(names), len(schema.Fields)) diff --git a/schema/indexer/start.go b/schema/indexer/start.go index 48ab3f7fc7e0..909db71ed61a 100644 --- a/schema/indexer/start.go +++ b/schema/indexer/start.go @@ -196,7 +196,7 @@ func unmarshalIndexingConfig(cfg interface{}) (*IndexingConfig, error) { return &res, err } -func unmarshalIndexerCustomConfig(cfg interface{}, expectedType interface{}) (interface{}, error) { +func unmarshalIndexerCustomConfig(cfg, expectedType interface{}) (interface{}, error) { typ := reflect.TypeOf(expectedType) if reflect.TypeOf(cfg).AssignableTo(typ) { return cfg, nil diff --git a/schema/indexer/start_test.go b/schema/indexer/start_test.go index e7efc9f62322..f586af435420 100644 --- a/schema/indexer/start_test.go +++ b/schema/indexer/start_test.go @@ -80,6 +80,7 @@ func TestStart(t *testing.T) { } func callCommit(t *testing.T, listener appdata.Listener) { + t.Helper() cb, err := listener.Commit(appdata.CommitData{}) if err != nil { t.Fatal(err) diff --git a/simapp/app.go b/simapp/app.go index 50b6d21df396..b02c5e14599f 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -442,7 +442,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(), ) // If evidence needs to be handled for the app, set routes in router here and seal app.EvidenceKeeper = *evidenceKeeper diff --git a/tests/integration/evidence/keeper/infraction_test.go b/tests/integration/evidence/keeper/infraction_test.go index 745407688cb2..6be779630903 100644 --- a/tests/integration/evidence/keeper/infraction_test.go +++ b/tests/integration/evidence/keeper/infraction_test.go @@ -154,7 +154,7 @@ func initFixture(tb testing.TB) *fixture { stakingKeeper.SetHooks(stakingtypes.NewMultiStakingHooks(slashingKeeper.Hooks())) - evidenceKeeper := keeper.NewKeeper(cdc, runtime.NewEnvironment(runtime.NewKVStoreService(keys[evidencetypes.StoreKey]), log.NewNopLogger(), runtime.EnvWithQueryRouterService(grpcQueryRouter), runtime.EnvWithMsgRouterService(msgRouter)), stakingKeeper, slashingKeeper, consensusParamsKeeper, addresscodec.NewBech32Codec(sdk.Bech32PrefixAccAddr)) + evidenceKeeper := keeper.NewKeeper(cdc, runtime.NewEnvironment(runtime.NewKVStoreService(keys[evidencetypes.StoreKey]), log.NewNopLogger(), runtime.EnvWithQueryRouterService(grpcQueryRouter), runtime.EnvWithMsgRouterService(msgRouter)), stakingKeeper, slashingKeeper, consensusParamsKeeper, addresscodec.NewBech32Codec(sdk.Bech32PrefixAccAddr), stakingKeeper.ConsensusAddressCodec()) router := evidencetypes.NewRouter() router = router.AddRoute(evidencetypes.RouteEquivocation, testEquivocationHandler(evidenceKeeper)) evidenceKeeper.SetRouter(router) diff --git a/tools/hubl/internal/remote.go b/tools/hubl/internal/remote.go index 0584c0800eb4..7f0fb24db8ab 100644 --- a/tools/hubl/internal/remote.go +++ b/tools/hubl/internal/remote.go @@ -45,7 +45,6 @@ func RemoteCommand(config *config.Config, configDir string) ([]*cobra.Command, e commands := []*cobra.Command{} for chain, chainConfig := range config.Chains { - chain, chainConfig := chain, chainConfig // load chain info chainInfo := NewChainInfo(configDir, chain, chainConfig) diff --git a/x/evidence/CHANGELOG.md b/x/evidence/CHANGELOG.md index b2e8c9d76acc..fb5ee8a1e34a 100644 --- a/x/evidence/CHANGELOG.md +++ b/x/evidence/CHANGELOG.md @@ -32,6 +32,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#19482](https://github.com/cosmos/cosmos-sdk/pull/19482) `appmodule.Environment` is passed to `NewKeeper` instead of individual services * [#19627](https://github.com/cosmos/cosmos-sdk/pull/19627) `NewAppModule` now takes in a `codec.Codec` as its first argument * [#21480](https://github.com/cosmos/cosmos-sdk/pull/21480) ConsensusKeeper is required to be passed to the keeper. +* [#21859](https://github.com/cosmos/cosmos-sdk/pull/21859) `NewKeeper` now takes in a consensus codec to avoid reliance on staking for decoding addresses. ## [v0.1.1](https://github.com/cosmos/cosmos-sdk/releases/tag/x/evidence/v0.1.1) - 2024-04-22 diff --git a/x/evidence/depinject.go b/x/evidence/depinject.go index ecbbbbde0330..ebd5c7bba40b 100644 --- a/x/evidence/depinject.go +++ b/x/evidence/depinject.go @@ -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 { @@ -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} diff --git a/x/evidence/keeper/abci.go b/x/evidence/keeper/abci.go index 86dbd78adf36..38a84ba4ce01 100644 --- a/x/evidence/keeper/abci.go +++ b/x/evidence/keeper/abci.go @@ -23,7 +23,7 @@ func (k Keeper) BeginBlocker(ctx context.Context, cometService comet.Service) er // 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) err := k.handleEquivocationEvidence(ctx, evidence) if err != nil { return err diff --git a/x/evidence/keeper/infraction.go b/x/evidence/keeper/infraction.go index 6e2c40289495..98482358cc05 100644 --- a/x/evidence/keeper/infraction.go +++ b/x/evidence/keeper/infraction.go @@ -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) validator, err := k.stakingKeeper.ValidatorByConsAddr(ctx, consAddr) if err != nil { diff --git a/x/evidence/keeper/keeper.go b/x/evidence/keeper/keeper.go index 8cab89fbdd0e..954fc18c5e18 100644 --- a/x/evidence/keeper/keeper.go +++ b/x/evidence/keeper/keeper.go @@ -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 @@ -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, 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 { diff --git a/x/evidence/keeper/keeper_test.go b/x/evidence/keeper/keeper_test.go index 6820bb826511..0113c9deaa28 100644 --- a/x/evidence/keeper/keeper_test.go +++ b/x/evidence/keeper/keeper_test.go @@ -111,6 +111,7 @@ func (suite *KeeperTestSuite) SetupTest() { slashingKeeper, ck, address.NewBech32Codec("cosmos"), + address.NewBech32Codec("cosmosvalcons"), ) suite.stakingKeeper = stakingKeeper diff --git a/x/evidence/testutil/expected_keepers_mocks.go b/x/evidence/testutil/expected_keepers_mocks.go index eb6c0f2785ad..b9efeb2f1385 100644 --- a/x/evidence/testutil/expected_keepers_mocks.go +++ b/x/evidence/testutil/expected_keepers_mocks.go @@ -10,7 +10,6 @@ import ( time "time" stakingv1beta1 "cosmossdk.io/api/cosmos/staking/v1beta1" - address "cosmossdk.io/core/address" math "cosmossdk.io/math" types "github.com/cosmos/cosmos-sdk/crypto/types" types0 "github.com/cosmos/cosmos-sdk/types" @@ -40,20 +39,6 @@ func (m *MockStakingKeeper) EXPECT() *MockStakingKeeperMockRecorder { return m.recorder } -// ConsensusAddressCodec mocks base method. -func (m *MockStakingKeeper) ConsensusAddressCodec() address.Codec { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ConsensusAddressCodec") - ret0, _ := ret[0].(address.Codec) - return ret0 -} - -// ConsensusAddressCodec indicates an expected call of ConsensusAddressCodec. -func (mr *MockStakingKeeperMockRecorder) ConsensusAddressCodec() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConsensusAddressCodec", reflect.TypeOf((*MockStakingKeeper)(nil).ConsensusAddressCodec)) -} - // ValidatorByConsAddr mocks base method. func (m *MockStakingKeeper) ValidatorByConsAddr(arg0 context.Context, arg1 types0.ConsAddress) (types0.ValidatorI, error) { m.ctrl.T.Helper() diff --git a/x/evidence/types/expected_keepers.go b/x/evidence/types/expected_keepers.go index a7ecdaf2ca38..7cb8486e145d 100644 --- a/x/evidence/types/expected_keepers.go +++ b/x/evidence/types/expected_keepers.go @@ -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" @@ -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) }