ICS07 follow up changes

docs/architecture/README.md

@@ -43,3 +43,4 @@ Please add a entry below in your Pull Request for an ADR.
 - [ADR 016: Validator Consensus Key Rotation](./adr-016-validator-consensus-key-rotation.md)
 - [ADR 017: Historical Header Module](./adr-017-historical-header-module.md)
 - [ADR 018: Extendable Voting Periods](./adr-018-extendable-voting-period.md)
+- [ADR 019: Protocol Buffer State Encoding](./adr-019-protobuf-state-encoding.md)

docs/architecture/adr-019-protobuf-state-encoding.md

@@ -0,0 +1,248 @@
+# ADR 019: Protocol Buffer State Encoding
+
+## Changelog
+
+- 2020 Feb 15: Initial Draft
+
+## Status
+
+Accepted
+
+## Context
+
+Currently, the Cosmos SDK utilizes [go-amino](https://github.com/tendermint/go-amino/) for binary
+and JSON object encoding over the wire bringing parity between logical objects and persistence objects.
+
+From the Amino docs:
+
+> Amino is an object encoding specification. It is a subset of Proto3 with an extension for interface
+> support. See the [Proto3 spec](https://developers.google.com/protocol-buffers/docs/proto3) for more
+> information on Proto3, which Amino is largely compatible with (but not with Proto2).
+>
+> The goal of the Amino encoding protocol is to bring parity into logic objects and persistence objects.
+
+Amino also aims to have the following goals (not a complete list):
+
+- Binary bytes must be decode-able with a schema.
+- Schema must be upgradeable.
+- The encoder and decoder logic must be reasonably simple.
+
+However, we believe that Amino does not fulfill these goals completely and does not fully meet the
+needs of a truly flexible cross-language and multi-client compatible encoding protocol in the Cosmos SDK.
+Namely, Amino has proven to be a big pain-point in regards to supporting object serialization across
+clients written in various languages while providing virtually little in the way of true backwards
+compatibility and upgradeability. Furthermore, through profiling and various benchmarks, Amino has
+been shown to be an extremely large performance bottleneck in the Cosmos SDK <sup>1</sup>. This is
+largely reflected in the performance of simulations and application transaction throughput.
+
+Thus, we need to adopt an encoding protocol that meets the following criteria for state serialization:
+
+- Language agnostic
+- Platform agnostic
+- Rich client support and thriving ecosystem
+- High performance
+- Minimal encoded message size
+- Codegen-based over reflection-based
+- Supports backward and forward compatibility
+
+Note, migrating away from Amino should be viewed as a two-pronged approach, state and client encoding.
+This ADR focuses on state serialization in the Cosmos SDK state machine. A corresponding ADR will be
+made to address client-side encoding.
+
+## Decision
+
+We will adopt [Protocol Buffers](https://developers.google.com/protocol-buffers) for serializing
+persisted structured data in the Cosmos SDK while providing a clean mechanism and developer UX for
+applications wishing to continue to use Amino. We will provide this mechanism by updating modules to
+accept a codec interface, `Marshaler`, instead of a concrete Amino codec. Furthermore, the Cosmos SDK
+will provide three concrete implementations of the `Marshaler` interface: `AminoCodec`, `ProtoCodec`,
+and `HybridCodec`.
+
+- `AminoCodec`: Uses Amino for both binary and JSON encoding.
+- `ProtoCodec`: Uses Protobuf for or both binary and JSON encoding.
+- `HybridCodec`: Uses Amino for JSON encoding and Protobuf for binary encoding.
+
+Until the client migration landscape is fully understood and designed, modules will use a `HybridCodec`
+as the concrete codec it accepts and/or extends. This means that all client JSON encoding, including
+genesis state, will still use Amino. The ultimate goal will be to replace Amino JSON encoding with
+Protbuf encoding and thus have modules accept and/or extend `ProtoCodec`.
+
+### Module Design
+
+Modules that do not require the ability to work with and serialize interfaces, the path to Protobuf
+migration is pretty straightforward. These modules are to simply migrate any existing types that
+are encoded and persisted via their concrete Amino codec to Protobuf and have their keeper accept a
+`Marshaler` that will be a `HybridCodec`. This migration is simple as things will just work as-is.
+
+Note, any business logic that needs to encode primitive types like `bool` or `int64` should use
+[gogoprotobuf](https://github.com/gogo/protobuf) Value types.
+
+Example:
+
+```go
+  ts, err := gogotypes.TimestampProto(completionTime)
+  if err != nil {
+    // ...
+  }
+
+  bz := cdc.MustMarshalBinaryLengthPrefixed(ts)
+```
+
+However, modules can vary greatly in purpose and design and so we must support the ability for modules
+to be able to encode and work with interfaces (e.g. `Account` or `Content`). For these modules, they
+must define their own codec interface that extends `Marshaler`. These specific interfaces are unique
+to the module and will contain method contracts that know how to serialize the needed interfaces.
+
+Example:
+
+```go
+// x/auth/types/codec.go
+
+type Codec interface {
+  codec.Marshaler
+
+  MarshalAccount(acc exported.Account) ([]byte, error)
+  UnmarshalAccount(bz []byte) (exported.Account, error)
+
+  MarshalAccountJSON(acc exported.Account) ([]byte, error)
+  UnmarshalAccountJSON(bz []byte) (exported.Account, error)
+}
+```
+
+Note, concrete types implementing these interfaces can be defined outside the scope of the module
+that defines the interface (e.g. `ModuleAccount` in `x/supply`). To handle these cases, a Protobuf
+message must be defined at the application-level along with a single codec that will be passed to _all_
+modules using a `oneof` approach.
+
+Example:
+
+```protobuf
+// app/codec/codec.proto
+
+import "third_party/proto/cosmos-proto/cosmos.proto";
+import "x/auth/types/types.proto";
+import "x/auth/vesting/types/types.proto";
+import "x/supply/types/types.proto";
+
+message Account {
+  option (cosmos_proto.interface_type) = "*github.com/cosmos/cosmos-sdk/x/auth/exported.Account";
+
+  // sum defines a list of all acceptable concrete Account implementations.
+  oneof sum {
+    cosmos_sdk.x.auth.v1.BaseAccount                      base_account               = 1;
+    cosmos_sdk.x.auth.vesting.v1.ContinuousVestingAccount continuous_vesting_account = 2;
+    cosmos_sdk.x.auth.vesting.v1.DelayedVestingAccount    delayed_vesting_account    = 3;
+    cosmos_sdk.x.auth.vesting.v1.PeriodicVestingAccount   periodic_vesting_account   = 4;
+    cosmos_sdk.x.supply.v1.ModuleAccount                  module_account             = 5;
+  }
+
+  // ...
+}
+```
+
+```go
+// app/codec/codec.go
+
+import (
+  "github.com/cosmos/cosmos-sdk/codec"
+  "github.com/cosmos/cosmos-sdk/x/auth"
+  "github.com/cosmos/cosmos-sdk/x/supply"
+  authexported "github.com/cosmos/cosmos-sdk/x/auth/exported"
+  // ...
+)
+
+var (
+  _ auth.Codec   = (*Codec)(nil)
+  // ...
+)
+
+type Codec struct {
+  codec.Marshaler
+
+
+  amino *codec.Codec
+}
+
+func NewAppCodec(amino *codec.Codec) *Codec {
+  return &Codec{Marshaler: codec.NewHybridCodec(amino), amino: amino}
+}
+
+func (c *Codec) MarshalAccount(accI authexported.Account) ([]byte, error) {
+  acc := &Account{}
+  if err := acc.SetAccount(accI); err != nil {
+    return nil, err
+  }
+
+  return c.Marshaler.MarshalBinaryLengthPrefixed(acc)
+}
+
+func (c *Codec) UnmarshalAccount(bz []byte) (authexported.Account, error) {
+  acc := &Account{}
+  if err := c.Marshaler.UnmarshalBinaryLengthPrefixed(bz, acc); err != nil {
+    return nil, err
+  }
+
+  return acc.GetAccount(), nil
+}
+```
+
+Since the `Codec` implements `auth.Codec` (and all other required interfaces), it is passed to _all_
+the modules and satisfies all the interfaces. Now each module needing to work with interfaces will know
+about all the required types. Note, the use of `interface_type` allows us to avoid a significant
+amount of code boilerplate when implementing the `Codec`.
+
+A similar concept is to be applied for messages that contain interfaces fields. The module will
+define a "base" concrete message type (e.g. `MsgSubmitProposalBase`) that the application-level codec
+will extend via `oneof` (e.g. `MsgSubmitProposal`) that fulfills the required interface
+(e.g. `MsgSubmitProposalI`). Note, however, the module's message handler must now switch on the
+interface rather than the concrete type for this particular message.
+
+### Why Wasn't X Chosen Instead
+
+For a more complete comparison to alternative protocols, see [here](https://codeburst.io/json-vs-protocol-buffers-vs-flatbuffers-a4247f8bda6f).
+
+### Cap'n Proto
+
+While [Cap’n Proto](https://capnproto.org/) does seem like an advantageous alternative to Protobuf
+due to it's native support for interfaces/generics and built in canonicalization, it does lack the
+rich client ecosystem compared to Protobuf and is a bit less mature.
+
+### FlatBuffers
+
+[FlatBuffers](https://google.github.io/flatbuffers/) is also a potentially viable alternative, with the
+primary difference being that FlatBuffers does not need a parsing/unpacking step to a secondary
+representation before you can access data, often coupled with per-object memory allocation.
+
+However, it would require great efforts into research and full understanding the scope of the migration
+and path forward -- which isn't immediately clear. In addition, FlatBuffers aren't designed for
+untrusted inputs.
+
+## Future Improvements & Roadmap
+
+The landscape and roadmap to restructuring queriers and tx generation to fully support
+Protobuf isn't fully understood yet. Once all modules are migrated, we will have a better
+understanding on how to proceed with client improvements (e.g. gRPC) <sup>2</sup>.
+
+## Consequences
+
+### Positive
+
+- Significant performance gains.
+- Supports backward and forward type compatibility.
+- Better support for cross-language clients.
+
+### Negative
+
+- Learning curve required to understand and implement Protobuf messages.
+- Less flexibility in cross-module type registration. We now need to define types
+at the application-level.
+- Client business logic and tx generation may become a bit more complex.
+
+### Neutral
+
+{neutral consequences}
+
+## References
+
+1. https://github.com/cosmos/cosmos-sdk/issues/4977
+2. https://github.com/cosmos/cosmos-sdk/issues/5444

docs/architecture/adr-template.md

@@ -37,4 +37,4 @@
 
 ## References
 
-- {reference link}
+- {reference link}

docs/intro/overview.md

@@ -25,7 +25,7 @@ The Cosmos SDK is the most advanced framework for building custom application-sp
 - The default consensus engine available within the SDK is [Tendermint Core](https://github.com/tendermint/tendermint). Tendermint is the most (and only) mature BFT consensus engine in existence. It is widely used across the industry and is considered the gold standard consensus engine for building Proof-of-Stake systems.
 - The SDK is open source and designed to make it easy to build blockchains out of composable [modules](../../x/). As the ecosystem of open source SDK modules grows, it will become increasingly easier to build complex decentralised platforms with it. 
 - The SDK is inspired by capabilities-based security, and informed by years of wrestling with blockchain state-machines. This makes the Cosmos SDK a very secure environment to build blockchains. 
-- Most importantly, the Cosmos SDK has already been used to build many application-specific blockchains that are already in production. Among others, we can cite [Cosmos Hub](https://hub.cosmos.network), [IRIS Hub](https://irisnet.org), [Binance Chain](https://docs.binance.org/), [Terra](https://terra.money/) or [Lino](https://lino.network/). [Many more](https://cosmos.network/ecosystem) are building on the Cosmos SDK. 
+- Most importantly, the Cosmos SDK has already been used to build many application-specific blockchains that are already in production. Among others, we can cite [Cosmos Hub](https://hub.cosmos.network), [IRIS Hub](https://irisnet.org), [Binance Chain](https://docs.binance.org/), [Terra](https://terra.money/) or [Kava](https://www.kava.io/). [Many more](https://cosmos.network/ecosystem) are building on the Cosmos SDK. 
 
 ## Getting started with the Cosmos SDK
 
@@ -34,4 +34,4 @@ The Cosmos SDK is the most advanced framework for building custom application-sp
 
 ## Next {hide}
 
-Learn about [application-specific blockchains](./why-app-specific.md) {hide}
+Learn about [application-specific blockchains](./why-app-specific.md) {hide}

types/rest/rest.go

@@ -20,8 +20,8 @@ import (
 )
 
 const (
-	DefaultPage  = 1
-	DefaultLimit = 30 // should be consistent with tendermint/tendermint/rpc/core/pipe.go:19
+	DefaultPage    = 1
+	DefaultLimit   = 30             // should be consistent with tendermint/tendermint/rpc/core/pipe.go:19
 	TxMinHeightKey = "tx.minheight" // Inclusive minimum height filter
 	TxMaxHeightKey = "tx.maxheight" // Inclusive maximum height filter
 )
@@ -337,13 +337,14 @@ func ParseHTTPArgsWithLimit(r *http.Request, defaultLimit int) (tags []string, p
 		}
 
 		var tag string
-		if key == types.TxHeightKey {
+		switch key {
+		case types.TxHeightKey:
 			tag = fmt.Sprintf("%s=%s", key, value)
-		} else if key == TxMinHeightKey {
+		case TxMinHeightKey:
 			tag = fmt.Sprintf("%s>=%s", types.TxHeightKey, value)
-		} else if key == TxMaxHeightKey {
+		case TxMaxHeightKey:
 			tag = fmt.Sprintf("%s<=%s", types.TxHeightKey, value)
-		} else {
+		default:
 			tag = fmt.Sprintf("%s='%s'", key, value)
 		}
 		tags = append(tags, tag)

types/rest/rest_test.go

@@ -7,6 +7,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"sort"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -91,12 +92,15 @@ func TestParseHTTPArgs(t *testing.T) {
 		{"error limit 0", reqE2, httptest.NewRecorder(), []string{}, DefaultPage, DefaultLimit, true},
 
 		{"tags", req4, httptest.NewRecorder(), []string{"foo='faa'"}, DefaultPage, DefaultLimit, false},
-		{"tags", reqTxH, httptest.NewRecorder(), []string{"tx.height>=12", "tx.height<=14"}, DefaultPage, DefaultLimit, false},
+		{"tags", reqTxH, httptest.NewRecorder(), []string{"tx.height<=14", "tx.height>=12"}, DefaultPage, DefaultLimit, false},
 	}
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			tags, page, limit, err := ParseHTTPArgs(tt.req)
+
+			sort.Strings(tags)
+
 			if tt.err {
 				require.NotNil(t, err)
 			} else {

x/ibc/02-client/alias.go

@@ -37,8 +37,6 @@ var (
 	ErrRootNotFound           = types.ErrRootNotFound
 	ErrInvalidHeader          = types.ErrInvalidHeader
 	ErrInvalidEvidence        = types.ErrInvalidEvidence
-	NewMsgCreateClient        = types.NewMsgCreateClient
-	NewMsgUpdateClient        = types.NewMsgUpdateClient
 
 	// variable aliases
 	SubModuleCdc           = types.SubModuleCdc
@@ -48,8 +46,6 @@ var (
 )
 
 type (
-	Keeper          = keeper.Keeper
-	StakingKeeper   = types.StakingKeeper
-	MsgCreateClient = types.MsgCreateClient
-	MsgUpdateClient = types.MsgUpdateClient
+	Keeper        = keeper.Keeper
+	StakingKeeper = types.StakingKeeper
 )

x/ibc/02-client/client/cli/cli.go

@@ -26,21 +26,3 @@ func GetQueryCmd(queryRoute string, cdc *codec.Codec) *cobra.Command {
 	)...)
 	return ics02ClientQueryCmd
 }
-
-// GetTxCmd returns the transaction commands for IBC clients
-func GetTxCmd(storeKey string, cdc *codec.Codec) *cobra.Command {
-	ics02ClientTxCmd := &cobra.Command{
-		Use:                        "client",
-		Short:                      "Client transaction subcommands",
-		DisableFlagParsing:         true,
-		SuggestionsMinimumDistance: 2,
-	}
-
-	ics02ClientTxCmd.AddCommand(flags.PostCommands(
-		GetCmdCreateClient(cdc),
-		GetCmdUpdateClient(cdc),
-		GetCmdSubmitMisbehaviour(cdc),
-	)...)
-
-	return ics02ClientTxCmd
-}