Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Summary of Ledger Nano Transaction Signing #590

Closed
ebuchman opened this issue Jan 28, 2021 · 4 comments
Closed

Summary of Ledger Nano Transaction Signing #590

ebuchman opened this issue Jan 28, 2021 · 4 comments
Assignees

Comments

@ebuchman
Copy link
Member

ebuchman commented Jan 28, 2021

Cosmos has always prided itself on the extensibility of its Ledger Nano App for signing transactions. See praise.

While our Ledger app is still awesome, the situation post Stargate is a bit complicated.

In summary, with the Stargate release:

  • existing message types (send, staking, gov, etc.) work as normal with the existing Ledger app
  • new IBC message type MsgTransfer works with the existing Ledger app, allowing users to send tokens across chains
  • no other IBC messages work with the existing Ledger app

Read on for details

Current Ledger App, JSON Signing

The Cosmos-SDK (and all chains built with) used JSON for the actual message data to be signed. This makes them human readable, and allows a general purpose signing application. Since the CosmosSDK allows chains to define arbitrary transaction messages, a general purpose Ledger app for parsing and displaying JSON was developed. It just shows the user the JSON blob containing all fields of the tx, and signs it on approval.

However, the JSON format was tied to our old Amino serialization format. We refer to it as Amino JSON. With the Stargate release, we have gutted Amino from the codebase, replacing it with Protobuf. But Protobuf JSON is not very well defined, and because transaction signing is a critical aspect of the ecosystem we did not want to break, we worked to preserve Amino JSON signing of messages. This means the existing Ledger Nano app works as is with Stargate.

New IBC Messages - Limited Support

However, Stargate also includes some new transaction message types, namely for IBC. There are many (~20). Most of these transaction types are never sent by end-users - they are used by relayers to setup IBC channels and relay messages between chains. Since they are new for Stargate, we did not add support for the old Amino JSON signing messages for IBC. This means IBC messages cannot be signed with Ledger Nanos.

The only exception is the ICS20 MsgTransfer. This is the one IBC message (so far) that is user facing - it's how users actually signal their intent to transfer tokens from one chain to another. In the future, there will be other user-facing IBC messages. Amino JSON support was added for MsgTransfer to ensure that these messages could be signed by existing Ledger apps for Stargate.

Do we care if IBC messages can be signed by Ledger Nanos? Yes and No. Ideally, there's no reason they shouldn't be. However, there isn't necessarily a strong use case, since these messages are mostly not user facing and thus have no need for the human-in-the-loop design of Ledger Nano. We will want hardware support for them, but this will likely be done through the TMKMS and YubiHSM - see #577.

Is there any use case for these other IBC messages to be signed by Ledger Nanos? Probably, but much less prominently. Especially for "Solo Machines" (basically accounts pretending to be blockchains so they can communicate over IBC). Solo machine UX still needs alot of love - #575. But they will not have Ledger support for Amino JSON any time soon (if ever, but see below).

We looked into adding Amino JSON support to all the IBC messages. There are some non-trivial complications buried in the protobuf dependencies - see #568 (comment)

Future of Cosmos Transaction Signing

Ultimately, the goal is to move away from the legacy Amino JSON format all together to new "Sign Modes", described in CosmosSDK ADR20. There are currently only two SignModes supported in the SDK: SIGN_MODE_DIRECT and SIGN_MODE_LEGACY_AMINO.

We have so far been talking about SIGN_MODE_LEGACY_AMINO, which is supported by the existing Ledger Nano.

SIGN_MODE_DIRECT is the new, protobuf native signing mode. But the bytes signed are not human readable. These bytes are a direct protobuf encoding of the message. So a Ledger app would have to deserialize the protobuf bytes to present them to users. However, protobuf is not self describing, in the sense that, unlike JSON, serialized protobuf does not include the field names/keys, just the values. Getting the field names requires having access to the protobuf schemas, which are chain specific. This makes it hard to develop a general purpose Ledger app for SIGN_MODE_DIRECT, since it will either (1) need all schemas for all chains (impossible), (2) require a new Ledger app for each chain (and thus no longer be generic, and require significant overhead for each new cosmos SDK chain), or (3) require the protobuf schema be passed along to the Ledger app along with the sign bytes (possible in principle I think but a bit clumsy).

In principle, we could roll out a Ledger app with SIGN_MODE_DIRECT that just shows the Protobuf field values, without the field names. This might be a useful stop gap, but with significant UX deficiencies. But it would automatically support all message types, including all IBC messages.

The ultimate solution is to adopt a new signing mode, called SIGN_MODE_TEXTUAL, as mentioned in the ADR and tracked in cosmos/cosmos-sdk#6513.

Ultimately, we need to specify and implement the SIGN_MODE_TEXTUAL scheme, and then update the Ledger app to support it. If you are interested in this or have input, please see the issue cosmos/cosmos-sdk#6513.

Decisions to Make

  1. Do we update the Ledger app to support SIGN_MODE_DIRECT? Presumably we would take either option (2) of just showing the field values and not the names, or option (3) of passing the protobuf schema. It's not clear to me how difficult this protobuf deserializing would be on a Ledger device.

  2. Finalize the specification for SIGN_MODE_TEXTUAL and implement it in the CosmosSDK and add support to the Ledgers.

Given we ultimately want SIGN_MODE_TEXTUAL, and supporting SIGN_MODE_DIRECT may be a lot of work, I suspect we will just go straight to SIGN_MODE_TEXTUAL. Again, see cosmos/cosmos-sdk#6513

@jleni
Copy link
Member

jleni commented Jan 28, 2021

Awesome summary!
I think we need something similar about derivation paths. At the moment, some projects share 44'/118' while others decided to fork&hack. There are multiple proposed solutions but a standard and formal ADR would be ideal for the community.

@webmaster128
Copy link
Member

I think we need something similar about derivation paths. At the moment, some projects share 44'/118' while others decided to fork&hack. There are multiple proposed solutions but a standard and formal ADR would be ideal for the community.

I'm coordinating with @okwme these days to propose a solution and write a detailed spec for that particular matter.

@webmaster128
Copy link
Member

I think we need something similar about derivation paths. At the moment, some projects share 44'/118' while others decided to fork&hack. There are multiple proposed solutions but a standard and formal ADR would be ideal for the community.

This is discussed in detail in cosmos/cips#11, where a solution is proposed as well. Happy to hear any thoughts.

@mmulji-ic
Copy link
Contributor

Closed in lieu of cosmos/cosmos-sdk#6513

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants