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

Update to node 0.9.12 #70

Closed
wants to merge 9 commits into from
Closed

Conversation

nzpr
Copy link

@nzpr nzpr commented Jul 31, 2019

No description provided.

@nzpr nzpr requested review from dckc and JoshOrndorff July 31, 2019 08:24
@dckc
Copy link
Collaborator

dckc commented Jul 31, 2019

Looks pretty good so far. Are you working on the RHOCore unit test failures?

@nzpr
Copy link
Author

nzpr commented Jul 31, 2019

@dckc yes, looking into it, my current guess that this is caused by changing GPrivate to GUnforgeable in RhoTypes.proto.

@dckc
Copy link
Collaborator

dckc commented Aug 3, 2019

@nutzipper you're right that the code builds DeployService two different ways: one at "compile" time and one at "run" time. I have work-in-progress to switch to all compile time (statically generated code)

`${host}:${port}`, grpc.credentials.createInsecure(), // ISSUE: let caller do secure?
);

const proposeProto = grpc.loadPackageDefinition(proposeServiceDefinition);
const proposeClient /*: ProposeService */ = new proposeProto.coop.rchain.casper.protocol.ProposeService(
Copy link
Member

Choose a reason for hiding this comment

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

@nzpr strange thing, in some cases, when ProposeService is undefined, DeployService exist in the same namespace. I couldn't fine where is the problem.

Tests are passing with something like this.

const ProposeServiceFixed =
  proposeProto.coop.rchain.casper.protocol.ProposeService || 
  proposeProto.coop.rchain.casper.protocol.DeployService;

Copy link
Author

Choose a reason for hiding this comment

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

@dckc @tgrospic Digging into static codegen, this convo grpc/grpc-node#528 makes me think that static code generated with protobufjs library that rnode-api was using does not allow to actually call gRPC methods.

While protobufjs CAN generate code for service definition, it can NOT be used for gRPC, because it'd be lacking a lot of features.

So I'm trying to use protoc compiler from https://github.com/grpc/grpc-node.
Or maybe its a good idea to use runtime proto loader.

Copy link
Member

Choose a reason for hiding this comment

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

@nzpr I think you right, protobufjs is just a wrapper around protoc and needs protocol implementation for communication. I tried static generated js with grpc-node but I couldn't find a way to create correct rpc implementation.

There is an example which uses grpc-tools but I couldn't make it work to generate grpc (service) part.
https://github.com/grpc/grpc/tree/master/examples/node/static_codegen

But I have some good news also. 🎉 With grpc-web, Envoy proxy and protoc (with web plugin) I managed to successfully connect to RNode from the browser.
Now I see your point with Either type which holds a value of type byte array. Which is like untyped because it must be deserialized to correct type. And this (generic) type of Either is written in comments in proto files e.g. DeployService.

Copy link
Author

Choose a reason for hiding this comment

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

Thats is cool!. 🎉
I switched to static codegen here https://github.com/nzpr/rchain-api/tree/static, and successfully deployed/proposed on live RNode. Try npm run integrationTest to check. But I'm struggling with RHOCore part now and some other problems emerged.

Copy link
Author

@nzpr nzpr Aug 19, 2019

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. Now I see that grpc-tools is nodejs version of protoc which accepts the same arguments and generates the same code. Works with web plugin also.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@nzpr

@dckc @tgrospic Digging into static codegen, this convo grpc/grpc-node#528 makes me think that static code generated with protobufjs library that rnode-api was using does not allow to actually call gRPC methods.

While protobufjs CAN generate code for service definition, it can NOT be used for gRPC, because it'd be lacking a lot of features.

Odd. protobufjs and grpc get along pretty well, in my experience. protobufjs only does serialization, not the RPC part. But I haven't seen any problems with that arrangement.

So I'm trying to use protoc compiler from https://github.com/grpc/grpc-node.

I tried that. I don't recall why, but I found that it conflicted with my design ideas.

@nzpr nzpr changed the title Update to node 0.9.11 Update to node 0.9.12 Aug 19, 2019
@dckc
Copy link
Collaborator

dckc commented Aug 24, 2019

I tried to carry this PR forward but I'm losing:
https://github.com/dckc/RChain-API/tree/v0.9.12-wip 50eeb61

npm run integrationTest gives:

# smart contract deploy
not ok 5 13 INTERNAL: Received RST_STREAM with error code 2

@dckc
Copy link
Collaborator

dckc commented Aug 24, 2019

I just pushed some earlier work: https://github.com/dckc/RChain-API/tree/rnode-0.9 088ac9c Apr 20.

The diagnostic from this version at least makes sense:

# smart contract deploy
not ok 5 12 UNIMPLEMENTED: Method not found: coop.rchain.casper.protocol.DeployService/createBlock

@dckc
Copy link
Collaborator

dckc commented Aug 24, 2019

still not winning, but maybe getting closer...

https://github.com/dckc/RChain-API/tree/rnode-0.9 2903258

current symptoms:

# smart contract deploy
...
not ok 5 $root.coop.rchain.casper.protocol.DeployServiceResponse is not a constructor

looks similar to protobufjs/protobuf.js#860
and perhaps protobufjs/protobuf.js#1128

@dckc
Copy link
Collaborator

dckc commented Oct 5, 2020

no longer supporting gRPC

@dckc dckc closed this Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants