Cosmos-sdk: Makefile protobuf commands (proto-update-deps, proto-gen,..) not working

[dauTT]

Summary of Bug

Many protobuf commands in the MakeFile are not working. For instance

make proto-update-deps
make proto-gen
make proto-all

Version

master branch, commitID: 4b0c66982a87e824d43873187ae049a9f572ed7a

Steps to Reproduce

OS: Ubuntu 18.04
GO version: go1.14.3

---

For Admin Use

• [ ] Not duplicate issue
• [ ] Appropriate labels applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

[anilCSE]

@dauTT I couldn't reproduce the issue, make proto-gen working fine for me. I am using Ubuntu 18.04, Go 1.14. Can you try the command directly from the terminal once?

[fedekunze]

try running make tools

[dauTT]

@fedekunze , no it does not work.

I also tried following the contribution guideline (CONTRIBUTING.md) but unsuccessful :

To generate the protobuf stubs you must have protoc and protoc-gen-gocosmos installed. To install these tools run make protoc & make protoc-gen-gocosmos. After this step you will be able to run make proto-gen to generate the protobuf stubs

[dauTT]

@anilCSE , I believe you have all the tools installed. Therefore you can run make proto-gen.

I think, I have some missing tools but don't know how to fix that.

[marbar3778]

it seems you actually have an error in one of the proto files. Is this you pulling from master and just running the cmd or altering somethings

The current proto development process has quite poor ux. A developer should need to download everything. In tendermint we offer two options, download everything or use docker. This should be offered in the sdk

[dauTT]

thanks, @marbar3778. for the explanation. You are right.

from a clean master, running only make proto-gen will work.
But from a clean master, running make proto-update-deps and make proto-gen won't because the first command change a proto file in the master branch:

[marbar3778]

could you try pulling the most recent master? There were some changes that may have affected this. I'm not sure what is going on here as I can't reproduce it.

[dauTT]

Hi @marbar3778 ,

with the latest in master branch (commitID: 6700d776b82bbb4f756315858eadc69298313528 ), I still get an error when I run make proto-update-deps:

[marbar3778]

Hey, this is partially fixed in a pr I made and the update to latest Tendermint will close this.

I wasnt able to debug what your issue was tho. in the future sed won't be needed

[marbar3778]

this has been fixed

[dauTT]

Hi @marbar3778 ,

can we re-open this issue? I have try with the latest commit in the master branch (commit: f3c6ed61b433871427bf54b884a52be1e3dca328) and it is not working:

[marbar3778]

seems like the tool is for grpc-gateway is not downloaded.

I run into different issues on master, we can reopen this

[dauTT]

For me, the proto commands were working fine with commit 79cee0696

Something has changed in the lastest commit.

[colin-axner]

another relevant issue that happened recently is the proto-tools make file checks if you have the dependency installed but not that you have the correct version. My versioning seems to be off for generating gateway files, but none of the make file commands will fix it

If anyone know what might be wrong with my setup, I'd really like to have it fixed:

protoc-gen-grpc-gateway --version
Version 1.14.7, commit 37837bc5882204897dfb5b17ee231fc645c9102d, built at 2020-08-12T08:49:14Z
protoc --version
libprotoc 3.13.0

[marbar3778]

Hmm, we should be keeping up with the latest versions. What is the error you are observing when running the script?

[colin-axner]

        modified:   client/grpc/reflection/reflection.pb.gw.go
    modified:   client/grpc/simulate/simulate.pb.gw.go
    modified:   x/auth/types/query.pb.gw.go
    modified:   x/bank/types/query.pb.gw.go
    modified:   x/distribution/types/query.pb.gw.go
    modified:   x/evidence/types/query.pb.gw.go
    modified:   x/gov/types/query.pb.gw.go
    modified:   x/ibc-transfer/types/query.pb.gw.go
    modified:   x/ibc/02-client/types/query.pb.gw.go
    modified:   x/ibc/03-connection/types/query.pb.gw.go
    modified:   x/ibc/04-channel/types/query.pb.gw.go
    modified:   x/mint/types/query.pb.gw.go
    modified:   x/params/types/proposal/query.pb.gw.go
    modified:   x/slashing/types/query.pb.gw.go
    modified:   x/staking/types/query.pb.gw.go
    modified:   x/staking/types/staking.pb.go
    modified:   x/upgrade/types/query.pb.gw.go

