GGist / bip-rs

BitTorrent Infrastructure Project In Rust
Apache License 2.0
296 stars 33 forks source link

Implement Common Message Types For Extension Protocol #91

Open GGist opened 7 years ago

GGist commented 7 years ago

We need message containers for common extension messages, as well as some standard extension protocol container, which itself can contain multiple extension protocols (along with their messages):

GGist commented 7 years ago

Currently struggling to come up with a good way to push certain messages down to nested protocols.

The problem is that we handle the ExtendedMessage as specified in the handshake extension bits, as part of the PeerWireProtocol. Anytime a peer or us sends an ExtendedMessage (which contains the mapping of extension identifier to message id), the PeerWireProtocol handles it. However, ExtendedMessage is special in that it affects the logic of any nested protocols in that they will have to look for (at runtime) the id they are responsible for parsing messages from.

The nested protocols can't know about that message if our outer PeerWireProtocol is the only protocol that sees and handles that message. We would ideally like a mechanism to notify nested protocols of messages that were sent/received from outer protocols, so that the inner protocols can adapt as necessary to such "control" messages.

Currently, our impl for our PeerWireProtocol looks like:

impl<P> PeerProtocol for PeerWireProtocol<P> where P: PeerProtocol {
    type ProtocolMessage = PeerWireProtocolMessage<P>;

    fn bytes_needed(&mut self, bytes: &[u8]) -> io::Result<Option<usize>> {
        PeerWireProtocolMessage::bytes_needed(bytes, &mut self.ext_protocol)
    }

    fn parse_bytes(&mut self, bytes: Bytes) -> io::Result<Self::ProtocolMessage> {
        PeerWireProtocolMessage::parse_bytes(bytes, &mut self.ext_protocol)
    }

    fn write_bytes<W>(&mut self, message: &Self::ProtocolMessage, writer: W) -> io::Result<()>
        where W: Write {
        message.write_bytes(writer, &mut self.ext_protocol)
    }

    fn message_size(&mut self, message: &Self::ProtocolMessage) -> usize {
        message.message_size(&mut self.ext_protocol)
    }
}

We can see that we pass no state information down to the nested protocol P. And we should keep in mind, maybe the nested protocols don't want to accept any outer protocol message sent/recvd information (we shouldnt require them to if they dont want to). We could decide to have another impl to pass our entire message down to the nested protocol (but it could get tricky with type nesting). The other option is to pass just certain messages down, which is easier for nested types to reason about.

A new trait is proposed as being introduced:

pub trait NestedPeerProtocol<M> {
    fn received_message(&mut self, message: &M);

    fn sent_message(&mut self, message: &M);
}

So protocols that are expected to be nested could implement this trait, and it is up to the outer protocol to recognize that the inner protocol implements this trait (maybe for a specific message that the outer protocol handles), and call the appropriate methods.

That being said, we could update our PeerWireProtocol impl as such:

impl<P> PeerProtocol for PeerWireProtocol<P> where P: PeerProtocol {
    type ProtocolMessage = PeerWireProtocolMessage<P>;

    fn bytes_needed(&mut self, bytes: &[u8]) -> io::Result<Option<usize>> {
        PeerWireProtocolMessage::bytes_needed(bytes, &mut self.ext_protocol)
    }

    default fn parse_bytes(&mut self, bytes: Bytes) -> io::Result<Self::ProtocolMessage> {
        PeerWireProtocolMessage::parse_bytes(bytes, &mut self.ext_protocol)
    }

    default fn write_bytes<W>(&mut self, message: &Self::ProtocolMessage, writer: W) -> io::Result<()>
        where W: Write {
        message.write_bytes(writer, &mut self.ext_protocol)
    }

    fn message_size(&mut self, message: &Self::ProtocolMessage) -> usize {
        message.message_size(&mut self.ext_protocol)
    }
}

impl<P> PeerProtocol for PeerWireProtocol<P> where P: PeerProtocol + NestedPeerProtocol<ExtendedMessage> {
    fn parse_bytes(&mut self, bytes: Bytes) -> io::Result<Self::ProtocolMessage> {
        match PeerWireProtocolMessage::parse_bytes(bytes, &mut self.ext_protocol) {
            Ok(PeerWireProtocolMessage::BitsExtension(BitsExtensionMessage::Extended(msg))) => {
                self.ext_protocol.received_message(&msg);

                Ok(PeerWireProtocolMessage::BitsExtension(BitsExtensionMessage::Extended(msg)))
            },
            other                                                                           => other
        }
    }

    fn write_bytes<W>(&mut self, message: &Self::ProtocolMessage, writer: W) -> io::Result<()>
        where W: Write {
        match (message.write_bytes(writer, &mut self.ext_protocol), message) {
            (Ok(()), &PeerWireProtocolMessage::BitsExtension(BitsExtensionMessage::Extended(ref msg))) => {
                self.ext_protocol.sent_message(msg);

                Ok(())
            },
            (other, _)                                                                                 => other
        }
    }
}

This is fine, though as we said before, we would have to have a new impl for every message we think our inner protocol would like to see (which may be fine in that ExtendedMessage is a bit special...). We also are taking advantage of impl specialization which afaik is nightly only, which is definitely a major downside of this approach.