Skip to content
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

fix: Status call returns properly when node is not registered but organisation is #1467

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

SamMayWork
Copy link
Contributor

@SamMayWork SamMayWork commented Feb 15, 2024

ref: #1453 and this change

When a namespace is registered to the organisation but the node has not been registered, if the name has not been set (i.e. it's "") then we throw an error. If a name is set, then we return nil because while there is no information on the node (as it's not registered) it is possible for the node to be registered.

Side note: getting an environment in which to test this was a colossal pain and largely undocumented, so I'm going to put a comment below with how I got a testing configuration in place to test this out.

Signed-off-by: SamMayWork <sam.may@kaleido.io>
@SamMayWork SamMayWork requested a review from a team as a code owner February 15, 2024 11:00
@codecov-commenter
Copy link

codecov-commenter commented Feb 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b1fd7dc) 99.99% compared to head (9922203) 99.99%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1467   +/-   ##
=======================================
  Coverage   99.99%   99.99%           
=======================================
  Files         322      322           
  Lines       23301    23300    -1     
=======================================
- Hits        23299    23298    -1     
  Misses          1        1           
  Partials        1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SamMayWork
Copy link
Contributor Author

SamMayWork commented Feb 15, 2024

Testing setup

To test this out we need a multi-party network configured and running, and then need to start a local node which is not registered to the network which we can then register to the network. Bonus points if we can run it through the VS Code debugger so we can dive into the code as/when. The steps to configure this are:

  1. Create a new stack using the ff CLI and start the stack
  2. Go into the stack directory (~/.firefly/stacks/<stack_name> by default) and open it up in your favourite text-editor
  3. From the runtime directory copy the configuration for one of the existing FireFly namespaces e.g. firefly_core_0.yml
  4. Create a new file for your new namespace and paste the contents from the existing namespace
  5. Change the DB location URL to be a location on your local machine with an SQLite DB file
  6. Change Docker URLs (i.e. http://ipfs_0) to be local (127.0.0.1) address with correct ports
  7. Change ports on the new configuration to be distinct from the ports for the existing namespace
  8. Run the command ff accounts create <stack_name> to generate a new account for your namespace to use
  9. Change the node name and org name in the new configuration to be something distinct from the other namespace
  10. Provide the key generated in step 8 as the key values for default key and org key
  11. Verify that the contract address in the new configuration is the same as it is in the other namespaces config
  12. Change your launch.json in FireFly to point to the new configuration file

At this point, you can run FireFly core through the VS Code debugger and you'll get a multi-party configured namespace which has not yet registered its org or node.

  1. Navigate to the swagger API page for your newly created namespace
  2. Call /network/organizations/self to register the org for the namespace
  3. Call /network/nodes/self to register the node for the namespace

In the specific case of this issue, calling the /status API between steps 15 and 16 are what caused the issue this PR resolves.

Signed-off-by: SamMayWork <sam.may@kaleido.io>
@nguyer nguyer merged commit a290c8e into hyperledger:main Feb 15, 2024
14 checks passed
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

Successfully merging this pull request may close these issues.

None yet

4 participants