protocols/noise: Inline handshake functions into upgrade traits

[thomaseizinger]

Description

This fell out of #2903. It removes a layer of indirection by inlining the various handshake functions into the respective upgrade traits. This should hopefully make it clearer, what message exchange actually happens for a particular handshake pattern.

This also removes a bit of complexity in terms of what the {send,receive}_identity functions do as several parts of it were conditional based on actually statically known state.

This is a breaking change because we make the entire handshake module private and thus prevent external users from re-using our handshake functions. However, I don't think having these public in the first place was a good move. The libp2p-noise crate specifically offers support for noise within libp2p and should not be used for noise handshakes in general.

Conveniently, this is also a net-negative in terms of code size :)

Links to any relevant issues

Open Questions

Change checklist

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• [ ] I have added tests that prove my fix is effective or that my feature works
• A changelog entry has been made in the appropriate crates

[thomaseizinger]

@mxinden I removed the Clone of the keypair again in here by moving the functions to ProtocolParams as you suggested earlier. It actually works out well enough because the new function signature forces us to pass the prologue.

[thomaseizinger]

@elenaf9 Do you have any idea why the autonat tests are failing on this branch? I can't figure it out :(

[elenaf9]

@elenaf9 Do you have any idea why the autonat tests are failing on this branch? I can't figure it out :(

I don't think the test failures are autonat specific, but instead autonat is just the first package that is being tested. I just ran teh tests it locally and when I ignore all autonat tests, the next relevant test in line (libp2p_core::tests::upgrade_pipeline) fails as well.

[thomaseizinger]

@elenaf9 Do you have any idea why the autonat tests are failing on this branch? I can't figure it out :(

I don't think the test failures are autonat specific, but instead autonat is just the first package that is being tested. I just ran teh tests it locally and when I ignore all autonat tests, the next relevant test in line (libp2p_core::tests::upgrade_pipeline) fails as well.

Good thinking, I shall investigate!

[thomaseizinger]

git bisect saves the day once again. Our offending commit is ae18dd7 :)

[thomaseizinger]

This is ready for a full review @elenaf9 @mxinden !

[mxinden]

This looks good to me. Thanks for spotting the simplification!