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.

Sign up for GitHub

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

Followup suggestions from ADR-027 updates (#7232) #7379

Merged
merged 4 commits into from
Sep 28, 2020
Merged

Followup suggestions from ADR-027 updates (#7232) #7379

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0de710f
add grammar suggestions to adr-027
clevinson Sep 23, 2020
6193d45
Update docs/architecture/adr-027-deterministic-protobuf-serialization.md
clevinson Sep 25, 2020
715b18c
Merge branch 'master' into clevinson/adr-027-grammar
clevinson Sep 25, 2020
ef6dbb0
Merge branch 'master' into clevinson/adr-027-grammar
mergify[bot] Sep 28, 2020
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
14 changes: 7 additions & 7 deletions docs/architecture/adr-027-deterministic-protobuf-serialization.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,20 @@ Proposed
## Abstract

Fully deterministic structure serialization, which works across many languages and clients,
is needed for structure signature use-case. We need to be sure that whenever we serialize
is needed for the use case of structure signatures. We need to be sure that whenever we serialize
Copy link
Member

Choose a reason for hiding this comment

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

What are structure signatures. That's really not very clear

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe is needed for generating a signature payload. ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

How about is needed to sign a structure.?

Copy link
Contributor

@amaury1093 amaury1093 Sep 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

My reasoning was, Fully deterministic structure serialization is needed... to sign a struct, an array, a string, an integer, any kind of signature payload. I'm fine either way, I'll let you push a commit Robert, and we can merge this.

Copy link
Member

Choose a reason for hiding this comment

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

Message is the protobuf term.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to "needed when signing messages". I hope that is more clear.

clevinson marked this conversation as resolved.
Show resolved Hide resolved
a data structure, no matter in which supported language, the raw bytes
will stay the same.
[Protobuf](https://developers.google.com/protocol-buffers/docs/proto3)
serialization is not bijective (i.e. there exist a practically unlimited number of
valid binary representations for a protobuf document)<sup>1</sup>.
valid binary representations for a given protobuf document)<sup>1</sup>.

This document describes a deterministic serialization scheme for
a subset of protobuf documents, that covers this use case but can be reused in
other cases as well.

### Context

For signature verification in Cosmos SDK, signer and verifier need to agree on
For signature verification in Cosmos SDK, the signer and verifier need to agree on
the same serialization of a `SignDoc` as defined in
[ADR-020](./adr-020-protobuf-transaction-encoding.md) without transmitting the
serialization.
Expand All @@ -38,8 +38,8 @@ step when sending and signing transactions.

### Decision

The following encoding scheme is proposed to be used by other ADRs.
Currently we are using the this ADR to for `SignDoc` serialization.
The following encoding scheme is to be used by other ADRs,
and in particular for `SignDoc` serialization.

## Specification

Expand Down Expand Up @@ -271,7 +271,7 @@ for all protobuf documents we need in the context of Cosmos SDK signing.
- The need for rule number 3. adds some complexity to implementations.
- Some data structures may require custom code for serialization. Thus
the code is not very portable - it will require additional work for each
client implementing serialization for not compatible data structures.
client implementing serialization to properly handle custom data structures.

### Neutral

Expand All @@ -281,7 +281,7 @@ for all protobuf documents we need in the context of Cosmos SDK signing.
For the reasons mentioned above ("Negative" section) we prefer to keep workarounds
for shared data structure. Example: the aforementioned `TxRaw` is using raw bytes
as a workaround. This allows them to use any valid Protobuf library without
a need of implementing this standard (and related risks of bugs).
the need of implementing a custom serializer that adheres to this standard (and related risks of bugs).

## References

Expand Down