to/into consistency for PublicKey and PeerId

[rubdos]

As per discussion in #2142:

• Change PublicKey::into_protobuf_encoding(self) to PublicKey::to_protobuf_encoding(&self)
• Change PublicKey::into_peer_id(self) to PublicKey::to_peer_id(&self)
  • Yeet out a whole bunch of clone()'s.

The latter goes via a From implementation.

[rubdos]

Feel free to only pick the first commit.

Some comment on the PeerId conversion: there's a From<PublicKey> (and now also From<&PublicKey> for PeerId. Usually, From is not really advised for "expensive" stuff or allocating stuff. It feels a bit out-of-place and unexpected for me, here. Just my 2¢.

Anyway, this include the changelog entry in two separate steps, such that you can cherry-pick the first commit. Preferrably, #2142 is merged before this one, because that yields one less conflict (in core/CHANGELOG...).

[thomaseizinger]

A few nits!

[thomaseizinger]

This and other clones are redundant now I believe.

[thomaseizinger]

Unfortunately, we don't (yet) have rustfmt in here.

For this diff size, I think it is okay to leave it in but with larger diffs, it is best to not autoformat in rust-libp2p :)

See #1966.

[rubdos]

Damnit, I thought I had turned it off for every file. Will fix.

[thomaseizinger]

These signatures are identical 😁

[rubdos]

I probably didn't catch all of the clone()s, mostly because clippy also creates some noise in several other places. If wanted, I can squash that last commit into the PeerId commit. 7cbf674 should address your nits, and I'll gladly squash that into 99d0834 :-)

[thomaseizinger]

I probably didn't catch all of the clone()s, mostly because clippy also creates some noise in several other places.

That should be fixed if you merge latest master into your PR. That will include #2139.

If wanted, I can squash that last commit into the PeerId commit. 7cbf674 should address your nits, and I'll gladly squash that into 99d0834 :-)

Appreciate the effort! Rust-libp2p uses squash-merge though so the individual patches of the PR will be gone anyway :)
Still nice to not force-push because then at least GitHub gives you a nice "changes since your last review" button.

LGTM!

[rubdos]

Still nice to not force-push because then at least GitHub gives you a nice "changes since your last review" button.

I rebased on master, then squashed as I said, I'll add more fixups for the clones now.

[rubdos]

Still quite a bit of clippy noise going on, I fixed the clone's that were still visible in my tmux buffer 🤣

When rustfmt gets default in this crate, I'll volunteer to take some more clippy stuff on me. Without rustfmt it's gonna be too much of a mess to submit a PR 😢

I fixed some stuff that rustfmt messed up, and removed two more clone's related to to_protobuf_encoding.

[rubdos]

I think this is ready for merge, if everyone is okay with the slightly breaking change.

[mxinden]

Thank you for the patch @rubdos. Appreciate the work you put into this and also very much in favor of being consistent with Rust's naming conventions.

Given that this is the first patch after the recent release, and also given that this is a breaking change in libp2p-core, would you mind cherry-picking the commit below into this branch?

mxinden@c27758f

[rubdos]

Thank you for the patch @rubdos. Appreciate the work you put into this and also very much in favor of being consistent with Rust's naming conventions.

My pleasure!

Given that this is the first patch after the recent release, and also given that this is a breaking change in libp2p-core, would you mind cherry-picking the commit below into this branch?

Of course, done!

[rubdos]

Seems like there was later an incompatible commit. Here's a patch, @mxinden .

diff --git a/core/src/identity.rs b/core/src/identity.rs                  
index 1569d4db..298797cd 100644                                           
--- a/core/src/identity.rs                                                
+++ b/core/src/identity.rs                                                
@@ -285,12 +285,12 @@ mod tests {                                         
     #[test]                                                              
     fn keypair_protobuf_roundtrip() {                                    
         let expected_keypair = Keypair::generate_ed25519();              
-        let expected_peer_id = expected_keypair.public().into_peer_id(); 
+        let expected_peer_id = expected_keypair.public().to_peer_id();   
                                                                          
         let encoded = expected_keypair.to_protobuf_encoding().unwrap();  
                                                                          
         let keypair = Keypair::from_protobuf_encoding(&encoded).unwrap();
-        let peer_id = keypair.public().into_peer_id();                   
+        let peer_id = keypair.public().to_peer_id();                     
                                                                          
         assert_eq!(expected_peer_id, peer_id);                           
     }