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

ADR 028: Public Key Addresses #7086

Merged
merged 18 commits into from
Oct 21, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 40 additions & 12 deletions docs/architecture/adr-028-public-key-addresses.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,43 @@ The current multisig public keys use amino serialization to generate the address
those public keys and their address formatting, and call them "legacy amino" multisig public keys
in protobuf. We will also create multisig public keys without amino addresses to be described below.

### Canonical Public Key Addresses

Following on the discussion in [\#5694](https://github.com/cosmos/cosmos-sdk/issues/5694), we propose the
following approach.
### Canonical Address Format

We have three types of accounts we would like to create addresses for in the future:
- regular public key addresses for new signature algorithms (ex. `sr25519`).
- public key addresses for multisig public keys that don't use amino encoding
- module accounts: basically any accounts which cannot sign transactions and
which are managed internally by modules

To address all of these use cases we propose the following basic `AddressHash` function,
based on the discussions in [\#5694](https://github.com/cosmos/cosmos-sdk/issues/5694):

```go
func AddressHash(prefix string, contents []byte) []byte {
preImage := []byte(prefix)
if len(contents) != 0 {
preImage = append(preImage, 0)
preImage = append(preImage, contents...)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When implementing this we should use use make([]byte, correctSize) and copy they content using for or copy (into slice starting from index 2).

return blake2b.Sum256(preImage)[:20]
}
```

`AddressHash` always take a string `prefix` as a starting point which should represent the
type of public key (ex. `sr25519`) or module account being used (ex. `staking` or `group`).
For public keys, the `contents` parameter is used to specify the binary contents of the public
key. For module accounts, `contents` can be left empty (for modules which don't manage "sub-accounts"),
or can be some module-specific content to specify different pools (ex. `bonded` or `not-bonded` for `staking`)
or managed accounts (ex. different accounts managed by the `group` module).

In the `preImage`, the byte value `0` is used as the separator between `prefix` and `contents`. This is a logical
choice given that `0` is an invalid value for a string character and is commonly used as a null terminator.

We use a 256-bit `blake2b` hash instead of `sha256` because it is generally considered more secure. Blake hashes
are considered "random oracle indifferentiable", a stronger property which `sha256` does not have.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Random oracle indifferentiability is cryptographically irrelevant for constructing addresses.

https://blog.cryptographyengineering.com/2012/07/17/indifferentiability/

Cause see the prefix of an address always specifies the length of suffix. Ie. sr25519 key is always 32bytes. In the context of this protocol, a MD hash like sha256 is "random oracle indeffentiable"

I'm just saying this because I'm always going to strongly advocate for a really high bar for adding a new hash function to the Cosmos trusted computing base.

Copy link
Collaborator

@robert-zaremba robert-zaremba Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zmanian I agree with your general approach. But we don't propose a any exotic algorithm. Blake2 is a part of at least Go and Python3 stdlib and it's an improvement. Some cryptographers claim that sha256 is easier to break than blake2 because we already know how to break other hash functions from the MD family.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some cryptographers claim that sha256 is easier to break than blake2.

Some references would be appreciated 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just saying this because I'm always going to strongly advocate for a really high bar for adding a new hash function to the Cosmos trusted computing base.

So I want to summarize what I heard from @zmanian on our last call about this. Basically:

  • there is no consensus on the replacement for SHA256 yet in the crypto community and there's a widespread sense that it won't be broken anytime soon
  • we want to minimize what is needed for someone to implement "cosmos" on a broad array of devices including hardware wallets and embedded devices

Thus for now it's probably preferable to stick with SHA256. Is that accurate @zmanian ?

@alessio any progress on getting the cryptographer at AiB to take a look at this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W3F Research / Polkadot is using blake2b and avoiding sha2.


### Canonical Public Key Address Prefixes

All public key types will have a unique protobuf message type such as:
aaronc marked this conversation as resolved.
Show resolved Hide resolved

Expand All @@ -70,14 +103,10 @@ All protobuf messages have unique fully qualified names, in this example `cosmos
These names are derived directly from .proto files in a standardized way and used
in other places such as the type URL in `Any`s. Since there is an easy and obvious
way to get this name for every protobuf type, we can use this message name as the
key type prefix when creating addresses.

We define the canonical address format for new (non-legacy) public keys as
`Sha256(fmt.Sprintf("%s/%x, proto.MessageName(key), key.Bytes()))[:20]`. This takes
the first 20 bytes of an SHA-256 hash of a string with the proto message name for the key
type joined by an `/` with the hex encoding of the key bytes.
key type `prefix` when creating addresses. For all basic public keys, `contents`
should just be the raw unencoded public key bytes.

For all basic public keys, key bytes should just be the raw unencoded public key bytes.
Thus the canonical address for new public key types would be `AddressHash(proto.MessageName(pk), pk.Bytes)`.

### Multisig Addresses
aaronc marked this conversation as resolved.
Show resolved Hide resolved

Expand All @@ -104,13 +133,12 @@ using `Sha256(fmt.Sprintf("cosmos.crypto.multisig.PubKey/%d/%s", pk.Threshold, j
## Consequences

### Positive
- a simple algorithm for generating addresses for new public keys
- a simple algorithm for generating addresses for new public keys and module accounts

### Negative
- addresses do not communicate key type, a prefixed approach would have done this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be helpful to try and put this info in bech32 encodings? Is access to this info really that important?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know. You're saying that maybe we can have the bech32 have an extra prefix that isn't present in the actual address? Would this be something like cosmossecp256k1sdgh3sghlsdsdg. Or would the prefix get added before bech32 encoding?


### Neutral
- protobuf message names are used as key type prefixes
- public key bytes are hex encoded before generating addresses

## References
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,6 @@ github.com/tendermint/go-amino v0.16.0 h1:GyhmgQKvqF82e2oZeuMSp9JTN0N09emoSZlb2l
github.com/tendermint/go-amino v0.16.0/go.mod h1:TQU0M1i/ImAo+tYpZi73AU3V/dKeCoMC9Sphe2ZwGME=
github.com/tendermint/tendermint v0.34.0-rc3 h1:d7Fsd5rdbxq4GmJ0kRfx7l7LesQM7e70f0ytWLTQ/Go=
github.com/tendermint/tendermint v0.34.0-rc3/go.mod h1:BoHcEpjfpBHc1Be7RQz3AHaXFNObcDG7SNHCev6Or4g=
github.com/tendermint/tendermint v0.34.0-rc4 h1:fnPyDFz9QGAU6tjExoQ8ZY63eHkzdBg5StQgDoeuK0s=
github.com/tendermint/tendermint v0.34.0-rc4/go.mod h1:yotsojf2C1QBOw4dZrTcxbyxmPUrT4hNuOQWX9XUwB4=
github.com/tendermint/tendermint v0.34.0-rc4.0.20201005135527-d7d0ffea13c6 h1:gqZ0WDpDYgMm/iaiMEXvI1nt/GoWCuwtBomVpUMiAIs=
github.com/tendermint/tendermint v0.34.0-rc4.0.20201005135527-d7d0ffea13c6/go.mod h1:BSXqR6vWbOecet726v66qVwSkFDLfEeBrq+EhkKbij4=
github.com/tendermint/tm-db v0.6.1 h1:w3X87itMPXopcRPlFiqspEKhw4FXihPk2rnFFkP0zGk=
Expand Down