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

Migrate x/staking to Protobuf #5600

Merged
merged 26 commits into from
Feb 6, 2020
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
dec703b
Migrate staking types to proto
alexanderbez Feb 1, 2020
31311cb
Use int32 for validator status
alexanderbez Feb 3, 2020
0d2938b
Update staking types
alexanderbez Feb 3, 2020
60dc367
Add benchmarks for bech32 pubkey funcs
alexanderbez Feb 3, 2020
f0f6f50
Update BondStatus type alias to int32
alexanderbez Feb 3, 2020
c268a9e
Remove PubKey proto type
alexanderbez Feb 3, 2020
95edc79
Fix unit test
alexanderbez Feb 3, 2020
46be4b4
Update staking keeper
alexanderbez Feb 3, 2020
7503396
Fix staking keeper tests
alexanderbez Feb 3, 2020
97ac0b5
Update query client
alexanderbez Feb 3, 2020
8e84801
Update staking genesis and app_test
alexanderbez Feb 3, 2020
c35939a
Update staking handler
alexanderbez Feb 4, 2020
febaae8
Use Int64Value instead of IntProto
alexanderbez Feb 4, 2020
5b83d9b
Updates tests and APIs
alexanderbez Feb 4, 2020
0a904f1
Fix iteration
alexanderbez Feb 4, 2020
2450409
Linting
alexanderbez Feb 4, 2020
97148e2
Fix linting
alexanderbez Feb 4, 2020
da9d51f
Merge branch 'master' into bez/5444-staking-proto-enc
alexanderbez Feb 4, 2020
11a0d8b
Add changelog entry
alexanderbez Feb 4, 2020
db45cfc
Update changelog entry
alexanderbez Feb 4, 2020
c5a7baa
Merge branch 'master' into bez/5444-staking-proto-enc
alexanderbez Feb 4, 2020
3e1b445
Merge branch 'master' into bez/5444-staking-proto-enc
alexanderbez Feb 5, 2020
d31577c
Start boilerplate for app-level codec
alexanderbez Feb 5, 2020
3dcfea6
Merge branch 'master' into bez/5444-staking-proto-enc
alexanderbez Feb 6, 2020
05ceef7
Merge branch 'master' into bez/5444-staking-proto-enc
alexanderbez Feb 6, 2020
c84d960
Lint proto and update codecov settings to ignore proto codegen
alexanderbez Feb 6, 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
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ issues:
- text: "ST1003:"
linters:
- stylecheck
# FIXME disabled until golangci-lint updates stylecheck with this fix:
# FIXME: Disabled until golangci-lint updates stylecheck with this fix:
# https://github.com/dominikh/go-tools/issues/389
- text: "ST1016:"
linters:
Expand Down
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@ balances or a single balance by denom when the `denom` query parameter is presen
* Callers to `NewBaseVestingAccount` are responsible for verifying account balance in relation to
the original vesting amount.
* The `SendKeeper` and `ViewKeeper` interfaces in `x/bank` have been modified to account for changes.
* (staking) [\#5600](https://github.com/cosmos/cosmos-sdk/pull/5600) Migrate the `x/staking` module to use Protocol Buffer for state
serialization instead of Amino. The exact codec used is `codec.HybridCodec` which utilizes Protobuf for binary encoding and Amino
for JSON encoding.
* `BondStatus` is now of type `int32` instead of `byte`.
* Types of `int16` in the `Params` type are now of type `int32`.
* Every reference of `crypto.Pubkey` in context of a `Validator` is now of type string. `GetPubKeyFromBech32` must be used to get the `crypto.Pubkey`.
* The `Keeper` constructor now takes a `codec.Marshaler` instead of a concrete Amino codec. This exact type
provided is specified by `ModuleCdc`.

### Improvements

Expand Down
8 changes: 4 additions & 4 deletions codec/amino.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ func RegisterEvidences(cdc *Codec) {
// MarshalJSONIndent provides a utility for indented JSON encoding of an object
// via an Amino codec. It returns an error if it cannot serialize or indent as
// JSON.
func MarshalJSONIndent(cdc *Codec, obj interface{}) ([]byte, error) {
bz, err := cdc.MarshalJSON(obj)
func MarshalJSONIndent(m JSONMarshaler, obj interface{}) ([]byte, error) {
bz, err := m.MarshalJSON(obj)
if err != nil {
return nil, err
}
Expand All @@ -60,8 +60,8 @@ func MarshalJSONIndent(cdc *Codec, obj interface{}) ([]byte, error) {
}

// MustMarshalJSONIndent executes MarshalJSONIndent except it panics upon failure.
func MustMarshalJSONIndent(cdc *Codec, obj interface{}) []byte {
bz, err := MarshalJSONIndent(cdc, obj)
func MustMarshalJSONIndent(m JSONMarshaler, obj interface{}) []byte {
bz, err := MarshalJSONIndent(m, obj)
if err != nil {
panic(fmt.Sprintf("failed to marshal JSON: %s", err))
}
Expand Down
4 changes: 4 additions & 0 deletions codec/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ type (
UnmarshalBinaryLengthPrefixed(bz []byte, ptr ProtoMarshaler) error
MustUnmarshalBinaryLengthPrefixed(bz []byte, ptr ProtoMarshaler)

JSONMarshaler
}

JSONMarshaler interface {
MarshalJSON(o interface{}) ([]byte, error) // nolint: stdmethods
MustMarshalJSON(o interface{}) []byte

Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
github.com/cosmos/ledger-cosmos-go v0.11.1
github.com/gogo/protobuf v1.3.1
github.com/golang/mock v1.3.1-0.20190508161146-9fa652df1129
github.com/golang/protobuf v1.3.2
Copy link
Member

@aaronc aaronc Feb 5, 2020

Choose a reason for hiding this comment

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

Why is golang/protobuf required? This likely means there's a directive needed to make a golang/protobuf type point to a gogo/protobuf type.

Copy link
Contributor Author

@alexanderbez alexanderbez Feb 5, 2020

Choose a reason for hiding this comment

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

Because of our dependency on

import "google/protobuf/timestamp.proto";
import "google/protobuf/duration.proto";

e.g.

message Commission {
  option (gogoproto.equal) = true;
  option (gogoproto.goproto_stringer) = false;

  // ...
  google.protobuf.Timestamp update_time = 2 [
    (gogoproto.nullable) = false,
    (gogoproto.stdtime) = true,
    (gogoproto.moretags) = "yaml:\"update_time\""
  ];
}

In addition, gogo proto itself depends on google/protobuf. Does gogo proto itself provide these? If so, what advantage is there with one over the other?

Copy link
Member

Choose a reason for hiding this comment

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

You need this in protocgen.sh:

protoc -I=. -I=$GOPATH/src -I=$GOPATH/src/github.com/gogo/protobuf/protobuf --{binary}_out=\
Mgoogle/protobuf/any.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/duration.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/struct.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/timestamp.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/wrappers.proto=github.com/gogo/protobuf/types:. \
myproto.proto

See https://github.com/gogo/protobuf#more-speed-and-more-generated-code

github.com/gorilla/mux v1.7.3
github.com/hashicorp/golang-lru v0.5.4
github.com/mattn/go-isatty v0.0.12
Expand Down
16 changes: 4 additions & 12 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/cosmos/cosmos-sdk/version"
"github.com/cosmos/cosmos-sdk/x/auth"
"github.com/cosmos/cosmos-sdk/x/auth/ante"
"github.com/cosmos/cosmos-sdk/x/auth/vesting"
"github.com/cosmos/cosmos-sdk/x/bank"
"github.com/cosmos/cosmos-sdk/x/crisis"
distr "github.com/cosmos/cosmos-sdk/x/distribution"
Expand Down Expand Up @@ -79,16 +78,6 @@ var (
}
)

// MakeCodec - custom tx codec
func MakeCodec() *codec.Codec {
var cdc = codec.New()
ModuleBasics.RegisterCodec(cdc)
vesting.RegisterCodec(cdc)
sdk.RegisterCodec(cdc)
codec.RegisterCrypto(cdc)
return cdc
}

// Verify app interface at compile time
var _ App = (*SimApp)(nil)

Expand Down Expand Up @@ -135,6 +124,9 @@ func NewSimApp(
invCheckPeriod uint, baseAppOptions ...func(*bam.BaseApp),
) *SimApp {

appCodec := NewAppCodec()

// TODO: Remove cdc in favor of appCodec once all modules are migrated.
cdc := MakeCodec()

bApp := bam.NewBaseApp(appName, logger, db, auth.DefaultTxDecoder(cdc), baseAppOptions...)
Expand Down Expand Up @@ -180,7 +172,7 @@ func NewSimApp(
app.cdc, keys[supply.StoreKey], app.AccountKeeper, app.BankKeeper, maccPerms,
)
stakingKeeper := staking.NewKeeper(
app.cdc, keys[staking.StoreKey], app.BankKeeper, app.SupplyKeeper, app.subspaces[staking.ModuleName],
appCodec.Staking, keys[staking.StoreKey], app.BankKeeper, app.SupplyKeeper, app.subspaces[staking.ModuleName],
Copy link
Member

Choose a reason for hiding this comment

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

Okay, I get it now. Would keepers also receive a Marshaler instance? Would AppCodec include Marshaler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would keepers also receive a Marshaler instance?

Yes! Unless they need a custom interface like x/auth that needs to know how to decode its custom types.

)
app.MintKeeper = mint.NewKeeper(
app.cdc, keys[mint.StoreKey], app.subspaces[mint.ModuleName], &stakingKeeper,
Expand Down
40 changes: 40 additions & 0 deletions simapp/codec.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package simapp

import (
"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/vesting"
"github.com/cosmos/cosmos-sdk/x/staking"
)

// AppCodec defines the application-level codec. This codec contains all the
// required module-specific codecs that are to be provided upon initialization.
type AppCodec struct {
amino *codec.Codec

Staking *staking.Codec
}

func NewAppCodec() *AppCodec {
amino := MakeCodec()

return &AppCodec{
amino: amino,
Staking: staking.NewCodec(amino),
}
}

// MakeCodec creates and returns a reference to an Amino codec that has all the
// necessary types and interfaces registered. This codec is provided to all the
// modules the application depends on.
//
// NOTE: This codec will be deprecated in favor of AppCodec once all modules are
// migrated.
func MakeCodec() *codec.Codec {
var cdc = codec.New()
ModuleBasics.RegisterCodec(cdc)
vesting.RegisterCodec(cdc)
sdk.RegisterCodec(cdc)
codec.RegisterCrypto(cdc)
return cdc
}
Loading