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

feat: x/cosmwasmpool proto and query boilerplate #4764

Merged
merged 9 commits into from
Mar 29, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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 go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.19
require (
github.com/CosmWasm/wasmd v0.30.0
github.com/cosmos/cosmos-proto v1.0.0-alpha8
github.com/cosmos/cosmos-sdk v0.46.11
github.com/cosmos/cosmos-sdk v0.47.1
github.com/cosmos/go-bip39 v1.0.0
github.com/cosmos/ibc-go/v4 v4.3.0
github.com/gogo/protobuf v1.3.3
Expand Down
19 changes: 19 additions & 0 deletions proto/osmosis/cosmwasmpool/v1beta1/genesis.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
syntax = "proto3";
package osmosis.cosmwasmpool.v1beta1;

import "gogoproto/gogo.proto";
import "google/protobuf/any.proto";
import "cosmos_proto/cosmos.proto";
import "google/protobuf/duration.proto";
import "cosmos/base/v1beta1/coin.proto";

option go_package = "github.com/osmosis-labs/osmosis/v15/x/cosmwasmpool/types";

// Params holds parameters for the cosmwasmpool module
message Params {}

// GenesisState defines the cosmwasmpool module's genesis state.
message GenesisState {
// params is the container of cosmwasmpool parameters.
Params params = 1 [ (gogoproto.nullable) = false ];
}
22 changes: 22 additions & 0 deletions proto/osmosis/cosmwasmpool/v1beta1/model/pool.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
syntax = "proto3";

package osmosis.cosmwasmpool.v1beta1;

import "cosmos_proto/cosmos.proto";
import "gogoproto/gogo.proto";
import "google/protobuf/timestamp.proto";

option go_package = "github.com/osmosis-labs/osmosis/v15/x/cosmwasmpool/model";

message CosmWasmPool {
option (gogoproto.goproto_getters) = false;
option (gogoproto.goproto_stringer) = false;
option (cosmos_proto.implements_interface) = "PoolI";
string pool_address = 1 [ (gogoproto.moretags) = "yaml:\"pool_address\"" ];
string contract_address = 2
[ (gogoproto.moretags) = "yaml:\"contract_address\"" ];
uint64 pool_id = 3;
uint64 code_id = 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to store the code_id? It can be retrieved from the ContractInfo (using the addr) once the contract is instantiated.

Copy link
Member

Choose a reason for hiding this comment

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

Doesn't storing it here prevent one more query in the required path though?

Copy link
Member Author

Choose a reason for hiding this comment

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

Doesn't storing it here prevent one more query in the required path though?

I think it does.

Also, I think we might want to store the code id for security purposes so that it is impossible to upgrade the contract to something malicious.

We can allow this to be upgradeable via gov prop. WDYT?

Going to make an issue to track this discussion

Copy link
Member Author

Choose a reason for hiding this comment

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

Tracking discussion here: #4779

Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't storing it here prevent one more query in the required path though?

Only when the code id is being used, which is only on instantiation (AFAICT). Not sure when else we would need the code id.

Also, I think we might want to store the code id for security purposes so that it is impossible to upgrade the contract to something malicious.

We can make the contract non-upgradable by giving it no admin.

Also if we wanted to enforce this by checking the code id, we would need to query the ContractInfo on every call to see that it matches what is stored.

bytes instantiate_msg = 5
p0mvn marked this conversation as resolved.
Show resolved Hide resolved
[ (gogoproto.moretags) = "yaml:\"instantiate_msg\"" ];
}
25 changes: 25 additions & 0 deletions proto/osmosis/cosmwasmpool/v1beta1/model/tx.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
syntax = "proto3";
package osmosis.cosmwasmpool.v1beta1;

import "gogoproto/gogo.proto";
import "cosmos/base/v1beta1/coin.proto";

option go_package = "github.com/osmosis-labs/osmosis/v15/x/cosmwasmpool/model";

service MsgCreator {
rpc CreateCosmWasmPool(MsgCreateCosmWasmPool)
returns (MsgCreateCosmWasmPoolResponse);
}

// ===================== MsgCreateCosmwasmPool
message MsgCreateCosmWasmPool {
uint64 code_id = 1 [ (gogoproto.moretags) = "yaml:\"code_id\"" ];
bytes instantiate_msg = 2
[ (gogoproto.moretags) = "yaml:\"instantiate_msg\"" ];
string sender = 3 [ (gogoproto.moretags) = "yaml:\"sender\"" ];
}

// Returns a unique poolID to identify the pool with.
message MsgCreateCosmWasmPoolResponse {
uint64 pool_id = 1 [ (gogoproto.customname) = "PoolID" ];
}
25 changes: 25 additions & 0 deletions proto/osmosis/cosmwasmpool/v1beta1/query.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
syntax = "proto3";
package osmosis.cosmwasmpool.v1beta1;

import "gogoproto/gogo.proto";
import "osmosis/cosmwasmpool/v1beta1/genesis.proto";
import "osmosis/cosmwasmpool/v1beta1/tx.proto";

import "cosmos/base/v1beta1/coin.proto";
import "cosmos/base/query/v1beta1/pagination.proto";
import "google/api/annotations.proto";
import "google/protobuf/any.proto";
import "cosmos_proto/cosmos.proto";
import "google/protobuf/timestamp.proto";

option go_package = "github.com/osmosis-labs/osmosis/v15/x/cosmwasmpool/client/queryproto";

service Query {
rpc Params(ParamsRequest) returns (ParamsResponse) {
option (google.api.http).get = "/osmosis/cosmwasmpool/v1beta1/Params";
}
}

//=============================== Params
message ParamsRequest {}
message ParamsResponse { Params params = 1 [ (gogoproto.nullable) = false ]; }
10 changes: 10 additions & 0 deletions proto/osmosis/cosmwasmpool/v1beta1/query.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
keeper:
path: "github.com/osmosis-labs/osmosis/v15/x/cosmwasmpool"
struct: "Keeper"
client_path: "github.com/osmosis-labs/osmosis/v15/x/cosmwasmpool/client"
queries:
Params:
proto_wrapper:
query_func: "k.GetParams"
cli:
cmd: "GetParams"
9 changes: 9 additions & 0 deletions proto/osmosis/cosmwasmpool/v1beta1/tx.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
syntax = "proto3";
package osmosis.cosmwasmpool.v1beta1;

import "gogoproto/gogo.proto";
import "cosmos/base/v1beta1/coin.proto";

option go_package = "github.com/osmosis-labs/osmosis/v15/x/cosmwasmpool/types";

service Msg {}
Copy link
Member Author

@p0mvn p0mvn Mar 28, 2023

Choose a reason for hiding this comment

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

Note to reviewer revisit having messages is tracked in: #4765

3 changes: 3 additions & 0 deletions proto/osmosis/poolmanager/v1beta1/module_route.proto
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ enum PoolType {
// Concentrated is the pool model specific to concentrated liquidity. It is
// defined in x/concentrated-liquidity.
Concentrated = 2;
// CosmWasm is the pool model specific to CosmWasm. It is defined in
// x/cosmwasmpool.
CosmWasm = 3;
}

// ModuleRouter defines a route encapsulating pool type.
Expand Down
7 changes: 4 additions & 3 deletions proto/osmosis/poolmanager/v1beta1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ service Query {
rpc EstimateSinglePoolSwapExactAmountIn(
EstimateSinglePoolSwapExactAmountInRequest)
returns (EstimateSwapExactAmountInResponse) {
option (google.api.http).get =
"/osmosis/poolmanager/v1beta1/{pool_id}/estimate/single_pool_swap_exact_amount_in";
option (google.api.http).get = "/osmosis/poolmanager/v1beta1/{pool_id}/"
"estimate/single_pool_swap_exact_amount_in";
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be one line. The linter splits it, but apparently protobufs don't support that. ref #4549 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Is this confirmed? Would be really odd if the linter did something that wasn't proto compatible...

Copy link
Member Author

@p0mvn p0mvn Mar 29, 2023

Choose a reason for hiding this comment

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

I'm also not convinced this is a problem. I searched the repo and there are multiple places where this is happening.

If this is indeed an issue, we should investigate fixing the linter IMO as it is almost guaranteed it will get missed in one of the PRs

Going to follow-up on Slack

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, not sure. Just going off what Matt said. @mattverse have you confirmed that stuff breaks here? what breaks?

Copy link
Member

Choose a reason for hiding this comment

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

From what I have manually tested, I have came to a conclusion that these multiple liners are not correctly supported when they are getting converted to rest endpoints.

On the second look, it might have been just me having effect by other testing variants. Worthwhile re-testing, cannot assure that its 100% not supported

}

// Estimates swap amount in given out.
Expand All @@ -45,7 +45,8 @@ service Query {
EstimateSinglePoolSwapExactAmountOutRequest)
returns (EstimateSwapExactAmountOutResponse) {
option (google.api.http).get =
"/osmosis/poolmanager/v1beta1/{pool_id}/estimate_out/single_pool_swap_exact_amount_out";
"/osmosis/poolmanager/v1beta1/{pool_id}/estimate_out/"
p0mvn marked this conversation as resolved.
Show resolved Hide resolved
"single_pool_swap_exact_amount_out";
}

// Returns the total number of pools existing in Osmosis.
Expand Down
32 changes: 32 additions & 0 deletions x/cosmwasmpool/client/grpc/grpc_query.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package grpc

// THIS FILE IS GENERATED CODE, DO NOT EDIT
// SOURCE AT `proto/osmosis/cosmwasmpool/v1beta1/query.yml`

import (
context "context"

"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/osmosis-labs/osmosis/v15/x/cosmwasmpool/client"
"github.com/osmosis-labs/osmosis/v15/x/cosmwasmpool/client/queryproto"
)

type Querier struct {
Q client.Querier
}

var _ queryproto.QueryServer = Querier{}

func (q Querier) Params(grpcCtx context.Context,
req *queryproto.ParamsRequest,
) (*queryproto.ParamsResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "empty request")
}
ctx := sdk.UnwrapSDKContext(grpcCtx)
return q.Q.Params(ctx, *req)
}

25 changes: 25 additions & 0 deletions x/cosmwasmpool/client/query_proto_wrap.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package client

import (
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/osmosis-labs/osmosis/v15/x/cosmwasmpool"
"github.com/osmosis-labs/osmosis/v15/x/cosmwasmpool/client/queryproto"
)

// This file should evolve to being code gen'd, off of `proto/poolmanager/v1beta/query.yml`

type Querier struct {
K cosmwasmpool.Keeper
}

func NewQuerier(k cosmwasmpool.Keeper) Querier {
return Querier{k}
}

func (q Querier) Params(ctx sdk.Context,
req queryproto.ParamsRequest,
) (*queryproto.ParamsResponse, error) {
params := q.K.GetParams(ctx)
return &queryproto.ParamsResponse{Params: params}, nil
}
Loading