-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Improve signature collection UX #6174
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,33 +54,54 @@ package cosmos_sdk.v1; | |
|
||
message Tx { | ||
TxBody body = 1; | ||
repeated bytes signatures = 2; | ||
AuthInfo auth_info = 2; | ||
repeated bytes signatures = 3; | ||
} | ||
|
||
message TxBody { | ||
repeated google.protobuf.Any messages = 1; | ||
repeated SignerInfo signer_info = 2; | ||
Fee fee = 3; | ||
string memo = 4; | ||
int64 timeout_height = 5; | ||
repeated google.protobuf.Any extension_options = 6; | ||
string memo = 2; | ||
int64 timeout_height = 3; | ||
repeated google.protobuf.Any extension_options = 1023; | ||
} | ||
|
||
message AuthInfo { | ||
// The provided SignerInfo's may occur in any order and may include signers | ||
// that are not explicitly referenced in `TxBody.messages`. These other | ||
// signers could simply be there to pay fees or could be utilized by | ||
// message handlers that examine the full list of signers on the transaction | ||
// context | ||
repeated SignerInfo signer_infos = 1; | ||
// The first signer is the primary signer and the one which pays the fee | ||
Fee fee = 2; | ||
} | ||
|
||
message SignerInfo { | ||
PublicKey pub_key = 1; | ||
SignMode mode = 2; | ||
oneof identity { | ||
// public_key must be set the first time an account makes a transaction | ||
PublicKey public_key = 1; | ||
// address can be used for accounts that already have a public key in state | ||
bytes address = 2; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is not part of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is needed for both signing (by the primary signer) and tx broadcasting. Allowing the option to use address can reduce the tx size which could be significant if using a multisig pubkey. |
||
SignMode mode = 3; | ||
} | ||
|
||
enum SignMode { | ||
SIGN_MODE_DEFAULT = 0; | ||
SIGN_MODE_LEGACY_AMINO = -1; | ||
SIGN_MODE_EXTENDED = 1; | ||
SIGN_MODE_UNSPECIFIED = 0; | ||
|
||
SIGN_MODE_DIRECT = 1; | ||
SIGN_MODE_DIRECT_AUX = 2; | ||
|
||
SIGN_MODE_TEXTUAL = 3; | ||
SIGN_MODE_TEXTUAL_AUX = 4; | ||
|
||
SIGN_MODE_LEGACY_AMINO_JSON = 127; | ||
} | ||
``` | ||
|
||
As will be discussed below, in order to include as much of the `Tx` as possible | ||
in the `SignDoc`, `SignerInfo` is separated from signatures so that only the | ||
raw signatures themselves live outside of `TxBody`. | ||
raw signatures themselves live outside of what is signed over. | ||
|
||
Because we are aiming for be a flexible, extensible cross-chain transaction | ||
format, new transaction processing options should be added to `TxBody` as soon | ||
|
@@ -93,22 +114,19 @@ attempt to upstream important improvements to `Tx`. | |
|
||
### Signing | ||
|
||
Signatures are structured using the `SignDoc` below which reuses `TxBody` and only | ||
adds the fields which are needed for signatures but not present on `TxBody`: | ||
All of the signing modes below aim to provide the following guarantees: | ||
|
||
```proto | ||
// types/types.proto | ||
message SignDoc { | ||
TxBody body = 1; | ||
string chain_id = 2; | ||
uint64 account_number = 3; | ||
uint64 account_sequence = 4; | ||
} | ||
``` | ||
* **No Malleability**: `TxBody` and `AuthInfo` cannot change once the primary | ||
signer has signed | ||
* **Predictable Gas**: if I am signing a transaction where I am paying a fee, | ||
the final gas is fully dependent on what I am signing | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you help me understand this point better: do we (right now and in the future) support an undefined number of signatures? If not, the only purpose of this added complexity is to set different gas prices for different verification modes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I understand it, with multisigs currently it could be unpredictable because the signatures are nested. Also, I am noting this in contrast to weave where my reading is that it's generally unpredictable. |
||
|
||
These guarantees give the maximum amount confidence to message signers that | ||
manipulation of `Tx`s by intermediaries can't result in any meaningful changes. | ||
|
||
#### `SIGN_MODE_DEFAULT` | ||
#### `SIGN_MODE_DIRECT` | ||
|
||
The default signing behavior is to sign the raw `TxBody` bytes as broadcast over | ||
The "direct" signing behavior is to sign the raw `TxBody` bytes as broadcast over | ||
the wire. This has the advantages of: | ||
|
||
* requiring the minimum additional client capabilities beyond a standard protocol | ||
|
@@ -117,18 +135,47 @@ buffers implementation | |
subtle differences between the signing and encoding formats which could | ||
potentially be exploited by an attacker) | ||
|
||
Signatures are structured using the `SignDoc` below which reuses `TxBody` and | ||
`AuthInfo` and only adds the fields which are needed for signatures: | ||
|
||
```proto | ||
// types/types.proto | ||
message SignDoc { | ||
TxBody body = 1; | ||
AuthInfo auth_info = 2; | ||
string chain_id = 3; | ||
uint64 account_number = 4; | ||
// account_sequence starts at 1 rather than 0 to avoid the case where | ||
// the default 0 value must be omitted in protobuf serialization | ||
uint64 account_sequence = 5; | ||
} | ||
``` | ||
|
||
In order to sign in the default mode, clients take the following steps: | ||
|
||
1. Encode `TxBody` | ||
2. Sign `SignDocRaw` | ||
1. Encode `SignDoc`. (The only requirement of the underlying protobuf | ||
implementation is that fields are serialized in order). | ||
2. Sign the encoded `SignDoc` bytes | ||
3. Build and broadcast `Tx`. (The underlying protobuf implementation must encode | ||
`TxBody` and `AuthInfo` with the same binary representation as encoded in | ||
`SignDoc`. If this is a problem for clients, the "raw" types described under | ||
verification can be used for signing as well.) | ||
|
||
The raw encoded `TxBody` bytes are encoded into `SignDocRaw` below so that the | ||
encoded body bytes exactly match the signed body bytes with no need for | ||
["canonicalization"](https://github.com/regen-network/canonical-proto3) of that | ||
message. | ||
Signature verification is based on comparing the raw `TxBody` and `AuthInfo` | ||
bytes encoded in `Tx` not based on any ["canonicalization"](https://github.com/regen-network/canonical-proto3) | ||
algorithm which creates added complexity for clients in addition to preventing | ||
some forms of upgradeability (to be addressed later in this document). | ||
|
||
Signature verifiers should use a special set of "raw" types to perform this | ||
binary signature verification rather than attempting to re-encode protobuf | ||
documents which could result in a different binary encoding: | ||
|
||
```proto | ||
// types/types.proto | ||
message TxRaw { | ||
bytes body_bytes = 1; | ||
repeated bytes signatures = 2; | ||
} | ||
|
||
message SignDocRaw { | ||
bytes body_bytes = 1; | ||
string chain_id = 2; | ||
|
@@ -137,72 +184,106 @@ message SignDocRaw { | |
} | ||
``` | ||
|
||
The only requirements are that the client _must_ encode `SignDocRaw` itself canonically. | ||
This means that: | ||
|
||
* all of the fields must be encoded in order | ||
* default values (i.e. empty/zero values) must be omitted | ||
To verify signatures, follow these steps: | ||
|
||
1. Decode `TxRaw` | ||
2. For each signature copy `body_bytes` from `TxRaw` to `SignDocRaw` and fill | ||
in the other fields for the given signer | ||
3. Encode `SignDocRaw` and verify signatures with these sign bytes | ||
|
||
|
||
#### `SIGN_MODE_DIRECT_AUX` | ||
|
||
If a protobuf implementation does not by default encode `SignDocRaw` canonically, | ||
the client _must_ manually encode `SignDocRaw` following the guidelines above. | ||
`SIGN_MODE_DIRECT_AUX` is used to support scenarios where multiple signatures | ||
are being gathered into a single transaction but the message composer does not | ||
yet know which signatures will be included in the final transaction. For instance, | ||
I may have a 3/5 multisig wallet and want to send a `TxBody` to all 5 | ||
signers to see who signs first. As soon as I have 3 signatures then I will go | ||
ahead and build the full transaction. | ||
|
||
Again, it does not matter if `TxBody` was encoded canonically or not. | ||
With `SIGN_MODE_DIRECT`, each signer needs | ||
to sign the full `AuthInfo` which includes the full list of all signers and | ||
their signing modes, making the above scenario very hard. | ||
|
||
Note that in order to decode `SignDocRaw` for displaying contents to users, | ||
the regular `SignDoc` type should be used. | ||
`SIGN_MODE_DIRECT_AUX` allows "auxiliary" signers to create their signature | ||
using only `TxBody` and their own `PublicKey`. This allows the full list of | ||
signers in `AuthInfo` to be delayed until signatures have been collected. | ||
|
||
3. Broadcast `TxRaw` | ||
An "auxiliary" signer is any signer besides the primary signer who is paying | ||
the fee. For the primary signer, the full `AuthInfo` is actually needed to calculate gas and fees | ||
because that is dependent on how many signers and which key types and signing | ||
modes they are using. Auxiliary signers, however, do not need to worry about | ||
fees or gas and thus can just sign `TxBody`. | ||
|
||
In order to make sure that the signed body bytes exactly match the encoded body | ||
bytes, clients should encode and broadcast `TxRaw` with the same body bytes used | ||
in `SignDocRaw`. | ||
To generate a signature in `SIGN_MODE_DIRECT_AUX` follow these steps: | ||
|
||
1. Encode `SignDocAux` (with the same requirement that fields must be serialized | ||
in order): | ||
|
||
```proto | ||
// types/types.proto | ||
message TxRaw { | ||
message SignDocAux { | ||
bytes body_bytes = 1; | ||
repeated bytes signatures = 2; | ||
// PublicKey is included in SignDocAux : | ||
// 1. as a special case for multisig public keys to be described later | ||
// in this document | ||
// 2. to guard against scenario where configuration information is encoded | ||
// in public keys (it has been proposed) such that two keys can generate | ||
// the same signature but have different security properties | ||
// | ||
// By including it here, the composer of AuthInfo cannot reference the | ||
// a public key variant the signer did not intend to use | ||
PublicKey public_key = 2; | ||
string chain_id = 3; | ||
uint64 account_number = 4; | ||
// account_sequence starts at 1 rather than 0 to avoid the case where | ||
// the default 0 value must be omitted in protobuf serialization | ||
uint64 account_sequence = 5; | ||
} | ||
``` | ||
|
||
Signature verifiers should verify signatures by decoding `TxRaw` and then | ||
encoding `SignDocRaw` with the raw body bytes. | ||
|
||
The standard `Tx` type (which is byte compatible with `TxRaw`) can be used to | ||
decode transactions for all other use cases. | ||
|
||
#### `SIGN_MODE_LEGACY_AMINO` | ||
|
||
In order to support legacy wallets and exchanges, Amino JSON will be emporarily | ||
supported transaction signing. Once wallets and exchanges have had a | ||
chance to upgrade to protobuf based signing, this option will be disabled. In | ||
the meantime, it is foreseen that disabling the current Amino signing would cause | ||
too much breakage to be feasible. | ||
2. Sign the encoded `SignDocAux` bytes | ||
3. Send their signature and `SignerInfo` to primary signer who will then | ||
sign and broadcast the final transaction (with `SIGN_MODE_DIRECT` and `Fee` | ||
added) once enough signatures have been collected | ||
|
||
Legacy clients will be able to sign a transaction using the current Amino | ||
JSON format and have it encoded to protobuf using the REST `/tx/encode` | ||
endpoint before broadcasting. | ||
For signature verification to succeed, the fee field for signers using | ||
`SIGN_MODE_DIRECT_AUX` must be empty. | ||
|
||
#### `SIGN_MODE_EXTENDED` | ||
#### `SIGN_MODE_TEXTUAL` and `SIGN_MODE_TEXTUAL_AUX` | ||
|
||
As was discussed extensively in [\#6078](https://github.com/cosmos/cosmos-sdk/issues/6078), | ||
there is a desire for a human-readable signing encoding, especially for hardware | ||
wallets like the [Ledger](https://www.ledger.com) which display | ||
transaction contents to users before signing. JSON was an attempt at this but | ||
falls short of the ideal. | ||
|
||
`SIGN_MODE_EXTENDED` is intended as a placeholder for a human-readable | ||
`SIGN_MODE_TEXTUAL` and its counterpart `SIGN_MODULE_TEXTAL_AUX` are | ||
intended as placeholders for a human-readable | ||
encoding which will replace Amino JSON. This new encoding should be even more | ||
focused on readability than JSON, possibly based on formatting strings like | ||
[MessageFormat](http://userguide.icu-project.org/formatparse/messages). | ||
|
||
In order to ensure that the new human-readable format does not suffer from | ||
transaction malleability issues, `SIGN_MODE_EXTENDED` | ||
requires that the _human-readable bytes are concatenated with the raw `SignDoc`_ | ||
to generate sign bytes. | ||
transaction malleability issues, `SIGN_MODE_TEXTUAL` and `SIGN_MODE_TEXTUAL_AUX` | ||
require that the _human-readable bytes are concatenated with the raw `SignDoc` | ||
or `SignDocAux`_ to generate sign bytes. | ||
|
||
Multiple human-readable formats (maybe even localized messages) may be supported | ||
by `SIGN_MODE_EXTENDED` when it is implemented. | ||
by `SIGN_MODE_TEXTUAL` when it is implemented. | ||
|
||
|
||
#### `SIGN_MODE_LEGACY_AMINO_JSON` | ||
|
||
In order to support legacy wallets and exchanges, Amino JSON will be emporarily | ||
supported transaction signing. Once wallets and exchanges have had a | ||
chance to upgrade to protobuf based signing, this option will be disabled. In | ||
the meantime, it is foreseen that disabling the current Amino signing would cause | ||
too much breakage to be feasible. | ||
|
||
Legacy clients will be able to sign a transaction using the current Amino | ||
JSON format and have it encoded to protobuf using the REST `/tx/encode` | ||
endpoint before broadcasting. | ||
|
||
### CLI & REST | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you map signatures to the signers' pubkeys when
AuthInfo.signer_infos
is not sorted?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SignerInfo
should be in the same order as signatures. This just proposes that the order of signers does not need to match the SDK's existing requirement about signature ordering which you pointed out could be hard for a client to compute.