Skip to content

Join channel fails with imported ordering service #819

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TsvetanG opened this issue Feb 11, 2025 · 7 comments
Open

Join channel fails with imported ordering service #819

TsvetanG opened this issue Feb 11, 2025 · 7 comments

Comments

@TsvetanG
Copy link

There is an issue (seems to be a regression as it works with earlier console images - i.e. v1.0.8-18) joining a peer on a channel using an imported ordering service.
The error is:
_An unexpected error occurred.

Hide error details
{"details":{"function_name":"getChannelBlockFromOrderer","error":true,"msp_id":"orderingsrvmsp","stitch_msg":"["Missing an argument - client certificate (\"client_cert_b64pem\"). Provide a ECDSA signed certificate that is a base 64 encoded PEM file.","Missing an argument - client private key (\"client_prv_key_b64pem\"). Provide a ECDSA private key that is a base 64 encoded PEM file."]","grpc_resp":null,"orderer_host":"https://demo-hlf-console-console.texas.demo.senofi.net:443/grpcwp/https%3A%2F%2Fdemo-orderingsrvnode1-_

The setup:

  • Two consoles for 2 different orgs. One of the consoles operates the ordering service and an MSP. The second console operates a single MSP.
  • The MSP of the second console is part of the channel.
  • The MSP definition of the first org and the ordering service definition are imported to the second console.
  • The second console fails to join the channel with an error (see above). Console -> Channels (Peer Channels) -> Join Channel -> Select the imported ordering service (Next) -> Enter the Channel Name (Next) -> Error occurs.
    The same error appears when the join channel is triggered from the Peer page.

Expected result:
The console should list the peers to join the channels and allow the user to finish the process.

@TsvetanG
Copy link
Author

After further investigation, it looks like the following change introduced the issue:
6a8befd
See line 487, where the error is introduced in the filterPeers method if the config block cannot be retrieved.

The change above seems to resolve an issue with filtering the peers that belong to MSPs on the channel.

However, when the ordering service is imported and managed outside of the console instance (i.e. by another organization), there will be no admin identities of the ordering service available in the wallet. Therefore the getChannelConfigBlock of OrdererRestApi will fail (internally it gets the cert/private key of the ordering node associated admin identity).
To solve this problem the API should rather use the admin identity certs of the MSP of the peer to fetch the ordering service config block (considering the ordering service update is already done to include the joining MSP certs).

There are 2 paths of joining peer to channel that are affected:
(1) Trigger the join to channel from the peer page
In this scenario, we know the peer and the MSP of the peer. In case there are no ordering service identities available the impl can use the MSP admin to fetch the config block

(2) Trigger the join to channel from the channels page
In this case, there is no peer/MSP context and there is no ordering service identity available the console cannot fetch the config block. Note that at this point there is no information about the MSPs of the peers that are joining the channel. Therefore the console cannot detect and use the proper MSP admin user to fetch the config block.
There is an option to try fetching the block using the available MSP admins but this has side effects like performance impacts. Furthermore, the user may not be aware of what exact admin identities the console is using to fetch the block (the console may expose unnecessary admin certs to the ordering service). The console may expose an input to the user to choose the MSP admin for the call, however, this requires a change of the current screen.

Proposed solution:

  • Pass the MSP id from the JoinChannelModal to the OrdererRestApi getChannelConfigBlock. Adjust the impl of the getChannelConfigBlock to use the MSP admin certs to fetch the config block when there is no ordering service admin identity available.
  • Adjust the logic in getChannelConfigBlock of the OrdererRestApi to return a specific error in case no certs are determined for the call to fetch the block.
  • Adjust the code in JoinChannelModal when processing the error in the catch block to consider the specific error when no certs are determined, the config cannot be fetched and therefore the MSPs on the channel are currently unknown. Let the user continue with the process and decide what peers to join. The admin should be already aware what are the MSPs on the channel based on the out-of-band communication with the admin of the other organizations.

I am available to contribute a fix once the proposed solution has been reviewed and accepted by the maintainers.

@denyeart
Copy link
Contributor

@TsvetanG Thank you for investigating.
Were you able to determine how it used to work in previous versions?
Does your proposal essentially restore how it used to work?

@TsvetanG
Copy link
Author

TsvetanG commented Mar 4, 2025

@denyeart

Yes, from a functional perspective, it restores the way it worked, considering also the functionality that was added (and unfortunately introduced the regression issue).
The proposal is to fix it properly so that it doesn't completely revert the aforementioned functionality.

@denyeart
Copy link
Contributor

denyeart commented Mar 5, 2025

@TsvetanG
We have reproduced the issue in recent console versions, go ahead and submit the fix PR.

From my assessment the bug may have been introduced even further back than the commit you mentioned. This is a potential culprit:
ad920d2

I haven't tested my theory, but it looks like in this commit OrdererRestApi.js was changed to remove the passed identityInfo:

			msp_id: options.identityInfo.msp_id || test.msp_id,
			client_cert_b64pem: options.identityInfo.cert || test.cert,
			client_prv_key_b64pem: options.identityInfo.private_key || test.private_key,

And instead it always uses the orderer mspid, cert, and private key (which is not found when joining a peer from a different console than the ordering service console).

Can you assess if that is indeed the issue, and potentially revert to the old code that used to work.

@TsvetanG
Copy link
Author

TsvetanG commented Mar 6, 2025

@denyeart

Yes it was kind of broken before but it worked as the error was just logged but not propagated.
The purpose of that filterPeer method is to show only the peers that belong to MSPs that are currently on the channel. However , if the ordering service is external it is not possible to retrieve that MSP list from the channel as the channel config can't be fetched.

The identityInfo might be useful in case the user selects the MSP to be used to fetch the config block. However, this is a new functionality and needs to be looked at in details.

If we revert to the older code we will revert also the filter peer logic that still make sense and is useful in case the ordering service admin identity is available in the console.

The fix I am proposing is about keeping the current logic of filtering peers if the ordering admin identity is available and skipping it if the identity is not available.

In both cases the end user should be anyways well aware of what MPS are on the channel and make the correct selection of peers to join the channel.

@denyeart
Copy link
Contributor

denyeart commented Mar 6, 2025

@TsvetanG
I agree that we should keep the improved logic in each of the prior commits including the peer filtering logic. I was only pointing out that prior to commit ad920d2 there appeared to be logic in OrdererRestAPI.js to use the peer MSP admin credentials rather than simply use the orderer MSP admin credentials (which won't be available in this use case scenario). I thought you may be able to use some of the prior code (without reverting the entire commit), since the prior code appeared to be aligned with your suggested logic.

Whichever approach you like should be fine, go ahead and open the pull request so that we can start reviews and testing.

Thanks again!

@TsvetanG
Copy link
Author

@denyeart Pull request is ready pls review:

#841

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants