-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Replace tmcrypto.PubKey by our own cryptotypes.PubKey #7419
Changes from all commits
735e283
a328fff
c35c504
597e52e
bc80e35
73554f8
dac3a5f
40bf06d
8b53143
b486ced
e186e34
8d30ba4
8f2d66e
51a353f
7c9d764
173ff81
1fdc967
57582ed
c4643b9
c1b5137
019055e
ea134ea
1599b14
c9f0aef
4684bf0
27210a0
189590a
19be854
a35a905
fffd3f4
545e619
e422dbb
0048063
f0d97b9
62e8f69
3c72331
9800d82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,57 @@ | ||||||||||
package codec | ||||||||||
|
||||||||||
import ( | ||||||||||
tmcrypto "github.com/tendermint/tendermint/crypto" | ||||||||||
"github.com/tendermint/tendermint/crypto/encoding" | ||||||||||
tmprotocrypto "github.com/tendermint/tendermint/proto/tendermint/crypto" | ||||||||||
|
||||||||||
"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" | ||||||||||
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" | ||||||||||
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | ||||||||||
) | ||||||||||
|
||||||||||
// FromTmProtoPublicKey converts a TM's tmprotocrypto.PublicKey into our own PubKey. | ||||||||||
func FromTmProtoPublicKey(protoPk tmprotocrypto.PublicKey) (cryptotypes.PubKey, error) { | ||||||||||
switch protoPk := protoPk.Sum.(type) { | ||||||||||
case *tmprotocrypto.PublicKey_Ed25519: | ||||||||||
return &ed25519.PubKey{ | ||||||||||
Key: protoPk.Ed25519, | ||||||||||
}, nil | ||||||||||
default: | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
secp was added in rc6 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to keep things nice and clean (this PR is getting big already), i created a separate one #7838 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah okay, thank you. Should this turn the pubkey straight into the tendermint proto public key? does the tendermint pubkey get used anywhere in the sdk that makes it needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good question, and I was hesitating about this one. In the SDK, we use both:
I'm happy for this function to turn it directly to a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh I didnt know tmtypes.validator is used in the sdk other than sending updates to tm. maybe its fine for now and if the work to decouple that happens this can be done then. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually changed this function to return proto PublicKey, in @marbar3778 btw question: why doesn't the proto PublicKey implement There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question! |
||||||||||
return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "cannot convert %v from Tendermint public key", protoPk) | ||||||||||
} | ||||||||||
robert-zaremba marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
} | ||||||||||
|
||||||||||
// ToTmProtoPublicKey converts our own PubKey to TM's tmprotocrypto.PublicKey. | ||||||||||
func ToTmProtoPublicKey(pk cryptotypes.PubKey) (tmprotocrypto.PublicKey, error) { | ||||||||||
switch pk := pk.(type) { | ||||||||||
case *ed25519.PubKey: | ||||||||||
return tmprotocrypto.PublicKey{ | ||||||||||
Sum: &tmprotocrypto.PublicKey_Ed25519{ | ||||||||||
Ed25519: pk.Key, | ||||||||||
}, | ||||||||||
}, nil | ||||||||||
default: | ||||||||||
amaury1093 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
return tmprotocrypto.PublicKey{}, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "cannot convert %v to Tendermint public key", pk) | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
// FromTmPubKeyInterface converts TM's tmcrypto.PubKey to our own PubKey. | ||||||||||
func FromTmPubKeyInterface(tmPk tmcrypto.PubKey) (cryptotypes.PubKey, error) { | ||||||||||
tmProtoPk, err := encoding.PubKeyToProto(tmPk) | ||||||||||
if err != nil { | ||||||||||
return nil, err | ||||||||||
} | ||||||||||
|
||||||||||
return FromTmProtoPublicKey(tmProtoPk) | ||||||||||
} | ||||||||||
|
||||||||||
// ToTmPubKeyInterface converts our own PubKey to TM's tmcrypto.PubKey. | ||||||||||
func ToTmPubKeyInterface(pk cryptotypes.PubKey) (tmcrypto.PubKey, error) { | ||||||||||
tmProtoPk, err := ToTmProtoPublicKey(pk) | ||||||||||
if err != nil { | ||||||||||
return nil, err | ||||||||||
} | ||||||||||
|
||||||||||
return encoding.PubKeyFromProto(tmProtoPk) | ||||||||||
} | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why don't we use here the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TM supports secp256k1 too now. I could add a switch .(type) here, and do what you suggest. However, I feel it's a bit cleaner to have a central function to do conversion between sdk pubkey <-> tm pubkey (aka |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to stay for backwards compat for multisigs?