all these files get modified just running off of master. I usually run make proto-all add my specific modified files and then reset all these files. But it also means any of my prs modifying query files won't properly update the gateway files

[marbar3778]

hmm, what are the changes within these files? Are they all the same changes?

[colin-axner]

Oh wow, it is just an inserted comment into each file:

git diff 4faeefebd2abc50f89b02daf836f2edb77f61288
diff --git a/client/grpc/reflection/reflection.pb.gw.go b/client/grpc/reflection/reflection.pb.gw.go
index b3e8c4641..ab486750e 100644
--- a/client/grpc/reflection/reflection.pb.gw.go
+++ b/client/grpc/reflection/reflection.pb.gw.go
@@ -106,6 +106,7 @@ func local_request_ReflectionService_ListImplementations_0(ctx context.Context,
 // RegisterReflectionServiceHandlerServer registers the http handlers for service ReflectionService to "mux".
 // UnaryRPC     :call ReflectionServiceServer directly.
 // StreamingRPC :currently unsupported pending https://github.com/grpc/grpc-go/issues/906.
+// Note that using this registration option will cause many gRPC library features (such as grpc.SendHeader, etc) to stop working. Consider using RegisterReflectionServiceHandlerFromEndpoint instead.
 func RegisterReflectionServiceHandlerServer(ctx context.Context, mux *runtime.ServeMux, server ReflectionServiceServer) error {

        mux.Handle("GET", pattern_ReflectionService_ListAllInterfaces_0, func(w http.ResponseWriter, req *http.Request, pathParams map[string]string) {

where is this coming from?

[colin-axner]

I see the comment in v.14.7 release. Which tells me master is not on this version. I think it'd be good to add a check in the makefile to make sure the correct version is installed

[fedekunze]

I also have the same issue that @colin-axner described

[alexanderbez]

I have:

➜ protoc-gen-grpc-gateway -version
Version 1.14.7, commit 37837bc5882204897dfb5b17ee231fc645c9102d, built at 2020-08-12T08:49:14Z

Gives me the following diff:

+// Note that using this registration option will cause many gRPC library features (such as grpc.SendHeader, etc) to stop working. Consider using RegisterReflectionServiceHandlerFromEndpoint instead.

[anilCSE]

I have the same updates to proto generated files. We should enforce version in make file to avoid conflicts.

[alexanderbez]

I have the same updates to proto generated files. We should enforce version in make file to avoid conflicts.

1.14.7

[colin-axner]

Another issue/nit I've had with make proto-gen and subsequently make proto-all, if you run it with a broken .proto file, it leaves the github.com folder behind because it errors before cleanup.

[colin-axner]

I'm seeing issues similar to above again except with .pb.go files. My versions remain unchanged and this behaviour is observed after running make proto-all on the latest commit:

modified:   client/grpc/reflection/reflection.pb.go
    modified:   client/grpc/simulate/simulate.pb.go
    modified:   testutil/testdata/proto.pb.go
    modified:   types/abci.pb.go
    modified:   types/coin.pb.go
    modified:   types/tx/tx.pb.go
    modified:   x/auth/types/query.pb.go
    modified:   x/auth/vesting/types/tx.pb.go
    modified:   x/auth/vesting/types/vesting.pb.go
    modified:   x/bank/types/bank.pb.go
    modified:   x/bank/types/genesis.pb.go
    modified:   x/bank/types/query.pb.go
    modified:   x/bank/types/tx.pb.go
    modified:   x/distribution/types/distribution.pb.go
    modified:   x/distribution/types/genesis.pb.go
    modified:   x/distribution/types/query.pb.go
    modified:   x/distribution/types/tx.pb.go
    modified:   x/evidence/types/query.pb.go
    modified:   x/gov/types/genesis.pb.go
    modified:   x/gov/types/gov.pb.go
    modified:   x/gov/types/query.pb.go
    modified:   x/gov/types/tx.pb.go
    modified:   x/ibc-transfer/types/genesis.pb.go
    modified:   x/ibc-transfer/types/query.pb.go
    modified:   x/ibc/02-client/types/genesis.pb.go
    modified:   x/ibc/02-client/types/query.pb.go
    modified:   x/ibc/03-connection/types/query.pb.go
    modified:   x/ibc/04-channel/types/query.pb.go
    modified:   x/mint/types/mint.pb.go
    modified:   x/mint/types/query.pb.go
    modified:   x/params/types/proposal/query.pb.go
    modified:   x/slashing/types/query.pb.go
    modified:   x/staking/types/query.pb.go
    modified:   x/staking/types/staking.pb.go
    modified:   x/staking/types/tx.pb.go
    modified:   x/upgrade/types/query.pb.go

example diff:

diff --git a/client/grpc/reflection/reflection.pb.go b/client/grpc/reflection/reflection.pb.go
index 99878b2a3..c508154bd 100644
--- a/client/grpc/reflection/reflection.pb.go
+++ b/client/grpc/reflection/reflection.pb.go
@@ -6,7 +6,6 @@ package reflection
 import (
        context "context"
        fmt "fmt"
-       grpc1 "github.com/gogo/protobuf/grpc"
        proto "github.com/gogo/protobuf/proto"
        _ "google.golang.org/genproto/googleapis/api/annotations"
        grpc "google.golang.org/grpc"
@@ -265,10 +264,10 @@ type ReflectionServiceClient interface {
 }

 type reflectionServiceClient struct {
-       cc grpc1.ClientConn
+       cc *grpc.ClientConn
 }

-func NewReflectionServiceClient(cc grpc1.ClientConn) ReflectionServiceClient {
+func NewReflectionServiceClient(cc *grpc.ClientConn) ReflectionServiceClient {
        return &reflectionServiceClient{cc}
 }

@@ -311,7 +310,7 @@ func (*UnimplementedReflectionServiceServer) ListImplementations(ctx context.Con
        return nil, status.Errorf(codes.Unimplemented, "method ListImplementations not implemented")
 }

-func RegisterReflectionServiceServer(s grpc1.Server, srv ReflectionServiceServer) {
+func RegisterReflectionServiceServer(s *grpc.Server, srv ReflectionServiceServer) {
        s.RegisterService(&_ReflectionService_serviceDesc, srv)
 }

diff --git a/client/grpc/simulate/simulate.pb.go b/client/grpc/simulate/simulate.pb.go
index 8fe707913..ba0e7496d 100644
--- a/client/grpc/simulate/simulate.pb.go
+++ b/client/grpc/simulate/simulate.pb.go
@@ -8,7 +8,6 @@ import (
        fmt "fmt"
        types "github.com/cosmos/cosmos-sdk/types"
        tx "github.com/cosmos/cosmos-sdk/types/tx"
-       grpc1 "github.com/gogo/protobuf/grpc"
        proto "github.com/gogo/protobuf/proto"
        _ "google.golang.org/genproto/googleapis/api/annotations"
        grpc "google.golang.org/grpc"
@@ -185,10 +184,10 @@ type SimulateServiceClient interface {
 }

 type simulateServiceClient struct {
-       cc grpc1.ClientConn
+       cc *grpc.ClientConn
 }

not discarding the above changes also results in

go test

# github.com/cosmos/cosmos-sdk/types
types/coin.pb.go:300:8: invalid argument m.Amount (type Int) for len
types/coin.pb.go:301:11: invalid argument m.Amount (type Int) for len
types/coin.pb.go:302:7: second argument to copy should be slice or string; have struct { i *big.Int }
types/coin.pb.go:303:43: invalid argument m.Amount (type Int) for len
types/coin.pb.go:337:8: invalid argument m.Amount (type Dec) for len
types/coin.pb.go:338:11: invalid argument m.Amount (type Dec) for len
types/coin.pb.go:339:7: second argument to copy should be slice or string; have struct { i *big.Int }
types/coin.pb.go:340:43: invalid argument m.Amount (type Dec) for len
types/coin.pb.go:374:8: invalid argument m.Int (type Int) for len
types/coin.pb.go:375:11: invalid argument m.Int (type Int) for len
types/coin.pb.go:375:11: too many errors

[alexanderbez]

You must have a discrepancy in your binary version(s). We need to ensure those targets run against very specific versions of binary deps.