impl for get block from orderer

[SamYuan1990]

Hope by this PR we can support get block from orderer.

[bestbeforetoday]

I am not sure what the purpose is for providing an API the retrieve just the latest block from the orderer. If the intent is to get the current config block, perhaps it would be better to provide an API call that returns the config block, not just the latest block on the blockchain. The config block is obtained by:

1. Getting the latest block
2. Reading the latest config block number from the latest block
3. Getting the latest config block (by block number)

On the publicly exposed API, I don’t think it is helpful to require the caller to understand the Fabric protobuf bindings and create the AtomicBroadcast_DeliverClient themselves. It would be easier for them to just supply a gRPC client connection to the orderer node, and for the admin API implementation to create a AtomicBroadcast_DeliverClient from that gRPC connection, as the chaincode APIs do.

On the implementation, quite a few parameters seem to be passed around between multiple related functions. It might be easier to create a struct with those values as properties, then have the related functions all be receiver functions (or methods) on that struct so the required parameters are implicitly available to all the related functions and don’t need to be explicitly passed around between them.

[SamYuan1990]

I will try to update this PR, hope by End of this week.

[SamYuan1990]

I am not sure what the purpose is for providing an API the retrieve just the latest block from the orderer. If the intent is to get the current config block, perhaps it would be better to provide an API call that returns the config block, not just the latest block on the blockchain. The config block is obtained by:

1. Getting the latest block
2. Reading the latest config block number from the latest block
3. Getting the latest config block (by block number)

On the publicly exposed API, I don’t think it is helpful to require the caller to understand the Fabric protobuf bindings and create the AtomicBroadcast_DeliverClient themselves. It would be easier for them to just supply a gRPC client connection to the orderer node, and for the admin API implementation to create a AtomicBroadcast_DeliverClient from that gRPC connection, as the chaincode APIs do.

On the implementation, quite a few parameters seem to be passed around between multiple related functions. It might be easier to create a struct with those values as properties, then have the related functions all be receiver functions (or methods) on that struct so the required parameters are implicitly available to all the related functions and don’t need to be explicitly passed around between them.

ref to #105 (comment)
I suppose we just provide a way to get latest block and config block from orderer will be enough with this pr.

[SamYuan1990]

Hi @bestbeforetoday , any comments?

[bestbeforetoday]

This change is generally looking pretty good to me. Several suggestions below to:

1. Minimise the API surface to just what is actually required by the client application code; on other words, just getting the config block.
2. Simplify the code required in the client application.
3. Consistency with other parts of the API.

[bestbeforetoday]

Looks good!

[1gezhanghao]

@SamYuan1990 well done, I've tested the 'channel.GetConfigBlockFromOrderer(...)' function and it works great. BTW, could you implement a tool to deep marshal the block into JSON.

[SamYuan1990]

@SamYuan1990 well done, I've tested the 'channel.GetConfigBlockFromOrderer(...)' function and it works great. BTW, could you implement a tool to deep marshal the block into JSON.

I suppose we don't have planned this in recent implementation. would you like to provide a PR for this?

[bestbeforetoday]

@SamYuan1990 well done, I've tested the 'channel.GetConfigBlockFromOrderer(...)' function and it works great. BTW, could you implement a tool to deep marshal the block into JSON.

Note that this is something that the protolator package in fabric-tools does for the old Fabric protobuf (APIv1) Go bindings. There is already issue hyperledger/fabric-config#53 open there to update it for the newer protobuf (APIv2) bindings. I a not against implementing this capability in the fabric-admin-sdk package, but it is important to avoid duplication and co-ordinate across projects. It might be worth getting the opinion of people with an oversight of all the Fabric projects, like @denyeart.

[denyeart]

I've always thought that fabric-admin-sdk might be able to use the features that already exist in the fabric-config library, so yes, I'd recommend putting common utilities in fabric-config and using them as a dependency in fabric-admin-sdk.

[SamYuan1990]

I've always thought that fabric-admin-sdk might be able to use the features that already exist in the fabric-config library, so yes, I'd recommend putting common utilities in fabric-config and using them as a dependency in fabric-admin-sdk.

maybe @jt-nti can have a try?
I do agree that we should avoid duplicate works. but, in fabric-config, it seems no end to end test as integrated with fabric as an sample, I am not sure if there is an easy way to use it?(as replace existing code in admin-sdk by fabric-config.)

[SamYuan1990]

I've always thought that fabric-admin-sdk might be able to use the features that already exist in the fabric-config library, so yes, I'd recommend putting common utilities in fabric-config and using them as a dependency in fabric-admin-sdk.

btw, agree with @bestbeforetoday 's comments, fabric-config should better upgrade to protobuf with v2 as admin sdk already made it.