fix(proto): use 64 bit chan_id ints as strings

[sangaman]

This adds jstype = JS_STRING annotations on the channel ids in the lnd proto definition so that they use string types instead of numbers. Javascript does not natively support 64 bit numbers and precision can be lost, and the channel ids are known to cause issues because of this.

This also adds a new package to generate type definitions from the .proto files. This is because of a known bug in the previously used plugin, agreatfool/grpc_tools_node_protoc_ts#10, which does not update the generated type definition when jstype = JS_STRING is used.

I hit a wall with my first approach of generating code and type definitions using the protobuf.js package. I found out the hard way that does not support generating the grpc server/client code and type definitions, as discussed in grpc/grpc-node#528 and protobufjs/protobuf.js#1017. Fortunately I found this other plugin for generating type definitions from proto files, and that seems to work.

Having to update the lnd proto definition manually to add jstype = JS_STRING is not ideal, but it's at least a temporary solution until the grpc/protobuf tools and packages for javascript come up with a better approach. I may also open a PR with lnd to add these annotations to chan_id at least.

Fixes #745.

[moshababo]

Where is this new plugin being used? it supposed to replace grpc_tools_node_protoc_plugin/bin/protoc-gen-ts, no?

[sangaman]

It does replace it, it's used in the proto scripts, but it has the same filename which is why the scripts didn't change. I'd have removed the grpc-tools package but we still need it for the grpc_tools_node_protoc_plugin plugin to generate the service definitions.

[offerm]

Good news - The change is working fine.

Side effect - when I checkout (on my mac) this branch and run npm i and npm run proto

I see this

admins-MacBook-Pro-2:xud admin$ git status
On branch proto-64-bit
Your branch is up to date with 'origin/proto-64-bit'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   lib/proto/annotations_pb.d.ts
	modified:   lib/proto/google/api/http_pb.d.ts
	modified:   lib/proto/google/protobuf/descriptor_pb.d.ts
	modified:   lib/proto/hash_resolver_pb.d.ts
	modified:   lib/proto/lndrpc_pb.d.ts
	modified:   lib/proto/xudrpc.swagger.json
	modified:   lib/proto/xudrpc_pb.d.ts

no changes added to commit (use "git add" and/or "git commit -a")

I recall that we had this problem in the past and it was solved by removing this package.....

[kilrau]

Nice!

[sangaman]

@offerm I removed the grpc_tools_node_protoc_ts package because it looked like it had a conflict on the file in the /.bin folder we're using, and it looks like we're not longer using this. Can you confirm this issue goes away for you? I didn't get it even when rerunning npm i but I'm guessing there's something slightly different in our environments. Apparently lack of determinism when it comes to bin file name collisions is a known complication in npm: npm/npm#9040.

[offerm]

@daniel, you did force push?

I took the last branch. npm run proto is not working any more. getting an error.

Also, there are many many changes. please check, mainly in the lock file.

[sangaman]

@offerm Yes I did to keep it in one commit - all I did was remove that package but I'll keep commits separate going forward until this is approved.

Can you try running npm install again and if you're still getting the error, npm rebuild?

If you're seeing changes in the lock file after installing, can you make sure you're on at least npm 6.4? npm update -g npm might work if not.

(Also I think you tagged the wrong person in the last comment)

[offerm]

@sangaman I removed the node_modules directory and run npm i and npm run compile. I don't see local changes now.
npm run proto still fails with

admins-MacBook-Pro-2:xud admin$ npm run proto

> [email protected] proto /Users/admin/xud-simnet/xud
> cross-os proto && cross-os swagger && cross-os protodocs && cross-os protoFixDeprecation

node_modules/grpc_tools_node_protoc_ts/bin/protoc-gen-ts: program not found or is not executable
--ts_out: protoc-gen-ts: Plugin failed with status code 1.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] proto: `cross-os proto && cross-os swagger && cross-os protodocs && cross-os protoFixDeprecation`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] proto script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/admin/.npm/_logs/2018-12-17T21_48_14_885Z-debug.log

The fix itself is working.

You can see the proto problem on XUD1 or XUD3:
login:

offerm@xud-test-3:~$ sudo su - xud
xud@xud-test-3:~$ cd /opt/xud/
xud@xud-test-3:/opt/xud$ npm run proto
> [email protected] proto /opt/xud
> cross-os proto && cross-os swagger && cross-os protodocs && cross-os protoFixDeprecation
node_modules/grpc_tools_node_protoc_ts/bin/protoc-gen-ts: program not found or is not executable
--ts_out: protoc-gen-ts: Plugin failed with status code 1.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] proto: `cross-os proto && cross-os swagger && cross-os protodocs && cross-os protoFixDeprecation`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] proto script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR!     /home/xud/.npm/_logs/2018-12-17T21_52_50_754Z-debug.log

[sangaman]

Thanks @offerm, I didn't notice that the linux/mac scripts point to the sepcific package directory rather than the general .bin folder. I've fixed that and tested it on linux with success.

[offerm]

@sangaman Mac is OK now but Linux still has a problem:

xud@xud-test-3:/opt/xud$ npm i
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: [email protected] (node_modules/fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for [email protected]: wanted {"os":"darwin","arch":"any"} (current: {"os":"linux","arch":"x64"})
audited 8418 packages in 8.119s
found 8 moderate severity vulnerabilities
  run `npm audit fix` to fix them, or `npm audit` for details
xud@xud-test-3:/opt/xud$ npm run proto
> [email protected] proto /opt/xud
> cross-os proto && cross-os swagger && cross-os protodocs && cross-os protoFixDeprecation
protoc-gen-swagger: program not found or is not executable
--swagger_out: protoc-gen-swagger: Plugin failed with status code 1.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] proto: `cross-os proto && cross-os swagger && cross-os protodocs && cross-os protoFixDeprecation`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] proto script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR!     /home/xud/.npm/_logs/2018-12-18T09_01_39_216Z-debug.log

[sangaman]

protoc-gen-swagger: program not found or is not executable
--swagger_out: protoc-gen-swagger: Plugin failed with status code 1.

That suggests to me that you're either missing the protoc-gen-swagger plugin on that machine, or the go/bin folder is not in your path. You can sanity check this by running npm run proto against master on the same machine. You should be able to fix this by running go get -u github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger and making sure the go binaries are in your path.

[offerm]

@sangaman I will need more time to properly check the proto.

Since #745 is a critical bug, and this PR solves it, I suggest that we will merge to master and in parallel continue checking this as part of a separate issue.

[sangaman]

Sounds good to me, I've gotten this to work on linux so I'm comfortable merging and yes as you say if we see any more issues we can fix separately. Thanks.

[kilrau]

Great, testing this now in my demo