fabric icon indicating copy to clipboard operation
fabric copied to clipboard

bug fix 4509. Configuration parameters CA and address in peer.deliver…

Open marianothiago opened this issue 1 year ago • 5 comments

Type of change

  • Bug fix

Description

Fabric reads the values stored in core.yaml in the peer.deliveryclient.addressOverrides section, but when connecting to orderers through the connect method in the blocksprovider.go file (internal/pkg/peer/blocksprovider/blocksprovider.go) it is using the RandomEndpoint function (endpoint, err := d.Orderers.RandomEndpoint()) located in the connection.go file (internal/pkg/peer/orderers/connection.go) to set up the endpoint that will make the connection. However the RandomEndpoint function does not use the peer.deliveryclient.addressOverrides information read by Fabric.

Additional details

Related issues

https://github.com/hyperledger/fabric/issues/4509

marianothiago avatar Oct 31 '23 12:10 marianothiago

Thanks for providing the fix. Would you also mind adding some unit tests to demonstrate the issue described and to make sure the fix indeed solves the problem? Moreover, to ensure we got it covered for potential future modifications?

C0rWin avatar Oct 31 '23 12:10 C0rWin

Does the problem still occur in main branch? Typically fixes are made to main branch and then backported as desired to the release branches.

denyeart avatar Oct 31 '23 12:10 denyeart

Thanks for providing the fix. Would you also mind adding some unit tests to demonstrate the issue described and to make sure the fix indeed solves the problem? Moreover, to ensure we got it covered for potential future modifications?

OK. I will do this.

marianothiago avatar Oct 31 '23 13:10 marianothiago

Does the problem still occur in main branch? Typically fixes are made to main branch and then backported as desired to the release branches.

I tested branches 2.1, 2.2, 2.3, 2.4 and 2.5. I haven't tested it in main. I'll test it and let you know.

marianothiago avatar Oct 31 '23 13:10 marianothiago

@marianothiago @C0rWin @denyeart

I don't think this is the right approach. The override endpoints are first loaded from the core yaml when the peer starts. Then when ever

Update(globalAddrs []string, orgs map[string]OrdererOrg)

is called, after a config TX, the source slice is rebuilt (this is the way the current config is applied as well after the peer starts). The sources slice is rebuilt by comparing the overrides to the addresses that come from the config block - globalAddrs or per org orgs . If an orderer override has the same address as one of the endpoints in the globalAddrs or orgs, it will be used instead of that specific address. This is the place to look for a bug, if there is one, in my opinion. According to the current code, overrides are a mechanism to override the definition of an endpoint, not to add more endpoints, which is what you do here.

With respect to your use case: you describe a case where a peer falls behind the ordering cluster config, and cannot connect to it. While the overrides may give you a partial solution, I think the more "updated" way to approach this is by starting the peer from a snapshot taken from another peer that is up to date.

And finally, this object is about to change in V3, as a result of the work we do to implement a byzantine block puller.

tock-ibm avatar Nov 06 '23 14:11 tock-ibm