chafey
commented
4 years ago Notes from discussion with @hannahhoward
) go-graphsync design was based on bitswap implementation which sends blocks on a separate stream
) seems reasonable to be able to allow responses back over incoming stream - if we do this, go-graphsync should be updated to do this as well.
) go-graphsync will in some case send messages with request, response and data
) metadata extension provides mapping between responses and blocks!! this should be moved into the core protocol!!
) metadata extension also specifies whether or not responder has block!! - in this case, requester should not fail selector traversal validation...
) requester will silently ignore/discard blocks not referenced in the metadata!!
While implementing js-graphsync I was surprised to learn that go-ipfs/go-graphsync responder creates a separate libp2p stream to send message responses back to the requester on rather than re-use the incoming stream that the request messages came in on. I had assumed that the response messages would be sent back to the requester on the stream it created since it is a duplex stream. This behaviour should be documented as part of the spec with supporting rationale. It should also be clarified if responses can be sent back over the incoming stream, or if the streams are unidirectional only. It should also be clarified if a requester should allow multiple incoming streams from a single requester peer and if it can reuse the outbound stream to send messages responses for all incoming streams, or if separate outbound streams should be created for each incoming stream.
Related to this is how request message ids map to connections. I assume they are bound to the peerid, but again this should be clearly specified
Finally is how messages map blocks to responses. Right now the message can contain multiple responses and multiple blocks, but there is no connection between the blocks and responses leaving the responder to figure out how to dispatch/map them. This is problematic given the requirement for blocks to be sent in traversal order as the responder will have to check each request referenced by the response to determine if the block satisfies its in order selector traversal requirement. It was mentioned that a requester should hard abort the request/stream/connection if an out of traversal order block is received, but this requires extra logic requester side to handle due to the blocks not referencing the response they are associated with (imagine multiple responses for different requests in one message - a requester does not know which requester (or requesters!!?) a given block should be dispatched?)
Logging this issue for tracking purposes only, I will update the graphsync spec with this and other issues in batch later