From 37a9bc3bb67bd82d4493d2d86f8cd31c0e768880 Mon Sep 17 00:00:00 2001 From: Likhita Polavarapu <78951027+likhita-809@users.noreply.github.com> Date: Thu, 1 Dec 2022 14:09:10 +0530 Subject: [PATCH] refactor: x/evidence audit changes (#14092) ## Description ref: #14063 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- .../evidence/v1beta1/evidence.pulsar.go | 12 ++-- api/cosmos/evidence/v1beta1/tx.pulsar.go | 6 +- proto/cosmos/evidence/v1beta1/evidence.proto | 7 ++ proto/cosmos/evidence/v1beta1/tx.proto | 3 + x/evidence/client/cli/query.go | 2 + .../client/cli/{tx_test.go => query_test.go} | 0 x/evidence/client/evidence_handler.go | 1 + x/evidence/keeper/grpc_query_test.go | 27 +++++--- x/evidence/keeper/infraction_test.go | 7 +- x/evidence/keeper/keeper.go | 1 + x/evidence/keeper/keeper_test.go | 23 ++++--- x/evidence/keeper/msg_server_test.go | 65 +++++++++++++++++++ x/evidence/module.go | 4 +- x/evidence/types/codec.go | 1 + x/evidence/types/evidence.pb.go | 12 ++-- x/evidence/types/querier.go | 1 + x/evidence/types/tx.pb.go | 6 +- 17 files changed, 146 insertions(+), 32 deletions(-) rename x/evidence/client/cli/{tx_test.go => query_test.go} (100%) create mode 100644 x/evidence/keeper/msg_server_test.go diff --git a/api/cosmos/evidence/v1beta1/evidence.pulsar.go b/api/cosmos/evidence/v1beta1/evidence.pulsar.go index b1bc0d399a..4f526190ba 100644 --- a/api/cosmos/evidence/v1beta1/evidence.pulsar.go +++ b/api/cosmos/evidence/v1beta1/evidence.pulsar.go @@ -631,10 +631,14 @@ type Equivocation struct { sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - Height int64 `protobuf:"varint,1,opt,name=height,proto3" json:"height,omitempty"` - Time *timestamppb.Timestamp `protobuf:"bytes,2,opt,name=time,proto3" json:"time,omitempty"` - Power int64 `protobuf:"varint,3,opt,name=power,proto3" json:"power,omitempty"` - ConsensusAddress string `protobuf:"bytes,4,opt,name=consensus_address,json=consensusAddress,proto3" json:"consensus_address,omitempty"` + // height is the equivocation height. + Height int64 `protobuf:"varint,1,opt,name=height,proto3" json:"height,omitempty"` + // time is the equivocation time. + Time *timestamppb.Timestamp `protobuf:"bytes,2,opt,name=time,proto3" json:"time,omitempty"` + // power is the equivocation validator power. + Power int64 `protobuf:"varint,3,opt,name=power,proto3" json:"power,omitempty"` + // consensus_address is the equivocation validator consensus address. + ConsensusAddress string `protobuf:"bytes,4,opt,name=consensus_address,json=consensusAddress,proto3" json:"consensus_address,omitempty"` } func (x *Equivocation) Reset() { diff --git a/api/cosmos/evidence/v1beta1/tx.pulsar.go b/api/cosmos/evidence/v1beta1/tx.pulsar.go index 771e8d7a50..ce6a457ab6 100644 --- a/api/cosmos/evidence/v1beta1/tx.pulsar.go +++ b/api/cosmos/evidence/v1beta1/tx.pulsar.go @@ -958,8 +958,10 @@ type MsgSubmitEvidence struct { sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - Submitter string `protobuf:"bytes,1,opt,name=submitter,proto3" json:"submitter,omitempty"` - Evidence *anypb.Any `protobuf:"bytes,2,opt,name=evidence,proto3" json:"evidence,omitempty"` + // submitter is the signer account address of evidence. + Submitter string `protobuf:"bytes,1,opt,name=submitter,proto3" json:"submitter,omitempty"` + // evidence defines the evidence of misbehavior. + Evidence *anypb.Any `protobuf:"bytes,2,opt,name=evidence,proto3" json:"evidence,omitempty"` } func (x *MsgSubmitEvidence) Reset() { diff --git a/proto/cosmos/evidence/v1beta1/evidence.proto b/proto/cosmos/evidence/v1beta1/evidence.proto index 18a605b41d..e77ab82f70 100644 --- a/proto/cosmos/evidence/v1beta1/evidence.proto +++ b/proto/cosmos/evidence/v1beta1/evidence.proto @@ -16,9 +16,16 @@ message Equivocation { option (gogoproto.goproto_getters) = false; option (gogoproto.equal) = false; + // height is the equivocation height. int64 height = 1; + + // time is the equivocation time. google.protobuf.Timestamp time = 2 [(gogoproto.nullable) = false, (amino.dont_omitempty) = true, (gogoproto.stdtime) = true]; + + // power is the equivocation validator power. int64 power = 3; + + // consensus_address is the equivocation validator consensus address. string consensus_address = 4 [(cosmos_proto.scalar) = "cosmos.AddressString"]; } \ No newline at end of file diff --git a/proto/cosmos/evidence/v1beta1/tx.proto b/proto/cosmos/evidence/v1beta1/tx.proto index 20d5b21d10..8262395297 100644 --- a/proto/cosmos/evidence/v1beta1/tx.proto +++ b/proto/cosmos/evidence/v1beta1/tx.proto @@ -28,7 +28,10 @@ message MsgSubmitEvidence { option (gogoproto.equal) = false; option (gogoproto.goproto_getters) = false; + // submitter is the signer account address of evidence. string submitter = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; + + // evidence defines the evidence of misbehavior. google.protobuf.Any evidence = 2 [(cosmos_proto.accepts_interface) = "Evidence"]; } diff --git a/x/evidence/client/cli/query.go b/x/evidence/client/cli/query.go index f204e32462..12768fcbe7 100644 --- a/x/evidence/client/cli/query.go +++ b/x/evidence/client/cli/query.go @@ -62,6 +62,7 @@ func QueryEvidenceCmd() func(*cobra.Command, []string) error { } } +// queryEvidence queries for a single evidence by the given hash. func queryEvidence(clientCtx client.Context, hash string) error { queryClient := types.NewQueryClient(clientCtx) @@ -74,6 +75,7 @@ func queryEvidence(clientCtx client.Context, hash string) error { return clientCtx.PrintProto(res.Evidence) } +// queryAllEvidence returns all evidences. func queryAllEvidence(clientCtx client.Context, pageReq *query.PageRequest) error { queryClient := types.NewQueryClient(clientCtx) diff --git a/x/evidence/client/cli/tx_test.go b/x/evidence/client/cli/query_test.go similarity index 100% rename from x/evidence/client/cli/tx_test.go rename to x/evidence/client/cli/query_test.go diff --git a/x/evidence/client/evidence_handler.go b/x/evidence/client/evidence_handler.go index d677e58c3e..5bb577b916 100644 --- a/x/evidence/client/evidence_handler.go +++ b/x/evidence/client/evidence_handler.go @@ -12,6 +12,7 @@ type ( } ) +// NewEvidenceHandler returns an EvidenceHandler. func NewEvidenceHandler(cliHandler CLIHandlerFn) EvidenceHandler { return EvidenceHandler{ CLIHandler: cliHandler, diff --git a/x/evidence/keeper/grpc_query_test.go b/x/evidence/keeper/grpc_query_test.go index ebb7fd55a8..7103988410 100644 --- a/x/evidence/keeper/grpc_query_test.go +++ b/x/evidence/keeper/grpc_query_test.go @@ -2,6 +2,7 @@ package keeper_test import ( "fmt" + "strings" "github.com/cosmos/cosmos-sdk/types/query" "github.com/cosmos/cosmos-sdk/x/evidence/exported" @@ -18,24 +19,32 @@ func (suite *KeeperTestSuite) TestQueryEvidence() { msg string malleate func() expPass bool + expErrMsg string posttests func(res *types.QueryEvidenceResponse) }{ - { - "empty request", - func() { - req = &types.QueryEvidenceRequest{} - }, - false, - func(res *types.QueryEvidenceResponse) {}, - }, { "invalid request with empty evidence hash", func() { req = &types.QueryEvidenceRequest{Hash: ""} }, false, + "invalid request; hash is empty", func(res *types.QueryEvidenceResponse) {}, }, + { + "evidence not found", + func() { + numEvidence := 1 + evidence = suite.populateEvidence(suite.ctx, numEvidence) + evidenceHash := evidence[0].Hash().String() + reqHash := strings.Repeat("a", len(evidenceHash)) + req = types.NewQueryEvidenceRequest(reqHash) + }, + false, + "not found", + func(res *types.QueryEvidenceResponse) { + }, + }, { "success", func() { @@ -44,6 +53,7 @@ func (suite *KeeperTestSuite) TestQueryEvidence() { req = types.NewQueryEvidenceRequest(evidence[0].Hash().String()) }, true, + "", func(res *types.QueryEvidenceResponse) { var evi exported.Evidence err := suite.encCfg.InterfaceRegistry.UnpackAny(res.Evidence, &evi) @@ -66,6 +76,7 @@ func (suite *KeeperTestSuite) TestQueryEvidence() { suite.Require().NotNil(res) } else { suite.Require().Error(err) + suite.Require().Contains(err.Error(), tc.expErrMsg) suite.Require().Nil(res) } diff --git a/x/evidence/keeper/infraction_test.go b/x/evidence/keeper/infraction_test.go index 96fc36cfee..bd4940109c 100644 --- a/x/evidence/keeper/infraction_test.go +++ b/x/evidence/keeper/infraction_test.go @@ -3,6 +3,10 @@ package keeper_test import ( "time" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + codectypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/runtime" simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" @@ -17,9 +21,6 @@ import ( "github.com/cosmos/cosmos-sdk/x/staking" stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" stakingtestutil "github.com/cosmos/cosmos-sdk/x/staking/testutil" - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/suite" - tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) type InfractionTestSuite struct { diff --git a/x/evidence/keeper/keeper.go b/x/evidence/keeper/keeper.go index cf9640662e..7f374b094e 100644 --- a/x/evidence/keeper/keeper.go +++ b/x/evidence/keeper/keeper.go @@ -26,6 +26,7 @@ type Keeper struct { slashingKeeper types.SlashingKeeper } +// NewKeeper creates a new Keeper object. func NewKeeper( cdc codec.BinaryCodec, storeKey storetypes.StoreKey, stakingKeeper types.StakingKeeper, slashingKeeper types.SlashingKeeper, diff --git a/x/evidence/keeper/keeper_test.go b/x/evidence/keeper/keeper_test.go index 4bbc9932fd..a2ddefb1eb 100644 --- a/x/evidence/keeper/keeper_test.go +++ b/x/evidence/keeper/keeper_test.go @@ -5,21 +5,21 @@ import ( "fmt" "time" - "github.com/cosmos/cosmos-sdk/testutil" - moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" - "github.com/cosmos/cosmos-sdk/x/evidence" - evidencetestutil "github.com/cosmos/cosmos-sdk/x/evidence/testutil" "github.com/golang/mock/gomock" + "github.com/stretchr/testify/suite" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + "github.com/cosmos/cosmos-sdk/testutil" sdk "github.com/cosmos/cosmos-sdk/types" + moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" + "github.com/cosmos/cosmos-sdk/x/evidence" "github.com/cosmos/cosmos-sdk/x/evidence/exported" "github.com/cosmos/cosmos-sdk/x/evidence/keeper" + evidencetestutil "github.com/cosmos/cosmos-sdk/x/evidence/testutil" "github.com/cosmos/cosmos-sdk/x/evidence/types" - "github.com/stretchr/testify/suite" ) var ( @@ -81,14 +81,15 @@ type KeeperTestSuite struct { stakingKeeper *evidencetestutil.MockStakingKeeper queryClient types.QueryClient encCfg moduletestutil.TestEncodingConfig + msgServer types.MsgServer } func (suite *KeeperTestSuite) SetupTest() { encCfg := moduletestutil.MakeTestEncodingConfig(evidence.AppModuleBasic{}) key := sdk.NewKVStoreKey(types.StoreKey) tkey := sdk.NewTransientStoreKey("evidence_transient_store") - testCtx := testutil.DefaultContext(key, tkey) - suite.ctx = testCtx + testCtx := testutil.DefaultContextWithDB(suite.T(), key, tkey) + suite.ctx = testCtx.Ctx ctrl := gomock.NewController(suite.T()) @@ -111,7 +112,8 @@ func (suite *KeeperTestSuite) SetupTest() { router := types.NewRouter() router = router.AddRoute(types.RouteEquivocation, testEquivocationHandler(evidenceKeeper)) evidenceKeeper.SetRouter(router) - suite.ctx = testCtx.WithBlockHeader(tmproto.Header{Height: 1}) + + suite.ctx = testCtx.Ctx.WithBlockHeader(tmproto.Header{Height: 1}) suite.encCfg = moduletestutil.MakeTestEncodingConfig(evidence.AppModuleBasic{}) suite.accountKeeper = accountKeeper @@ -120,6 +122,11 @@ func (suite *KeeperTestSuite) SetupTest() { types.RegisterQueryServer(queryHelper, evidenceKeeper) suite.queryClient = types.NewQueryClient(queryHelper) suite.evidenceKeeper = *evidenceKeeper + + suite.Require().Equal(testCtx.Ctx.Logger().With("module", "x/"+types.ModuleName), + suite.evidenceKeeper.Logger(testCtx.Ctx)) + + suite.msgServer = keeper.NewMsgServerImpl(suite.evidenceKeeper) } func (suite *KeeperTestSuite) populateEvidence(ctx sdk.Context, numEvidence int) []exported.Evidence { diff --git a/x/evidence/keeper/msg_server_test.go b/x/evidence/keeper/msg_server_test.go new file mode 100644 index 0000000000..0637de3ca8 --- /dev/null +++ b/x/evidence/keeper/msg_server_test.go @@ -0,0 +1,65 @@ +package keeper_test + +import ( + "time" + + "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/evidence/types" +) + +func (s *KeeperTestSuite) TestSubmitEvidence() { + pk := ed25519.GenPrivKey() + + e := &types.Equivocation{ + Height: 1, + Power: 100, + Time: time.Now().UTC(), + ConsensusAddress: sdk.ConsAddress(pk.PubKey().Address().Bytes()).String(), + } + + validEvidence, err := types.NewMsgSubmitEvidence(sdk.AccAddress(valAddresses[0]), e) + s.Require().NoError(err) + + e2 := &types.Equivocation{ + Height: 0, + Power: 100, + Time: time.Now().UTC(), + ConsensusAddress: sdk.ConsAddress(pk.PubKey().Address().Bytes()).String(), + } + + invalidEvidence, err := types.NewMsgSubmitEvidence(sdk.AccAddress(valAddresses[0]), e2) + s.Require().NoError(err) + + testCases := []struct { + name string + req *types.MsgSubmitEvidence + expErr bool + expErrMsg string + }{ + { + name: "invalid evidence with height 0", + req: invalidEvidence, + expErr: true, + expErrMsg: "invalid equivocation height", + }, + { + name: "valid evidence", + req: validEvidence, + expErr: false, + }, + } + + for _, tc := range testCases { + tc := tc + s.Run(tc.name, func() { + _, err := s.msgServer.SubmitEvidence(s.ctx, tc.req) + if tc.expErr { + s.Require().Error(err) + s.Require().Contains(err.Error(), tc.expErrMsg) + } else { + s.Require().NoError(err) + } + }) + } +} diff --git a/x/evidence/module.go b/x/evidence/module.go index 2348a67f13..6edbd65c5a 100644 --- a/x/evidence/module.go +++ b/x/evidence/module.go @@ -45,7 +45,7 @@ type AppModuleBasic struct { evidenceHandlers []eviclient.EvidenceHandler // eviclient evidence submission handlers } -// NewAppModuleBasic crates a AppModuleBasic without the codec. +// NewAppModuleBasic creates a AppModuleBasic without the codec. func NewAppModuleBasic(evidenceHandlers ...eviclient.EvidenceHandler) AppModuleBasic { return AppModuleBasic{ evidenceHandlers: evidenceHandlers, @@ -100,6 +100,7 @@ func (AppModuleBasic) GetQueryCmd() *cobra.Command { return cli.GetQueryCmd() } +// RegisterInterfaces registers the evidence module's interface types func (AppModuleBasic) RegisterInterfaces(registry codectypes.InterfaceRegistry) { types.RegisterInterfaces(registry) } @@ -115,6 +116,7 @@ type AppModule struct { keeper keeper.Keeper } +// NewAppModule creates a new AppModule object. func NewAppModule(keeper keeper.Keeper) AppModule { return AppModule{ AppModuleBasic: AppModuleBasic{}, diff --git a/x/evidence/types/codec.go b/x/evidence/types/codec.go index 4ea3ef1ea8..67c221abbf 100644 --- a/x/evidence/types/codec.go +++ b/x/evidence/types/codec.go @@ -21,6 +21,7 @@ func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { cdc.RegisterConcrete(&Equivocation{}, "cosmos-sdk/Equivocation", nil) } +// RegisterInterfaces registers the interfaces types with the interface registry. func RegisterInterfaces(registry types.InterfaceRegistry) { registry.RegisterImplementations((*sdk.Msg)(nil), &MsgSubmitEvidence{}) registry.RegisterInterface( diff --git a/x/evidence/types/evidence.pb.go b/x/evidence/types/evidence.pb.go index 54f056957f..a79aa730b2 100644 --- a/x/evidence/types/evidence.pb.go +++ b/x/evidence/types/evidence.pb.go @@ -32,10 +32,14 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package // Equivocation implements the Evidence interface and defines evidence of double // signing misbehavior. type Equivocation struct { - Height int64 `protobuf:"varint,1,opt,name=height,proto3" json:"height,omitempty"` - Time time.Time `protobuf:"bytes,2,opt,name=time,proto3,stdtime" json:"time"` - Power int64 `protobuf:"varint,3,opt,name=power,proto3" json:"power,omitempty"` - ConsensusAddress string `protobuf:"bytes,4,opt,name=consensus_address,json=consensusAddress,proto3" json:"consensus_address,omitempty"` + // height is the equivocation height. + Height int64 `protobuf:"varint,1,opt,name=height,proto3" json:"height,omitempty"` + // time is the equivocation time. + Time time.Time `protobuf:"bytes,2,opt,name=time,proto3,stdtime" json:"time"` + // power is the equivocation validator power. + Power int64 `protobuf:"varint,3,opt,name=power,proto3" json:"power,omitempty"` + // consensus_address is the equivocation validator consensus address. + ConsensusAddress string `protobuf:"bytes,4,opt,name=consensus_address,json=consensusAddress,proto3" json:"consensus_address,omitempty"` } func (m *Equivocation) Reset() { *m = Equivocation{} } diff --git a/x/evidence/types/querier.go b/x/evidence/types/querier.go index 5c069a040f..87d9e25230 100644 --- a/x/evidence/types/querier.go +++ b/x/evidence/types/querier.go @@ -26,6 +26,7 @@ type QueryAllEvidenceParams struct { Limit int `json:"limit" yaml:"limit"` } +// NewQueryAllEvidenceParams creates a new instance to query all evidence params. func NewQueryAllEvidenceParams(page, limit int) QueryAllEvidenceParams { return QueryAllEvidenceParams{Page: page, Limit: limit} } diff --git a/x/evidence/types/tx.pb.go b/x/evidence/types/tx.pb.go index 9687187e02..a7e7de3e81 100644 --- a/x/evidence/types/tx.pb.go +++ b/x/evidence/types/tx.pb.go @@ -36,8 +36,10 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package // MsgSubmitEvidence represents a message that supports submitting arbitrary // Evidence of misbehavior such as equivocation or counterfactual signing. type MsgSubmitEvidence struct { - Submitter string `protobuf:"bytes,1,opt,name=submitter,proto3" json:"submitter,omitempty"` - Evidence *types.Any `protobuf:"bytes,2,opt,name=evidence,proto3" json:"evidence,omitempty"` + // submitter is the signer account address of evidence. + Submitter string `protobuf:"bytes,1,opt,name=submitter,proto3" json:"submitter,omitempty"` + // evidence defines the evidence of misbehavior. + Evidence *types.Any `protobuf:"bytes,2,opt,name=evidence,proto3" json:"evidence,omitempty"` } func (m *MsgSubmitEvidence) Reset() { *m = MsgSubmitEvidence{} }