libp2p / libp2p/go-libp2p

The Switch interface doesn't match the others

Open
#1,010 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

kind/enhancement
Dominant language
Go
Stars
6.9k
Forks
1.3k
Avg merge
13d 21h
Merged PRs (30d)
1

Description

The `Switch`, returned by `host.Mux()`, doesn't really fit the other libp2p interfaces. Reproduced below:

```go
type Switch interface {
// AddHandler registers the given handler to be invoked for
// an exact literal match of the given protocol ID string.
AddHandler(protocol string, handler HandlerFunc)

// AddHandlerWithFunc registers the given handler to be invoked
// when the provided match function returns true.
//
// The match function will be invoked with an incoming protocol
// ID string, and should return true if the handler supports
// the protocol. Note that the protocol ID argument is not
// used for matching; if you want to match the protocol ID
// string exactly, you must check for it in your match function.
AddHandlerWithFunc(protocol string, match func(string) bool, handler HandlerFunc)

// RemoveHandler removes the registered handler (if any) for the
// given protocol ID string.
RemoveHandler(protocol string)

// Protocols returns a list of all registered protocol ID strings.
// Note that the Router may be able to handle protocol IDs not
// included in this list if handlers were added with match functions
// using AddHandlerWithFunc.
Protocols() []string
// NegotiateLazy will return the registered protocol handler to use
// for a given inbound stream, returning as soon as the protocol has been
// determined. Returns an error if negotiation fails.
//
// NegotiateLazy may return before all protocol negotiation responses have been
// written to the stream. This is in contrast to Negotiate, which will block until
// the Negotiator is finished with the stream.
NegotiateLazy(rwc io.ReadWriteCloser) (io.ReadWriteCloser, string, HandlerFunc, error)

// Negotiate will return the registered protocol handler to use for a given
// inbound stream, returning after the protocol has been determined and the
// Negotiator has finished using the stream for negotiation. Returns an
// error if negotiation fails.
Negotiate(rwc io.ReadWriteCloser) (string, HandlerFunc, error)

// Handle calls Negotiate to determine which protocol handler to use for an
// inbound stream, then invokes the protocol handler function, passing it
// the protocol ID and the stream. Returns an error if negotiation fails.
Handle(rwc io.ReadWriteCloser) error
}
```

* AddHandler, RemoveHandler duplicate the functions on the host.
* All protocols are strings, not `protocol.ID` because that's what our go-multistream implementation uses (to avoid depending on libp2p).
* The negotiate functions aren't really safe. You actually need to pass them a `Stream`, otherwise they'll panic.

We should consider just moving the _required_ functions up one level into the host. Really, from what I can tell, we only need:

* `Protocols()` - needed by identify, at a minimum.
* `HandleStream(Stream)` - needed to feed a stream _back_ into the switch to negotiate a protocol on top of a protocol (e.g., /compress/gzip/1.0 -> compressor -> Handle(compressed) -> /bitswap/1.0).

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the Switch interface returned by host.Mux() and trace its callers, especially identify and the stream-handling path. Compare the duplicated handler and negotiation methods with the proposed Protocols() and HandleStream(Stream) responsibilities. Done means the required API boundary is agreed and the affected callers can use it without unsafe negotiation inputs.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
networking
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.