Skip to content

Commit

Permalink
accounts, signer: fix Ledger Live account derivation path (clef) (eth…
Browse files Browse the repository at this point in the history 
…ereum#21757)

* signer/core/api: fix derivation of ledger live accounts

For ledger hardware wallets, change account iteration as follows:

- ledger legacy: m/44'/60'/0'/X; for 0<=X<5
- ledger live: m/44'/60'/0'/0/X; for 0<=X<5

- ledger legacy: m/44'/60'/0'/X; for 0<=X<10
- ledger live: m/44'/60'/X'/0/0; for 0<=X<10

Non-ledger derivation is unchanged and remains as:
- non-ledger: m/44'/60'/0'/0/X; for 0<=X<10

* signer/core/api: derive ten default paths for all hardware wallets, plus ten legacy and ten live paths for ledger wallets

* signer/core/api: as .../0'/0/0 already included by default paths, do not include it again with ledger live paths

* accounts, signer: implement path iterators for hd wallets

Co-authored-by: Martin Holst Swende <martin@swende.se>
  • Loading branch information
@svenski123 @holiman
svenski123 and holiman committed Nov 29, 2020
1 parent fa572cd commit b71334a
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 47 deletions.
28 changes: 28 additions & 0 deletions accounts/hd.go
View file
Expand Up @@ -150,3 +150,31 @@ func (path *DerivationPath) UnmarshalJSON(b []byte) error {
*path, err = ParseDerivationPath(dp)
return err
}

// DefaultIterator creates a BIP-32 path iterator, which progresses by increasing the last component:
// i.e. m/44'/60'/0'/0/0, m/44'/60'/0'/0/1, m/44'/60'/0'/0/2, ... m/44'/60'/0'/0/N.
func DefaultIterator(base DerivationPath) func() DerivationPath {
path := make(DerivationPath, len(base))
copy(path[:], base[:])
// Set it back by one, so the first call gives the first result
path[len(path)-1]--
return func() DerivationPath {
path[len(path)-1]++
return path
}
}

// LedgerLiveIterator creates a bip44 path iterator for Ledger Live.
// Ledger Live increments the third component rather than the fifth component
// i.e. m/44'/60'/0'/0/0, m/44'/60'/1'/0/0, m/44'/60'/2'/0/0, ... m/44'/60'/N'/0/0.
func LedgerLiveIterator(base DerivationPath) func() DerivationPath {
path := make(DerivationPath, len(base))
copy(path[:], base[:])
// Set it back by one, so the first call gives the first result
path[2]--
return func() DerivationPath {
// ledgerLivePathIterator iterates on the third component
path[2]++
return path
}
}
39 changes: 39 additions & 0 deletions accounts/hd_test.go
View file
Expand Up @@ -17,6 +17,7 @@
package accounts

import (
"fmt"
"reflect"
"testing"
)
Expand Down Expand Up @@ -77,3 +78,41 @@ func TestHDPathParsing(t *testing.T) {
}
}
}

func testDerive(t *testing.T, next func() DerivationPath, expected []string) {
t.Helper()
for i, want := range expected {
if have := next(); fmt.Sprintf("%v", have) != want {
t.Errorf("step %d, have %v, want %v", i, have, want)
}
}
}

