Use new (non-deprecated) protocol buffer bindings

[bestbeforetoday]

As a user of Fabric
I want Fabric v3 to use fabric-protos Go bindings based on protocol buffer APIv2
So that Fabric is using actively maintained and non-deprecated protobuf APIs

In early 2020, a new Go API for protocol buffers was released, and the older protocol buffer API was deprecated.

• https://go.dev/blog/protobuf-apiv2
• Deprecated API: https://pkg.go.dev/github.com/golang/protobuf
• New API: https://pkg.go.dev/google.golang.org/protobuf

The two API versions produce 100% wire-level compatible protobuf messages so they can be used interchangeably with no compatibility implications for client applications, SDKs or other Fabric components. This has already been proven in the Fabric Gateway client API, which migrated seamlessly from the old to new protocol buffer API and maintained complete compatibility with Fabric.

Pre-built Go language bindings for the Fabric protobufs are already published based on both the old (fabric-protos-go) and new (fabric-protos-go-apiv2). The only work required is to change Fabric code to use the new protos package.

[denyeart]

Per https://github.com/hyperledger/fabric-protos/blob/main/RELEASING.md fabric-protos-go-apiv2 v0.3.0 is ready for use by fabric main branch.

[bestbeforetoday]

I briefly looked at this and it seems more involved than I'd hoped, particularly since migrating fabric-gateway was very easy (see commit). Other Fabric packages like fabric-config expose the older/deprecated protobuf package in their public API, which is a headache and likely a breaking change.

Since the old protobuf package got reimplemented using the newer protobuf package internally, you can translate between V1 and V2 protobuf Message structs, which might allow at least some changes to be made without breaking any public APIs. See this utility that used to be in fabric-gateway before it was migrated over entirely, which provided a few utility functions that can operate on both V1 and V2 messages by using the GeneratedMessage interface:

hyperledger/fabric-gateway/pkg/internal/util/protobuf.go

Ideally it would be good to entirely replace usage of the old protobuf API with the new one, and avoid juggling between the two message types, but taking advantage of the ability to transform message types might make it easier to migrate step-by-step instead of in one big bang.

[denyeart]

I would be in favor of updating all fabric repositories to APIv2 before Fabric v3 gets released (including Go chaincode repositories), if somebody tests and successfully proves that having a mixed set of Fabric v2.x orderers and peers (old protobuf) and new v3.x orderers and peers (new APIv2) works seamlessly. Any volunteers?

[jt-nti]

Getting everything on APIv2 for Fabric v3 would be great, although none of the modules involved follow the main Fabric versioning! :)

Working out what to do about versions for fabric-chaincode-go (currently pseudo-version numbers) and fabric-contract-api-go (currently v1.2.2) would be good. It seems like these would be breaking changes, so in theory v1 and v2 respectively would make sense...

https://go.dev/doc/modules/version-numbers

[davidkhala]

fabric-chaincode-go

I can help on fabric-chaincode-go v1 change dev.

[tobigiwa]

The sync.Mutex that was dragged into the apiV2 is the problem... function that takes it type and return it... are being flagged by go vet as copy of locked values.

[tobigiwa]

