refactor!(staking): remove comet crypto as a dep (#20295)

cosmos · May 7, 2024 · fc84ff6 · fc84ff6
1 parent 48d5ab4
commit fc84ff6
Show file tree

Hide file tree

Showing 10 changed files with 47 additions and 58 deletions.
diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go
@@ -38,6 +38,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/baseapp"
 	baseapptestutil "github.com/cosmos/cosmos-sdk/baseapp/testutil"
 	"github.com/cosmos/cosmos-sdk/baseapp/testutil/mock"
+	cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
 	"github.com/cosmos/cosmos-sdk/testutil"
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -1799,8 +1800,11 @@ func TestABCI_PrepareProposal_VoteExtensions(t *testing.T) {
 		},
 	}
 
+	pk, err := cryptocodec.FromCmtProtoPublicKey(tmPk)
+	require.NoError(t, err)
+
 	consAddr := sdk.ConsAddress(addr.String())
-	valStore.EXPECT().GetPubKeyByConsAddr(gomock.Any(), consAddr.Bytes()).Return(tmPk, nil)
+	valStore.EXPECT().GetPubKeyByConsAddr(gomock.Any(), consAddr.Bytes()).Return(pk, nil)
 
 	// set up baseapp
 	prepareOpt := func(bapp *baseapp.BaseApp) {
@@ -1828,7 +1832,7 @@ func TestABCI_PrepareProposal_VoteExtensions(t *testing.T) {
 
 	suite := NewBaseAppSuite(t, prepareOpt)
 
-	_, err := suite.baseApp.InitChain(&abci.InitChainRequest{
+	_, err = suite.baseApp.InitChain(&abci.InitChainRequest{
 		InitialHeight: 1,
 		ConsensusParams: &cmtproto.ConsensusParams{
 			Feature: &cmtproto.FeatureParams{
@@ -2092,7 +2096,10 @@ func TestBaseApp_VoteExtensions(t *testing.T) {
 				Secp256K1: pubKey.Bytes(),
 			},
 		}
-		valStore.EXPECT().GetPubKeyByConsAddr(gomock.Any(), val).Return(tmPk, nil)
+
+		pk, err := cryptocodec.FromCmtProtoPublicKey(tmPk)
+		require.NoError(t, err)
+		valStore.EXPECT().GetPubKeyByConsAddr(gomock.Any(), val).Return(pk, nil)
 	}
 
 	baseappOpts := func(app *baseapp.BaseApp) {

diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go
@@ -8,7 +8,6 @@ import (
 
 	"github.com/cockroachdb/errors"
 	abci "github.com/cometbft/cometbft/abci/types"
-	cmtprotocrypto "github.com/cometbft/cometbft/api/cometbft/crypto/v1"
 	cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1"
 	cryptoenc "github.com/cometbft/cometbft/crypto/encoding"
 	cmttypes "github.com/cometbft/cometbft/types"
@@ -17,6 +16,8 @@ import (
 
 	"cosmossdk.io/core/comet"
 
+	cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
+	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/mempool"
 )
@@ -26,7 +27,7 @@ type (
 	// extension signatures. Typically, this will be implemented by the x/staking
 	// module, which has knowledge of the CometBFT public key.
 	ValidatorStore interface {
-		GetPubKeyByConsAddr(context.Context, sdk.ConsAddress) (cmtprotocrypto.PublicKey, error)
+		GetPubKeyByConsAddr(context.Context, sdk.ConsAddress) (cryptotypes.PubKey, error)
 	}
 
 	// GasTx defines the contract that a transaction with a gas limit must implement.
@@ -110,7 +111,12 @@ func ValidateVoteExtensions(
 			return fmt.Errorf("failed to get validator %X public key: %w", valConsAddr, err)
 		}
 
-		cmtPubKey, err := cryptoenc.PubKeyFromProto(pubKeyProto)
+		cmtpk, err := cryptocodec.ToCmtProtoPublicKey(pubKeyProto)
+		if err != nil {
+			return fmt.Errorf("failed to convert validator %X public key: %w", valConsAddr, err)
+		}
+
+		cmtPubKey, err := cryptoenc.PubKeyFromProto(cmtpk)
 		if err != nil {
 			return fmt.Errorf("failed to convert validator %X public key: %w", valConsAddr, err)
 		}

diff --git a/baseapp/abci_utils_test.go b/baseapp/abci_utils_test.go
@@ -28,6 +28,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/baseapp/testutil/mock"
 	"github.com/cosmos/cosmos-sdk/client"
 	codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil"
+	cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
 	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -93,9 +94,11 @@ func NewABCIUtilsTestSuite(t *testing.T) *ABCIUtilsTestSuite {
 	s.valStore = valStore
 
 	// set up mock
-	s.valStore.EXPECT().GetPubKeyByConsAddr(gomock.Any(), s.vals[0].consAddr.Bytes()).Return(s.vals[0].tmPk, nil).AnyTimes()
-	s.valStore.EXPECT().GetPubKeyByConsAddr(gomock.Any(), s.vals[1].consAddr.Bytes()).Return(s.vals[1].tmPk, nil).AnyTimes()
-	s.valStore.EXPECT().GetPubKeyByConsAddr(gomock.Any(), s.vals[2].consAddr.Bytes()).Return(s.vals[2].tmPk, nil).AnyTimes()
+	for _, val := range s.vals {
+		pk, err := cryptocodec.FromCmtProtoPublicKey(val.tmPk)
+		require.NoError(t, err)
+		valStore.EXPECT().GetPubKeyByConsAddr(gomock.Any(), val.consAddr.Bytes()).Return(pk, nil).AnyTimes()
+	}
 
 	// create context
 	s.ctx = sdk.Context{}.WithConsensusParams(cmtproto.ConsensusParams{

diff --git a/baseapp/testutil/mock/mocks.go b/baseapp/testutil/mock/mocks.go
diff --git a/types/staking.go b/types/staking.go
@@ -1,8 +1,6 @@
 package types
 
 import (
-	cmtprotocrypto "github.com/cometbft/cometbft/api/cometbft/crypto/v1"
-
 	"cosmossdk.io/math"
 
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
@@ -87,7 +85,6 @@ type ValidatorI interface {
 	IsUnbonding() bool                                              // check if has status unbonding
 	GetOperator() string                                            // operator address to receive/return validators coins
 	ConsPubKey() (cryptotypes.PubKey, error)                        // validation consensus pubkey (cryptotypes.PubKey)
-	TmConsPublicKey() (cmtprotocrypto.PublicKey, error)             // validation consensus pubkey (CometBFT)
 	GetConsAddr() ([]byte, error)                                   // validation consensus address
 	GetTokens() math.Int                                            // validation tokens
 	GetBondedTokens() math.Int                                      // validator bonded tokens

diff --git a/x/staking/CHANGELOG.md b/x/staking/CHANGELOG.md
@@ -94,6 +94,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
     * remove `Keeper`: `SetLastTotalPower`, `GetLastTotalPower`
 * [#17335](https://github.com/cosmos/cosmos-sdk/pull/17335) Remove usage of `"cosmossdk.io/x/staking/types".Infraction_*` in favour of `"cosmossdk.io/api/cosmos/staking/v1beta1".Infraction_` in order to remove dependency between modules on staking
 * [#17655](https://github.com/cosmos/cosmos-sdk/pull/17655) `QueryHistoricalInfo` was adjusted to return `HistoricalRecord` and marked `Hist` as deprecated.
+* [#20295](https://github.com/cosmos/cosmos-sdk/pull/20295) `GetValidatorByConsAddr` now returns the Cosmos SDK `cryptotypes.Pubkey` instead of `cometcrypto.Publickey`. The caller is responsible to translate the returned value to the expected type. 
+    * Remove `CmtConsPublicKey()` and `TmConsPublicKey()` from `Validator` interface and as methods on the `Validator` struct.
 
 ### State Breaking changes
 

diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go
@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"time"
 
-	cmtprotocrypto "github.com/cometbft/cometbft/api/cometbft/crypto/v1"
 	gogotypes "github.com/cosmos/gogoproto/types"
 
 	"cosmossdk.io/collections"
@@ -17,6 +16,7 @@ import (
 	storetypes "cosmossdk.io/store/types"
 	"cosmossdk.io/x/staking/types"
 
+	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
@@ -594,15 +594,16 @@ func (k Keeper) IsValidatorJailed(ctx context.Context, addr sdk.ConsAddress) (bo
 }
 
 // GetPubKeyByConsAddr returns the consensus public key by consensus address.
-func (k Keeper) GetPubKeyByConsAddr(ctx context.Context, addr sdk.ConsAddress) (cmtprotocrypto.PublicKey, error) {
+// Caller receives a Cosmos SDK Pubkey type and must cast it to a comet type
+func (k Keeper) GetPubKeyByConsAddr(ctx context.Context, addr sdk.ConsAddress) (cryptotypes.PubKey, error) {
 	v, err := k.GetValidatorByConsAddr(ctx, addr)
 	if err != nil {
-		return cmtprotocrypto.PublicKey{}, err
+		return nil, err
 	}
 
-	pubkey, err := v.CmtConsPublicKey()
+	pubkey, err := v.ConsPubKey()
 	if err != nil {
-		return cmtprotocrypto.PublicKey{}, err
+		return nil, err
 	}
 
 	return pubkey, nil

diff --git a/x/staking/testutil/expected_keepers_mocks.go b/x/staking/testutil/expected_keepers_mocks.go
diff --git a/x/staking/types/expected_keepers.go b/x/staking/types/expected_keepers.go
@@ -3,8 +3,6 @@ package types
 import (
 	"context"
 
-	cmtprotocrypto "github.com/cometbft/cometbft/api/cometbft/crypto/v1"
-
 	st "cosmossdk.io/api/cosmos/staking/v1beta1"
 	"cosmossdk.io/core/address"
 	"cosmossdk.io/math"
@@ -74,7 +72,7 @@ type ValidatorSet interface {
 
 	// GetPubKeyByConsAddr returns the consensus public key for a validator. Used in vote
 	// extension validation.
-	GetPubKeyByConsAddr(context.Context, sdk.ConsAddress) (cmtprotocrypto.PublicKey, error)
+	GetPubKeyByConsAddr(context.Context, sdk.ConsAddress) (cryptotypes.PubKey, error)
 }
 
 // DelegationSet expected properties for the set of all delegations for a particular (noalias)

diff --git a/x/staking/types/validator.go b/x/staking/types/validator.go
@@ -7,16 +7,13 @@ import (
 	"strings"
 	"time"
 
-	cmtprotocrypto "github.com/cometbft/cometbft/api/cometbft/crypto/v1"
-
 	"cosmossdk.io/core/address"
 	"cosmossdk.io/core/appmodule"
 	"cosmossdk.io/errors"
 	"cosmossdk.io/math"
 
 	"github.com/cosmos/cosmos-sdk/codec"
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
-	cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
@@ -477,27 +474,6 @@ func (v Validator) ConsPubKey() (cryptotypes.PubKey, error) {
 	return pk, nil
 }
 
-// CmtConsPublicKey casts Validator.ConsensusPubkey to cmtprotocrypto.PubKey.
-func (v Validator) CmtConsPublicKey() (cmtprotocrypto.PublicKey, error) {
-	pk, err := v.ConsPubKey()
-	if err != nil {
-		return cmtprotocrypto.PublicKey{}, err
-	}
-
-	tmPk, err := cryptocodec.ToCmtProtoPublicKey(pk)
-	if err != nil {
-		return cmtprotocrypto.PublicKey{}, err
-	}
-
-	return tmPk, nil
-}
-
-// Deprecated: use CmtConsPublicKey instead
-// We do not delete this function as it is part of the ValidatorI interface
-func (v Validator) TmConsPublicKey() (cmtprotocrypto.PublicKey, error) {
-	return v.CmtConsPublicKey()
-}
-
 // GetConsAddr extracts Consensus key address
 func (v Validator) GetConsAddr() ([]byte, error) {
 	pk, ok := v.ConsensusPubkey.GetCachedValue().(cryptotypes.PubKey)