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

fix: validate validator addresses in update-validator-auths proposal #411

Merged
merged 15 commits into from
Jan 17, 2022
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
* (swagger) [\#391] (https://github.com/line/lbm-sdk/pull/391) fix swagger's config path for wasm
* (x/wasm) [\#393] (https://github.com/line/lbm-sdk/pull/393) fix bug where `StoreCodeAndInstantiateContract`, `UpdateContractStatus`, `UpdateContractStatusProposal` API does not work
* (x/slashing) [\#407] (https://github.com/line/lbm-sdk/pull/407) Fix query signing infos command
* (x/consortium) [\#411] (https://github.com/line/lbm-sdk/pull/411) Validate validator addresses in update-validator-auths proposal

### Breaking Changes

Expand Down
2 changes: 1 addition & 1 deletion docs/core/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -6072,8 +6072,8 @@ Query defines the gRPC querier service for consortium module.
| Method Name | Request Type | Response Type | Description | HTTP Verb | Endpoint |
| ----------- | ------------ | ------------- | ------------| ------- | -------- |
| `Params` | [QueryParamsRequest](#lbm.consortium.v1.QueryParamsRequest) | [QueryParamsResponse](#lbm.consortium.v1.QueryParamsResponse) | Params queries the module params. | GET|/lbm/consortium/v1/params|
| `ValidatorAuths` | [QueryValidatorAuthsRequest](#lbm.consortium.v1.QueryValidatorAuthsRequest) | [QueryValidatorAuthsResponse](#lbm.consortium.v1.QueryValidatorAuthsResponse) | ValidatorAuths queries authorization infos of validators. | GET|/lbm/consortium/v1/validators|
| `ValidatorAuth` | [QueryValidatorAuthRequest](#lbm.consortium.v1.QueryValidatorAuthRequest) | [QueryValidatorAuthResponse](#lbm.consortium.v1.QueryValidatorAuthResponse) | ValidatorAuth queries authorization info of a validator. | GET|/lbm/consortium/v1/validators/{validator_address}|
| `ValidatorAuths` | [QueryValidatorAuthsRequest](#lbm.consortium.v1.QueryValidatorAuthsRequest) | [QueryValidatorAuthsResponse](#lbm.consortium.v1.QueryValidatorAuthsResponse) | ValidatorAuths queries authorization infos of validators. | GET|/lbm/consortium/v1/validators|
Comment on lines -6075 to +6076
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

reposition the two functions


<!-- end services -->

Expand Down
36 changes: 18 additions & 18 deletions proto/lbm/consortium/v1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ service Query {
option (google.api.http).get = "/lbm/consortium/v1/params";
}

// ValidatorAuths queries authorization infos of validators.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

reposition the two functions

rpc ValidatorAuths(QueryValidatorAuthsRequest) returns (QueryValidatorAuthsResponse) {
option (google.api.http).get = "/lbm/consortium/v1/validators";
}

// ValidatorAuth queries authorization info of a validator.
rpc ValidatorAuth(QueryValidatorAuthRequest) returns (QueryValidatorAuthResponse) {
option (google.api.http).get = "/lbm/consortium/v1/validators/{validator_address}";
}

// ValidatorAuths queries authorization infos of validators.
rpc ValidatorAuths(QueryValidatorAuthsRequest) returns (QueryValidatorAuthsResponse) {
option (google.api.http).get = "/lbm/consortium/v1/validators";
}
}

// QueryParamsRequest is the request type for the Query/Params RPC method.
Expand All @@ -33,6 +33,19 @@ message QueryParamsResponse {
Params params = 1;
}

// QueryValidatorAuthRequest is the request type for the
// Query/ValidatorAuth RPC method.
message QueryValidatorAuthRequest {
// validator_address defines the validator address to query for.
string validator_address = 1;
}

// QueryValidatorAuthResponse is the request type for the
// Query/ValidatorAuth RPC method.
message QueryValidatorAuthResponse {
ValidatorAuth auth = 1;
}

// QueryValidatorAuthsRequest is the request type for the
// Query/ValidatorAuths RPC method.
message QueryValidatorAuthsRequest {
Expand All @@ -48,16 +61,3 @@ message QueryValidatorAuthsResponse {
// pagination defines the pagination in the response.
lbm.base.query.v1.PageResponse pagination = 2;
}

// QueryValidatorAuthRequest is the request type for the
// Query/ValidatorAuth RPC method.
message QueryValidatorAuthRequest {
// validator_address defines the validator address to query for.
string validator_address = 1;
}

// QueryValidatorAuthResponse is the request type for the
// Query/ValidatorAuth RPC method.
message QueryValidatorAuthResponse {
ValidatorAuth auth = 1;
}
44 changes: 44 additions & 0 deletions testutil/rest/rest.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Package rest provides HTTP types and primitives for REST
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

import rest.go from cosmos-sdk

// requests validation and responses handling.
package rest

import (
"bytes"
"fmt"
"io/ioutil"
"net/http"
)

// GetRequest defines a wrapper around an HTTP GET request with a provided URL.
// An error is returned if the request or reading the body fails.
func GetRequest(url string) ([]byte, error) {
res, err := http.Get(url) // nolint:gosec
if err != nil {
return nil, err
}
defer res.Body.Close()

body, err := ioutil.ReadAll(res.Body)
if err != nil {
return nil, err
}

return body, nil
}

// PostRequest defines a wrapper around an HTTP POST request with a provided URL and data.
// An error is returned if the request or reading the body fails.
func PostRequest(url string, contentType string, data []byte) ([]byte, error) {
res, err := http.Post(url, contentType, bytes.NewBuffer(data)) // nolint:gosec
if err != nil {
return nil, fmt.Errorf("error while sending post request: %w", err)
}
defer res.Body.Close()

bz, err := ioutil.ReadAll(res.Body)
if err != nil {
return nil, fmt.Errorf("error reading response body: %w", err)
}

return bz, nil
}
56 changes: 28 additions & 28 deletions x/consortium/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,24 @@ import (
"github.com/line/lbm-sdk/x/consortium/types"
)

// GetQueryCmd returns the parent command for all x/consortium CLi query commands.
func GetQueryCmd() *cobra.Command {
// NewQueryCmd returns the parent command for all x/consortium CLi query commands.
func NewQueryCmd() *cobra.Command {
cmd := &cobra.Command{
Use: types.ModuleName,
Short: "Querying commands for the consortium module",
}

cmd.AddCommand(
GetParamsCmd(),
GetValidatorAuthsCmd(),
GetValidatorAuthCmd(),
NewQueryCmdParams(),
NewQueryCmdValidatorAuth(),
NewQueryCmdValidatorAuths(),
Comment on lines -22 to +24
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

rename the functions

Copy link
Member

Choose a reason for hiding this comment

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

Why did you change the name with prefix New?
I understand the New prefix of function creates some instance. But I think this functions doesn't seems to do that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's a matter of choice. You can use both of New and Get, then they would mean creating a new command instance and get a command instance, respectively.

An example of New:
https://github.com/cosmos/cosmos-sdk/blob/a7c09a3278613a2c383968e73ee36150245cbdc1/x/nft/client/cli/tx.go#L27-L29

An example of Get:
https://github.com/cosmos/cosmos-sdk/blob/a7c09a3278613a2c383968e73ee36150245cbdc1/x/nft/client/cli/query.go#L31-L39

It is interesting that they are both in the same module.

Copy link
Member

Choose a reason for hiding this comment

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

I understood. Thank you.

)

return cmd
}

// GetParamsCmd returns the query consortium parameters command.
func GetParamsCmd() *cobra.Command {
// NewQueryCmdParams returns the query consortium parameters command.
func NewQueryCmdParams() *cobra.Command {
cmd := &cobra.Command{
Use: "params",
Short: "Query consortium params",
Expand Down Expand Up @@ -56,27 +56,27 @@ func GetParamsCmd() *cobra.Command {
return cmd
}

// GetValidatorAuthsCmd returns validator authorization by consortium
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

reposition the two functions

func GetValidatorAuthsCmd() *cobra.Command {
// NewQueryCmdValidatorAuth returns validator authorization by consortium
func NewQueryCmdValidatorAuth() *cobra.Command {
cmd := &cobra.Command{
Use: "validator-auths",
Short: "Query validator authorizations",
Long: "Gets validator authorizations by consortium",
Args: cobra.NoArgs,
Use: "validator-auth [validator-address]",
Short: "Query validator authorization",
Long: "Gets validator authorization by consortium",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientQueryContext(cmd)
if err != nil {
return err
}
queryClient := types.NewQueryClient(clientCtx)

pageReq, err := client.ReadPageRequest(cmd.Flags())
if err != nil {
valAddr := args[0]
if err = sdk.ValidateValAddress(valAddr); err != nil {
return err
}

params := types.QueryValidatorAuthsRequest{Pagination: pageReq}
res, err := queryClient.ValidatorAuths(context.Background(), &params)
params := types.QueryValidatorAuthRequest{ValidatorAddress: valAddr}
res, err := queryClient.ValidatorAuth(context.Background(), &params)
if err != nil {
return err
}
Expand All @@ -86,32 +86,31 @@ func GetValidatorAuthsCmd() *cobra.Command {
}

flags.AddQueryFlagsToCmd(cmd)
flags.AddPaginationFlagsToCmd(cmd, "validator auths")

return cmd
}

// GetValidatorAuthCmd returns validator authorizations by consortium
func GetValidatorAuthCmd() *cobra.Command {
// NewQueryCmdValidatorAuths returns validator authorizations by consortium
func NewQueryCmdValidatorAuths() *cobra.Command {
cmd := &cobra.Command{
Use: "validator-auth [validator-address]",
Short: "Query validator authorization",
Long: "Gets validator authorization by consortium",
Args: cobra.ExactArgs(1),
Use: "validator-auths",
Short: "Query validator authorizations",
Long: "Gets validator authorizations by consortium",
Args: cobra.NoArgs,
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientQueryContext(cmd)
if err != nil {
return err
}
queryClient := types.NewQueryClient(clientCtx)

valAddr := args[0]
if err = sdk.ValidateValAddress(valAddr); err != nil {
pageReq, err := client.ReadPageRequest(cmd.Flags())
if err != nil {
return err
}

params := types.QueryValidatorAuthRequest{ValidatorAddress: valAddr}
res, err := queryClient.ValidatorAuth(context.Background(), &params)
params := types.QueryValidatorAuthsRequest{Pagination: pageReq}
res, err := queryClient.ValidatorAuths(context.Background(), &params)
if err != nil {
return err
}
Expand All @@ -121,6 +120,7 @@ func GetValidatorAuthCmd() *cobra.Command {
}

flags.AddQueryFlagsToCmd(cmd)
flags.AddPaginationFlagsToCmd(cmd, "validator auths")

return cmd
}
30 changes: 7 additions & 23 deletions x/consortium/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/line/lbm-sdk/client"
"github.com/line/lbm-sdk/client/tx"
sdk "github.com/line/lbm-sdk/types"
sdkerrors "github.com/line/lbm-sdk/types/errors"
"github.com/line/lbm-sdk/version"
"github.com/line/lbm-sdk/x/consortium/types"
"github.com/line/lbm-sdk/x/gov/client/cli"
Expand All @@ -22,8 +21,8 @@ const (
FlagAllowedValidatorDelete = "delete"
)

// NewCmdSubmitUpdateConsortiumParamsProposal implements the command to submit an update-consortium-params proposal
func NewCmdSubmitUpdateConsortiumParamsProposal() *cobra.Command {
// NewProposalCmdUpdateConsortiumParams implements the command to submit an update-consortium-params proposal
func NewProposalCmdUpdateConsortiumParams() *cobra.Command {
cmd := &cobra.Command{
Use: "update-consortium-params",
Args: cobra.NoArgs,
Expand Down Expand Up @@ -69,7 +68,6 @@ $ %s tx gov submit-proposal update-consortium-params [flags]
Enabled: false,
}
content := types.NewUpdateConsortiumParamsProposal(title, description, params)

msg, err := govtypes.NewMsgSubmitProposal(content, deposit, from)
if err != nil {
return err
Expand All @@ -90,8 +88,8 @@ $ %s tx gov submit-proposal update-consortium-params [flags]
return cmd
}

// NewCmdSubmitUpdateValidatorAuthsProposal implements the command to submit an update-validator-auths proposal
func NewCmdSubmitUpdateValidatorAuthsProposal() *cobra.Command {
// NewProposalCmdUpdateValidatorAuths implements the command to submit an update-validator-auths proposal
func NewProposalCmdUpdateValidatorAuths() *cobra.Command {
cmd := &cobra.Command{
Use: "update-validator-auths",
Args: cobra.NoArgs,
Expand Down Expand Up @@ -151,18 +149,8 @@ $ %s tx gov submit-proposal update-validator-auths [flags]
}
deletingValidators := parseCommaSeparated(deletingValidatorsStr)

createAuths := func(addings, deletings []string) ([]*types.ValidatorAuth, error) {
createAuths := func(addings, deletings []string) []*types.ValidatorAuth {
var auths []*types.ValidatorAuth

// check duplications
usedAddrs := map[string]bool{}
for _, addr := range append(addings, deletings...) {
if usedAddrs[addr] {
return auths, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "multiple auths for same validator: %s", addr)
}
usedAddrs[addr] = true
}

for _, addr := range addings {
Comment on lines -156 to 154
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove the validation logic because we call ValidateBasic() which has the same logic

auth := &types.ValidatorAuth{
OperatorAddress: addr,
Expand All @@ -178,15 +166,11 @@ $ %s tx gov submit-proposal update-validator-auths [flags]
auths = append(auths, auth)
}

return auths, nil
return auths
}

auths, err := createAuths(addingValidators, deletingValidators)
if err != nil {
return err
}
auths := createAuths(addingValidators, deletingValidators)
content := types.NewUpdateValidatorAuthsProposal(title, description, auths)

msg, err := govtypes.NewMsgSubmitProposal(content, deposit, from)
if err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions x/consortium/client/proposal_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ import (
govclient "github.com/line/lbm-sdk/x/gov/client"
)

var UpdateConsortiumParamsProposalHandler = govclient.NewProposalHandler(cli.NewCmdSubmitUpdateConsortiumParamsProposal, rest.DummyRESTHandler)
var UpdateValidatorAuthsProposalHandler = govclient.NewProposalHandler(cli.NewCmdSubmitUpdateValidatorAuthsProposal, rest.DummyRESTHandler)
var UpdateConsortiumParamsProposalHandler = govclient.NewProposalHandler(cli.NewProposalCmdUpdateConsortiumParams, rest.DummyRESTHandler)
var UpdateValidatorAuthsProposalHandler = govclient.NewProposalHandler(cli.NewProposalCmdUpdateValidatorAuths, rest.DummyRESTHandler)
17 changes: 17 additions & 0 deletions x/consortium/client/testutil/cli_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// +build norace

package testutil

import (
"testing"

"github.com/stretchr/testify/suite"

"github.com/line/lbm-sdk/testutil/network"
)

func TestIntegrationTestSuite(t *testing.T) {
cfg := network.DefaultConfig()
cfg.NumValidators = 1
suite.Run(t, NewIntegrationTestSuite(cfg))
}
Loading