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
3 changes: 3 additions & 0 deletions collections/naming_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,23 @@ 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))
require.Equal(t, name, schema.Fields[0].Name)
}

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))
require.Equal(t, name, schema.Fields[0].Name)
}

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))
Expand Down
2 changes: 1 addition & 1 deletion schema/indexer/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions schema/indexer/start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 7 additions & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
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
2 changes: 1 addition & 1 deletion tests/integration/evidence/keeper/infraction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion tools/hubl/internal/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions x/evidence/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
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, 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
Loading