The solution was to introduce pointers (on such functions signatures, 2 of 'em actually, if I remember correctly), the effect cascaded and some other function signatures had to take pointers too (no internal logic was touched) ... everything was well in the patch(PR) I included...test passed, go vet laid to bed... BUT... it broke an Interface definition.

[tobigiwa]

@denyeart mentioned once a solution... I didn't quite figure it... Buh I'll try take another look at it again this week, hopefully it clicks.

[davidkhala]

While I also see the sync.Mutex warning issue, I guess it is inevitable to change some interface definition. I am fine with that since it will only affects v3 and onwards versions

[tobigiwa]

@davidkhala this would require a large community vote... maybe maintainers like @denyeart @bestbeforetoday @ryjones can shed light to that better.

[denyeart]

@tobigiwa Can you provide more details of the Interface definition that gets broken? If it impacts interoperability between v2 and v3 components then it would be a no-go.

[tobigiwa]

Sorry it took this long to respond, I'll get on it including a solution I have in mind. Pardon me again.

[denyeart]

@tobigiwa Any update?

[tobigiwa]

@denyeart Sincerely speaking, I thought I have responded to this already, Please pardon me.

So to the problem at hand, the correction would take place in fabric-chaincode-go itself, which then can be cascaded to fabric-contract-api-go then to fabric. I have updated my fix on fabric-chaincode-go to catch up with main branch.

Regarding the changes to the interfaces, here they are:
So it breaks to two interface, 3 method signature in total, by introduction of a pointer to the pb.Response (in the shim/interface.go file).

// Chaincode interface must be implemented by all chaincodes. The fabric runs
// the transactions by calling these functions as specified.
type Chaincode interface {
	// Init is called during Instantiate transaction after the chaincode container
	// has been established for the first time, allowing the chaincode to
	// initialize its internal data
	Init(stub ChaincodeStubInterface) *pb.Response

	// Invoke is called to update or query the ledger in a proposal transaction.
	// Updated state variables are not committed to the ledger until the
	// transaction is committed.
	Invoke(stub ChaincodeStubInterface) *pb.Response
}


type ChaincodeStubInterface interface {
InvokeChaincode(chaincodeName string, args [][]byte, channel string) *pb.Response
}

This alongside some function signatures too, I should add. Regarding the fix I was thinking, could not make it work, it was essentially a plan to fool go vet using generics regarding the copy lock values complaint. Using the new library as intended should be our solution, so no escaping those pointers.

[bestbeforetoday]

It seems like this might have highlighted an underlying issue with how protobuf messages are handled. There are cases where they are passed by value, whereas the protobuf packages are designed for message to be referred to with pointers, and are not safe to pass by value. Perhaps the right thing to do is bite the bullet and make the breaking API change for Fabric v3.

There were some good observations on breaking changes and versioning above. However, although fabric-contract-api-go already has a v1.x version and so seems like a more obvious problem, I'm not sure these changes significantly impact the public API of that module. Typically smart contract implementors are embedding the Contract type, which is not impacted. It does impact ContractChaincode, but this is typically just created with NewChaincode() and then has its Start() method called, neither of which are changed. People should generally be using this higher level API and not the lower level fabric-chaincode-go API so hopefully the end-user impact is minimised. Even so, I'm not against creating a v2 module. I don't think it's a lot of work and it's certainly the safest approach for both the chaincode APIs since it avoids anyone accidentally updating module dependencies and pulling in breaking changes.

[tobigiwa]

I was waiting for other comments, buh I'll just go ahead, you are very correct with this, it is the only way forward. Also, as you've highlighted, the changes are quite minimal.

What the way forward now? I'll like to do a little part in this.

[denyeart]

The last few comments have actually made me more concerned about a move to apiv2.

Since the apiv2 announcement at https://go.dev/blog/protobuf-apiv2 mentions that existing consumers can stay on apiv1 indefinitely, I haven't seen much motivation to move up with breaking changes that may impact fabric users.

[bestbeforetoday]

At the wire level the change is 100% compatible. It makes no difference to the client or server whether the other is using apiv1 or apiv2. The incompatibilities come when either:

1. Code is using the protobuf APIs (not just the fabric-protos bindings), which are largely similar but do have some key differences.
2. The namespace conflict that occurs in the Go protobuf implementation when multiple bindings for the exactly the same protobuf messages are mixed in the same application.

The first is contained to the application code so is resolved by making any necessary changes to (Fabric) code.

The second impacts users where their application code depends on both fabric-protos and on Fabric (core, chaincode or client API) code, which in turn depends on a specific fabric-protos version. To make certain the protobuf API version used doesn't catch out the application developer, I took the approach of a major version change for fabric-chaincode-go. For users who are making use of fabric-protos in their application code, they can chose which version implementation they want to use and migrate when they are ready.

The major version change for fabric-chaincode-go is also required to fix the problematic pass-by-value of protobuf messages in the public API.

[bestbeforetoday]

For reference, Go chaincodes are updated to use fabric-protos-go-apiv2:

• fabric-chaincode-go v2 is delivered in v2 implementation using fabric-protos-go-apiv2 fabric-chaincode-go#108
• fabric-contract-api-go v2 is delivered in v2 implementation based on fabric-chaincode-go/v2 fabric-contract-api-go#137
• fabric-samples is updated to use fabric-contract-api-go v2 in Update Go chaincode to fabric-contract-api-go/v2 fabric-samples#1228