Skip to content

Commit

Permalink
refactor: Implementing sigs.k8s.io YAML to remove .proto yaml annotat…
Browse files Browse the repository at this point in the history
…ions (#9780)

## Description

Draft of: #9705 

Started off with changing codec `MarshalYaml` function to directly go from JSON to yaml using the new library. Replaced the only usage of UnmarshalYaml per request.
---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [x] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [x] reviewed state machine logic
- [x] reviewed API design and naming
- [x] reviewed documentation is accurate
- [x] reviewed tests and test coverage
- [x] manually tested (if applicable)
  • Loading branch information
lukerhoads authored Sep 24, 2021
1 parent f265287 commit bf11b1b
Show file tree
Hide file tree
Showing 97 changed files with 2,046 additions and 2,199 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,11 @@ Ref: https://keepachangelog.com/en/1.0.0/

* [\#9695](https://github.com/cosmos/cosmos-sdk/pull/9695) `<app> keys migrate` CLI command now takes no arguments
* [\#9246](https://github.com/cosmos/cosmos-sdk/pull/9246) Removed the CLI flag `--setup-config-only` from the `testnet` command and added the subcommand `init-files`.
* [\#9780](https://github.com/cosmos/cosmos-sdk/pull/9780) Use sigs.k8s.io for yaml, which might lead to minor YAML output changes

### Improvements

* [\#9780](https://github.com/cosmos/cosmos-sdk/pull/9780) Remove gogoproto `moretags` YAML annotations and add `sigs.k8s.io/yaml` for YAML marshalling.
* (x/bank) [\#10134](https://github.com/cosmos/cosmos-sdk/pull/10134) Add `HasDenomMetadata` function to bank `Keeper` to check if a client coin denom metadata exists in state.
* (store) [\#10026](https://github.com/cosmos/cosmos-sdk/pull/10026) Improve CacheKVStore datastructures / algorithms, to no longer take O(N^2) time when interleaving iterators and insertions.
* (types) [\#10076](https://github.com/cosmos/cosmos-sdk/pull/10076) Significantly speedup and lower allocations for `Coins.String()`.
Expand Down
16 changes: 4 additions & 12 deletions client/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@ package client

import (
"bufio"
"encoding/json"
"io"
"os"

"github.com/spf13/viper"

"gopkg.in/yaml.v2"
"sigs.k8s.io/yaml"

"github.com/gogo/protobuf/proto"
rpcclient "github.com/tendermint/tendermint/rpc/client"
Expand Down Expand Up @@ -279,16 +278,9 @@ func (ctx Context) PrintObjectLegacy(toPrint interface{}) error {
}

func (ctx Context) printOutput(out []byte) error {
var err error
if ctx.OutputFormat == "text" {
// handle text format by decoding and re-encoding JSON as YAML
var j interface{}

err := json.Unmarshal(out, &j)
if err != nil {
return err
}

out, err = yaml.Marshal(j)
out, err = yaml.JSONToYAML(out)
if err != nil {
return err
}
Expand All @@ -299,7 +291,7 @@ func (ctx Context) printOutput(out []byte) error {
writer = os.Stdout
}

_, err := writer.Write(out)
_, err = writer.Write(out)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion client/keys/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

"github.com/spf13/cobra"
"github.com/tendermint/tendermint/libs/cli"
yaml "gopkg.in/yaml.v2"
"sigs.k8s.io/yaml"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/bech32"
Expand Down
2 changes: 1 addition & 1 deletion client/keys/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"fmt"
"io"

yaml "gopkg.in/yaml.v2"
"sigs.k8s.io/yaml"

cryptokeyring "github.com/cosmos/cosmos-sdk/crypto/keyring"
)
Expand Down
13 changes: 2 additions & 11 deletions codec/yaml.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package codec

import (
"encoding/json"

"github.com/gogo/protobuf/proto"
"gopkg.in/yaml.v2"
"sigs.k8s.io/yaml"
)

// MarshalYAML marshals toPrint using JSONCodec to leverage specialized MarshalJSON methods
Expand All @@ -18,12 +16,5 @@ func MarshalYAML(cdc JSONCodec, toPrint proto.Message) ([]byte, error) {
return nil, err
}

// generate YAML by decoding JSON and re-encoding to YAML
var j interface{}
err = json.Unmarshal(bz, &j)
if err != nil {
return nil, err
}

return yaml.Marshal(j)
return yaml.JSONToYAML(bz)
}
31 changes: 15 additions & 16 deletions crypto/keys/multisig/keys.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ require (
google.golang.org/grpc v1.40.0
google.golang.org/protobuf v1.27.1
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
gopkg.in/yaml.v2 v2.4.0
sigs.k8s.io/yaml v1.2.0
)

require (
Expand Down Expand Up @@ -118,6 +118,7 @@ require (
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 // indirect
golang.org/x/text v0.3.6 // indirect
gopkg.in/ini.v1 v1.63.2 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
nhooyr.io/websocket v1.8.6 // indirect
)
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1303,4 +1303,6 @@ rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8
rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0=
rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o=
sigs.k8s.io/yaml v1.2.0 h1:kr/MCeFWJWTwyaHoR9c8EjH9OumOmoF9YGiZd7lFm/Q=
sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc=
sourcegraph.com/sourcegraph/appdash v0.0.0-20190731080439-ebfcffb1b5c0/go.mod h1:hI742Nqp5OhwiqlzhgfbWU4mW4yO10fP+LoT9WOswdU=
16 changes: 8 additions & 8 deletions proto/cosmos/auth/v1beta1/auth.proto
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ message BaseAccount {

string address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
google.protobuf.Any pub_key = 2
[(gogoproto.jsontag) = "public_key,omitempty", (gogoproto.moretags) = "yaml:\"public_key\""];
uint64 account_number = 3 [(gogoproto.moretags) = "yaml:\"account_number\""];
[(gogoproto.jsontag) = "public_key,omitempty"];
uint64 account_number = 3;
uint64 sequence = 4;
}

Expand All @@ -30,7 +30,7 @@ message ModuleAccount {
option (gogoproto.goproto_stringer) = false;
option (cosmos_proto.implements_interface) = "ModuleAccountI";

BaseAccount base_account = 1 [(gogoproto.embed) = true, (gogoproto.moretags) = "yaml:\"base_account\""];
BaseAccount base_account = 1 [(gogoproto.embed) = true];
string name = 2;
repeated string permissions = 3;
}
Expand All @@ -40,11 +40,11 @@ message Params {
option (gogoproto.equal) = true;
option (gogoproto.goproto_stringer) = false;

uint64 max_memo_characters = 1 [(gogoproto.moretags) = "yaml:\"max_memo_characters\""];
uint64 tx_sig_limit = 2 [(gogoproto.moretags) = "yaml:\"tx_sig_limit\""];
uint64 tx_size_cost_per_byte = 3 [(gogoproto.moretags) = "yaml:\"tx_size_cost_per_byte\""];
uint64 max_memo_characters = 1;
uint64 tx_sig_limit = 2;
uint64 tx_size_cost_per_byte = 3;
uint64 sig_verify_cost_ed25519 = 4
[(gogoproto.customname) = "SigVerifyCostED25519", (gogoproto.moretags) = "yaml:\"sig_verify_cost_ed25519\""];
[(gogoproto.customname) = "SigVerifyCostED25519"];
uint64 sig_verify_cost_secp256k1 = 5
[(gogoproto.customname) = "SigVerifyCostSecp256k1", (gogoproto.moretags) = "yaml:\"sig_verify_cost_secp256k1\""];
[(gogoproto.customname) = "SigVerifyCostSecp256k1"];
}
4 changes: 2 additions & 2 deletions proto/cosmos/bank/v1beta1/bank.proto
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ option go_package = "github.com/cosmos/cosmos-sdk/x/bank/types";
// Params defines the parameters for the bank module.
message Params {
option (gogoproto.goproto_stringer) = false;
repeated SendEnabled send_enabled = 1 [(gogoproto.moretags) = "yaml:\"send_enabled,omitempty\""];
bool default_send_enabled = 2 [(gogoproto.moretags) = "yaml:\"default_send_enabled,omitempty\""];
repeated SendEnabled send_enabled = 1;
bool default_send_enabled = 2;
}

// SendEnabled maps coin denom to a send_enabled status (whether a denom is
Expand Down
2 changes: 1 addition & 1 deletion proto/cosmos/bank/v1beta1/genesis.proto
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ message GenesisState {
[(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins", (gogoproto.nullable) = false];

// denom_metadata defines the metadata of the differents coins.
repeated Metadata denom_metadata = 4 [(gogoproto.moretags) = "yaml:\"denom_metadata\"", (gogoproto.nullable) = false];
repeated Metadata denom_metadata = 4 [(gogoproto.nullable) = false];
}

// Balance defines an account address and balance pair used in the bank module's
Expand Down
4 changes: 2 additions & 2 deletions proto/cosmos/bank/v1beta1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ message MsgSend {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;

string from_address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString", (gogoproto.moretags) = "yaml:\"from_address\""];
string to_address = 2 [(cosmos_proto.scalar) = "cosmos.AddressString", (gogoproto.moretags) = "yaml:\"to_address\""];
string from_address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
string to_address = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"];
repeated cosmos.base.v1beta1.Coin amount = 3
[(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"];
}
Expand Down
10 changes: 5 additions & 5 deletions proto/cosmos/base/abci/v1beta1/abci.proto
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,10 @@ message Attribute {
// GasInfo defines tx execution gas context.
message GasInfo {
// GasWanted is the maximum units of work we allow this tx to perform.
uint64 gas_wanted = 1 [(gogoproto.moretags) = "yaml:\"gas_wanted\""];
uint64 gas_wanted = 1;

// GasUsed is the amount of gas actually consumed.
uint64 gas_used = 2 [(gogoproto.moretags) = "yaml:\"gas_used\""];
uint64 gas_used = 2;
}

// Result is the union of ResponseFormat and ResponseCheckTx.
Expand Down Expand Up @@ -123,13 +123,13 @@ message SearchTxsResult {
option (gogoproto.stringer) = true;

// Count of all txs
uint64 total_count = 1 [(gogoproto.moretags) = "yaml:\"total_count\"", (gogoproto.jsontag) = "total_count"];
uint64 total_count = 1 [(gogoproto.jsontag) = "total_count"];
// Count of txs in current page
uint64 count = 2;
// Index of current page, start from 1
uint64 page_number = 3 [(gogoproto.moretags) = "yaml:\"page_number\"", (gogoproto.jsontag) = "page_number"];
uint64 page_number = 3 [(gogoproto.jsontag) = "page_number"];
// Count of total pages
uint64 page_total = 4 [(gogoproto.moretags) = "yaml:\"page_total\"", (gogoproto.jsontag) = "page_total"];
uint64 page_total = 4 [(gogoproto.jsontag) = "page_total"];
// Max count txs per page
uint64 limit = 5;
// List of txs in current page
Expand Down
6 changes: 3 additions & 3 deletions proto/cosmos/capability/v1beta1/capability.proto
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import "gogoproto/gogo.proto";
message Capability {
option (gogoproto.goproto_stringer) = false;

uint64 index = 1 [(gogoproto.moretags) = "yaml:\"index\""];
uint64 index = 1;
}

// Owner defines a single capability owner. An owner is defined by the name of
Expand All @@ -19,8 +19,8 @@ message Owner {
option (gogoproto.goproto_stringer) = false;
option (gogoproto.goproto_getters) = false;

string module = 1 [(gogoproto.moretags) = "yaml:\"module\""];
string name = 2 [(gogoproto.moretags) = "yaml:\"name\""];
string module = 1;
string name = 2;
}

// CapabilityOwners defines a set of owners of a single Capability. The set of
Expand Down
2 changes: 1 addition & 1 deletion proto/cosmos/capability/v1beta1/genesis.proto
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ message GenesisOwners {
uint64 index = 1;

// index_owners are the owners at the given index.
CapabilityOwners index_owners = 2 [(gogoproto.nullable) = false, (gogoproto.moretags) = "yaml:\"index_owners\""];
CapabilityOwners index_owners = 2 [(gogoproto.nullable) = false];
}

// GenesisState defines the capability module's genesis state.
Expand Down
2 changes: 1 addition & 1 deletion proto/cosmos/crisis/v1beta1/genesis.proto
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ message GenesisState {
// constant_fee is the fee used to verify the invariant in the crisis
// module.
cosmos.base.v1beta1.Coin constant_fee = 3
[(gogoproto.nullable) = false, (gogoproto.moretags) = "yaml:\"constant_fee\""];
[(gogoproto.nullable) = false];
}
4 changes: 2 additions & 2 deletions proto/cosmos/crisis/v1beta1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ message MsgVerifyInvariant {
option (gogoproto.goproto_getters) = false;

string sender = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
string invariant_module_name = 2 [(gogoproto.moretags) = "yaml:\"invariant_module_name\""];
string invariant_route = 3 [(gogoproto.moretags) = "yaml:\"invariant_route\""];
string invariant_module_name = 2;
string invariant_route = 3;
}

// MsgVerifyInvariantResponse defines the Msg/VerifyInvariant response type.
Expand Down
4 changes: 2 additions & 2 deletions proto/cosmos/crypto/multisig/keys.proto
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ option go_package = "github.com/cosmos/cosmos-sdk/crypto/keys/multisig";
message LegacyAminoPubKey {
option (gogoproto.goproto_getters) = false;

uint32 threshold = 1 [(gogoproto.moretags) = "yaml:\"threshold\""];
uint32 threshold = 1;
repeated google.protobuf.Any public_keys = 2
[(gogoproto.customname) = "PubKeys", (gogoproto.moretags) = "yaml:\"pubkeys\""];
[(gogoproto.customname) = "PubKeys"];
}
Loading

0 comments on commit bf11b1b

Please sign in to comment.