Update tm pubkey references #7102

sahith-narahari · 2020-08-18T19:52:54Z

Description

This PR updates pubkey references from tendermint to sdk/crypto/keys/*
closes: #7008
ref: #7047

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.

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer
Review Codecov Report in the comment section below once CI passes

crypto/ledger/ledger_mock.go

tests/mocks/account_retriever.go

tac0turtle · 2020-08-26T12:35:46Z

any update on this?

tac0turtle · 2020-08-26T14:02:07Z

@alexanderbez @aaronc it seems this PR is failing because it is trying to use the sdk's ed25519 in places tendermint's ed25519 key is supposed to be used. Any idea why this is the case being that they are both []byte

EDIT: IGNORE

codecov · 2020-08-26T21:27:26Z

Codecov Report

Merging #7102 into master will increase coverage by 0.22%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##           master    #7102      +/-   ##
==========================================
+ Coverage   54.51%   54.74%   +0.22%     
==========================================
  Files         566      563       -3     
  Lines       38755    38624     -131     
==========================================
+ Hits        21128    21145      +17     
+ Misses      15897    15748     -149     
- Partials     1730     1731       +1

crypto/keys/ed25519/ed25519.go

tac0turtle · 2020-08-27T08:52:04Z

crypto/keys/secp256k1/secp256k1.go

@@ -19,6 +19,8 @@ var _ crypto.PrivKey = PrivKey{}
 const (
 	PrivKeySize = 32
 	keyType     = "secp256k1"
+	PrivKeyName = "tendermint/PrivKeySecp256k1"


this is to keep backwards compatibility

blushi · 2020-08-27T12:24:39Z

It looks like we're gonna stick with tendermint ed25519 keys, given how tightly coupled sdk is with tendermint.

But then that raises another issue with regards to defining pubkeys proto types: #7109.
In this draft PR #7147, I have just changed the proto ed25519 key types to use tendermint ed25519 (instead of cosmos ed25519). As you can see this test is failing

cosmos-sdk/crypto/keys/keys_test.go

Line 98 in ad81bd4

"tm ed25519 equals pb ed25519",

because it's trying to compare a tendermint pubkey with a cosmos proto pubkey, which will always return false since it'll use:
https://github.com/tendermint/tendermint/blob/9e98c74e3c00962a02732da3486b6f4d84ae5aaf/crypto/ed25519/ed25519.go#L165 instead of using Type() and Bytes() methods. @marbar3778 would there be a way to address this in tendermint?

tac0turtle · 2020-08-27T14:14:05Z

I think more or less this PR can be moved for review. Unless we want to further decouple areas that can be decoupled from tendermint?c

…smos-sdk into sahith/update-keys-ref

amaury1093

lgtm overall

x/staking/types/validator.go

crypto/keys/sr25519/encoding.go

Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com>

tac0turtle · 2020-08-28T13:09:46Z

aaron asked to add ed25519 back. Will add it back..

This reverts commit 66d2e1d.

…smos-sdk into sahith/update-keys-ref

tac0turtle · 2020-08-28T14:40:14Z

as per discussion on the sdk call, we will be removing ed25519 & sr25519 keys for now.

crypto/keys/encoding/codec.go

…smos-sdk into sahith/update-keys-ref

* Update pubkey references * Update ledger_mock * Migrate encoding from tm * Update pubkey prefix * revert ed25519 to tendermint key * random account revert * Revert ed25519 references * revert secp key name * test revert * remove ed25519 * Update x/staking/types/validator.go Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com> * Revert "remove ed25519" This reverts commit 66d2e1d. * remove ed25519 & sr25519 * Apply suggestions from code review * remove codec Co-authored-by: Marko Baricevic <marbar3778@yahoo.com> Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com>

Update pubkey references

7d2e3d5

sahith-narahari commented Aug 18, 2020

View reviewed changes

crypto/ledger/ledger_mock.go Show resolved Hide resolved

sahith-narahari commented Aug 18, 2020

View reviewed changes

tests/mocks/account_retriever.go Show resolved Hide resolved

sahith-narahari added 3 commits August 20, 2020 00:23

Merge branch 'master' into sahith/update-keys-ref

61efbb3

Update ledger_mock

dccf1ef

Migrate encoding from tm

71e415b

clevinson mentioned this pull request Aug 24, 2020

Tracking Issue: Stargate Release Candidate #7152

Closed

25 tasks

Update pubkey prefix

78b1b22

resolve conflicts

a33dd58

tac0turtle force-pushed the sahith/update-keys-ref branch from ef1692b to a33dd58 Compare August 26, 2020 13:16

revert ed25519 to tendermint key

6e28dab

tac0turtle and others added 2 commits August 26, 2020 16:15

random account revert

0f58161

Revert ed25519 references

9e3af96

revert secp key name

ea744da

tac0turtle reviewed Aug 27, 2020

View reviewed changes

crypto/keys/ed25519/ed25519.go Outdated Show resolved Hide resolved

tac0turtle reviewed Aug 27, 2020

View reviewed changes

tac0turtle added the C:Crypto label Aug 27, 2020

tac0turtle added 2 commits August 27, 2020 11:04

test revert

fd2ebe2

Merge branch 'master' into sahith/update-keys-ref

bc5ebbd

tac0turtle added 3 commits August 27, 2020 17:02

remove ed25519

66d2e1d

Merge branch 'sahith/update-keys-ref' of https://github.com/cosmos/co…

be33f1c

…smos-sdk into sahith/update-keys-ref

Merge branch 'master' into sahith/update-keys-ref

f1c6fcc

tac0turtle marked this pull request as ready for review August 27, 2020 15:16

tac0turtle requested review from alessio and alexanderbez as code owners August 27, 2020 15:16

tac0turtle requested a review from aaronc August 27, 2020 15:16

amaury1093 approved these changes Aug 27, 2020

View reviewed changes

x/staking/types/validator.go Outdated Show resolved Hide resolved

crypto/keys/sr25519/encoding.go Outdated Show resolved Hide resolved

Update x/staking/types/validator.go

00d040c

Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com>

tac0turtle added 2 commits August 28, 2020 15:11

Revert "remove ed25519"

42dd530

This reverts commit 66d2e1d.

Merge branch 'master' into sahith/update-keys-ref

1765d16

tac0turtle added the A:automerge Automatically merge PR once all prerequisites pass. label Aug 28, 2020

tac0turtle added 2 commits August 28, 2020 16:39

remove ed25519 & sr25519

2c1585f

Merge branch 'sahith/update-keys-ref' of https://github.com/cosmos/co…

bb6ddb2

…smos-sdk into sahith/update-keys-ref

tac0turtle reviewed Aug 28, 2020

View reviewed changes

crypto/keys/encoding/codec.go Outdated Show resolved Hide resolved

Apply suggestions from code review

9b97e9c

alexanderbez reviewed Aug 28, 2020

View reviewed changes

crypto/keys/encoding/codec.go Outdated Show resolved Hide resolved

alexanderbez reviewed Aug 28, 2020

View reviewed changes

crypto/keys/encoding/codec.go Outdated Show resolved Hide resolved

alexanderbez reviewed Aug 28, 2020

View reviewed changes

crypto/keys/encoding/codec.go Outdated Show resolved Hide resolved

alexanderbez approved these changes Aug 28, 2020

View reviewed changes

tac0turtle removed the A:automerge Automatically merge PR once all prerequisites pass. label Aug 28, 2020

tac0turtle added 2 commits August 28, 2020 17:53

remove codec

a6319ae

Merge branch 'sahith/update-keys-ref' of https://github.com/cosmos/co…

96d2d4c

…smos-sdk into sahith/update-keys-ref

tac0turtle added the A:automerge Automatically merge PR once all prerequisites pass. label Aug 28, 2020

mergify bot merged commit 23a9f46 into master Aug 28, 2020

mergify bot deleted the sahith/update-keys-ref branch August 28, 2020 16:02

blushi mentioned this pull request Sep 3, 2020

Create PubKey proto types #7109

Closed

amaury1093 mentioned this pull request Sep 4, 2020

PubKey proto types #7147

Merged

9 tasks

blushi mentioned this pull request Sep 18, 2020

Use protobuf public key in BaseAccount #6886

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update tm pubkey references #7102

Update tm pubkey references #7102

sahith-narahari commented Aug 18, 2020 •

edited

Loading

tac0turtle commented Aug 26, 2020

tac0turtle commented Aug 26, 2020 •

edited

Loading

codecov bot commented Aug 26, 2020 •

edited

Loading

tac0turtle Aug 27, 2020

blushi commented Aug 27, 2020 •

edited

Loading

tac0turtle commented Aug 27, 2020

amaury1093 left a comment

tac0turtle commented Aug 28, 2020

tac0turtle commented Aug 28, 2020

Update tm pubkey references #7102

Update tm pubkey references #7102

Conversation

sahith-narahari commented Aug 18, 2020 • edited Loading

Description

tac0turtle commented Aug 26, 2020

tac0turtle commented Aug 26, 2020 • edited Loading

codecov bot commented Aug 26, 2020 • edited Loading

Codecov Report

tac0turtle Aug 27, 2020

Choose a reason for hiding this comment

blushi commented Aug 27, 2020 • edited Loading

tac0turtle commented Aug 27, 2020

amaury1093 left a comment

Choose a reason for hiding this comment

tac0turtle commented Aug 28, 2020

tac0turtle commented Aug 28, 2020

sahith-narahari commented Aug 18, 2020 •

edited

Loading

tac0turtle commented Aug 26, 2020 •

edited

Loading

codecov bot commented Aug 26, 2020 •

edited

Loading

blushi commented Aug 27, 2020 •

edited

Loading