Categorise chaincode operations by peer and gateway

[bestbeforetoday]

Previous implementation was standalone functions, with all requiring both a gRPC connection and a client signing identity. This required:

• An underisably large number of prameters.
• Unwritten conventions on parameter ordering.
• Documentation comments to highlight when the connection should be to a specific peer or to an organization gateway.
• Duplication of arguments across multiple functions for calling code driving the chaincode lifecycle.

This implementation defines a Peer and Gateway struct that hold a gRPC connection and a client identity. Each of these types defines methods appropriate to their logical role to:

• Avoid the need to provide gRPC connection and client identity arguments on every call.
• Provide calling code with logical context for what they are interacting with.
• Allow IDE code completion to suggest appropriate methods for the connection type.

For example, this pseudo-code flow:

chaincode.Install(ctx, peerConnection, id, chaincodePackage)
chaincode.Approve(ctx, gatewayConnection, id, chaincodeDefinition)
chaincode.Commit(ctx, gatewayConnection, id, chaincodeDefinition)

becomes:

peer.Install(ctx, chaincodePackage)
gateway.Approve(ctx, chaincodeDefinition)
gateway.Commit(ctx, chaincodeDefinition)

[bestbeforetoday]

@SamYuan1990 @samuelvenzi What do you think of this as an approach? It's been bugging me for a long time that so many parameters are duplicated across chaincode functions. This is an alternative approach. Both work so I'm interested in your perspective on whether it's worth making a change from what we have. Any change needs to happen before we hit a proper v1 release.

I actually included the service discovery PeerMembershipQuery on Gateway. We could go further down that route and associate all functionality (not just chaincode lifecycle) with appropriate node connections (Gateway, Peer, Orderer) and hide all the implementation code/packages, or we could keep each category of functionality separate, like we do currently with a different package for each concern.

[SamYuan1990]

I am good to see we simply the logic, but I have a question as chaincode.Install mapping to the same commend as peer chaincode install do we have any plan/idea to simply the peer CLI?

[bestbeforetoday]

There are definitely some areas where a replacement admin CLI can be simplified compared to the existing peer CLI. The key one is that, for all existing peer CLI commands that submit a transaction to the ledger (which means all the ones on Gateway in this PR), the user needs to specify sufficient endorsing peers and the orderer(s) to which the endorsed transaction will be submitted. The admin SDK implementation makes use of the peer’s Gateway service to handle these interactions and so only ever requires a connection to a single (Gateway) peer for these interactions. This remains true for a BFT ordering service since the Gateway service handles the different ordering requirements for the consensus implementation.

Chaincode install talks directly to a specific peer so doesn’t use the Gateway service. It might not be possible to simplify that one. You could argue for a chaincode install command that allows multiple target peers to be specified, but I think that is going to make it harder to handle failure scenarios compared to just having the user script (or code) their own flow to drive install on each of the desired peers.

[SamYuan1990]

Chaincode install talks directly to a specific peer so doesn’t use the Gateway service. It might not be possible to simplify that one. You could argue for a chaincode install command that allows multiple target peers to be specified, but I think that is going to make it harder to handle failure scenarios compared to just having the user script (or code) their own flow to drive install on each of the desired peers.

let's make it step by step, also we need to considering with existing user habits, this PR LGTM.

[samuelvenzi]

@bestbeforetoday Sorry it took me this long to respond. I def like this approach. I see that this is now a draft, but I'll keep an eye to check it again once it's ready.

[bestbeforetoday]

There are two design aspects to consider here:

1. The change to a struct with receiver functions instead of standalone functions. The struct encapsulate the common properties (gRPC connection and client identity) to avoid them being passed to every function.
2. The separation of "Peer" and "Gateway" functions. This is a purely logical separation to distinguish between invocation that affect only the specific peer (like chaincode install) and those that affect the wider network (like chaincode commit).

The changes to the e2e test best demonstrate what this means to end users.

The e2e tests maintained a ConnectionDetails struct within the test code, since these parameters were required for each chaincode invocation:

type ConnectionDetails struct {
	id         identity.SigningIdentity
	connection grpc.ClientConnInterface
}

This and all of the code to populate and make of it is removed since the Peer (and Gateway) structs within the API perform the same function:

org1Peer1 := chaincode.NewPeer(peer1Connection, org1MSP)
org1Gateway := chaincode.NewGateway(peer1Connection, org1MSP)

The chaincode function invocations are similarly simplified. For example:

result, err := chaincode.Install(ctx, target.connection, target.id, bytes.NewReader(chaincodePackage))

becomes:

result, err := peer.Install(ctx, bytes.NewReader(chaincodePackage))

Note that a similar change is made to the discovery package. The chaincode and discovery packages do not share a single Peer definition because the discovery service may require some additional configuration not shared by the chaincode functions.

The separation of Peer and Gateway structs is a matter for debate. Gateway peers are of course also peers so a single Peer struct could be used to provide all the chaincode functions. This would require fewer objects in the client application code. The question in my mind is whether the logical separation delivers value in guiding users to correctly use the API. Does having Peer and Gateway separate help clarify the scope of each of the operations for an application developer, or is the simplicity of a single Peer struct preferred.

If you are happy with the approach implemented here, the PR is ready to merge. If you prefer an alternative design, I'm happy to make changes. What do you think?

[SamYuan1990]

I am going to approve this PR, @samuelvenzi , any further comments?

[samuelvenzi]

@SamYuan1990 No further comments. The change makes the operation scope clearer