From 62d2da5d5cf0baf7ab3d37c3170a7beb9c6fb2fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juli=C3=A1n=20Toledano?= Date: Fri, 12 Apr 2024 13:34:00 +0200 Subject: [PATCH] refactor(x/evidence)!: remove Address.String() (#20016) --- x/evidence/CHANGELOG.md | 1 + x/evidence/keeper/keeper_test.go | 25 ++++++++++++++++++++----- x/evidence/keeper/msg_server_test.go | 16 ++++++++++------ x/evidence/types/evidence_test.go | 23 ++++++++++++++--------- x/evidence/types/msgs.go | 4 ++-- 5 files changed, 47 insertions(+), 22 deletions(-) diff --git a/x/evidence/CHANGELOG.md b/x/evidence/CHANGELOG.md index 7683aa73566e..f92f3d5b2e44 100644 --- a/x/evidence/CHANGELOG.md +++ b/x/evidence/CHANGELOG.md @@ -27,6 +27,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Api Breaking Changes +* [#20016](https://github.com/cosmos/cosmos-sdk/pull/20016) `NewMsgSubmitEvidence` now takes a string as argument instead of an `AccAddress`. * [#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 diff --git a/x/evidence/keeper/keeper_test.go b/x/evidence/keeper/keeper_test.go index 61f9eb0ef4a2..1784b003b945 100644 --- a/x/evidence/keeper/keeper_test.go +++ b/x/evidence/keeper/keeper_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/suite" "cosmossdk.io/collections" + coreaddress "cosmossdk.io/core/address" "cosmossdk.io/core/header" "cosmossdk.io/log" storetypes "cosmossdk.io/store/types" @@ -74,6 +75,9 @@ type KeeperTestSuite struct { ctx sdk.Context + addressCodec coreaddress.Codec + consAddressCodec coreaddress.ConsensusAddressCodec + evidenceKeeper keeper.Keeper bankKeeper *evidencetestutil.MockBankKeeper accountKeeper *evidencetestutil.MockAccountKeeper @@ -91,6 +95,8 @@ func (suite *KeeperTestSuite) SetupTest() { tkey := storetypes.NewTransientStoreKey("evidence_transient_store") testCtx := testutil.DefaultContextWithDB(suite.T(), key, tkey) suite.ctx = testCtx.Ctx + suite.addressCodec = address.NewBech32Codec("cosmos") + suite.consAddressCodec = address.NewBech32Codec("cosmosvalcons") ctrl := gomock.NewController(suite.T()) @@ -137,11 +143,14 @@ func (suite *KeeperTestSuite) populateEvidence(ctx sdk.Context, numEvidence int) for i := 0; i < numEvidence; i++ { pk := ed25519.GenPrivKey() + consAddr, err := suite.consAddressCodec.BytesToString(pk.PubKey().Address()) + suite.Require().NoError(err) + evidence[i] = &types.Equivocation{ Height: 11, Power: 100, Time: time.Now().UTC(), - ConsensusAddress: sdk.ConsAddress(pk.PubKey().Address().Bytes()).String(), + ConsensusAddress: consAddr, } suite.Nil(suite.evidenceKeeper.SubmitEvidence(ctx, evidence[i])) @@ -153,12 +162,14 @@ func (suite *KeeperTestSuite) populateEvidence(ctx sdk.Context, numEvidence int) func (suite *KeeperTestSuite) TestSubmitValidEvidence() { ctx := suite.ctx.WithIsCheckTx(false) pk := ed25519.GenPrivKey() + consAddr, err := suite.consAddressCodec.BytesToString(pk.PubKey().Address()) + suite.Require().NoError(err) e := &types.Equivocation{ Height: 1, Power: 100, Time: time.Now().UTC(), - ConsensusAddress: sdk.ConsAddress(pk.PubKey().Address().Bytes()).String(), + ConsensusAddress: consAddr, } suite.Nil(suite.evidenceKeeper.SubmitEvidence(ctx, e)) @@ -171,12 +182,14 @@ func (suite *KeeperTestSuite) TestSubmitValidEvidence() { func (suite *KeeperTestSuite) TestSubmitValidEvidence_Duplicate() { ctx := suite.ctx.WithIsCheckTx(false) pk := ed25519.GenPrivKey() + consAddr, err := suite.consAddressCodec.BytesToString(pk.PubKey().Address()) + suite.Require().NoError(err) e := &types.Equivocation{ Height: 1, Power: 100, Time: time.Now().UTC(), - ConsensusAddress: sdk.ConsAddress(pk.PubKey().Address().Bytes()).String(), + ConsensusAddress: consAddr, } suite.Nil(suite.evidenceKeeper.SubmitEvidence(ctx, e)) @@ -190,14 +203,16 @@ func (suite *KeeperTestSuite) TestSubmitValidEvidence_Duplicate() { func (suite *KeeperTestSuite) TestSubmitInvalidEvidence() { ctx := suite.ctx.WithIsCheckTx(false) pk := ed25519.GenPrivKey() + consAddr, err := suite.consAddressCodec.BytesToString(pk.PubKey().Address()) + suite.Require().NoError(err) e := &types.Equivocation{ Height: 0, Power: 100, Time: time.Now().UTC(), - ConsensusAddress: sdk.ConsAddress(pk.PubKey().Address().Bytes()).String(), + ConsensusAddress: consAddr, } - err := suite.evidenceKeeper.SubmitEvidence(ctx, e) + err = suite.evidenceKeeper.SubmitEvidence(ctx, e) suite.ErrorIs(err, types.ErrInvalidEvidence) res, err := suite.evidenceKeeper.Evidences.Get(ctx, e.Hash()) diff --git a/x/evidence/keeper/msg_server_test.go b/x/evidence/keeper/msg_server_test.go index f5dd4b6b5fd2..86ac3447dcaa 100644 --- a/x/evidence/keeper/msg_server_test.go +++ b/x/evidence/keeper/msg_server_test.go @@ -6,30 +6,34 @@ import ( "cosmossdk.io/x/evidence/types" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" - sdk "github.com/cosmos/cosmos-sdk/types" ) func (s *KeeperTestSuite) TestSubmitEvidence() { pk := ed25519.GenPrivKey() + consAddr, err := s.consAddressCodec.BytesToString(pk.PubKey().Address()) + s.Require().NoError(err) e := &types.Equivocation{ Height: 1, Power: 100, Time: time.Now().UTC(), - ConsensusAddress: sdk.ConsAddress(pk.PubKey().Address().Bytes()).String(), + ConsensusAddress: consAddr, } - validEvidence, err := types.NewMsgSubmitEvidence(sdk.AccAddress(valAddress), e) + accAddr, err := s.addressCodec.BytesToString(valAddress) + s.Require().NoError(err) + + validEvidence, err := types.NewMsgSubmitEvidence(accAddr, e) s.Require().NoError(err) e2 := &types.Equivocation{ Height: 0, Power: 100, Time: time.Now().UTC(), - ConsensusAddress: sdk.ConsAddress(pk.PubKey().Address().Bytes()).String(), + ConsensusAddress: consAddr, } - invalidEvidence, err := types.NewMsgSubmitEvidence(sdk.AccAddress(valAddress), e2) + invalidEvidence, err := types.NewMsgSubmitEvidence(accAddr, e2) s.Require().NoError(err) testCases := []struct { @@ -47,7 +51,7 @@ func (s *KeeperTestSuite) TestSubmitEvidence() { { name: "missing evidence", req: &types.MsgSubmitEvidence{ - Submitter: sdk.AccAddress(valAddress).String(), + Submitter: accAddr, }, expErr: true, expErrMsg: "missing evidence: invalid evidence", diff --git a/x/evidence/types/evidence_test.go b/x/evidence/types/evidence_test.go index 6e527bf371b8..bfa20f82a98d 100644 --- a/x/evidence/types/evidence_test.go +++ b/x/evidence/types/evidence_test.go @@ -16,20 +16,24 @@ import ( ) func TestEquivocation_Valid(t *testing.T) { + consCodec := address.NewBech32Codec("cosmosvalcons") n, _ := time.Parse(time.RFC3339, "2006-01-02T15:04:05Z") - addr := sdk.ConsAddress("foo_________________") + addr, err := consCodec.BytesToString(sdk.ConsAddress("foo_________________")) + require.NoError(t, err) e := types.Equivocation{ Height: 100, Time: n, Power: 1000000, - ConsensusAddress: addr.String(), + ConsensusAddress: addr, } + consAddr, err := consCodec.BytesToString(e.GetConsensusAddress(consCodec)) + require.NoError(t, err) require.Equal(t, e.GetTotalPower(), int64(0)) require.Equal(t, e.GetValidatorPower(), e.Power) require.Equal(t, e.GetTime(), e.Time) - require.Equal(t, e.GetConsensusAddress(address.NewBech32Codec("cosmosvalcons")).String(), e.ConsensusAddress) + require.Equal(t, consAddr, e.ConsensusAddress) require.Equal(t, e.GetHeight(), e.Height) require.Equal(t, e.Route(), types.RouteEquivocation) require.Equal(t, strings.ToUpper(hex.EncodeToString(e.Hash())), "1E10F9267BEA3A9A4AB5302C2C510CC1AFD7C54E232DA5B2E3360DFAFACF7A76") @@ -39,7 +43,7 @@ func TestEquivocation_Valid(t *testing.T) { require.Equal(t, int64(0), e.GetTotalPower()) require.Equal(t, e.Power, e.GetValidatorPower()) require.Equal(t, e.Time, e.GetTime()) - require.Equal(t, e.ConsensusAddress, e.GetConsensusAddress(address.NewBech32Codec("cosmosvalcons")).String()) + require.Equal(t, e.ConsensusAddress, consAddr) require.Equal(t, e.Height, e.GetHeight()) require.Equal(t, types.RouteEquivocation, e.Route()) require.Equal(t, "1E10F9267BEA3A9A4AB5302C2C510CC1AFD7C54E232DA5B2E3360DFAFACF7A76", strings.ToUpper(hex.EncodeToString(e.Hash()))) @@ -49,7 +53,8 @@ func TestEquivocation_Valid(t *testing.T) { func TestEquivocationValidateBasic(t *testing.T) { var zeroTime time.Time - addr := sdk.ConsAddress("foo_________________") + addr, err := address.NewBech32Codec("cosmosvalcons").BytesToString(sdk.ConsAddress("foo_________________")) + require.NoError(t, err) n, _ := time.Parse(time.RFC3339, "2006-01-02T15:04:05Z") testCases := []struct { @@ -57,10 +62,10 @@ func TestEquivocationValidateBasic(t *testing.T) { e types.Equivocation expectErr bool }{ - {"valid", types.Equivocation{100, n, 1000000, addr.String()}, false}, - {"invalid time", types.Equivocation{100, zeroTime, 1000000, addr.String()}, true}, - {"invalid height", types.Equivocation{0, n, 1000000, addr.String()}, true}, - {"invalid power", types.Equivocation{100, n, 0, addr.String()}, true}, + {"valid", types.Equivocation{100, n, 1000000, addr}, false}, + {"invalid time", types.Equivocation{100, zeroTime, 1000000, addr}, true}, + {"invalid height", types.Equivocation{0, n, 1000000, addr}, true}, + {"invalid power", types.Equivocation{100, n, 0, addr}, true}, {"invalid address", types.Equivocation{100, n, 1000000, ""}, true}, } diff --git a/x/evidence/types/msgs.go b/x/evidence/types/msgs.go index 8d65aca64dbc..92df414692a5 100644 --- a/x/evidence/types/msgs.go +++ b/x/evidence/types/msgs.go @@ -18,7 +18,7 @@ var ( ) // NewMsgSubmitEvidence returns a new MsgSubmitEvidence with a signer/submitter. -func NewMsgSubmitEvidence(s sdk.AccAddress, evi exported.Evidence) (*MsgSubmitEvidence, error) { +func NewMsgSubmitEvidence(s string, evi exported.Evidence) (*MsgSubmitEvidence, error) { msg, ok := evi.(proto.Message) if !ok { return nil, fmt.Errorf("cannot proto marshal %T", evi) @@ -27,7 +27,7 @@ func NewMsgSubmitEvidence(s sdk.AccAddress, evi exported.Evidence) (*MsgSubmitEv if err != nil { return nil, err } - return &MsgSubmitEvidence{Submitter: s.String(), Evidence: any}, nil + return &MsgSubmitEvidence{Submitter: s, Evidence: any}, nil } func (m MsgSubmitEvidence) GetEvidence() exported.Evidence {