Noyez vis change hist identity show

[noyez]

Current Behavior

The ockam identity show --full command just shows a hex string representing many pieces of information.

Example before output:

$ ockam identity show --full --node n1
01cdb5565163e5b1278eb31e6dbd213066e335da0c3e5d8ffed3789ce1305...6778a9ae9204bf7fbc8adbc00138b2756a09

Proposed Changes

Decompose the hex string and display the data in a more human friendly format.

After:

$ ockam identity show --full --node n1
Change History:
  Change[0]:
    identifier: cdb5565163e5b1278eb31e6dbd213066e335da0c3e5d8ffed3789ce130523391
    change:
      prev_change_identifier: 0547c93239ba3d818ec26c9cdadd2a35cbdf1fa3b6d1a731e06164b1079fb7b8
      label:        OCKAM_RK
      public_key:   Ed25519 e7417e5ea17b05684cb56171f6f37ac92d37a03587fa43f66663bfda878a1322
    signatures:
      [0]: SelfSign 5e48f7a133ac1d9218f9fb7185cf890af6bbe9ca5ca58a741...9204bf7fbc8adbc00138b2756a09

This was implemented by de-serializing the original identity hex string using serde_bare and by adding the fmt::Display trait to the underlying types in the IdentityChangeHistory struct. There was a visibility change related to the IdentityChangeHistory struct, it went to pub from pub(crate) so that it can be de-serialized and used by the ockam_command crate.

Related Issue: #3258

Checks

• All commits in this Pull Request are signed and Verified by Github.
• All commits in this Pull Request follow the Ockam commit message convention.
• I accept the Ockam Community Code of Conduct.
• I have accepted the Ockam Contributor License Agreement by adding my Git/Github details in a row at the end of the CONTRIBUTORS.csv file in a separate pull request to the build-trust/ockam-contributors repository.

[mrinalwadhwa]

@noyez this looks great. I'll read in more detail soon and leave some feedback.

In the meantime, before we can merge please

• Accept the Ockam Contributor License Agreement by adding your Git/Github details in a row at the end of the CONTRIBUTORS.csv file in a separate pull request to the build-trust/ockam-contributors repository.

The easiest way to do this is to edit the CONTRIBUTORS.csv file in the github web UI and create a PR, this will make the commit as verified.

[noyez]

Hello @mrinalwadhwa -- I had added my details here:
https://github.com/build-trust/ockam-contributors/blob/main/CONTRIBUTORS.csv#L151

Perhaps i missed something else? Maybe i need to update/change my email?
thanks!

[mrinalwadhwa]

@noyez ah yes, sorry I missed it in my quick check, you're all set no update needed.
I'll try out the changes soone.

[adrianbenavides]

@mrinalwadhwa tested and reviewed!

@noyez it would be awesome if you could add a test in the ockam_command/tests.commands.bats file. You could do something similar to the create a node and show its identity test.

[noyez]

@noyez it would be awesome if you could add a test in the ockam_command/tests.commands.bats file. You could do something similar to the create a node and show its identity test.

@adrianbenavides Sure I can do this. It appears that the purpose of testing ockam identity show --node n1 is to check that process exits with Zero (i.e. success) and its output matches some rules. Am I understanding this correctly?

Another possible test would be to test that a specific long identity string deserialized correctly into components with expected values. This test would be a typical rust test denoted w/ #cfg(test). If this sounds desirable i can add this kind of test too. Naively it would place such a test in implementations/rust/ockam/ockam_identity/src/change_history.rs, however there are no tests there. So perhaps there is a more central spot for such a test?

[adrianbenavides]

@adrianbenavides Sure I can do this. It appears that the purpose of testing ockam identity show --node n1 is to check that process exits with Zero (i.e. success) and its output matches some rules. Am I understanding this correctly?

That's right!

Another possible test would be to test that a specific long identity string deserialized correctly into components with expected values. This test would be a typical rust test denoted w/ #cfg(test). If this sounds desirable i can add this kind of test too. Naively it would place such a test in implementations/rust/ockam/ockam_identity/src/change_history.rs, however there are no tests there. So perhaps there is a more central spot for such a test?

We don't have a clear strategy for writing rust tests on the ockam_command crate that run nodes. The problem is that, since rust tests are executed in parallel, creating nodes might fail because it reads/writes to shared config files.

[noyez]

@adrianbenavides bat tests added.

This leads me to another question. When writing the test i realized i am assuming that an IdentityChangeHistory will always contain at least one IdentitySignedChange in its internal vector. Is there any point where the vector would be empty? If so, then I believe i'd need to change the test at the very least.

[adrianbenavides]

Is there any point where the vector would be empty?

No, it looks great as it is. The identity is always initialized with a IdentitySignedChange.

[adrianbenavides]

Awesome contribution @noyez , thank you so much! 🥳