remove tendermint keys & toTMPubkey interface #7635
Conversation
crypto/keys/ed25519/ed25519.go
Outdated
| @@ -18,8 +18,8 @@ import ( | |||
| //------------------------------------- | |||
|
|
|||
| const ( | |||
| PrivKeyName = "cosmos/PrivKeyEd25519" | |||
| PubKeyName = "cosmos/PubKeyEd25519" | |||
| PrivKeyName = "tendermint/PrivKeyEd25519" | |||
There was a problem hiding this comment.
had to revert this to support backwards compatibility
There was a problem hiding this comment.
who is relaying on this?
There was a problem hiding this comment.
now that i think some more about it, it may not be needed, will test locally
Codecov Report
@@ Coverage Diff @@
## master #7635 +/- ##
==========================================
+ Coverage 54.21% 59.84% +5.63%
==========================================
Files 611 317 -294
Lines 38464 17983 -20481
==========================================
- Hits 20852 10762 -10090
+ Misses 15503 6073 -9430
+ Partials 2109 1148 -961 |
crypto/codec/amino.go
Outdated
| @@ -29,9 +28,6 @@ func RegisterCrypto(cdc *codec.LegacyAmino) { | |||
| cdc.RegisterInterface((*cryptotypes.PubKey)(nil), nil) | |||
| cdc.RegisterConcrete(sr25519.PubKey{}, | |||
| sr25519.PubKeyName, nil) | |||
| // TODO Same as above, for ED25519 | |||
| cdc.RegisterConcrete(tmed25519.PubKey{}, | |||
There was a problem hiding this comment.
Now we use sdk keys in the sdk and make them tm proto keys when we send them to Tendermint.
tac0turtle
force-pushed
the
marko/remove_toTM
branch
from
92681e8
to
9aa927b
Compare
tac0turtle
requested review from
aaronc,
alessio and
alexanderbez
as code owners
crypto/codec/proto.go
Outdated
|
|
||
| // PubKeyToProto takes a crypto.Pubkey and sets it into tendermint's public key oneof | ||
| // This is only used when sending information about validators to Tendermint (validator updates) | ||
| func PubKeyToProto(k tmcrypto.PubKey) (protocrypto.PublicKey, error) { |
There was a problem hiding this comment.
| func PubKeyToProto(k tmcrypto.PubKey) (protocrypto.PublicKey, error) { | |
| func PubKeyToProto(k cryptotypes.PubKey) (protocrypto.PublicKey, error) { |
So all in all:
- in the SDK, we manipulate
cryptotypes.PubKey(our own PubKey) - whenever we need to use a TM interface (validator updates,
tmtypes.NewValidator...), we call this function to convert toprotocrypto.PublicKey
In this case, the SDK should never have to use tmcrypto.PubKey, is that correct?
crypto/types/types.go
Outdated
|
|
||
| // IntoTmPubKey allows our own PubKey types be converted into Tendermint's | ||
| // pubkey types. | ||
| type IntoTmPubKey interface { |
There was a problem hiding this comment.
sounds good. A centralized function PubKeyToProto to make the conversion SDK-TM pubkeys is indeed probably a cleaner way to do thinngs.
crypto/keys/ed25519/ed25519.go
Outdated
| @@ -18,8 +18,8 @@ import ( | |||
| //------------------------------------- | |||
|
|
|||
| const ( | |||
| PrivKeyName = "cosmos/PrivKeyEd25519" | |||
| PubKeyName = "cosmos/PubKeyEd25519" | |||
| PrivKeyName = "tendermint/PrivKeyEd25519" | |||
There was a problem hiding this comment.
who is relaying on this?
types/address.go
Outdated
| } | ||
|
|
||
| return bech32.ConvertAndEncode(bech32Prefix, legacy.Cdc.MustMarshalBinaryBare(pkToMarshal)) | ||
| return bech32.ConvertAndEncode(bech32Prefix, legacy.Cdc.MustMarshalBinaryBare(pubkey)) |
There was a problem hiding this comment.
overall we want to remove this function.
x/staking/types/validator.go
Outdated
| if intoTmPk, ok := pk.(cryptotypes.IntoTmPubKey); ok { | ||
| return intoTmPk.AsTmPubKey() | ||
| } | ||
|
|
||
| return pk |
There was a problem hiding this comment.
let's see if all tests will pass after the #7597 merge
There was a problem hiding this comment.
unless we merge this one first muhahah lol
There was a problem hiding this comment.
dam, haha. looks like I have to undo some changes in that PR, it brought tendermint keys deeper into the sdk
crypto/codec/proto.go
Outdated
| @@ -25,3 +28,27 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) { | |||
| registry.RegisterImplementations((*cryptotypes.PubKey)(nil), &secp256k1.PubKey{}) | |||
| registry.RegisterImplementations((*cryptotypes.PubKey)(nil), &multisig.LegacyAminoPubKey{}) | |||
| } | |||
|
|
|||
| // PubKeyToProto takes a crypto.Pubkey and sets it into tendermint's public key oneof | |||
There was a problem hiding this comment.
| // PubKeyToProto takes a crypto.Pubkey and sets it into tendermint's public key oneof | |
| // PubKeyToProto takes a crypto.PubKey and sets it into tendermint's public key oneof |
x/staking/types/validator.go
Outdated
| if intoTmPk, ok := pk.(cryptotypes.IntoTmPubKey); ok { | ||
| return intoTmPk.AsTmPubKey() | ||
| } | ||
|
|
||
| return pk |
tac0turtle
force-pushed
the
marko/remove_toTM
branch
from
33685a2
to
73e7dad
Compare
|
no idea what happened here |
Description
Removes reliance on tendermint Pubkeys.
Removes totmPubkey interface
closes: #7529
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes