Full Proto Encoding

[alexanderbez]

Summary

Encoding in the SDK is currently handled by the go-amino wire codec. The implementation and use of this encoding protocol has introduced several major concerns over time, with the two biggest being performance and cross-client/language support.

The context and discussions had over amino are dense, so the above is solely meant to be a high-level primer.

Problem Definition

In order to facilitate easier integration for clients and drastically improve the usability of the SDK and its performance, alternative encoding methods need to be explored.

Proposal

As it stands, the proposals are as follows:

1. Update go-amino to be fully proto3 compatible. There is already a PR that attempts to achieve this.
   a. This is probably the path of least resistance but doesn't actually solve any of our problems directly and seems like a fragile attempt that will require handling corner-cases increasing the surface area for bugs and poor UX support.
2. Remove go-amino entirely in favor of gogo-proto.
   a. This seems to be the most viable and promising approach. It will address both performance and client UX concerns.
3. Remove go-amino entirely in favor of capn-proto.
   a. Similar to proposal (2) but with zero-copy and canonicalization features. However, the concern is somewhat of "vendor-lockin" in terms of not having as rich of a multi-client support as pure proto. AFAIK, encoding uses fixed-width integers so this approach could lead to larger encoding size. Compaction is available, but doing so negates the zero-copy feature.
4. Consider another encoding scheme entirely (e.g. flatbuffers).
   a. Too late for this most likely (atm). It would require heavy research and pretty compelling arguments to steer toward this direction. In addition, it would probably take drastically more time than any other proposal.

Accepted Proposal

Proposal (2) will be adopted. However, @jaekwon will continue to develop go-amino with the goal of it being fully wire-compatible with this proposal. The go-amino proposal is tracked in tendermint/go-amino#302.

What does this mean?

• When the amino changes are complete, we shall revisit reincorporating go-amino based mainly on UX and performance.
• Interfaces must adhere to the oneof implementation where the oneof is the only field member and the sum fields must start at 1.
• Modules will define their own messages and types that must be binary serialized and persisted using package name cosmos_sdk.{module}.v1.
• An application-level codec will exist with a single oneof to handle messages.

Roadmap & Actionables

State

• x/staking Migrate x/staking to Protobuf #5600
• x/distribution proto: migrate x/distr to use hybrid codec #5610
• x/crisis proto: migrate x/distr to use hybrid codec #5610
• x/auth Protobuf: x/auth & x/supply #5533
• x/supply Protobuf: x/auth & x/supply #5533
• x/evidence proto: migrate x/evidence to hybridcodec #5634
• x/params proto: migrate params to hybrid codec #5619
• x/upgrade Migrate upgrade module to protobuf #5659
• x/mint proto: migrate x/evidence to hybridcodec #5634
• x/bank proto: migrate bank to hybridcodec #5681
• x/gov
• x/slashing Regen Network/Slashing protobuf #5627
• x/capability Migrate x/capability to Protobuf #5926
• x/ibc x/ibc proto migration #5704 - only pending ibc client migration
• x/params reliance on Amino JSON encoding for parameter serialization
• Pubkeys???

Client

TX CLI/REST

• Transaction Client (CLI & REST) Protobuf Migration #5864
• Simulation Decoder Refactor #5664
• Reconsider Code Usage in x/auth Client #5663

Tutorials

• nameservice
• scavenge

Documentation & Upgrade Guide

• Update Encoding Doc #5647
• Write a blog post for how users should upgrade

Client libs

• cosmos/amino-js (if need be)

@jordansexton @aaronc

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[dshulyak]

There is one concern with using protobufs for blockchain protocols. Protobuf supports arbitrary maps, but serialization of maps is not deterministic. And usually depends on the library and the language, some libraries sort maps before serializing them. But afaik there is no specification or decision for it, and developers must not depend on sorting even if it happens that library provides such feature.

gogo-proto serializes map in the order of iteration, and hence it is almost guaranteed that two consecutive serialization will produce different results. easy to imagine all sort of bugs if someone will misuse it, consider example:

syntax = "proto3";

message Block {
        repeated Tx tx = 1;
}

message Tx {
        map<string, string> OpCodes = 1;
}

package types

import (
	"crypto/ed25519"
	"strconv"
	"testing"
)

func TestNondeterministic(t *testing.T) {
	tx := Tx{
		OpCodes: map[string]string{},
	}
	block := Block{
		Tx: []*Tx{&tx},
	}

	for i := 0; i < 100; i++ {
		ia := strconv.Itoa(i)
		tx.OpCodes[ia] = ia
	}

	data1, _ := block.Marshal()
	data2, _ := block.Marshal()

	pubk, pk, _ := ed25519.GenerateKey(nil)
	sig := ed25519.Sign(pk, data1)

	if !ed25519.Verify(pubk, data2, sig) {
		t.Error("how come?")
	}
}

I know that ethereum 2 client developers opted out from using protobuf because of this reason, but the protocol requirements might be a bit different. not sure if there is a safe workaround, that will make protobuf less error prone for these use cases.

[dshulyak]

i thought initially that the nondeterminism issue is only present if maps are used, but turns out there are more subtle cases https://gist.github.com/kchristidis/39c8b310fd9da43d515c4394c3cd9510

[aaronc]

Thanks for sharing @dshulyak. We are aware of these concerns and gist you linked above. First of all, we are not intending to use maps at all but yes there are still potential issues. Here is my current take on what can be done with protobuf:

1. we define a deterministic protobuf encoding, publish its behavior clelarly, and ensure that the go implementation and clients follow this for signing. Likely this is mostly covered by saying all fields should in order and no empty fields. Gogo proto has a deterministic mode, but I think we should be willing to upstream PR's to gogo proto if it isn't sufficient. I am aware that a generalized canonical encoding is impossible for protobuf because of the presence of unknown fields - so a necessary constraint on this deterministic encoding would be that messages can contain no unknown fields. This is maybe okay since generally transaction processors should reject messages they don't understand.
2. we champion Canonical JSON and insist that clients canonicalize the protobuf generated json for signing purposes. In my mind using JSON provides the most safety - because a textual encoding captures intent the best - with likely less effort for implementors, with a definite performance cost for signature verification. However, if this can be done in CheckTx and the results can be cached maybe it's irrelevant.
3. don't worry about deterministic encoding because tendermint is going to store the raw message bytes anyway and we can just use those for signing. This has the least performance hit and least effort, but least safety for clients.

I would love it if someone else could comment on the relative pros and cons of the above approaches.

If a canonical encoding is really important, I have been arguing that we should use Cap'n Proto where this is defined in the spec. The downsides for doing this now are a) less mature client support in general - some implementations are active, others less so, and b) we would likely need to fork and improve the go implementation - no json support, getters are hard to use. There are however other benefits like zero copy which I think are not completely negated by using a packed encoding. Anyway, if there were really strong argument for going this approach I could be convinced that the benefits outweigh the effort.

For the time-being, my current thinking is that we can make protobuf work with one of the above approaches, but definitely welcome other input.

[alexanderbez]

We don't and will not use maps, so that's not even a concern. Essentially, I second what @aaronc stated.

[dshulyak]

I was mainly worried that cosmos module (non-core) developers will be able to accidentally use encoding features, without full understanding what that can lead to.

[aaronc]

So I actually think even maps wouldn't be an issue if we figure out the correct signature scheme. Sorting map keys canonically would address this. We are currently leaning to approach (2) FYI.

[ebuchman]

Cool stuff!

Why do you say making go-amino proto compatible doesn't solve any problems? It doesn't solve performance, but doesn't it solve cross-client/language support (everyone else can use protobuf libs)?

My main concern with a change like this is the impact on the codebase, in terms of large PRs and API changes. There also does seem to be desire to consider experiments with other encoding formats, and so I would hope this work will proceed in a direction that could facilitate that (so if someone later wanted to try cap n' proto, it would be easier than it is today). I think this means preserving something like a Codec, but making it an interface (it probably always should have been one).

In general, gogo-proto seems like a fine direction. It's what we use in Tendermint for the ABCI and many popular projects use. But in Tendermint we don't use the proto generated types for business logic directly, only for communicating over the ABCI wire. We translate back and forth between the generated types and our Go types. While gogo-proto does have support for custom types, which in theory solve this, I recall this being a source of headaches when I experimented with it. I've found the proto generated types much less ergonomic, and I would consider depending on the output of their generators for your every day types to be a significant risk. So I would be a bit wary, and would probably recommend having distinct types for serialization and business logic and shuffling back and forth as necessary. Granted, shuffling back and forth between proto and native types can be a source of errors, but it can mitigate the dependency risk, and it can be very helpful for enforcing boundaries in the code and preserving APIs into the future in the case there may be changes, so it might be a net good thing.

As for determinism, where can I learn about the gogo-proto deterministic mode? In theory the SDK was always supposed to come with some level of static analysis tooling, since you can already shoot yourself in the foot by doing things like using maps, iteration over which is randomized by Go. Users shouldn't use maps and we should some day have tooling to check for it, both in proto types and in Go types.

As for unknown fields, I would expect this to be covered by a strict protocol versioning policy, such that where an unknown field would cause a problem (ie. it's expected to be in a hash), there would be a protocol version bump, so peers would already know they're incompatible.

In any case, championing deterministic protobuf for use in blockchains and pushing upstream to gogo-proto seem like valiant causes.

As for CanonicalJSON, seems right, and I think we basically already do that. But note for instance in Tendermint we don't and won't use JSON for validator signing because the performance impact is too large, there's no humans in the loop, and it's much more painful to verify in the EVM.

[liamsi]

As for determinism, where can I learn about the gogo-proto deterministic mode?

AFAIR, gogo-proto's deterministic mode only deals with maps. Seems like nowadays this is achievable via the marhsalto plugin and the stable_marshaler option:
https://godoc.org/github.com/gogo/protobuf/plugin/marshalto

[alexanderbez]

Why do you say making go-amino proto compatible doesn't solve any problems? It doesn't solve performance, but doesn't it solve cross-client/language support (everyone else can use protobuf libs)?

Is this under the context of necessary changes being made to amino like @liamsi's PR? Do we not see performance as a primary motivating factor? Overall, I'm just simply not optimistic about major features like this being upstreamed to amino any time soon. Not to mention the lack of domain knowledge and overall set of eyes on it.

My main concern with a change like this is the impact on the codebase, in terms of large PRs and API changes.

Mine as well -- it's a very big risk. But all technical decisions come with tradeoffs. The question is, is this decision worth those tradeoffs?

I think this means preserving something like a Codec, but making it an interface (it probably always should have been one).

What would this codec interface look like? Would it be unique per module's needs?

... I recall this being a source of headaches when I experimented with it. I've found the proto generated types much less ergonomic, and I would consider depending on the output of their generators for your every day types to be a significant risk.

Can you shed some light on as to why this is a big risk and why you recommend doing this? What issues do you come across that would warrant this?

As for determinism, protobuf provides us this already, no? The only thing that needs to be guaranteed deterministic outside of this context, is the tx bytes over which you sign and we're using canonical JSON for this.

[ebuchman]

Is this under the context of necessary changes being made to amino like @liamsi's PR?

Yes

Do we not see performance as a primary motivating factor?

Sure.

Overall, I'm just simply not optimistic about major features like this being upstreamed to amino any time soon. Not to mention the lack of domain knowledge and overall set of eyes on it.

Fair enough!

The question is, is this decision worth those tradeoffs?

I think the change can be done iteratively and that doing so will probably be worth it.

What would this codec interface look like? Would it be unique per module's needs?

I expect it would just have Marshal/Unmarshal methods, maybe one for Binary and one for JSON.

Can you shed some light on as to why this is a big risk and why you recommend doing this? What issues do you come across that would warrant this?

I don't remember the exact issues now. But I do remember it being very painful. Looking at the generated ABCI types, they're riddled with garbage fields, for starters. Maybe we're doing something wrong, but that's kind of the point, you become very dependent on the particulars of gogo-proto to get things working the way you want and that's an awfully tight coupling that is almost guaranteed to cause problems, especially for a large project that is itself a dependency of others. Perhaps this can be mitigated somewhat by looking at how other similar projects are using/doing things, but I think that investigation needs to happen first to derisk the effort.

And regardless, I think having separate types for use in the code and use in serialization is still a new positive and good code hygiene. It also may enable these changes to proceed more incrementally.

As for determinism, protobuf provides us this already, no? The only thing that needs to be guaranteed deterministic outside of this context, is the tx bytes over which you sign and we're using canonical JSON for this.

So AFAIK there's three sources of serialization in the SDK:

• serializing a transaction for the blockchain
• serializing a transaction for signing (JSON)
• serializing state to be stored in the merkle tree

The first one is most likely to be handled by code in many languages and so any differences in proto impls may cause certain clients to fail. Just flagging in, but as far as I understood it should work properly ...

---

In sum, my main recomendation is that work be done to ensure this change can proceed incrementally as much as possible. This will both have the effect of improving the structure of the SDK codebase, it will make the work more manageable, and it should keep us flexible to experiment with other encoding schemes down the road

[aaronc]

Thanks @ebuchman for chiming in here!

I want to say that I suspect the issue with gogo proto generated structs is mostly related to configuration. By default there are all these XXX_ fields but there's a flag to shut those off and we're using it. Also with things like customtype and castype, there's a fair amount of control over what's generated. Not perfect, but in general I'm quite happy with the generated structs in most cases - in many cases almost identical to what was there before. For cases where some conversion is needed, it's usually possible to either add conversion getters to the struct and/or create helper functions.

I agree it's possible to create an interface that looks sort of like amino and that will actually make the refactoring quicker with less code change. I will give it a try in #5478.

I do think, however, that it's unrealistic to think that we can easily swap in or out some other encoding scheme. I spent some time exploring Cap'n proto in #5438 and will maybe comment more on it in a separate post. Basically there's going to be some overhead to whatever encoding scheme you choose and it's good to choose wisely. Maybe there are some levels where things are hot-swappable - i.e. we can support json, yaml, xml, etc. representations on some level. But what we are discussing here is a high performance binary encoding and that comes at a relatively high cost.

This reminds me a bit of the phase of ORM's when people wanted to do everything they could to be compatible with all the SQL engines, only to later find out that vendor lock-in was 1) almost impossible to avoid and 2) not always a bad thing because embracing some of the vendor's custom stuff sometimes brought a lot of value. To me actually getting into some of these encoding decisions in practice feels more akin to marriage - you want to go in with eyes wide open and embrace what you're getting. There are subtle things at many levels of the system (how oneof's behave, determinism, etc.). Maybe it is worth discussing whether cap'n proto really would be the right solution, or something else. But having worked with these encodings for the past couple weeks, I think we're best off making the choice consciously and investing in the ecosystem of whatever we choose.

With gogo proto, there is the issue of generating switch statements to convert oneofs to interfaces. In weave they generated all those by hand initially and then someone eventually wrote a helper to use reflection. I spent an afternoon last week and found out gogo proto has a nice plugin system and now we have this: https://github.com/regen-network/cosmos-proto which auto-generates all those switch statements likely saving lots of developer time and reducing potential error surface. Gogo proto is not immutable, it can be improved. If we use cap'n proto we will need to invest even more in improving it, and sticking with amino would likely be an even bigger investment, but that's a separate discussion...