Display the identity hash of PeerIds when relevant

[tomaka]

The Debug and Display implementations call to_base58().
This PR transitions to_base58() to the "proper" string representation of the PeerId.

In other words, identity PeerIds will now start with 1... rather than Qm...

[romanb]

Can we not end the transition period and remove the temporary canonical field and the rest of the migration code? The reason I'm asking is that using borrow() in to_base58 is strictly what we want: Equal peer IDs should have an equal Display. If we remove canonical and the rest of the temporary workarounds, borrow() and as_ref() will again be the same, as it was before this identity hashing migration, and thus the Display will be what you expect and the same peer IDs will have the same Display output. By just changing borrow() to as_ref() in Display now, without removing all the other compatibility workarounds, in particular the custom PartialEq, we introduce another intermediate state where peer IDs can be equal according to PartialEq but give different output for Display.

[tomaka]

we introduce another intermediate state where peer IDs can be equal according to PartialEq but give different output for Display.

That was actually my intention when opening this.

The problem we're facing right now is that PeerIds are displayed to users in their SHA256 form. Users then copy these PeerIds around between machines, thereby losing the non-canonical version. The most obvious example is the bootstrap node addresses.

If we straight-up remove the migration code, then all these existing keys will become invalid.

The only way to fix that is to first have a step where we show the correct version printed out.

[romanb]

If we straight-up remove the migration code, then all these existing keys will become invalid.

So if I understand correctly, you would want to tell users explicitly to re-copy these new peer IDs to wherever they are used? Since without such explicit instruction, I don't see how it could make a difference. And since such explicit upgrade instruction(s) are needed, you need to tell users something of the form "Please (re)start your node or execute command foo to get your new peer ID output. Replace your old peer ID with this new one everywhere, e.g. in your list of bootstrap nodes.", right? Why is it important then that "everywhere" in substrate that a peer ID is printed, the new version is shown for that purpose? What I'm trying to get at is, wouldn't a simple one-time upgrade hint in substrate where the new ID is displayed suffice as preparation for removing the migration code here, so there doesn't need to be another intermediate transition state in libp2p itself?

[romanb]

If we straight-up remove the migration code, then all these existing keys will become invalid.

So if I understand correctly, you would want to tell users explicitly to re-copy these new peer IDs to wherever they are used? Since without such explicit instruction, I don't see how it could make a difference. And since such explicit upgrade instruction(s) are needed, you need to tell users something of the form "Please (re)start your node or execute command foo to get your new peer ID output. Replace your old peer ID with this new one everywhere, e.g. in your list of bootstrap nodes.", right? Why is it important then that "everywhere" in substrate that a peer ID is printed, the new version is shown for that purpose? What I'm trying to get at is, wouldn't a simple one-time upgrade hint in substrate where the new ID is displayed suffice as preparation for removing the migration code here, so there doesn't need to be another intermediate transition state in libp2p itself?

I guess you would say that, although this could be done, it would be quite confusing for users to be told "This is your new peer ID, use it everywhere" but then not actually see it in any output (e.g. logs), which is fair enough.