Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
    Jonathan de Jong
    @ShadowJonathan
    this is in the far future, and it's only implementing by the letter of how libp2p builds connections. What the spec is specifying is a recommended way to build connections by default, but that does not mean that other implementations/sides/applications would have to completely adhere and be locked into that spec, they would be allowed to override it with their own strategies, again, modularity.
    Jonathan de Jong
    @ShadowJonathan
    This library clearly wants to abstract everything (e.g. ReadWriteCloser, ABC), but I find it's implementation and structurization lackluster.
    This library is supposed to be implemented, so better to unlock every internal part (even if it's implantation can't be forseen now), than to lock it down into a specific strategy, and instead offer that strategy as an overridable default.
    These are all my opinions, and some might be wrong, so please tell me if i've overlooked something or misinterpreted something, or overstepped a boundary.
    Kevin Mai-Husan Chia
    @mhchia

    message queue flushed, stream reset/closed, protocol selected, etc

    Just want to add: Agree with message queue flushed, stream reset/closed but not protocol selected.

    Also, I notice IMuxedStream is missing in your latest diagram. Is there any specific reason?

    What the spec is specifying is a recommended way to build connections by default, but that does not mean that other implementations/sides/applications would have to completely adhere and be locked into that spec

    I don't quite agree with this. I'd think ITunnel which defines the same feature of Stream and Connection sounds good. But with too many architectures that are not too related to the spec induces maintenance overhead imo.

    This library clearly wants to abstract everything

    I don't think this library wants to abstract everything either. It abstracts transports, streams, connections, networks, but not more detailed.

    It's nice to see the diagram and your thought on abstractions. I appreciate that and also admit we failed to keep the abstractions clear. Would you mind putting them into an issue to keep track of it?

    Jonathan de Jong
    @ShadowJonathan
    IMuxedStream is IStream, I personally didn't really understand why there was a "muxed connection", "muxed stream", and others, I got confused as they all alluded to different usages/things while using similar wording.
    As far as I understand, "muxed connection" is basically a connection instance with a multiplexer baked in, while "muxed stream" is just a stream that comes from that multiplexer, correct?
    The multiplexer is just a protocol that's built on top of a raw connection, it uses a connection but does not impliment a completely different way to write to that connection specifically (like ISecureConnection would)
    Kevin Mai-Husan Chia
    @mhchia

    "muxed connection" is basically a connection instance with a multiplexer baked in, while "muxed stream" is just a stream that comes from that multiplexer, correct

    Correct

    Jonathan de Jong
    @ShadowJonathan
    Yeah, that'll confuse more people than just me, probably
    Kevin Mai-Husan Chia
    @mhchia

    The multiplexer is just a protocol that's built on top of a raw connection, it uses a connection but does not impliment a completely different way to write to that connection specifically (like ISecureConnection would)

    Because nothing is directly written into the multiplexer, the multiplexer multiplex messages from muxed_stream

    Jonathan de Jong
    @ShadowJonathan
    Still though, the effective interface diagram is a mess, imo
    Yeah, the multiplexer should only handle the messages coming from and to the connection
    Hmmm
    Kevin Mai-Husan Chia
    @mhchia

    Still though, the effective interface diagram is a mess, imo

    Yeah, but that doesn't change the fact that IStreamshould write to muxed_stream, right?

    It makes sense to change IMuxedConn to IMultiplexer, to avoid confusion, as you said, in the context that connections support read/write/close/...
    Jonathan de Jong
    @ShadowJonathan
    INetwork should probably have a generic function like call_protocol(peer, protocol), which it's implimentation should be smart enough to ask the IMultiplexer to give a new IStream (effectively ITunnel) to the protocol handler.
    (random thoughts)
    IStream has a reference to the Multiplexer that created it, which it uses to send and receive messages from and to
    iMuxedConn is effectively changed to IMultiplexer, yes, but with an explicit class variable pointing to the original connection as well
    Kevin Mai-Husan Chia
    @mhchia

    IStream has a reference to the Multiplexer that created it, which it uses to send and receive messages from and to

    Why not just let IStream reference IMuxedStream? IStream should only know that it should somehow write/read something to the IMultiplexer. How to do it should be decided by IMuxedStream imo

    Jonathan de Jong
    @ShadowJonathan
    IMultiplexer could impliment Closer, but not ITunnel or ReadWriteCloser
    A Stream's relationship with a Muxer is special, so IMultiplexer should have the muxer-specific stuff like send_message and all
    Kevin Mai-Husan Chia
    @mhchia

    A Stream's relationship with a Muxer is special, so IMultiplexer should have the muxer-specific stuff like send_message and all

    I thought that is what IMuxedStream does?

    Or whatever it's named. So it's bound with IMultiplexer
    Jonathan de Jong
    @ShadowJonathan
    IStream inpliments ReadWriteCloser, which it then internally uses send_message and read_message (on a loop) in the muxer
    On the outside, IStream is just a tunnel as much as IConnection, that's the way it should work on both ends, and to protocols
    Jonathan de Jong
    @ShadowJonathan
    Actually, reading the spec definitions again, im kinda inclined to remove IConnection (and ISecureConnection)
    Kevin Mai-Husan Chia
    @mhchia

    IStream inpliments ReadWriteCloser, which it then internally uses send_message and read_message (on a loop) in the muxer

    In this case, your implementation of IStream is affected by what IMultiplexer implementation you use.

    You will need YamuxStream(IStream) and MplexStream(IStream) to support both Yamux(IMultiplexer) and Mplex(IMultiplexer)

    Jonathan de Jong
    @ShadowJonathan
    Yeah, exactly, but of course there's gonna be different ways and algorithms to impliment a multiplexer
    Go-libp2p already has a few, so it makes no sense to lock down the multiplexer to an implimentation, and instead provide an interface
    Yeah, I think imma remove ITunnel, and make it IConnection
    IConnection is abstract enough, according to the definitions in that spec document
    Kevin Mai-Husan Chia
    @mhchia

    You will need YamuxStream(IStream) and MplexStream(IStream) to support both Yamux(IMultiplexer) and Mplex(IMultiplexer)

    Hmm probably not. If you can abstract the communication between IStream and IMultiplexer enough, maybe IStream can totally be unware of what implementation of IMultiplexer is.

    Jonathan de Jong
    @ShadowJonathan
    Exactly
    That's the point I'm going with
    Kevin Mai-Husan Chia
    @mhchia
    hmm okay
    Jonathan de Jong
    @ShadowJonathan
    Yamux might indeed provide YamuxStream (or similar), but that class doesn't need to know that it's multiplexer is yamux specifically (unless it needs custom behaviour)
    I'm currently giving the entire spec document a read, though, and I already have a feeling that it'll shake some of my previously-established opinions on how to handle this
    Kevin Mai-Husan Chia
    @mhchia

    YamuxStream (or similar)

    So does Mplex

    I think we can just enforce IMultiplexer, which wraps all YamuxMultiplexer and YamuxStream logics inside
    And IStream send/receive messages through some kind of abstracted channels, multiplexer-agnostic

    I'm currently giving the entire spec document a read

    Cool

    And IStream send/receive messages through some kind of abstracted channels, multiplexer-agnostic

    At least I think this one is applicable.

    Jonathan de Jong
    @ShadowJonathan
    Yes
    So basically what this document is mentioning (native transport multiplexing) can just be a class that both inpliments IConnection and IMultiplexer at the same time
    IStream is essentially IConnection, with the write, read, and closes going to the multiplexer instead of the underlying transport
    For example, the persistent metadata component could automatically add peer ids and addresses to its registry whenever a new peer connects, or a DHT module could update its routing tables when a connection is terminated.
    To support this, it's recommended that libp2p implementations support a notification or event delivery system that can inform interested parties about connection lifecycle events.
    Kevin Mai-Husan Chia
    @mhchia

    IStream is essentially IConnection

    Yes

    Jonathan de Jong
    @ShadowJonathan
    Which IConnection could have in the form of event properties that are set when those events occur
    Or have occurred