TcpTransport initial implementation NetService

[mikeyoung3k]

Some improvements to be made but an initial TcpTransport.

Given the trait had an add_session method which took an already fully established session, either TcpTransport or the trait (ideally the trait) needs a method to establish a new session - that's a todo

[mikeyoung3k]

There are some pretty hefty flaws in this. For one, the listen function throws away any data received on other streams that didn't respond first.

It would be much better to have every session start a new task once it's been established, that receives data, decrypts it, and puts it in a channel. Then listen should just pick off that channel

[vercel]

@mikeyoung3k is attempting to deploy a commit to the yefimku-protonme's projects Team on Vercel.

A member of the Team first needs to authorize it.

[mikeyoung3k]

Rewrote to make listen work properly.

It now also gracefully handles the buffer being too small.

PeerId info included in errors where relevant

[efimio16]

Thanks for the PR!

In short, I liked how you implemented this task; you really dug into the cryptographic structures I’d implemented. So here’s what I think was done well:

1. Tests that actually covers a variety of cases
2. Your TCP implementation is practically correct (see notes below)
3. You comment your code and I really appreciate it

And what could be improved to merge the commit:

1. Use Bytes instead Vec
   I suggest you to use Bytes over the Vec because it has zero-copy slices and it's optimized for u8. In read_message / read_from_stream you can even create a [u8; N] and fill it without allocating for each chunk
2. Your test_bind fails because of OS permissions for small ports.
   For me work ports such as 20000, 30000 instead of 80

But in general, it's really great, thanks for supporting the project.

[mikeyoung3k]

Thanks very much. I had initially used [byte] instead of Vec and changed it with a worry about what happens if the data size grows above the buffer size - but I think I've dealt with that in a later call (although that does still require a vec or some other growable buffer)

Will go and fix test bind.

There are definitely some other tests that I'd like to do (caller hangs up before transmission ends - what error do we expect, e.g.) but on my to-do list next is see if I can go and contribute docs to Postcard because at the moment there aren't really any.

[mikeyoung3k]

Ah I do apologise, you're referring to the crate bytes? https://docs.rs/bytes/latest/bytes/struct.BytesMut.html

[mikeyoung3k]

Updated. Thanks for your comments

[efimio16]

Ah I do apologise, you're referring to the crate bytes? https://docs.rs/bytes/latest/bytes/struct.BytesMut.html

Yes.

I had initially used [byte] instead of Vec and changed it with a worry about what happens if the data size grows above the buffer size

read_buf won't overflow a &mut [u8] because it checks how many bytes (capacity - length) it should read. It also returns the number of bytes read so that the slice can be cut off accordingly.

[mikeyoung3k]

Sorry yes that won't overflow but if the buffer isn't big enough for the whole amount of data for a single serialised object then it will fail, so either we need to be very sure that the buffer will always be big enough for (at least) one object or it needs to be growable