[go: up one dir, main page]

Fix many issues (incl. perfs) while improving ledger.ml #Refactor

This MR is mostly about cleaning-up and using the module system to properly encode the invariants that have been accumulating in weird places across ledger.ml since even before there was a Baking app.

Among other things:

  • Make sure there is now only one way of creating the crouching-tiger name of the ledger.
    • I don't know why that one was initially chosen (public-key-hash corresponding to the /ed25519/ empty path), but other (quite confusing) ones were still computed and logged (with log_info so not by default).
    • Moreover, the corresponding hash was picked with a List.hd (List.rev <result of some fold over list of curves>) → that was very confusing for the code-reader.
  • Handle theClic parameter uniformly, whether it is ledger://... or a key-alias from the wallet.
  • Enforce that all paths are hardened and throw an error if you try to use a non-hardened path. This will un-confuse users who try to call tezos-client from a shell without proper escaping of the URI: if the single-quotes are eaten by bash the bip32-path will not be silently accepted (cf. complaints in the baking-slack).
    • The previous version also happened to be accepting any ASCII character instead of the hardening apostrophe :)
    • Future versions of the ledger apps may allow non-hardened paths, but they'll most likely be in a different /<curve>/ namespace, and we are not there yet.
  • Add CLI alias for correct spelling: get/set ledger high water mark: the watermark spelling is still there but with a deprecation note in the description.
  • Clean-up NULL byte from list connected ledgers output: the contents of the git-describe that are sent by the ledger contain the \x00 from the C-String and tezos-client was just outputting it.
  • Debug logging of ledger-APDU communication is made more “real-time” so if the ledger or tezos-client crash in the middle of an exchange the debug-logs (which include the hex-dumps of the APDUs) are not missing.
  • Fix the Client_keys.SIGNER.description which was very outdated.
  • Get rid of backwards compatibility code for Ledger apps < 1.4.0.

This also improves performance of most ledger interactions by not asking over and over the same thing or useless things (public keys), for instance on my laptop:

  • list connected ledgers: 1.2 → 0.3 seconds.
  • show ledger ...: 2.8 → 0.8 seconds.

The operations of the implementation of Client_keys.SIGNER should be a bit faster too and hence less Lwt-unfriendly; which hopefully means that daemons using the ledger should become slightly more “responsive.”

Merge request reports

Loading