refactor(noise): modify message framing

[0xcrust]

Description

Resolves #4488.

Notes & 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]

Thank you! That is great progress and one can already see that the implementation will become much simpler with this.

I've left some comments with a few ideas! :)

[0xcrust]

Want to take a look at this? @thomaseizinger . I made the changes we discussed

[thomaseizinger]

Nice work! Despite the large changes, the diff is really clean and easy to read, well done.

I think the stats also speak for itself that this is a nice improvement :)

[thomaseizinger]

The interoperability tests seem to be failing repeatedly with our previously released version. I'll look into that!

[thomaseizinger]

Thanks for this, very clean PR :)

Mind fixing up the remaining CI failures regarding the docs? I've restarted the interop tests to see whether they pass this time. They've been very stable recently though so I'd be surprised if that is just a flake this time.

Weirdly, interop seems to be fine with all the other versions, it is just our previously release that is acting up!

[thomaseizinger]

I looked into the incompatibility issues and it turns out the problem was that the LengthCodec I pointed you to was using u64 but we need to use a u16. Sorry for sending you down that path.

That fixed some of the errors but there seem to be more problems ...

[thomaseizinger]

I noticed that with the structure of nesting codec's, we can reuse buffers properly because Rust can't detect that we don't have overlapping mutable borrows. So out of interest, I tried to refactor this to use free functions instead and pass the buffers in which does work!

I also investigated the incompatibility issues locally and thought I had fixed them too because they pass locally but now they are failing again in CI 🙄

These aren't normally flaky so there must be something wrong still.

[thomaseizinger]

Okay, tiny progress: The error happens when this PR is the listener in the ping interop test.

[thomaseizinger]

Found the issue! We were dropping the read and write buffers when mapping the codec and thus threw away data the remote had already sent us as part of being in TransportMode.

[thomaseizinger]

Found the issue! We were dropping the read and write buffers when mapping the codec and thus threw away data the remote had already sent us as part of being in TransportMode.

I had to change another thing in asynchronous-codec to make this work: mxinden/asynchronous-codec#11

[0xcrust]

Thanks for engaging with this @thomaseizinger. Much appreciated!

[thomaseizinger]

@0xcrust We have a new release of asynchronous-codec. Can you update this PR to move away from the git dependency?

See mxinden/asynchronous-codec#9 (comment).

[0xcrust]

@0xcrust We have a new release of asynchronous-codec. Can you update this PR to move away from the git dependency?

Sure! Will do in a bit

[thomaseizinger]

Friendly ping @mxinden. This is ready for review!

[thomaseizinger]

Merging here because it is passing tests and it will otherwise go stale again. Obviously still feel free to review this post-merge @mxinden.

Approvals have been dismissed because the PR was updated after the send-it label was applied.

[mxinden]

Thank you for the work here. Great to see these manually implemented state machines removed.