search

ecadlabs / taquito

A library for building dApps on the Tezos Blockchain - JavaScript / TypeScript
https://taquito.io
Apache License 2.0
298 stars 116 forks source link

Feedback/Changes to Ledger signer doc #445

Closed tezit closed 4 years ago

tezit commented 4 years ago

The following feedback comes from a review by Klas from the Kukai team regarding the ledger signer document https://tezostaquito.io/docs/ledger_signer/

  1. The doc mentions 3 derivation types, but there are actually 4 (its poorly documented and an example of an area we have been unable to get info from Obsidian)

  2. The derivation types being named tzX is potentially an area of confusion in the future. It's possible that the 4th derivation type will become more widely used in the future (used by the CLI wallet today when paired with ledger), which also uses the tz1 prefix. note: tz1, tz2, tz3 are associated with signature schemes in tezos whereas different derivation schemes/types can use the same signature scheme

  3. Tech spec list should mention and link to "SLIP-10: Derivation Scheme". The important thing to communicate is what standards are being used for derivations. We want to avoid fragmentation in derivation schemes used.

  4. "We can see other implementations that adhere to BIP44 and use 44'/1729'/0'/0'/0'" is technically incorrect because according to BIP44 the last two levels must be unhardened so the quoted path is not actually BIP44 compliant

  5. Having an incomplete example of path scanning can be counter productive. A better example would require an indexer (checking balance and reveal status is not enough). Could be better to illustrate it with a flowchart to add checks for incoming transactions, current balance, previous balance, and less of a priority for now until we see it in practice but also checking for fa1.2 and fa2 tokens assigned to the account.

  6. Doc describes HD as enabling “access to unlimited” and later “an infinity of address” — this is not technically correct as there is a limit to how many can be used, better to use language such as “almost or nearly” unlimited

roxaneletourneau commented 4 years ago

Thank you. I really appreciate your feedback. Good catch for the derivation type, I will improve the naming by replacing tzX with curve name (ED25519, SECP256K1, SECP256R1). I will also make the corrections that you pointed to the documentation.

Innkst commented 4 years ago

All the concerns were addressed except for item 5. #492 was created to review this use case and address it.