Hey @ethanfrey, short of fixing up all the signatures to return (crypto.PrivKey, error),
here is a hack that I think could help:
a) instead of making genLegder a GenFunc, make it a struct that can save state i.e the last error
e.g

type genLedger struct {
   mu sync.Mutex
   lastErr error
}

b) On encountering any error, instead of panicking, save that lastErr e.g gl.lastErr = err then return normally
c) Make genLedger implement the LastErrorer LastError() error interface which evicts the last error on every read. This ensures that we capture the last encountered error only once, sort of as if it were a function call. Thus

func (gl *genLedger) LastError() error {
   gl.mu.Lock()
   var err error = gl.lastErr
   gl.lastErr = nil
   gl.mu.Unlock()
   return err
}

func (gl *genLedger) setLastErr(err error) {
   gl.mu.Lock()
   gl.lastErr = err
   gl.mu.Unlock()
}

func (gl *genLedger) Generate(secret []byte) crypto.PrivKey {
   key, err := nano.NewPrivKeyLedger()
   if err != nil {
       gl.setLastErr(err)
   }
   return key
}

type LastErrorer interface {
   LastError() error
}

and then for usage:

key := gen.Generate()
if le, ok := gen.(LastErrorer); ok {
   if err := le.LastError(); err != nil {
     // Handle the error here
   }
}

or you can make that a helper inside the code that needs it

func generateKey(gen Generator) (crypto.PrivKey, error) {
   key := gen.Generate()
   le, ok := gen.(LastErrorer)
   if !ok {
      return key, nil
   }
   err := le.LastError()
   return key, err
}

and then use it normally in any of the usages as

key, err := generateKey(gen)

which will prevent mortally breaking this repo. With the mentioned pattern, we can slowly refactor all our code and eventually make all usages return (crypto.PrivKey, error) eventually achieving the proper signature.

Please let me know what you think.

What if instead of exposing this thing as a crypto.PrivKey we abstract the Signer, and give it a Sign( ...) (crypto.Signature, error). Would this solve the problem ? Then the Signer could have additional rules that might cause it to err too. The consumers can just check for errors on calling Sign. Everything else should work fine, and constructors should populate pubkeys and return errors if they can't.

For instance this is how the PrivValidator works in Tendermint, via a Signer interface with a Sign function. And here's the commit where I just added the error: tendermint/tendermint@90c0267

Thank you both for the ideas. I will take a deeper look at the code you both suggested on Monday. But happy to see some solutions.

I fixed up the Generator signature to allow for errors when creating new keys in the key manager. I also extended PrivKeyFromBytes to allow a PrivKey to validate its correctness upon loading, as an optional check. Only done by nano by pinging the app, but ed25519 could make sure the priv/pub key parts match for example, as we serialize them both.

Now errors are returned, not panics, in the key manager upon creating a key on the ledger or signing with the ledger, if it is not plugged in with the cosmos app running.