func TestHdPathIteration(t *testing.T) {
testDerive(t, DefaultIterator(DefaultBaseDerivationPath),
[]string{
"m/44'/60'/0'/0/0", "m/44'/60'/0'/0/1",
"m/44'/60'/0'/0/2", "m/44'/60'/0'/0/3",
"m/44'/60'/0'/0/4", "m/44'/60'/0'/0/5",
"m/44'/60'/0'/0/6", "m/44'/60'/0'/0/7",
"m/44'/60'/0'/0/8", "m/44'/60'/0'/0/9",
})

testDerive(t, DefaultIterator(LegacyLedgerBaseDerivationPath),
[]string{
"m/44'/60'/0'/0", "m/44'/60'/0'/1",
"m/44'/60'/0'/2", "m/44'/60'/0'/3",
"m/44'/60'/0'/4", "m/44'/60'/0'/5",
"m/44'/60'/0'/6", "m/44'/60'/0'/7",
"m/44'/60'/0'/8", "m/44'/60'/0'/9",
})

testDerive(t, LedgerLiveIterator(DefaultBaseDerivationPath),
[]string{
"m/44'/60'/0'/0/0", "m/44'/60'/1'/0/0",
"m/44'/60'/2'/0/0", "m/44'/60'/3'/0/0",
"m/44'/60'/4'/0/0", "m/44'/60'/5'/0/0",
"m/44'/60'/6'/0/0", "m/44'/60'/7'/0/0",
"m/44'/60'/8'/0/0", "m/44'/60'/9'/0/0",
})
}
97 changes: 50 additions & 47 deletions signer/core/api.go
View file
Expand Up @@ -322,62 +322,65 @@ func (api *SignerAPI) openTrezor(url accounts.URL) {

// startUSBListener starts a listener for USB events, for hardware wallet interaction
func (api *SignerAPI) startUSBListener() {
events := make(chan accounts.WalletEvent, 16)
eventCh := make(chan accounts.WalletEvent, 16)
am := api.am
am.Subscribe(events)
go func() {
am.Subscribe(eventCh)
// Open any wallets already attached
for _, wallet := range am.Wallets() {
if err := wallet.Open(""); err != nil {
log.Warn("Failed to open wallet", "url", wallet.URL(), "err", err)
if err == usbwallet.ErrTrezorPINNeeded {
go api.openTrezor(wallet.URL())
}
}
}
go api.derivationLoop(eventCh)
}

// Open any wallets already attached
for _, wallet := range am.Wallets() {
if err := wallet.Open(""); err != nil {
log.Warn("Failed to open wallet", "url", wallet.URL(), "err", err)
// derivationLoop listens for wallet events
func (api *SignerAPI) derivationLoop(events chan accounts.WalletEvent) {
// Listen for wallet event till termination
for event := range events {
switch event.Kind {
case accounts.WalletArrived:
if err := event.Wallet.Open(""); err != nil {
log.Warn("New wallet appeared, failed to open", "url", event.Wallet.URL(), "err", err)
if err == usbwallet.ErrTrezorPINNeeded {
go api.openTrezor(wallet.URL())
go api.openTrezor(event.Wallet.URL())
}
}
}
// Listen for wallet event till termination
for event := range events {
switch event.Kind {
case accounts.WalletArrived:
if err := event.Wallet.Open(""); err != nil {
log.Warn("New wallet appeared, failed to open", "url", event.Wallet.URL(), "err", err)
if err == usbwallet.ErrTrezorPINNeeded {
go api.openTrezor(event.Wallet.URL())
case accounts.WalletOpened:
status, _ := event.Wallet.Status()
log.Info("New wallet appeared", "url", event.Wallet.URL(), "status", status)
var derive = func(limit int, next func() accounts.DerivationPath) {
// Derive first N accounts, hardcoded for now
for i := 0; i < limit; i++ {
path := next()
if acc, err := event.Wallet.Derive(path, true); err != nil {
log.Warn("Account derivation failed", "error", err)
} else {
log.Info("Derived account", "address", acc.Address, "path", path)
}
}
case accounts.WalletOpened:
status, _ := event.Wallet.Status()
log.Info("New wallet appeared", "url", event.Wallet.URL(), "status", status)
var derive = func(numToDerive int, base accounts.DerivationPath) {
// Derive first N accounts, hardcoded for now
var nextPath = make(accounts.DerivationPath, len(base))
copy(nextPath[:], base[:])

for i := 0; i < numToDerive; i++ {
acc, err := event.Wallet.Derive(nextPath, true)
if err != nil {
log.Warn("Account derivation failed", "error", err)
} else {
log.Info("Derived account", "address", acc.Address, "path", nextPath)
}
nextPath[len(nextPath)-1]++
}
}
if event.Wallet.URL().Scheme == "ledger" {
log.Info("Deriving ledger default paths")
derive(numberOfAccountsToDerive/2, accounts.DefaultBaseDerivationPath)
log.Info("Deriving ledger legacy paths")
derive(numberOfAccountsToDerive/2, accounts.LegacyLedgerBaseDerivationPath)
} else {
derive(numberOfAccountsToDerive, accounts.DefaultBaseDerivationPath)
}
case accounts.WalletDropped:
log.Info("Old wallet dropped", "url", event.Wallet.URL())
event.Wallet.Close()
}
log.Info("Deriving default paths")
derive(numberOfAccountsToDerive, accounts.DefaultIterator(accounts.DefaultBaseDerivationPath))
if event.Wallet.URL().Scheme == "ledger" {
log.Info("Deriving ledger legacy paths")
derive(numberOfAccountsToDerive, accounts.DefaultIterator(accounts.LegacyLedgerBaseDerivationPath))
log.Info("Deriving ledger live paths")
// For ledger live, since it's based off the same (DefaultBaseDerivationPath)
// as one we've already used, we need to step it forward one step to avoid
// hitting the same path again
nextFn := accounts.LedgerLiveIterator(accounts.DefaultBaseDerivationPath)
nextFn()
derive(numberOfAccountsToDerive, nextFn)
}
case accounts.WalletDropped:
log.Info("Old wallet dropped", "url", event.Wallet.URL())
event.Wallet.Close()
}
}()
}
}

// List returns the set of wallet this signer manages. Each wallet can contain
Expand Down

0 comments on commit b71334a

Please sign in to comment.