search

hyperledger / fabric-admin-sdk

Hyperledger Fabric admin SDK
Apache License 2.0
31 stars 19 forks source link

impl for get block from orderer #115

Closed SamYuan1990 closed 1 year ago

SamYuan1990 commented 1 year ago

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

SamYuan1990 commented 1 year ago

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

SamYuan1990 commented 1 year ago

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 https://github.com/hyperledger/fabric-admin-sdk/issues/105#issuecomment-1468465893 I suppose we just provide a way to get latest block and config block from orderer will be enough with this pr.

SamYuan1990 commented 1 year ago

Hi @bestbeforetoday , any comments?

1gezhanghao commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.