Skip to content

Commit

Permalink
refactor(x/evidence)!: remove Address.String() (cosmos#20016)
Browse files Browse the repository at this point in the history
  • Loading branch information
JulianToledano authored Apr 12, 2024
1 parent d40677a commit 62d2da5
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 22 deletions.
1 change: 1 addition & 0 deletions x/evidence/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
25 changes: 20 additions & 5 deletions x/evidence/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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())

Expand Down Expand Up @@ -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]))
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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())
Expand Down
16 changes: 10 additions & 6 deletions x/evidence/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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",
Expand Down
23 changes: 14 additions & 9 deletions x/evidence/types/evidence_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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())))
Expand All @@ -49,18 +53,19 @@ 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 {
name string
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},
}

Expand Down
4 changes: 2 additions & 2 deletions x/evidence/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down

0 comments on commit 62d2da5

Please sign in to comment.