Skip to content

Commit

Permalink
Remove error from priv.PubKey() (cosmos#131)
Browse files Browse the repository at this point in the history
* Remove error from priv.PubKey()

* Update changelog
  • Loading branch information
cwgoes authored and liamsi committed Jun 12, 2018
1 parent 42c6a64 commit 8e27322
Show file tree
Hide file tree
Showing 11 changed files with 37 additions and 70 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## 0.9.0

BREAKING CHANGES

- `priv.PubKey()` no longer returns an error. Any applicable errors (such as when fetching the public key from a hardware wallet) should be checked and returned when constructing the private key.

## 0.8.0

**TBD**
Expand Down
8 changes: 4 additions & 4 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions encode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ func TestKeyEncodings(t *testing.T) {
assert.EqualValues(t, sig1, sig3)

// Check (de/en)codings of PubKeys.
pubKey, err := tc.privKey.PubKey()
assert.NoError(t, err)
pubKey := tc.privKey.PubKey()
var pub2, pub3 PubKey
checkAminoBinary(t, pubKey, &pub2, tc.pubSize)
assert.EqualValues(t, pubKey, pub2)
Expand Down
15 changes: 3 additions & 12 deletions keys/keybase.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ func (kb dbKeybase) CreateLedger(name string, path crypto.DerivationPath, algo S
if err != nil {
return nil, err
}
pub, err := priv.PubKey()
if err != nil {
return nil, err
}
pub := priv.PubKey()
return kb.writeLedgerKey(pub, path, name), nil
}

Expand Down Expand Up @@ -169,10 +166,7 @@ func (kb dbKeybase) Sign(name, passphrase string, msg []byte) (sig crypto.Signat
if err != nil {
return nil, nil, err
}
pub, err = priv.PubKey()
if err != nil {
return nil, nil, err
}
pub = priv.PubKey()
return sig, pub, nil
}

Expand Down Expand Up @@ -290,10 +284,7 @@ func (kb dbKeybase) writeLocalKey(priv crypto.PrivKey, name, passphrase string)
// encrypt private key using passphrase
privArmor := encryptArmorPrivKey(priv, passphrase)
// make Info
pub, err := priv.PubKey()
if err != nil {
panic(err)
}
pub := priv.PubKey()
info := newLocalInfo(name, pub, privArmor)
kb.writeInfo(info, name)
return info
Expand Down
3 changes: 1 addition & 2 deletions keys/keybase_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ func TestKeyManagement(t *testing.T) {
// create an offline key
o1 := "offline"
priv1 := crypto.GenPrivKeyEd25519()
pub1, err := priv1.PubKey()
require.Nil(t, err)
pub1 := priv1.PubKey()
i, err = cstore.CreateOffline(o1, pub1)
require.Nil(t, err)
require.Equal(t, pub1, i.GetPubKey())
Expand Down
42 changes: 10 additions & 32 deletions ledger_secp256k1.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,20 @@ type PrivKeyLedgerSecp256k1 struct {
func NewPrivKeyLedgerSecp256k1(path DerivationPath) (PrivKey, error) {
var pk PrivKeyLedgerSecp256k1
pk.Path = path
// getPubKey will cache the pubkey for later use,
// this allows us to return an error early if the ledger
// is not plugged in
_, err := pk.getPubKey()
// cache the pubkey for later use
pubKey, err := pk.getPubKey()
if err != nil {
return nil, err
}
pk.CachedPubKey = pubKey
return &pk, err
}

// ValidateKey allows us to verify the sanity of a key
// after loading it from disk
func (pk PrivKeyLedgerSecp256k1) ValidateKey() error {
// getPubKey will return an error if the ledger is not
// properly set up...
pub, err := pk.forceGetPubKey()
pub, err := pk.getPubKey()
if err != nil {
return err
}
Expand Down Expand Up @@ -86,45 +87,22 @@ func (pk PrivKeyLedgerSecp256k1) Sign(msg []byte) (Signature, error) {
if err != nil {
return nil, err
}

sig, err := signLedgerSecp256k1(dev, pk.Path, msg)
if err != nil {
return nil, err
}

pub, err := pubkeyLedgerSecp256k1(dev, pk.Path)
if err != nil {
return nil, err
}

// if we have no pubkey yet, store it for future queries
if pk.CachedPubKey == nil {
pk.CachedPubKey = pub
} else if !pk.CachedPubKey.Equals(pub) {
return nil, fmt.Errorf("stored key does not match signing key")
}
return sig, nil
}

// PubKey returns the stored PubKey
func (pk PrivKeyLedgerSecp256k1) PubKey() (PubKey, error) {
return pk.getPubKey()
func (pk PrivKeyLedgerSecp256k1) PubKey() PubKey {
return pk.CachedPubKey
}

// getPubKey reads the pubkey from cache or from the ledger itself
// getPubKey reads the pubkey the ledger itself
// since this involves IO, it may return an error, which is not exposed
// in the PubKey interface, so this function allows better error handling
func (pk PrivKeyLedgerSecp256k1) getPubKey() (key PubKey, err error) {
// if we have no pubkey, set it
if pk.CachedPubKey == nil {
pk.CachedPubKey, err = pk.forceGetPubKey()
}
return pk.CachedPubKey, err
}

// forceGetPubKey is like getPubKey but ignores any cached key
// and ensures we get it from the ledger itself.
func (pk PrivKeyLedgerSecp256k1) forceGetPubKey() (key PubKey, err error) {
dev, err := getLedger()
if err != nil {
return key, fmt.Errorf("cannot connect to Ledger device - error: %v", err)
Expand Down
6 changes: 2 additions & 4 deletions ledger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ func TestRealLedgerSecp256k1(t *testing.T) {

priv, err := NewPrivKeyLedgerSecp256k1(path)
require.Nil(t, err, "%+v", err)
pub, err := priv.PubKey()
require.Nil(t, err)
pub := priv.PubKey()
sig, err := priv.Sign(msg)
require.Nil(t, err)

Expand All @@ -33,8 +32,7 @@ func TestRealLedgerSecp256k1(t *testing.T) {
require.Nil(t, err, "%+v", err)

// make sure we get the same pubkey when we load from disk
pub2, err := priv2.PubKey()
require.Nil(t, err)
pub2 := priv2.PubKey()
require.Equal(t, pub, pub2)

// signing with the loaded key should match the original pubkey
Expand Down
10 changes: 5 additions & 5 deletions priv_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func PrivKeyFromBytes(privKeyBytes []byte) (privKey PrivKey, err error) {
type PrivKey interface {
Bytes() []byte
Sign(msg []byte) (Signature, error)
PubKey() (PubKey, error)
PubKey() PubKey
Equals(PrivKey) bool
}

Expand All @@ -39,10 +39,10 @@ func (privKey PrivKeyEd25519) Sign(msg []byte) (Signature, error) {
return SignatureEd25519(*signatureBytes), nil
}

func (privKey PrivKeyEd25519) PubKey() (PubKey, error) {
func (privKey PrivKeyEd25519) PubKey() PubKey {
privKeyBytes := [64]byte(privKey)
pubBytes := *ed25519.MakePublicKey(&privKeyBytes)
return PubKeyEd25519(pubBytes), nil
return PubKeyEd25519(pubBytes)
}

// Equals - you probably don't need to use this.
Expand Down Expand Up @@ -115,11 +115,11 @@ func (privKey PrivKeySecp256k1) Sign(msg []byte) (Signature, error) {
return SignatureSecp256k1(sig__.Serialize()), nil
}

func (privKey PrivKeySecp256k1) PubKey() (PubKey, error) {
func (privKey PrivKeySecp256k1) PubKey() PubKey {
_, pub__ := secp256k1.PrivKeyFromBytes(secp256k1.S256(), privKey[:])
var pub PubKeySecp256k1
copy(pub[:], pub__.SerializeCompressed())
return pub, nil
return pub
}

// Equals - you probably don't need to use this.
Expand Down
3 changes: 1 addition & 2 deletions priv_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ func TestGeneratePrivKey(t *testing.T) {
testPriv := crypto.GenPrivKeyEd25519()
testGenerate := testPriv.Generate(1)
signBytes := []byte("something to sign")
pub, err := testGenerate.PubKey()
assert.NoError(t, err)
pub := testGenerate.PubKey()
sig, err := testGenerate.Sign(signBytes)
assert.NoError(t, err)
assert.True(t, pub.VerifyBytes(signBytes, sig))
Expand Down
3 changes: 1 addition & 2 deletions pub_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ func TestPubKeySecp256k1Address(t *testing.T) {
var priv PrivKeySecp256k1
copy(priv[:], privB)

pubKey, err := priv.PubKey()
assert.NoError(t, err)
pubKey := priv.PubKey()
pubT, _ := pubKey.(PubKeySecp256k1)
pub := pubT[:]
addr := pubKey.Address()
Expand Down
8 changes: 3 additions & 5 deletions signature_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@ package crypto
import (
"testing"

"github.com/stretchr/testify/require"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestSignAndValidateEd25519(t *testing.T) {

privKey := GenPrivKeyEd25519()
pubKey, err := privKey.PubKey()
require.Nil(t, err)
pubKey := privKey.PubKey()

msg := CRandBytes(128)
sig, err := privKey.Sign(msg)
Expand All @@ -30,8 +29,7 @@ func TestSignAndValidateEd25519(t *testing.T) {

func TestSignAndValidateSecp256k1(t *testing.T) {
privKey := GenPrivKeySecp256k1()
pubKey, err := privKey.PubKey()
require.Nil(t, err)
pubKey := privKey.PubKey()

msg := CRandBytes(128)
sig, err := privKey.Sign(msg)
Expand Down

0 comments on commit 8e27322

Please sign in to comment.