Using the CLI provided gRPC configurations when starting the chaincode

[delaooliveira]

in external mode (#318)

When utilizing the chaincode as a service, it is possible to pass some gRPC options as an argument, such as grpc.max_send_message_length. However, these server options were not utilized when declaring the server.
Resolves #318

[bestbeforetoday]

Thank you for the contribution. An inline comment and a few things that should be addressed:

• This change is exposed in the public API on the Shim.server() function in libraries/fabric-shim/lib/chaincode.js. The JSDoc for that function (specifically the ChaincodeServerOpts interface) should be updated to reflect the new behaviour. Off the top of my head, I am not sure the best (or if there even is) JSDoc syntax for expressing an arbitrary set of properties all beginning with a certain string.
• TypeScript type definitions in libraries/fabric-shim/types/index.d.ts need to be updated to reflect the API change. The ChaincodeServerOpts interface probably needs an index property added similar to:

  [grpcOption: `grpc.${string}`]: unknown;

If practical, it would be great to also have a unit test in libraries/fabric-shim/test/unit/server.js to confirm that gRPC options are passed to the grpc.Server() constructor.

[bestbeforetoday]

There are more options that can be passed to the grpc.Server constructor, and that list of options might change over time. Maybe just passing through any properties beginning with "grpc." would cover the possibilities more simply?

Suggested change

	this._server = new grpc.Server({
	'grpc.max_send_message_length': serverOpts['grpc.max_send_message_length'],
	'grpc.max_receive_message_length': serverOpts['grpc.max_receive_message_length'],
	'grpc.keepalive_time_ms': serverOpts['grpc.keepalive_time_ms'],
	'grpc.http2.min_time_between_pings_ms': serverOpts['grpc.http2.min_time_between_pings_ms'],
	'grpc.keepalive_timeout_ms': serverOpts['grpc.keepalive_timeout_ms'],
	'grpc.http2.max_pings_without_data': serverOpts['grpc.http2.max_pings_without_data'],
	'grpc.keepalive_permit_without_calls': serverOpts['grpc.keepalive_permit_without_calls'],
	});
	const grpcOptions = Object.fromEntries(
	Object.entries(serverOpts).filter(([key]) => key.startsWith('grpc.'))
	);
	this._server = new grpc.Server(grpcOptions);

[delaooliveira]

Hello Mark, I chose to be explicit on the entries for the object because those were the ones specified on fabric-shim/cmds/serverCommand.js under validOptions. I treated this as a bug, since any one of those weren't being passed to the gRPC Server.

I agree that there is discrepancies in the index.d.ts in regards to this ChaincodeServerOpts interface and the what serverCommand/startCommand.js are implying with their validOptions.

[bestbeforetoday]

To keep the code simple and the CLI command more easily extended to accommodate additional gRPC options, I think I would still be tempted to pass any property that starts with grpc. on to the gRPC Server, and let the CLI command worry about explicitly defining the options that the CLI command accepts. Whichever approach you prefer is OK with me though. Just the JSDoc and TypeScript type definitions need to match the implementation.

[delaooliveira]

Hello @bestbeforetoday,
So, I was trying to find a way to create a JSDoc that would fit and I didn't find any way to make it something like the index property of TS. To avoid duplicating every one of the options (and because they could change in the future), I thought we could do something like this:

`/**
 * @interface GRPCOptions
 * @description ChannelOptions on "@grpc/grpc-js". For a complete list, refer to <a href=https://www.npmjs.com/package/@grpc/grpc-js#supported-channel-options>@grpc/grpc-js's Documentation</a>
 * @property {unknown} ['grpc.${string}'] Connection options defined on "@grpc/grpc-js"
 */
/**
 * @interface ChaincodeServerTLSProperties
 * @property {Buffer} key Private key for TLS
 * @property {Buffer} cert Certificate for TLS
 * @property {Buffer} [clientCACerts] CA certificate for client certificates if mutual TLS is used.
 */
/**
 * @interface ChaincodeServerOpts
 * @extends GRPCOptions
 * @property {string} ccid Chaincode ID
 * @property {string} address Listen address for the server
 * @property {ChaincodeServerTLSProperties} [tlsProps] TLS properties if TLS is required.
 */
/**
 * Returns a new Chaincode server. Should be called when the chaincode is launched in a server mode.
 * @static
 * @param {ChaincodeInterface} chaincode User-provided object that must implement <code>ChaincodeInterface</code>
 * @param {ChaincodeServerOpts} serverOpts Chaincode server options
 */
static server(chaincode, serverOpts) {
    return new ChaincodeServer(chaincode, serverOpts);
}`

What do you think?

[bestbeforetoday]

It is annoying that the grpc-js GRPCOptions also contains the index signature [key: string]: any, which makes our type extending it less helpful for users since it won't flag up invalid or typos in property names as an error.

How about this as an approach?

Suggested change

	export interface ChaincodeServerOpts extends Partial<GRPCOptions> {
	type GRPCOptions = {
	[K in keyof ChannelOptions as string extends K ? never : K]?: ChannelOptions[K];
	}
	export interface ChaincodeServerOpts extends GRPCOptions {

I think it more accurately reflects the code behaviour, where only property names starting with grpc. are considered.

[bestbeforetoday]

I couldn't dig up any way of expressing the type in JSDoc so your solution looks OK to me. Just as long as it builds the documentation successfully and produces something that makes sense to somebody ready it. I guess the alternative is to explicitly list all (or perhaps a smaller set) of gRPC option property names. I suspect it's better to just reference the gRPC documentation, as you've done.