Add Lisk support to trezorctl

[alepop]

Messages: trezor/trezor-common#68

Progress:

• Lisk get address
• Lisk get public key
• Lisk sign message
• Lisk verify message
• Lisk sign tx
  • send transaction
  • signature ransaction
  • delegate transaction
  • vote transaction
  • multisignature transaction
    dApp tx types wil not allowed in the Lisk 1.0 release
    dapp tansaction
    dapp In transfer transaction
    dapp out transfer transaction

[prusnak]

@matejcik I merged in trezor-common changes for Lisk messages in trezor/trezor-common@6a7710c

The Protobuf API is VERY similar to the Bitcoin API, so trezorctl/client changes should be also very similar.

Can you please review this PR to python-trezor?

Not sure about auto-generated messages in trezorlib/messages do we still want to generate and commit them into git? Shouldn't they be generated when needed, so a simple bump of vendor/trezor-common will do the trick?

One final note: Lisk is implemented only for T2 (in trezor-core), so tests should include skip_t1.

[alepop]

@prusnak skip_t1added

[matejcik]

@prusnak the "generated messages in git" are to support people who don't have (the appropriate version of) protoc -- of course you need that to contribute the regenerated messages, but users who checkout the repository or use the sdist from PyPI don't need it. Not to mention Windows where you'd need to run a shell script.
Given that it's a single-command step for contributors, and that you can regenerate messages and bump trezor-common in one commit (so no churn), to me it seems a reasonable tradeoff.

[prusnak]

to me it seems a reasonable tradeoff

Agreed!

[matejcik]

mostly LGTM, apart from one nitpick.

[matejcik]

please change this to return isinstance(...)
(yes, all other verify_message functions have this. that will be changed, let's not put it into new code)

[alepop]

@matejcik done

[matejcik]

the code looks mostly good.

however what I see as a problem is that several clearly numeric fields are declared as strings?
that doesn't seem right - unless that's intentional (Lisk docs seem to suggest that it isn't), it should be fixed. in trezor-common first and then in the other PRs

[prusnak]

Please wait with the merge until we resolve int/string discussion in trezor/trezor-core#90

[matejcik]

@alepop when the int conversion goes through, please rebase this PR onto current master, update the trezor-common submodule and regenerate the protobuf messages. thanks.

[alepop]

@matejcik done.

[matejcik]

looks good, thanks