From fc20f757ecb0761de17c189de4380a0feea405e0 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 27 Aug 2018 18:42:03 -0400 Subject: [PATCH 1/5] Load ledger device at runtime --- crypto/ledger.go | 17 ++++++++++------- crypto/ledger_secp256k1.go | 31 ++++++++++++++++++------------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/crypto/ledger.go b/crypto/ledger.go index 9d446202f760..5489810bca59 100644 --- a/crypto/ledger.go +++ b/crypto/ledger.go @@ -6,13 +6,16 @@ import ( ledger "github.com/zondax/ledger-goclient" ) -// If ledger support (build tag) has been enabled, automically attempt to load -// and set the ledger device, ledgerDevice, if it has not already been set. +// If ledger support (build tag) has been enabled, set the DiscoverLedger +// function which is responsible for loading the Ledger device at runtime or +// returning an error. func init() { - device, err := ledger.FindLedger() - if err != nil { - ledgerDeviceErr = err - } else { - ledgerDevice = device + DiscoverLedger = func() (LedgerSECP256K1, error) { + device, err := ledger.FindLedger() + if err != nil { + return nil, err + } + + return device, nil } } diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index 8cb175d4f589..5f6bcb1ad762 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -1,24 +1,26 @@ package crypto import ( - "errors" "fmt" + "github.com/pkg/errors" + secp256k1 "github.com/btcsuite/btcd/btcec" tmcrypto "github.com/tendermint/tendermint/crypto" tmsecp256k1 "github.com/tendermint/tendermint/crypto/secp256k1" ) var ( - ledgerDevice LedgerSECP256K1 - ledgerDeviceErr error - - // ErrMissingLedgerDevice is used to reflect that a ledger device load has - // not been attempted. - ErrMissingLedgerDevice = errors.New("missing ledger device") + // DiscoverLedger defines a function to be invoked at runtime for discovering + // a connected Ledger device. + DiscoverLedger DiscoverLedgerFn ) type ( + // DiscoverLedgerFn defines a Ledger discovery function that returns a + // connected device or an error upon failure. + DiscoverLedgerFn func() (LedgerSECP256K1, error) + // DerivationPath represents a Ledger derivation path. DerivationPath []uint32 @@ -47,18 +49,21 @@ type ( // CONTRACT: The ledger device, ledgerDevice, must be loaded and set prior to // any creation of a PrivKeyLedgerSecp256k1. func NewPrivKeyLedgerSecp256k1(path DerivationPath) (tmcrypto.PrivKey, error) { - if ledgerDevice == nil { - err := ErrMissingLedgerDevice - if ledgerDeviceErr != nil { - err = ledgerDeviceErr + var ledgerDevice LedgerSECP256K1 + + if DiscoverLedger != nil { + device, err := DiscoverLedger() + if err != nil { + return nil, errors.Wrap(err, "failed to create PrivKeyLedgerSecp256k1") } - return nil, fmt.Errorf("failed to create PrivKeyLedgerSecp256k1: %v", err) + ledgerDevice = device + } else { + return nil, errors.New("no ledger discovery function defined") } pkl := &PrivKeyLedgerSecp256k1{Path: path, ledger: ledgerDevice} - // cache the pubkey for later use pubKey, err := pkl.getPubKey() if err != nil { return nil, err From 0245a4b5ee6cd0a85f037b83c96325a147862112 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 27 Aug 2018 18:45:34 -0400 Subject: [PATCH 2/5] Update pending log --- PENDING.md | 2 ++ crypto/ledger_secp256k1.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/PENDING.md b/PENDING.md index cee2ae14c4b3..11f718031210 100644 --- a/PENDING.md +++ b/PENDING.md @@ -79,5 +79,7 @@ BUG FIXES * SDK * \#1988 Make us compile on OpenBSD (disable ledger) [#1988] (https://github.com/cosmos/cosmos-sdk/issues/1988) * \#2105 Fix DB Iterator leak, which may leak a go routine. + * [ledger] \#2064 Fix inability to sign and send transactions via the LCD by + loading a Ledger device at runtime. * Tendermint diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index 5f6bcb1ad762..b295701792dd 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -59,7 +59,7 @@ func NewPrivKeyLedgerSecp256k1(path DerivationPath) (tmcrypto.PrivKey, error) { ledgerDevice = device } else { - return nil, errors.New("no ledger discovery function defined") + return nil, errors.New("no Ledger discovery function defined") } pkl := &PrivKeyLedgerSecp256k1{Path: path, ledger: ledgerDevice} From de061fa8e195f41b7669556761fc26cea00c40f7 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 27 Aug 2018 18:57:06 -0400 Subject: [PATCH 3/5] Do not export ledger discovery types --- crypto/ledger.go | 4 ++-- crypto/ledger_secp256k1.go | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crypto/ledger.go b/crypto/ledger.go index 5489810bca59..346d4bfb32a2 100644 --- a/crypto/ledger.go +++ b/crypto/ledger.go @@ -6,11 +6,11 @@ import ( ledger "github.com/zondax/ledger-goclient" ) -// If ledger support (build tag) has been enabled, set the DiscoverLedger +// If ledger support (build tag) has been enabled, set the discoverLedger // function which is responsible for loading the Ledger device at runtime or // returning an error. func init() { - DiscoverLedger = func() (LedgerSECP256K1, error) { + discoverLedger = func() (LedgerSECP256K1, error) { device, err := ledger.FindLedger() if err != nil { return nil, err diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index b295701792dd..c59f8e3b4a62 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -11,15 +11,15 @@ import ( ) var ( - // DiscoverLedger defines a function to be invoked at runtime for discovering + // discoverLedger defines a function to be invoked at runtime for discovering // a connected Ledger device. - DiscoverLedger DiscoverLedgerFn + discoverLedger discoverLedgerFn ) type ( - // DiscoverLedgerFn defines a Ledger discovery function that returns a + // discoverLedgerFn defines a Ledger discovery function that returns a // connected device or an error upon failure. - DiscoverLedgerFn func() (LedgerSECP256K1, error) + discoverLedgerFn func() (LedgerSECP256K1, error) // DerivationPath represents a Ledger derivation path. DerivationPath []uint32 @@ -51,8 +51,8 @@ type ( func NewPrivKeyLedgerSecp256k1(path DerivationPath) (tmcrypto.PrivKey, error) { var ledgerDevice LedgerSECP256K1 - if DiscoverLedger != nil { - device, err := DiscoverLedger() + if discoverLedger != nil { + device, err := discoverLedger() if err != nil { return nil, errors.Wrap(err, "failed to create PrivKeyLedgerSecp256k1") } From 1a6c4785e9b824a2f229050eb3644d951fb03716 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 28 Aug 2018 08:16:50 -0400 Subject: [PATCH 4/5] Cleanup ledger godocs and reduce complexity --- crypto/ledger.go | 6 +++--- crypto/ledger_secp256k1.go | 18 +++++++----------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/crypto/ledger.go b/crypto/ledger.go index 346d4bfb32a2..b9aa65b56f33 100644 --- a/crypto/ledger.go +++ b/crypto/ledger.go @@ -6,9 +6,9 @@ import ( ledger "github.com/zondax/ledger-goclient" ) -// If ledger support (build tag) has been enabled, set the discoverLedger -// function which is responsible for loading the Ledger device at runtime or -// returning an error. +// If ledger support (build tag) has been enabled, which implies a CGO dependency, +// set the discoverLedger function which is responsible for loading the Ledger +// device at runtime or returning an error. func init() { discoverLedger = func() (LedgerSECP256K1, error) { device, err := ledger.FindLedger() diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index c59f8e3b4a62..49f110b3d7aa 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -49,20 +49,16 @@ type ( // CONTRACT: The ledger device, ledgerDevice, must be loaded and set prior to // any creation of a PrivKeyLedgerSecp256k1. func NewPrivKeyLedgerSecp256k1(path DerivationPath) (tmcrypto.PrivKey, error) { - var ledgerDevice LedgerSECP256K1 - - if discoverLedger != nil { - device, err := discoverLedger() - if err != nil { - return nil, errors.Wrap(err, "failed to create PrivKeyLedgerSecp256k1") - } - - ledgerDevice = device - } else { + if discoverLedger == nil { return nil, errors.New("no Ledger discovery function defined") } - pkl := &PrivKeyLedgerSecp256k1{Path: path, ledger: ledgerDevice} + device, err := discoverLedger() + if err != nil { + return nil, errors.Wrap(err, "failed to create PrivKeyLedgerSecp256k1") + } + + pkl := &PrivKeyLedgerSecp256k1{Path: path, ledger: device} pubKey, err := pkl.getPubKey() if err != nil { From bdbdd1a8323f14b27b4d5d6f5cf879282a326658 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 28 Aug 2018 08:24:19 -0400 Subject: [PATCH 5/5] Further godoc updates --- crypto/ledger_secp256k1.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index 49f110b3d7aa..ff05d31bae66 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -18,7 +18,8 @@ var ( type ( // discoverLedgerFn defines a Ledger discovery function that returns a - // connected device or an error upon failure. + // connected device or an error upon failure. Its allows a method to avoid CGO + // dependencies when Ledger support is potentially not enabled. discoverLedgerFn func() (LedgerSECP256K1, error) // DerivationPath represents a Ledger derivation path.