hyperledger / fabric-chaincode-go

Hyperledger Fabric Packages for Go Chaincode
https://pkg.go.dev/github.com/hyperledger/fabric-chaincode-go
Apache License 2.0
136 stars 132 forks source link

Draft: Resolves #80, Bumped fabric-proto-go to fabric-protos-go-apiv2 #84

Closed tobigiwa closed 4 weeks ago

tobigiwa commented 10 months ago

Draft: This is a working change on bumping the fabric-chaincode-go to use fabric-protos-go-apiv2 and the new protobufs lib. This commits includes changes to fuction signatures, return values, interface definition and type definition to type "ChaincodeMessage" and "pb.Response".

It is a breaking change.

Here is a repository with a branch that house the state of the codebase after resolving the imports with fabric-protos-go-apiV2 lib, this does not breaks anything yet and it is intended to showcase to the maintainers the complaint of the go vet . The go vet ./... command should be run at the root of the project.

https://github.com/tobigiwa/fabric-chaincode-go/tree/fix

Resolves https://github.com/hyperledger/fabric-chaincode-go/issues/80

tobigiwa commented 10 months ago

I'll be awaiting reply, discord username is @tobytobias.

ryjones commented 10 months ago

@denyeart take a look

denyeart commented 10 months ago

Thank you for the contribution!

Could you check the "go vet" errors in the failed check?

Since the shift to fabric-protos-go-apiv2 is a significant change, we'll want to prove that everything works fine end-to-end. This would imply updating your fork of fabric-contract-api-go to use your updated fabric-chaincode-go, and then trying a fabric-sample with your updated fabric-contract-api-go and your updated fabric-chaincode-go. Here's the sample that I would recommend trying: https://github.com/hyperledger/fabric-samples/blob/main/asset-transfer-basic/chaincode-go/go.mod

For doc tutorial about this chaincode, see: https://hyperledger-fabric.readthedocs.io/en/latest/chaincode4ade.html https://hyperledger-fabric.readthedocs.io/en/latest/deploy_chaincode.html

Once you demonstrate that it all works end-to-end, we can merge this PR, then merge the fabric-contract-api-go PR that uses the updated fabric-chaincode-go, and then merge the fabric-sample that uses them both.

jt-nti commented 6 months ago

It would be great to get everything updated to use fabric-protos-go-apiv2 however since it is such a significant (breaking?) change, there are probably implications for module versioning.

fabric-chaincode-go is basically unversioned, with pseudo-version numbers, so technically it makes no guarantees about backward compatibility but making this change is likely to be disruptive.

fabric-contract-api-go is currently at v1.2.2, which will make things even more interesting! A breaking change should take it to v2.

denyeart commented 6 months ago

I commented in the main fabric APIv2 issue.

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?

davidkhala commented 6 months ago

@tobigiwa Are you still working on it?

tobigiwa commented 6 months ago

Not again... buh am hoping to take another look at it again.

tobigiwa commented 6 months ago

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

bestbeforetoday commented 4 weeks ago

The change to fabric-protos-go-apiv2 was delivered in PR #108.